[PATCH v7 0/4] s390x: Add partial z15 support and tests

[PATCH v7 0/4] s390x: Add partial z15 support and tests

[David Miller]

Add partial support for s390x z15 ga1 and specific tests for mie3 


v6 -> v7:
* Modified SELFHR insn-data + test to ensure high 32bits are copied.
* Changed m3 mask test value for popcnt to fix mie3 variant.

v5 -> v6:
* Swap operands for sel* instructions
* Use .insn in tests for z15 arch instructions

v4 -> v5:
* Readd missing tests/tcg/s390x/mie3-*.c to patch

v3 -> v4:
* Change popcnt encoding RRE -> RRF_c
* Remove redundant code op_sel -> op_loc
* Cleanup for checkpatch.pl
* Readded mie3-* to Makefile.target

v2 -> v3:
* Moved tests to separate patch.
* Combined patches into series.

David Miller (4):
  s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3
    for the s390x
   * Reviewed-by: David Hildenbrand <david@redhat.com>
  s390x/cpumodel: Bump up QEMU model to a stripped-down IBM z15 GA1
   * Reviewed-by: David Hildenbrand <david@redhat.com>
  tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions
    Facility 3
  tests/tcg/s390x: changed to using .insn for tests requiring z15
   * Reviewed-by: Thomas Huth <thuth@redhat.com>

 hw/s390x/s390-virtio-ccw.c      |  3 ++
 target/s390x/cpu_models.c       |  6 ++--
 target/s390x/gen-features.c     |  6 +++-
 target/s390x/helper.h           |  1 +
 target/s390x/tcg/insn-data.def  | 30 +++++++++++++++--
 target/s390x/tcg/mem_helper.c   | 20 +++++++++++
 target/s390x/tcg/translate.c    | 60 +++++++++++++++++++++++++++++++--
 tests/tcg/s390x/Makefile.target |  5 ++-
 tests/tcg/s390x/mie3-compl.c    | 56 ++++++++++++++++++++++++++++++
 tests/tcg/s390x/mie3-mvcrl.c    | 31 +++++++++++++++++
 tests/tcg/s390x/mie3-sel.c      | 42 +++++++++++++++++++++++
 11 files changed, 250 insertions(+), 10 deletions(-)
 create mode 100644 tests/tcg/s390x/mie3-compl.c
 create mode 100644 tests/tcg/s390x/mie3-mvcrl.c
 create mode 100644 tests/tcg/s390x/mie3-sel.c

-- 
2.32.0

[PATCH v7 1/4] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x

[David Miller]

resolves: https://gitlab.com/qemu-project/qemu/-/issues/737
implements:
AND WITH COMPLEMENT   (NCRK, NCGRK)
NAND                  (NNRK, NNGRK)
NOT EXCLUSIVE OR      (NXRK, NXGRK)
NOR                   (NORK, NOGRK)
OR WITH COMPLEMENT    (OCRK, OCGRK)
SELECT                (SELR, SELGR)
SELECT HIGH           (SELFHR)
MOVE RIGHT TO LEFT    (MVCRL)
POPULATION COUNT      (POPCNT)

Signed-off-by: David Miller <dmiller423@gmail.com>
---
 target/s390x/gen-features.c    |  1 +
 target/s390x/helper.h          |  1 +
 target/s390x/tcg/insn-data.def | 30 +++++++++++++++--
 target/s390x/tcg/mem_helper.c  | 20 ++++++++++++
 target/s390x/tcg/translate.c   | 60 ++++++++++++++++++++++++++++++++--
 5 files changed, 107 insertions(+), 5 deletions(-)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 7cb1a6ec10..a3f30f69d9 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -740,6 +740,7 @@ static uint16_t qemu_LATEST[] = {
 
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
+    S390_FEAT_MISC_INSTRUCTION_EXT3,
     /* generates a dependency warning, leave it out for now */
     S390_FEAT_MSA_EXT_5,
 };
diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 271b081e8c..69f69cf718 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -4,6 +4,7 @@ DEF_HELPER_FLAGS_4(nc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(oc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(xc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(mvc, TCG_CALL_NO_WG, void, env, i32, i64, i64)
+DEF_HELPER_FLAGS_4(mvcrl, TCG_CALL_NO_WG, void, env, i64, i64, i64)
 DEF_HELPER_FLAGS_4(mvcin, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(clc, TCG_CALL_NO_WG, i32, env, i32, i64, i64)
 DEF_HELPER_3(mvcl, i32, env, i32, i32)
diff --git a/target/s390x/tcg/insn-data.def b/target/s390x/tcg/insn-data.def
index 1c3e115712..35e55d454e 100644
--- a/target/s390x/tcg/insn-data.def
+++ b/target/s390x/tcg/insn-data.def
@@ -105,6 +105,9 @@
     D(0xa507, NILL,    RI_a,  Z,   r1_o, i2_16u, r1, 0, andi, 0, 0x1000)
     D(0x9400, NI,      SI,    Z,   la1, i2_8u, new, 0, ni, nz64, MO_UB)
     D(0xeb54, NIY,     SIY,   LD,  la1, i2_8u, new, 0, ni, nz64, MO_UB)
+/* AND WITH COMPLEMENT */
+    C(0xb9f5, NCRK,    RRF_a, MIE3, r2, r3, new, r1_32, andc, nz32)
+    C(0xb9e5, NCGRK,   RRF_a, MIE3, r2, r3, r1, 0, andc, nz64)
 
 /* BRANCH AND LINK */
     C(0x0500, BALR,    RR_a,  Z,   0, r2_nz, r1, 0, bal, 0)
@@ -640,6 +643,8 @@
     C(0xeb8e, MVCLU,   RSY_a, E2,  0, a2, 0, 0, mvclu, 0)
 /* MOVE NUMERICS */
     C(0xd100, MVN,     SS_a,  Z,   la1, a2, 0, 0, mvn, 0)
+/* MOVE RIGHT TO LEFT */
+    C(0xe50a, MVCRL,   SSE,  MIE3, la1, a2, 0, 0, mvcrl, 0)
 /* MOVE PAGE */
     C(0xb254, MVPG,    RRE,   Z,   0, 0, 0, 0, mvpg, 0)
 /* MOVE STRING */
@@ -707,6 +712,16 @@
     F(0xed0f, MSEB,    RXF,   Z,   e1, m2_32u, new, e1, mseb, 0, IF_BFP)
     F(0xed1f, MSDB,    RXF,   Z,   f1, m2_64, new, f1, msdb, 0, IF_BFP)
 
+/* NAND */
+    C(0xb974, NNRK,    RRF_a, MIE3, r2, r3, new, r1_32, nand, nz32)
+    C(0xb964, NNGRK,   RRF_a, MIE3, r2, r3, r1, 0, nand, nz64)
+/* NOR */
+    C(0xb976, NORK,    RRF_a, MIE3, r2, r3, new, r1_32, nor, nz32)
+    C(0xb966, NOGRK,   RRF_a, MIE3, r2, r3, r1, 0, nor, nz64)
+/* NOT EXCLUSIVE OR */
+    C(0xb977, NXRK,    RRF_a, MIE3, r2, r3, new, r1_32, nxor, nz32)
+    C(0xb967, NXGRK,   RRF_a, MIE3, r2, r3, r1, 0, nxor, nz64)
+
 /* OR */
     C(0x1600, OR,      RR_a,  Z,   r1, r2, new, r1_32, or, nz32)
     C(0xb9f6, ORK,     RRF_a, DO,  r2, r3, new, r1_32, or, nz32)
@@ -725,6 +740,9 @@
     D(0xa50b, OILL,    RI_a,  Z,   r1_o, i2_16u, r1, 0, ori, 0, 0x1000)
     D(0x9600, OI,      SI,    Z,   la1, i2_8u, new, 0, oi, nz64, MO_UB)
     D(0xeb56, OIY,     SIY,   LD,  la1, i2_8u, new, 0, oi, nz64, MO_UB)
+/* OR WITH COMPLEMENT */
+    C(0xb975, OCRK,    RRF_a, MIE3, r2, r3, new, r1_32, orc, nz32)
+    C(0xb965, OCGRK,   RRF_a, MIE3, r2, r3, r1, 0, orc, nz64)
 
 /* PACK */
     /* Really format SS_b, but we pack both lengths into one argument
@@ -735,6 +753,9 @@
 /* PACK UNICODE */
     C(0xe100, PKU,     SS_f,  E2,  la1, a2, 0, 0, pku, 0)
 
+/* POPULATION COUNT */
+    C(0xb9e1, POPCNT,  RRF_c, PC,  0, r2_o, r1, 0, popcnt, nz64)
+
 /* PREFETCH */
     /* Implemented as nops of course.  */
     C(0xe336, PFD,     RXY_b, GIE, 0, 0, 0, 0, 0, 0)
@@ -743,9 +764,6 @@
     /* Implemented as nop of course.  */
     C(0xb2e8, PPA,     RRF_c, PPA, 0, 0, 0, 0, 0, 0)
 
-/* POPULATION COUNT */
-    C(0xb9e1, POPCNT,  RRE,   PC,  0, r2_o, r1, 0, popcnt, nz64)
-
 /* ROTATE LEFT SINGLE LOGICAL */
     C(0xeb1d, RLL,     RSY_a, Z,   r3_o, sh, new, r1_32, rll32, 0)
     C(0xeb1c, RLLG,    RSY_a, Z,   r3_o, sh, r1, 0, rll64, 0)
@@ -765,6 +783,12 @@
 /* SEARCH STRING UNICODE */
     C(0xb9be, SRSTU,   RRE,   ETF3, 0, 0, 0, 0, srstu, 0)
 
+/* SELECT */
+    C(0xb9f0, SELR,    RRF_a, MIE3, r3, r2, new, r1_32, loc, 0)
+    C(0xb9e3, SELGR,   RRF_a, MIE3, r3, r2, r1, 0, loc, 0)
+/* SELECT HIGH */
+    C(0xb9c0, SELFHR,  RRF_a, MIE3, r3_sr32, r2_sr32, new, r1_32h, loc, 0)
+
 /* SET ACCESS */
     C(0xb24e, SAR,     RRE,   Z,   0, r2_o, 0, 0, sar, 0)
 /* SET ADDRESSING MODE */
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 406578d105..ed1a77ebe8 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -546,6 +546,26 @@ void HELPER(mvc)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
     do_helper_mvc(env, l, dest, src, GETPC());
 }
 
+/* move right to left */
+void HELPER(mvcrl)(CPUS390XState *env, uint64_t l, uint64_t dest, uint64_t src)
+{
+    const int mmu_idx = cpu_mmu_index(env, false);
+    const uint64_t ra = GETPC();
+    S390Access srca, desta;
+    int32_t i;
+
+    /* MVCRL always copies one more byte than specified - maximum is 256 */
+    l++;
+
+    srca = access_prepare(env, src, l, MMU_DATA_LOAD, mmu_idx, ra);
+    desta = access_prepare(env, dest, l, MMU_DATA_STORE, mmu_idx, ra);
+
+    for (i = l - 1; i >= 0; i--) {
+        uint8_t byte = access_get_byte(env, &srca, i, ra);
+        access_set_byte(env, &desta, i, byte, ra);
+    }
+}
+
 /* move inverse  */
 void HELPER(mvcin)(CPUS390XState *env, uint32_t l, uint64_t dest, uint64_t src)
 {
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 46dea73357..92faa1b65f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -1498,6 +1498,36 @@ static DisasJumpType op_andi(DisasContext *s, DisasOps *o)
     return DISAS_NEXT;
 }
 
+static DisasJumpType op_andc(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_andc_i64(o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
+static DisasJumpType op_orc(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_orc_i64(o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
+static DisasJumpType op_nand(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_nand_i64(o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
+static DisasJumpType op_nor(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_nor_i64(o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
+static DisasJumpType op_nxor(DisasContext *s, DisasOps *o)
+{
+    tcg_gen_eqv_i64(o->out, o->in1, o->in2);
+    return DISAS_NEXT;
+}
+
 static DisasJumpType op_ni(DisasContext *s, DisasOps *o)
 {
     o->in1 = tcg_temp_new_i64();
@@ -2958,7 +2988,13 @@ static DisasJumpType op_loc(DisasContext *s, DisasOps *o)
 {
     DisasCompare c;
 
-    disas_jcc(s, &c, get_field(s, m3));
+    if (have_field(s, m3)) {
+        /* LOAD * ON CONDITION */
+        disas_jcc(s, &c, get_field(s, m3));
+    } else {
+        /* SELECT */
+        disas_jcc(s, &c, get_field(s, m4));
+    }
 
     if (c.is_64) {
         tcg_gen_movcond_i64(c.cond, o->out, c.u.s64.a, c.u.s64.b,
@@ -3358,6 +3394,12 @@ static DisasJumpType op_mvc(DisasContext *s, DisasOps *o)
     return DISAS_NEXT;
 }
 
+static DisasJumpType op_mvcrl(DisasContext *s, DisasOps *o)
+{
+    gen_helper_mvcrl(cpu_env, regs[0], o->addr1, o->in2);
+    return DISAS_NEXT;
+}
+
 static DisasJumpType op_mvcin(DisasContext *s, DisasOps *o)
 {
     TCGv_i32 l = tcg_const_i32(get_field(s, l1));
@@ -3744,7 +3786,13 @@ static DisasJumpType op_pku(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_popcnt(DisasContext *s, DisasOps *o)
 {
-    gen_helper_popcnt(o->out, o->in2);
+    const uint8_t m3 = get_field(s, m3);
+
+    if ((m3 & 8) && s390_has_feat(S390_FEAT_MISC_INSTRUCTION_EXT3)) {
+        tcg_gen_ctpop_i64(o->out, o->in2);
+    } else {
+        gen_helper_popcnt(o->out, o->in2);
+    }
     return DISAS_NEXT;
 }
 
@@ -5668,6 +5716,13 @@ static void in1_r3_D32(DisasContext *s, DisasOps *o)
 }
 #define SPEC_in1_r3_D32 SPEC_r3_even
 
+static void in1_r3_sr32(DisasContext *s, DisasOps *o)
+{
+    o->in1 = tcg_temp_new_i64();
+    tcg_gen_shri_i64(o->in1, regs[get_field(s, r3)], 32);
+}
+#define SPEC_in1_r3_sr32 0
+
 static void in1_e1(DisasContext *s, DisasOps *o)
 {
     o->in1 = load_freg32_i64(get_field(s, r1));
@@ -6170,6 +6225,7 @@ enum DisasInsnEnum {
 #define FAC_V           S390_FEAT_VECTOR /* vector facility */
 #define FAC_VE          S390_FEAT_VECTOR_ENH /* vector enhancements facility 1 */
 #define FAC_MIE2        S390_FEAT_MISC_INSTRUCTION_EXT2 /* miscellaneous-instruction-extensions facility 2 */
+#define FAC_MIE3        S390_FEAT_MISC_INSTRUCTION_EXT3 /* miscellaneous-instruction-extensions facility 3 */
 
 static const DisasInsn insn_info[] = {
 #include "insn-data.def"
-- 
2.32.0

[PATCH v7 2/4] s390x/cpumodel: Bump up QEMU model to a stripped-down IBM z15 GA1

[David Miller]

TCG implements everything we need to run basic z15 OS+software

Signed-off-by: David Miller <dmiller423@gmail.com>
---
 hw/s390x/s390-virtio-ccw.c  | 3 +++
 target/s390x/cpu_models.c   | 6 +++---
 target/s390x/gen-features.c | 7 +++++--
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 84e3e63c43..90480e7cf9 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -802,7 +802,10 @@ DEFINE_CCW_MACHINE(7_0, "7.0", true);
 
 static void ccw_machine_6_2_instance_options(MachineState *machine)
 {
+    static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V6_2 };
+
     ccw_machine_7_0_instance_options(machine);
+    s390_set_qemu_cpu_model(0x3906, 14, 2, qemu_cpu_feat);
 }
 
 static void ccw_machine_6_2_class_options(MachineClass *mc)
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 11e06cc51f..89f83e81d5 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -85,9 +85,9 @@ static S390CPUDef s390_cpu_defs[] = {
     CPUDEF_INIT(0x3932, 16, 1, 47, 0x08000000U, "gen16b", "IBM 3932 GA1"),
 };
 
-#define QEMU_MAX_CPU_TYPE 0x3906
-#define QEMU_MAX_CPU_GEN 14
-#define QEMU_MAX_CPU_EC_GA 2
+#define QEMU_MAX_CPU_TYPE 0x8561
+#define QEMU_MAX_CPU_GEN 15
+#define QEMU_MAX_CPU_EC_GA 1
 static const S390FeatInit qemu_max_cpu_feat_init = { S390_FEAT_LIST_QEMU_MAX };
 static S390FeatBitmap qemu_max_cpu_feat;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index a3f30f69d9..22846121c4 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -731,16 +731,18 @@ static uint16_t qemu_V6_0[] = {
     S390_FEAT_ESOP,
 };
 
-static uint16_t qemu_LATEST[] = {
+static uint16_t qemu_V6_2[] = {
     S390_FEAT_INSTRUCTION_EXEC_PROT,
     S390_FEAT_MISC_INSTRUCTION_EXT2,
     S390_FEAT_MSA_EXT_8,
     S390_FEAT_VECTOR_ENH,
 };
 
+static uint16_t qemu_LATEST[] = {
+    S390_FEAT_MISC_INSTRUCTION_EXT3,
+};
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
-    S390_FEAT_MISC_INSTRUCTION_EXT3,
     /* generates a dependency warning, leave it out for now */
     S390_FEAT_MSA_EXT_5,
 };
@@ -863,6 +865,7 @@ static FeatGroupDefSpec QemuFeatDef[] = {
     QEMU_FEAT_INITIALIZER(V4_0),
     QEMU_FEAT_INITIALIZER(V4_1),
     QEMU_FEAT_INITIALIZER(V6_0),
+    QEMU_FEAT_INITIALIZER(V6_2),
     QEMU_FEAT_INITIALIZER(LATEST),
     QEMU_FEAT_INITIALIZER(MAX),
 };
-- 
2.32.0

[PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[David Miller]

tests/tcg/s390x/mie3-compl.c: [N]*K instructions
tests/tcg/s390x/mie3-mvcrl.c: MVCRL instruction
tests/tcg/s390x/mie3-sel.c:  SELECT instruction

Signed-off-by: David Miller <dmiller423@gmail.com>
---
 tests/tcg/s390x/Makefile.target |  5 ++-
 tests/tcg/s390x/mie3-compl.c    | 55 +++++++++++++++++++++++++++++++++
 tests/tcg/s390x/mie3-mvcrl.c    | 31 +++++++++++++++++++
 tests/tcg/s390x/mie3-sel.c      | 42 +++++++++++++++++++++++++
 4 files changed, 132 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/s390x/mie3-compl.c
 create mode 100644 tests/tcg/s390x/mie3-mvcrl.c
 create mode 100644 tests/tcg/s390x/mie3-sel.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 1a7238b4eb..54e67446aa 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -1,12 +1,15 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
-CFLAGS+=-march=zEC12 -m64
+CFLAGS+=-march=z15 -m64
 TESTS+=hello-s390x
 TESTS+=csst
 TESTS+=ipm
 TESTS+=exrl-trt
 TESTS+=exrl-trtr
 TESTS+=pack
+TESTS+=mie3-compl
+TESTS+=mie3-mvcrl
+TESTS+=mie3-sel
 TESTS+=mvo
 TESTS+=mvc
 TESTS+=shift
diff --git a/tests/tcg/s390x/mie3-compl.c b/tests/tcg/s390x/mie3-compl.c
new file mode 100644
index 0000000000..98281ee683
--- /dev/null
+++ b/tests/tcg/s390x/mie3-compl.c
@@ -0,0 +1,55 @@
+#include <stdint.h>
+
+
+#define F_EPI "stg %%r0, %[res] " : [res] "+m" (res) : : "r0", "r2", "r3"
+
+#define F_PRO    asm ( \
+    "llihf %%r0,801\n" \
+    "lg %%r2, %[a]\n"  \
+    "lg %%r3, %[b] "   \
+    : : [a] "m" (a),   \
+        [b] "m" (b)    \
+    : "r2", "r3")
+
+#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
+{ uint64_t res = 0; F_PRO; ASM; return res; }
+
+/* AND WITH COMPLEMENT */
+FbinOp(_ncrk,  asm("ncrk  %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))
+
+/* NAND */
+FbinOp(_nnrk,  asm("nnrk  %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nngrk, asm("nngrk %%r0, %%r3, %%r2\n" F_EPI))
+
+/* NOT XOR */
+FbinOp(_nxrk,  asm("nxrk  %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nxgrk, asm("nxgrk %%r0, %%r3, %%r2\n" F_EPI))
+
+/* NOR */
+FbinOp(_nork,  asm("nork  %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nogrk, asm("nogrk %%r0, %%r3, %%r2\n" F_EPI))
+
+/* OR WITH COMPLEMENT */
+FbinOp(_ocrk,  asm("ocrk  %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_ocgrk, asm("ocgrk %%r0, %%r3, %%r2\n" F_EPI))
+
+
+int main(int argc, char *argv[])
+{
+    if (_ncrk(0xFF88, 0xAA11)  != 0x0000032100000011ull ||
+        _nnrk(0xFF88, 0xAA11)  != 0x00000321FFFF55FFull ||
+        _nork(0xFF88, 0xAA11)  != 0x00000321FFFF0066ull ||
+        _nxrk(0xFF88, 0xAA11)  != 0x00000321FFFFAA66ull ||
+        _ocrk(0xFF88, 0xAA11)  != 0x00000321FFFFAA77ull ||
+        _ncgrk(0xFF88, 0xAA11) != 0x0000000000000011ull ||
+        _nngrk(0xFF88, 0xAA11) != 0xFFFFFFFFFFFF55FFull ||
+        _nogrk(0xFF88, 0xAA11) != 0xFFFFFFFFFFFF0066ull ||
+        _nxgrk(0xFF88, 0xAA11) != 0xFFFFFFFFFFFFAA66ull ||
+        _ocgrk(0xFF88, 0xAA11) != 0xFFFFFFFFFFFFAA77ull)
+    {
+        return 1;
+    }
+
+    return 0;
+}
diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
new file mode 100644
index 0000000000..81cf3ad702
--- /dev/null
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -0,0 +1,31 @@
+#include <stdint.h>
+#include <string.h>
+
+
+static inline void mvcrl_8(const char *dst, const char *src)
+{
+    asm volatile (
+    "llill %%r0, 8\n"
+    "mvcrl 0(%[dst]), 0(%[src])\n"
+    : : [dst] "d" (dst), [src] "d" (src)
+    : "memory");
+}
+
+
+int main(int argc, char *argv[])
+{
+    const char *alpha = "abcdefghijklmnop";
+
+    /* array missing 'i' */
+    char tstr[17] = "abcdefghjklmnop\0" ;
+
+    /* mvcrl reference use: 'open a hole in an array' */
+    mvcrl_8(tstr + 9, tstr + 8);
+
+    /* place missing 'i' */
+    tstr[8] = 'i';
+
+    return strncmp(alpha, tstr, 16ul);
+}
+
+
diff --git a/tests/tcg/s390x/mie3-sel.c b/tests/tcg/s390x/mie3-sel.c
new file mode 100644
index 0000000000..2e99e00b47
--- /dev/null
+++ b/tests/tcg/s390x/mie3-sel.c
@@ -0,0 +1,42 @@
+#include <stdint.h>
+
+
+#define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
+{                       \
+uint64_t res = 0;       \
+asm (                   \
+    "lg %%r2, %[a]\n"   \
+    "lg %%r3, %[b]\n"   \
+    "lg %%r0, %[c]\n"   \
+    "ltgr %%r0, %%r0\n" \
+    ASM                 \
+    "stg %%r0, %[res] " \
+    : [res] "=m" (res)  \
+    : [a] "m" (a),      \
+      [b] "m" (b),      \
+      [c] "m" (c)       \
+    : "r0", "r2",       \
+      "r3", "r4"        \
+    );                  \
+    return res;         \
+}
+
+
+Fi3 (_selre,    "selre    %%r0, %%r3, %%r2\n")
+Fi3 (_selgrz,   "selgrz   %%r0, %%r3, %%r2\n")
+Fi3 (_selfhrnz, "selfhrnz %%r0, %%r3, %%r2\n")
+
+
+int main(int argc, char *argv[])
+{
+    uint64_t a = ~0, b = ~0, c = ~0;
+    a =    _selre(0x066600000066ull, 0x066600000006ull, a);
+    b =   _selgrz(0xF00D00000005ull, 0xF00D00000055ull, b);
+    c = _selfhrnz(0x043200000044ull, 0x065400000004ull, c);
+
+    return (int) (
+        (0xFFFFFFFF00000066ull != a) ||
+        (0x0000F00D00000005ull != b) ||
+        (0x00000654FFFFFFFFull != c) );
+}
+
-- 
2.32.0

[PATCH v7 4/4] tests/tcg/s390x: changed to using .insn for tests requiring z15

[David Miller]

Signed-off-by: David Miller <dmiller423@gmail.com>
---
 tests/tcg/s390x/mie3-compl.c | 21 +++++++++++----------
 tests/tcg/s390x/mie3-mvcrl.c |  2 +-
 tests/tcg/s390x/mie3-sel.c   |  6 +++---
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/tests/tcg/s390x/mie3-compl.c b/tests/tcg/s390x/mie3-compl.c
index 98281ee683..31820e4a2a 100644
--- a/tests/tcg/s390x/mie3-compl.c
+++ b/tests/tcg/s390x/mie3-compl.c
@@ -14,25 +14,26 @@
 #define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
 { uint64_t res = 0; F_PRO; ASM; return res; }
 
+
 /* AND WITH COMPLEMENT */
-FbinOp(_ncrk,  asm("ncrk  %%r0, %%r3, %%r2\n" F_EPI))
-FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_ncrk,  asm(".insn rrf, 0xB9F50000, %%r0, %%r3, %%r2, 0\n" F_EPI))
+FbinOp(_ncgrk, asm(".insn rrf, 0xB9E50000, %%r0, %%r3, %%r2, 0\n" F_EPI))
 
 /* NAND */
-FbinOp(_nnrk,  asm("nnrk  %%r0, %%r3, %%r2\n" F_EPI))
-FbinOp(_nngrk, asm("nngrk %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nnrk,  asm(".insn rrf, 0xB9740000, %%r0, %%r3, %%r2, 0\n" F_EPI))
+FbinOp(_nngrk, asm(".insn rrf, 0xB9640000, %%r0, %%r3, %%r2, 0\n" F_EPI))
 
 /* NOT XOR */
-FbinOp(_nxrk,  asm("nxrk  %%r0, %%r3, %%r2\n" F_EPI))
-FbinOp(_nxgrk, asm("nxgrk %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nxrk,  asm(".insn rrf, 0xB9770000, %%r0, %%r3, %%r2, 0\n" F_EPI))
+FbinOp(_nxgrk, asm(".insn rrf, 0xB9670000, %%r0, %%r3, %%r2, 0\n" F_EPI))
 
 /* NOR */
-FbinOp(_nork,  asm("nork  %%r0, %%r3, %%r2\n" F_EPI))
-FbinOp(_nogrk, asm("nogrk %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_nork,  asm(".insn rrf, 0xB9760000, %%r0, %%r3, %%r2, 0\n" F_EPI))
+FbinOp(_nogrk, asm(".insn rrf, 0xB9660000, %%r0, %%r3, %%r2, 0\n" F_EPI))
 
 /* OR WITH COMPLEMENT */
-FbinOp(_ocrk,  asm("ocrk  %%r0, %%r3, %%r2\n" F_EPI))
-FbinOp(_ocgrk, asm("ocgrk %%r0, %%r3, %%r2\n" F_EPI))
+FbinOp(_ocrk,  asm(".insn rrf, 0xB9750000, %%r0, %%r3, %%r2, 0\n" F_EPI))
+FbinOp(_ocgrk, asm(".insn rrf, 0xB9650000, %%r0, %%r3, %%r2, 0\n" F_EPI))
 
 
 int main(int argc, char *argv[])
diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
index 81cf3ad702..f0be83b197 100644
--- a/tests/tcg/s390x/mie3-mvcrl.c
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -6,7 +6,7 @@ static inline void mvcrl_8(const char *dst, const char *src)
 {
     asm volatile (
     "llill %%r0, 8\n"
-    "mvcrl 0(%[dst]), 0(%[src])\n"
+    ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
     : : [dst] "d" (dst), [src] "d" (src)
     : "memory");
 }
diff --git a/tests/tcg/s390x/mie3-sel.c b/tests/tcg/s390x/mie3-sel.c
index 2e99e00b47..ee619a763d 100644
--- a/tests/tcg/s390x/mie3-sel.c
+++ b/tests/tcg/s390x/mie3-sel.c
@@ -22,9 +22,9 @@ asm (                   \
 }
 
 
-Fi3 (_selre,    "selre    %%r0, %%r3, %%r2\n")
-Fi3 (_selgrz,   "selgrz   %%r0, %%r3, %%r2\n")
-Fi3 (_selfhrnz, "selfhrnz %%r0, %%r3, %%r2\n")
+Fi3 (_selre,     ".insn rrf, 0xB9F00000, %%r0, %%r3, %%r2, 8\n")
+Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %%r0, %%r3, %%r2, 8\n")
+Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %%r0, %%r3, %%r2, 7\n")
 
 
 int main(int argc, char *argv[])
-- 
2.32.0

Re: [PATCH v7 1/4] s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x

[Richard Henderson]

On 2/23/22 12:31, David Miller wrote:
> resolves:https://gitlab.com/qemu-project/qemu/-/issues/737
> implements:
> AND WITH COMPLEMENT   (NCRK, NCGRK)
> NAND                  (NNRK, NNGRK)
> NOT EXCLUSIVE OR      (NXRK, NXGRK)
> NOR                   (NORK, NOGRK)
> OR WITH COMPLEMENT    (OCRK, OCGRK)
> SELECT                (SELR, SELGR)
> SELECT HIGH           (SELFHR)
> MOVE RIGHT TO LEFT    (MVCRL)
> POPULATION COUNT      (POPCNT)
> 
> Signed-off-by: David Miller<dmiller423@gmail.com>
> ---
>   target/s390x/gen-features.c    |  1 +
>   target/s390x/helper.h          |  1 +
>   target/s390x/tcg/insn-data.def | 30 +++++++++++++++--
>   target/s390x/tcg/mem_helper.c  | 20 ++++++++++++
>   target/s390x/tcg/translate.c   | 60 ++++++++++++++++++++++++++++++++--
>   5 files changed, 107 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[Richard Henderson]

On 2/23/22 12:31, David Miller wrote:
> +#define F_EPI "stg %%r0, %[res] " : [res] "+m" (res) : : "r0", "r2", "r3"
> +
> +#define F_PRO    asm ( \
> +    "llihf %%r0,801\n" \
> +    "lg %%r2, %[a]\n"  \
> +    "lg %%r3, %[b] "   \
> +    : : [a] "m" (a),   \
> +        [b] "m" (b)    \
> +    : "r2", "r3")
> +
> +#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
> +{ uint64_t res = 0; F_PRO; ASM; return res; }
> +
> +/* AND WITH COMPLEMENT */
> +FbinOp(_ncrk,  asm("ncrk  %%r0, %%r3, %%r2\n" F_EPI))
> +FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))

Better written as

   asm("ncrk %0, %3, %2" : "=&r"(res) : "r"(a), "r"(b) : "cc");

and drop F_PRO and F_EPI.  Use the asm constraints properly to place the operands.

> +/* NAND */
> +FbinOp(_nnrk,  asm("nnrk  %%r0, %%r3, %%r2\n" F_EPI))
> +FbinOp(_nngrk, asm("nngrk %%r0, %%r3, %%r2\n" F_EPI))
> +
> +/* NOT XOR */
> +FbinOp(_nxrk,  asm("nxrk  %%r0, %%r3, %%r2\n" F_EPI))
> +FbinOp(_nxgrk, asm("nxgrk %%r0, %%r3, %%r2\n" F_EPI))
> +
> +/* NOR */
> +FbinOp(_nork,  asm("nork  %%r0, %%r3, %%r2\n" F_EPI))
> +FbinOp(_nogrk, asm("nogrk %%r0, %%r3, %%r2\n" F_EPI))
> +
> +/* OR WITH COMPLEMENT */
> +FbinOp(_ocrk,  asm("ocrk  %%r0, %%r3, %%r2\n" F_EPI))
> +FbinOp(_ocgrk, asm("ocgrk %%r0, %%r3, %%r2\n" F_EPI))

Similarly.

> +++ b/tests/tcg/s390x/mie3-sel.c
> @@ -0,0 +1,42 @@
> +#include <stdint.h>
> +
> +
> +#define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
> +{                       \
> +uint64_t res = 0;       \
> +asm (                   \
> +    "lg %%r2, %[a]\n"   \
> +    "lg %%r3, %[b]\n"   \
> +    "lg %%r0, %[c]\n"   \
> +    "ltgr %%r0, %%r0\n" \
> +    ASM                 \
> +    "stg %%r0, %[res] " \
> +    : [res] "=m" (res)  \
> +    : [a] "m" (a),      \
> +      [b] "m" (b),      \
> +      [c] "m" (c)       \
> +    : "r0", "r2",       \
> +      "r3", "r4"        \
> +    );                  \
> +    return res;         \
> +}
> +
> +
> +Fi3 (_selre,    "selre    %%r0, %%r3, %%r2\n")
> +Fi3 (_selgrz,   "selgrz   %%r0, %%r3, %%r2\n")
> +Fi3 (_selfhrnz, "selfhrnz %%r0, %%r3, %%r2\n")

Similarly:

   asm("ltgr %3, %3; selre %0, %2, %1"
       : "=&r"(res) : "r"(a), "r"(b), "r"(c) : "cc");

Although none of this is going to work with .insn.  We *ought* to be able to use the 
debian11 update plus a change to tests/tcg/configure.sh to detect host support for 
-march=z15 to drop that change.


r~

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[Richard Henderson]

On 2/23/22 13:43, Richard Henderson wrote:
> Although none of this is going to work with .insn...

I beg your pardon, this is incorrect: .insn does have fields for the register arguments.


r~

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[Thomas Huth]

On 24/02/2022 00.43, Richard Henderson wrote:
> On 2/23/22 12:31, David Miller wrote:
>> +#define F_EPI "stg %%r0, %[res] " : [res] "+m" (res) : : "r0", "r2", "r3"
>> +
>> +#define F_PRO    asm ( \
>> +    "llihf %%r0,801\n" \
>> +    "lg %%r2, %[a]\n"  \
>> +    "lg %%r3, %[b] "   \
>> +    : : [a] "m" (a),   \
>> +        [b] "m" (b)    \
>> +    : "r2", "r3")
>> +
>> +#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
>> +{ uint64_t res = 0; F_PRO; ASM; return res; }
>> +
>> +/* AND WITH COMPLEMENT */
>> +FbinOp(_ncrk,  asm("ncrk  %%r0, %%r3, %%r2\n" F_EPI))
>> +FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))
> 
> Better written as
> 
>    asm("ncrk %0, %3, %2" : "=&r"(res) : "r"(a), "r"(b) : "cc");

I agree with Richard, especially since it's kind of "dangerous" to chain 
multiple asm() statements (without "volatile") and hoping that the compiler 
keeps the values in the registers in between (without reordering the 
statements).

Anyway, since I'll be away for most the rest of the week and we already have 
soft-freeze next week, I'd like to get this fixed for my pull request that I 
plan later for today or tomorrow, so I now went ahead and modified the code 
to look like this:

#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
{ \
     uint64_t res = 0; \
     asm ("llihf %[res],801\n" ASM \
          : [res]"=&r"(res) : [a]"r"(a), [b]"r"(b) : "cc"); \
     return res; \
}

/* AND WITH COMPLEMENT */
FbinOp(_ncrk,  ".insn rrf, 0xB9F50000, %[res], %[b], %[a], 0\n")
FbinOp(_ncgrk, ".insn rrf, 0xB9E50000, %[res], %[b], %[a], 0\n")

/* NAND */
FbinOp(_nnrk,  ".insn rrf, 0xB9740000, %[res], %[b], %[a], 0\n")
FbinOp(_nngrk, ".insn rrf, 0xB9640000, %[res], %[b], %[a], 0\n")

/* NOT XOR */
FbinOp(_nxrk,  ".insn rrf, 0xB9770000, %[res], %[b], %[a], 0\n")
FbinOp(_nxgrk, ".insn rrf, 0xB9670000, %[res], %[b], %[a], 0\n")

/* NOR */
FbinOp(_nork,  ".insn rrf, 0xB9760000, %[res], %[b], %[a], 0\n")
FbinOp(_nogrk, ".insn rrf, 0xB9660000, %[res], %[b], %[a], 0\n")

/* OR WITH COMPLEMENT */
FbinOp(_ocrk,  ".insn rrf, 0xB9750000, %[res], %[b], %[a], 0\n")
FbinOp(_ocgrk, ".insn rrf, 0xB9650000, %[res], %[b], %[a], 0\n")

Full patch can be seen here:

https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473

I hope that's ok for everybody?

  Thomas

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[David Hildenbrand]

On 28.02.22 11:14, Thomas Huth wrote:
> On 24/02/2022 00.43, Richard Henderson wrote:
>> On 2/23/22 12:31, David Miller wrote:
>>> +#define F_EPI "stg %%r0, %[res] " : [res] "+m" (res) : : "r0", "r2", "r3"
>>> +
>>> +#define F_PRO    asm ( \
>>> +    "llihf %%r0,801\n" \
>>> +    "lg %%r2, %[a]\n"  \
>>> +    "lg %%r3, %[b] "   \
>>> +    : : [a] "m" (a),   \
>>> +        [b] "m" (b)    \
>>> +    : "r2", "r3")
>>> +
>>> +#define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
>>> +{ uint64_t res = 0; F_PRO; ASM; return res; }
>>> +
>>> +/* AND WITH COMPLEMENT */
>>> +FbinOp(_ncrk,  asm("ncrk  %%r0, %%r3, %%r2\n" F_EPI))
>>> +FbinOp(_ncgrk, asm("ncgrk %%r0, %%r3, %%r2\n" F_EPI))
>>
>> Better written as
>>
>>    asm("ncrk %0, %3, %2" : "=&r"(res) : "r"(a), "r"(b) : "cc");
> 
> I agree with Richard, especially since it's kind of "dangerous" to chain 
> multiple asm() statements (without "volatile") and hoping that the compiler 
> keeps the values in the registers in between (without reordering the 
> statements).
> 
> Anyway, since I'll be away for most the rest of the week and we already have 
> soft-freeze next week, I'd like to get this fixed for my pull request that I 
> plan later for today or tomorrow, so I now went ahead and modified the code 
> to look like this:
> 
> #define FbinOp(S, ASM) uint64_t S(uint64_t a, uint64_t b) \
> { \
>      uint64_t res = 0; \
>      asm ("llihf %[res],801\n" ASM \
>           : [res]"=&r"(res) : [a]"r"(a), [b]"r"(b) : "cc"); \
>      return res; \
> }
> 
> /* AND WITH COMPLEMENT */
> FbinOp(_ncrk,  ".insn rrf, 0xB9F50000, %[res], %[b], %[a], 0\n")
> FbinOp(_ncgrk, ".insn rrf, 0xB9E50000, %[res], %[b], %[a], 0\n")
> 
> /* NAND */
> FbinOp(_nnrk,  ".insn rrf, 0xB9740000, %[res], %[b], %[a], 0\n")
> FbinOp(_nngrk, ".insn rrf, 0xB9640000, %[res], %[b], %[a], 0\n")
> 
> /* NOT XOR */
> FbinOp(_nxrk,  ".insn rrf, 0xB9770000, %[res], %[b], %[a], 0\n")
> FbinOp(_nxgrk, ".insn rrf, 0xB9670000, %[res], %[b], %[a], 0\n")
> 
> /* NOR */
> FbinOp(_nork,  ".insn rrf, 0xB9760000, %[res], %[b], %[a], 0\n")
> FbinOp(_nogrk, ".insn rrf, 0xB9660000, %[res], %[b], %[a], 0\n")
> 
> /* OR WITH COMPLEMENT */
> FbinOp(_ocrk,  ".insn rrf, 0xB9750000, %[res], %[b], %[a], 0\n")
> FbinOp(_ocgrk, ".insn rrf, 0xB9650000, %[res], %[b], %[a], 0\n")
> 
> Full patch can be seen here:
> 
> https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473
> 
> I hope that's ok for everybody?

Fine with me.


-- 
Thanks,

David / dhildenb

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[Richard Henderson]

On 2/28/22 00:14, Thomas Huth wrote:
> Full patch can be seen here:
> 
> https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473


> static inline void mvcrl_8(const char *dst, const char *src)
> {
>     asm volatile (
>     "llill %%r0, 8\n"
>     ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
>     : : [dst] "d" (dst), [src] "d" (src)
>     : "memory");
> }

Need clobber of r0 here.

> #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
> {                            \
>     uint64_t res = 0;        \
>     asm (                    \
>          "lg %%r2, %[a]\n"   \
>          "lg %%r3, %[b]\n"   \
>          "lg %%r0, %[c]\n"   \
>          "ltgr %%r0, %%r0\n" \
>          ASM                 \
>          "stg %%r0, %[res] " \
>          : [res] "=m" (res)  \
>          : [a] "m" (a),      \
>            [b] "m" (b),      \
>            [c] "m" (c)       \
>          : "r0", "r2",       \
>            "r3", "r4"        \
>     );                       \
>     return res;              \
> }
> 
> Fi3 (_selre,     ".insn rrf, 0xB9F00000, %%r0, %%r3, %%r2, 8\n")
> Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %%r0, %%r3, %%r2, 8\n")
> Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %%r0, %%r3, %%r2, 7\n")

This isn't actively broken, but could use the same treatment as NCRK et al:

#define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
{                            \
     uint64_t res;            \
     asm("ltgr %[c], %[c]\n\t" ASM
         : [res] "=&r" (res)
         : [a] "r" (a), [b] "r" (b), [c] "r" (c)
         : "cc");
     return res;
}

Fi3(_selre,   ".insn rrf, 0xB9F00000, %[res], %[a], %[b], 8")

etc.


r~

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[David Miller]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

Had it on my TODO list for this morning, thank you.

On Mon, Feb 28, 2022 at 12:59 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/28/22 00:14, Thomas Huth wrote:
> > Full patch can be seen here:
> >
> > https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473
>
>
> > static inline void mvcrl_8(const char *dst, const char *src)
> > {
> >     asm volatile (
> >     "llill %%r0, 8\n"
> >     ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
> >     : : [dst] "d" (dst), [src] "d" (src)
> >     : "memory");
> > }
>
> Need clobber of r0 here.
>
> > #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
> > {                            \
> >     uint64_t res = 0;        \
> >     asm (                    \
> >          "lg %%r2, %[a]\n"   \
> >          "lg %%r3, %[b]\n"   \
> >          "lg %%r0, %[c]\n"   \
> >          "ltgr %%r0, %%r0\n" \
> >          ASM                 \
> >          "stg %%r0, %[res] " \
> >          : [res] "=m" (res)  \
> >          : [a] "m" (a),      \
> >            [b] "m" (b),      \
> >            [c] "m" (c)       \
> >          : "r0", "r2",       \
> >            "r3", "r4"        \
> >     );                       \
> >     return res;              \
> > }
> >
> > Fi3 (_selre,     ".insn rrf, 0xB9F00000, %%r0, %%r3, %%r2, 8\n")
> > Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %%r0, %%r3, %%r2, 8\n")
> > Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %%r0, %%r3, %%r2, 7\n")
>
> This isn't actively broken, but could use the same treatment as NCRK et al:
>
> #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
> {                            \
>      uint64_t res;            \
>      asm("ltgr %[c], %[c]\n\t" ASM
>          : [res] "=&r" (res)
>          : [a] "r" (a), [b] "r" (b), [c] "r" (c)
>          : "cc");
>      return res;
> }
>
> Fi3(_selre,   ".insn rrf, 0xB9F00000, %[res], %[a], %[b], 8")
>
> etc.
>
>
> r~
>

[-- Attachment #2: Type: text/html, Size: 3209 bytes --]

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[Thomas Huth]

On 28/02/2022 19.31, David Miller wrote:
> Had it on my TODO list for this morning, thank you.

Thanks! Please send it as additional patch on top of my s390x-next, since I 
already sent a pull request for the other patches yesterday:

  https://gitlab.com/thuth/qemu/-/commits/s390x-next/

> On Mon, Feb 28, 2022 at 12:59 PM Richard Henderson 
> <richard.henderson@linaro.org <mailto:richard.henderson@linaro.org>> wrote:
> 
>     On 2/28/22 00:14, Thomas Huth wrote:
>      > Full patch can be seen here:
>      >
>      > https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473
>     <https://gitlab.com/thuth/qemu/-/commit/38af118ea2fef0c473>
> 
> 
>      > static inline void mvcrl_8(const char *dst, const char *src)
>      > {
>      >     asm volatile (
>      >     "llill %%r0, 8\n"
>      >     ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
>      >     : : [dst] "d" (dst), [src] "d" (src)
>      >     : "memory");
>      > }
> 
>     Need clobber of r0 here.

Right. This test fails with Clang, indeed, as I discovered today, since 
Clang uses r0 more often than GCC, as it seems. I've already sent some 
patches for some other tests today, so there'll be another s390x pull 
request next week for TCG tests fixups :-)

  Thomas


>      > #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
>      > {                            \
>      >     uint64_t res = 0;        \
>      >     asm (                    \
>      >          "lg %%r2, %[a]\n"   \
>      >          "lg %%r3, %[b]\n"   \
>      >          "lg %%r0, %[c]\n"   \
>      >          "ltgr %%r0, %%r0\n" \
>      >          ASM                 \
>      >          "stg %%r0, %[res] " \
>      >          : [res] "=m" (res)  \
>      >          : [a] "m" (a),      \
>      >            [b] "m" (b),      \
>      >            [c] "m" (c)       \
>      >          : "r0", "r2",       \
>      >            "r3", "r4"        \
>      >     );                       \
>      >     return res;              \
>      > }
>      >
>      > Fi3 (_selre,     ".insn rrf, 0xB9F00000, %%r0, %%r3, %%r2, 8\n")
>      > Fi3 (_selgrz,    ".insn rrf, 0xB9E30000, %%r0, %%r3, %%r2, 8\n")
>      > Fi3 (_selfhrnz,  ".insn rrf, 0xB9C00000, %%r0, %%r3, %%r2, 7\n")
> 
>     This isn't actively broken, but could use the same treatment as NCRK et al:
> 
>     #define Fi3(S, ASM) uint64_t S(uint64_t a, uint64_t b, uint64_t c) \
>     {                            \
>           uint64_t res;            \
>           asm("ltgr %[c], %[c]\n\t" ASM
>               : [res] "=&r" (res)
>               : [a] "r" (a), [b] "r" (b), [c] "r" (c)
>               : "cc");
>           return res;
>     }
> 
>     Fi3(_selre,   ".insn rrf, 0xB9F00000, %[res], %[a], %[b], 8")
> 
>     etc.
> 
> 
>     r~
> 

Re: [PATCH v7 3/4] tests/tcg/s390x: Tests for Miscellaneous-Instruction-Extensions Facility 3

[David Miller]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

>On 28/02/2022 19.31, David Miller wrote:
> > Had it on my TODO list for this morning, thank you.

> Thanks! Please send it as additional patch on top of my s390x-next, since
I
> already sent a pull request for the other patches yesterday:
>
>  https://gitlab.com/thuth/qemu/-/commits/s390x-next/

Partial misread yesterday,  I was on mobile and saw that you had modified
the patch to stage.

I will look at it now,  as soon as gitlab comes back up,  it's been
problematic lately.


- David Miller

[-- Attachment #2: Type: text/html, Size: 2147 bytes --]