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

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

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

This patch fixes the registers for the 'g' and 'G' packets for the 
qemu-system-x86_64 target.  It allows gdb 6.5 to debug a linux kernel 
and get a stack back trace.

signed-off-by: jason.wessel@windriver.com

Thanks,
Jason.

[-- Attachment #2: x86-64_gdb.patch --]
[-- Type: text/plain, Size: 2151 bytes --]

Index: qemu/gdbstub.c
===================================================================
--- qemu.orig/gdbstub.c
+++ qemu/gdbstub.c
@@ -184,9 +184,70 @@ static int put_packet(GDBState *s, char 
     }
     return 0;
 }
+#if defined(TARGET_X86_64)
+/* Defines from GDB register struct numbers */
+#define _RAX 0
+#define _RDX 1
+#define _RCX 2
+#define _RBX 3
+#define _RSI 4
+#define _RDI 5
+#define _RBP 6
+#define _RSP 7
+#define _R8  8
+#define _R9  9
+#define _R10 10
+#define _R11 11
+#define _R12 12
+#define _R13 13
+#define _R14 14
+#define _R15 15
+#define _PC  16
+#define _PS  17
 
-#if defined(TARGET_I386)
+static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
+{
+    uint64_t *registers = (uint64_t *)mem_buf;
+    int i;
+
+    registers[_RAX] = env->regs[R_EAX];
+    registers[_RBX] = env->regs[R_EBX];
+    registers[_RCX] = env->regs[R_ECX];
+    registers[_RDX] = env->regs[R_EDX];
+    registers[_RSI] = env->regs[R_ESI];
+    registers[_RDI] = env->regs[R_EDI];
+    registers[_RBP] = env->regs[R_EBP];
+    registers[_RSP] = env->regs[R_ESP];
+    for (i = 8; i < 16; i++)
+        registers[i] = env->regs[i];
+    registers[_PC] = env->eip;
+    registers[_PS] = env->eflags;
+    for(i = 0; i < 18; i++)
+        tswapl(registers[i]);
+
+    return 18 * 8;
+}
+
+static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
+{
+    uint32_t *registers = (uint32_t *)mem_buf;
+    int i;
+
+    env->regs[R_EAX] = tswapl(registers[_RAX]);
+    env->regs[R_EBX] = tswapl(registers[_RBX]);
+    env->regs[R_ECX] = tswapl(registers[_RCX]);
+    env->regs[R_EDX] = tswapl(registers[_RDX]);
+    env->regs[R_ESI] = tswapl(registers[_RSI]);
+    env->regs[R_EDI] = tswapl(registers[_RDI]);
+    env->regs[R_EBP] = tswapl(registers[_RBP]);
+    env->regs[R_ESP] = tswapl(registers[_RSP]);
+    for (i = 8; i < 16; i++)
+        env->regs[i] = tswapl(registers[i]);
+    env->eip = tswapl(registers[_PC]);
+    env->eflags = tswapl(registers[_PS]);
+}
 
+#elif defined(TARGET_I386)
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
     uint32_t *registers = (uint32_t *)mem_buf;

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3048 bytes --]

Jason Wessel wrote:
> This patch fixes the registers for the 'g' and 'G' packets for the
> qemu-system-x86_64 target.  It allows gdb 6.5 to debug a linux kernel
> and get a stack back trace.

Here comes a corrected (RBX and RDX were mixed) and slightly enhanced
(segment register reading, don't know how writing should look like)
version of this patch. Tested successfully with qemu-0.9.0 and gdb-6.6.
Would be nice to see this support in the next qemu release.

Jan


Index: qemu-0.9.0/gdbstub.c
===================================================================
--- qemu-0.9.0.orig/gdbstub.c
+++ qemu-0.9.0/gdbstub.c
@@ -220,9 +220,78 @@ static int put_packet(GDBState *s, char 
     }
     return 0;
 }
+#if defined(TARGET_X86_64)
+/* Defines from GDB register struct numbers */
+#define _RAX	0
+#define _RBX	1
+#define _RCX	2
+#define _RDX	3
+#define _RSI	4
+#define _RDI	5
+#define _RBP	6
+#define _RSP	7
+#define _R8	8
+#define _R15	15
+#define _PC	16
+#define _PS	17
+#define _CS	18
+#define _SS	19
+#define _DS	20
+#define _ES	21
+#define _FS	22
+#define _GS	23
+#define _NREGS	24
 
-#if defined(TARGET_I386)
+static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
+{
+    uint64_t *registers = (uint64_t *)mem_buf;
+    int i;
+
+    registers[_RAX] = env->regs[R_EAX];
+    registers[_RBX] = env->regs[R_EBX];
+    registers[_RCX] = env->regs[R_ECX];
+    registers[_RDX] = env->regs[R_EDX];
+    registers[_RSI] = env->regs[R_ESI];
+    registers[_RDI] = env->regs[R_EDI];
+    registers[_RBP] = env->regs[R_EBP];
+    registers[_RSP] = env->regs[R_ESP];
+    for (i = 8; i < 16; i++)
+        registers[i] = env->regs[i];
+    registers[_PC] = env->eip;
+    registers[_PS] = env->eflags;
+    registers[_CS] = env->segs[R_CS].selector;
+    registers[_SS] = env->segs[R_SS].selector;
+    registers[_DS] = env->segs[R_DS].selector;
+    registers[_ES] = env->segs[R_ES].selector;
+    registers[_FS] = env->segs[R_FS].selector;
+    registers[_GS] = env->segs[R_GS].selector;
+
+    for(i = 0; i < _NREGS; i++)
+        tswapl(registers[i]);
+
+    return _NREGS * 8;
+}
+
+static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
+{
+    uint32_t *registers = (uint32_t *)mem_buf;
+    int i;
+
+    env->regs[R_EAX] = tswapl(registers[_RAX]);
+    env->regs[R_EBX] = tswapl(registers[_RBX]);
+    env->regs[R_ECX] = tswapl(registers[_RCX]);
+    env->regs[R_EDX] = tswapl(registers[_RDX]);
+    env->regs[R_ESI] = tswapl(registers[_RSI]);
+    env->regs[R_EDI] = tswapl(registers[_RDI]);
+    env->regs[R_EBP] = tswapl(registers[_RBP]);
+    env->regs[R_ESP] = tswapl(registers[_RSP]);
+    for (i = 8; i < 16; i++)
+        env->regs[i] = tswapl(registers[i]);
+    env->eip = tswapl(registers[_PC]);
+    env->eflags = tswapl(registers[_PS]);
+}
 
+#elif defined(TARGET_I386)
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
     uint32_t *registers = (uint32_t *)mem_buf;



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

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

[Paul Brook]

On Wednesday 18 April 2007 21:53, Jan Kiszka wrote:
> Jason Wessel wrote:
> > This patch fixes the registers for the 'g' and 'G' packets for the
> > qemu-system-x86_64 target.  It allows gdb 6.5 to debug a linux kernel
> > and get a stack back trace.
>
> Here comes a corrected (RBX and RDX were mixed) and slightly enhanced
> (segment register reading, don't know how writing should look like)
> version of this patch. Tested successfully with qemu-0.9.0 and gdb-6.6.
> Would be nice to see this support in the next qemu release.

Can't you recycle the i386 code, like other 64-bit targets do?

Paul

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 5403 bytes --]

Paul Brook wrote:
> On Wednesday 18 April 2007 21:53, Jan Kiszka wrote:
>> Jason Wessel wrote:
>>> This patch fixes the registers for the 'g' and 'G' packets for the
>>> qemu-system-x86_64 target.  It allows gdb 6.5 to debug a linux kernel
>>> and get a stack back trace.
>> Here comes a corrected (RBX and RDX were mixed) and slightly enhanced
>> (segment register reading, don't know how writing should look like)
>> version of this patch. Tested successfully with qemu-0.9.0 and gdb-6.6.
>> Would be nice to see this support in the next qemu release.
> 
> Can't you recycle the i386 code, like other 64-bit targets do?
> 

For sure I can :). Is this version better? According to a quick check, it gives identical binaries on i386 and appears to work fine on x86_64.

Jan


---
 gdbstub.c |   99 ++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 62 insertions(+), 37 deletions(-)

Index: qemu-0.9.0/gdbstub.c
===================================================================
--- qemu-0.9.0.orig/gdbstub.c
+++ qemu-0.9.0/gdbstub.c
@@ -223,63 +223,88 @@ static int put_packet(GDBState *s, char 
 
 #if defined(TARGET_I386)
 
+#if defined(TARGET_X86_64)
+#define _GP_REGS    16
+#define _FPCTL_OFS  34
+#define _NREGS      42
+#else /* i386 */
+#define _GP_REGS    8
+#define _FPCTL_OFS  36
+#define _NREGS      44
+#endif
+
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
-    uint32_t *registers = (uint32_t *)mem_buf;
+    target_ulong *registers = (target_ulong *)mem_buf;
     int i, fpus;
 
-    for(i = 0; i < 8; i++) {
+    for(i = 0; i < _GP_REGS; 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;
+#if defined(TARGET_X86_64)
+    /* Fix-up register mapping */
+    registers[1] = env->regs[R_EBX];
+    registers[2] = env->regs[R_ECX];
+    registers[3] = env->regs[R_EDX];
+#endif
+
+    registers[_GP_REGS]   = env->eip;
+    registers[_GP_REGS+1] = env->eflags;
+
+    registers[_GP_REGS+2] = env->segs[R_CS].selector;
+    registers[_GP_REGS+3] = env->segs[R_SS].selector;
+    registers[_GP_REGS+4] = env->segs[R_DS].selector;
+    registers[_GP_REGS+5] = env->segs[R_ES].selector;
+    registers[_GP_REGS+6] = env->segs[R_FS].selector;
+    registers[_GP_REGS+7] = 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 < 8; i++)
+        memcpy(mem_buf + (_GP_REGS+8) * sizeof(target_ulong) + i * 10,
+               &env->fpregs[i], 10);
+
+    registers[_FPCTL_OFS] = 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++)
+    registers[_FPCTL_OFS+1] = fpus;
+    registers[_FPCTL_OFS+2] = 0; /* XXX: convert tags */
+    registers[_FPCTL_OFS+3] = 0; /* fiseg */
+    registers[_FPCTL_OFS+4] = 0; /* fioff */
+    registers[_FPCTL_OFS+5] = 0; /* foseg */
+    registers[_FPCTL_OFS+6] = 0; /* fooff */
+    registers[_FPCTL_OFS+7] = 0; /* fop */
+
+    for (i = 0; i < _GP_REGS+8; i++)
         tswapls(&registers[i]);
-    for(i = 36; i < 44; i++)
+    for (i = _FPCTL_OFS; i < _FPCTL_OFS+8; i++)
         tswapls(&registers[i]);
-    return 44 * 4;
+    return _NREGS * sizeof(target_ulong);
 }
 
 static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
 {
-    uint32_t *registers = (uint32_t *)mem_buf;
+    target_ulong *registers = (target_ulong *)mem_buf;
     int i;
 
-    for(i = 0; i < 8; i++) {
+    for(i = 0; i < _GP_REGS; i++)
         env->regs[i] = tswapl(registers[i]);
-    }
-    env->eip = tswapl(registers[8]);
-    env->eflags = tswapl(registers[9]);
+#if defined(TARGET_X86_64)
+    /* Fix-up register mapping */
+    env->regs[R_EBX] = tswapl(registers[1]);
+    env->regs[R_ECX] = tswapl(registers[2]);
+    env->regs[R_EDX] = tswapl(registers[3]);
+#endif
+
+    env->eip = tswapl(registers[_GP_REGS]);
+    env->eflags = tswapl(registers[_GP_REGS+1]);
 #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);
+            LOAD_SEG(_GP_REGS+2, R_CS);
+            LOAD_SEG(_GP_REGS+3, R_SS);
+            LOAD_SEG(_GP_REGS+4, R_DS);
+            LOAD_SEG(_GP_REGS+5, R_ES);
+            LOAD_SEG(_GP_REGS+6, R_FS);
+            LOAD_SEG(_GP_REGS+7, R_GS);
 #endif
 }
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

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

[Paul Brook]

On Thursday 19 April 2007 22:30, Jan Kiszka wrote:
> +    registers[_GP_REGS+2] = env->segs[R_CS].selector;
> +    registers[_GP_REGS+3] = env->segs[R_SS].selector;
> +    registers[_GP_REGS+4] = env->segs[R_DS].selector;
> +    registers[_GP_REGS+5] = env->segs[R_ES].selector;
> +    registers[_GP_REGS+6] = env->segs[R_FS].selector;
> +    registers[_GP_REGS+7] = env->segs[R_GS].selector;
>
> +    registers[_FPCTL_OFS] = env->fpuc;
> +    registers[_FPCTL_OFS+1] = fpus;
> +    registers[_FPCTL_OFS+2] = 0; /* XXX: convert tags */
> +    registers[_FPCTL_OFS+3] = 0; /* fiseg */
> +    registers[_FPCTL_OFS+4] = 0; /* fioff */
> +    registers[_FPCTL_OFS+5] = 0; /* foseg */
> +    registers[_FPCTL_OFS+6] = 0; /* fooff */
> +    registers[_FPCTL_OFS+7] = 0; /* fop */
> +    for (i = _FPCTL_OFS; i < _FPCTL_OFS+8; i++)
>          tswapls(&registers[i]);

These are all wrong on x86-64.

Paul

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

Paul Brook wrote:
> On Thursday 19 April 2007 22:30, Jan Kiszka wrote:
>> +    registers[_GP_REGS+2] = env->segs[R_CS].selector;
>> +    registers[_GP_REGS+3] = env->segs[R_SS].selector;
>> +    registers[_GP_REGS+4] = env->segs[R_DS].selector;
>> +    registers[_GP_REGS+5] = env->segs[R_ES].selector;
>> +    registers[_GP_REGS+6] = env->segs[R_FS].selector;
>> +    registers[_GP_REGS+7] = env->segs[R_GS].selector;
>>
>> +    registers[_FPCTL_OFS] = env->fpuc;
>> +    registers[_FPCTL_OFS+1] = fpus;
>> +    registers[_FPCTL_OFS+2] = 0; /* XXX: convert tags */
>> +    registers[_FPCTL_OFS+3] = 0; /* fiseg */
>> +    registers[_FPCTL_OFS+4] = 0; /* fioff */
>> +    registers[_FPCTL_OFS+5] = 0; /* foseg */
>> +    registers[_FPCTL_OFS+6] = 0; /* fooff */
>> +    registers[_FPCTL_OFS+7] = 0; /* fop */
>> +    for (i = _FPCTL_OFS; i < _FPCTL_OFS+8; i++)
>>          tswapls(&registers[i]);
> 
> 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.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

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

[Paul Brook]

> > 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

Paul

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 6088 bytes --]

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
 }
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 5170 bytes --]

Jan Kiszka wrote:
> +    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));
> +    }

Copy&paste nonsense. Corrected version attached.

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
 }
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 5230 bytes --]

Hi again,

here is now a version that I tested a bit longer than 5 minutes (it
actually helped to nail down a Xenomai kernel bug).

The general purpose register mapping for x86_64 was still wrong. I'm now
directing the indices through a mapping table because I didn't dare to
redefine R_EBX&friends on x86_64. The latter would be the simplest way
if nothing else depends on the current values - which I can't tell.

My other questions are still valid as well.

Jan


---
 gdbstub.c |  116 +++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 71 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,90 @@ 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));
+
+#if defined(TARGET_X86_64)
+static int gpreg_map[] = {
+    R_EAX, R_EBX, R_ECX, R_EDX, R_ESI, R_EDI, R_EBP, R_ESP,
+    8, 9, 10, 11, 12, 13, 14, 15
+};
+#define MAP_GPREG(n)    gpreg_map[n]
+#else
+#define MAP_GPREG(n)    (n)
+#endif
 
 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[MAP_GPREG(i)]);
+
+    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[MAP_GPREG(i)] = tswapl(regs->gp[i]);
+
+    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
 }
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]