On Fri, 18 Jul 2025 at 20:20, Vacha Bhavsar
<vacha.bhav...@oss.qualcomm.com> wrote:
>
> Upon examining the current implementation for getting/setting SIMD
> and SVE registers via remote GDB, there is a concern about mixed
> endian support.

Yes, we discussed this on a different thread.

> The current implementation for the SIMD functionality for getting and setting
> registers via the gdbstub is implemented as follows:
>
> aarch64_gdb_set_fpu_reg:
> <omitted code>
> uint64_t *q = aa64_vfp_qreg(env, reg);
> q[0] = ldq_le_p(buf);
> q[1] = ldq_le_p(buf + 8);
> return 16;
> <omitted code>
>
> The following code is a suggested fix for the current implementation that
> should allow for mixed endian support for getting/setting SIMD registers
> via the remote GDB protocol.
>
> aarch64_gdb_set_fpu_reg:
> <omitted code>
> // case 0...31

It would be easier to review your proposed changes if you'd
sent them in a proper patch format...

> uint64_t *q = aa64_vfp_qreg(env, reg);
> if (target_big_endian()){

What config are you using to test this "target big endian"
code path? On system emulation target/arm is *always*
target_big_endian() == false (even if CPSR.E is set).
The QEMU "target endianness" is a compile-time property of the
system, not a runtime property of the CPU. CPSR.E just causes us to
byteswap values as they go out from the CPU to the memory subsystem.

I think for the user mode emulator aarch64_be-linux-user
we may have target_big_endian() true, but I forget. BE
linux-user is pretty unmaintained...

> q[1] = ldq_p(buf);
>         q[0] = ldq_p(buf + 8);
> }
> else{
> q[0] = ldq_p(buf);
>         q[1] = ldq_p(buf);

(You're loading q[0] and q[1] from the same buffer address here.)

> }> return 16;
> <omitted code>
>
> This use of ldq_p rather than ldq_le_p (which the current implementation
> uses) to load bytes into host endian struct is inspired by the current
> implementation for getting/setting general purpose registers via remote
> GDB (which works appropriately regardless of target endianness), as well
> as the current implementation for getting/setting gprs via GDB with ppc
> as a target (refer to ppc_cpu_gdb_write_register() for example).

Note that for PPC, we build both ppc64-softmmu and ppc64le-softmmu configs,
so for PPC target_endianness() really can be either LE or BE even
in system emulation mode.

> For SVE, the current implementation is as follows for the zregs:
>
> aarch64_gdb_set_sve_reg:
> <omitted code>
> // case 0...31
> int vq, len = 0;
> uint64_t *p = (uint64_t *) buf;
> for (vq = 0; vq < cpu->sve_max_vq; vq++) {
> env->vfp.zregs[reg].d[vq * 2 + 1] = *p++;
>         env->vfp.zregs[reg].d[vq * 2] = *p++;
>         len += 16;
> }

This code definitely is wrong, because the gdbstub protocol buffer
is target-endianness and the env->vfp.zregs values are host-endianness.

thanks
-- PMM

Reply via email to