On 6/10/25 16:04, Thomas Huth wrote:
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?
Yes and no :) The point is to trigger reviewers/maintainers to fill the
missing parts when they notice them, otherwise this is just a no-op.
It worked because now I know the error path you expect, which is
currently ignored.
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;
}
