target/sparc: Stop using the legacy native-endian APIs

[PATCH v2 3/9] target/sparc: Use address_space_ld/st[n]_be() in ld/st_asi helpers

Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago

Do not open-code address_space_{ld,st}n(), use it passing
the access size as argument. 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/ldst_helper.c | 42 ++++----------------------------------
 1 file changed, 4 insertions(+), 38 deletions(-)

diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index a87a0b3eee0..e9139814c26 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -701,25 +701,8 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
         MemTxResult result;
         hwaddr access_addr = (hwaddr)addr | ((hwaddr)(asi & 0xf) << 32);
 
-        switch (size) {
-        case 1:
-            ret = address_space_ldub(cs->as, access_addr,
-                                     MEMTXATTRS_UNSPECIFIED, &result);
-            break;
-        case 2:
-            ret = address_space_lduw(cs->as, access_addr,
-                                     MEMTXATTRS_UNSPECIFIED, &result);
-            break;
-        default:
-        case 4:
-            ret = address_space_ldl(cs->as, access_addr,
-                                    MEMTXATTRS_UNSPECIFIED, &result);
-            break;
-        case 8:
-            ret = address_space_ldq(cs->as, access_addr,
-                                    MEMTXATTRS_UNSPECIFIED, &result);
-            break;
-        }
+        ret = address_space_ldn_be(cs->as, access_addr, size,
+                                   MEMTXATTRS_UNSPECIFIED, &result);
 
         if (result != MEMTX_OK) {
             sparc_raise_mmu_fault(cs, access_addr, false, false, false,
@@ -1066,25 +1049,8 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
             MemTxResult result;
             hwaddr access_addr = (hwaddr)addr | ((hwaddr)(asi & 0xf) << 32);
 
-            switch (size) {
-            case 1:
-                address_space_stb(cs->as, access_addr, val,
-                                  MEMTXATTRS_UNSPECIFIED, &result);
-                break;
-            case 2:
-                address_space_stw(cs->as, access_addr, val,
-                                  MEMTXATTRS_UNSPECIFIED, &result);
-                break;
-            case 4:
-            default:
-                address_space_stl(cs->as, access_addr, val,
-                                  MEMTXATTRS_UNSPECIFIED, &result);
-                break;
-            case 8:
-                address_space_stq(cs->as, access_addr, val,
-                                  MEMTXATTRS_UNSPECIFIED, &result);
-                break;
-            }
+            address_space_stn_be(cs->as, access_addr, val, size,
+                                 MEMTXATTRS_UNSPECIFIED, &result);
             if (result != MEMTX_OK) {
                 sparc_raise_mmu_fault(cs, access_addr, true, false, false,
                                       size, GETPC());
-- 
2.52.0

Re: [PATCH v2 3/9] target/sparc: Use address_space_ld/st[n]_be() in ld/st_asi helpers

Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago

On 19/12/25 17:36, Philippe Mathieu-Daudé wrote:
> Do not open-code address_space_{ld,st}n(), use it passing
> the access size as argument. 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/ldst_helper.c | 42 ++++----------------------------------
>   1 file changed, 4 insertions(+), 38 deletions(-)
> 
> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
> index a87a0b3eee0..e9139814c26 100644
> --- a/target/sparc/ldst_helper.c
> +++ b/target/sparc/ldst_helper.c
> @@ -701,25 +701,8 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong addr,
>           MemTxResult result;
>           hwaddr access_addr = (hwaddr)addr | ((hwaddr)(asi & 0xf) << 32);
>   
> -        switch (size) {
> -        case 1:
> -            ret = address_space_ldub(cs->as, access_addr,
> -                                     MEMTXATTRS_UNSPECIFIED, &result);
> -            break;
> -        case 2:
> -            ret = address_space_lduw(cs->as, access_addr,
> -                                     MEMTXATTRS_UNSPECIFIED, &result);
> -            break;
> -        default:

Unfortunately I missed this 'default' case (which I don't understand),
so this patch is not a faithful API conversion, thus incorrect.

> -        case 4:
> -            ret = address_space_ldl(cs->as, access_addr,
> -                                    MEMTXATTRS_UNSPECIFIED, &result);
> -            break;
> -        case 8:
> -            ret = address_space_ldq(cs->as, access_addr,
> -                                    MEMTXATTRS_UNSPECIFIED, &result);
> -            break;
> -        }
> +        ret = address_space_ldn_be(cs->as, access_addr, size,
> +                                   MEMTXATTRS_UNSPECIFIED, &result);
>   
>           if (result != MEMTX_OK) {
>               sparc_raise_mmu_fault(cs, access_addr, false, false, false,
> @@ -1066,25 +1049,8 @@ void helper_st_asi(CPUSPARCState *env, target_ulong addr, uint64_t val,
>               MemTxResult result;
>               hwaddr access_addr = (hwaddr)addr | ((hwaddr)(asi & 0xf) << 32);
>   
> -            switch (size) {
> -            case 1:
> -                address_space_stb(cs->as, access_addr, val,
> -                                  MEMTXATTRS_UNSPECIFIED, &result);
> -                break;
> -            case 2:
> -                address_space_stw(cs->as, access_addr, val,
> -                                  MEMTXATTRS_UNSPECIFIED, &result);
> -                break;
> -            case 4:
> -            default:

Ditto.

> -                address_space_stl(cs->as, access_addr, val,
> -                                  MEMTXATTRS_UNSPECIFIED, &result);
> -                break;
> -            case 8:
> -                address_space_stq(cs->as, access_addr, val,
> -                                  MEMTXATTRS_UNSPECIFIED, &result);
> -                break;
> -            }
> +            address_space_stn_be(cs->as, access_addr, val, size,
> +                                 MEMTXATTRS_UNSPECIFIED, &result);
>               if (result != MEMTX_OK) {
>                   sparc_raise_mmu_fault(cs, access_addr, true, false, false,
>                                         size, GETPC());

Re: [PATCH v2 3/9] target/sparc: Use address_space_ld/st[n]_be() in ld/st_asi helpers

Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago

On 21/12/25 12:11, Philippe Mathieu-Daudé wrote:
> On 19/12/25 17:36, Philippe Mathieu-Daudé wrote:
>> Do not open-code address_space_{ld,st}n(), use it passing
>> the access size as argument. 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/ldst_helper.c | 42 ++++----------------------------------
>>   1 file changed, 4 insertions(+), 38 deletions(-)
>>
>> diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
>> index a87a0b3eee0..e9139814c26 100644
>> --- a/target/sparc/ldst_helper.c
>> +++ b/target/sparc/ldst_helper.c
>> @@ -701,25 +701,8 @@ uint64_t helper_ld_asi(CPUSPARCState *env, 
>> target_ulong addr,
>>           MemTxResult result;
>>           hwaddr access_addr = (hwaddr)addr | ((hwaddr)(asi & 0xf) << 
>> 32);
>> -        switch (size) {
>> -        case 1:
>> -            ret = address_space_ldub(cs->as, access_addr,
>> -                                     MEMTXATTRS_UNSPECIFIED, &result);
>> -            break;
>> -        case 2:
>> -            ret = address_space_lduw(cs->as, access_addr,
>> -                                     MEMTXATTRS_UNSPECIFIED, &result);
>> -            break;
>> -        default:
> 
> Unfortunately I missed this 'default' case (which I don't understand),
> so this patch is not a faithful API conversion, thus incorrect.
> 
>> -        case 4:
>> -            ret = address_space_ldl(cs->as, access_addr,
>> -                                    MEMTXATTRS_UNSPECIFIED, &result);
>> -            break;
>> -        case 8:
>> -            ret = address_space_ldq(cs->as, access_addr,
>> -                                    MEMTXATTRS_UNSPECIFIED, &result);
>> -            break;
>> -        }
>> +        ret = address_space_ldn_be(cs->as, access_addr, size,
>> +                                   MEMTXATTRS_UNSPECIFIED, &result);

I just inverted size <-> access_addr :) The default case shouldn't be
reachable IMHO.

[PATCH v2 1/9] hw/sparc: Use explicit big-endian LD/ST API
[PATCH v2 2/9] target/sparc: Replace MO_TE -> MO_BE
[PATCH v2 3/9] target/sparc: Use address_space_ld/st[n]_be() in ld/st_asi helpers
[PATCH v2 4/9] target/sparc: Use explicit big-endian LD/ST API
[PATCH v2 5/9] target/sparc: Use big-endian variant of cpu_ld/st_data*()
[PATCH v2 6/9] target/sparc: Inline translator_ldl()
[PATCH v2 7/9] target/sparc: Inline cpu_ldl_code() call in cpu_do_interrupt()
[PATCH v2 8/9] target/sparc: Simplify gdbstub sparc_cpu_gdb_write_register()
[PATCH v2 9/9] configs/targets: Forbid SPARC to use legacy native endianness APIs