Hi Peter,

I am using the user mode emulator to test the "target big endian" code path.

I apologize for the error, I have sent this content as a patch series to
make it easier to review!

Looking forward to your feedback!

Thanks,
Vacha

On Mon, Jul 21, 2025 at 10:36 AM Peter Maydell <peter.mayd...@linaro.org>
wrote:

> 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