Re: [Qemu-devel] [PATCH v3 5/7] ARM big-endian system-mode gdbstub support.

[Peter Maydell]

On 20 January 2017 at 16:32, Julian Brown <address@hidden> wrote:
> When debugging a big-endian (either BE8 or BE32) executable, GDB uses
> a big-endian byte ordering for its remote protocol.  The gdb stub
> code in QEMU needs to interpret data in host (little-endian) order in
> arm_cpu_gdb_read_register and arm_cpu_gdb_write_register, so this patch
> arranges to byte-swap the data to/from GDB in those cases.
>
> Signed-off-by: Julian Brown <address@hidden>
> ---
>  target/arm/gdbstub.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
> index 04c1208..1e9fe68 100644
> --- a/target/arm/gdbstub.c
> +++ b/target/arm/gdbstub.c
> @@ -21,6 +21,7 @@
>  #include "qemu-common.h"
>  #include "cpu.h"
>  #include "exec/gdbstub.h"
> +#include "exec/softmmu-arm-semi.h"
>
>  /* Old gdb always expect FPA registers.  Newer (xml-aware) gdb only expect
>     whatever the target description contains.  Due to a historical mishap
> @@ -32,10 +33,22 @@ int arm_cpu_gdb_read_register(CPUState *cs, uint8_t 
> *mem_buf, int n)
>  {
>      ARMCPU *cpu = ARM_CPU(cs);
>      CPUARMState *env = &cpu->env;
> +#ifndef CONFIG_USER_ONLY
> +    bool targ_bigendian = arm_bswap_needed(env);
> +#endif

This is a "what is the state of the CPU right this instant" test,
but surely gdb doesn't flip its protocol definition as the CPU
flips between big and little endian at runtime? I'm not sure
what the right check is but it probably isn't this.

>
>      if (n < 16) {
>          /* Core integer register.  */
> +#ifdef CONFIG_USER_ONLY
>          return gdb_get_reg32(mem_buf, env->regs[n]);
> +#else
> +        if (targ_bigendian) {
> +            stl_be_p(mem_buf, env->regs[n]);
> +        } else {
> +            stl_le_p(mem_buf, env->regs[n]);
> +        }
> +        return 4;
> +#endif

There's probably a phrasing here that avoids the ifdeffery...

thanks
-- PMM