[Qemu-devel] [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Qemu-devel] [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Jan Kiszka]

Commit 5459 broke the dynamic register set switching of qemu's gdbstub
for x86-64. This prevents setting the correct architecture in gdb when
debugging 32 or 16-bit code in a 64-bit emulator. Fix this.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c         |   54 +++++++++++++++++++++++++++++++++++++++++++-----------
 target-i386/cpu.h |    7 +++++--
 2 files changed, 48 insertions(+), 13 deletions(-)

Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -302,15 +302,22 @@ static const int gpr_map[16] = {
     8, 9, 10, 11, 12, 13, 14, 15
 };
 #else
-static const int gpr_map[8] = {0, 1, 2, 3, 4, 5, 6, 7};
+#define gpr_map gpr_map32
 #endif
+static const int gpr_map32[8] = { 0, 1, 2, 3, 4, 5, 6, 7 };
 
 #define NUM_CORE_REGS (CPU_NB_REGS * 2 + 25)
 
 static int cpu_gdb_read_register(CPUState *env, uint8_t *mem_buf, int n)
 {
     if (n < CPU_NB_REGS) {
-        GET_REGL(env->regs[gpr_map[n]]);
+        if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+            GET_REGL(env->regs[gpr_map[n]]);
+        } else if (n < CPU_NB_REGS32) {
+            GET_REG32(env->regs[gpr_map32[n]]);
+        } else {
+            return 0;
+        }
     } else if (n >= CPU_NB_REGS + 8 && n < CPU_NB_REGS + 16) {
         /* FIXME: byteswap float values.  */
 #ifdef USE_X86LDOUBLE
@@ -321,17 +328,25 @@ static int cpu_gdb_read_register(CPUStat
         return 10;
     } else if (n >= CPU_NB_REGS + 24) {
         n -= CPU_NB_REGS + 24;
-        if (n < CPU_NB_REGS) {
+        if (n < CPU_NB_REGS32
+            || (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                && n < CPU_NB_REGS64)) {
             stq_p(mem_buf, env->xmm_regs[n].XMM_Q(0));
             stq_p(mem_buf + 8, env->xmm_regs[n].XMM_Q(1));
             return 16;
-        } else if (n == CPU_NB_REGS) {
+        } else if ((TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                    && n == CPU_NB_REGS64) || n == CPU_NB_REGS32) {
             GET_REG32(env->mxcsr);
-        } 
+        }
     } else {
         n -= CPU_NB_REGS;
         switch (n) {
-        case 0: GET_REGL(env->eip);
+        case 0:
+            if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+                GET_REG64(env->eip);
+            } else {
+                GET_REG32(env->eip);
+            }
         case 1: GET_REG32(env->eflags);
         case 2: GET_REG32(env->segs[R_CS].selector);
         case 3: GET_REG32(env->segs[R_SS].selector);
@@ -359,8 +374,15 @@ static int cpu_gdb_write_register(CPUSta
     uint32_t tmp;
 
     if (i < CPU_NB_REGS) {
-        env->regs[gpr_map[i]] = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
+        if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+            env->regs[gpr_map[i]] = ldtul_p(mem_buf);
+            return sizeof(target_ulong);
+        } else if (i < CPU_NB_REGS32) {
+            env->regs[gpr_map32[i]] = ldl_p(mem_buf);
+            return 4;
+        } else {
+            return 0;
+        }
     } else if (i >= CPU_NB_REGS + 8 && i < CPU_NB_REGS + 16) {
         i -= CPU_NB_REGS + 8;
 #ifdef USE_X86LDOUBLE
@@ -369,18 +391,28 @@ static int cpu_gdb_write_register(CPUSta
         return 10;
     } else if (i >= CPU_NB_REGS + 24) {
         i -= CPU_NB_REGS + 24;
-        if (i < CPU_NB_REGS) {
+        if (i < CPU_NB_REGS32
+            || (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                && i < CPU_NB_REGS64)) {
             env->xmm_regs[i].XMM_Q(0) = ldq_p(mem_buf);
             env->xmm_regs[i].XMM_Q(1) = ldq_p(mem_buf + 8);
             return 16;
-        } else if (i == CPU_NB_REGS) {
+        } else if ((TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)
+                    && i == CPU_NB_REGS64) || i == CPU_NB_REGS32) {
             env->mxcsr = ldl_p(mem_buf);
             return 4;
         }
     } else {
         i -= CPU_NB_REGS;
         switch (i) {
-        case 0: env->eip = ldtul_p(mem_buf); return sizeof(target_ulong);
+        case 0:
+            if (TARGET_LONG_BITS == 64 && (env->hflags & HF_CS64_MASK)) {
+                env->eip = ldq_p(mem_buf);
+                return 8;
+            } else {
+                env->eip = ldl_p(mem_buf);
+                return 4;
+            }
         case 1: env->eflags = ldl_p(mem_buf); return 4;
 #if defined(CONFIG_USER_ONLY)
 #define LOAD_SEG(index, sreg)\
Index: b/target-i386/cpu.h
===================================================================
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -494,10 +494,13 @@ typedef union {
 #endif
 #define MMX_Q(n) q
 
+#define CPU_NB_REGS64 16
+#define CPU_NB_REGS32 8
+
 #ifdef TARGET_X86_64
-#define CPU_NB_REGS 16
+#define CPU_NB_REGS CPU_NB_REGS64
 #else
-#define CPU_NB_REGS 8
+#define CPU_NB_REGS CPU_NB_REGS32
 #endif
 
 #define NB_MMU_MODES 2

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Paul Brook]

On Tuesday 04 November 2008, Jan Kiszka wrote:
> Commit 5459 broke the dynamic register set switching of qemu's gdbstub
> for x86-64. This prevents setting the correct architecture in gdb when
> debugging 32 or 16-bit code in a 64-bit emulator. Fix this.

Is this really a feature? Surely any attached gdb is going to break horribly 
when we transition from a 64-bit to a 32-bit code segment.

Paul

Re: [Qemu-devel] [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Jan Kiszka]

Paul Brook wrote:
> On Tuesday 04 November 2008, Jan Kiszka wrote:
>> Commit 5459 broke the dynamic register set switching of qemu's gdbstub
>> for x86-64. This prevents setting the correct architecture in gdb when
>> debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
> 
> Is this really a feature? Surely any attached gdb is going to break horribly 
> when we transition from a 64-bit to a 32-bit code segment.

Well, it would be real feature if gdb was smart enough to track those
switches automatically...

However, you can (and obviously have to) call "set arch ..." after that
switch in order to get the proper disassembly. Or you happen to use
qemu-system-x86_64 with a 32-bit guest and fire up gdb with the
appropriate 32-bit binary directly. Both used to work fine.

But if there is a way to tell gdb to switch x86 archs without switching
the remote gdb packet length for register queries, I would happily use
that and drop my patch!

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Jamie Lokier]

Jan Kiszka wrote:
> Paul Brook wrote:
> > On Tuesday 04 November 2008, Jan Kiszka wrote:
> >> Commit 5459 broke the dynamic register set switching of qemu's gdbstub
> >> for x86-64. This prevents setting the correct architecture in gdb when
> >> debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
> > 
> > Is this really a feature? Surely any attached gdb is going to break horribly 
> > when we transition from a 64-bit to a 32-bit code segment.
> 
> Well, it would be real feature if gdb was smart enough to track those
> switches automatically...
> 
> However, you can (and obviously have to) call "set arch ..." after that
> switch in order to get the proper disassembly. Or you happen to use
> qemu-system-x86_64 with a 32-bit guest and fire up gdb with the
> appropriate 32-bit binary directly. Both used to work fine.

How does it handle mixed 32-bit and 16-bit code?  (Since you mentioned
it supports 16-bit code).

Does that require manual intervention too?

-- Jamie

[Qemu-devel] Re: [PATCH] gdbstub: x86-64: reintroduce dynamic register sets

[Jan Kiszka]

Jamie Lokier wrote:
> Jan Kiszka wrote:
>> Paul Brook wrote:
>>> On Tuesday 04 November 2008, Jan Kiszka wrote:
>>>> Commit 5459 broke the dynamic register set switching of qemu's gdbstub
>>>> for x86-64. This prevents setting the correct architecture in gdb when
>>>> debugging 32 or 16-bit code in a 64-bit emulator. Fix this.
>>> Is this really a feature? Surely any attached gdb is going to break horribly 
>>> when we transition from a 64-bit to a 32-bit code segment.
>> Well, it would be real feature if gdb was smart enough to track those
>> switches automatically...
>>
>> However, you can (and obviously have to) call "set arch ..." after that
>> switch in order to get the proper disassembly. Or you happen to use
>> qemu-system-x86_64 with a 32-bit guest and fire up gdb with the
>> appropriate 32-bit binary directly. Both used to work fine.
> 
> How does it handle mixed 32-bit and 16-bit code?  (Since you mentioned
> it supports 16-bit code).
> 
> Does that require manual intervention too?

Same here: 'set arch i8086' will make gdb switch to 16-bit disassembly
mode. But the remote protocol's register set format does not change
then, compared to arch i386.

BTW, more glue (including gdb patches) is required for full support of
x86 segmentation. I'm currently trying to get out changes in shape again
that were once based on older versions of qemu and gdb.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux