i386 gdbstub handles both i386 and x86_64. Factor out two functions
for reading and writing registers without knowing their bitness.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/i386/gdbstub.c | 52 ++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 21 deletions(-)

diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index 4acf485879e..ec64ab6c53f 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -96,6 +96,19 @@ static int gdb_write_reg_cs64(uint32_t hflags, uint8_t *buf, target_ulong *val)
     return 4;
 }
 
+static int gdb_get_reg(CPUX86State *env, GByteArray *mem_buf, target_ulong val)
+{
+    if (TARGET_LONG_BITS == 64) {
+        if (env->hflags & HF_CS64_MASK) {
+            return gdb_get_reg64(mem_buf, val);
+        } else {
+            return gdb_get_reg64(mem_buf, val & 0xffffffffUL);
+        }
+    } else {
+        return gdb_get_reg32(mem_buf, val);
+    }
+}
+
 int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
     X86CPU *cpu = X86_CPU(cs);
@@ -137,15 +150,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
     } else {
         switch (n) {
         case IDX_IP_REG:
-            if (TARGET_LONG_BITS == 64) {
-                if (env->hflags & HF_CS64_MASK) {
-                    return gdb_get_reg64(mem_buf, env->eip);
-                } else {
-                    return gdb_get_reg64(mem_buf, env->eip & 0xffffffffUL);
-                }
-            } else {
-                return gdb_get_reg32(mem_buf, env->eip);
-            }
+            return gdb_get_reg(env, mem_buf, env->eip);
         case IDX_FLAGS_REG:
             return gdb_get_reg32(mem_buf, env->eflags);
 
@@ -248,6 +253,22 @@ static int x86_cpu_gdb_load_seg(X86CPU *cpu, X86Seg sreg, uint8_t *mem_buf)
     return 4;
 }
 
+static int gdb_write_reg(CPUX86State *env, uint8_t *mem_buf, target_ulong *val)
+{
+    if (TARGET_LONG_BITS == 64) {
+        if (env->hflags & HF_CS64_MASK) {
+            *val = ldq_p(mem_buf);
+        } else {
+            *val = ldq_p(mem_buf) & 0xffffffffUL;
+        }
+        return 8;
+    } else {
+        *val &= ~0xffffffffUL;
+        *val |= (uint32_t)ldl_p(mem_buf);
+        return 4;
+    }
+}
+
 int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     X86CPU *cpu = X86_CPU(cs);
@@ -288,18 +309,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
     } else {
         switch (n) {
         case IDX_IP_REG:
-            if (TARGET_LONG_BITS == 64) {
-                if (env->hflags & HF_CS64_MASK) {
-                    env->eip = ldq_p(mem_buf);
-                } else {
-                    env->eip = ldq_p(mem_buf) & 0xffffffffUL;
-                }
-                return 8;
-            } else {
-                env->eip &= ~0xffffffffUL;
-                env->eip |= (uint32_t)ldl_p(mem_buf);
-                return 4;
-            }
+            return gdb_write_reg(env, mem_buf, &env->eip);
         case IDX_FLAGS_REG:
             env->eflags = ldl_p(mem_buf);
             return 4;
-- 
2.45.2

On 8/2/24 02:59, Ilya Leoshkevich wrote:
> @@ -248,6 +253,22 @@ static int x86_cpu_gdb_load_seg(X86CPU *cpu, X86Seg sreg, uint8_t *mem_buf)
>       return 4;
>   }
>   
> +static int gdb_write_reg(CPUX86State *env, uint8_t *mem_buf, target_ulong *val)
> +{
> +    if (TARGET_LONG_BITS == 64) {
> +        if (env->hflags & HF_CS64_MASK) {
> +            *val = ldq_p(mem_buf);
> +        } else {
> +            *val = ldq_p(mem_buf) & 0xffffffffUL;
> +        }
> +        return 8;
> +    } else {
> +        *val &= ~0xffffffffUL;
> +        *val |= (uint32_t)ldl_p(mem_buf);
> +        return 4;
> +    }
> +}
> +
>   int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>   {
>       X86CPU *cpu = X86_CPU(cs);
> @@ -288,18 +309,7 @@ int x86_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>       } else {
>           switch (n) {
>           case IDX_IP_REG:
> -            if (TARGET_LONG_BITS == 64) {
> -                if (env->hflags & HF_CS64_MASK) {
> -                    env->eip = ldq_p(mem_buf);
> -                } else {
> -                    env->eip = ldq_p(mem_buf) & 0xffffffffUL;
> -                }
> -                return 8;
> -            } else {
> -                env->eip &= ~0xffffffffUL;
> -                env->eip |= (uint32_t)ldl_p(mem_buf);
> -                return 4;
> -            }
> +            return gdb_write_reg(env, mem_buf, &env->eip);

Existing bug, but the insert in the !(TARGET_LONG_BITS == 64) case is silly.
Because TARGET_LONG_BITS == 32, target_ulong eip is 32-bits, so the "insert" doesn't 
really insert anything.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~