[PATCH v6 08/22] target/i386: use gdb_get_reg helpers

Alex Bennée posted 22 patches 6 years ago
[PATCH v6 08/22] target/i386: use gdb_get_reg helpers
Posted by Alex Bennée 6 years ago
This is cleaner than poking memory directly and will make later
clean-ups easier.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/i386/gdbstub.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index 572ead641ca..208b3de70da 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -95,29 +95,26 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
             if (env->hflags & HF_CS64_MASK) {
                 return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]);
             } else if (n < CPU_NB_REGS32) {
+
                 return gdb_get_reg64(mem_buf,
                                      env->regs[gpr_map[n]] & 0xffffffffUL);
             } else {
-                memset(mem_buf, 0, sizeof(target_ulong));
-                return sizeof(target_ulong);
+                return gdb_get_regl(mem_buf, 0);
             }
         } else {
             return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
         }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-#ifdef USE_X86LDOUBLE
-        /* FIXME: byteswap float values - after fixing fpregs layout. */
-        memcpy(mem_buf, &env->fpregs[n - IDX_FP_REGS], 10);
-#else
-        memset(mem_buf, 0, 10);
-#endif
+        guint8 *data = (guint8 *) &env->fpregs[n - IDX_FP_REGS];
+        memcpy(mem_buf, data, 10);
+        /* byte swap? this is at least correct of x86-on-x86 */
         return 10;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
         if (n < CPU_NB_REGS32 || TARGET_LONG_BITS == 64) {
-            stq_p(mem_buf, env->xmm_regs[n].ZMM_Q(0));
-            stq_p(mem_buf + 8, env->xmm_regs[n].ZMM_Q(1));
-            return 16;
+            return gdb_get_reg128(mem_buf,
+                                  env->xmm_regs[n].ZMM_Q(0),
+                                  env->xmm_regs[n].ZMM_Q(1));
         }
     } else {
         switch (n) {
@@ -290,10 +287,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
             return 4;
         }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
-#ifdef USE_X86LDOUBLE
-        /* FIXME: byteswap float values - after fixing fpregs layout. */
         memcpy(&env->fpregs[n - IDX_FP_REGS], mem_buf, 10);
-#endif
         return 10;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
-- 
2.20.1


Re: [PATCH v6 08/22] target/i386: use gdb_get_reg helpers
Posted by Richard Henderson 6 years ago
On 2/5/20 5:10 PM, Alex Bennée wrote:
> This is cleaner than poking memory directly and will make later
> clean-ups easier.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  target/i386/gdbstub.c | 22 ++++++++--------------
>  1 file changed, 8 insertions(+), 14 deletions(-)
> 
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index 572ead641ca..208b3de70da 100644
> --- a/target/i386/gdbstub.c
> +++ b/target/i386/gdbstub.c
> @@ -95,29 +95,26 @@ int x86_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
>              if (env->hflags & HF_CS64_MASK) {
>                  return gdb_get_reg64(mem_buf, env->regs[gpr_map[n]]);
>              } else if (n < CPU_NB_REGS32) {
> +
>                  return gdb_get_reg64(mem_buf,
>                                       env->regs[gpr_map[n]] & 0xffffffffUL);

Watch the stray spaces.

>      } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
> -#ifdef USE_X86LDOUBLE
> -        /* FIXME: byteswap float values - after fixing fpregs layout. */
> -        memcpy(mem_buf, &env->fpregs[n - IDX_FP_REGS], 10);
> -#else
> -        memset(mem_buf, 0, 10);
> -#endif
> +        guint8 *data = (guint8 *) &env->fpregs[n - IDX_FP_REGS];
> +        memcpy(mem_buf, data, 10);
> +        /* byte swap? this is at least correct of x86-on-x86 */

Yes, byte swap.

typedef struct {
    uint64_t low;
    uint16_t high;
} floatx80;

Output little-endian low, followed by little-endian high.

> @@ -290,10 +287,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>              return 4;
>          }
>      } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
> -#ifdef USE_X86LDOUBLE
> -        /* FIXME: byteswap float values - after fixing fpregs layout. */
>          memcpy(&env->fpregs[n - IDX_FP_REGS], mem_buf, 10);
> -#endif
>          return 10;

Similarly.


r~