On Fri, 14 Jul 2023 at 18:52, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > Hi Peter, > > On 14/7/23 19:26, Peter Maydell 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. > > > > (None of the other fields referenced by the r32[] array in > > sparc_tcg_init() have the wrong type.) > > > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > > --- > > Another in my occasional series of "fix an avocado failure on > > s390" Friday afternoon patches :-) > > :) > > > diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c > > index a1c8fdc4d55..bddb9609b7b 100644 > > --- a/target/sparc/gdbstub.c > > +++ b/target/sparc/gdbstub.c > > @@ -96,7 +96,10 @@ int sparc_cpu_gdb_read_register(CPUState *cs, GByteArray > > *mem_buf, int n) > > case 83: > > return gdb_get_regl(mem_buf, env->fsr); > > case 84: > > - return gdb_get_regl(mem_buf, env->fprs); > > + { > > + target_ulong fprs = env->fprs; > > + return gdb_get_regl(mem_buf, fprs); > > Why not return gdb_get_reg32() ?
Because that would cause different on-the-wire data to be sent to gdb -- gdb_get_reg32() puts 4 bytes of data into the gdb remote protocol packet, whereas gdb_get_regl() puts either 4 or 8 bytes depending on TARGET_LONG_BITS (as it happens, here we'll always send 8 because this register is sparc64- specific). Anyway, Richard is correct and we don't need to change this at all, because gdb_get_regl() takes an integer argument, it isn't a magic macro that implicitly takes the address or looks at the type of what it gets passed. So passing it env->fprs will zero-extend that and DTRT. thanks -- PMM