[v1] target/s390x: Prohibit target_ulong and TARGET_PAGE_SIZE uses

[PATCH 02/18] target/s390x: Use address_space_ldl_be() in read_table_entry()

Posted by Philippe Mathieu-Daudé 1 month ago

Keep address_space_read/write() API to access blob of memory.
When the access size is known, use the address_space_ld/st()
API which can directly swap endianness.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/mmu_helper.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 026502a3e40..9ee1d778876 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -108,6 +108,7 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
                                     uint64_t *entry)
 {
     CPUState *cs = env_cpu(env);
+    MemTxResult ret;
 
     /*
      * According to the PoP, these table addresses are "unpredictably real
@@ -116,13 +117,9 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
      *
      * We treat them as absolute addresses and don't wrap them.
      */
-    if (unlikely(address_space_read(cs->as, gaddr, MEMTXATTRS_UNSPECIFIED,
-                                    entry, sizeof(*entry)) !=
-                 MEMTX_OK)) {
-        return false;
-    }
-    *entry = be64_to_cpu(*entry);
-    return true;
+    *entry = address_space_ldl_be(cs->as, gaddr, MEMTXATTRS_UNSPECIFIED, &ret);
+
+    return ret == MEMTX_OK;
 }
 
 static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
-- 
2.52.0

Re: [PATCH 02/18] target/s390x: Use address_space_ldl_be() in read_table_entry()

Posted by Thomas Huth 4 weeks ago

On 07/01/2026 14.07, Philippe Mathieu-Daudé wrote:
> Keep address_space_read/write() API to access blob of memory.
> When the access size is known, use the address_space_ld/st()
> API which can directly swap endianness.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/s390x/mmu_helper.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 026502a3e40..9ee1d778876 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -108,6 +108,7 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
>                                       uint64_t *entry)
>   {
>       CPUState *cs = env_cpu(env);
> +    MemTxResult ret;
>   
>       /*
>        * According to the PoP, these table addresses are "unpredictably real
> @@ -116,13 +117,9 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
>        *
>        * We treat them as absolute addresses and don't wrap them.
>        */
> -    if (unlikely(address_space_read(cs->as, gaddr, MEMTXATTRS_UNSPECIFIED,
> -                                    entry, sizeof(*entry)) !=
> -                 MEMTX_OK)) {
> -        return false;
> -    }
> -    *entry = be64_to_cpu(*entry);
> -    return true;
> +    *entry = address_space_ldl_be(cs->as, gaddr, MEMTXATTRS_UNSPECIFIED, &ret);
> +
> +    return ret == MEMTX_OK;
>   }

  Hi Philippe,

this patch seems to break "make check-functional-s390x" ... looks like there 
is a bug somewhere, please have a look!

  Thanks,
   Thomas

Re: [PATCH 02/18] target/s390x: Use address_space_ldl_be() in read_table_entry()

Posted by Philippe Mathieu-Daudé 4 weeks ago

On 12/1/26 10:32, Thomas Huth wrote:
> On 07/01/2026 14.07, Philippe Mathieu-Daudé wrote:
>> Keep address_space_read/write() API to access blob of memory.
>> When the access size is known, use the address_space_ld/st()
>> API which can directly swap endianness.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/s390x/mmu_helper.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
>> index 026502a3e40..9ee1d778876 100644
>> --- a/target/s390x/mmu_helper.c
>> +++ b/target/s390x/mmu_helper.c
>> @@ -108,6 +108,7 @@ static inline bool read_table_entry(CPUS390XState 
>> *env, hwaddr gaddr,
>>                                       uint64_t *entry)
>>   {
>>       CPUState *cs = env_cpu(env);
>> +    MemTxResult ret;
>>       /*
>>        * According to the PoP, these table addresses are 
>> "unpredictably real
>> @@ -116,13 +117,9 @@ static inline bool read_table_entry(CPUS390XState 
>> *env, hwaddr gaddr,
>>        *
>>        * We treat them as absolute addresses and don't wrap them.
>>        */
>> -    if (unlikely(address_space_read(cs->as, gaddr, 
>> MEMTXATTRS_UNSPECIFIED,
>> -                                    entry, sizeof(*entry)) !=
>> -                 MEMTX_OK)) {
>> -        return false;
>> -    }
>> -    *entry = be64_to_cpu(*entry);
>> -    return true;
>> +    *entry = address_space_ldl_be(cs->as, gaddr, 
>> MEMTXATTRS_UNSPECIFIED, &ret);
>> +
>> +    return ret == MEMTX_OK;
>>   }
> 
>   Hi Philippe,
> 
> this patch seems to break "make check-functional-s390x" ... looks like 
> there is a bug somewhere, please have a look!

Oops, address_space_ldQ_be.

Re: [PATCH 02/18] target/s390x: Use address_space_ldl_be() in read_table_entry()

Posted by Thomas Huth 1 month ago

On 07/01/2026 14.07, Philippe Mathieu-Daudé wrote:
> Keep address_space_read/write() API to access blob of memory.

Maybe rather: "address_space_read/write() is meant for accessing random 
amount of memory blobs"? ... the "Keep" initially confused me here, since 
you don't keep it in this patch.

> When the access size is known, use the address_space_ld/st()
> API which can directly swap endianness.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/s390x/mmu_helper.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)

Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>