[PATCH] align gdbstub with QEMU

[PATCH] align gdbstub with QEMU

[Jan Kiszka]

We've noticed some problems with current gdbstub in kvm's qemu:

# qemu-system-x86_64 -hda myimage -S -s &
# gdb
(gdb) tar re :1234
Remote debugging using :1234
Remote 'g' packet reply is too long:
000000000000000000000000000000000000[...]

This issue did not occur with QEMU from CVS. As I was aware of an
x86_64-related problem in QEMU's gdbstub, I merged its current state
into kvm - and things start to work again. Find the update patch below.

Signed-off-by: Jan Kiszka <jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>

---
 qemu/gdbstub.c |  290 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 143 insertions(+), 147 deletions(-)

Index: kvm-54/qemu/gdbstub.c
===================================================================
--- kvm-54.orig/qemu/gdbstub.c
+++ kvm-54/qemu/gdbstub.c
@@ -222,146 +222,60 @@ static int put_packet(GDBState *s, char 
     return 0;
 }
 
-#if defined(TARGET_X86_64)
+#if defined(TARGET_I386)
 
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
-    uint8_t *p = mem_buf;
     int i, fpus;
+    uint32_t *registers = (uint32_t *)mem_buf;
 
-#define PUTREG(x) do { \
-	target_ulong reg = tswapl(x); \
-	memcpy(p, &reg, sizeof reg); \
-	p += sizeof reg; \
-    } while (0)
-#define PUTREG32(x) do { \
-	uint32_t reg = tswap32(x);		\
-	memcpy(p, &reg, sizeof reg); \
-	p += sizeof reg; \
-    } while (0)
-#define PUTREGF(x) do { \
-	memcpy(p, &(x), 10);			\
-	p += sizeof (x);			\
-    } while (0)
-
-    PUTREG(env->regs[R_EAX]);
-    PUTREG(env->regs[R_EBX]);
-    PUTREG(env->regs[R_ECX]);
-    PUTREG(env->regs[R_EDX]);
-    PUTREG(env->regs[R_ESI]);
-    PUTREG(env->regs[R_EDI]);
-    PUTREG(env->regs[R_EBP]);
-    PUTREG(env->regs[R_ESP]);
-    PUTREG(env->regs[8]);
-    PUTREG(env->regs[9]);
-    PUTREG(env->regs[10]);
-    PUTREG(env->regs[11]);
-    PUTREG(env->regs[12]);
-    PUTREG(env->regs[13]);
-    PUTREG(env->regs[14]);
-    PUTREG(env->regs[15]);
-
-    PUTREG(env->eip);
-    PUTREG32(env->eflags);
-    PUTREG32(env->segs[R_CS].selector);
-    PUTREG32(env->segs[R_SS].selector);
-    PUTREG32(env->segs[R_DS].selector);
-    PUTREG32(env->segs[R_ES].selector);
-    PUTREG32(env->segs[R_FS].selector);
-    PUTREG32(env->segs[R_GS].selector);
-    /* XXX: convert floats */
-    for(i = 0; i < 8; i++) {
-        PUTREGF(env->fpregs[i]);
-    }
-    PUTREG32(env->fpuc);
-    fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
-    PUTREG32(fpus);
-    PUTREG32(0); /* XXX: convert tags */
-    PUTREG32(0); /* fiseg */
-    PUTREG32(0); /* fioff */
-    PUTREG32(0); /* foseg */
-    PUTREG32(0); /* fooff */
-    PUTREG32(0); /* fop */
-
-#undef PUTREG
-#undef PUTREG32
-#undef PUTREGF
-
-    return p - mem_buf;
-}
+#ifdef TARGET_X86_64
+    /* This corresponds with amd64_register_info[] in gdb/amd64-tdep.c */
+    uint64_t *registers64 = (uint64_t *)mem_buf;
+
+    if (env->hflags & HF_CS64_MASK) {
+        registers64[0] = tswap64(env->regs[R_EAX]);
+        registers64[1] = tswap64(env->regs[R_EBX]);
+        registers64[2] = tswap64(env->regs[R_ECX]);
+        registers64[3] = tswap64(env->regs[R_EDX]);
+        registers64[4] = tswap64(env->regs[R_ESI]);
+        registers64[5] = tswap64(env->regs[R_EDI]);
+        registers64[6] = tswap64(env->regs[R_EBP]);
+        registers64[7] = tswap64(env->regs[R_ESP]);
+        for(i = 8; i < 16; i++) {
+            registers64[i] = tswap64(env->regs[i]);
+        }
+        registers64[16] = tswap64(env->eip);
 
-static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
-{
-    uint8_t *p = mem_buf;
-    uint32_t junk;
-    int i, fpus;
+        registers = (uint32_t *)&registers64[17];
+        registers[0] = tswap32(env->eflags);
+        registers[1] = tswap32(env->segs[R_CS].selector);
+        registers[2] = tswap32(env->segs[R_SS].selector);
+        registers[3] = tswap32(env->segs[R_DS].selector);
+        registers[4] = tswap32(env->segs[R_ES].selector);
+        registers[5] = tswap32(env->segs[R_FS].selector);
+        registers[6] = tswap32(env->segs[R_GS].selector);
+        /* XXX: convert floats */
+        for(i = 0; i < 8; i++) {
+            memcpy(mem_buf + 16 * 8 + 7 * 4 + i * 10, &env->fpregs[i], 10);
+        }
+        registers[27] = tswap32(env->fpuc); /* fctrl */
+        fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
+        registers[28] = tswap32(fpus); /* fstat */
+        registers[29] = 0; /* ftag */
+        registers[30] = 0; /* fiseg */
+        registers[31] = 0; /* fioff */
+        registers[32] = 0; /* foseg */
+        registers[33] = 0; /* fooff */
+        registers[34] = 0; /* fop */
+        for(i = 0; i < 16; i++) {
+            memcpy(mem_buf + 16 * 8 + 35 * 4 + i * 16, &env->xmm_regs[i], 16);
+        }
+        registers[99] = tswap32(env->mxcsr);
 
-#define GETREG(x) do { \
-	target_ulong reg; \
-    	memcpy(&reg, p, sizeof reg);     \
-	x = tswapl(reg);		 \
-	p += sizeof reg;                 \
-    } while (0)
-#define GETREG32(x) do { \
-	uint32_t reg; \
-    	memcpy(&reg, p, sizeof reg);     \
-	x = tswap32(reg);		 \
-	p += sizeof reg;                 \
-    } while (0)
-#define GETREGF(x) do { \
-    	memcpy(&(x), p, 10);		 \
-	p += 10;                         \
-    } while (0)
-
-    GETREG(env->regs[R_EAX]);
-    GETREG(env->regs[R_EBX]);
-    GETREG(env->regs[R_ECX]);
-    GETREG(env->regs[R_EDX]);
-    GETREG(env->regs[R_ESI]);
-    GETREG(env->regs[R_EDI]);
-    GETREG(env->regs[R_EBP]);
-    GETREG(env->regs[R_ESP]);
-    GETREG(env->regs[8]);
-    GETREG(env->regs[9]);
-    GETREG(env->regs[10]);
-    GETREG(env->regs[11]);
-    GETREG(env->regs[12]);
-    GETREG(env->regs[13]);
-    GETREG(env->regs[14]);
-    GETREG(env->regs[15]);
-
-    GETREG(env->eip);
-    GETREG32(env->eflags);
-    GETREG32(env->segs[R_CS].selector);
-    GETREG32(env->segs[R_SS].selector);
-    GETREG32(env->segs[R_DS].selector);
-    GETREG32(env->segs[R_ES].selector);
-    GETREG32(env->segs[R_FS].selector);
-    GETREG32(env->segs[R_GS].selector);
-    /* XXX: convert floats */
-    for(i = 0; i < 8; i++) {
-        GETREGF(env->fpregs[i]);
+        return 8 * 17 + 4 * 7 + 10 * 8 + 4 * 8 + 16 * 16 + 4;
     }
-    GETREG32(env->fpuc);
-    GETREG32(fpus); /* XXX: convert fpus */
-    GETREG32(junk); /* XXX: convert tags */
-    GETREG32(junk); /* fiseg */
-    GETREG32(junk); /* fioff */
-    GETREG32(junk); /* foseg */
-    GETREG32(junk); /* fooff */
-    GETREG32(junk); /* fop */
-
-#undef GETREG
-#undef GETREG32
-#undef GETREGF
-}
-
-#elif defined(TARGET_I386)
-
-static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
-{
-    uint32_t *registers = (uint32_t *)mem_buf;
-    int i, fpus;
+#endif
 
     for(i = 0; i < 8; i++) {
         registers[i] = env->regs[i];
@@ -435,7 +349,7 @@ static int cpu_gdb_read_registers(CPUSta
     }
     /* nip, msr, ccr, lnk, ctr, xer, mq */
     registers[96] = tswapl(env->nip);
-    registers[97] = tswapl(do_load_msr(env));
+    registers[97] = tswapl(env->msr);
     tmp = 0;
     for (i = 0; i < 8; i++)
         tmp |= env->crf[i] << (32 - ((i + 1) * 4));
@@ -464,7 +378,7 @@ static void cpu_gdb_write_registers(CPUS
     }
     /* nip, msr, ccr, lnk, ctr, xer, mq */
     env->nip = tswapl(registers[96]);
-    do_store_msr(env, tswapl(registers[97]));
+    ppc_store_msr(env, tswapl(registers[97]));
     registers[98] = tswapl(registers[98]);
     for (i = 0; i < 8; i++)
         env->crf[i] = (registers[98] >> (32 - ((i + 1) * 4))) & 0xF;
@@ -698,7 +612,7 @@ static int cpu_gdb_read_registers(CPUSta
         ptr += sizeof(target_ulong);
       }
 
-    *(target_ulong *)ptr = tswapl(env->CP0_Status);
+    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_Status);
     ptr += sizeof(target_ulong);
 
     *(target_ulong *)ptr = tswapl(env->LO[0][env->current_tc]);
@@ -710,7 +624,7 @@ static int cpu_gdb_read_registers(CPUSta
     *(target_ulong *)ptr = tswapl(env->CP0_BadVAddr);
     ptr += sizeof(target_ulong);
 
-    *(target_ulong *)ptr = tswapl(env->CP0_Cause);
+    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_Cause);
     ptr += sizeof(target_ulong);
 
     *(target_ulong *)ptr = tswapl(env->PC[env->current_tc]);
@@ -720,19 +634,34 @@ static int cpu_gdb_read_registers(CPUSta
       {
         for (i = 0; i < 32; i++)
           {
-            *(target_ulong *)ptr = tswapl(env->fpu->fpr[i].fs[FP_ENDIAN_IDX]);
+            if (env->CP0_Status & (1 << CP0St_FR))
+              *(target_ulong *)ptr = tswapl(env->fpu->fpr[i].d);
+            else
+              *(target_ulong *)ptr = tswap32(env->fpu->fpr[i].w[FP_ENDIAN_IDX]);
             ptr += sizeof(target_ulong);
           }
 
-        *(target_ulong *)ptr = tswapl(env->fpu->fcr31);
+        *(target_ulong *)ptr = (int32_t)tswap32(env->fpu->fcr31);
+        ptr += sizeof(target_ulong);
+
+        *(target_ulong *)ptr = (int32_t)tswap32(env->fpu->fcr0);
         ptr += sizeof(target_ulong);
+      }
+
+    /* "fp", pseudo frame pointer. Not yet implemented in gdb. */
+    *(target_ulong *)ptr = 0;
+    ptr += sizeof(target_ulong);
 
-        *(target_ulong *)ptr = tswapl(env->fpu->fcr0);
+    /* Registers for embedded use, we just pad them. */
+    for (i = 0; i < 16; i++)
+      {
+        *(target_ulong *)ptr = 0;
         ptr += sizeof(target_ulong);
       }
 
-    /* 32 FP registers, fsr, fir, fp.  Not yet implemented.  */
-    /* what's 'fp' mean here?  */
+    /* Processor ID. */
+    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_PRid);
+    ptr += sizeof(target_ulong);
 
     return ptr - mem_buf;
 }
@@ -782,15 +711,17 @@ static void cpu_gdb_write_registers(CPUS
       {
         for (i = 0; i < 32; i++)
           {
-            env->fpu->fpr[i].fs[FP_ENDIAN_IDX] = tswapl(*(target_ulong *)ptr);
+            if (env->CP0_Status & (1 << CP0St_FR))
+              env->fpu->fpr[i].d = tswapl(*(target_ulong *)ptr);
+            else
+              env->fpu->fpr[i].w[FP_ENDIAN_IDX] = tswapl(*(target_ulong *)ptr);
             ptr += sizeof(target_ulong);
           }
 
-        env->fpu->fcr31 = tswapl(*(target_ulong *)ptr) & 0x0183FFFF;
+        env->fpu->fcr31 = tswapl(*(target_ulong *)ptr) & 0xFF83FFFF;
         ptr += sizeof(target_ulong);
 
-        env->fpu->fcr0 = tswapl(*(target_ulong *)ptr);
-        ptr += sizeof(target_ulong);
+        /* The remaining registers are assumed to be read-only. */
 
         /* set rounding mode */
         RESTORE_ROUNDING_MODE;
@@ -863,6 +794,66 @@ static void cpu_gdb_write_registers(CPUS
   for (i = 0; i < 8; i++) LOAD(env->gregs[i]);
   for (i = 0; i < 8; i++) LOAD(env->gregs[i + 16]);
 }
+#elif defined (TARGET_CRIS)
+
+static int cris_save_32 (unsigned char *d, uint32_t value)
+{
+	*d++ = (value);
+	*d++ = (value >>= 8);
+	*d++ = (value >>= 8);
+	*d++ = (value >>= 8);
+	return 4;
+}
+static int cris_save_16 (unsigned char *d, uint32_t value)
+{
+	*d++ = (value);
+	*d++ = (value >>= 8);
+	return 2;
+}
+static int cris_save_8 (unsigned char *d, uint32_t value)
+{
+	*d++ = (value);
+	return 1;
+}
+
+/* FIXME: this will bug on archs not supporting unaligned word accesses.  */
+static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
+{
+  uint8_t *ptr = mem_buf;
+  uint8_t srs;
+  int i;
+
+  for (i = 0; i < 16; i++)
+	  ptr += cris_save_32 (ptr, env->regs[i]);
+
+  srs = env->pregs[SR_SRS];
+
+  ptr += cris_save_8 (ptr, env->pregs[0]);
+  ptr += cris_save_8 (ptr, env->pregs[1]);
+  ptr += cris_save_32 (ptr, env->pregs[2]);
+  ptr += cris_save_8 (ptr, srs);
+  ptr += cris_save_16 (ptr, env->pregs[4]);
+
+  for (i = 5; i < 16; i++)
+	  ptr += cris_save_32 (ptr, env->pregs[i]);
+
+  ptr += cris_save_32 (ptr, env->pc);
+
+  for (i = 0; i < 16; i++)
+	  ptr += cris_save_32 (ptr, env->sregs[srs][i]);
+
+  return ((uint8_t *)ptr - mem_buf);
+}
+
+static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
+{
+  uint32_t *ptr = (uint32_t *)mem_buf;
+  int i;
+
+#define LOAD(x) (x)=*ptr++;
+  for (i = 0; i < 16; i++) LOAD(env->regs[i]);
+  LOAD (env->pc);
+}
 #else
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
@@ -880,7 +871,7 @@ static int gdb_handle_packet(GDBState *s
     const char *p;
     int ch, reg_size, type;
     char buf[4096];
-    uint8_t mem_buf[2000];
+    uint8_t mem_buf[4096];
     uint32_t *registers;
     target_ulong addr, len;
 
@@ -914,6 +905,8 @@ static int gdb_handle_packet(GDBState *s
             env->pc = addr;
 #elif defined (TARGET_MIPS)
             env->PC[env->current_tc] = addr;
+#elif defined (TARGET_CRIS)
+            env->pc = addr;
 #endif
         }
 #ifdef CONFIG_USER_ONLY
@@ -941,6 +934,8 @@ static int gdb_handle_packet(GDBState *s
             env->pc = addr;
 #elif defined (TARGET_MIPS)
             env->PC[env->current_tc] = addr;
+#elif defined (TARGET_CRIS)
+            env->pc = addr;
 #endif
         }
         cpu_single_step(env, 1);
@@ -1070,7 +1065,8 @@ static int gdb_handle_packet(GDBState *s
             TaskState *ts = env->opaque;
 
             sprintf(buf,
-                    "Text=" TARGET_FMT_lx ";Data=" TARGET_FMT_lx ";Bss=" TARGET_FMT_lx,
+                    "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
+                    ";Bss=" TARGET_ABI_FMT_lx,
                     ts->info->code_offset,
                     ts->info->data_offset,
                     ts->info->data_offset);


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

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php

Re: [PATCH] align gdbstub with QEMU

[Jan Kiszka]

Jan Kiszka wrote:
> We've noticed some problems with current gdbstub in kvm's qemu:
> 
> # qemu-system-x86_64 -hda myimage -S -s &
> # gdb
> (gdb) tar re :1234
> Remote debugging using :1234
> Remote 'g' packet reply is too long:
> 000000000000000000000000000000000000[...]
> 
> This issue did not occur with QEMU from CVS. As I was aware of an
> x86_64-related problem in QEMU's gdbstub, I merged its current state
> into kvm - and things start to work again. Find the update patch below.

Any feedback on this? Any chance to see this patch (or some similar
version) in kvm-57?

Thanks,
Jan

> 
> Signed-off-by: Jan Kiszka <jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>
> 
> ---
>  qemu/gdbstub.c |  290 ++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 143 insertions(+), 147 deletions(-)
> 
> Index: kvm-54/qemu/gdbstub.c
> ===================================================================
> --- kvm-54.orig/qemu/gdbstub.c
> +++ kvm-54/qemu/gdbstub.c
> @@ -222,146 +222,60 @@ static int put_packet(GDBState *s, char 
>      return 0;
>  }
>  
> -#if defined(TARGET_X86_64)
> +#if defined(TARGET_I386)
>  
>  static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
>  {
> -    uint8_t *p = mem_buf;
>      int i, fpus;
> +    uint32_t *registers = (uint32_t *)mem_buf;
>  
> -#define PUTREG(x) do { \
> -	target_ulong reg = tswapl(x); \
> -	memcpy(p, &reg, sizeof reg); \
> -	p += sizeof reg; \
> -    } while (0)
> -#define PUTREG32(x) do { \
> -	uint32_t reg = tswap32(x);		\
> -	memcpy(p, &reg, sizeof reg); \
> -	p += sizeof reg; \
> -    } while (0)
> -#define PUTREGF(x) do { \
> -	memcpy(p, &(x), 10);			\
> -	p += sizeof (x);			\
> -    } while (0)
> -
> -    PUTREG(env->regs[R_EAX]);
> -    PUTREG(env->regs[R_EBX]);
> -    PUTREG(env->regs[R_ECX]);
> -    PUTREG(env->regs[R_EDX]);
> -    PUTREG(env->regs[R_ESI]);
> -    PUTREG(env->regs[R_EDI]);
> -    PUTREG(env->regs[R_EBP]);
> -    PUTREG(env->regs[R_ESP]);
> -    PUTREG(env->regs[8]);
> -    PUTREG(env->regs[9]);
> -    PUTREG(env->regs[10]);
> -    PUTREG(env->regs[11]);
> -    PUTREG(env->regs[12]);
> -    PUTREG(env->regs[13]);
> -    PUTREG(env->regs[14]);
> -    PUTREG(env->regs[15]);
> -
> -    PUTREG(env->eip);
> -    PUTREG32(env->eflags);
> -    PUTREG32(env->segs[R_CS].selector);
> -    PUTREG32(env->segs[R_SS].selector);
> -    PUTREG32(env->segs[R_DS].selector);
> -    PUTREG32(env->segs[R_ES].selector);
> -    PUTREG32(env->segs[R_FS].selector);
> -    PUTREG32(env->segs[R_GS].selector);
> -    /* XXX: convert floats */
> -    for(i = 0; i < 8; i++) {
> -        PUTREGF(env->fpregs[i]);
> -    }
> -    PUTREG32(env->fpuc);
> -    fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
> -    PUTREG32(fpus);
> -    PUTREG32(0); /* XXX: convert tags */
> -    PUTREG32(0); /* fiseg */
> -    PUTREG32(0); /* fioff */
> -    PUTREG32(0); /* foseg */
> -    PUTREG32(0); /* fooff */
> -    PUTREG32(0); /* fop */
> -
> -#undef PUTREG
> -#undef PUTREG32
> -#undef PUTREGF
> -
> -    return p - mem_buf;
> -}
> +#ifdef TARGET_X86_64
> +    /* This corresponds with amd64_register_info[] in gdb/amd64-tdep.c */
> +    uint64_t *registers64 = (uint64_t *)mem_buf;
> +
> +    if (env->hflags & HF_CS64_MASK) {
> +        registers64[0] = tswap64(env->regs[R_EAX]);
> +        registers64[1] = tswap64(env->regs[R_EBX]);
> +        registers64[2] = tswap64(env->regs[R_ECX]);
> +        registers64[3] = tswap64(env->regs[R_EDX]);
> +        registers64[4] = tswap64(env->regs[R_ESI]);
> +        registers64[5] = tswap64(env->regs[R_EDI]);
> +        registers64[6] = tswap64(env->regs[R_EBP]);
> +        registers64[7] = tswap64(env->regs[R_ESP]);
> +        for(i = 8; i < 16; i++) {
> +            registers64[i] = tswap64(env->regs[i]);
> +        }
> +        registers64[16] = tswap64(env->eip);
>  
> -static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
> -{
> -    uint8_t *p = mem_buf;
> -    uint32_t junk;
> -    int i, fpus;
> +        registers = (uint32_t *)&registers64[17];
> +        registers[0] = tswap32(env->eflags);
> +        registers[1] = tswap32(env->segs[R_CS].selector);
> +        registers[2] = tswap32(env->segs[R_SS].selector);
> +        registers[3] = tswap32(env->segs[R_DS].selector);
> +        registers[4] = tswap32(env->segs[R_ES].selector);
> +        registers[5] = tswap32(env->segs[R_FS].selector);
> +        registers[6] = tswap32(env->segs[R_GS].selector);
> +        /* XXX: convert floats */
> +        for(i = 0; i < 8; i++) {
> +            memcpy(mem_buf + 16 * 8 + 7 * 4 + i * 10, &env->fpregs[i], 10);
> +        }
> +        registers[27] = tswap32(env->fpuc); /* fctrl */
> +        fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
> +        registers[28] = tswap32(fpus); /* fstat */
> +        registers[29] = 0; /* ftag */
> +        registers[30] = 0; /* fiseg */
> +        registers[31] = 0; /* fioff */
> +        registers[32] = 0; /* foseg */
> +        registers[33] = 0; /* fooff */
> +        registers[34] = 0; /* fop */
> +        for(i = 0; i < 16; i++) {
> +            memcpy(mem_buf + 16 * 8 + 35 * 4 + i * 16, &env->xmm_regs[i], 16);
> +        }
> +        registers[99] = tswap32(env->mxcsr);
>  
> -#define GETREG(x) do { \
> -	target_ulong reg; \
> -    	memcpy(&reg, p, sizeof reg);     \
> -	x = tswapl(reg);		 \
> -	p += sizeof reg;                 \
> -    } while (0)
> -#define GETREG32(x) do { \
> -	uint32_t reg; \
> -    	memcpy(&reg, p, sizeof reg);     \
> -	x = tswap32(reg);		 \
> -	p += sizeof reg;                 \
> -    } while (0)
> -#define GETREGF(x) do { \
> -    	memcpy(&(x), p, 10);		 \
> -	p += 10;                         \
> -    } while (0)
> -
> -    GETREG(env->regs[R_EAX]);
> -    GETREG(env->regs[R_EBX]);
> -    GETREG(env->regs[R_ECX]);
> -    GETREG(env->regs[R_EDX]);
> -    GETREG(env->regs[R_ESI]);
> -    GETREG(env->regs[R_EDI]);
> -    GETREG(env->regs[R_EBP]);
> -    GETREG(env->regs[R_ESP]);
> -    GETREG(env->regs[8]);
> -    GETREG(env->regs[9]);
> -    GETREG(env->regs[10]);
> -    GETREG(env->regs[11]);
> -    GETREG(env->regs[12]);
> -    GETREG(env->regs[13]);
> -    GETREG(env->regs[14]);
> -    GETREG(env->regs[15]);
> -
> -    GETREG(env->eip);
> -    GETREG32(env->eflags);
> -    GETREG32(env->segs[R_CS].selector);
> -    GETREG32(env->segs[R_SS].selector);
> -    GETREG32(env->segs[R_DS].selector);
> -    GETREG32(env->segs[R_ES].selector);
> -    GETREG32(env->segs[R_FS].selector);
> -    GETREG32(env->segs[R_GS].selector);
> -    /* XXX: convert floats */
> -    for(i = 0; i < 8; i++) {
> -        GETREGF(env->fpregs[i]);
> +        return 8 * 17 + 4 * 7 + 10 * 8 + 4 * 8 + 16 * 16 + 4;
>      }
> -    GETREG32(env->fpuc);
> -    GETREG32(fpus); /* XXX: convert fpus */
> -    GETREG32(junk); /* XXX: convert tags */
> -    GETREG32(junk); /* fiseg */
> -    GETREG32(junk); /* fioff */
> -    GETREG32(junk); /* foseg */
> -    GETREG32(junk); /* fooff */
> -    GETREG32(junk); /* fop */
> -
> -#undef GETREG
> -#undef GETREG32
> -#undef GETREGF
> -}
> -
> -#elif defined(TARGET_I386)
> -
> -static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
> -{
> -    uint32_t *registers = (uint32_t *)mem_buf;
> -    int i, fpus;
> +#endif
>  
>      for(i = 0; i < 8; i++) {
>          registers[i] = env->regs[i];
> @@ -435,7 +349,7 @@ static int cpu_gdb_read_registers(CPUSta
>      }
>      /* nip, msr, ccr, lnk, ctr, xer, mq */
>      registers[96] = tswapl(env->nip);
> -    registers[97] = tswapl(do_load_msr(env));
> +    registers[97] = tswapl(env->msr);
>      tmp = 0;
>      for (i = 0; i < 8; i++)
>          tmp |= env->crf[i] << (32 - ((i + 1) * 4));
> @@ -464,7 +378,7 @@ static void cpu_gdb_write_registers(CPUS
>      }
>      /* nip, msr, ccr, lnk, ctr, xer, mq */
>      env->nip = tswapl(registers[96]);
> -    do_store_msr(env, tswapl(registers[97]));
> +    ppc_store_msr(env, tswapl(registers[97]));
>      registers[98] = tswapl(registers[98]);
>      for (i = 0; i < 8; i++)
>          env->crf[i] = (registers[98] >> (32 - ((i + 1) * 4))) & 0xF;
> @@ -698,7 +612,7 @@ static int cpu_gdb_read_registers(CPUSta
>          ptr += sizeof(target_ulong);
>        }
>  
> -    *(target_ulong *)ptr = tswapl(env->CP0_Status);
> +    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_Status);
>      ptr += sizeof(target_ulong);
>  
>      *(target_ulong *)ptr = tswapl(env->LO[0][env->current_tc]);
> @@ -710,7 +624,7 @@ static int cpu_gdb_read_registers(CPUSta
>      *(target_ulong *)ptr = tswapl(env->CP0_BadVAddr);
>      ptr += sizeof(target_ulong);
>  
> -    *(target_ulong *)ptr = tswapl(env->CP0_Cause);
> +    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_Cause);
>      ptr += sizeof(target_ulong);
>  
>      *(target_ulong *)ptr = tswapl(env->PC[env->current_tc]);
> @@ -720,19 +634,34 @@ static int cpu_gdb_read_registers(CPUSta
>        {
>          for (i = 0; i < 32; i++)
>            {
> -            *(target_ulong *)ptr = tswapl(env->fpu->fpr[i].fs[FP_ENDIAN_IDX]);
> +            if (env->CP0_Status & (1 << CP0St_FR))
> +              *(target_ulong *)ptr = tswapl(env->fpu->fpr[i].d);
> +            else
> +              *(target_ulong *)ptr = tswap32(env->fpu->fpr[i].w[FP_ENDIAN_IDX]);
>              ptr += sizeof(target_ulong);
>            }
>  
> -        *(target_ulong *)ptr = tswapl(env->fpu->fcr31);
> +        *(target_ulong *)ptr = (int32_t)tswap32(env->fpu->fcr31);
> +        ptr += sizeof(target_ulong);
> +
> +        *(target_ulong *)ptr = (int32_t)tswap32(env->fpu->fcr0);
>          ptr += sizeof(target_ulong);
> +      }
> +
> +    /* "fp", pseudo frame pointer. Not yet implemented in gdb. */
> +    *(target_ulong *)ptr = 0;
> +    ptr += sizeof(target_ulong);
>  
> -        *(target_ulong *)ptr = tswapl(env->fpu->fcr0);
> +    /* Registers for embedded use, we just pad them. */
> +    for (i = 0; i < 16; i++)
> +      {
> +        *(target_ulong *)ptr = 0;
>          ptr += sizeof(target_ulong);
>        }
>  
> -    /* 32 FP registers, fsr, fir, fp.  Not yet implemented.  */
> -    /* what's 'fp' mean here?  */
> +    /* Processor ID. */
> +    *(target_ulong *)ptr = (int32_t)tswap32(env->CP0_PRid);
> +    ptr += sizeof(target_ulong);
>  
>      return ptr - mem_buf;
>  }
> @@ -782,15 +711,17 @@ static void cpu_gdb_write_registers(CPUS
>        {
>          for (i = 0; i < 32; i++)
>            {
> -            env->fpu->fpr[i].fs[FP_ENDIAN_IDX] = tswapl(*(target_ulong *)ptr);
> +            if (env->CP0_Status & (1 << CP0St_FR))
> +              env->fpu->fpr[i].d = tswapl(*(target_ulong *)ptr);
> +            else
> +              env->fpu->fpr[i].w[FP_ENDIAN_IDX] = tswapl(*(target_ulong *)ptr);
>              ptr += sizeof(target_ulong);
>            }
>  
> -        env->fpu->fcr31 = tswapl(*(target_ulong *)ptr) & 0x0183FFFF;
> +        env->fpu->fcr31 = tswapl(*(target_ulong *)ptr) & 0xFF83FFFF;
>          ptr += sizeof(target_ulong);
>  
> -        env->fpu->fcr0 = tswapl(*(target_ulong *)ptr);
> -        ptr += sizeof(target_ulong);
> +        /* The remaining registers are assumed to be read-only. */
>  
>          /* set rounding mode */
>          RESTORE_ROUNDING_MODE;
> @@ -863,6 +794,66 @@ static void cpu_gdb_write_registers(CPUS
>    for (i = 0; i < 8; i++) LOAD(env->gregs[i]);
>    for (i = 0; i < 8; i++) LOAD(env->gregs[i + 16]);
>  }
> +#elif defined (TARGET_CRIS)
> +
> +static int cris_save_32 (unsigned char *d, uint32_t value)
> +{
> +	*d++ = (value);
> +	*d++ = (value >>= 8);
> +	*d++ = (value >>= 8);
> +	*d++ = (value >>= 8);
> +	return 4;
> +}
> +static int cris_save_16 (unsigned char *d, uint32_t value)
> +{
> +	*d++ = (value);
> +	*d++ = (value >>= 8);
> +	return 2;
> +}
> +static int cris_save_8 (unsigned char *d, uint32_t value)
> +{
> +	*d++ = (value);
> +	return 1;
> +}
> +
> +/* FIXME: this will bug on archs not supporting unaligned word accesses.  */
> +static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
> +{
> +  uint8_t *ptr = mem_buf;
> +  uint8_t srs;
> +  int i;
> +
> +  for (i = 0; i < 16; i++)
> +	  ptr += cris_save_32 (ptr, env->regs[i]);
> +
> +  srs = env->pregs[SR_SRS];
> +
> +  ptr += cris_save_8 (ptr, env->pregs[0]);
> +  ptr += cris_save_8 (ptr, env->pregs[1]);
> +  ptr += cris_save_32 (ptr, env->pregs[2]);
> +  ptr += cris_save_8 (ptr, srs);
> +  ptr += cris_save_16 (ptr, env->pregs[4]);
> +
> +  for (i = 5; i < 16; i++)
> +	  ptr += cris_save_32 (ptr, env->pregs[i]);
> +
> +  ptr += cris_save_32 (ptr, env->pc);
> +
> +  for (i = 0; i < 16; i++)
> +	  ptr += cris_save_32 (ptr, env->sregs[srs][i]);
> +
> +  return ((uint8_t *)ptr - mem_buf);
> +}
> +
> +static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
> +{
> +  uint32_t *ptr = (uint32_t *)mem_buf;
> +  int i;
> +
> +#define LOAD(x) (x)=*ptr++;
> +  for (i = 0; i < 16; i++) LOAD(env->regs[i]);
> +  LOAD (env->pc);
> +}
>  #else
>  static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
>  {
> @@ -880,7 +871,7 @@ static int gdb_handle_packet(GDBState *s
>      const char *p;
>      int ch, reg_size, type;
>      char buf[4096];
> -    uint8_t mem_buf[2000];
> +    uint8_t mem_buf[4096];
>      uint32_t *registers;
>      target_ulong addr, len;
>  
> @@ -914,6 +905,8 @@ static int gdb_handle_packet(GDBState *s
>              env->pc = addr;
>  #elif defined (TARGET_MIPS)
>              env->PC[env->current_tc] = addr;
> +#elif defined (TARGET_CRIS)
> +            env->pc = addr;
>  #endif
>          }
>  #ifdef CONFIG_USER_ONLY
> @@ -941,6 +934,8 @@ static int gdb_handle_packet(GDBState *s
>              env->pc = addr;
>  #elif defined (TARGET_MIPS)
>              env->PC[env->current_tc] = addr;
> +#elif defined (TARGET_CRIS)
> +            env->pc = addr;
>  #endif
>          }
>          cpu_single_step(env, 1);
> @@ -1070,7 +1065,8 @@ static int gdb_handle_packet(GDBState *s
>              TaskState *ts = env->opaque;
>  
>              sprintf(buf,
> -                    "Text=" TARGET_FMT_lx ";Data=" TARGET_FMT_lx ";Bss=" TARGET_FMT_lx,
> +                    "Text=" TARGET_ABI_FMT_lx ";Data=" TARGET_ABI_FMT_lx
> +                    ";Bss=" TARGET_ABI_FMT_lx,
>                      ts->info->code_offset,
>                      ts->info->data_offset,
>                      ts->info->data_offset);
> 
> 

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

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH] align gdbstub with QEMU

[Avi Kivity]

Jan Kiszka wrote:
> Jan Kiszka wrote:
>   
>> We've noticed some problems with current gdbstub in kvm's qemu:
>>
>> # qemu-system-x86_64 -hda myimage -S -s &
>> # gdb
>> (gdb) tar re :1234
>> Remote debugging using :1234
>> Remote 'g' packet reply is too long:
>> 000000000000000000000000000000000000[...]
>>
>> This issue did not occur with QEMU from CVS. As I was aware of an
>> x86_64-related problem in QEMU's gdbstub, I merged its current state
>> into kvm - and things start to work again. Find the update patch below.
>>     
>
> Any feedback on this? Any chance to see this patch (or some similar
> version) in kvm-57?
>
>   

Sorry for the non-response. I am hoping to get qemu-cvs merged for 
kvm-57, which will naturally include this. If that doesn't work out 
(i.e. we discover regressions), and if this is important for you, I will 
apply the patch.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH] align gdbstub with QEMU

[Jan Kiszka]

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>  
>>> We've noticed some problems with current gdbstub in kvm's qemu:
>>>
>>> # qemu-system-x86_64 -hda myimage -S -s &
>>> # gdb
>>> (gdb) tar re :1234
>>> Remote debugging using :1234
>>> Remote 'g' packet reply is too long:
>>> 000000000000000000000000000000000000[...]
>>>
>>> This issue did not occur with QEMU from CVS. As I was aware of an
>>> x86_64-related problem in QEMU's gdbstub, I merged its current state
>>> into kvm - and things start to work again. Find the update patch below.
>>>     
>>
>> Any feedback on this? Any chance to see this patch (or some similar
>> version) in kvm-57?
>>
>>   
> 
> Sorry for the non-response. I am hoping to get qemu-cvs merged for
> kvm-57, which will naturally include this. If that doesn't work out
> (i.e. we discover regressions), and if this is important for you, I will
> apply the patch.

OK, great.

BTW, are there plans to merge the kvm branch into qemu at some point and
continue development unitedly with the qemu people?

Jan

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

-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH] align gdbstub with QEMU

[Avi Kivity]

Jan Kiszka wrote:
> BTW, are there plans to merge the kvm branch into qemu at some point and
> continue development unitedly with the qemu people?
>   

There is the intent, but no concrete plans. What is missing is someone 
to prepare a suitable patchset for qemu-devel; that's no small amount of 
work.

I would also like to have libkvm in better shape for this; but if we 
include it in the patchset, that is not a hard requirement.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
SF.Net email is sponsored by:
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services
for just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

Re: [PATCH] align gdbstub with QEMU

[Jan Kiszka]

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Jan Kiszka wrote:
>>  
>>> We've noticed some problems with current gdbstub in kvm's qemu:
>>>
>>> # qemu-system-x86_64 -hda myimage -S -s &
>>> # gdb
>>> (gdb) tar re :1234
>>> Remote debugging using :1234
>>> Remote 'g' packet reply is too long:
>>> 000000000000000000000000000000000000[...]
>>>
>>> This issue did not occur with QEMU from CVS. As I was aware of an
>>> x86_64-related problem in QEMU's gdbstub, I merged its current state
>>> into kvm - and things start to work again. Find the update patch below.
>>>     
>>
>> Any feedback on this? Any chance to see this patch (or some similar
>> version) in kvm-57?
>>
>>   
> 
> Sorry for the non-response. I am hoping to get qemu-cvs merged for
> kvm-57, which will naturally include this. If that doesn't work out
> (i.e. we discover regressions), and if this is important for you, I will
> apply the patch.

The qemu update did take place for -57, but a bit too mechanically.
Please merge this patch to finally fix the gdbstub.

Thanks,
Jan



Remove left-over from qemu CVS update.

Signed-off-by: Jan Kiszka <jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>

---
 qemu/gdbstub.c |  136 ---------------------------------------------------------
 1 file changed, 1 insertion(+), 135 deletions(-)

Index: kvm-userspace/qemu/gdbstub.c
===================================================================
--- kvm-userspace.orig/qemu/gdbstub.c
+++ kvm-userspace/qemu/gdbstub.c
@@ -225,141 +225,7 @@ static int put_packet(GDBState *s, char 
     return 0;
 }
 
-#if defined(TARGET_X86_64)
-
-static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
-{
-    uint8_t *p = mem_buf;
-    int i, fpus;
-
-#define PUTREG(x) do { \
-	target_ulong reg = tswapl(x); \
-	memcpy(p, &reg, sizeof reg); \
-	p += sizeof reg; \
-    } while (0)
-#define PUTREG32(x) do { \
-	uint32_t reg = tswap32(x);		\
-	memcpy(p, &reg, sizeof reg); \
-	p += sizeof reg; \
-    } while (0)
-#define PUTREGF(x) do { \
-	memcpy(p, &(x), 10);			\
-	p += sizeof (x);			\
-    } while (0)
-
-    PUTREG(env->regs[R_EAX]);
-    PUTREG(env->regs[R_EBX]);
-    PUTREG(env->regs[R_ECX]);
-    PUTREG(env->regs[R_EDX]);
-    PUTREG(env->regs[R_ESI]);
-    PUTREG(env->regs[R_EDI]);
-    PUTREG(env->regs[R_EBP]);
-    PUTREG(env->regs[R_ESP]);
-    PUTREG(env->regs[8]);
-    PUTREG(env->regs[9]);
-    PUTREG(env->regs[10]);
-    PUTREG(env->regs[11]);
-    PUTREG(env->regs[12]);
-    PUTREG(env->regs[13]);
-    PUTREG(env->regs[14]);
-    PUTREG(env->regs[15]);
-
-    PUTREG(env->eip);
-    PUTREG32(env->eflags);
-    PUTREG32(env->segs[R_CS].selector);
-    PUTREG32(env->segs[R_SS].selector);
-    PUTREG32(env->segs[R_DS].selector);
-    PUTREG32(env->segs[R_ES].selector);
-    PUTREG32(env->segs[R_FS].selector);
-    PUTREG32(env->segs[R_GS].selector);
-    /* XXX: convert floats */
-    for(i = 0; i < 8; i++) {
-        PUTREGF(env->fpregs[i]);
-    }
-    PUTREG32(env->fpuc);
-    fpus = (env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11;
-    PUTREG32(fpus);
-    PUTREG32(0); /* XXX: convert tags */
-    PUTREG32(0); /* fiseg */
-    PUTREG32(0); /* fioff */
-    PUTREG32(0); /* foseg */
-    PUTREG32(0); /* fooff */
-    PUTREG32(0); /* fop */
-
-#undef PUTREG
-#undef PUTREG32
-#undef PUTREGF
-
-    return p - mem_buf;
-}
-
-static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
-{
-    uint8_t *p = mem_buf;
-    uint32_t junk;
-    int i, fpus;
-
-#define GETREG(x) do { \
-	target_ulong reg; \
-    	memcpy(&reg, p, sizeof reg);     \
-	x = tswapl(reg);		 \
-	p += sizeof reg;                 \
-    } while (0)
-#define GETREG32(x) do { \
-	uint32_t reg; \
-    	memcpy(&reg, p, sizeof reg);     \
-	x = tswap32(reg);		 \
-	p += sizeof reg;                 \
-    } while (0)
-#define GETREGF(x) do { \
-    	memcpy(&(x), p, 10);		 \
-	p += 10;                         \
-    } while (0)
-
-    GETREG(env->regs[R_EAX]);
-    GETREG(env->regs[R_EBX]);
-    GETREG(env->regs[R_ECX]);
-    GETREG(env->regs[R_EDX]);
-    GETREG(env->regs[R_ESI]);
-    GETREG(env->regs[R_EDI]);
-    GETREG(env->regs[R_EBP]);
-    GETREG(env->regs[R_ESP]);
-    GETREG(env->regs[8]);
-    GETREG(env->regs[9]);
-    GETREG(env->regs[10]);
-    GETREG(env->regs[11]);
-    GETREG(env->regs[12]);
-    GETREG(env->regs[13]);
-    GETREG(env->regs[14]);
-    GETREG(env->regs[15]);
-
-    GETREG(env->eip);
-    GETREG32(env->eflags);
-    GETREG32(env->segs[R_CS].selector);
-    GETREG32(env->segs[R_SS].selector);
-    GETREG32(env->segs[R_DS].selector);
-    GETREG32(env->segs[R_ES].selector);
-    GETREG32(env->segs[R_FS].selector);
-    GETREG32(env->segs[R_GS].selector);
-    /* XXX: convert floats */
-    for(i = 0; i < 8; i++) {
-        GETREGF(env->fpregs[i]);
-    }
-    GETREG32(env->fpuc);
-    GETREG32(fpus); /* XXX: convert fpus */
-    GETREG32(junk); /* XXX: convert tags */
-    GETREG32(junk); /* fiseg */
-    GETREG32(junk); /* fioff */
-    GETREG32(junk); /* foseg */
-    GETREG32(junk); /* fooff */
-    GETREG32(junk); /* fop */
-
-#undef GETREG
-#undef GETREG32
-#undef GETREGF
-}
-
-#elif defined(TARGET_I386)
+#if defined(TARGET_I386)
 
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] align gdbstub with QEMU

[Avi Kivity]

Jan Kiszka wrote:
> The qemu update did take place for -57, but a bit too mechanically.
> Please merge this patch to finally fix the gdbstub.
>
>   

Thanks, applied.

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/