[PATCH] target/i386: Improve 16-bit/real mode debug support in GDB

Davidson Francis posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20241221054549.21883-1-davidsondfgl@gmail.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>
target/i386/cpu.c     |  8 +++++++-
target/i386/gdbstub.c | 15 +++++++++++++--
2 files changed, 20 insertions(+), 3 deletions(-)
[PATCH] target/i386: Improve 16-bit/real mode debug support in GDB
Posted by Davidson Francis 1 year, 1 month ago
Debugging 16-bit/real mode code in QEMU+GDB is challenging due to
incorrect architecture detection and segmented memory addressing issues.

This patch improves the debugging experience by reporting i8086
architecture to GDB when in real mode and converting segmented addresses
(CS:EIP, SS:ESP) to their physical equivalents when reporting to GDB.
This enables proper instruction disassembly and stack inspection without
complex workarounds.

Note: Mode switches after GDB attachment still require manual
architecture change, as GDB RSP does not support runtime architecture
switches.

Signed-off-by: Davidson Francis <davidsondfgl@gmail.com>
---
 target/i386/cpu.c     |  8 +++++++-
 target/i386/gdbstub.c | 15 +++++++++++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5253399459..65bdc48cc0 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6404,7 +6404,13 @@ static const gchar *x86_gdb_arch_name(CPUState *cs)
 #ifdef TARGET_X86_64
     return "i386:x86-64";
 #else
-    return "i386";
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+    if (env->cr[0] & 1) {
+        return "i386";
+    } else {
+        return "i8086";
+    }
 #endif
 }
 
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index 04c49e802d..d600aee953 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -136,7 +136,13 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
                 return gdb_get_regl(mem_buf, 0);
             }
         } else {
-            return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
+            if (n != R_ESP || (env->cr[0] & 1)) {
+                return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
+            } else {
+                return gdb_get_reg32(mem_buf,
+                                     (env->segs[R_SS].selector << 4) +
+                                      env->regs[gpr_map32[n]]);
+            }
         }
     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
         int st_index = n - IDX_FP_REGS;
@@ -155,7 +161,12 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
     } else {
         switch (n) {
         case IDX_IP_REG:
-            return gdb_get_reg(env, mem_buf, env->eip);
+            if (TARGET_LONG_BITS != 32 || (env->cr[0] & 1)) {
+                return gdb_get_reg(env, mem_buf, env->eip);
+            } else {
+                return gdb_get_reg(env, mem_buf,
+                                   (env->segs[R_CS].selector << 4) + env->eip);
+            }
         case IDX_FLAGS_REG:
             return gdb_get_reg32(mem_buf, env->eflags);
 
-- 
2.37.3
Re: [PATCH] target/i386: Improve 16-bit/real mode debug support in GDB
Posted by Bernhard Beschow 11 months, 1 week ago

Am 21. Dezember 2024 05:45:49 UTC schrieb Davidson Francis <davidsondfgl@gmail.com>:
>Debugging 16-bit/real mode code in QEMU+GDB is challenging due to
>incorrect architecture detection and segmented memory addressing issues.
>
>This patch improves the debugging experience by reporting i8086
>architecture to GDB when in real mode and converting segmented addresses
>(CS:EIP, SS:ESP) to their physical equivalents when reporting to GDB.
>This enables proper instruction disassembly and stack inspection without
>complex workarounds.
>
>Note: Mode switches after GDB attachment still require manual
>architecture change, as GDB RSP does not support runtime architecture
>switches.
>
>Signed-off-by: Davidson Francis <davidsondfgl@gmail.com>
>---
> target/i386/cpu.c     |  8 +++++++-
> target/i386/gdbstub.c | 15 +++++++++++++--
> 2 files changed, 20 insertions(+), 3 deletions(-)

Ping

I don't feel confident reviewing this patch. Paolo? Zhao?

Best regards,
Bernhard

>
>diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>index 5253399459..65bdc48cc0 100644
>--- a/target/i386/cpu.c
>+++ b/target/i386/cpu.c
>@@ -6404,7 +6404,13 @@ static const gchar *x86_gdb_arch_name(CPUState *cs)
> #ifdef TARGET_X86_64
>     return "i386:x86-64";
> #else
>-    return "i386";
>+    X86CPU *cpu = X86_CPU(cs);
>+    CPUX86State *env = &cpu->env;
>+    if (env->cr[0] & 1) {
>+        return "i386";
>+    } else {
>+        return "i8086";
>+    }
> #endif
> }
> 
>diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
>index 04c49e802d..d600aee953 100644
>--- a/target/i386/gdbstub.c
>+++ b/target/i386/gdbstub.c
>@@ -136,7 +136,13 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>                 return gdb_get_regl(mem_buf, 0);
>             }
>         } else {
>-            return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
>+            if (n != R_ESP || (env->cr[0] & 1)) {
>+                return gdb_get_reg32(mem_buf, env->regs[gpr_map32[n]]);
>+            } else {
>+                return gdb_get_reg32(mem_buf,
>+                                     (env->segs[R_SS].selector << 4) +
>+                                      env->regs[gpr_map32[n]]);
>+            }
>         }
>     } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
>         int st_index = n - IDX_FP_REGS;
>@@ -155,7 +161,12 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
>     } else {
>         switch (n) {
>         case IDX_IP_REG:
>-            return gdb_get_reg(env, mem_buf, env->eip);
>+            if (TARGET_LONG_BITS != 32 || (env->cr[0] & 1)) {
>+                return gdb_get_reg(env, mem_buf, env->eip);
>+            } else {
>+                return gdb_get_reg(env, mem_buf,
>+                                   (env->segs[R_CS].selector << 4) + env->eip);
>+            }
>         case IDX_FLAGS_REG:
>             return gdb_get_reg32(mem_buf, env->eflags);
>