1. Patchew
  2. QEMU
  3. [PATCH v3 0/9] target/sparc: Use explicit big-endian LD/ST API
  4. View patch

[PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()

Philippe Mathieu-Daudé posted 9 patches 1 month, 2 weeks ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Gerd Hoffmann <kraxel@redhat.com>, "Clément Chigot" <chigot@adacore.com>, Frederic Konrad <konrad.frederic@yahoo.fr>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>
There is a newer version of this series
[PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago 
Rather than ldtul_p() which uses the underlying 'unsigned
long' size, use the ldn() variant, passing the access size
as argument (evaluating TARGET_LONG_BITS / 8).

No need to use #ifdef'ry to check for TARGET_ABI32, since
it is 64-bit:

  $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
  configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
  configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
  configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
  configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
  configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
  configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64

Directly expand to the big-endian variant (with the '_be' suffix)
since we only build the SPARC targets as big-endian.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sparc/gdbstub.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
index 134617fb232..d265681f6d2 100644
--- a/target/sparc/gdbstub.c
+++ b/target/sparc/gdbstub.c
@@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 {
     SPARCCPU *cpu = SPARC_CPU(cs);
     CPUSPARCState *env = &cpu->env;
-#if defined(TARGET_ABI32)
-    uint32_t tmp;
-
-    tmp = ldl_p(mem_buf);
-#else
-    target_ulong tmp;
-
-    tmp = ldtul_p(mem_buf);
-#endif
+    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);
 
     if (n < 8) {
         /* g0..g7 */
@@ -170,7 +162,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
 #else
     else if (n < 64) {
         /* f0-f31 */
-        tmp = ldl_p(mem_buf);
+        tmp = ldl_be_p(mem_buf);
         if (n & 1) {
             env->fpr[(n - 32) / 2].l.lower = tmp;
         } else {
-- 
2.52.0
Re: [PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Richard Henderson 1 month ago 
On 12/25/25 03:26, Philippe Mathieu-Daudé wrote:
> Rather than ldtul_p() which uses the underlying 'unsigned
> long' size, use the ldn() variant, passing the access size
> as argument (evaluating TARGET_LONG_BITS / 8).
> 
> No need to use #ifdef'ry to check for TARGET_ABI32, since
> it is 64-bit:
> 
>    $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
>    configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
>    configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
>    configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>    configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
>    configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
>    configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64
> 
> Directly expand to the big-endian variant (with the '_be' suffix)
> since we only build the SPARC targets as big-endian.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/sparc/gdbstub.c | 12 ++----------
>   1 file changed, 2 insertions(+), 10 deletions(-)
> 
> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
> index 134617fb232..d265681f6d2 100644
> --- a/target/sparc/gdbstub.c
> +++ b/target/sparc/gdbstub.c
> @@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>   {
>       SPARCCPU *cpu = SPARC_CPU(cs);
>       CPUSPARCState *env = &cpu->env;
> -#if defined(TARGET_ABI32)
> -    uint32_t tmp;
> -
> -    tmp = ldl_p(mem_buf);
> -#else
> -    target_ulong tmp;
> -
> -    tmp = ldtul_p(mem_buf);
> -#endif
> +    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);

No, this changes the behaviour of sparc32plus.


r~
Re: [PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Philippe Mathieu-Daudé 1 month ago 
On 5/1/26 01:37, Richard Henderson wrote:
> On 12/25/25 03:26, Philippe Mathieu-Daudé wrote:
>> Rather than ldtul_p() which uses the underlying 'unsigned
>> long' size, use the ldn() variant, passing the access size
>> as argument (evaluating TARGET_LONG_BITS / 8).
>>
>> No need to use #ifdef'ry to check for TARGET_ABI32, since
>> it is 64-bit:
>>
>>    $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
>>    configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
>>    configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
>>    configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>>    configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
>>    configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
>>    configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64
>>
>> Directly expand to the big-endian variant (with the '_be' suffix)
>> since we only build the SPARC targets as big-endian.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/sparc/gdbstub.c | 12 ++----------
>>   1 file changed, 2 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
>> index 134617fb232..d265681f6d2 100644
>> --- a/target/sparc/gdbstub.c
>> +++ b/target/sparc/gdbstub.c
>> @@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, 
>> uint8_t *mem_buf, int n)
>>   {
>>       SPARCCPU *cpu = SPARC_CPU(cs);
>>       CPUSPARCState *env = &cpu->env;
>> -#if defined(TARGET_ABI32)
>> -    uint32_t tmp;
>> -
>> -    tmp = ldl_p(mem_buf);
>> -#else
>> -    target_ulong tmp;
>> -
>> -    tmp = ldtul_p(mem_buf);
>> -#endif
>> +    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);
> 
> No, this changes the behaviour of sparc32plus.

$ git grep TARGET_ABI32 configs/targets/sparc*
configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y

$ cat configs/targets/sparc32plus-linux-user.mak
TARGET_ABI32=y
TARGET_BIG_ENDIAN=y
TARGET_LONG_BITS=64

Isn't it the same?
Re: [PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Richard Henderson 1 month ago 
On 1/5/26 21:56, Philippe Mathieu-Daudé wrote:
> On 5/1/26 01:37, Richard Henderson wrote:
>> On 12/25/25 03:26, Philippe Mathieu-Daudé wrote:
>>> Rather than ldtul_p() which uses the underlying 'unsigned
>>> long' size, use the ldn() variant, passing the access size
>>> as argument (evaluating TARGET_LONG_BITS / 8).
>>>
>>> No need to use #ifdef'ry to check for TARGET_ABI32, since
>>> it is 64-bit:
>>>
>>>    $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
>>>    configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
>>>    configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
>>>    configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>>>    configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
>>>    configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
>>>    configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64
>>>
>>> Directly expand to the big-endian variant (with the '_be' suffix)
>>> since we only build the SPARC targets as big-endian.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   target/sparc/gdbstub.c | 12 ++----------
>>>   1 file changed, 2 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
>>> index 134617fb232..d265681f6d2 100644
>>> --- a/target/sparc/gdbstub.c
>>> +++ b/target/sparc/gdbstub.c
>>> @@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, 
>>> int n)
>>>   {
>>>       SPARCCPU *cpu = SPARC_CPU(cs);
>>>       CPUSPARCState *env = &cpu->env;
>>> -#if defined(TARGET_ABI32)
>>> -    uint32_t tmp;
>>> -
>>> -    tmp = ldl_p(mem_buf);
>>> -#else
>>> -    target_ulong tmp;
>>> -
>>> -    tmp = ldtul_p(mem_buf);
>>> -#endif
>>> +    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);
>>
>> No, this changes the behaviour of sparc32plus.
> 
> $ git grep TARGET_ABI32 configs/targets/sparc*
> configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
> 
> $ cat configs/targets/sparc32plus-linux-user.mak
> TARGET_ABI32=y
> TARGET_BIG_ENDIAN=y
> TARGET_LONG_BITS=64
> 
> Isn't it the same?

No.  ABI32 uses uint32_t, not uint64_t, even with 64-bit registers.

Which is probably functionally broken, because, history.


r~
Re: [PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Philippe Mathieu-Daudé 1 month ago 
On 5/1/26 12:45, Richard Henderson wrote:
> On 1/5/26 21:56, Philippe Mathieu-Daudé wrote:
>> On 5/1/26 01:37, Richard Henderson wrote:
>>> On 12/25/25 03:26, Philippe Mathieu-Daudé wrote:
>>>> Rather than ldtul_p() which uses the underlying 'unsigned
>>>> long' size, use the ldn() variant, passing the access size
>>>> as argument (evaluating TARGET_LONG_BITS / 8).
>>>>
>>>> No need to use #ifdef'ry to check for TARGET_ABI32, since
>>>> it is 64-bit:
>>>>
>>>>    $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
>>>>    configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
>>>>    configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
>>>>    configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>>>>    configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
>>>>    configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
>>>>    configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64
>>>>
>>>> Directly expand to the big-endian variant (with the '_be' suffix)
>>>> since we only build the SPARC targets as big-endian.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> ---
>>>>   target/sparc/gdbstub.c | 12 ++----------
>>>>   1 file changed, 2 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
>>>> index 134617fb232..d265681f6d2 100644
>>>> --- a/target/sparc/gdbstub.c
>>>> +++ b/target/sparc/gdbstub.c
>>>> @@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, 
>>>> uint8_t *mem_buf, int n)
>>>>   {
>>>>       SPARCCPU *cpu = SPARC_CPU(cs);
>>>>       CPUSPARCState *env = &cpu->env;
>>>> -#if defined(TARGET_ABI32)
>>>> -    uint32_t tmp;
>>>> -
>>>> -    tmp = ldl_p(mem_buf);
>>>> -#else
>>>> -    target_ulong tmp;
>>>> -
>>>> -    tmp = ldtul_p(mem_buf);
>>>> -#endif
>>>> +    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);
>>>
>>> No, this changes the behaviour of sparc32plus.
>>
>> $ git grep TARGET_ABI32 configs/targets/sparc*
>> configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>>
>> $ cat configs/targets/sparc32plus-linux-user.mak
>> TARGET_ABI32=y
>> TARGET_BIG_ENDIAN=y
>> TARGET_LONG_BITS=64
>>
>> Isn't it the same?
> 
> No.  ABI32 uses uint32_t, not uint64_t, even with 64-bit registers.

Oh right, I missed that.

> Which is probably functionally broken, because, history.

And still we maintain it...
Re: [PATCH v3 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
Posted by Manos Pitsidianakis 1 month, 1 week ago 
On Wed, Dec 24, 2025 at 6:27 PM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Rather than ldtul_p() which uses the underlying 'unsigned
> long' size, use the ldn() variant, passing the access size
> as argument (evaluating TARGET_LONG_BITS / 8).
>
> No need to use #ifdef'ry to check for TARGET_ABI32, since
> it is 64-bit:
>
>   $ git grep -E '(ABI32|LONG_BITS)' configs/targets/sparc*
>   configs/targets/sparc-linux-user.mak:5:TARGET_LONG_BITS=32
>   configs/targets/sparc-softmmu.mak:4:TARGET_LONG_BITS=32
>   configs/targets/sparc32plus-linux-user.mak:2:TARGET_ABI32=y
>   configs/targets/sparc32plus-linux-user.mak:8:TARGET_LONG_BITS=64
>   configs/targets/sparc64-linux-user.mak:8:TARGET_LONG_BITS=64
>   configs/targets/sparc64-softmmu.mak:6:TARGET_LONG_BITS=64
>
> Directly expand to the big-endian variant (with the '_be' suffix)
> since we only build the SPARC targets as big-endian.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>

>  target/sparc/gdbstub.c | 12 ++----------
>  1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/target/sparc/gdbstub.c b/target/sparc/gdbstub.c
> index 134617fb232..d265681f6d2 100644
> --- a/target/sparc/gdbstub.c
> +++ b/target/sparc/gdbstub.c
> @@ -112,15 +112,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>  {
>      SPARCCPU *cpu = SPARC_CPU(cs);
>      CPUSPARCState *env = &cpu->env;
> -#if defined(TARGET_ABI32)
> -    uint32_t tmp;
> -
> -    tmp = ldl_p(mem_buf);
> -#else
> -    target_ulong tmp;
> -
> -    tmp = ldtul_p(mem_buf);
> -#endif
> +    uint64_t tmp = ldn_be_p(mem_buf, TARGET_LONG_BITS / 8);
>
>      if (n < 8) {
>          /* g0..g7 */
> @@ -170,7 +162,7 @@ int sparc_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
>  #else
>      else if (n < 64) {
>          /* f0-f31 */
> -        tmp = ldl_p(mem_buf);
> +        tmp = ldl_be_p(mem_buf);
>          if (n & 1) {
>              env->fpr[(n - 32) / 2].l.lower = tmp;
>          } else {
> --
> 2.52.0
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.