[PATCH 0/5] target/s390x: Forbid to use legacy ldst_phys() API

[PATCH 0/5] target/s390x: Forbid to use legacy ldst_phys() API

[Philippe Mathieu-Daudé]

Replace legacy ld/st_phys() by address_space_ld/st()
then lock the target again further legacy API uses.

Philippe Mathieu-Daudé (5):
  target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (cpu)
  target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (mmu)
  target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (tcg)
  target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (kvm)
  configs/targets: Restrict the legacy ldst_phys() API on s390x target

 configs/targets/s390x-softmmu.mak |  1 +
 target/s390x/cpu-system.c         |  2 +-
 target/s390x/kvm/kvm.c            | 12 ++++++++----
 target/s390x/mmu_helper.c         |  5 +++--
 target/s390x/tcg/excp_helper.c    | 21 +++++++++++++--------
 target/s390x/tcg/mem_helper.c     | 12 +++++++-----
 6 files changed, 33 insertions(+), 20 deletions(-)

-- 
2.53.0

[PATCH 1/5] target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (cpu)

[Philippe Mathieu-Daudé]

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). No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/tcg/excp_helper.c | 21 +++++++++++++--------
 target/s390x/tcg/mem_helper.c  | 12 +++++++-----
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
index 41b0017d767..05e448d3f20 100644
--- a/target/s390x/tcg/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -54,8 +54,9 @@ G_NORETURN void tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
     g_assert(dxc <= 0xff);
 #if !defined(CONFIG_USER_ONLY)
     /* Store the DXC into the lowcore */
-    stl_be_phys(env_cpu(env)->as,
-                env->psa + offsetof(LowCore, data_exc_code), dxc);
+    address_space_stl_be(env_cpu(env)->as,
+                         env->psa + offsetof(LowCore, data_exc_code), dxc,
+                         MEMTXATTRS_UNSPECIFIED, NULL);
 #endif
 
     /* Store the DXC into the FPC if AFP is enabled */
@@ -71,8 +72,9 @@ G_NORETURN void tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
     g_assert(vxc <= 0xff);
 #if !defined(CONFIG_USER_ONLY)
     /* Always store the VXC into the lowcore, without AFP it is undefined */
-    stl_be_phys(env_cpu(env)->as,
-                env->psa + offsetof(LowCore, data_exc_code), vxc);
+    address_space_stl_be(env_cpu(env)->as,
+                         env->psa + offsetof(LowCore, data_exc_code), vxc,
+                         MEMTXATTRS_UNSPECIFIED, NULL);
 #endif
 
     /* Always store the VXC into the FPC, without AFP it is undefined */
@@ -619,11 +621,14 @@ void monitor_event(CPUS390XState *env,
                    uint64_t monitor_code,
                    uint8_t monitor_class, uintptr_t ra)
 {
+    const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
+    AddressSpace *as = env_cpu(env)->as;
+
     /* Store the Monitor Code and the Monitor Class Number into the lowcore */
-    stq_be_phys(env_cpu(env)->as,
-                env->psa + offsetof(LowCore, monitor_code), monitor_code);
-    stw_be_phys(env_cpu(env)->as,
-                env->psa + offsetof(LowCore, mon_class_num), monitor_class);
+    address_space_stq_be(as, env->psa + offsetof(LowCore, monitor_code),
+                         monitor_code, attrs, NULL);
+    address_space_stw_be(as, env->psa + offsetof(LowCore, mon_class_num),
+                         monitor_class, attrs, NULL);
 
     tcg_s390_program_interrupt(env, PGM_MONITOR, ra);
 }
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 2a79a789f63..8474a694110 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -41,6 +41,7 @@
 #else
 #include "hw/s390x/storage-keys.h"
 #include "hw/core/boards.h"
+#include "system/memory.h"
 #endif
 
 #ifdef CONFIG_USER_ONLY
@@ -958,13 +959,14 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint32_t r1, uint32_t r2)
 inject_exc:
 #if !defined(CONFIG_USER_ONLY)
     if (exc != PGM_ADDRESSING) {
-        stq_be_phys(env_cpu(env)->as,
-                    env->psa + offsetof(LowCore, trans_exc_code),
-                    env->tlb_fill_tec);
+        address_space_stq_be(env_cpu(env)->as,
+                             env->psa + offsetof(LowCore, trans_exc_code),
+                             env->tlb_fill_tec, MEMTXATTRS_UNSPECIFIED, NULL);
     }
     if (exc == PGM_PAGE_TRANS) {
-        stb_phys(env_cpu(env)->as, env->psa + offsetof(LowCore, op_access_id),
-                 r1 << 4 | r2);
+        address_space_stb(env_cpu(env)->as,
+                          env->psa + offsetof(LowCore, op_access_id),
+                          r1 << 4 | r2, MEMTXATTRS_UNSPECIFIED, NULL);
     }
 #endif
     tcg_s390_program_interrupt(env, exc, ra);
-- 
2.53.0

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

[Philippe Mathieu-Daudé]

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). No logical change intended.

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

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 246573ff642..255271b9d34 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -44,8 +44,9 @@ static void trigger_access_exception(CPUS390XState *env, uint32_t type,
     } else {
         CPUState *cs = env_cpu(env);
         if (type != PGM_ADDRESSING) {
-            stq_be_phys(cs->as, env->psa + offsetof(LowCore, trans_exc_code),
-                        tec);
+            address_space_stq_be(cs->as,
+                                 env->psa + offsetof(LowCore, trans_exc_code),
+                                 tec, MEMTXATTRS_UNSPECIFIED, NULL);
         }
         trigger_pgm_exception(env, type);
     }
-- 
2.53.0

[PATCH 3/5] target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (tcg)

[Philippe Mathieu-Daudé]

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). No logical change intended.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/cpu-system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/cpu-system.c b/target/s390x/cpu-system.c
index 881171d71a2..285f5999661 100644
--- a/target/s390x/cpu-system.c
+++ b/target/s390x/cpu-system.c
@@ -63,7 +63,7 @@ static void s390_cpu_load_normal(CPUState *s)
     uint64_t spsw;
 
     if (!s390_is_pv()) {
-        spsw = ldq_be_phys(s->as, 0);
+        spsw = address_space_ldq_be(s->as, 0, MEMTXATTRS_UNSPECIFIED, NULL);
         cpu->env.psw.mask = spsw & PSW_MASK_SHORT_CTRL;
         /*
          * Invert short psw indication, so SIE will report a specification
-- 
2.53.0

[PATCH 4/5] target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (kvm)

[Philippe Mathieu-Daudé]

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). No logical change intended.

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

diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 54d28e37d4d..bd22f6f0188 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -36,6 +36,7 @@
 #include "qemu/main-loop.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/log.h"
+#include "system/memory.h"
 #include "system/system.h"
 #include "system/hw_accel.h"
 #include "system/runstate.h"
@@ -1664,13 +1665,16 @@ static void unmanageable_intercept(S390CPU *cpu, S390CrashReason reason,
 /* try to detect pgm check loops */
 static int handle_oper_loop(S390CPU *cpu, struct kvm_run *run)
 {
+    const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
     CPUState *cs = CPU(cpu);
     PSW oldpsw, newpsw;
 
-    newpsw.mask = ldq_be_phys(cs->as, cpu->env.psa +
-                              offsetof(LowCore, program_new_psw));
-    newpsw.addr = ldq_be_phys(cs->as, cpu->env.psa +
-                              offsetof(LowCore, program_new_psw) + 8);
+    newpsw.mask = address_space_ldq_be(cs->as, cpu->env.psa +
+                                       offsetof(LowCore, program_new_psw),
+                                       attrs, NULL);
+    newpsw.addr = address_space_ldq_be(cs->as, cpu->env.psa +
+                                       offsetof(LowCore, program_new_psw) + 8,
+                                       attrs, NULL);
     oldpsw.mask  = run->psw_mask;
     oldpsw.addr  = run->psw_addr;
     /*
-- 
2.53.0

[PATCH 5/5] configs/targets: Restrict the legacy ldst_phys() API on s390x target

[Philippe Mathieu-Daudé]

The s390x target doesn't use the legacy ldst_phys() API anymore.
Set the TARGET_NOT_USING_LEGACY_LDST_PHYS_API variable to hide
the legacy API to the qemu-system-loongarch64 binary, avoiding
further API uses to creep in.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 configs/targets/s390x-softmmu.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/targets/s390x-softmmu.mak b/configs/targets/s390x-softmmu.mak
index e273075f1a9..478988e6d4a 100644
--- a/configs/targets/s390x-softmmu.mak
+++ b/configs/targets/s390x-softmmu.mak
@@ -4,3 +4,4 @@ TARGET_KVM_HAVE_GUEST_DEBUG=y
 TARGET_XML_FILES= s390x-core64.xml s390-acr.xml s390-fpr.xml s390-vx.xml s390-cr.xml s390-virt.xml s390-virt-kvm.xml s390-gs.xml
 TARGET_LONG_BITS=64
 TARGET_NOT_USING_LEGACY_NATIVE_ENDIAN_API=y
+TARGET_NOT_USING_LEGACY_LDST_PHYS_API=y
-- 
2.53.0

Re: [PATCH 0/5] target/s390x: Forbid to use legacy ldst_phys() API

[Matthew Rosato]

On 3/19/26 2:51 PM, Philippe Mathieu-Daudé wrote:
> Replace legacy ld/st_phys() by address_space_ld/st()
> then lock the target again further legacy API uses.
> 
> Philippe Mathieu-Daudé (5):
>   target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (cpu)
>   target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (mmu)
>   target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (tcg)
>   target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (kvm)
>   configs/targets: Restrict the legacy ldst_phys() API on s390x target

For the series:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

For good measure I also ran some s390x kvm regression tests, took a less comprehensive spin with a tcg s390x guest (on s390 host), and forced a few of these codepaths as a sanity check.

> 
>  configs/targets/s390x-softmmu.mak |  1 +
>  target/s390x/cpu-system.c         |  2 +-
>  target/s390x/kvm/kvm.c            | 12 ++++++++----
>  target/s390x/mmu_helper.c         |  5 +++--
>  target/s390x/tcg/excp_helper.c    | 21 +++++++++++++--------
>  target/s390x/tcg/mem_helper.c     | 12 +++++++-----
>  6 files changed, 33 insertions(+), 20 deletions(-)
> 

Re: [PATCH 0/5] target/s390x: Forbid to use legacy ldst_phys() API

[Philippe Mathieu-Daudé]

On 20/3/26 20:28, Matthew Rosato wrote:
> On 3/19/26 2:51 PM, Philippe Mathieu-Daudé wrote:
>> Replace legacy ld/st_phys() by address_space_ld/st()
>> then lock the target again further legacy API uses.
>>
>> Philippe Mathieu-Daudé (5):
>>    target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (cpu)
>>    target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (mmu)
>>    target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (tcg)
>>    target/s390x: Replace legacy ld/st_phys -> address_space_ld/st (kvm)
>>    configs/targets: Restrict the legacy ldst_phys() API on s390x target
> 
> For the series:
> 
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 
> For good measure I also ran some s390x kvm regression tests, took a less comprehensive spin with a tcg s390x guest (on s390 host), and forced a few of these codepaths as a sanity check.

Thank you very much Matthew!