On Mon, 30 Sept 2024 at 23:12, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
>
> Replace the target-specific tswap32() calls by stl_endian_p()
> which does the same but takes the endianness as argument, thus
> is target-agnostic.
> Get the vCPU endianness calling arm_cpu_code_is_big_endian().
>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
> ---
>  hw/arm/boot.c        | 8 +++++---
>  hw/arm/exynos4210.c  | 7 +++----
>  hw/arm/npcm7xx.c     | 6 ++++--
>  hw/arm/xilinx_zynq.c | 5 +++--
>  4 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 6efd21f9c2..6e8dc00e6d 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -137,6 +137,7 @@ void arm_write_bootloader(const char *name,
>                            const uint32_t *fixupcontext)
>  {
>      AddressSpace *as = arm_boot_address_space(cpu, info);
> +    bool be = arm_cpu_code_is_big_endian(&cpu->env);
>      /* Fix up the specified bootloader fragment and write it into
>       * guest memory using rom_add_blob_fixed(). fixupcontext is
>       * an array giving the values to write in for the fixup types
> @@ -173,7 +174,7 @@ void arm_write_bootloader(const char *name,
>          default:
>              abort();
>          }
> -        code[i] = tswap32(insn);
> +        stl_endian_p(be, &code[i], insn);
>      }

This is a behaviour change. For Arm, TARGET_BIG_ENDIAN
is always false, so tswap32() is "swap to/from little endian".
But arm_cpu_code_is_big_endian() looks at the state of
the guest vCPU (specifically, its SCTLR.B bit) and so
may swap to either big or little endian.

These functions are also called before the CPU is
reset for the first time, and before do_cpu_reset()
has maybe set SCTLR_B based on the ELF file. So we
can't guarantee SCTLR.B to be set correctly here where
we're trying to use it.

Maybe we do get this wrong for the old ARMv6-and-earlier
BE32 model where SCTLR.B might be non-zero -- they're
such a niche case that I don't suppose gets tested often.
(ARMv7-and-up is BE8 and instructions are always little
endian even when data is big endian.) But we should
separate out bug fixes from refactorings.

thanks
-- PMM

Reply via email to