Re: [Qemu-devel] Re: [PATCH] x86_64 debug registers for gdb

[Jan Kiszka]

Paul Brook wrote:
>>> These are all wrong on x86-64.
>> Then what about pointing me to the correct information to fix this, hmm?
>>  I browsed gdb code and docs but found no obvious packet format
>> description. Constructive help is welcome, likely I'm just blind.
> 
> gdb/amd64-tdep.c is the authorative source. Specifically amd64_register_info.
> Similar information also in gdb/regformats/reg-x86-64.dat
> 

Yep, I was blind.

Here comes another try. Please have a look/comment on the following
specifically:

 - Why is there no fpu-related code in cpu_gdb_write_registers()? Is fpu
   debugging only partially supported yet?
 - I included xmm and mxcsr regs into cpu_gdb_read_registers(), but I'm
   not sure if it's correct (byte ordering, format of mxcsr)
 - If I understood gdb correctly, fpu registers (st0..7) are always
   encoded in an arch-neutral format, no conversion on exchange is
   needed. Thus, is it correct to remove the related "XXX" comment from
   cpu_gdb_read_registers()?
 - Would it be simple to get the remaining fpu control registers right?
   Just to take the chance and finalise this code if we are already at
   it.

Jan


---
 gdbstub.c |  118 ++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 73 insertions(+), 45 deletions(-)

Index: qemu-0.9.0/gdbstub.c
===================================================================
--- qemu-0.9.0.orig/gdbstub.c
+++ qemu-0.9.0/gdbstub.c
@@ -222,64 +222,92 @@ static int put_packet(GDBState *s, char 
 }
 
 #if defined(TARGET_I386)
+struct x86_gdb_regs {
+    target_ulong gp[CPU_NB_REGS];
+    target_ulong ip;
+    uint32_t flags;
+    uint32_t segm[6];
+    uint8_t fpreg[8][10];
+    uint32_t fpctrl[8];
+    uint64_t xmm[CPU_NB_REGS][2];
+    uint32_t mxcsr;
+} __attribute__((packed));
 
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
-    uint32_t *registers = (uint32_t *)mem_buf;
+    struct x86_gdb_regs *regs = (struct x86_gdb_regs *)mem_buf;
     int i, fpus;
 
-    for(i = 0; i < 8; i++) {
-        registers[i] = env->regs[i];
-    }
-    registers[8] = env->eip;
-    registers[9] = env->eflags;
-    registers[10] = env->segs[R_CS].selector;
-    registers[11] = env->segs[R_SS].selector;
-    registers[12] = env->segs[R_DS].selector;
-    registers[13] = env->segs[R_ES].selector;
-    registers[14] = env->segs[R_FS].selector;
-    registers[15] = env->segs[R_GS].selector;
-    /* XXX: convert floats */
-    for(i = 0; i < 8; i++) {
-        memcpy(mem_buf + 16 * 4 + i * 10, &env->fpregs[i], 10);
-    }
-    registers[36] = env->fpuc;
+    for(i = 0; i < CPU_NB_REGS; i++)
+        regs->gp[i] = tswapl(env->regs[i]);
+#if defined(TARGET_X86_64)
+    /* Fix-up register mapping */
+    regs->gp[1] = tswapl(env->regs[R_EBX]);
+    regs->gp[2] = tswapl(env->regs[R_ECX]);
+    regs->gp[3] = tswapl(env->regs[R_EDX]);
+#endif
+
+    regs->ip    = tswapl(env->eip);
+    regs->flags = tswap32(env->eflags);
+
+    regs->segm[0] = tswap32(env->segs[R_CS].selector);
+    regs->segm[1] = tswap32(env->segs[R_SS].selector);
+    regs->segm[2] = tswap32(env->segs[R_DS].selector);
+    regs->segm[3] = tswap32(env->segs[R_ES].selector);
+    regs->segm[4] = tswap32(env->segs[R_FS].selector);
+    regs->segm[5] = tswap32(env->segs[R_GS].selector);
+
+    for (i = 0; i < 8; i++)
+        memcpy(regs->fpreg[i], &env->fpregs[i], 10);
+
+    regs->fpctrl[0] = tswap32(env->fpuc);
     fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
-    registers[37] = fpus;
-    registers[38] = 0; /* XXX: convert tags */
-    registers[39] = 0; /* fiseg */
-    registers[40] = 0; /* fioff */
-    registers[41] = 0; /* foseg */
-    registers[42] = 0; /* fooff */
-    registers[43] = 0; /* fop */
-    
-    for(i = 0; i < 16; i++)
-        tswapls(&registers[i]);
-    for(i = 36; i < 44; i++)
-        tswapls(&registers[i]);
-    return 44 * 4;
+    regs->fpctrl[1] = tswap32(fpus);
+    regs->fpctrl[2] = 0; /* XXX: convert tags */
+    regs->fpctrl[3] = 0; /* fiseg */
+    regs->fpctrl[4] = 0; /* fioff */
+    regs->fpctrl[5] = 0; /* foseg */
+    regs->fpctrl[6] = 0; /* fooff */
+    regs->fpctrl[7] = 0; /* fop */
+
+    for (i = 0; i < CPU_NB_REGS; i++) {
+        regs->xmm[i][0] = tswap64(&env->xmm_regs[i].XMM_Q(0));
+        regs->xmm[i][1] = tswap64(&env->xmm_regs[i].XMM_Q(1));
+    }
+
+    regs->mxcsr = tswap32(env->mxcsr);
+
+    return sizeof(struct x86_gdb_regs);
 }
 
 static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
 {
-    uint32_t *registers = (uint32_t *)mem_buf;
+    struct x86_gdb_regs *regs = (struct x86_gdb_regs *)mem_buf;
     int i;
 
-    for(i = 0; i < 8; i++) {
-        env->regs[i] = tswapl(registers[i]);
-    }
-    env->eip = tswapl(registers[8]);
-    env->eflags = tswapl(registers[9]);
+    for (i = 0; i < CPU_NB_REGS; i++)
+        env->regs[i] = tswapl(regs->gp[i]);
+#if defined(TARGET_X86_64)
+    /* Fix-up register mapping */
+    env->regs[R_EBX] = tswapl(regs->gp[1]);
+    env->regs[R_ECX] = tswapl(regs->gp[2]);
+    env->regs[R_EDX] = tswapl(regs->gp[3]);
+#endif
+
+    env->eip    = tswapl(regs->ip);
+    env->eflags = tswap32(regs->flags);
+
 #if defined(CONFIG_USER_ONLY)
-#define LOAD_SEG(index, sreg)\
-            if (tswapl(registers[index]) != env->segs[sreg].selector)\
-                cpu_x86_load_seg(env, sreg, tswapl(registers[index]));
-            LOAD_SEG(10, R_CS);
-            LOAD_SEG(11, R_SS);
-            LOAD_SEG(12, R_DS);
-            LOAD_SEG(13, R_ES);
-            LOAD_SEG(14, R_FS);
-            LOAD_SEG(15, R_GS);
+#define LOAD_SEG(index, sreg) \
+    if (tswap32(regs->segm[index]) != env->segs[sreg].selector) \
+        cpu_x86_load_seg(env, sreg, tswap32(regs->segm[index]));
+
+    LOAD_SEG(0, R_CS);
+    LOAD_SEG(1, R_SS);
+    LOAD_SEG(2, R_DS);
+    LOAD_SEG(3, R_ES);
+    LOAD_SEG(4, R_FS);
+    LOAD_SEG(5, R_GS);
 #endif
 }

signature.asc
Description: OpenPGP digital signature