[PATCH 5/8] target/ppc/mmu: Replace legacy ld/st_phys() -> address_space_ld/st()

Philippe Mathieu-Daudé posted 8 patches 2 weeks, 3 days ago
Maintainers: BALATON Zoltan <balaton@eik.bme.hu>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Chinmay Rath <rathc@linux.ibm.com>, Glenn Miles <milesg@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH 5/8] target/ppc/mmu: Replace legacy ld/st_phys() -> address_space_ld/st()
Posted by Philippe Mathieu-Daudé 2 weeks, 3 days ago
Prefer the address_space_ld/st API over the legacy ld_phys()
because it allow checking for bus access fault.

This code however doesn't check for fault, so we simply inline
the calls (not specifying any memory transaction attribute nor
expecting transation result) per the definition in
"system/memory_ldst_phys_endian.h.inc":

 27 static inline uint32_t LD_PHYS(l)(ARG1_DECL, hwaddr addr)
 28 {
 29     return ADDRESS_SPACE_LD(l)(ARG1, addr, MEMTXATTRS_UNSPECIFIED, NULL);
 30 }

 28 static inline void glue(stb_phys, SUFFIX)(ARG1_DECL, hwaddr addr, uint8_t val)
 29 {
 30     glue(address_space_stb, SUFFIX)(ARG1, addr, val,
 31                                     MEMTXATTRS_UNSPECIFIED, NULL);
 32 }

No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/cpu.c        |  3 ++-
 target/ppc/mmu-hash32.c | 14 ++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index f24801a9731..89fad5356b4 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -110,7 +110,8 @@ uint64_t ppc_load_epr(CPUPPCState *env)
 {
     CPUState *cs = env_cpu(env);
 
-    return ldl_phys(cs->as, env->mpic_iack);
+    return address_space_ldl(cs->as, env->mpic_iack,
+                             MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 #if defined(TARGET_PPC64)
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 08c9f63a132..81fa7336b76 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -23,6 +23,7 @@
 #include "exec/page-protection.h"
 #include "exec/target_page.h"
 #include "system/kvm.h"
+#include "system/memory.h"
 #include "kvm_ppc.h"
 #include "internal.h"
 #include "mmu-hash32.h"
@@ -205,14 +206,17 @@ static target_ulong ppc_hash32_load_hpte0(PowerPCCPU *cpu, hwaddr pte_offset)
 {
     target_ulong base = ppc_hash32_hpt_base(cpu);
 
-    return ldl_phys(CPU(cpu)->as, base + pte_offset);
+    return address_space_ldl(CPU(cpu)->as, base + pte_offset,
+                             MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 static target_ulong ppc_hash32_load_hpte1(PowerPCCPU *cpu, hwaddr pte_offset)
 {
     target_ulong base = ppc_hash32_hpt_base(cpu);
 
-    return ldl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2);
+    return address_space_ldl(CPU(cpu)->as,
+                             base + pte_offset + HASH_PTE_SIZE_32 / 2,
+                             MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 static hwaddr ppc_hash32_pteg_search(PowerPCCPU *cpu, hwaddr pteg_off,
@@ -253,7 +257,8 @@ static void ppc_hash32_set_r(PowerPCCPU *cpu, hwaddr pte_offset, uint32_t pte1)
     hwaddr offset = pte_offset + 6;
 
     /* The HW performs a non-atomic byte update */
-    stb_phys(CPU(cpu)->as, base + offset, ((pte1 >> 8) & 0xff) | 0x01);
+    address_space_stb(CPU(cpu)->as, base + offset, ((pte1 >> 8) & 0xff) | 0x01,
+                      MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 static void ppc_hash32_set_c(PowerPCCPU *cpu, hwaddr pte_offset, uint64_t pte1)
@@ -262,7 +267,8 @@ static void ppc_hash32_set_c(PowerPCCPU *cpu, hwaddr pte_offset, uint64_t pte1)
     hwaddr offset = pte_offset + 7;
 
     /* The HW performs a non-atomic byte update */
-    stb_phys(CPU(cpu)->as, base + offset, (pte1 & 0xff) | 0x80);
+    address_space_stb(CPU(cpu)->as, base + offset, (pte1 & 0xff) | 0x80,
+                      MEMTXATTRS_UNSPECIFIED, NULL);
 }
 
 static hwaddr ppc_hash32_htab_lookup(PowerPCCPU *cpu,
-- 
2.53.0


Re: [PATCH 5/8] target/ppc/mmu: Replace legacy ld/st_phys() -> address_space_ld/st()
Posted by BALATON Zoltan 2 weeks, 3 days ago
On Thu, 19 Mar 2026, Philippe Mathieu-Daudé wrote:
> Prefer the address_space_ld/st API over the legacy ld_phys()
> because it allow checking for bus access fault.
>
> This code however doesn't check for fault, so we simply inline
> the calls (not specifying any memory transaction attribute nor
> expecting transation result) per the definition in
> "system/memory_ldst_phys_endian.h.inc":

Recently when trying to remove _nomigrate memory region functions I was 
told if some convenience function has more than 1 use it's probably worth 
to keep it. This looks like similar case even more so as the replacement 
is unnecessarily more complicated. So what's the problem with ld_phys in 
the first place and why do you want to replace it with a less convenient 
function? If there's a reason maybe say that in the commit message.

Regards,
BALATON Zoltan