[PATCH v2 0/2] accel/tcg: Make probe_access() a generic TCG helper

[PATCH v2 0/2] accel/tcg: Make probe_access() a generic TCG helper

[Philippe Mathieu-Daudé]

probe_access() is not ARM specific and can be reused by
other targets, so make it a generic helper (after widening
its address argument to 64-bits).

Philippe Mathieu-Daudé (2):
  target/arm: Widen probe_access()'s address argument to 64-bits
  accel/tcg: Make probe_access() a generic TCG helper

 accel/tcg/tcg-runtime.h    |  2 ++
 target/arm/helper.h        |  2 --
 accel/tcg/tcg-runtime.c    | 16 ++++++++++++++++
 target/arm/tcg/op_helper.c | 16 ----------------
 4 files changed, 18 insertions(+), 18 deletions(-)

-- 
2.38.1

[PATCH v2 1/2] target/arm: Widen probe_access()'s address argument to 64-bits

[Philippe Mathieu-Daudé]

Make probe_access() agnostic to the address size of the guest,
using a uint64_t type since usage varies between guests.

Since we don't access any CPUARMState particular env field,
use the generic CPUArchState type insted.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/arm/helper.h        | 2 +-
 target/arm/tcg/op_helper.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.h b/target/arm/helper.h
index 3335c2b10b..cee9462c73 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -101,7 +101,7 @@ DEF_HELPER_FLAGS_1(rebuild_hflags_a32_newel, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, int)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, int)
 
-DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, tl, i32, i32, i32)
+DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, i64, i32, i32, i32)
 
 DEF_HELPER_1(vfp_get_fpscr, i32, env)
 DEF_HELPER_2(vfp_set_fpscr, void, env, i32)
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 3baf8004f6..6cb84bc994 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -1009,7 +1009,7 @@ uint32_t HELPER(ror_cc)(CPUARMState *env, uint32_t x, uint32_t i)
     }
 }
 
-void HELPER(probe_access)(CPUARMState *env, target_ulong ptr,
+void HELPER(probe_access)(CPUArchState *env, uint64_t ptr,
                           uint32_t access_type, uint32_t mmu_idx,
                           uint32_t size)
 {
-- 
2.38.1

[PATCH v2 2/2] accel/tcg: Make probe_access() a generic TCG helper

[Philippe Mathieu-Daudé]

probe_access() is not ARM specific and can be reused by
other targets.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tcg-runtime.h    |  2 ++
 target/arm/helper.h        |  2 --
 accel/tcg/tcg-runtime.c    | 16 ++++++++++++++++
 target/arm/tcg/op_helper.c | 16 ----------------
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index 39e68007f9..a93a5d87a5 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -28,6 +28,8 @@ DEF_HELPER_FLAGS_1(lookup_tb_ptr, TCG_CALL_NO_WG_SE, cptr, env)
 
 DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env)
 
+DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, i64, i32, i32, i32)
+
 #ifndef IN_HELPER_PROTO
 /*
  * Pass calls to memset directly to libc, without a thunk in qemu.
diff --git a/target/arm/helper.h b/target/arm/helper.h
index cee9462c73..4336571d88 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -101,8 +101,6 @@ DEF_HELPER_FLAGS_1(rebuild_hflags_a32_newel, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a32, TCG_CALL_NO_RWG, void, env, int)
 DEF_HELPER_FLAGS_2(rebuild_hflags_a64, TCG_CALL_NO_RWG, void, env, int)
 
-DEF_HELPER_FLAGS_5(probe_access, TCG_CALL_NO_WG, void, env, i64, i32, i32, i32)
-
 DEF_HELPER_1(vfp_get_fpscr, i32, env)
 DEF_HELPER_2(vfp_set_fpscr, void, env, i32)
 
diff --git a/accel/tcg/tcg-runtime.c b/accel/tcg/tcg-runtime.c
index 9fa539ad3d..b241a11791 100644
--- a/accel/tcg/tcg-runtime.c
+++ b/accel/tcg/tcg-runtime.c
@@ -152,3 +152,19 @@ void HELPER(exit_atomic)(CPUArchState *env)
 {
     cpu_loop_exit_atomic(env_cpu(env), GETPC());
 }
+
+void HELPER(probe_access)(CPUArchState *env, uint64_t ptr,
+                          uint32_t access_type, uint32_t mmu_idx,
+                          uint32_t size)
+{
+    uint32_t in_page = -((uint32_t)ptr | TARGET_PAGE_SIZE);
+    uintptr_t ra = GETPC();
+
+    if (likely(size <= in_page)) {
+        probe_access(env, ptr, size, access_type, mmu_idx, ra);
+    } else {
+        probe_access(env, ptr, in_page, access_type, mmu_idx, ra);
+        probe_access(env, ptr + in_page, size - in_page,
+                     access_type, mmu_idx, ra);
+    }
+}
diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index 6cb84bc994..d507a3df16 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -1009,22 +1009,6 @@ uint32_t HELPER(ror_cc)(CPUARMState *env, uint32_t x, uint32_t i)
     }
 }
 
-void HELPER(probe_access)(CPUArchState *env, uint64_t ptr,
-                          uint32_t access_type, uint32_t mmu_idx,
-                          uint32_t size)
-{
-    uint32_t in_page = -((uint32_t)ptr | TARGET_PAGE_SIZE);
-    uintptr_t ra = GETPC();
-
-    if (likely(size <= in_page)) {
-        probe_access(env, ptr, size, access_type, mmu_idx, ra);
-    } else {
-        probe_access(env, ptr, in_page, access_type, mmu_idx, ra);
-        probe_access(env, ptr + in_page, size - in_page,
-                     access_type, mmu_idx, ra);
-    }
-}
-
 /*
  * This function corresponds to AArch64.vESBOperation().
  * Note that the AArch32 version is not functionally different.
-- 
2.38.1

Re: [PATCH v2 2/2] accel/tcg: Make probe_access() a generic TCG helper

[Richard Henderson]

On 6/19/23 11:19, Philippe Mathieu-Daudé wrote:
> +
> +void HELPER(probe_access)(CPUArchState *env, uint64_t ptr,
> +                          uint32_t access_type, uint32_t mmu_idx,
> +                          uint32_t size)
> +{
> +    uint32_t in_page = -((uint32_t)ptr | TARGET_PAGE_SIZE);
> +    uintptr_t ra = GETPC();
> +
> +    if (likely(size <= in_page)) {
> +        probe_access(env, ptr, size, access_type, mmu_idx, ra);
> +    } else {
> +        probe_access(env, ptr, in_page, access_type, mmu_idx, ra);
> +        probe_access(env, ptr + in_page, size - in_page,
> +                     access_type, mmu_idx, ra);
> +    }
> +}

This works if and only if size is <= TARGET_PAGE_SIZE.
This should probably have been documented for Arm, but
should definitely be documented for its new location.

It looks like this could be used for

hexagon: HELPER(probe_noshuf_load)
s390x/tcg/mem_helper.c:void HELPER(probe_write_access)

Although s390x is written to handle much larger SIZE, the maximum that it will ever issue 
is 16 * 16 = 256 for op_vstm.  But it also handles target-specific address wrap at the end 
of the address space, which the above does not.

So perhaps it's not worth moving this after all?


r~