[PATCH 0/3] 64 Bit support for hppa gdbstub

[PATCH 0/3] 64 Bit support for hppa gdbstub

[Sven Schnelle]

Hi List,

this patchset allows to debug the hppa target when running in wide (64 bit)
mode. gdb needs a small patch to switch to 64 bit mode. I pushed the
patch to https://github.com/bminor/binutils-gdb/commit/fd8662ec282d688d1f8100b4365823e57516857b
With this patch gdb will switch to the appropriate mode when connecting
to qemu/gdbstub.

Sven Schnelle (3):
  Revert "target/hppa: Drop attempted gdbstub support for hppa64"
  target/hppa: add 64 bit support to gdbstub
  target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub

 target/hppa/gdbstub.c | 66 +++++++++++++++++++++++++++++--------------
 1 file changed, 45 insertions(+), 21 deletions(-)

-- 
2.43.2

[PATCH 1/3] Revert "target/hppa: Drop attempted gdbstub support for hppa64"

[Sven Schnelle]

Despite commit e207b4aa718e ("target/hppa: Drop attempted gdbstub
support for hppa64") saying that hppa-linux-gdb doesn't support 64 bit
mode via remote protocol, it is actually working with a small add-on
patch which enables gdb to guess the size from the g protocol:

$ hppa64-linux-gnu-gdb ~/seabios-hppa/out-64/hppa-firmware64.img
[..]
Reading symbols from /home/svens/seabios-hppa/out-64/hppa-firmware64.img...
(gdb) target remote :1234
Remote debugging using :1234
warning: remote target does not support file transfer, attempting to access files from local filesystem.
warning: Unable to find dynamic linker breakpoint function.
GDB will be unable to debug shared library initializers
and track explicitly loaded dynamic code.
startup () at src/parisc/head.S:144
144		rsm	PSW_I, %r0	/* disable local irqs */
(gdb)

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/gdbstub.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
index 4a965b38d7..48a514384f 100644
--- a/target/hppa/gdbstub.c
+++ b/target/hppa/gdbstub.c
@@ -21,16 +21,11 @@
 #include "cpu.h"
 #include "gdbstub/helpers.h"
 
-/*
- * GDB 15 only supports PA1.0 via the remote protocol, and ignores
- * any provided xml.  Which means that any attempt to provide more
- * data results in "Remote 'g' packet reply is too long".
- */
-
 int hppa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
-    CPUHPPAState *env = cpu_env(cs);
-    uint32_t val;
+    HPPACPU *cpu = HPPA_CPU(cs);
+    CPUHPPAState *env = &cpu->env;
+    target_ureg val;
 
     switch (n) {
     case 0:
@@ -144,13 +139,24 @@ int hppa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
         break;
     }
 
-    return gdb_get_reg32(mem_buf, val);
+    if (TARGET_REGISTER_BITS == 64) {
+        return gdb_get_reg64(mem_buf, val);
+    } else {
+        return gdb_get_reg32(mem_buf, val);
+    }
 }
 
 int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
-    CPUHPPAState *env = cpu_env(cs);
-    uint32_t val = ldl_p(mem_buf);
+    HPPACPU *cpu = HPPA_CPU(cs);
+    CPUHPPAState *env = &cpu->env;
+    target_ureg val;
+
+    if (TARGET_REGISTER_BITS == 64) {
+        val = ldq_p(mem_buf);
+    } else {
+        val = ldl_p(mem_buf);
+    }
 
     switch (n) {
     case 0:
@@ -160,7 +166,7 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         env->gr[n] = val;
         break;
     case 32:
-        env->cr[CR_SAR] = val & (hppa_is_pa20(env) ? 63 : 31);
+        env->cr[CR_SAR] = val;
         break;
     case 33:
         env->iaoq_f = val;
@@ -272,5 +278,5 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         }
         break;
     }
-    return 4;
+    return sizeof(target_ureg);
 }
-- 
2.43.2

[PATCH 2/3] target/hppa: add 64 bit support to gdbstub

[Sven Schnelle]

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/gdbstub.c | 48 +++++++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 15 deletions(-)

diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
index 48a514384f..a5b2c80c07 100644
--- a/target/hppa/gdbstub.c
+++ b/target/hppa/gdbstub.c
@@ -21,11 +21,25 @@
 #include "cpu.h"
 #include "gdbstub/helpers.h"
 
+static int hppa_num_regs(CPUHPPAState *env)
+{
+    return hppa_is_pa20(env) ? 96 : 128;
+}
+
+static int hppa_reg_size(CPUHPPAState *env)
+{
+    return hppa_is_pa20(env) ? 8 : 4;
+}
+
 int hppa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
 {
     HPPACPU *cpu = HPPA_CPU(cs);
     CPUHPPAState *env = &cpu->env;
-    target_ureg val;
+    target_ulong val;
+
+    if (n >= hppa_num_regs(env)) {
+        return 0;
+    }
 
     switch (n) {
     case 0:
@@ -128,18 +142,18 @@ int hppa_cpu_gdb_read_register(CPUState *cs, GByteArray *mem_buf, int n)
         val = env->cr[30];
         break;
     case 64 ... 127:
-        val = extract64(env->fr[(n - 64) / 2], (n & 1 ? 0 : 32), 32);
-        break;
-    default:
-        if (n < 128) {
-            val = 0;
+        if (hppa_is_pa20(env)) {
+            val = env->fr[n - 64];
         } else {
-            return 0;
+            val = extract64(env->fr[(n - 64) / 2], (n & 1 ? 0 : 32), 32);
         }
         break;
+    default:
+        val = 0;
+        break;
     }
 
-    if (TARGET_REGISTER_BITS == 64) {
+    if (hppa_is_pa20(env)) {
         return gdb_get_reg64(mem_buf, val);
     } else {
         return gdb_get_reg32(mem_buf, val);
@@ -150,9 +164,13 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     HPPACPU *cpu = HPPA_CPU(cs);
     CPUHPPAState *env = &cpu->env;
-    target_ureg val;
+    target_ulong val;
+
+    if (n >= hppa_num_regs(env)) {
+        return 0;
+    }
 
-    if (TARGET_REGISTER_BITS == 64) {
+    if (hppa_is_pa20(env)) {
         val = ldq_p(mem_buf);
     } else {
         val = ldl_p(mem_buf);
@@ -267,16 +285,16 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         cpu_hppa_loaded_fr0(env);
         break;
     case 65 ... 127:
-        {
+        if (hppa_is_pa20(env)) {
+            env->fr[n - 64] = val;
+        } else {
             uint64_t *fr = &env->fr[(n - 64) / 2];
             *fr = deposit64(*fr, (n & 1 ? 0 : 32), 32, val);
         }
         break;
     default:
-        if (n >= 128) {
-            return 0;
-        }
         break;
     }
-    return sizeof(target_ureg);
+
+    return hppa_reg_size(env);
 }
-- 
2.43.2

[PATCH 3/3] target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub

[Sven Schnelle]

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/gdbstub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
index a5b2c80c07..049b2d6381 100644
--- a/target/hppa/gdbstub.c
+++ b/target/hppa/gdbstub.c
@@ -184,7 +184,7 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         env->gr[n] = val;
         break;
     case 32:
-        env->cr[CR_SAR] = val;
+        env->cr[CR_SAR] = val & (hppa_is_pa20(env) ? 63 : 31);
         break;
     case 33:
         env->iaoq_f = val;
-- 
2.43.2

Re: [PATCH 3/3] target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub

[Richard Henderson]

On 2/28/24 10:14, Sven Schnelle wrote:
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/gdbstub.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
> index a5b2c80c07..049b2d6381 100644
> --- a/target/hppa/gdbstub.c
> +++ b/target/hppa/gdbstub.c
> @@ -184,7 +184,7 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>           env->gr[n] = val;
>           break;
>       case 32:
> -        env->cr[CR_SAR] = val;
> +        env->cr[CR_SAR] = val & (hppa_is_pa20(env) ? 63 : 31);
>           break;
>       case 33:
>           env->iaoq_f = val;

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


r~

Re: [PATCH 0/3] 64 Bit support for hppa gdbstub

[Sven Schnelle]

Hi Richard,

Sven Schnelle <svens@stackframe.org> writes:

> Hi List,
>
> this patchset allows to debug the hppa target when running in wide (64 bit)
> mode. gdb needs a small patch to switch to 64 bit mode. I pushed the
> patch to https://github.com/bminor/binutils-gdb/commit/fd8662ec282d688d1f8100b4365823e57516857b
> With this patch gdb will switch to the appropriate mode when connecting
> to qemu/gdbstub.
>
> Sven Schnelle (3):
>   Revert "target/hppa: Drop attempted gdbstub support for hppa64"
>   target/hppa: add 64 bit support to gdbstub
>   target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub
>
>  target/hppa/gdbstub.c | 66 +++++++++++++++++++++++++++++--------------
>  1 file changed, 45 insertions(+), 21 deletions(-)

gentle ping - if i followed correctly only one patch was reviewed so far.

Re: [PATCH 0/3] 64 Bit support for hppa gdbstub

[Richard Henderson]

On 3/17/24 20:32, Sven Schnelle wrote:
> Hi Richard,
> 
> Sven Schnelle <svens@stackframe.org> writes:
> 
>> Hi List,
>>
>> this patchset allows to debug the hppa target when running in wide (64 bit)
>> mode. gdb needs a small patch to switch to 64 bit mode. I pushed the
>> patch to https://github.com/bminor/binutils-gdb/commit/fd8662ec282d688d1f8100b4365823e57516857b
>> With this patch gdb will switch to the appropriate mode when connecting
>> to qemu/gdbstub.
>>
>> Sven Schnelle (3):
>>    Revert "target/hppa: Drop attempted gdbstub support for hppa64"
>>    target/hppa: add 64 bit support to gdbstub
>>    target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub
>>
>>   target/hppa/gdbstub.c | 66 +++++++++++++++++++++++++++++--------------
>>   1 file changed, 45 insertions(+), 21 deletions(-)
> 
> gentle ping - if i followed correctly only one patch was reviewed so far.

We can't really proceed until you get your gdb patch reviewed upstream.
All that will happen in the meantime is that qemu make-check will fail.


r~

Re: [PATCH 0/3] 64 Bit support for hppa gdbstub

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> On 3/17/24 20:32, Sven Schnelle wrote:
>> Hi Richard,
>> Sven Schnelle <svens@stackframe.org> writes:
>> 
>>> Hi List,
>>>
>>> this patchset allows to debug the hppa target when running in wide (64 bit)
>>> mode. gdb needs a small patch to switch to 64 bit mode. I pushed the
>>> patch to https://github.com/bminor/binutils-gdb/commit/fd8662ec282d688d1f8100b4365823e57516857b
>>> With this patch gdb will switch to the appropriate mode when connecting
>>> to qemu/gdbstub.
>>>
>>> Sven Schnelle (3):
>>>    Revert "target/hppa: Drop attempted gdbstub support for hppa64"
>>>    target/hppa: add 64 bit support to gdbstub
>>>    target/hppa: mask CR_SAR register writes to 5/6 bit in gdbstub
>>>
>>>   target/hppa/gdbstub.c | 66 +++++++++++++++++++++++++++++--------------
>>>   1 file changed, 45 insertions(+), 21 deletions(-)
>> gentle ping - if i followed correctly only one patch was reviewed so
>> far.
>
> We can't really proceed until you get your gdb patch reviewed upstream.
> All that will happen in the meantime is that qemu make-check will fail.

I see. Thanks!