[PATCH] target/s390x/mmu_helper: Do not ignore address_space_rw() errors

[PATCH] target/s390x/mmu_helper: Do not ignore address_space_rw() errors

[Philippe Mathieu-Daudé]

Break the loop and return an error if address_space_rw() ever failed.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Based-on: <20251007014958.19086-1-philmd@linaro.org>
---
 target/s390x/mmu_helper.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 7bcf1810bca..643141e7847 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -544,12 +544,18 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
         trigger_access_exception(&cpu->env, ret, tec);
     } else if (hostbuf != NULL) {
         AddressSpace *as = CPU(cpu)->as;
+        MemTxResult res;
 
         /* Copy data by stepping through the area page by page */
         for (i = 0; i < nr_pages; i++) {
             currlen = MIN(len, TARGET_PAGE_SIZE - (laddr % TARGET_PAGE_SIZE));
-            address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
-                             MEMTXATTRS_UNSPECIFIED, hostbuf, currlen, is_write);
+            res = address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
+                                   MEMTXATTRS_UNSPECIFIED, hostbuf, currlen,
+                                   is_write);
+            if (res != MEMTX_OK) {
+                ret = 1;
+                break;
+            }
             laddr += currlen;
             hostbuf += currlen;
             len -= currlen;
-- 
2.51.0

Re: [PATCH] target/s390x/mmu_helper: Do not ignore address_space_rw() errors

[Thomas Huth]

On 07/10/2025 04.10, Philippe Mathieu-Daudé wrote:
> Break the loop and return an error if address_space_rw() ever failed.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20251007014958.19086-1-philmd@linaro.org>
> ---
>   target/s390x/mmu_helper.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index 7bcf1810bca..643141e7847 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -544,12 +544,18 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
>           trigger_access_exception(&cpu->env, ret, tec);
>       } else if (hostbuf != NULL) {
>           AddressSpace *as = CPU(cpu)->as;
> +        MemTxResult res;
>   
>           /* Copy data by stepping through the area page by page */
>           for (i = 0; i < nr_pages; i++) {
>               currlen = MIN(len, TARGET_PAGE_SIZE - (laddr % TARGET_PAGE_SIZE));
> -            address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
> -                             MEMTXATTRS_UNSPECIFIED, hostbuf, currlen, is_write);
> +            res = address_space_rw(as, pages[i] | (laddr & ~TARGET_PAGE_MASK),
> +                                   MEMTXATTRS_UNSPECIFIED, hostbuf, currlen,
> +                                   is_write);
> +            if (res != MEMTX_OK) {
> +                ret = 1;

I think you'd need to call trigger_access_exception() here like it is done 
when translate_pages() failed earlier in this function. And I think I'd 
rather use ret = PGM_ADDRESSING in this case.

  Thomas


> +                break;
> +            }
>               laddr += currlen;
>               hostbuf += currlen;
>               len -= currlen;