diff mbox

gdbstub: x86: Switch 64/32 bit registers dynamically

Message ID 4AB26055.2060506@siemens.com
State Superseded
Headers show

Commit Message

Jan Kiszka Sept. 17, 2009, 4:14 p.m. UTC 
Commit 56aebc891674cd2d07b3f64183415697be200084 changed gdbstub in way
that debugging 32 or 16-bit guest code is no longer possible with qemu
for x86_64 guest CPUs. Since that commit, qemu only provides registers
sets for 64-bit, forcing current and foreseeable gdb to also switch its
architecture to 64-bit. And this breaks if the inferior is 32 or 16 bit.

No question, this is a gdb issue. But, as it was confirmed in several
discusssions with gdb people, it is a non-trivial thing to fix. So until
qemu finds a gdb version attach with a rework x86 support, we have to
work around it by switching the register layout as the guest switches
its execution mode between 16/32 and 64 bit.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Sent to qemu-kvm for inclusion on Avi's request as this workaround is
still disliked upstream.

Note: qemu-kvm's gdbstub support in kvm mode is currently borken in git
- I'm bisecting...

 gdbstub.c         |   55 ++++++++++++++++++++++++++++++++++++++++-------------
 target-i386/cpu.h |    7 +++++--
 2 files changed, 47 insertions(+), 15 deletions(-)

Comments

Jan Kiszka Sept. 17, 2009, 4:38 p.m. UTC | #1 
Jan Kiszka wrote:
> Commit 56aebc891674cd2d07b3f64183415697be200084 changed gdbstub in way
> that debugging 32 or 16-bit guest code is no longer possible with qemu
> for x86_64 guest CPUs. Since that commit, qemu only provides registers
> sets for 64-bit, forcing current and foreseeable gdb to also switch its
> architecture to 64-bit. And this breaks if the inferior is 32 or 16 bit.
> 
> No question, this is a gdb issue. But, as it was confirmed in several
> discusssions with gdb people, it is a non-trivial thing to fix. So until
> qemu finds a gdb version attach with a rework x86 support, we have to
> work around it by switching the register layout as the guest switches
> its execution mode between 16/32 and 64 bit.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> Sent to qemu-kvm for inclusion on Avi's request as this workaround is
> still disliked upstream.

Sorry, mistyped the list address.

Jan
Paul Brook Nov. 10, 2009, 1:35 p.m. UTC | #2 
> No question, this is a gdb issue. But, as it was confirmed in several
> discusssions with gdb people, it is a non-trivial thing to fix. So until
> qemu finds a gdb version attach with a rework x86 support, we have to
> work around it by switching the register layout as the guest switches
> its execution mode between 16/32 and 64 bit.

Again, absolutely not. Changing the register set reported to a connected gdb 
is *never* correct.  An option (possibly monitor command) to change the 
register set reported to gdb *before* a connection is establisted, that may be 
acceptable.

Paul
diff mbox

Patch

diff --git a/gdbstub.c b/gdbstub.c
index c9304cd..a33aba0 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -506,8 +506,9 @@  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)
 
@@ -521,7 +522,11 @@  static const int gpr_map[8] = {0, 1, 2, 3, 4, 5, 6, 7};
 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_REG64(env->regs[gpr_map[n]]);
+        } else if (n < CPU_NB_REGS32) {
+            GET_REG32(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. */
@@ -532,12 +537,20 @@  static int cpu_gdb_read_register(CPUState *env, uint8_t *mem_buf, int n)
         return 10;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
-        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;
+        if (n < CPU_NB_REGS32 ||
+            (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) {
+            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 {
         switch (n) {
-        case IDX_IP_REG:    GET_REGL(env->eip);
+        case IDX_IP_REG:
+            if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) {
+                GET_REG64(env->eip);
+            } else {
+                GET_REG32(env->eip);
+            }
         case IDX_FLAGS_REG: GET_REG32(env->eflags);
 
         case IDX_SEG_REGS:     GET_REG32(env->segs[R_CS].selector);
@@ -593,8 +606,15 @@  static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
     uint32_t tmp;
 
     if (n < CPU_NB_REGS) {
-        env->regs[gpr_map[n]] = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
+        if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) {
+            env->regs[gpr_map[n]] = ldtul_p(mem_buf);
+            return sizeof(target_ulong);
+        } else if (n < CPU_NB_REGS32) {
+            n = gpr_map32[n];
+            env->regs[n] &= ~0xffffffffUL;
+            env->regs[n] |= (uint32_t)ldl_p(mem_buf);
+            return 4;
+        }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
 #ifdef USE_X86LDOUBLE
         /* FIXME: byteswap float values - after fixing fpregs layout. */
@@ -603,14 +623,23 @@  static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
         return 10;
     } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
         n -= IDX_XMM_REGS;
-        env->xmm_regs[n].XMM_Q(0) = ldq_p(mem_buf);
-        env->xmm_regs[n].XMM_Q(1) = ldq_p(mem_buf + 8);
-        return 16;
+        if (n < CPU_NB_REGS32 ||
+            (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK)) {
+            env->xmm_regs[n].XMM_Q(0) = ldq_p(mem_buf);
+            env->xmm_regs[n].XMM_Q(1) = ldq_p(mem_buf + 8);
+            return 16;
+        }
     } else {
         switch (n) {
         case IDX_IP_REG:
-            env->eip = ldtul_p(mem_buf);
-            return sizeof(target_ulong);
+            if (TARGET_LONG_BITS == 64 && env->hflags & HF_CS64_MASK) {
+                env->eip = ldq_p(mem_buf);
+                return 8;
+            } else {
+                env->eip &= ~0xffffffffUL;
+                env->eip |= (uint32_t)ldl_p(mem_buf);
+                return 4;
+            }
         case IDX_FLAGS_REG:
             env->eflags = ldl_p(mem_buf);
             return 4;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b9a6392..4506d73 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -555,10 +555,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