[Qemu-devel] Could any body fix the bug in gdbstub.c for x86_64?

[Qemu-devel] Could any body fix the bug in gdbstub.c for x86_64?

[MingyanGuo]

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

Hi,
The -s option for qemu-system-x86_64 is not useable.
I've already post a patch, but it is ignored.
http://lists.gnu.org/archive/html/qemu-devel/2007-05/msg00062.html
This patch is for version 0.9.0, could some body merge it
in the cvs tree?

-----------------------------------><--------------------------------------
--- qemu-0.9.0/gdbstub.c    Tue Feb  6 07:01:54 2007
+++ gdbstub.c    Sat May  5 02:22:39 2007
@@ -223,63 +223,83 @@

 #if defined(TARGET_I386)

+#if defined(TARGET_X86_64)
+/*
+ * XXX
+ * This is a ugly hack (in my opinion...), is it better to redefine R_EXX
+ * in target-i386/cpu.h to match 'amd64_regnum' in gdb (gdb/amd64-tdep.h)?
+ */
+static int regnames[CPU_NB_REGS] = { R_EAX, R_EBX, R_ECX, R_EDX, R_ESI,
R_EDI,
+        R_EBP, R_ESP, 8, 9, 10, 11, 12, 13, 14, 15 };
+#endif
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
-    uint32_t *registers = (uint32_t *)mem_buf;
-    int i, fpus;
+    target_ulong *registers = (target_ulong *)mem_buf;
+    int i, fpus, regno0, regno1, regno2;

-    for(i = 0; i < 8; i++) {
+    for(i = 0; i < CPU_NB_REGS; i++) {
+#if defined(TARGET_X86_64)
+        registers[i] = env->regs[regnames[i]];
+#else
         registers[i] = env->regs[i];
+#endif
     }
-    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;
+    registers[i++] = env->eip;
+    registers[i++] = env->eflags;
+    registers[i++] = env->segs[R_CS].selector;
+    registers[i++] = env->segs[R_SS].selector;
+    registers[i++] = env->segs[R_DS].selector;
+    registers[i++] = env->segs[R_ES].selector;
+    registers[i++] = env->segs[R_FS].selector;
+    registers[i++] = env->segs[R_GS].selector;
+    regno0 = i;
     /* XXX: convert floats */
     for(i = 0; i < 8; i++) {
-        memcpy(mem_buf + 16 * 4 + i * 10, &env->fpregs[i], 10);
+        memcpy(mem_buf + regno0 * sizeof(target_ulong) + i * 10,
&env->fpregs[i], 10);
     }
-    registers[36] = env->fpuc;
+    regno1 = regno0 + (i * 10)/sizeof(target_ulong);
+    i = regno1;
+    registers[i++] = 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[i++] = fpus;
+    registers[i++] = 0; /* XXX: convert tags */
+    registers[i++] = 0; /* fiseg */
+    registers[i++] = 0; /* fioff */
+    registers[i++] = 0; /* foseg */
+    registers[i++] = 0; /* fooff */
+    registers[i++] = 0; /* fop */
+    regno2 = i;
+    for(i = 0; i < regno0; i++)
         tswapls(&registers[i]);
-    for(i = 36; i < 44; i++)
+    for(i = regno1; i < regno2; i++)
         tswapls(&registers[i]);
-    return 44 * 4;
+    return (regno2 * 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 < CPU_NB_REGS; i++) {
+#if defined(TARGET_X86_64)
+        env->regs[regnames[i]] = tswapl(registers[i]);
+#else
         env->regs[i] = tswapl(registers[i]);
+#endif
     }
-    env->eip = tswapl(registers[8]);
-    env->eflags = tswapl(registers[9]);
+    env->eip = tswapl(registers[CPU_NB_REGS]);
+    env->eflags = tswapl(registers[CPU_NB_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(CPU_NB_REGS + 2, R_CS);
+            LOAD_SEG(CPU_NB_REGS + 3, R_SS);
+            LOAD_SEG(CPU_NB_REGS + 4, R_DS);
+            LOAD_SEG(CPU_NB_REGS + 5, R_ES);
+            LOAD_SEG(CPU_NB_REGS + 6, R_FS);
+            LOAD_SEG(CPU_NB_REGS + 7, R_GS);
 #endif
 }


---------------------------------------------><-----------------------------------


Regards,
MingyanGuo
-- 
Three passions, simple but overwhelmingly strong, have governed my life:
the longing for love, the search for knowledge, and unbearable pity for
the suffering of mankind.
                         ---------Bertrand Russell

[-- Attachment #2: Type: text/html, Size: 7915 bytes --]

[Qemu-devel] Re: Could any body fix the bug in gdbstub.c for x86_64?

[Jan Kiszka]

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

MingyanGuo wrote:
> Hi,
> The -s option for qemu-system-x86_64 is not useable.
> I've already post a patch, but it is ignored.

Have you seen my patch [1] on this already? Does it work for you? It's
running fine here, but there are still some open questions [2], and I
unfortunately didn't received feedback on my last revisions as well.

I haven't looked in details at your approach, but you stumbled over the
same thing as I did, that redefining the register constants would
simplify things significantly - if this is fine with the rest of qemu.

> http://lists.gnu.org/archive/html/qemu-devel/2007-05/msg00062.html
> This patch is for version 0.9.0, could some body merge it
> in the cvs tree?
> 

However the solution will finally look like, a fix for the next release
would be welcome here as well! One just need to know which way is
preferred...

Jan


[1] http://lists.gnu.org/archive/html/qemu-devel/2007-04/msg00554.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2007-04/msg00524.html


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

[Qemu-devel] Re: Could any body fix the bug in gdbstub.c for x86_64?

[MingyanGuo]

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

On 5/9/07, Jan Kiszka <jan.kiszka@web.de> wrote:
>
> MingyanGuo wrote:
> > Hi,
> > The -s option for qemu-system-x86_64 is not useable.
> > I've already post a patch, but it is ignored.
>
> Have you seen my patch [1] on this already? Does it work for you? It's
> running fine here, but there are still some open questions [2], and I
> unfortunately didn't received feedback on my last revisions as well.
>
> I haven't looked in details at your approach, but you stumbled over the
> same thing as I did, that redefining the register constants would
> simplify things significantly - if this is fine with the rest of qemu.


I read your patch now, our approaches are the same I think, just in
different
forms. I don't know if there are any adverse side effects to redefine the
register
constants (see my comments in the patch), so a simple register mapping is a
quick fix :-)

> http://lists.gnu.org/archive/html/qemu-devel/2007-05/msg00062.html
> > This patch is for version 0.9.0, could some body merge it
> > in the cvs tree?
> >
>
> However the solution will finally look like, a fix for the next release
> would be welcome here as well! One just need to know which way is
> preferred...
>
> Jan
>
>
> [1] http://lists.gnu.org/archive/html/qemu-devel/2007-04/msg00554.html
> [2] http://lists.gnu.org/archive/html/qemu-devel/2007-04/msg00524.html
>
>
>
I should cancel my patch now, as I am debugging an OS kernel, no xmms/fpu
registers considered, and your patch is better. :D

Regards,
MingyanGuo

-- 
Three passions, simple but overwhelmingly strong, have governed my life:
the longing for love, the search for knowledge, and unbearable pity for
the suffering of mankind.
                         ---------Bertrand Russell

[-- Attachment #2: Type: text/html, Size: 2656 bytes --]

Re: [Qemu-devel] Could any body fix the bug in gdbstub.c for x86_64?

[Paul Brook]

On Wednesday 09 May 2007, MingyanGuo wrote:
> Hi,
> The -s option for qemu-system-x86_64 is not useable.
> I've already post a patch, but it is ignored.
> http://lists.gnu.org/archive/html/qemu-devel/2007-05/msg00062.html

Please read the list archives for previous patches/discussions on this topic. 
Your patch is wrong in several ways

Paul