Re: [PATCH] target/sparc: Handle FPRS correctly on big-endian hosts

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 14 Jul 2023 at 18:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In CPUSparcState we define the fprs field as uint64_t.  However we
> then refer to it in translate.c via a TCGv_i32 which we set up with
> tcg_global_mem_new_ptr().  This means that on a big-endian host when
> the guest does something to writo te the FPRS register this value
> ends up in the wrong half of the uint64_t, and the QEMU C code that
> refers to env->fprs sees the wrong value.  The effect of this is that
> guest code that enables the FPU crashes with spurious FPU Disabled
> exceptions.  In particular, this is why
>  tests/avocado/machine_sparc64_sun4u.py:Sun4uMachine.test_sparc64_sun4u
> times out on an s390 host.
>
> There are multiple ways we could fix this; since there are actually
> only three bits in the FPRS register and the code in translate.c
> would be a bit painful to convert to dealing with a TCGv_i64, change
> the type of the CPU state struct field to match what translate.c is
> expecting.

> diff --git a/target/sparc/machine.c b/target/sparc/machine.c
> index 44b9e7d75d6..5a8502804a4 100644
> --- a/target/sparc/machine.c
> +++ b/target/sparc/machine.c
> @@ -168,7 +168,8 @@ const VMStateDescription vmstate_sparc_cpu = {
>          VMSTATE_UINT64_ARRAY(env.bgregs, SPARCCPU, 8),
>          VMSTATE_UINT64_ARRAY(env.igregs, SPARCCPU, 8),
>          VMSTATE_UINT64_ARRAY(env.mgregs, SPARCCPU, 8),
> -        VMSTATE_UINT64(env.fprs, SPARCCPU),
> +        VMSTATE_UINT32(env.fprs, SPARCCPU),
> +        VMSTATE_UNUSED(4), /* was unused high half of uint64_t fprs */

Whoops, these two fields are the wrong way around. The on-the-wire
format for migration is always big-endian, so we need to put
the VMSTATE_UNUSED() placeholder before the VMSTATE_UINT32(),
not after it.

thanks
-- PMM