On 16/7/23 19:32, Peter Maydell wrote:
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).
Right, I missed the TARGET_LONG_BITS part.
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.
OK.
