[Qemu-devel] [PATCH v4 0/3] target/s390x: implement loads/store quadword

[Qemu-devel] [PATCH v4 0/3] target/s390x: implement loads/store quadword

[Aurelien Jarno]

This patchset implements the LOAD PAIR FROM QUADWORD and STORE PAIR TO
QUADWORD instructions. The corresponding patches have been in my previous
patchset and the pull request from Richard, but they failed to build on a
host without atomic128 support.

This new version fixes that. It has to be applied over the pull request
as it makes uses of the check_alignment function.

Finally the latest patch fixes a lack of alignement check in CDSG,
discovered as I used it as an example about how to properly handle hosts
without atomic128 support.

Aurelien Jarno (3):
  target/s390x: implement LOAD PAIR FROM QUADWORD
  target/s390x: implement STORE PAIR TO QUADWORD
  target/s390x: check alignment in CDSG in the !CONFIG_ATOMIC128 case

 target/s390x/helper.h      |  2 ++
 target/s390x/insn-data.def |  4 ++++
 target/s390x/mem_helper.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 target/s390x/translate.c   | 13 ++++++++++++
 4 files changed, 72 insertions(+)

-- 
2.11.0

[Qemu-devel] [PATCH v4 1/3] target/s390x: implement LOAD PAIR FROM QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 27 +++++++++++++++++++++++++++
 target/s390x/translate.c   |  7 +++++++
 4 files changed, 37 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 5724691d1b..61d4ef899e 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -103,6 +103,7 @@ DEF_HELPER_2(stfle, i32, env, i64)
 DEF_HELPER_FLAGS_3(keb, TCG_CALL_NO_WG, i32, env, i64, i64)
 DEF_HELPER_FLAGS_3(kdb, TCG_CALL_NO_WG, i32, env, i64, i64)
 DEF_HELPER_FLAGS_5(kxb, TCG_CALL_NO_WG, i32, env, i64, i64, i64, i64)
+DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 9976d290c4..32dee40269 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -517,6 +517,8 @@
 /* LOAD PAIR DISJOINT */
     D(0xc804, LPD,     SSF,   ILA, 0, 0, new_P, r3_P32, lpd, 0, MO_TEUL)
     D(0xc805, LPDG,    SSF,   ILA, 0, 0, new_P, r3_P64, lpd, 0, MO_TEQ)
+/* LOAD PAIR FROM QUADWORD */
+    C(0xe38f, LPQ,     RXY_a, Z,   0, a2, r1_P, 0, lpq, 0)
 /* LOAD POSITIVE */
     C(0x1000, LPR,     RR_a,  Z,   0, r2_32s, new, r1_32, abs, abs32)
     C(0xb900, LPGR,    RRE,   Z,   0, r2, r1, 0, abs, abs64)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index be89cc4fb4..f48908cecb 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1665,6 +1665,33 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 }
 #endif
 
+/* load pair from quadword */
+uint64_t HELPER(lpq)(CPUS390XState *env, uint64_t addr)
+{
+    uintptr_t ra = GETPC();
+    uint64_t hi, lo;
+
+    if (parallel_cpus) {
+#ifndef CONFIG_ATOMIC128
+        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+#else
+        int mem_idx = cpu_mmu_index(env, false);
+        TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+        Int128 v = helper_atomic_ldo_be_mmu(env, addr, oi, ra);
+        hi = int128_gethi(v);
+        lo = int128_getlo(v);
+#endif
+    } else {
+        check_alignment(env, addr, 16, ra);
+
+        hi = cpu_ldq_data_ra(env, addr + 0, ra);
+        lo = cpu_ldq_data_ra(env, addr + 8, ra);
+    }
+
+    env->retxl = lo;
+    return hi;
+}
+
 /* Execute instruction.  This instruction executes an insn modified with
    the contents of r1.  It does not change the executed instruction in memory;
    it does not change the program counter.
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 4f4dffbdf4..3b621993cf 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2904,6 +2904,13 @@ static ExitStatus op_lpd(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_lpq(DisasContext *s, DisasOps *o)
+{
+    gen_helper_lpq(o->out, cpu_env, o->in2);
+    return_low128(o->out2);
+    return NO_EXIT;
+}
+
 #ifndef CONFIG_USER_ONLY
 static ExitStatus op_lura(DisasContext *s, DisasOps *o)
 {
-- 
2.11.0

[Qemu-devel] [PATCH v4 2/3] target/s390x: implement STORE PAIR TO QUADWORD

[Aurelien Jarno]

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/helper.h      |  1 +
 target/s390x/insn-data.def |  2 ++
 target/s390x/mem_helper.c  | 24 ++++++++++++++++++++++++
 target/s390x/translate.c   |  6 ++++++
 4 files changed, 33 insertions(+)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 61d4ef899e..12d7f8fe95 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -104,6 +104,7 @@ DEF_HELPER_FLAGS_3(keb, TCG_CALL_NO_WG, i32, env, i64, i64)
 DEF_HELPER_FLAGS_3(kdb, TCG_CALL_NO_WG, i32, env, i64, i64)
 DEF_HELPER_FLAGS_5(kxb, TCG_CALL_NO_WG, i32, env, i64, i64, i64, i64)
 DEF_HELPER_FLAGS_2(lpq, TCG_CALL_NO_WG, i64, env, i64)
+DEF_HELPER_FLAGS_4(stpq, TCG_CALL_NO_WG, void, env, i64, i64, i64)
 
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_3(servc, i32, env, i64, i64)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 32dee40269..73dd05daf0 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -796,6 +796,8 @@
 /* STORE ACCESS MULTIPLE */
     C(0x9b00, STAM,    RS_a,  Z,   0, a2, 0, 0, stam, 0)
     C(0xeb9b, STAMY,   RSY_a, LD,  0, a2, 0, 0, stam, 0)
+/* STORE PAIR TO QUADWORD */
+    C(0xe38e, STPQ,    RXY_a, Z,   0, a2, r1_P, 0, stpq, 0)
 
 /* SUBTRACT */
     C(0x1b00, SR,      RR_a,  Z,   r1, r2, new, r1_32, sub, subs32)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index f48908cecb..a8988e0293 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1692,6 +1692,30 @@ uint64_t HELPER(lpq)(CPUS390XState *env, uint64_t addr)
     return hi;
 }
 
+/* store pair to quadword */
+void HELPER(stpq)(CPUS390XState *env, uint64_t addr,
+                  uint64_t low, uint64_t high)
+{
+    uintptr_t ra = GETPC();
+
+    if (parallel_cpus) {
+#ifndef CONFIG_ATOMIC128
+        cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+#else
+        int mem_idx = cpu_mmu_index(env, false);
+        TCGMemOpIdx oi = make_memop_idx(MO_TEQ | MO_ALIGN_16, mem_idx);
+
+        Int128 v = int128_make128(low, high);
+        helper_atomic_sto_be_mmu(env, addr, v, oi, ra);
+#endif
+    } else {
+        check_alignment(env, addr, 16, ra);
+
+        cpu_stq_data_ra(env, addr + 0, high, ra);
+        cpu_stq_data_ra(env, addr + 8, low, ra);
+    }
+}
+
 /* Execute instruction.  This instruction executes an insn modified with
    the contents of r1.  It does not change the executed instruction in memory;
    it does not change the program counter.
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 3b621993cf..8702cc8cc7 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -4198,6 +4198,12 @@ static ExitStatus op_stmh(DisasContext *s, DisasOps *o)
     return NO_EXIT;
 }
 
+static ExitStatus op_stpq(DisasContext *s, DisasOps *o)
+{
+    gen_helper_stpq(cpu_env, o->in2, o->out2, o->out);
+    return NO_EXIT;
+}
+
 static ExitStatus op_srst(DisasContext *s, DisasOps *o)
 {
     gen_helper_srst(o->in1, cpu_env, regs[0], o->in1, o->in2);
-- 
2.11.0

[Qemu-devel] [PATCH v4 3/3] target/s390x: check alignment in CDSG in the !CONFIG_ATOMIC128 case

[Aurelien Jarno]

The CDSG instruction requires a 16-byte alignement, as expressed in
the MO_ALIGN_16 passed to helper_atomic_cmpxchgo_be_mmu. In the non
parallel case, use check_alignment to enforce this.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/mem_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index a8988e0293..80caab9c9d 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1262,6 +1262,8 @@ void HELPER(cdsg)(CPUS390XState *env, uint64_t addr,
     } else {
         uint64_t oldh, oldl;
 
+        check_alignment(env, addr, 16, ra);
+
         oldh = cpu_ldq_data_ra(env, addr + 0, ra);
         oldl = cpu_ldq_data_ra(env, addr + 8, ra);
 
-- 
2.11.0

Re: [Qemu-devel] [PATCH v4 0/3] target/s390x: implement loads/store quadword

[Richard Henderson]

On 06/04/2017 01:20 PM, Aurelien Jarno wrote:
> This patchset implements the LOAD PAIR FROM QUADWORD and STORE PAIR TO
> QUADWORD instructions. The corresponding patches have been in my previous
> patchset and the pull request from Richard, but they failed to build on a
> host without atomic128 support.
> 
> This new version fixes that. It has to be applied over the pull request
> as it makes uses of the check_alignment function.
> 
> Finally the latest patch fixes a lack of alignement check in CDSG,
> discovered as I used it as an example about how to properly handle hosts
> without atomic128 support.
> 
> Aurelien Jarno (3):
>    target/s390x: implement LOAD PAIR FROM QUADWORD
>    target/s390x: implement STORE PAIR TO QUADWORD
>    target/s390x: check alignment in CDSG in the !CONFIG_ATOMIC128 case
> 
>   target/s390x/helper.h      |  2 ++
>   target/s390x/insn-data.def |  4 ++++
>   target/s390x/mem_helper.c  | 53 ++++++++++++++++++++++++++++++++++++++++++++++
>   target/s390x/translate.c   | 13 ++++++++++++
>   4 files changed, 72 insertions(+)
> 

Thanks.  Incorporated into my v2 pull.


r~