Re: [PATCH 3/9] target/s390x: Replace legacy cpu_physical_memory_read/write() calls

[Thomas Huth]

On 02/10/2025 11.11, Philippe Mathieu-Daudé wrote:
cpu_physical_memory_read() and cpu_physical_memory_write() are
legacy (see commit b7ecba0f6f6), replace by address_space_read()
and address_space_write() respectively.

I'm not sure whether this patch is a good idea in the current way it is done.

Commit b7ecba0f6f6 says: "there is likely to be behaviour you need to model 
correctly for a failed read or write operation" ... so if we switch to the 
address_space_* API, I think you should also implement the correct handling 
for the case where the memory transaction failed. Otherwise this is more or 
less just code churn, isn't it?

 Thomas

diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index c2fedc55213..737c3bbc5be 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -17,6 +17,7 @@
  #include "s390x-internal.h"
  #include "hw/watchdog/wdt_diag288.h"
  #include "system/cpus.h"
+#include "system/memory.h"
  #include "hw/s390x/ipl.h"
  #include "hw/s390x/s390-virtio-ccw.h"
  #include "system/kvm.h"
@@ -82,11 +83,14 @@ static bool diag_iplb_read(IplParameterBlock *iplb, S390CPU 
*cpu, uint64_t addr)
          }
          s390_cpu_pv_mem_read(cpu, 0, iplb, be32_to_cpu(iplb->len));
      } else {
-        cpu_physical_memory_read(addr, iplb, sizeof(iplb->len));
+        const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+        AddressSpace *as = CPU(cpu)->as;
+
+        address_space_read(as, addr, attrs, iplb, sizeof(iplb->len));
          if (!iplb_valid_len(iplb)) {
              return false;
          }
-        cpu_physical_memory_read(addr, iplb, be32_to_cpu(iplb->len));
+        address_space_read(as, addr, attrs, iplb, be32_to_cpu(iplb->len));
      }
      return true;
  }
@@ -98,7 +102,10 @@ static void diag_iplb_write(IplParameterBlock *iplb, 
S390CPU *cpu, uint64_t addr
      if (s390_is_pv()) {
          s390_cpu_pv_mem_write(cpu, 0, iplb, iplb_len);
      } else {
-        cpu_physical_memory_write(addr, iplb, iplb_len);
+        const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+        AddressSpace *as = CPU(cpu)->as;
+
+        address_space_write(as, addr, attrs, iplb, iplb_len);
      }
  }