[Qemu-devel] [PATCH 1/3] gdbstub: x86: Refactor register access

[Jan Kiszka]

Refactor cpu_gdb_read_register and cpu_gdb_write_register, basically
dispatching all core registers via a single switch-case and making
register indexes easier readable.

Signed-off-by: Jan Kiszka <address@hidden>
---
 gdbstub.c |  166 +++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 90 insertions(+), 76 deletions(-)

Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -307,81 +307,76 @@ static const int gpr_map[8] = {0, 1, 2,
 
 #define NUM_CORE_REGS (CPU_NB_REGS * 2 + 25)
 
+#define IDX_IP_REG      CPU_NB_REGS
+#define IDX_FLAG_REG    (IDX_IP_REG + 1)
+#define IDX_SEG_REGS    (IDX_FLAG_REG + 1)
+#define IDX_FP_REGS     (IDX_SEG_REGS + 6)
+#define IDX_XMM_REGS    (IDX_FP_REGS + 16)
+
 static int cpu_gdb_read_register(CPUState *env, uint8_t *mem_buf, int n)
 {
-    if (n < CPU_NB_REGS) {
+    switch (n) {
+    case 0 ... CPU_NB_REGS - 1:
         GET_REGL(env->regs[gpr_map[n]]);
-    } else if (n >= CPU_NB_REGS + 8 && n < CPU_NB_REGS + 16) {
-        /* FIXME: byteswap float values.  */
+
+    case IDX_IP_REG:   GET_REGL(env->eip);
+    case IDX_FLAG_REG: GET_REG32(env->eflags);
+
+    case IDX_SEG_REGS + 0: GET_REG32(env->segs[R_CS].selector);
+    case IDX_SEG_REGS + 1: GET_REG32(env->segs[R_SS].selector);
+    case IDX_SEG_REGS + 2: GET_REG32(env->segs[R_DS].selector);
+    case IDX_SEG_REGS + 3: GET_REG32(env->segs[R_ES].selector);
+    case IDX_SEG_REGS + 4: GET_REG32(env->segs[R_FS].selector);
+    case IDX_SEG_REGS + 5: GET_REG32(env->segs[R_GS].selector);
+
+    case IDX_FP_REGS ... IDX_FP_REGS + 7:
 #ifdef USE_X86LDOUBLE
+        /* FIXME: byteswap float values.  */
         memcpy(mem_buf, &env->fpregs[n - (CPU_NB_REGS + 8)], 10);
 #else
         memset(mem_buf, 0, 10);
 #endif
         return 10;
-    } else if (n >= CPU_NB_REGS + 24) {
-        n -= CPU_NB_REGS + 24;
+    case IDX_FP_REGS + 8:  GET_REG32(env->fpuc);
+    case IDX_FP_REGS + 9:  GET_REG32((env->fpus & ~0x3800) |
+                                     (env->fpstt & 0x7) << 11);
+    case IDX_FP_REGS + 10: GET_REG32(0); /* ftag */
+    case IDX_FP_REGS + 11: GET_REG32(0); /* fiseg */
+    case IDX_FP_REGS + 12: GET_REG32(0); /* fioff */
+    case IDX_FP_REGS + 13: GET_REG32(0); /* foseg */
+    case IDX_FP_REGS + 14: GET_REG32(0); /* fooff */
+    case IDX_FP_REGS + 15: GET_REG32(0); /* fop */
+
+    case IDX_XMM_REGS ... IDX_XMM_REGS + CPU_NB_REGS:
+        n -= IDX_XMM_REGS;
         if (n < CPU_NB_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;
-        } else if (n == CPU_NB_REGS) {
+        } else
             GET_REG32(env->mxcsr);
-        } 
-    } else {
-        n -= CPU_NB_REGS;
-        switch (n) {
-        case 0: GET_REGL(env->eip);
-        case 1: GET_REG32(env->eflags);
-        case 2: GET_REG32(env->segs[R_CS].selector);
-        case 3: GET_REG32(env->segs[R_SS].selector);
-        case 4: GET_REG32(env->segs[R_DS].selector);
-        case 5: GET_REG32(env->segs[R_ES].selector);
-        case 6: GET_REG32(env->segs[R_FS].selector);
-        case 7: GET_REG32(env->segs[R_GS].selector);
-        /* 8...15 x87 regs.  */
-        case 16: GET_REG32(env->fpuc);
-        case 17: GET_REG32((env->fpus & ~0x3800) | (env->fpstt & 0x7) << 11);
-        case 18: GET_REG32(0); /* ftag */
-        case 19: GET_REG32(0); /* fiseg */
-        case 20: GET_REG32(0); /* fioff */
-        case 21: GET_REG32(0); /* foseg */
-        case 22: GET_REG32(0); /* fooff */
-        case 23: GET_REG32(0); /* fop */
-        /* 24+ xmm regs.  */
-        }
+
+    default: /* Unrecognised register */
+        return 0;
     }
-    return 0;
 }
 
-static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int i)
+static int cpu_gdb_write_register(CPUState *env, uint8_t *mem_buf, int n)
 {
     uint32_t tmp;
 
-    if (i < CPU_NB_REGS) {
-        env->regs[gpr_map[i]] = ldtul_p(mem_buf);
+    switch (n) {
+    case 0 ... CPU_NB_REGS - 1:
+        env->regs[gpr_map[n]] = ldtul_p(mem_buf);
         return sizeof(target_ulong);
-    } else if (i >= CPU_NB_REGS + 8 && i < CPU_NB_REGS + 16) {
-        i -= CPU_NB_REGS + 8;
-#ifdef USE_X86LDOUBLE
-        memcpy(&env->fpregs[i], mem_buf, 10);
-#endif
-        return 10;
-    } else if (i >= CPU_NB_REGS + 24) {
-        i -= CPU_NB_REGS + 24;
-        if (i < CPU_NB_REGS) {
-            env->xmm_regs[i].XMM_Q(0) = ldq_p(mem_buf);
-            env->xmm_regs[i].XMM_Q(1) = ldq_p(mem_buf + 8);
-            return 16;
-        } else if (i == CPU_NB_REGS) {
-            env->mxcsr = ldl_p(mem_buf);
-            return 4;
-        }
-    } else {
-        i -= CPU_NB_REGS;
-        switch (i) {
-        case 0: env->eip = ldtul_p(mem_buf); return sizeof(target_ulong);
-        case 1: env->eflags = ldl_p(mem_buf); return 4;
+
+    case IDX_IP_REG:
+        env->eip = ldtul_p(mem_buf);
+        return sizeof(target_ulong);
+    case IDX_FLAG_REG:
+        env->eflags = ldl_p(mem_buf);
+        return 4;
+
 #if defined(CONFIG_USER_ONLY)
 #define LOAD_SEG(index, sreg)\
             tmp = ldl_p(mem_buf);\
@@ -392,30 +387,49 @@ static int cpu_gdb_write_register(CPUSta
    when the selector is invalid.  */
 #define LOAD_SEG(index, sreg) do {} while(0)
 #endif
-        case 2: LOAD_SEG(10, R_CS); return 4;
-        case 3: LOAD_SEG(11, R_SS); return 4;
-        case 4: LOAD_SEG(12, R_DS); return 4;
-        case 5: LOAD_SEG(13, R_ES); return 4;
-        case 6: LOAD_SEG(14, R_FS); return 4;
-        case 7: LOAD_SEG(15, R_GS); return 4;
-        /* 8...15 x87 regs.  */
-        case 16: env->fpuc = ldl_p(mem_buf); return 4;
-        case 17:
-                 tmp = ldl_p(mem_buf);
-                 env->fpstt = (tmp >> 11) & 7;
-                 env->fpus = tmp & ~0x3800;
-                 return 4;
-        case 18: /* ftag */ return 4;
-        case 19: /* fiseg */ return 4;
-        case 20: /* fioff */ return 4;
-        case 21: /* foseg */ return 4;
-        case 22: /* fooff */ return 4;
-        case 23: /* fop */ return 4;
-        /* 24+ xmm regs.  */
+    case IDX_SEG_REGS + 0: LOAD_SEG(10, R_CS); return 4;
+    case IDX_SEG_REGS + 1: LOAD_SEG(11, R_SS); return 4;
+    case IDX_SEG_REGS + 2: LOAD_SEG(12, R_DS); return 4;
+    case IDX_SEG_REGS + 3: LOAD_SEG(13, R_ES); return 4;
+    case IDX_SEG_REGS + 4: LOAD_SEG(14, R_FS); return 4;
+    case IDX_SEG_REGS + 5: LOAD_SEG(15, R_GS); return 4;
+
+    case IDX_FP_REGS ... IDX_FP_REGS + 7:
+#ifdef USE_X86LDOUBLE
+        /* FIXME: byteswap float values.  */
+        memcpy(&env->fpregs[n - CPU_NB_REGS - 8], mem_buf, 10);
+#endif
+        return 10;
+    case IDX_FP_REGS + 8:
+        env->fpuc = ldl_p(mem_buf);
+        return 4;
+    case IDX_FP_REGS + 9:
+        tmp = ldl_p(mem_buf);
+        env->fpstt = (tmp >> 11) & 7;
+        env->fpus = tmp & ~0x3800;
+        return 4;
+    case IDX_FP_REGS + 10: /* ftag */
+    case IDX_FP_REGS + 11: /* fiseg */
+    case IDX_FP_REGS + 12: /* fioff */
+    case IDX_FP_REGS + 13: /* foseg */
+    case IDX_FP_REGS + 14: /* fooff */
+    case IDX_FP_REGS + 15: /* fop */
+        return 4;
+
+    case IDX_XMM_REGS ... IDX_XMM_REGS + CPU_NB_REGS:
+        n -= IDX_XMM_REGS;
+        if (n < CPU_NB_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;
+        } else {
+            env->mxcsr = ldl_p(mem_buf);
+            return 4;
         }
+
+    default: /* Unrecognised register */
+        return 0;
     }
-    /* Unrecognised register.  */
-    return 0;
 }
 
 #elif defined (TARGET_PPC)