qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 2/2] target/s390x: Store r1/r2 for page-translation except


From: Thomas Huth
Subject: Re: [PATCH v6 2/2] target/s390x: Store r1/r2 for page-translation exceptions during MVPG
Date: Fri, 12 Mar 2021 10:04:48 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0

On 11/03/2021 20.44, David Hildenbrand wrote:
The PoP states:

     When EDAT-1 does not apply, and a program interruption due to a
     page-translation exception is recognized by the MOVE PAGE
     instruction, the contents of the R1 field of the instruction are
     stored in bit positions 0-3 of location 162, and the contents of
     the R2 field are stored in bit positions 4-7.

     If [...] an ASCE-type, region-first-translation,
     region-second-translation, region-third-translation, or
     segment-translation exception was recognized, the contents of
     location 162 are unpredictable.

So we have to write r1/r2 into the lowcore on page-translation
exceptions. Simply handle all exceptions inside our mvpg helper now.

Signed-off-by: David Hildenbrand <david@redhat.com>
---
  target/s390x/helper.h      |  2 +-
  target/s390x/insn-data.def |  2 +-
  target/s390x/mem_helper.c  | 46 +++++++++++++++++++++++---------------
  target/s390x/translate.c   |  7 +++++-
  4 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 55bd1551e6..d4e4f3388f 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -18,7 +18,7 @@ DEF_HELPER_3(srstu, void, env, i32, i32)
  DEF_HELPER_4(clst, i64, env, i64, i64, i64)
  DEF_HELPER_FLAGS_4(mvn, TCG_CALL_NO_WG, void, env, i32, i64, i64)
  DEF_HELPER_FLAGS_4(mvo, TCG_CALL_NO_WG, void, env, i32, i64, i64)
-DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i64, i64)
+DEF_HELPER_FLAGS_4(mvpg, TCG_CALL_NO_WG, i32, env, i64, i32, i32)
  DEF_HELPER_FLAGS_4(mvz, TCG_CALL_NO_WG, void, env, i32, i64, i64)
  DEF_HELPER_3(mvst, i32, env, i32, i32)
  DEF_HELPER_4(ex, void, env, i32, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index e5b6efabf3..0bb1886a2e 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -641,7 +641,7 @@
  /* MOVE NUMERICS */
      C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
  /* MOVE PAGE */
-    C(0xb254, MVPG,    RRE,   Z,   r1_o, r2_o, 0, 0, mvpg, 0)
+    C(0xb254, MVPG,    RRE,   Z,   0, 0, 0, 0, mvpg, 0)
  /* MOVE STRING */
      C(0xb255, MVST,    RRE,   Z,   0, 0, 0, 0, mvst, 0)
  /* MOVE WITH OPTIONAL SPECIFICATION */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 6f6e2f80f4..12e84a4285 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -915,8 +915,10 @@ uint64_t HELPER(clst)(CPUS390XState *env, uint64_t c, 
uint64_t s1, uint64_t s2)
  }
/* move page */
-uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t 
r2)
+uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint32_t r1, uint32_t 
r2)
  {
+    const uint64_t src = get_address(env, r2) & TARGET_PAGE_MASK;
+    const uint64_t dst = get_address(env, r1) & TARGET_PAGE_MASK;
      const int mmu_idx = cpu_mmu_index(env, false);
      const bool f = extract64(r0, 11, 1);
      const bool s = extract64(r0, 10, 1);
@@ -929,34 +931,42 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, 
uint64_t r1, uint64_t r2)
          tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
      }
- r1 = wrap_address(env, r1 & TARGET_PAGE_MASK);
-    r2 = wrap_address(env, r2 & TARGET_PAGE_MASK);
-
      /*
-     * TODO:
-     * - Access key handling
-     * - Store r1/r2 register identifiers at real location 162
+     * We always manually handle exceptions such that we can properly store
+     * r1/r2 to the lowcore on page-translation exceptions.
+     *
+     * TODO: Access key handling
       */
-    exc = access_prepare_nf(&srca, env, cco, r2, TARGET_PAGE_SIZE,
+    exc = access_prepare_nf(&srca, env, true, src, TARGET_PAGE_SIZE,
                              MMU_DATA_LOAD, mmu_idx, ra);
      if (exc) {
-        return 2;
+        if (cco) {
+            return 2;
+        }
+        goto inject_exc;
      }
-    exc = access_prepare_nf(&desta, env, cco, r1, TARGET_PAGE_SIZE,
+    exc = access_prepare_nf(&desta, env, true, dst, TARGET_PAGE_SIZE,
                              MMU_DATA_STORE, mmu_idx, ra);
      if (exc) {
-#if !defined(CONFIG_USER_ONLY)
-        if (exc == PGM_PROTECTION) {
-            stq_phys(env_cpu(env)->as,
-                     env->psa + offsetof(LowCore, trans_exc_code),
-                     env->tlb_fill_tec);
-            tcg_s390_program_interrupt(env, PGM_PROTECTION, ra);
+        if (cco && exc != PGM_PROTECTION) {
+            return 1;
          }
-#endif
-        return 1;
+        goto inject_exc;
      }
      access_memmove(env, &desta, &srca, ra);
      return 0; /* data moved */
+inject_exc:
+#if !defined(CONFIG_USER_ONLY)
+    if (exc != PGM_ADDRESSING) {
+        stq_phys(env_cpu(env)->as, env->psa + offsetof(LowCore, 
trans_exc_code),
+                 env->tlb_fill_tec);
+    }
+    if (exc == PGM_PAGE_TRANS) {
+        stb_phys(env_cpu(env)->as, env->psa + offsetof(LowCore, op_access_id),
+                 r1 << 4 | r2);
+    }
+#endif
+    tcg_s390_program_interrupt(env, exc, ra);
  }
/* string copy */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 61dd0341e4..4f953ddfba 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3513,7 +3513,12 @@ static DisasJumpType op_mvo(DisasContext *s, DisasOps *o)
static DisasJumpType op_mvpg(DisasContext *s, DisasOps *o)
  {
-    gen_helper_mvpg(cc_op, cpu_env, regs[0], o->in1, o->in2);
+    TCGv_i32 t1 = tcg_const_i32(get_field(s, r1));
+    TCGv_i32 t2 = tcg_const_i32(get_field(s, r2));
+
+    gen_helper_mvpg(cc_op, cpu_env, regs[0], t1, t2);
+    tcg_temp_free_i32(t1);
+    tcg_temp_free_i32(t2);
      set_cc_static(s);
      return DISAS_NEXT;
  }


Looks fine to me.

Reviewed-by: Thomas Huth <thuth@redhat.com>




reply via email to

[Prev in Thread] Current Thread [Next in Thread]