[PATCH v2] target/s390x: Implement the MVPG condition-code-option bit

[PATCH v2] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

If the CCO bit is set, MVPG should not generate an exception
but report page translation faults via a CC code.  Create a new
helper, access_prepare_nf, which can use probe_access_flags in
non-faulting mode.

Cc: David Hildenbrand <david@redhat.com>
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/mem_helper.c | 76 +++++++++++++++++++++++++++------------
 1 file changed, 53 insertions(+), 23 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 25cfede806..9e359b0d19 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -130,28 +130,51 @@ typedef struct S390Access {
     int mmu_idx;
 } S390Access;
 
+static bool access_prepare_nf(S390Access *access, CPUS390XState *env,
+                              bool nofault, vaddr vaddr1, int size,
+                              MMUAccessType access_type,
+                              int mmu_idx, uintptr_t ra)
+{
+    void *haddr1, *haddr2 = NULL;
+    int size1, size2;
+    vaddr vaddr2 = 0;
+    bool ok;
+
+    g_assert(size > 0 && size <= 4096);
+
+    size1 = MIN(size, -(vaddr1 | TARGET_PAGE_MASK)),
+    size2 = size - size1;
+
+    ok = probe_access_flags(env, vaddr1, access_type, mmu_idx,
+                            nofault, &haddr1, ra);
+    if (likely(ok) && unlikely(size2)) {
+        /* The access crosses page boundaries. */
+        vaddr2 = wrap_address(env, vaddr1 + size1);
+        ok = probe_access_flags(env, vaddr2, access_type, mmu_idx,
+                                nofault, &haddr2, ra);
+    }
+
+    *access = (S390Access) {
+        .vaddr1 = vaddr1,
+        .vaddr2 = vaddr2,
+        .haddr1 = haddr1,
+        .haddr2 = haddr2,
+        .size1 = size1,
+        .size2 = size2,
+        .mmu_idx = mmu_idx
+    };
+    return ok;
+}
+
 static S390Access access_prepare(CPUS390XState *env, vaddr vaddr, int size,
                                  MMUAccessType access_type, int mmu_idx,
                                  uintptr_t ra)
 {
-    S390Access access = {
-        .vaddr1 = vaddr,
-        .size1 = MIN(size, -(vaddr | TARGET_PAGE_MASK)),
-        .mmu_idx = mmu_idx,
-    };
-
-    g_assert(size > 0 && size <= 4096);
-    access.haddr1 = probe_access(env, access.vaddr1, access.size1, access_type,
-                                 mmu_idx, ra);
-
-    if (unlikely(access.size1 != size)) {
-        /* The access crosses page boundaries. */
-        access.vaddr2 = wrap_address(env, vaddr + access.size1);
-        access.size2 = size - access.size1;
-        access.haddr2 = probe_access(env, access.vaddr2, access.size2,
-                                     access_type, mmu_idx, ra);
-    }
-    return access;
+    S390Access ret;
+    bool ok = access_prepare_nf(&ret, env, false, vaddr, size,
+                                access_type, mmu_idx, ra);
+    g_assert(ok);
+    return ret;
 }
 
 /* Helper to handle memset on a single page. */
@@ -845,8 +868,10 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
     const int mmu_idx = cpu_mmu_index(env, false);
     const bool f = extract64(r0, 11, 1);
     const bool s = extract64(r0, 10, 1);
+    const bool cco = extract64(r0, 8, 1);
     uintptr_t ra = GETPC();
     S390Access srca, desta;
+    bool ok;
 
     if ((f && s) || extract64(r0, 12, 4)) {
         tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
@@ -858,13 +883,18 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
     /*
      * TODO:
      * - Access key handling
-     * - CC-option with surpression of page-translation exceptions
      * - Store r1/r2 register identifiers at real location 162
      */
-    srca = access_prepare(env, r2, TARGET_PAGE_SIZE, MMU_DATA_LOAD, mmu_idx,
-                          ra);
-    desta = access_prepare(env, r1, TARGET_PAGE_SIZE, MMU_DATA_STORE, mmu_idx,
-                           ra);
+    ok = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+                           MMU_DATA_LOAD, mmu_idx, ra);
+    if (!ok) {
+        return 2;
+    }
+    ok = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
+                           MMU_DATA_STORE, mmu_idx, ra);
+    if (!ok) {
+        return 1;
+    }
     access_memmove(env, &desta, &srca, ra);
     return 0; /* data moved */
 }
-- 
2.25.1

Re: [PATCH v2] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

On 3/2/21 5:39 PM, Richard Henderson wrote:
> If the CCO bit is set, MVPG should not generate an exception
> but report page translation faults via a CC code.  Create a new
> helper, access_prepare_nf, which can use probe_access_flags in
> non-faulting mode.
> 
> Cc: David Hildenbrand<david@redhat.com>
> Reported-by: Thomas Huth<thuth@redhat.com>
> Signed-off-by: Richard Henderson<richard.henderson@linaro.org>
> ---
>   target/s390x/mem_helper.c | 76 +++++++++++++++++++++++++++------------
>   1 file changed, 53 insertions(+), 23 deletions(-)

Oh, I meant to say that this is untested, but is what I was
talking about wrt performing only a single lookup per page.


r~

Re: [PATCH v2] target/s390x: Implement the MVPG condition-code-option bit

[Richard Henderson]

On 3/2/21 5:39 PM, Richard Henderson wrote:
> +    ok = probe_access_flags(env, vaddr1, access_type, mmu_idx,
> +                            nofault, &haddr1, ra);

Bah, I confused myself and remembered the wrong interface.
Expect a v3.

r~