[PATCH 00/12] target/s390x: Miscellaneous TCG fixes

[PATCH 00/12] target/s390x: Miscellaneous TCG fixes

[Ilya Leoshkevich]

Hi,

Randomized testing found a number of issues in the s390x emulation.
This series fixes 6 of them (patches 2-7) and adds tests (patches
8-12); patch 1 is a cosmetic improvement needed for the EPSW test.

There are more issues, but I thought it would be better to send this
batch now.

Best regards,
Ilya

Ilya Leoshkevich (12):
  linux-user: elfload: Add more initial s390x PSW bits
  target/s390x: Fix EPSW CC reporting
  target/s390x: Fix MDEB and MDEBR
  target/s390x: Fix MVCRL with a large value in R0
  target/s390x: Fix LRA overwriting the top 32 bits on DAT error
  target/s390x: Fix LRA when DAT is off
  target/s390x: Fix relative long instructions with large offsets
  tests/tcg/s390x: Test EPSW
  tests/tcg/s390x: Test LARL with a large offset
  tests/tcg/s390x: Test LRA
  tests/tcg/s390x: Test MDEB and MDEBR
  tests/tcg/s390x: Test MVCRL with a large value in R0

 linux-user/elfload.c                    |  4 ++-
 target/s390x/helper.h                   |  2 +-
 target/s390x/mmu_helper.c               |  2 +-
 target/s390x/tcg/fpu_helper.c           |  3 +-
 target/s390x/tcg/insn-data.h.inc        |  4 +--
 target/s390x/tcg/mem_helper.c           |  5 +--
 target/s390x/tcg/translate.c            |  8 +++--
 tests/tcg/s390x/Makefile.softmmu-target |  1 +
 tests/tcg/s390x/Makefile.target         |  3 ++
 tests/tcg/s390x/epsw.c                  | 23 +++++++++++++
 tests/tcg/s390x/larl.c                  | 17 +++++++++
 tests/tcg/s390x/lra.S                   | 19 ++++++++++
 tests/tcg/s390x/mdeb.c                  | 30 ++++++++++++++++
 tests/tcg/s390x/mie3-mvcrl.c            | 46 +++++++++++++++++++------
 14 files changed, 147 insertions(+), 20 deletions(-)
 create mode 100644 tests/tcg/s390x/epsw.c
 create mode 100644 tests/tcg/s390x/larl.c
 create mode 100644 tests/tcg/s390x/lra.S
 create mode 100644 tests/tcg/s390x/mdeb.c

-- 
2.41.0

[PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits

[Ilya Leoshkevich]

Make the PSW look more similar to the real s390x userspace PSW.
Except for being there, the newly added bits should not affect the
userspace code execution.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 linux-user/elfload.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 6900974c373..7935110bff4 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -1635,7 +1635,9 @@ const char *elf_hwcap_str(uint32_t bit)
 static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
 {
     regs->psw.addr = infop->entry;
-    regs->psw.mask = PSW_MASK_64 | PSW_MASK_32;
+    regs->psw.mask = PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
+                     PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_MASK_64 | \
+                     PSW_MASK_32;
     regs->gprs[15] = infop->start_stack;
 }
 
-- 
2.41.0

[PATCH 02/12] target/s390x: Fix EPSW CC reporting

[Ilya Leoshkevich]

EPSW should explicitly calculate and insert CC, like IPM does.

Fixes: e30a9d3fea58 ("target-s390: Implement EPSW")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/translate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a6ee2d44234..0cef6efbef4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2383,10 +2383,14 @@ static DisasJumpType op_epsw(DisasContext *s, DisasOps *o)
     int r1 = get_field(s, r1);
     int r2 = get_field(s, r2);
     TCGv_i64 t = tcg_temp_new_i64();
+    TCGv_i64 t_cc = tcg_temp_new_i64();
 
     /* Note the "subsequently" in the PoO, which implies a defined result
        if r1 == r2.  Thus we cannot defer these writes to an output hook.  */
+    gen_op_calc_cc(s);
+    tcg_gen_extu_i32_i64(t_cc, cc_op);
     tcg_gen_shri_i64(t, psw_mask, 32);
+    tcg_gen_deposit_i64(t, t, t_cc, 12, 2);
     store_reg32_i64(r1, t);
     if (r2 != 0) {
         store_reg32_i64(r2, psw_mask);
-- 
2.41.0

[PATCH 03/12] target/s390x: Fix MDEB and MDEBR

[Ilya Leoshkevich]

These instructions multiply 32 bits by 32 bits, not 32 bits by 64 bits.

Fixes: 83b00736f3d8 ("target-s390: Convert FP MULTIPLY")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/fpu_helper.c    | 3 ++-
 target/s390x/tcg/insn-data.h.inc | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/s390x/tcg/fpu_helper.c b/target/s390x/tcg/fpu_helper.c
index 57e58292833..4b7fa58af3e 100644
--- a/target/s390x/tcg/fpu_helper.c
+++ b/target/s390x/tcg/fpu_helper.c
@@ -306,8 +306,9 @@ uint64_t HELPER(mdb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
 /* 64/32-bit FP multiplication */
 uint64_t HELPER(mdeb)(CPUS390XState *env, uint64_t f1, uint64_t f2)
 {
+    float64 f1_64 = float32_to_float64(f1, &env->fpu_status);
     float64 ret = float32_to_float64(f2, &env->fpu_status);
-    ret = float64_mul(f1, ret, &env->fpu_status);
+    ret = float64_mul(f1_64, ret, &env->fpu_status);
     handle_exceptions(env, false, GETPC());
     return ret;
 }
diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc
index 0a45dbbcda8..457ed25d2fa 100644
--- a/target/s390x/tcg/insn-data.h.inc
+++ b/target/s390x/tcg/insn-data.h.inc
@@ -667,11 +667,11 @@
     F(0xb317, MEEBR,   RRE,   Z,   e1, e2, new, e1, meeb, 0, IF_BFP)
     F(0xb31c, MDBR,    RRE,   Z,   f1, f2, new, f1, mdb, 0, IF_BFP)
     F(0xb34c, MXBR,    RRE,   Z,   x1, x2, new_x, x1, mxb, 0, IF_BFP)
-    F(0xb30c, MDEBR,   RRE,   Z,   f1, e2, new, f1, mdeb, 0, IF_BFP)
+    F(0xb30c, MDEBR,   RRE,   Z,   e1, e2, new, f1, mdeb, 0, IF_BFP)
     F(0xb307, MXDBR,   RRE,   Z,   f1, f2, new_x, x1, mxdb, 0, IF_BFP)
     F(0xed17, MEEB,    RXE,   Z,   e1, m2_32u, new, e1, meeb, 0, IF_BFP)
     F(0xed1c, MDB,     RXE,   Z,   f1, m2_64, new, f1, mdb, 0, IF_BFP)
-    F(0xed0c, MDEB,    RXE,   Z,   f1, m2_32u, new, f1, mdeb, 0, IF_BFP)
+    F(0xed0c, MDEB,    RXE,   Z,   e1, m2_32u, new, f1, mdeb, 0, IF_BFP)
     F(0xed07, MXDB,    RXE,   Z,   f1, m2_64, new_x, x1, mxdb, 0, IF_BFP)
 /* MULTIPLY HALFWORD */
     C(0x4c00, MH,      RX_a,  Z,   r1_o, m2_16s, new, r1_32, mul, 0)
-- 
2.41.0

[PATCH 04/12] target/s390x: Fix MVCRL with a large value in R0

[Ilya Leoshkevich]

Using a large R0 causes an assertion error:

    qemu-s390x: target/s390x/tcg/mem_helper.c:183: access_prepare_nf: Assertion `size > 0 && size <= 4096' failed.

Even though PoP explicitly advises against using more than 8 bits for the
size, an emulator crash is never a good thing.

Fix by truncating the size to 8 bits.

Fixes: ea0a1053e276 ("s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/mem_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index d02ec861d8b..84ad85212c9 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -514,6 +514,7 @@ void HELPER(mvcrl)(CPUS390XState *env, uint64_t l, uint64_t dest, uint64_t src)
     int32_t i;
 
     /* MVCRL always copies one more byte than specified - maximum is 256 */
+    l &= 0xff;
     l++;
 
     access_prepare(&srca, env, src, l, MMU_DATA_LOAD, mmu_idx, ra);
-- 
2.41.0

[PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error

[Ilya Leoshkevich]

When a DAT error occurs, LRA is supposed to write the error information
to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.

Fix by passing the original value of R1 into helper and copying the
top 32 bits to the return value.

Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/helper.h         | 2 +-
 target/s390x/tcg/mem_helper.c | 4 ++--
 target/s390x/tcg/translate.c  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/s390x/helper.h b/target/s390x/helper.h
index 6bc01df73d7..05102578fc9 100644
--- a/target/s390x/helper.h
+++ b/target/s390x/helper.h
@@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
 DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
-DEF_HELPER_2(lra, i64, env, i64)
+DEF_HELPER_3(lra, i64, env, i64, i64)
 DEF_HELPER_1(per_check_exception, void, env)
 DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
 DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 84ad85212c9..94d93d7ea78 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
 }
 
 /* load real address */
-uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
+uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
 {
     uint64_t asc = env->psw.mask & PSW_MASK_ASC;
     uint64_t ret, tec;
@@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
     exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec);
     if (exc) {
         cc = 3;
-        ret = exc | 0x80000000;
+        ret = (r1 & 0xFFFFFFFF00000000) | exc | 0x80000000;
     } else {
         cc = 0;
         ret |= addr & ~TARGET_PAGE_MASK;
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0cef6efbef4..a6079ab7b4f 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
 {
-    gen_helper_lra(o->out, cpu_env, o->in2);
+    gen_helper_lra(o->out, cpu_env, o->out, o->in2);
     set_cc_static(s);
     return DISAS_NEXT;
 }
-- 
2.41.0

[PATCH 06/12] target/s390x: Fix LRA when DAT is off

[Ilya Leoshkevich]

LRA should perform DAT regardless of whether it's on or off.
Disable DAT check for MMU_S390_LRA.

Fixes: defb0e3157af ("s390x: Implement opcode helpers")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/mmu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index b04b57c2356..fbb2f1b4d48 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -417,7 +417,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 
     vaddr &= TARGET_PAGE_MASK;
 
-    if (!(env->psw.mask & PSW_MASK_DAT)) {
+    if (rw != MMU_S390_LRA && !(env->psw.mask & PSW_MASK_DAT)) {
         *raddr = vaddr;
         goto nodat;
     }
-- 
2.41.0

[PATCH 07/12] target/s390x: Fix relative long instructions with large offsets

[Ilya Leoshkevich]

The expression "imm * 2" in gen_ri2() can wrap around if imm is large
enough.

Fix by casting imm to int64_t, like it's done in disas_jdest().

Fixes: e8ecdfeb30f0 ("Fix EXECUTE of relative branches")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 target/s390x/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index a6079ab7b4f..6661b27efa4 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -5794,7 +5794,7 @@ static TCGv gen_ri2(DisasContext *s)
 
     disas_jdest(s, i2, is_imm, imm, ri2);
     if (is_imm) {
-        ri2 = tcg_constant_i64(s->base.pc_next + imm * 2);
+        ri2 = tcg_constant_i64(s->base.pc_next + (int64_t)imm * 2);
     }
 
     return ri2;
-- 
2.41.0

[PATCH 08/12] tests/tcg/s390x: Test EPSW

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/epsw.c          | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)
 create mode 100644 tests/tcg/s390x/epsw.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 85abfbb98c0..2ef22c88d95 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -36,6 +36,7 @@ TESTS+=rxsbg
 TESTS+=ex-relative-long
 TESTS+=ex-branch
 TESTS+=mxdb
+TESTS+=epsw
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/epsw.c b/tests/tcg/s390x/epsw.c
new file mode 100644
index 00000000000..affb1a5e3a1
--- /dev/null
+++ b/tests/tcg/s390x/epsw.c
@@ -0,0 +1,23 @@
+/*
+ * Test the EPSW instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+    unsigned long r1 = 0x1234567887654321UL, r2 = 0x8765432112345678UL;
+
+    asm("cr %[r1],%[r2]\n"  /* cc = 1 */
+        "epsw %[r1],%[r2]"
+        : [r1] "+r" (r1), [r2] "+r" (r2) : : "cc");
+
+    /* Do not check the R and RI bits. */
+    r1 &= ~0x40000008UL;
+    assert(r1 == 0x1234567807051001UL);
+    assert(r2 == 0x8765432180000000UL);
+
+    return EXIT_SUCCESS;
+}
-- 
2.41.0

[PATCH 09/12] tests/tcg/s390x: Test LARL with a large offset

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/larl.c          | 17 +++++++++++++++++
 2 files changed, 18 insertions(+)
 create mode 100644 tests/tcg/s390x/larl.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 2ef22c88d95..dbf64c991e9 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -37,6 +37,7 @@ TESTS+=ex-relative-long
 TESTS+=ex-branch
 TESTS+=mxdb
 TESTS+=epsw
+TESTS+=larl
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
new file mode 100644
index 00000000000..b9ced99a023
--- /dev/null
+++ b/tests/tcg/s390x/larl.c
@@ -0,0 +1,17 @@
+/*
+ * Test the LARL instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdlib.h>
+
+int main(void)
+{
+    long algfi = (long)main;
+    long larl;
+
+    asm("algfi %[r],0xd0000000" : [r] "+r" (algfi) : : "cc");
+    asm("larl %[r],main+0xd0000000" : [r] "=r" (larl));
+
+    return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
+}
-- 
2.41.0

[PATCH 10/12] tests/tcg/s390x: Test LRA

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.softmmu-target |  1 +
 tests/tcg/s390x/lra.S                   | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 tests/tcg/s390x/lra.S

diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target
index 44dfd716291..242c7b0f83c 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -20,6 +20,7 @@ ASM_TESTS =                                                                    \
     sam                                                                        \
     lpsw                                                                       \
     lpswe-early                                                                \
+    lra                                                                        \
     ssm-early                                                                  \
     stosm-early                                                                \
     unaligned-lowcore
diff --git a/tests/tcg/s390x/lra.S b/tests/tcg/s390x/lra.S
new file mode 100644
index 00000000000..79ab86f36bb
--- /dev/null
+++ b/tests/tcg/s390x/lra.S
@@ -0,0 +1,19 @@
+    .org 0x200                         /* lowcore padding */
+    .globl _start
+_start:
+    lgrl %r1,initial_r1
+    lra %r1,0(%r1)
+    cgrl %r1,expected_r1
+    jne 1f
+    lpswe success_psw
+1:
+    lpswe failure_psw
+    .align 8
+initial_r1:
+    .quad 0x8765432112345678
+expected_r1:
+    .quad 0x8765432180000038           /* ASCE type exception */
+success_psw:
+    .quad 0x2000000000000,0xfff        /* see is_special_wait_psw() */
+failure_psw:
+    .quad 0x2000000000000,0            /* disabled wait */
-- 
2.41.0

[PATCH 11/12] tests/tcg/s390x: Test MDEB and MDEBR

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/Makefile.target |  1 +
 tests/tcg/s390x/mdeb.c          | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 tests/tcg/s390x/mdeb.c

diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index dbf64c991e9..19fbbc6e531 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -38,6 +38,7 @@ TESTS+=ex-branch
 TESTS+=mxdb
 TESTS+=epsw
 TESTS+=larl
+TESTS+=mdeb
 
 cdsg: CFLAGS+=-pthread
 cdsg: LDFLAGS+=-pthread
diff --git a/tests/tcg/s390x/mdeb.c b/tests/tcg/s390x/mdeb.c
new file mode 100644
index 00000000000..4897d28069f
--- /dev/null
+++ b/tests/tcg/s390x/mdeb.c
@@ -0,0 +1,30 @@
+/*
+ * Test the MDEB and MDEBR instructions.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+    union {
+        float f[2];
+        double d;
+    } a;
+    float b;
+
+    a.f[0] = 1.2345;
+    a.f[1] = 999;
+    b = 6.789;
+    asm("mdeb %[a],%[b]" : [a] "+f" (a.d) : [b] "R" (b));
+    assert(a.d > 8.38 && a.d < 8.39);
+
+    a.f[0] = 1.2345;
+    a.f[1] = 999;
+    b = 6.789;
+    asm("mdebr %[a],%[b]" : [a] "+f" (a.d) : [b] "f" (b));
+    assert(a.d > 8.38 && a.d < 8.39);
+
+    return EXIT_SUCCESS;
+}
-- 
2.41.0

[PATCH 12/12] tests/tcg/s390x: Test MVCRL with a large value in R0

[Ilya Leoshkevich]

Add a small test to prevent regressions.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 tests/tcg/s390x/mie3-mvcrl.c | 46 ++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 10 deletions(-)

diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
index 93c7b0a2903..ec78dd1d493 100644
--- a/tests/tcg/s390x/mie3-mvcrl.c
+++ b/tests/tcg/s390x/mie3-mvcrl.c
@@ -1,29 +1,55 @@
+#include <stdbool.h>
 #include <stdint.h>
+#include <stdlib.h>
 #include <string.h>
 
-
-static inline void mvcrl_8(const char *dst, const char *src)
+static void mvcrl(const char *dst, const char *src, size_t len)
 {
+    register long r0 asm("r0") = len;
+
     asm volatile (
-        "llill %%r0, 8\n"
         ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
-        : : [dst] "d" (dst), [src] "d" (src)
-        : "r0", "memory");
+        : : [dst] "d" (dst), [src] "d" (src), "r" (r0)
+        : "memory");
 }
 
-
-int main(int argc, char *argv[])
+static bool test(void)
 {
     const char *alpha = "abcdefghijklmnop";
 
     /* array missing 'i' */
-    char tstr[17] = "abcdefghjklmnop\0" ;
+    char tstr[17] = "abcdefghjklmnop\0";
 
     /* mvcrl reference use: 'open a hole in an array' */
-    mvcrl_8(tstr + 9, tstr + 8);
+    mvcrl(tstr + 9, tstr + 8, 8);
 
     /* place missing 'i' */
     tstr[8] = 'i';
 
-    return strncmp(alpha, tstr, 16ul);
+    return strncmp(alpha, tstr, 16ul) == 0;
+}
+
+static bool test_bad_r0(void)
+{
+    char src[256];
+
+    /*
+     * PoP says: Bits 32-55 of general register 0 should contain zeros;
+     * otherwise, the program may not operate compatibly in the future.
+     *
+     * Try it anyway in order to check whether this would crash QEMU itself.
+     */
+    mvcrl(src, src, (size_t)-1);
+
+    return true;
+}
+
+int main(void)
+{
+    bool ok = true;
+
+    ok &= test();
+    ok &= test_bad_r0();
+
+    return ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }
-- 
2.41.0

Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Make the PSW look more similar to the real s390x userspace PSW.
> Except for being there, the newly added bits should not affect the
> userspace code execution.

What's the purpose of this then? Required for follow-up patches?

> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   linux-user/elfload.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 6900974c373..7935110bff4 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -1635,7 +1635,9 @@ const char *elf_hwcap_str(uint32_t bit)
>   static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
>   {
>       regs->psw.addr = infop->entry;
> -    regs->psw.mask = PSW_MASK_64 | PSW_MASK_32;
> +    regs->psw.mask = PSW_MASK_DAT | PSW_MASK_IO | PSW_MASK_EXT | \
> +                     PSW_MASK_MCHECK | PSW_MASK_PSTATE | PSW_MASK_64 | \
> +                     PSW_MASK_32;
>       regs->gprs[15] = infop->start_stack;
>   }
>   

-- 
Cheers,

David / dhildenb

Re: [PATCH 02/12] target/s390x: Fix EPSW CC reporting

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> EPSW should explicitly calculate and insert CC, like IPM does.
> 
> Fixes: e30a9d3fea58 ("target-s390: Implement EPSW")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 03/12] target/s390x: Fix MDEB and MDEBR

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> These instructions multiply 32 bits by 32 bits, not 32 bits by 64 bits.
> 
> Fixes: 83b00736f3d8 ("target-s390: Convert FP MULTIPLY")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 04/12] target/s390x: Fix MVCRL with a large value in R0

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Using a large R0 causes an assertion error:
> 
>      qemu-s390x: target/s390x/tcg/mem_helper.c:183: access_prepare_nf: Assertion `size > 0 && size <= 4096' failed.
> 
> Even though PoP explicitly advises against using more than 8 bits for the
> size, an emulator crash is never a good thing.
> 
> Fix by truncating the size to 8 bits.
> 
> Fixes: ea0a1053e276 ("s390x/tcg: Implement Miscellaneous-Instruction-Extensions Facility 3 for the s390x")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits

[Ilya Leoshkevich]

On Tue, 2023-07-04 at 09:32 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > Make the PSW look more similar to the real s390x userspace PSW.
> > Except for being there, the newly added bits should not affect the
> > userspace code execution.
> 
> What's the purpose of this then? Required for follow-up patches?

That's required for the EPSW test.
I could, of course, mask out the bits that are not emulated in the
test, but I thought it was better to make the emulation closer to
reality, if only for cosmetic purposes.

[...]

Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> When a DAT error occurs, LRA is supposed to write the error information
> to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
> 
> Fix by passing the original value of R1 into helper and copying the
> top 32 bits to the return value.
> 
> Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/helper.h         | 2 +-
>   target/s390x/tcg/mem_helper.c | 4 ++--
>   target/s390x/tcg/translate.c  | 2 +-
>   3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> index 6bc01df73d7..05102578fc9 100644
> --- a/target/s390x/helper.h
> +++ b/target/s390x/helper.h
> @@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
>   DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64, i32)
>   DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
>   DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
> -DEF_HELPER_2(lra, i64, env, i64)
> +DEF_HELPER_3(lra, i64, env, i64, i64)
>   DEF_HELPER_1(per_check_exception, void, env)
>   DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64, i64)
>   DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
> index 84ad85212c9..94d93d7ea78 100644
> --- a/target/s390x/tcg/mem_helper.c
> +++ b/target/s390x/tcg/mem_helper.c
> @@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
>   }
>   
>   /* load real address */
> -uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> +uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t addr)
>   {
>       uint64_t asc = env->psw.mask & PSW_MASK_ASC;
>       uint64_t ret, tec;
> @@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
>       exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret, &flags, &tec);
>       if (exc) {
>           cc = 3;
> -        ret = exc | 0x80000000;
> +        ret = (r1 & 0xFFFFFFFF00000000) | exc | 0x80000000;

ull missing for large constant?

>       } else {
>           cc = 0;
>           ret |= addr & ~TARGET_PAGE_MASK;
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 0cef6efbef4..a6079ab7b4f 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext *s, DisasOps *o)
>   
>   static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
>   {
> -    gen_helper_lra(o->out, cpu_env, o->in2);
> +    gen_helper_lra(o->out, cpu_env, o->out, o->in2);
>       set_cc_static(s);
>       return DISAS_NEXT;
>   }

Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe* cleaner :)

-- 
Cheers,

David / dhildenb

Re: [PATCH 01/12] linux-user: elfload: Add more initial s390x PSW bits

[David Hildenbrand]

On 04.07.23 09:40, Ilya Leoshkevich wrote:
> On Tue, 2023-07-04 at 09:32 +0200, David Hildenbrand wrote:
>> On 03.07.23 17:50, Ilya Leoshkevich wrote:
>>> Make the PSW look more similar to the real s390x userspace PSW.
>>> Except for being there, the newly added bits should not affect the
>>> userspace code execution.
>>
>> What's the purpose of this then? Required for follow-up patches?
> 
> That's required for the EPSW test.
> I could, of course, mask out the bits that are not emulated in the
> test, but I thought it was better to make the emulation closer to
> reality, if only for cosmetic purposes.

Thanks

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 06/12] target/s390x: Fix LRA when DAT is off

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> LRA should perform DAT regardless of whether it's on or off.
> Disable DAT check for MMU_S390_LRA.
> 
> Fixes: defb0e3157af ("s390x: Implement opcode helpers")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/mmu_helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> index b04b57c2356..fbb2f1b4d48 100644
> --- a/target/s390x/mmu_helper.c
> +++ b/target/s390x/mmu_helper.c
> @@ -417,7 +417,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
>   
>       vaddr &= TARGET_PAGE_MASK;
>   
> -    if (!(env->psw.mask & PSW_MASK_DAT)) {
> +    if (rw != MMU_S390_LRA && !(env->psw.mask & PSW_MASK_DAT)) {
>           *raddr = vaddr;
>           goto nodat;
>       }

Interesting

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 07/12] target/s390x: Fix relative long instructions with large offsets

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> The expression "imm * 2" in gen_ri2() can wrap around if imm is large
> enough.
> 
> Fix by casting imm to int64_t, like it's done in disas_jdest().
> 
> Fixes: e8ecdfeb30f0 ("Fix EXECUTE of relative branches")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   target/s390x/tcg/translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index a6079ab7b4f..6661b27efa4 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -5794,7 +5794,7 @@ static TCGv gen_ri2(DisasContext *s)
>   
>       disas_jdest(s, i2, is_imm, imm, ri2);
>       if (is_imm) {
> -        ri2 = tcg_constant_i64(s->base.pc_next + imm * 2);
> +        ri2 = tcg_constant_i64(s->base.pc_next + (int64_t)imm * 2);
>       }
>   
>       return ri2;

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 08/12] tests/tcg/s390x: Test EPSW

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/epsw.c          | 23 +++++++++++++++++++++++
>   2 files changed, 24 insertions(+)
>   create mode 100644 tests/tcg/s390x/epsw.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 85abfbb98c0..2ef22c88d95 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -36,6 +36,7 @@ TESTS+=rxsbg
>   TESTS+=ex-relative-long
>   TESTS+=ex-branch
>   TESTS+=mxdb
> +TESTS+=epsw
>   
>   cdsg: CFLAGS+=-pthread
>   cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/epsw.c b/tests/tcg/s390x/epsw.c
> new file mode 100644
> index 00000000000..affb1a5e3a1
> --- /dev/null
> +++ b/tests/tcg/s390x/epsw.c
> @@ -0,0 +1,23 @@
> +/*
> + * Test the EPSW instruction.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> +    unsigned long r1 = 0x1234567887654321UL, r2 = 0x8765432112345678UL;
> +
> +    asm("cr %[r1],%[r2]\n"  /* cc = 1 */
> +        "epsw %[r1],%[r2]"
> +        : [r1] "+r" (r1), [r2] "+r" (r2) : : "cc");
> +
> +    /* Do not check the R and RI bits. */
> +    r1 &= ~0x40000008UL;
> +    assert(r1 == 0x1234567807051001UL);
> +    assert(r2 == 0x8765432180000000UL);
> +
> +    return EXIT_SUCCESS;
> +}

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 09/12] tests/tcg/s390x: Test LARL with a large offset

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/larl.c          | 17 +++++++++++++++++
>   2 files changed, 18 insertions(+)
>   create mode 100644 tests/tcg/s390x/larl.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 2ef22c88d95..dbf64c991e9 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -37,6 +37,7 @@ TESTS+=ex-relative-long
>   TESTS+=ex-branch
>   TESTS+=mxdb
>   TESTS+=epsw
> +TESTS+=larl
>   
>   cdsg: CFLAGS+=-pthread
>   cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
> new file mode 100644
> index 00000000000..b9ced99a023
> --- /dev/null
> +++ b/tests/tcg/s390x/larl.c
> @@ -0,0 +1,17 @@
> +/*
> + * Test the LARL instruction.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> +    long algfi = (long)main;
> +    long larl;
> +
> +    asm("algfi %[r],0xd0000000" : [r] "+r" (algfi) : : "cc");
> +    asm("larl %[r],main+0xd0000000" : [r] "=r" (larl));

Not sure if worth combining both statements.

> +
> +    return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
> +}

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 11/12] tests/tcg/s390x: Test MDEB and MDEBR

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/Makefile.target |  1 +
>   tests/tcg/s390x/mdeb.c          | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
>   create mode 100644 tests/tcg/s390x/mdeb.c
> 
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index dbf64c991e9..19fbbc6e531 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -38,6 +38,7 @@ TESTS+=ex-branch
>   TESTS+=mxdb
>   TESTS+=epsw
>   TESTS+=larl
> +TESTS+=mdeb
>   
>   cdsg: CFLAGS+=-pthread
>   cdsg: LDFLAGS+=-pthread
> diff --git a/tests/tcg/s390x/mdeb.c b/tests/tcg/s390x/mdeb.c
> new file mode 100644
> index 00000000000..4897d28069f
> --- /dev/null
> +++ b/tests/tcg/s390x/mdeb.c
> @@ -0,0 +1,30 @@
> +/*
> + * Test the MDEB and MDEBR instructions.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +#include <assert.h>
> +#include <stdlib.h>
> +
> +int main(void)
> +{
> +    union {
> +        float f[2];
> +        double d;
> +    } a;
> +    float b;
> +
> +    a.f[0] = 1.2345;
> +    a.f[1] = 999;
> +    b = 6.789;
> +    asm("mdeb %[a],%[b]" : [a] "+f" (a.d) : [b] "R" (b));
> +    assert(a.d > 8.38 && a.d < 8.39);
> +
> +    a.f[0] = 1.2345;
> +    a.f[1] = 999;
> +    b = 6.789;
> +    asm("mdebr %[a],%[b]" : [a] "+f" (a.d) : [b] "f" (b));
> +    assert(a.d > 8.38 && a.d < 8.39);
> +
> +    return EXIT_SUCCESS;
> +}

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 09/12] tests/tcg/s390x: Test LARL with a large offset

[Ilya Leoshkevich]

On Tue, 2023-07-04 at 09:56 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > Add a small test to prevent regressions.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   tests/tcg/s390x/Makefile.target |  1 +
> >   tests/tcg/s390x/larl.c          | 17 +++++++++++++++++
> >   2 files changed, 18 insertions(+)
> >   create mode 100644 tests/tcg/s390x/larl.c
> > 
> > diff --git a/tests/tcg/s390x/Makefile.target
> > b/tests/tcg/s390x/Makefile.target
> > index 2ef22c88d95..dbf64c991e9 100644
> > --- a/tests/tcg/s390x/Makefile.target
> > +++ b/tests/tcg/s390x/Makefile.target
> > @@ -37,6 +37,7 @@ TESTS+=ex-relative-long
> >   TESTS+=ex-branch
> >   TESTS+=mxdb
> >   TESTS+=epsw
> > +TESTS+=larl
> >   
> >   cdsg: CFLAGS+=-pthread
> >   cdsg: LDFLAGS+=-pthread
> > diff --git a/tests/tcg/s390x/larl.c b/tests/tcg/s390x/larl.c
> > new file mode 100644
> > index 00000000000..b9ced99a023
> > --- /dev/null
> > +++ b/tests/tcg/s390x/larl.c
> > @@ -0,0 +1,17 @@
> > +/*
> > + * Test the LARL instruction.
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +#include <stdlib.h>
> > +
> > +int main(void)
> > +{
> > +    long algfi = (long)main;
> > +    long larl;
> > +
> > +    asm("algfi %[r],0xd0000000" : [r] "+r" (algfi) : : "cc");
> > +    asm("larl %[r],main+0xd0000000" : [r] "=r" (larl));
> 
> Not sure if worth combining both statements.

I thought it would be easier on the eyes; this way one immediately sees
that they are independent.

And maybe I should've added a comment about this, but the reason I used
algfi instead of C addition was that I feared that the compiler might
generate larl, making the test useless.

> 
> > +
> > +    return algfi == larl ? EXIT_SUCCESS : EXIT_FAILURE;
> > +}
> 
> Acked-by: David Hildenbrand <david@redhat.com>
> 

Re: [PATCH 12/12] tests/tcg/s390x: Test MVCRL with a large value in R0

[David Hildenbrand]

On 03.07.23 17:50, Ilya Leoshkevich wrote:
> Add a small test to prevent regressions.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   tests/tcg/s390x/mie3-mvcrl.c | 46 ++++++++++++++++++++++++++++--------
>   1 file changed, 36 insertions(+), 10 deletions(-)
> 
> diff --git a/tests/tcg/s390x/mie3-mvcrl.c b/tests/tcg/s390x/mie3-mvcrl.c
> index 93c7b0a2903..ec78dd1d493 100644
> --- a/tests/tcg/s390x/mie3-mvcrl.c
> +++ b/tests/tcg/s390x/mie3-mvcrl.c
> @@ -1,29 +1,55 @@
> +#include <stdbool.h>
>   #include <stdint.h>
> +#include <stdlib.h>
>   #include <string.h>
>   
> -
> -static inline void mvcrl_8(const char *dst, const char *src)
> +static void mvcrl(const char *dst, const char *src, size_t len)
>   {
> +    register long r0 asm("r0") = len;
> +
>       asm volatile (
> -        "llill %%r0, 8\n"
>           ".insn sse, 0xE50A00000000, 0(%[dst]), 0(%[src])"
> -        : : [dst] "d" (dst), [src] "d" (src)
> -        : "r0", "memory");
> +        : : [dst] "d" (dst), [src] "d" (src), "r" (r0)
> +        : "memory");
>   }
>   
> -
> -int main(int argc, char *argv[])
> +static bool test(void)
>   {
>       const char *alpha = "abcdefghijklmnop";
>   
>       /* array missing 'i' */
> -    char tstr[17] = "abcdefghjklmnop\0" ;
> +    char tstr[17] = "abcdefghjklmnop\0";
>   
>       /* mvcrl reference use: 'open a hole in an array' */
> -    mvcrl_8(tstr + 9, tstr + 8);
> +    mvcrl(tstr + 9, tstr + 8, 8);
>   
>       /* place missing 'i' */
>       tstr[8] = 'i';
>   
> -    return strncmp(alpha, tstr, 16ul);
> +    return strncmp(alpha, tstr, 16ul) == 0;
> +}
> +
> +static bool test_bad_r0(void)
> +{
> +    char src[256];
> +
> +    /*
> +     * PoP says: Bits 32-55 of general register 0 should contain zeros;
> +     * otherwise, the program may not operate compatibly in the future.
> +     *
> +     * Try it anyway in order to check whether this would crash QEMU itself.
> +     */
> +    mvcrl(src, src, (size_t)-1);
> +
> +    return true;
> +}
> +
> +int main(void)
> +{
> +    bool ok = true;
> +
> +    ok &= test();
> +    ok &= test_bad_r0();
> +
> +    return ok ? EXIT_SUCCESS : EXIT_FAILURE;
>   }

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

-- 
Cheers,

David / dhildenb

Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error

[Ilya Leoshkevich]

On Tue, 2023-07-04 at 09:47 +0200, David Hildenbrand wrote:
> On 03.07.23 17:50, Ilya Leoshkevich wrote:
> > When a DAT error occurs, LRA is supposed to write the error
> > information
> > to the bottom 32 bits of R1, and leave the top 32 bits of R1 alone.
> > 
> > Fix by passing the original value of R1 into helper and copying the
> > top 32 bits to the return value.
> > 
> > Fixes: d8fe4a9c284f ("target-s390: Convert LRA")
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   target/s390x/helper.h         | 2 +-
> >   target/s390x/tcg/mem_helper.c | 4 ++--
> >   target/s390x/tcg/translate.c  | 2 +-
> >   3 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/target/s390x/helper.h b/target/s390x/helper.h
> > index 6bc01df73d7..05102578fc9 100644
> > --- a/target/s390x/helper.h
> > +++ b/target/s390x/helper.h
> > @@ -355,7 +355,7 @@ DEF_HELPER_FLAGS_4(idte, TCG_CALL_NO_RWG, void,
> > env, i64, i64, i32)
> >   DEF_HELPER_FLAGS_4(ipte, TCG_CALL_NO_RWG, void, env, i64, i64,
> > i32)
> >   DEF_HELPER_FLAGS_1(ptlb, TCG_CALL_NO_RWG, void, env)
> >   DEF_HELPER_FLAGS_1(purge, TCG_CALL_NO_RWG, void, env)
> > -DEF_HELPER_2(lra, i64, env, i64)
> > +DEF_HELPER_3(lra, i64, env, i64, i64)
> >   DEF_HELPER_1(per_check_exception, void, env)
> >   DEF_HELPER_FLAGS_3(per_branch, TCG_CALL_NO_RWG, void, env, i64,
> > i64)
> >   DEF_HELPER_FLAGS_2(per_ifetch, TCG_CALL_NO_RWG, void, env, i64)
> > diff --git a/target/s390x/tcg/mem_helper.c
> > b/target/s390x/tcg/mem_helper.c
> > index 84ad85212c9..94d93d7ea78 100644
> > --- a/target/s390x/tcg/mem_helper.c
> > +++ b/target/s390x/tcg/mem_helper.c
> > @@ -2356,7 +2356,7 @@ void HELPER(purge)(CPUS390XState *env)
> >   }
> >   
> >   /* load real address */
> > -uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
> > +uint64_t HELPER(lra)(CPUS390XState *env, uint64_t r1, uint64_t
> > addr)
> >   {
> >       uint64_t asc = env->psw.mask & PSW_MASK_ASC;
> >       uint64_t ret, tec;
> > @@ -2370,7 +2370,7 @@ uint64_t HELPER(lra)(CPUS390XState *env,
> > uint64_t addr)
> >       exc = mmu_translate(env, addr, MMU_S390_LRA, asc, &ret,
> > &flags, &tec);
> >       if (exc) {
> >           cc = 3;
> > -        ret = exc | 0x80000000;
> > +        ret = (r1 & 0xFFFFFFFF00000000) | exc | 0x80000000;
> 
> ull missing for large constant?

Will do.

Just for my understanding, why is this necessary?
The current code base tends towards using ULL, but it's a little bit
inconsistent:

$ git grep -i 0xfffffffff | wc -l
2338
$ git grep -i 0xfffffffff | grep -i -v ul | wc -l
95


> 
> >       } else {
> >           cc = 0;
> >           ret |= addr & ~TARGET_PAGE_MASK;
> > diff --git a/target/s390x/tcg/translate.c
> > b/target/s390x/tcg/translate.c
> > index 0cef6efbef4..a6079ab7b4f 100644
> > --- a/target/s390x/tcg/translate.c
> > +++ b/target/s390x/tcg/translate.c
> > @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext
> > *s, DisasOps *o)
> >   
> >   static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
> >   {
> > -    gen_helper_lra(o->out, cpu_env, o->in2);
> > +    gen_helper_lra(o->out, cpu_env, o->out, o->in2);
> >       set_cc_static(s);
> >       return DISAS_NEXT;
> >   }
> 
> Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe*
> cleaner :)
> 

The problem is that we want all 64 bits for the non-error case.

Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error

[Richard Henderson]

On 7/4/23 10:05, Ilya Leoshkevich wrote:
>>> +        ret = (r1 & 0xFFFFFFFF00000000) | exc | 0x80000000;
>>
>> ull missing for large constant?
> 
> Will do.
> 
> Just for my understanding, why is this necessary?

32-bit host; you'll get a warning for the large constant.


r~

Re: [PATCH 05/12] target/s390x: Fix LRA overwriting the top 32 bits on DAT error

[David Hildenbrand]

>>
>>>        } else {
>>>            cc = 0;
>>>            ret |= addr & ~TARGET_PAGE_MASK;
>>> diff --git a/target/s390x/tcg/translate.c
>>> b/target/s390x/tcg/translate.c
>>> index 0cef6efbef4..a6079ab7b4f 100644
>>> --- a/target/s390x/tcg/translate.c
>>> +++ b/target/s390x/tcg/translate.c
>>> @@ -2932,7 +2932,7 @@ static DisasJumpType op_lctlg(DisasContext
>>> *s, DisasOps *o)
>>>    
>>>    static DisasJumpType op_lra(DisasContext *s, DisasOps *o)
>>>    {
>>> -    gen_helper_lra(o->out, cpu_env, o->in2);
>>> +    gen_helper_lra(o->out, cpu_env, o->out, o->in2);
>>>        set_cc_static(s);
>>>        return DISAS_NEXT;
>>>    }
>>
>> Can't we use something like in1_r1 + wout_r1_32 instead ? *maybe*
>> cleaner :)
>>
> 
> The problem is that we want all 64 bits for the non-error case.
> 

Ah, I missed that detail, thanks.

-- 
Cheers,

David / dhildenb