Re: [PATCH v3 7/7] hw/ppc/epapr: Do not swap ePAPR magic value

[BALATON Zoltan]

On Wed, 18 Dec 2024, Philippe Mathieu-Daudé wrote:
> The ePAPR magic value in $r6 doesn't need to be byte swapped.
>
> See ePAPR-v1.1.pdf chapter 5.4.1 "Boot CPU Initial Register State"
> and the following mailing-list thread:
> https://lore.kernel.org/qemu-devel/CAFEAcA_NR4XW5DNL4nq7vnH4XRH5UWbhQCxuLyKqYk6_FCBrAA@mail.gmail.com/
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> hw/ppc/sam460ex.c     | 2 +-
> hw/ppc/virtex_ml507.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
> index 78e2a46e753..db9c8f3fa6e 100644
> --- a/hw/ppc/sam460ex.c
> +++ b/hw/ppc/sam460ex.c
> @@ -234,7 +234,7 @@ static void main_cpu_reset(void *opaque)
>
>         /* Create a mapping for the kernel.  */
>         booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1 << 31);
> -        env->gpr[6] = tswap32(EPAPR_MAGIC);
> +        env->gpr[6] = EPAPR_MAGIC;

I don't know how to test this (or if anything actually uses it). What I'm 
not sure about is what endianness env->gpr is? It's a host array so maybe 
it needs to match the host endianness which is little endian most of the 
time as opposed to PPC big endian on this machine. So maybe tswap is wrong 
but is removing it right? Maybe we need to only swap on LE hosts. I think 
it's only used by Linux kernels so maybe trying to boot one could test 
this change but I'm not sure.

Regards,
BALATON Zoltan

>         env->gpr[7] = (16 * MiB) - 8; /* bi->ima_size; */
>
>     } else {
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index f378e5c4a90..6197d31d88f 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -119,7 +119,7 @@ static void main_cpu_reset(void *opaque)
>     /* Create a mapping spanning the 32bit addr space. */
>     booke_set_tlb(&env->tlb.tlbe[0], 0, 0, 1U << 31);
>     booke_set_tlb(&env->tlb.tlbe[1], 0x80000000, 0x80000000, 1U << 31);
> -    env->gpr[6] = tswap32(EPAPR_MAGIC);
> +    env->gpr[6] = EPAPR_MAGIC;
>     env->gpr[7] = bi->ima_size;
> }