[Qemu-devel] [PATCH v10 0/2] target-ppc: gdbstub little endian support patches

[Qemu-devel] [PATCH v10 0/2] target-ppc: gdbstub little endian support patches

[Thomas Falcon]

The first patch pulls the register length calculation into its own
function, and the second adds support for little endian ppc in gdbstub.

Thomas Falcon (2):
  target-ppc: extract register length calculation in gdbstub
  target-ppc: gdbstub allow byte swapping for reading/writing registers

 target-ppc/gdbstub.c | 124 +++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 90 insertions(+), 34 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v10 1/2] target-ppc: extract register length calculation in gdbstub

[Thomas Falcon]

This patch extracts the method to determine a register's size
into a separate function.

Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
 target-ppc/gdbstub.c | 105 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 34 deletions(-)

diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
index 1c91090..0740af8 100644
--- a/target-ppc/gdbstub.c
+++ b/target-ppc/gdbstub.c
@@ -21,6 +21,44 @@
 #include "qemu-common.h"
 #include "exec/gdbstub.h"
 
+static int ppc_gdb_register_len(int n)
+{
+    switch (n) {
+    case 0 ... 31:
+        /* gprs */
+        return sizeof(target_ulong);
+    case 32 ... 63:
+        /* fprs */
+        if (gdb_has_xml) {
+            return 0;
+        }
+        return 8;
+    case 66:
+        /* cr */
+        return 4;
+    case 64:
+        /* nip */
+    case 65:
+        /* msr */
+    case 67:
+        /* lr */
+    case 68:
+        /* ctr */
+    case 69:
+        /* xer */
+        return sizeof(target_ulong);
+    case 70:
+        /* fpscr */
+        if (gdb_has_xml) {
+            return 0;
+        }
+        return sizeof(target_ulong);
+    default:
+        return 0;
+    }
+}
+
+
 /* Old gdb always expects FP registers.  Newer (xml-aware) gdb only
  * expects whatever the target description contains.  Due to a
  * historical mishap the FP registers appear in between core integer
@@ -32,23 +70,26 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
+    int r = ppc_gdb_register_len(n);
+
+    if (!r) {
+        return r;
+    }
 
     if (n < 32) {
         /* gprs */
-        return gdb_get_regl(mem_buf, env->gpr[n]);
+        gdb_get_regl(mem_buf, env->gpr[n]);
     } else if (n < 64) {
         /* fprs */
-        if (gdb_has_xml) {
-            return 0;
-        }
         stfq_p(mem_buf, env->fpr[n-32]);
-        return 8;
     } else {
         switch (n) {
         case 64:
-            return gdb_get_regl(mem_buf, env->nip);
+            gdb_get_regl(mem_buf, env->nip);
+            break;
         case 65:
-            return gdb_get_regl(mem_buf, env->msr);
+            gdb_get_regl(mem_buf, env->msr);
+            break;
         case 66:
             {
                 uint32_t cr = 0;
@@ -56,50 +97,49 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
                 for (i = 0; i < 8; i++) {
                     cr |= env->crf[i] << (32 - ((i + 1) * 4));
                 }
-                return gdb_get_reg32(mem_buf, cr);
+                gdb_get_reg32(mem_buf, cr);
+                break;
             }
         case 67:
-            return gdb_get_regl(mem_buf, env->lr);
+            gdb_get_regl(mem_buf, env->lr);
+            break;
         case 68:
-            return gdb_get_regl(mem_buf, env->ctr);
+            gdb_get_regl(mem_buf, env->ctr);
+            break;
         case 69:
-            return gdb_get_regl(mem_buf, env->xer);
+            gdb_get_regl(mem_buf, env->xer);
+            break;
         case 70:
-            {
-                if (gdb_has_xml) {
-                    return 0;
-                }
-                return gdb_get_reg32(mem_buf, env->fpscr);
-            }
+            gdb_get_reg32(mem_buf, env->fpscr);
+            break;
         }
     }
-    return 0;
+    return r;
 }
 
 int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
+    int r = ppc_gdb_register_len(n);
 
+    if (!r) {
+        return r;
+    }
     if (n < 32) {
         /* gprs */
         env->gpr[n] = ldtul_p(mem_buf);
-        return sizeof(target_ulong);
     } else if (n < 64) {
         /* fprs */
-        if (gdb_has_xml) {
-            return 0;
-        }
         env->fpr[n-32] = ldfq_p(mem_buf);
-        return 8;
     } else {
         switch (n) {
         case 64:
             env->nip = ldtul_p(mem_buf);
-            return sizeof(target_ulong);
+            break;
         case 65:
             ppc_store_msr(env, ldtul_p(mem_buf));
-            return sizeof(target_ulong);
+            break;
         case 66:
             {
                 uint32_t cr = ldl_p(mem_buf);
@@ -107,25 +147,22 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
                 for (i = 0; i < 8; i++) {
                     env->crf[i] = (cr >> (32 - ((i + 1) * 4))) & 0xF;
                 }
-                return 4;
+                break;
             }
         case 67:
             env->lr = ldtul_p(mem_buf);
-            return sizeof(target_ulong);
+            break;
         case 68:
             env->ctr = ldtul_p(mem_buf);
-            return sizeof(target_ulong);
+            break;
         case 69:
             env->xer = ldtul_p(mem_buf);
-            return sizeof(target_ulong);
+            break;
         case 70:
             /* fpscr */
-            if (gdb_has_xml) {
-                return 0;
-            }
             store_fpscr(env, ldtul_p(mem_buf), 0xffffffff);
-            return sizeof(target_ulong);
+            break;
         }
     }
-    return 0;
+    return r;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v10 2/2] target-ppc: gdbstub allow byte swapping for reading/writing registers

[Thomas Falcon]

This patch allows registers to be properly read from and written to
when using the gdbstub to debug a ppc guest running in little
endian mode.

Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
---
Difference from v9:

Fixed a coding style mistake
---
 target-ppc/gdbstub.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/target-ppc/gdbstub.c b/target-ppc/gdbstub.c
index 0740af8..381a3c7 100644
--- a/target-ppc/gdbstub.c
+++ b/target-ppc/gdbstub.c
@@ -59,6 +59,17 @@ static int ppc_gdb_register_len(int n)
 }
 
 
+static void ppc_gdb_swap_register(uint8_t *mem_buf, int n, int len)
+{
+    if (len == 4) {
+        bswap32s((uint32_t *)mem_buf);
+    } else if (len == 8) {
+        bswap64s((uint64_t *)mem_buf);
+    } else {
+        g_assert_not_reached();
+    }
+}
+
 /* Old gdb always expects FP registers.  Newer (xml-aware) gdb only
  * expects whatever the target description contains.  Due to a
  * historical mishap the FP registers appear in between core integer
@@ -114,6 +125,10 @@ int ppc_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
             break;
         }
     }
+    if (msr_le) {
+        /* If cpu is in LE mode, convert memory contents to LE. */
+        ppc_gdb_swap_register(mem_buf, n, r);
+    }
     return r;
 }
 
@@ -126,6 +141,10 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     if (!r) {
         return r;
     }
+    if (msr_le) {
+        /* If cpu is in LE mode, convert memory contents to LE. */
+        ppc_gdb_swap_register(mem_buf, n, r);
+    }
     if (n < 32) {
         /* gprs */
         env->gpr[n] = ldtul_p(mem_buf);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v10 0/2] target-ppc: gdbstub little endian support patches

[Alexander Graf]

On 07.04.2014, at 23:40, Thomas Falcon <tlfalcon@linux.vnet.ibm.com> wrote:

> The first patch pulls the register length calculation into its own
> function, and the second adds support for little endian ppc in gdbstub.

Thanks, applied to ppc-next.

(For some reason your to: mails don't arrive in my inbox, I only see the ML copies. Strange...)


Alex