[Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[Pavel Zbitskiy]

Found while attempting to run an old tool in qemu.

* BAL and BALR:    Added.
* CSST:            Qemu crashed after an accidental jump to garbage.
* IPM:             A tool produced an incorrect output.
* EX TRT/TRTR:     A tool ran quite slow.
* PACK:            A tool produced an incorrect output.
* CVB, CVBY, CVBG: Added.

Changes since v1:
* Tests.
* Call pc_to_link_info() instead of op_bas().
* Clarified CSST commit message.
* Rewrote IPM using extract/deposit.
* Clarified PACK commit message.
* Do not use LowCore for CONFIG_USER_ONLY.
* Reduce duplication in CVB code.

Pavel Zbitskiy (7):
  tests/tcg: add a simple s390x test
  target/s390x: add BAL and BALR instructions
  target/s390x: fix CSST decoding and runtime alignment check
  target/s390x: fix IPM polluting irrelevant bits
  target/s390x: add EX support for TRT and TRTR
  target/s390x: fix PACK reading 1 byte less and writing 1 byte more
  target/s390x: implement CVB, CVBY and CVBG

 target/s390x/helper.h           |  1 +
 target/s390x/insn-data.def      |  7 ++++
 target/s390x/int_helper.c       | 50 ++++++++++++++++++++++++++
 target/s390x/mem_helper.c       | 24 ++++++++++---
 target/s390x/translate.c        | 64 ++++++++++++++++++++++++++-------
 tests/tcg/s390x/Makefile.target |  9 +++++
 tests/tcg/s390x/csst.c          | 43 ++++++++++++++++++++++
 tests/tcg/s390x/cvb.c           | 18 ++++++++++
 tests/tcg/s390x/exrl-trt.c      | 48 +++++++++++++++++++++++++
 tests/tcg/s390x/exrl-trtr.c     | 48 +++++++++++++++++++++++++
 tests/tcg/s390x/hello-s390x.c   |  7 ++++
 tests/tcg/s390x/ipm.c           | 22 ++++++++++++
 tests/tcg/s390x/pack.c          | 21 +++++++++++
 13 files changed, 346 insertions(+), 16 deletions(-)
 create mode 100644 tests/tcg/s390x/Makefile.target
 create mode 100644 tests/tcg/s390x/csst.c
 create mode 100644 tests/tcg/s390x/cvb.c
 create mode 100644 tests/tcg/s390x/exrl-trt.c
 create mode 100644 tests/tcg/s390x/exrl-trtr.c
 create mode 100644 tests/tcg/s390x/hello-s390x.c
 create mode 100644 tests/tcg/s390x/ipm.c
 create mode 100644 tests/tcg/s390x/pack.c

-- 
2.18.0

[Qemu-devel] [PATCH 1/7] tests/tcg: add a simple s390x test

[Pavel Zbitskiy]

Copied from alpha.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 tests/tcg/s390x/Makefile.target | 3 +++
 tests/tcg/s390x/hello-s390x.c   | 7 +++++++
 2 files changed, 10 insertions(+)
 create mode 100644 tests/tcg/s390x/Makefile.target
 create mode 100644 tests/tcg/s390x/hello-s390x.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
new file mode 100644
index 0000000000..9f4076901f
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.target
@@ -0,0 +1,3 @@
+VPATH+=$(SRC_PATH)/tests/tcg/s390x
+CFLAGS+=-march=zEC12 -m64
+TESTS+=hello-s390x
diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
new file mode 100644
index 0000000000..3dc0a05f2b
--- /dev/null
+++ b/tests/tcg/s390x/hello-s390x.c
@@ -0,0 +1,7 @@
+#include <unistd.h>
+
+int main(void)
+{
+    write(1, "hello\n", 6);
+    return 0;
+}
-- 
2.18.0

[Qemu-devel] [PATCH 2/7] target/s390x: add BAL and BALR instructions

[Pavel Zbitskiy]

These instructions are provided for compatibility purposes and are
used only by old software, in the new code BAS and BASR are preferred.
The difference between the old and new instruction exists only in the
24-bit mode.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/insn-data.def |  3 +++
 target/s390x/translate.c   | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 5c6f33ed9c..9c7b434fca 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -102,6 +102,9 @@
     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)
 
+/* BRANCH AND LINK */
+    C(0x0500, BALR,    RR_a,  Z,   0, r2_nz, r1, 0, bal, 0)
+    C(0x4500, BAL,     RX_a,  Z,   0, a2, r1, 0, bal, 0)
 /* BRANCH AND SAVE */
     C(0x0d00, BASR,    RR_a,  Z,   0, r2_nz, r1, 0, bas, 0)
     C(0x4d00, BAS,     RX_a,  Z,   0, a2, r1, 0, bas, 0)
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 57c03cbf58..316ff79250 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -1463,6 +1463,39 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o)
     }
 }
 
+static void save_link_info(DisasContext *s, DisasOps *o)
+{
+    TCGv_i64 t;
+
+    if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) {
+        tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));
+        return;
+    }
+    gen_op_calc_cc(s);
+    tcg_gen_andi_i64(o->out, o->out, 0xffffffff00000000ull);
+    tcg_gen_ori_i64(o->out, o->out, ((s->ilen / 2) << 30) | s->pc_tmp);
+    t = tcg_temp_new_i64();
+    tcg_gen_shri_i64(t, psw_mask, 16);
+    tcg_gen_andi_i64(t, t, 0x0f000000);
+    tcg_gen_or_i64(o->out, o->out, t);
+    tcg_gen_extu_i32_i64(t, cc_op);
+    tcg_gen_shli_i64(t, t, 28);
+    tcg_gen_or_i64(o->out, o->out, t);
+    tcg_temp_free_i64(t);
+}
+
+static DisasJumpType op_bal(DisasContext *s, DisasOps *o)
+{
+    save_link_info(s, o);
+    if (o->in2) {
+        tcg_gen_mov_i64(psw_addr, o->in2);
+        per_branch(s, false);
+        return DISAS_PC_UPDATED;
+    } else {
+        return DISAS_NEXT;
+    }
+}
+
 static DisasJumpType op_basi(DisasContext *s, DisasOps *o)
 {
     tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));
-- 
2.18.0

[Qemu-devel] [PATCH 3/7] target/s390x: fix CSST decoding and runtime alignment check

[Pavel Zbitskiy]

CSST is defined as:

    C(0xc802, CSST,    SSF,   CASS, la1, a2, 0, 0, csst, 0)

It means that the first parameter is handled by in1_la1().
in1_la1() fills addr1 field, and not in1.

Furthermore, when extract32() is used for the alignment check, the
third parameter should specify the number of trailing bits that must
be 0. For FC these numbers are:

    FC=0 (word, 4 bytes):        2
    FC=1 (double word, 8 bytes): 3
    FC=2 (quad word, 16 bytes):  4

For SC these numbers correspond to the size:

    SC=0: 0
    SC=1: 1
    SC=2: 2
    SC=3: 3
    SC=4: 4

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/mem_helper.c       |  2 +-
 target/s390x/translate.c        |  4 +--
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/csst.c          | 43 +++++++++++++++++++++++++++++++++
 4 files changed, 47 insertions(+), 3 deletions(-)
 create mode 100644 tests/tcg/s390x/csst.c

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e21a47fb4d..c94dbf3fcb 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1442,7 +1442,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
     }
 
     /* Sanity check the alignments.  */
-    if (extract32(a1, 0, 4 << fc) || extract32(a2, 0, 1 << sc)) {
+    if (extract32(a1, 0, fc + 2) || extract32(a2, 0, sc)) {
         goto spec_exception;
     }
 
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 316ff79250..d0d2c3412f 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2051,9 +2051,9 @@ static DisasJumpType op_csst(DisasContext *s, DisasOps *o)
     TCGv_i32 t_r3 = tcg_const_i32(r3);
 
     if (tb_cflags(s->base.tb) & CF_PARALLEL) {
-        gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->in1, o->in2);
+        gen_helper_csst_parallel(cc_op, cpu_env, t_r3, o->addr1, o->in2);
     } else {
-        gen_helper_csst(cc_op, cpu_env, t_r3, o->in1, o->in2);
+        gen_helper_csst(cc_op, cpu_env, t_r3, o->addr1, o->in2);
     }
     tcg_temp_free_i32(t_r3);
 
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 9f4076901f..f62f950d8e 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -1,3 +1,4 @@
 VPATH+=$(SRC_PATH)/tests/tcg/s390x
 CFLAGS+=-march=zEC12 -m64
 TESTS+=hello-s390x
+TESTS+=csst
diff --git a/tests/tcg/s390x/csst.c b/tests/tcg/s390x/csst.c
new file mode 100644
index 0000000000..1dae9071fb
--- /dev/null
+++ b/tests/tcg/s390x/csst.c
@@ -0,0 +1,43 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    uint64_t parmlist[] = {
+        0xfedcba9876543210ull,
+        0,
+        0x7777777777777777ull,
+        0,
+    };
+    uint64_t op1 = 0x0123456789abcdefull;
+    uint64_t op2 = 0;
+    uint64_t op3 = op1;
+    uint64_t cc;
+
+    asm volatile(
+        "    lghi %%r0,%[flags]\n"
+        "    la %%r1,%[parmlist]\n"
+        "    csst %[op1],%[op2],%[op3]\n"
+        "    ipm %[cc]\n"
+        : [op1] "+m" (op1),
+          [op2] "+m" (op2),
+          [op3] "+r" (op3),
+          [cc] "=r" (cc)
+        : [flags] "K" (0x0301),
+          [parmlist] "m" (parmlist)
+        : "r0", "r1", "cc", "memory");
+    cc = (cc >> 28) & 3;
+    if (cc) {
+        write(1, "bad cc\n", 7);
+        return 1;
+    }
+    if (op1 != parmlist[0]) {
+        write(1, "bad op1\n", 8);
+        return 1;
+    }
+    if (op2 != parmlist[2]) {
+        write(1, "bad op2\n", 8);
+        return 1;
+    }
+    return 0;
+}
-- 
2.18.0

[Qemu-devel] [PATCH 4/7] target/s390x: fix IPM polluting irrelevant bits

[Pavel Zbitskiy]

Suppose psw.mask=0x0000000080000000, cc=2, r1=0 and we do "ipm 1".
This command must touch only bits 32-39, so the expected output
is r1=0x20000000. However, currently qemu yields r1=0x20008000,
because irrelevant parts of PSW leak into r1 during program mask
transfer.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/translate.c        | 17 +++++++----------
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/ipm.c           | 22 ++++++++++++++++++++++
 3 files changed, 30 insertions(+), 10 deletions(-)
 create mode 100644 tests/tcg/s390x/ipm.c

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index d0d2c3412f..6f8fbda222 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2437,20 +2437,17 @@ static DisasJumpType op_insi(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_ipm(DisasContext *s, DisasOps *o)
 {
-    TCGv_i64 t1;
+    TCGv_i64 t1, t2;
 
     gen_op_calc_cc(s);
-    tcg_gen_andi_i64(o->out, o->out, ~0xff000000ull);
-
     t1 = tcg_temp_new_i64();
-    tcg_gen_shli_i64(t1, psw_mask, 20);
-    tcg_gen_shri_i64(t1, t1, 36);
-    tcg_gen_or_i64(o->out, o->out, t1);
-
-    tcg_gen_extu_i32_i64(t1, cc_op);
-    tcg_gen_shli_i64(t1, t1, 28);
-    tcg_gen_or_i64(o->out, o->out, t1);
+    tcg_gen_extract_i64(t1, psw_mask, 40, 4);
+    t2 = tcg_temp_new_i64();
+    tcg_gen_extu_i32_i64(t2, cc_op);
+    tcg_gen_deposit_i64(t1, t1, t2, 4, 60);
+    tcg_gen_deposit_i64(o->out, o->out, t1, 24, 8);
     tcg_temp_free_i64(t1);
+    tcg_temp_free_i64(t2);
     return DISAS_NEXT;
 }
 
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index f62f950d8e..c800a582e5 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -2,3 +2,4 @@ VPATH+=$(SRC_PATH)/tests/tcg/s390x
 CFLAGS+=-march=zEC12 -m64
 TESTS+=hello-s390x
 TESTS+=csst
+TESTS+=ipm
diff --git a/tests/tcg/s390x/ipm.c b/tests/tcg/s390x/ipm.c
new file mode 100644
index 0000000000..742f3a18c5
--- /dev/null
+++ b/tests/tcg/s390x/ipm.c
@@ -0,0 +1,22 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    uint32_t op1 = 0x55555555;
+    uint32_t op2 = 0x44444444;
+    uint64_t cc = 0xffffffffffffffffull;
+
+    asm volatile(
+        "    clc 0(4,%[op1]),0(%[op2])\n"
+        "    ipm %[cc]\n"
+        : [cc] "+r" (cc)
+        : [op1] "r" (&op1),
+          [op2] "r" (&op2)
+        : "cc");
+    if (cc != 0xffffffff20ffffffull) {
+        write(1, "bad cc\n", 7);
+        return 1;
+    }
+    return 0;
+}
-- 
2.18.0

[Qemu-devel] [PATCH 5/7] target/s390x: add EX support for TRT and TRTR

[Pavel Zbitskiy]

Improves "b213c9f5: target/s390x: Implement TRTR" by introducing the
intermediate functions, which are compatible with dx_helper type.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/mem_helper.c       | 16 +++++++++++
 tests/tcg/s390x/Makefile.target |  2 ++
 tests/tcg/s390x/exrl-trt.c      | 48 +++++++++++++++++++++++++++++++++
 tests/tcg/s390x/exrl-trtr.c     | 48 +++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+)
 create mode 100644 tests/tcg/s390x/exrl-trt.c
 create mode 100644 tests/tcg/s390x/exrl-trtr.c

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index c94dbf3fcb..704d0193b5 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1299,12 +1299,26 @@ static inline uint32_t do_helper_trt(CPUS390XState *env, int len,
     return 0;
 }
 
+static uint32_t do_helper_trt_fwd(CPUS390XState *env, uint32_t len,
+                                  uint64_t array, uint64_t trans,
+                                  uintptr_t ra)
+{
+    return do_helper_trt(env, len, array, trans, 1, ra);
+}
+
 uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
                      uint64_t trans)
 {
     return do_helper_trt(env, len, array, trans, 1, GETPC());
 }
 
+static uint32_t do_helper_trt_bkwd(CPUS390XState *env, uint32_t len,
+                                   uint64_t array, uint64_t trans,
+                                   uintptr_t ra)
+{
+    return do_helper_trt(env, len, array, trans, -1, ra);
+}
+
 uint32_t HELPER(trtr)(CPUS390XState *env, uint32_t len, uint64_t array,
                       uint64_t trans)
 {
@@ -2193,12 +2207,14 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
         typedef uint32_t (*dx_helper)(CPUS390XState *, uint32_t, uint64_t,
                                       uint64_t, uintptr_t);
         static const dx_helper dx[16] = {
+            [0x0] = do_helper_trt_bkwd,
             [0x2] = do_helper_mvc,
             [0x4] = do_helper_nc,
             [0x5] = do_helper_clc,
             [0x6] = do_helper_oc,
             [0x7] = do_helper_xc,
             [0xc] = do_helper_tr,
+            [0xd] = do_helper_trt_fwd,
         };
         dx_helper helper = dx[opc & 0xf];
 
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index c800a582e5..7de4376f52 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -3,3 +3,5 @@ CFLAGS+=-march=zEC12 -m64
 TESTS+=hello-s390x
 TESTS+=csst
 TESTS+=ipm
+TESTS+=exrl-trt
+TESTS+=exrl-trtr
diff --git a/tests/tcg/s390x/exrl-trt.c b/tests/tcg/s390x/exrl-trt.c
new file mode 100644
index 0000000000..3c5323aecb
--- /dev/null
+++ b/tests/tcg/s390x/exrl-trt.c
@@ -0,0 +1,48 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    char op1[] = "hello";
+    char op2[256];
+    uint64_t r1 = 0xffffffffffffffffull;
+    uint64_t r2 = 0xffffffffffffffffull;
+    uint64_t cc;
+    int i;
+
+    for (i = 0; i < 256; i++) {
+        if (i == 0) {
+            op2[i] = 0xaa;
+        } else {
+            op2[i] = 0;
+        }
+    }
+    asm volatile(
+        "    j 2f\n"
+        "1:  trt 0(1,%[op1]),0(%[op2])\n"
+        "2:  exrl %[op1_len],1b\n"
+        "    lgr %[r1],%%r1\n"
+        "    lgr %[r2],%%r2\n"
+        "    ipm %[cc]\n"
+        : [r1] "+r" (r1),
+          [r2] "+r" (r2),
+          [cc] "=r" (cc)
+        : [op1] "r" (&op1),
+          [op1_len] "r" (5),
+          [op2] "r" (&op2)
+        : "r1", "r2", "cc");
+    cc = (cc >> 28) & 3;
+    if (cc != 2) {
+        write(1, "bad cc\n", 7);
+        return 1;
+    }
+    if ((char *)r1 != &op1[5]) {
+        write(1, "bad r1\n", 7);
+        return 1;
+    }
+    if (r2 != 0xffffffffffffffaaull) {
+        write(1, "bad r2\n", 7);
+        return 1;
+    }
+    return 0;
+}
diff --git a/tests/tcg/s390x/exrl-trtr.c b/tests/tcg/s390x/exrl-trtr.c
new file mode 100644
index 0000000000..c33153ad7e
--- /dev/null
+++ b/tests/tcg/s390x/exrl-trtr.c
@@ -0,0 +1,48 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    char op1[] = {0, 1, 2, 3};
+    char op2[256];
+    uint64_t r1 = 0xffffffffffffffffull;
+    uint64_t r2 = 0xffffffffffffffffull;
+    uint64_t cc;
+    int i;
+
+    for (i = 0; i < 256; i++) {
+        if (i == 1) {
+            op2[i] = 0xbb;
+        } else {
+            op2[i] = 0;
+        }
+    }
+    asm volatile(
+        "    j 2f\n"
+        "1:  trtr 3(1,%[op1]),0(%[op2])\n"
+        "2:  exrl %[op1_len],1b\n"
+        "    lgr %[r1],%%r1\n"
+        "    lgr %[r2],%%r2\n"
+        "    ipm %[cc]\n"
+        : [r1] "+r" (r1),
+          [r2] "+r" (r2),
+          [cc] "=r" (cc)
+        : [op1] "r" (&op1),
+          [op1_len] "r" (3),
+          [op2] "r" (&op2)
+        : "r1", "r2", "cc");
+    cc = (cc >> 28) & 3;
+    if (cc != 1) {
+        write(1, "bad cc\n", 7);
+        return 1;
+    }
+    if ((char *)r1 != &op1[1]) {
+        write(1, "bad r1\n", 7);
+        return 1;
+    }
+    if (r2 != 0xffffffffffffffbbull) {
+        write(1, "bad r2\n", 7);
+        return 1;
+    }
+    return 0;
+}
-- 
2.18.0

[Qemu-devel] [PATCH 6/7] target/s390x: fix PACK reading 1 byte less and writing 1 byte more

[Pavel Zbitskiy]

PACK fails on the test from the Principles of Operation: F1F2F3F4
becomes 0000234C instead of 0001234C due to an off-by-one error.
Furthermore, it overwrites one extra byte to the left of F1.

If len_dest is 0, then we only want to flip the 1st byte and never loop
over the rest. Therefore, the loop condition should be > and not >=.

If len_src is 1, then we should flip the 1st byte and pack the 2nd.
Since len_src is already decremented before the loop, the first
condition should be >=, and not >.

Likewise for len_src == 2 and the second condition.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/mem_helper.c       |  6 +++---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/pack.c          | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 3 deletions(-)
 create mode 100644 tests/tcg/s390x/pack.c

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 704d0193b5..bacae4f503 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1019,15 +1019,15 @@ void HELPER(pack)(CPUS390XState *env, uint32_t len, uint64_t dest, uint64_t src)
     len_src--;
 
     /* now pack every value */
-    while (len_dest >= 0) {
+    while (len_dest > 0) {
         b = 0;
 
-        if (len_src > 0) {
+        if (len_src >= 0) {
             b = cpu_ldub_data_ra(env, src, ra) & 0x0f;
             src--;
             len_src--;
         }
-        if (len_src > 0) {
+        if (len_src >= 0) {
             b |= cpu_ldub_data_ra(env, src, ra) << 4;
             src--;
             len_src--;
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 7de4376f52..151dc075aa 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -5,3 +5,4 @@ TESTS+=csst
 TESTS+=ipm
 TESTS+=exrl-trt
 TESTS+=exrl-trtr
+TESTS+=pack
diff --git a/tests/tcg/s390x/pack.c b/tests/tcg/s390x/pack.c
new file mode 100644
index 0000000000..4be36f29a7
--- /dev/null
+++ b/tests/tcg/s390x/pack.c
@@ -0,0 +1,21 @@
+#include <unistd.h>
+
+int main(void)
+{
+    char data[] = {0xaa, 0xaa, 0xf1, 0xf2, 0xf3, 0xc4, 0xaa, 0xaa};
+    char exp[] = {0xaa, 0xaa, 0x00, 0x01, 0x23, 0x4c, 0xaa, 0xaa};
+    int i;
+
+    asm volatile(
+        "    pack 2(4,%[data]),2(4,%[data])\n"
+        :
+        : [data] "r" (&data[0])
+        : "memory");
+    for (i = 0; i < 8; i++) {
+        if (data[i] != exp[i]) {
+            write(1, "bad data\n", 9);
+            return 1;
+        }
+    }
+    return 0;
+}
-- 
2.18.0

[Qemu-devel] [PATCH 7/7] target/s390x: implement CVB, CVBY and CVBG

[Pavel Zbitskiy]

Convert to Binary - counterparts of the already implemented Convert
to Decimal (CVD*) instructions.
Example from the Principles of Operation: 25594C becomes 63FA.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 target/s390x/helper.h           |  1 +
 target/s390x/insn-data.def      |  4 +++
 target/s390x/int_helper.c       | 50 +++++++++++++++++++++++++++++++++
 target/s390x/translate.c        | 10 +++++++
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/cvb.c           | 18 ++++++++++++
 6 files changed, 84 insertions(+)
 create mode 100644 tests/tcg/s390x/cvb.c

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 97c60ca7bc..46baaee0ab 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -88,6 +88,7 @@ DEF_HELPER_FLAGS_4(tcxb, TCG_CALL_NO_RWG_SE, i32, env, i64, i64, i64)
 DEF_HELPER_FLAGS_2(sqeb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_2(sqdb, TCG_CALL_NO_WG, i64, env, i64)
 DEF_HELPER_FLAGS_3(sqxb, TCG_CALL_NO_WG, i64, env, i64, i64)
+DEF_HELPER_FLAGS_3(cvb, TCG_CALL_NO_WG, i64, env, i64, i32)
 DEF_HELPER_FLAGS_1(cvd, TCG_CALL_NO_RWG_SE, i64, s32)
 DEF_HELPER_FLAGS_4(pack, TCG_CALL_NO_WG, void, env, i32, i64, i64)
 DEF_HELPER_FLAGS_4(pka, TCG_CALL_NO_WG, void, env, i64, i64, i32)
diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 9c7b434fca..1b29a5e044 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -284,6 +284,10 @@
     D(0xec73, CLFIT,   RIE_a, GIE, r1_32u, i2_32u, 0, 0, ct, 0, 1)
     D(0xec71, CLGIT,   RIE_a, GIE, r1_o, i2_32u, 0, 0, ct, 0, 1)
 
+/* CONVERT TO BINARY */
+    C(0x4f00, CVB,     RX_a,  Z,   0, a2, new, r1_32, cvb, 0)
+    C(0xe306, CVBY,    RXY_a, LD,  0, a2, new, r1_32, cvb, 0)
+    C(0xe30e, CVBG,    RXY_a, Z,   0, a2, r1, 0, cvb, 0)
 /* CONVERT TO DECIMAL */
     C(0x4e00, CVD,     RX_a,  Z,   r1_o, a2, 0, 0, cvd, 0)
     C(0xe326, CVDY,    RXY_a, LD,  r1_o, a2, 0, 0, cvd, 0)
diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
index abf77a94e6..66c6dc8b3a 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/int_helper.c
@@ -24,6 +24,7 @@
 #include "exec/exec-all.h"
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
+#include "exec/cpu_ldst.h"
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER
@@ -118,6 +119,55 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
     return ret;
 }
 
+static void general_operand_exception(CPUS390XState *env, uintptr_t ra)
+{
+#ifndef CONFIG_USER_ONLY
+    LowCore *lowcore;
+
+    lowcore = cpu_map_lowcore(env);
+    lowcore->data_exc_code = 0;
+    cpu_unmap_lowcore(lowcore);
+#endif
+    s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);
+}
+
+uint64_t HELPER(cvb)(CPUS390XState *env, uint64_t src, uint32_t n)
+{
+    int i, j;
+    uintptr_t ra = GETPC();
+    int64_t dec, sign = 0, digit, val = 0, pow10 = 0;
+
+    for (i = 0; i < n; i++) {
+        dec = cpu_ldq_data_ra(env, src + (n - i - 1) * 8, ra);
+        for (j = 0; j < 16; j++, dec >>= 4) {
+            if (i == 0 && j == 0) {
+                sign = dec & 0xf;
+                if (sign < 0xa) {
+                    general_operand_exception(env, ra);
+                }
+                continue;
+            }
+            digit = dec & 0xf;
+            if (digit > 0x9) {
+                general_operand_exception(env, ra);
+            }
+            if (i == 0 && j == 1) {
+                if (sign == 0xb || sign == 0xd) {
+                    val = -digit;
+                    pow10 = -10;
+                } else {
+                    val = digit;
+                    pow10 = 10;
+                }
+            } else {
+                val += digit * pow10;
+                pow10 *= 10;
+            }
+        }
+    }
+    return val;
+}
+
 uint64_t HELPER(cvd)(int32_t reg)
 {
     /* positive 0 */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 6f8fbda222..48a97a37ff 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -2107,6 +2107,16 @@ static DisasJumpType op_csp(DisasContext *s, DisasOps *o)
 }
 #endif
 
+static DisasJumpType op_cvb(DisasContext *s, DisasOps *o)
+{
+    uint64_t n = ((s->fields->op == 0xE3) && (s->fields->op2 == 0x0E)) ?
+        /* CVBG */      2 :
+        /* CVB, CVBY */ 1;
+
+    gen_helper_cvb(o->out, cpu_env, o->in2, tcg_const_i32(n));
+    return DISAS_NEXT;
+}
+
 static DisasJumpType op_cvd(DisasContext *s, DisasOps *o)
 {
     TCGv_i64 t1 = tcg_temp_new_i64();
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 151dc075aa..990dfb26ff 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -6,3 +6,4 @@ TESTS+=ipm
 TESTS+=exrl-trt
 TESTS+=exrl-trtr
 TESTS+=pack
+TESTS+=cvb
diff --git a/tests/tcg/s390x/cvb.c b/tests/tcg/s390x/cvb.c
new file mode 100644
index 0000000000..3a72e132aa
--- /dev/null
+++ b/tests/tcg/s390x/cvb.c
@@ -0,0 +1,18 @@
+#include <stdint.h>
+#include <unistd.h>
+
+int main(void)
+{
+    uint64_t data = 0x000000000025594cull;
+    uint64_t result = 0;
+
+    asm volatile(
+        "    cvb %[result],%[data]\n"
+        : [result] "+r" (result)
+        : [data] "m" (data));
+    if (result != 0x63fa) {
+        write(1, "bad result\n", 11);
+        return 1;
+    }
+    return 0;
+}
-- 
2.18.0

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[Cornelia Huck]

On Thu,  9 Aug 2018 23:01:32 -0400
Pavel Zbitskiy <pavel.zbitskiy@gmail.com> wrote:

> Found while attempting to run an old tool in qemu.
> 
> * BAL and BALR:    Added.
> * CSST:            Qemu crashed after an accidental jump to garbage.
> * IPM:             A tool produced an incorrect output.
> * EX TRT/TRTR:     A tool ran quite slow.
> * PACK:            A tool produced an incorrect output.
> * CVB, CVBY, CVBG: Added.
> 
> Changes since v1:
> * Tests.

Nice, thanks for adding these.

> * Call pc_to_link_info() instead of op_bas().
> * Clarified CSST commit message.
> * Rewrote IPM using extract/deposit.
> * Clarified PACK commit message.
> * Do not use LowCore for CONFIG_USER_ONLY.
> * Reduce duplication in CVB code.
> 
> Pavel Zbitskiy (7):
>   tests/tcg: add a simple s390x test
>   target/s390x: add BAL and BALR instructions
>   target/s390x: fix CSST decoding and runtime alignment check
>   target/s390x: fix IPM polluting irrelevant bits
>   target/s390x: add EX support for TRT and TRTR
>   target/s390x: fix PACK reading 1 byte less and writing 1 byte more
>   target/s390x: implement CVB, CVBY and CVBG

I'll wait for some acks/reviews before applying these.

Re: [Qemu-devel] [PATCH 3/7] target/s390x: fix CSST decoding and runtime alignment check

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> CSST is defined as:
> 
>     C(0xc802, CSST,    SSF,   CASS, la1, a2, 0, 0, csst, 0)
> 
> It means that the first parameter is handled by in1_la1().
> in1_la1() fills addr1 field, and not in1.
> 
> Furthermore, when extract32() is used for the alignment check, the
> third parameter should specify the number of trailing bits that must
> be 0. For FC these numbers are:
> 
>     FC=0 (word, 4 bytes):        2
>     FC=1 (double word, 8 bytes): 3
>     FC=2 (quad word, 16 bytes):  4
> 
> For SC these numbers correspond to the size:
> 
>     SC=0: 0
>     SC=1: 1
>     SC=2: 2
>     SC=3: 3
>     SC=4: 4
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 4/7] target/s390x: fix IPM polluting irrelevant bits

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> Suppose psw.mask=0x0000000080000000, cc=2, r1=0 and we do "ipm 1".
> This command must touch only bits 32-39, so the expected output
> is r1=0x20000000. However, currently qemu yields r1=0x20008000,
> because irrelevant parts of PSW leak into r1 during program mask
> transfer.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/translate.c        | 17 +++++++----------
>  tests/tcg/s390x/Makefile.target |  1 +
>  tests/tcg/s390x/ipm.c           | 22 ++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 10 deletions(-)
>  create mode 100644 tests/tcg/s390x/ipm.c
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index d0d2c3412f..6f8fbda222 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2437,20 +2437,17 @@ static DisasJumpType op_insi(DisasContext *s, DisasOps *o)
>  
>  static DisasJumpType op_ipm(DisasContext *s, DisasOps *o)
>  {
> -    TCGv_i64 t1;
> +    TCGv_i64 t1, t2;
>  
>      gen_op_calc_cc(s);
> -    tcg_gen_andi_i64(o->out, o->out, ~0xff000000ull);
> -
>      t1 = tcg_temp_new_i64();
> -    tcg_gen_shli_i64(t1, psw_mask, 20);
> -    tcg_gen_shri_i64(t1, t1, 36);
> -    tcg_gen_or_i64(o->out, o->out, t1);
> -
> -    tcg_gen_extu_i32_i64(t1, cc_op);
> -    tcg_gen_shli_i64(t1, t1, 28);
> -    tcg_gen_or_i64(o->out, o->out, t1);
> +    tcg_gen_extract_i64(t1, psw_mask, 40, 4);
> +    t2 = tcg_temp_new_i64();
> +    tcg_gen_extu_i32_i64(t2, cc_op);
> +    tcg_gen_deposit_i64(t1, t1, t2, 4, 60);
> +    tcg_gen_deposit_i64(o->out, o->out, t1, 24, 8);

Not checked, but I wonder if you could avoid the second temp variable by
simply depositing right from psw_mask/cc into t1 and from there into out
(one step at a time).

>      tcg_temp_free_i64(t1);
> +    tcg_temp_free_i64(t2);
>      return DISAS_NEXT;
>  }
>  
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index f62f950d8e..c800a582e5 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -2,3 +2,4 @@ VPATH+=$(SRC_PATH)/tests/tcg/s390x
>  CFLAGS+=-march=zEC12 -m64
>  TESTS+=hello-s390x
>  TESTS+=csst
> +TESTS+=ipm
> diff --git a/tests/tcg/s390x/ipm.c b/tests/tcg/s390x/ipm.c
> new file mode 100644
> index 0000000000..742f3a18c5
> --- /dev/null
> +++ b/tests/tcg/s390x/ipm.c
> @@ -0,0 +1,22 @@
> +#include <stdint.h>
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    uint32_t op1 = 0x55555555;
> +    uint32_t op2 = 0x44444444;
> +    uint64_t cc = 0xffffffffffffffffull;
> +
> +    asm volatile(
> +        "    clc 0(4,%[op1]),0(%[op2])\n"
> +        "    ipm %[cc]\n"
> +        : [cc] "+r" (cc)
> +        : [op1] "r" (&op1),
> +          [op2] "r" (&op2)
> +        : "cc");
> +    if (cc != 0xffffffff20ffffffull) {
> +        write(1, "bad cc\n", 7);
> +        return 1;
> +    }
> +    return 0;
> +}
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> Found while attempting to run an old tool in qemu.
> 
> * BAL and BALR:    Added.
> * CSST:            Qemu crashed after an accidental jump to garbage.
> * IPM:             A tool produced an incorrect output.
> * EX TRT/TRTR:     A tool ran quite slow.
> * PACK:            A tool produced an incorrect output.
> * CVB, CVBY, CVBG: Added.
> 
> Changes since v1:
> * Tests.
> * Call pc_to_link_info() instead of op_bas().
> * Clarified CSST commit message.
> * Rewrote IPM using extract/deposit.
> * Clarified PACK commit message.
> * Do not use LowCore for CONFIG_USER_ONLY.
> * Reduce duplication in CVB code.

Thanks for looking into this and also for providing test cases!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH 5/7] target/s390x: add EX support for TRT and TRTR

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> Improves "b213c9f5: target/s390x: Implement TRTR" by introducing the
> intermediate functions, which are compatible with dx_helper type.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/mem_helper.c       | 16 +++++++++++
>  tests/tcg/s390x/Makefile.target |  2 ++
>  tests/tcg/s390x/exrl-trt.c      | 48 +++++++++++++++++++++++++++++++++
>  tests/tcg/s390x/exrl-trtr.c     | 48 +++++++++++++++++++++++++++++++++
>  4 files changed, 114 insertions(+)
>  create mode 100644 tests/tcg/s390x/exrl-trt.c
>  create mode 100644 tests/tcg/s390x/exrl-trtr.c
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index c94dbf3fcb..704d0193b5 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -1299,12 +1299,26 @@ static inline uint32_t do_helper_trt(CPUS390XState *env, int len,
>      return 0;
>  }
>  
> +static uint32_t do_helper_trt_fwd(CPUS390XState *env, uint32_t len,
> +                                  uint64_t array, uint64_t trans,
> +                                  uintptr_t ra)
> +{
> +    return do_helper_trt(env, len, array, trans, 1, ra);
> +}
> +
>  uint32_t HELPER(trt)(CPUS390XState *env, uint32_t len, uint64_t array,
>                       uint64_t trans)
>  {
>      return do_helper_trt(env, len, array, trans, 1, GETPC());
>  }
>  
> +static uint32_t do_helper_trt_bkwd(CPUS390XState *env, uint32_t len,
> +                                   uint64_t array, uint64_t trans,
> +                                   uintptr_t ra)
> +{
> +    return do_helper_trt(env, len, array, trans, -1, ra);
> +}
> +
>  uint32_t HELPER(trtr)(CPUS390XState *env, uint32_t len, uint64_t array,
>                        uint64_t trans)
>  {
> @@ -2193,12 +2207,14 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
>          typedef uint32_t (*dx_helper)(CPUS390XState *, uint32_t, uint64_t,
>                                        uint64_t, uintptr_t);
>          static const dx_helper dx[16] = {
> +            [0x0] = do_helper_trt_bkwd,
>              [0x2] = do_helper_mvc,
>              [0x4] = do_helper_nc,
>              [0x5] = do_helper_clc,
>              [0x6] = do_helper_oc,
>              [0x7] = do_helper_xc,
>              [0xc] = do_helper_tr,
> +            [0xd] = do_helper_trt_fwd,
>          };
>          dx_helper helper = dx[opc & 0xf];
>  
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index c800a582e5..7de4376f52 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -3,3 +3,5 @@ CFLAGS+=-march=zEC12 -m64
>  TESTS+=hello-s390x
>  TESTS+=csst
>  TESTS+=ipm
> +TESTS+=exrl-trt
> +TESTS+=exrl-trtr
> diff --git a/tests/tcg/s390x/exrl-trt.c b/tests/tcg/s390x/exrl-trt.c
> new file mode 100644
> index 0000000000..3c5323aecb
> --- /dev/null
> +++ b/tests/tcg/s390x/exrl-trt.c
> @@ -0,0 +1,48 @@
> +#include <stdint.h>
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    char op1[] = "hello";
> +    char op2[256];
> +    uint64_t r1 = 0xffffffffffffffffull;
> +    uint64_t r2 = 0xffffffffffffffffull;
> +    uint64_t cc;
> +    int i;
> +
> +    for (i = 0; i < 256; i++) {
> +        if (i == 0) {
> +            op2[i] = 0xaa;
> +        } else {
> +            op2[i] = 0;
> +        }
> +    }
> +    asm volatile(
> +        "    j 2f\n"
> +        "1:  trt 0(1,%[op1]),0(%[op2])\n"
> +        "2:  exrl %[op1_len],1b\n"
> +        "    lgr %[r1],%%r1\n"
> +        "    lgr %[r2],%%r2\n"
> +        "    ipm %[cc]\n"
> +        : [r1] "+r" (r1),
> +          [r2] "+r" (r2),
> +          [cc] "=r" (cc)
> +        : [op1] "r" (&op1),
> +          [op1_len] "r" (5),
> +          [op2] "r" (&op2)
> +        : "r1", "r2", "cc");
> +    cc = (cc >> 28) & 3;
> +    if (cc != 2) {
> +        write(1, "bad cc\n", 7);
> +        return 1;
> +    }
> +    if ((char *)r1 != &op1[5]) {
> +        write(1, "bad r1\n", 7);
> +        return 1;
> +    }
> +    if (r2 != 0xffffffffffffffaaull) {
> +        write(1, "bad r2\n", 7);
> +        return 1;
> +    }
> +    return 0;
> +}
> diff --git a/tests/tcg/s390x/exrl-trtr.c b/tests/tcg/s390x/exrl-trtr.c
> new file mode 100644
> index 0000000000..c33153ad7e
> --- /dev/null
> +++ b/tests/tcg/s390x/exrl-trtr.c
> @@ -0,0 +1,48 @@
> +#include <stdint.h>
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    char op1[] = {0, 1, 2, 3};
> +    char op2[256];
> +    uint64_t r1 = 0xffffffffffffffffull;
> +    uint64_t r2 = 0xffffffffffffffffull;
> +    uint64_t cc;
> +    int i;
> +
> +    for (i = 0; i < 256; i++) {
> +        if (i == 1) {
> +            op2[i] = 0xbb;
> +        } else {
> +            op2[i] = 0;
> +        }
> +    }
> +    asm volatile(
> +        "    j 2f\n"
> +        "1:  trtr 3(1,%[op1]),0(%[op2])\n"
> +        "2:  exrl %[op1_len],1b\n"
> +        "    lgr %[r1],%%r1\n"
> +        "    lgr %[r2],%%r2\n"
> +        "    ipm %[cc]\n"
> +        : [r1] "+r" (r1),
> +          [r2] "+r" (r2),
> +          [cc] "=r" (cc)
> +        : [op1] "r" (&op1),
> +          [op1_len] "r" (3),
> +          [op2] "r" (&op2)
> +        : "r1", "r2", "cc");
> +    cc = (cc >> 28) & 3;
> +    if (cc != 1) {
> +        write(1, "bad cc\n", 7);
> +        return 1;
> +    }
> +    if ((char *)r1 != &op1[1]) {
> +        write(1, "bad r1\n", 7);
> +        return 1;
> +    }
> +    if (r2 != 0xffffffffffffffbbull) {
> +        write(1, "bad r2\n", 7);
> +        return 1;
> +    }
> +    return 0;
> +}
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH 2/7] target/s390x: add BAL and BALR instructions

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> These instructions are provided for compatibility purposes and are
> used only by old software, in the new code BAS and BASR are preferred.
> The difference between the old and new instruction exists only in the
> 24-bit mode.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  target/s390x/insn-data.def |  3 +++
>  target/s390x/translate.c   | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
> index 5c6f33ed9c..9c7b434fca 100644
> --- a/target/s390x/insn-data.def
> +++ b/target/s390x/insn-data.def
> @@ -102,6 +102,9 @@
>      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)
>  
> +/* BRANCH AND LINK */
> +    C(0x0500, BALR,    RR_a,  Z,   0, r2_nz, r1, 0, bal, 0)
> +    C(0x4500, BAL,     RX_a,  Z,   0, a2, r1, 0, bal, 0)
>  /* BRANCH AND SAVE */
>      C(0x0d00, BASR,    RR_a,  Z,   0, r2_nz, r1, 0, bas, 0)
>      C(0x4d00, BAS,     RX_a,  Z,   0, a2, r1, 0, bas, 0)
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 57c03cbf58..316ff79250 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -1463,6 +1463,39 @@ static DisasJumpType op_bas(DisasContext *s, DisasOps *o)
>      }
>  }
>  
> +static void save_link_info(DisasContext *s, DisasOps *o)
> +{
> +    TCGv_i64 t;
> +
> +    if (s->base.tb->flags & (FLAG_MASK_32 | FLAG_MASK_64)) {
> +        tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));

"In the 24-bit or 31-bit addressing mode, bits 0-31 of
the first-operand location remain unchanged." I think this is also right
now broken for BAS.


> +        return;
> +    }
> +    gen_op_calc_cc(s);
> +    tcg_gen_andi_i64(o->out, o->out, 0xffffffff00000000ull);
> +    tcg_gen_ori_i64(o->out, o->out, ((s->ilen / 2) << 30) | s->pc_tmp);
> +    t = tcg_temp_new_i64();
> +    tcg_gen_shri_i64(t, psw_mask, 16);
> +    tcg_gen_andi_i64(t, t, 0x0f000000);
> +    tcg_gen_or_i64(o->out, o->out, t);
> +    tcg_gen_extu_i32_i64(t, cc_op);
> +    tcg_gen_shli_i64(t, t, 28);
> +    tcg_gen_or_i64(o->out, o->out, t);
> +    tcg_temp_free_i64(t);

This looks good to me (ilen really belongs to the current instruction
(not pc_tmp), which seems to be what BAL expects)

> +}
> +
> +static DisasJumpType op_bal(DisasContext *s, DisasOps *o)
> +{
> +    save_link_info(s, o);
> +    if (o->in2) {
> +        tcg_gen_mov_i64(psw_addr, o->in2);
> +        per_branch(s, false);
> +        return DISAS_PC_UPDATED;
> +    } else {
> +        return DISAS_NEXT;
> +    }
> +}
> +
>  static DisasJumpType op_basi(DisasContext *s, DisasOps *o)
>  {
>      tcg_gen_movi_i64(o->out, pc_to_link_info(s, s->pc_tmp));
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [qemu-s390x] [PATCH 1/7] tests/tcg: add a simple s390x test

[David Hildenbrand]

On 10.08.2018 05:01, Pavel Zbitskiy wrote:
> Copied from alpha.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  tests/tcg/s390x/Makefile.target | 3 +++
>  tests/tcg/s390x/hello-s390x.c   | 7 +++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 tests/tcg/s390x/Makefile.target
>  create mode 100644 tests/tcg/s390x/hello-s390x.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> new file mode 100644
> index 0000000000..9f4076901f
> --- /dev/null
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -0,0 +1,3 @@
> +VPATH+=$(SRC_PATH)/tests/tcg/s390x
> +CFLAGS+=-march=zEC12 -m64
> +TESTS+=hello-s390x
> diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
> new file mode 100644
> index 0000000000..3dc0a05f2b
> --- /dev/null
> +++ b/tests/tcg/s390x/hello-s390x.c
> @@ -0,0 +1,7 @@
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    write(1, "hello\n", 6);
> +    return 0;
> +}
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180810030139.25916-1-pavel.zbitskiy@gmail.com
Subject: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
fe6ceb115b target/s390x: implement CVB, CVBY and CVBG
1252e06f7b target/s390x: fix PACK reading 1 byte less and writing 1 byte more
051847fbce target/s390x: add EX support for TRT and TRTR
c8fb5a8ab8 target/s390x: fix IPM polluting irrelevant bits
beeec6e5c4 target/s390x: fix CSST decoding and runtime alignment check
0d91395d44 target/s390x: add BAL and BALR instructions
06e504d408 tests/tcg: add a simple s390x test

=== OUTPUT BEGIN ===
Checking PATCH 1/7: tests/tcg: add a simple s390x test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100644

total: 0 errors, 1 warnings, 10 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/7: target/s390x: add BAL and BALR instructions...
Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#98: FILE: tests/tcg/s390x/csst.c:22:
+        : [op1] "+m" (op1),

ERROR: space prohibited before open square bracket '['
#102: FILE: tests/tcg/s390x/csst.c:26:
+        : [flags] "K" (0x0301),

total: 2 errors, 1 warnings, 66 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/7: target/s390x: fix IPM polluting irrelevant bits...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#74: FILE: tests/tcg/s390x/ipm.c:13:
+        : [cc] "+r" (cc)

ERROR: space prohibited before open square bracket '['
#75: FILE: tests/tcg/s390x/ipm.c:14:
+        : [op1] "r" (&op1),

total: 2 errors, 1 warnings, 53 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/7: target/s390x: add EX support for TRT and TRTR...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#101: FILE: tests/tcg/s390x/exrl-trt.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#104: FILE: tests/tcg/s390x/exrl-trt.c:30:
+        : [op1] "r" (&op1),

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/exrl-trtr.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#158: FILE: tests/tcg/s390x/exrl-trtr.c:30:
+        : [op1] "r" (&op1),

total: 4 errors, 1 warnings, 141 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/7: target/s390x: fix PACK reading 1 byte less and writing 1 byte more...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#72: FILE: tests/tcg/s390x/pack.c:12:
+        : [data] "r" (&data[0])

total: 1 errors, 1 warnings, 43 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/7: target/s390x: implement CVB, CVBY and CVBG...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#139: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#154: FILE: tests/tcg/s390x/cvb.c:11:
+        : [result] "+r" (result)

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/cvb.c:12:
+        : [data] "m" (data));

total: 2 errors, 1 warnings, 117 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[Cornelia Huck]

On Wed, 15 Aug 2018 11:25:26 -0700 (PDT)
no-reply@patchew.org wrote:

> === OUTPUT BEGIN ===
> Checking PATCH 1/7: tests/tcg: add a simple s390x test...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #13: 
> new file mode 100644

tests/tcg/s390x/ does not seem to have a MAINTAINERS entry yet (it did
not contain anything interesting before, anyway.) Probably should be
added to the s390x/tcg section, as for the other architectures.

> 
> total: 0 errors, 1 warnings, 10 lines checked
> 
> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
> Checking PATCH 2/7: target/s390x: add BAL and BALR instructions...
> Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #72: 
> new file mode 100644
> 
> ERROR: space prohibited before open square bracket '['
> #98: FILE: tests/tcg/s390x/csst.c:22:
> +        : [op1] "+m" (op1),
> 
> ERROR: space prohibited before open square bracket '['
> #102: FILE: tests/tcg/s390x/csst.c:26:
> +        : [flags] "K" (0x0301),

Checkpatch often seems to have a hard time dealing with inline
assemblies. Probably best to just ignore the errors.

Re: [Qemu-devel] [qemu-s390x] [PATCH 1/7] tests/tcg: add a simple s390x test

[Thomas Huth]

On 08/10/2018 05:01 AM, Pavel Zbitskiy wrote:
> Copied from alpha.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  tests/tcg/s390x/Makefile.target | 3 +++
>  tests/tcg/s390x/hello-s390x.c   | 7 +++++++
>  2 files changed, 10 insertions(+)
>  create mode 100644 tests/tcg/s390x/Makefile.target
>  create mode 100644 tests/tcg/s390x/hello-s390x.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> new file mode 100644
> index 0000000000..9f4076901f
> --- /dev/null
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -0,0 +1,3 @@
> +VPATH+=$(SRC_PATH)/tests/tcg/s390x
> +CFLAGS+=-march=zEC12 -m64
> +TESTS+=hello-s390x
> diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
> new file mode 100644
> index 0000000000..3dc0a05f2b
> --- /dev/null
> +++ b/tests/tcg/s390x/hello-s390x.c
> @@ -0,0 +1,7 @@
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    write(1, "hello\n", 6);
> +    return 0;
> +}
> 

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

Re: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180810030139.25916-1-pavel.zbitskiy@gmail.com
Subject: [Qemu-devel] [PATCH 0/7] Some improvements in z/Arch instructions support

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bd3e0382ce target/s390x: implement CVB, CVBY and CVBG
f4fba58f1c target/s390x: fix PACK reading 1 byte less and writing 1 byte more
164afdcf5f target/s390x: add EX support for TRT and TRTR
56d0f419b7 target/s390x: fix IPM polluting irrelevant bits
f48adc25a1 target/s390x: fix CSST decoding and runtime alignment check
63fd9ce84b target/s390x: add BAL and BALR instructions
d7d028e76f tests/tcg: add a simple s390x test

=== OUTPUT BEGIN ===
Checking PATCH 1/7: tests/tcg: add a simple s390x test...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#14: 
new file mode 100644

total: 0 errors, 1 warnings, 10 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/7: target/s390x: add BAL and BALR instructions...
Checking PATCH 3/7: target/s390x: fix CSST decoding and runtime alignment check...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#72: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#98: FILE: tests/tcg/s390x/csst.c:22:
+        : [op1] "+m" (op1),

ERROR: space prohibited before open square bracket '['
#102: FILE: tests/tcg/s390x/csst.c:26:
+        : [flags] "K" (0x0301),

total: 2 errors, 1 warnings, 66 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/7: target/s390x: fix IPM polluting irrelevant bits...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#57: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#74: FILE: tests/tcg/s390x/ipm.c:13:
+        : [cc] "+r" (cc)

ERROR: space prohibited before open square bracket '['
#75: FILE: tests/tcg/s390x/ipm.c:14:
+        : [op1] "r" (&op1),

total: 2 errors, 1 warnings, 53 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 5/7: target/s390x: add EX support for TRT and TRTR...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#70: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#101: FILE: tests/tcg/s390x/exrl-trt.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#104: FILE: tests/tcg/s390x/exrl-trt.c:30:
+        : [op1] "r" (&op1),

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/exrl-trtr.c:27:
+        : [r1] "+r" (r1),

ERROR: space prohibited before open square bracket '['
#158: FILE: tests/tcg/s390x/exrl-trtr.c:30:
+        : [op1] "r" (&op1),

total: 4 errors, 1 warnings, 141 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 6/7: target/s390x: fix PACK reading 1 byte less and writing 1 byte more...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#56: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#72: FILE: tests/tcg/s390x/pack.c:12:
+        : [data] "r" (&data[0])

total: 1 errors, 1 warnings, 43 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 7/7: target/s390x: implement CVB, CVBY and CVBG...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#139: 
new file mode 100644

ERROR: space prohibited before open square bracket '['
#154: FILE: tests/tcg/s390x/cvb.c:11:
+        : [result] "+r" (result)

ERROR: space prohibited before open square bracket '['
#155: FILE: tests/tcg/s390x/cvb.c:12:
+        : [data] "m" (data));

total: 2 errors, 1 warnings, 117 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 1/7] tests/tcg: add a simple s390x test

[David Hildenbrand]

On 21.08.2018 04:50, Pavel Zbitskiy wrote:
> Copied from alpha.
> 
> Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
> ---
>  MAINTAINERS                     | 1 +
>  tests/tcg/s390x/Makefile.target | 3 +++
>  tests/tcg/s390x/hello-s390x.c   | 7 +++++++
>  3 files changed, 11 insertions(+)
>  create mode 100644 tests/tcg/s390x/Makefile.target
>  create mode 100644 tests/tcg/s390x/hello-s390x.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 6902a568f4..27af17caa7 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -253,6 +253,7 @@ S: Maintained
>  F: target/s390x/
>  F: hw/s390x/
>  F: disas/s390.c
> +F: tests/tcg/s390x/
>  L: qemu-s390x@nongnu.org
>  
>  SH4
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> new file mode 100644
> index 0000000000..9f4076901f
> --- /dev/null
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -0,0 +1,3 @@
> +VPATH+=$(SRC_PATH)/tests/tcg/s390x
> +CFLAGS+=-march=zEC12 -m64
> +TESTS+=hello-s390x
> diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
> new file mode 100644
> index 0000000000..3dc0a05f2b
> --- /dev/null
> +++ b/tests/tcg/s390x/hello-s390x.c
> @@ -0,0 +1,7 @@
> +#include <unistd.h>
> +
> +int main(void)
> +{
> +    write(1, "hello\n", 6);
> +    return 0;
> +}
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH 1/7] tests/tcg: add a simple s390x test

[Pavel Zbitskiy]

Copied from alpha.

Signed-off-by: Pavel Zbitskiy <pavel.zbitskiy@gmail.com>
---
 MAINTAINERS                     | 1 +
 tests/tcg/s390x/Makefile.target | 3 +++
 tests/tcg/s390x/hello-s390x.c   | 7 +++++++
 3 files changed, 11 insertions(+)
 create mode 100644 tests/tcg/s390x/Makefile.target
 create mode 100644 tests/tcg/s390x/hello-s390x.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6902a568f4..27af17caa7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -253,6 +253,7 @@ S: Maintained
 F: target/s390x/
 F: hw/s390x/
 F: disas/s390.c
+F: tests/tcg/s390x/
 L: qemu-s390x@nongnu.org
 
 SH4
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
new file mode 100644
index 0000000000..9f4076901f
--- /dev/null
+++ b/tests/tcg/s390x/Makefile.target
@@ -0,0 +1,3 @@
+VPATH+=$(SRC_PATH)/tests/tcg/s390x
+CFLAGS+=-march=zEC12 -m64
+TESTS+=hello-s390x
diff --git a/tests/tcg/s390x/hello-s390x.c b/tests/tcg/s390x/hello-s390x.c
new file mode 100644
index 0000000000..3dc0a05f2b
--- /dev/null
+++ b/tests/tcg/s390x/hello-s390x.c
@@ -0,0 +1,7 @@
+#include <unistd.h>
+
+int main(void)
+{
+    write(1, "hello\n", 6);
+    return 0;
+}
-- 
2.18.0