[PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12

[PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12

[David Hildenbrand]

This series fixes booting current upstream Linux kernel compiled by
clang-11 and clang-12 under TCG.

Decided to pull in already separatly sent patches. The last patch is
not required to fix the boot issues, but related to patch #3.

Latest version of the patches available at:
git@github.com:davidhildenbrand/qemu.git clang

v1 -> v2:
- Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or
  "x"'
- Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling
  EXEUTE'
- "s390x/tcg: Fix ALGSI"
-- Fixup subject
- "s390x/tcg: Fix RISBHG"
-- Rephrase description, stating that it fixes clang-11

David Hildenbrand (4):
  s390x/tcg: Fix ALGSI
  s390x/tcg: Fix RISBHG
  s390x/tcg: Only ignore content in r0 when specified via "b" or "x"
  s390x/tcg: Ignore register content if b1/b2 is zero when handling
    EXECUTE

 target/s390x/insn-data.def | 10 +++++-----
 target/s390x/mem_helper.c  |  4 ++--
 target/s390x/translate.c   | 33 +++++++++++++++++----------------
 3 files changed, 24 insertions(+), 23 deletions(-)

-- 
2.29.2

[PATCH v2 1/4] s390x/tcg: Fix ALGSI

[David Hildenbrand]

Looks like something went wrong whiel touching that line. Instead of "r1"
we need a new temporary. Also, we have to pass MO_TEQ, to indicate that
we are working with 64-bit values. Let's revert these changes.

Fixes: ff26d287bddc ("target/s390x: Improve cc computation for ADD LOGICAL")
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index 26badb663a..eac5136ee5 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -76,7 +76,7 @@
 /* ADD LOGICAL WITH SIGNED IMMEDIATE */
     D(0xeb6e, ALSI,    SIY,   GIE, la1, i2_32u, new, 0, asi, addu32, MO_TEUL)
     C(0xecda, ALHSIK,  RIE_d, DO,  r3_32u, i2_32u, new, r1_32, add, addu32)
-    C(0xeb7e, ALGSI,   SIY,   GIE, la1, i2, r1, 0, asiu64, addu64)
+    D(0xeb7e, ALGSI,   SIY,   GIE, la1, i2, new, 0, asiu64, addu64, MO_TEQ)
     C(0xecdb, ALGHSIK, RIE_d, DO,  r3, i2, r1, 0, addu64, addu64)
 /* ADD LOGICAL WITH SIGNED IMMEDIATE HIGH */
     C(0xcc0a, ALSIH,   RIL_a, HW,  r1_sr32, i2_32u, new, r1_32h, add, addu32)
-- 
2.29.2

[PATCH v2 2/4] s390x/tcg: Fix RISBHG

[David Hildenbrand]

RISBHG is broken and currently hinders clang-11 builds of upstream kernels
from booting: the kernel crashes early, while decompressing the image.

  [...]
   Kernel fault: interruption code 0005 ilc:2
   Kernel random base: 0000000000000000
   PSW : 0000200180000000 0000000000017a1e
         R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
   GPRS: 0000000000000001 0000000c00000000 00000003fffffff4 00000000fffffff0
         0000000000000000 00000000fffffff4 000000000000000c 00000000fffffff0
         00000000fffffffc 0000000000000000 00000000fffffff8 00000000008e25a8
         0000000000000009 0000000000000002 0000000000000008 000000000000bce0

One example of a buggy instruction is:

    17dde:       ec 1e 00 9f 20 5d       risbhg  %r1,%r14,0,159,32

With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x900000007, however,
results in %r1 = 0.

Let's interpret values of i3/i4 as documented in the PoP and make
computation of "mask" only based on i3 and i4 and use "pmask" only at the
very end to make sure wrapping is only applied to the high/low doubleword.

With this patch, I can successfully boot a v5.11-rc2 kernel built with
clang-11, and gcc builds keep on working.

Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/translate.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 3d5c0d6106..39e33eeb67 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3815,22 +3815,23 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps *o)
         pmask = 0xffffffff00000000ull;
         break;
     case 0x51: /* risblg */
-        i3 &= 31;
-        i4 &= 31;
+        i3 = (i3 & 31) + 32;
+        i4 = (i4 & 31) + 32;
         pmask = 0x00000000ffffffffull;
         break;
     default:
         g_assert_not_reached();
     }
 
-    /* MASK is the set of bits to be inserted from R2.
-       Take care for I3/I4 wraparound.  */
-    mask = pmask >> i3;
+    /* MASK is the set of bits to be inserted from R2. */
     if (i3 <= i4) {
-        mask ^= pmask >> i4 >> 1;
+        /* [0...i3---i4...63] */
+        mask = (-1ull >> i3) & (-1ull << (63 - i4));
     } else {
-        mask |= ~(pmask >> i4 >> 1);
+        /* [0---i4...i3---63] */
+        mask = (-1ull >> i3) | (-1ull << (63 - i4));
     }
+    /* For RISBLG/RISBHG, the wrapping is limited to the high/low doubleword. */
     mask &= pmask;
 
     /* IMASK is the set of bits to be kept from R1.  In the case of the high/low
@@ -3843,9 +3844,6 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps *o)
     len = i4 - i3 + 1;
     pos = 63 - i4;
     rot = i5 & 63;
-    if (s->fields.op2 == 0x5d) {
-        pos += 32;
-    }
 
     /* In some cases we can implement this with extract.  */
     if (imask == 0 && pos == 0 && len > 0 && len <= rot) {
-- 
2.29.2

[PATCH v2 3/4] s390x/tcg: Only ignore content in r0 when specified via "b" or "x"

[David Hildenbrand]

Using get_address() with register identifiers comming from an "r" field
is wrong: if the "r" field designates "r0", we don't read the content
and instead assume 0 - which should only be applied when the register
was specified via "b" or "x".

PoP 5-11 "Operand-Address Generation":
  "A zero in any of the B1, B2, X2, B3, or B4 fields indicates the absence
   of the corresponding address component. For the absent component, a zero
   is used in forming the intermediate sum, regardless of the contents of
   general register 0. A displacement of zero has no special significance."

This BUG became visible for CSPG as generated by LLVM-12 in the upstream
Linux kernel (v5.11-rc2), used while creating the linear mapping in
vmem_map_init(): Trying to store to address 0 results in a Low Address
Protection exception.

Debugging this was more complicated than it could have been: The program
interrupt handler in the kernel will try to crash the kernel: doing so, it
will enable DAT. As the linear mapping is not created yet (asce=0), we run
into an addressing exception while tring to walk non-existant DAT tables,
resulting in a program exception loop.

This allows for booting upstream Linux kernels compiled by clang-12. Most
of these cases seem to be broken forever.

Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/insn-data.def |  8 ++++----
 target/s390x/translate.c   | 15 +++++++++------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/target/s390x/insn-data.def b/target/s390x/insn-data.def
index eac5136ee5..e5b6efabf3 100644
--- a/target/s390x/insn-data.def
+++ b/target/s390x/insn-data.def
@@ -1290,8 +1290,8 @@
     F(0xe313, LRAY,    RXY_a, LD,  0, a2, r1, 0, lra, 0, IF_PRIV)
     F(0xe303, LRAG,    RXY_a, Z,   0, a2, r1, 0, lra, 0, IF_PRIV)
 /* LOAD USING REAL ADDRESS */
-    E(0xb24b, LURA,    RRE,   Z,   0, 0, new, r1_32, lura, 0, MO_TEUL, IF_PRIV)
-    E(0xb905, LURAG,   RRE,   Z,   0, 0, r1, 0, lura, 0, MO_TEQ, IF_PRIV)
+    E(0xb24b, LURA,    RRE,   Z,   0, ra2, new, r1_32, lura, 0, MO_TEUL, IF_PRIV)
+    E(0xb905, LURAG,   RRE,   Z,   0, ra2, r1, 0, lura, 0, MO_TEQ, IF_PRIV)
 /* MOVE TO PRIMARY */
     F(0xda00, MVCP,    SS_d,  Z,   la1, a2, 0, 0, mvcp, 0, IF_PRIV)
 /* MOVE TO SECONDARY */
@@ -1344,8 +1344,8 @@
 /* STORE THEN OR SYSTEM MASK */
     F(0xad00, STOSM,   SI,    Z,   la1, 0, 0, 0, stnosm, 0, IF_PRIV)
 /* STORE USING REAL ADDRESS */
-    E(0xb246, STURA,   RRE,   Z,   r1_o, 0, 0, 0, stura, 0, MO_TEUL, IF_PRIV)
-    E(0xb925, STURG,   RRE,   Z,   r1_o, 0, 0, 0, stura, 0, MO_TEQ, IF_PRIV)
+    E(0xb246, STURA,   RRE,   Z,   r1_o, ra2, 0, 0, stura, 0, MO_TEUL, IF_PRIV)
+    E(0xb925, STURG,   RRE,   Z,   r1_o, ra2, 0, 0, stura, 0, MO_TEQ, IF_PRIV)
 /* TEST BLOCK */
     F(0xb22c, TB,      RRE,   Z,   0, r2_o, 0, 0, testblock, 0, IF_PRIV)
 /* TEST PROTECTION */
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 39e33eeb67..61dd0341e4 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -3285,8 +3285,7 @@ static DisasJumpType op_lpq(DisasContext *s, DisasOps *o)
 #ifndef CONFIG_USER_ONLY
 static DisasJumpType op_lura(DisasContext *s, DisasOps *o)
 {
-    o->addr1 = get_address(s, 0, get_field(s, r2), 0);
-    tcg_gen_qemu_ld_tl(o->out, o->addr1, MMU_REAL_IDX, s->insn->data);
+    tcg_gen_qemu_ld_tl(o->out, o->in2, MMU_REAL_IDX, s->insn->data);
     return DISAS_NEXT;
 }
 #endif
@@ -4234,7 +4233,8 @@ static DisasJumpType op_ectg(DisasContext *s, DisasOps *o)
     tcg_gen_addi_i64(o->in1, regs[b1], d1);
     o->in2 = tcg_temp_new_i64();
     tcg_gen_addi_i64(o->in2, regs[b2], d2);
-    o->addr1 = get_address(s, 0, r3, 0);
+    o->addr1 = tcg_temp_new_i64();
+    gen_addi_and_wrap_i64(s, o->addr1, regs[r3], 0);
 
     /* load the third operand into r3 before modifying anything */
     tcg_gen_qemu_ld64(regs[r3], o->addr1, get_mem_index(s));
@@ -4539,8 +4539,7 @@ static DisasJumpType op_stnosm(DisasContext *s, DisasOps *o)
 
 static DisasJumpType op_stura(DisasContext *s, DisasOps *o)
 {
-    o->addr1 = get_address(s, 0, get_field(s, r2), 0);
-    tcg_gen_qemu_st_tl(o->in1, o->addr1, MMU_REAL_IDX, s->insn->data);
+    tcg_gen_qemu_st_tl(o->in1, o->in2, MMU_REAL_IDX, s->insn->data);
 
     if (s->base.tb->flags & FLAG_MASK_PER) {
         update_psw_addr(s);
@@ -5923,7 +5922,11 @@ static void in2_x2l(DisasContext *s, DisasOps *o)
 
 static void in2_ra2(DisasContext *s, DisasOps *o)
 {
-    o->in2 = get_address(s, 0, get_field(s, r2), 0);
+    int r2 = get_field(s, r2);
+
+    /* Note: *don't* treat !r2 as 0, use the reg value. */
+    o->in2 = tcg_temp_new_i64();
+    gen_addi_and_wrap_i64(s, o->in2, regs[r2], 0);
 }
 #define SPEC_in2_ra2 0
 
-- 
2.29.2

[PATCH v2 4/4] s390x/tcg: Ignore register content if b1/b2 is zero when handling EXECUTE

[David Hildenbrand]

In our EXECUTE fast path, we have to ignore the content of r0, if
specified by b1 or b2.

Fixes: d376f123c7de ("target/s390x: Re-implement a few EXECUTE target insns directly")
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mem_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 0108611cc9..1901e9dfc7 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2473,8 +2473,8 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
             uint32_t d1 = extract64(insn, 32, 12);
             uint32_t b2 = extract64(insn, 28, 4);
             uint32_t d2 = extract64(insn, 16, 12);
-            uint64_t a1 = wrap_address(env, env->regs[b1] + d1);
-            uint64_t a2 = wrap_address(env, env->regs[b2] + d2);
+            uint64_t a1 = wrap_address(env, (b1 ? env->regs[b1] : 0) + d1);
+            uint64_t a2 = wrap_address(env, (b2 ? env->regs[b2] : 0) + d2);
 
             env->cc_op = helper(env, l, a1, a2, 0);
             env->psw.addr += ilen;
-- 
2.29.2

Re: [PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12

[Nick Desaulniers via]

On Fri, Jan 8, 2021 at 5:21 AM David Hildenbrand <david@redhat.com> wrote:
>
> This series fixes booting current upstream Linux kernel compiled by
> clang-11 and clang-12 under TCG.
>
> Decided to pull in already separatly sent patches. The last patch is
> not required to fix the boot issues, but related to patch #3.
>
> Latest version of the patches available at:
> git@github.com:davidhildenbrand/qemu.git clang

Hey looks like we're off to the races!

$ qemu/build/qemu-system-s390x -M s390-ccw-virtio -display none
-initrd /android1/boot-utils/images/s390/rootfs.cpio -kernel
/android0/linux-next/arch/s390/boot/bzImage -m 512m -nodefaults
-serial mon:stdio
...
[    0.365077] Linux version 5.11.0-rc2-01914-g16586f130181-dirty
(ndesaulniers@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers
clang version 12.0.0 (git@github.com:llvm/llvm-project.git
e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for
Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021
...
/ # cat /proc/version
Linux version 5.11.0-rc2-01914-g16586f130181-dirty
(ndesaulniers@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers
clang version 12.0.0 (git@github.com:llvm/llvm-project.git
e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for
Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021
/ # uname -a
Linux (none) 5.11.0-rc2-01914-g16586f130181-dirty #76 SMP Thu Jan 7
17:51:34 PST 2021 s390x GNU/Linux

For the series:
Tested-by: Nick Desaulniers <ndesaulniers@google.com>


>
> v1 -> v2:
> - Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or
>   "x"'
> - Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling
>   EXEUTE'
> - "s390x/tcg: Fix ALGSI"
> -- Fixup subject
> - "s390x/tcg: Fix RISBHG"
> -- Rephrase description, stating that it fixes clang-11
>
> David Hildenbrand (4):
>   s390x/tcg: Fix ALGSI
>   s390x/tcg: Fix RISBHG
>   s390x/tcg: Only ignore content in r0 when specified via "b" or "x"
>   s390x/tcg: Ignore register content if b1/b2 is zero when handling
>     EXECUTE
>
>  target/s390x/insn-data.def | 10 +++++-----
>  target/s390x/mem_helper.c  |  4 ++--
>  target/s390x/translate.c   | 33 +++++++++++++++++----------------
>  3 files changed, 24 insertions(+), 23 deletions(-)
>
> --
> 2.29.2
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12

[Guenter Roeck]

On 1/8/21 5:20 AM, David Hildenbrand wrote:
> This series fixes booting current upstream Linux kernel compiled by
> clang-11 and clang-12 under TCG.
> 
> Decided to pull in already separatly sent patches. The last patch is
> not required to fix the boot issues, but related to patch #3.
> 
> Latest version of the patches available at:
> git@github.com:davidhildenbrand/qemu.git clang
> 
> v1 -> v2:
> - Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or
>   "x"'
> - Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling
>   EXEUTE'
> - "s390x/tcg: Fix ALGSI"
> -- Fixup subject
> - "s390x/tcg: Fix RISBHG"
> -- Rephrase description, stating that it fixes clang-11
> 
> David Hildenbrand (4):
>   s390x/tcg: Fix ALGSI
>   s390x/tcg: Fix RISBHG
>   s390x/tcg: Only ignore content in r0 when specified via "b" or "x"
>   s390x/tcg: Ignore register content if b1/b2 is zero when handling
>     EXECUTE
> 
>  target/s390x/insn-data.def | 10 +++++-----
>  target/s390x/mem_helper.c  |  4 ++--
>  target/s390x/translate.c   | 33 +++++++++++++++++----------------
>  3 files changed, 24 insertions(+), 23 deletions(-)
> 

FWIW, for the series, with gcc 8.3.0 and 10.2.0, booting Linux kernel
v5.11-rc2-178-gf5e6c330254a:

Tested-by: Guenter Roeck <linux@roeck-us.net>

Guenter

Re: [PATCH v2 1/4] s390x/tcg: Fix ALGSI

[Richard Henderson]

On 1/8/21 3:20 AM, David Hildenbrand wrote:
> Looks like something went wrong whiel touching that line. Instead of "r1"
> we need a new temporary. Also, we have to pass MO_TEQ, to indicate that
> we are working with 64-bit values. Let's revert these changes.
> 
> Fixes: ff26d287bddc ("target/s390x: Improve cc computation for ADD LOGICAL")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Oops.  Sorry about that.

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

r~

Re: [PATCH v2 2/4] s390x/tcg: Fix RISBHG

[Richard Henderson]

On 1/8/21 3:20 AM, David Hildenbrand wrote:
> RISBHG is broken and currently hinders clang-11 builds of upstream kernels
> from booting: the kernel crashes early, while decompressing the image.
> 
>   [...]
>    Kernel fault: interruption code 0005 ilc:2
>    Kernel random base: 0000000000000000
>    PSW : 0000200180000000 0000000000017a1e
>          R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
>    GPRS: 0000000000000001 0000000c00000000 00000003fffffff4 00000000fffffff0
>          0000000000000000 00000000fffffff4 000000000000000c 00000000fffffff0
>          00000000fffffffc 0000000000000000 00000000fffffff8 00000000008e25a8
>          0000000000000009 0000000000000002 0000000000000008 000000000000bce0
> 
> One example of a buggy instruction is:
> 
>     17dde:       ec 1e 00 9f 20 5d       risbhg  %r1,%r14,0,159,32
> 
> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x900000007, however,
> results in %r1 = 0.
> 
> Let's interpret values of i3/i4 as documented in the PoP and make
> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> very end to make sure wrapping is only applied to the high/low doubleword.
> 
> With this patch, I can successfully boot a v5.11-rc2 kernel built with
> clang-11, and gcc builds keep on working.
> 
> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/translate.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)

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

r~

Re: [PATCH v2 3/4] s390x/tcg: Only ignore content in r0 when specified via "b" or "x"

[Richard Henderson]

On 1/8/21 3:20 AM, David Hildenbrand wrote:
> Using get_address() with register identifiers comming from an "r" field
> is wrong: if the "r" field designates "r0", we don't read the content
> and instead assume 0 - which should only be applied when the register
> was specified via "b" or "x".
> 
> PoP 5-11 "Operand-Address Generation":
>   "A zero in any of the B1, B2, X2, B3, or B4 fields indicates the absence
>    of the corresponding address component. For the absent component, a zero
>    is used in forming the intermediate sum, regardless of the contents of
>    general register 0. A displacement of zero has no special significance."
> 
> This BUG became visible for CSPG as generated by LLVM-12 in the upstream
> Linux kernel (v5.11-rc2), used while creating the linear mapping in
> vmem_map_init(): Trying to store to address 0 results in a Low Address
> Protection exception.
> 
> Debugging this was more complicated than it could have been: The program
> interrupt handler in the kernel will try to crash the kernel: doing so, it
> will enable DAT. As the linear mapping is not created yet (asce=0), we run
> into an addressing exception while tring to walk non-existant DAT tables,
> resulting in a program exception loop.
> 
> This allows for booting upstream Linux kernels compiled by clang-12. Most
> of these cases seem to be broken forever.
> 
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/insn-data.def |  8 ++++----
>  target/s390x/translate.c   | 15 +++++++++------
>  2 files changed, 13 insertions(+), 10 deletions(-)

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

r~

Re: [PATCH v2 4/4] s390x/tcg: Ignore register content if b1/b2 is zero when handling EXECUTE

[Richard Henderson]

On 1/8/21 3:20 AM, David Hildenbrand wrote:
> In our EXECUTE fast path, we have to ignore the content of r0, if
> specified by b1 or b2.
> 
> Fixes: d376f123c7de ("target/s390x: Re-implement a few EXECUTE target insns directly")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  target/s390x/mem_helper.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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

r~

Re: [PATCH v2 4/4] s390x/tcg: Ignore register content if b1/b2 is zero when handling EXECUTE

[Thomas Huth]

On 08/01/2021 14.20, David Hildenbrand wrote:
> In our EXECUTE fast path, we have to ignore the content of r0, if
> specified by b1 or b2.
> 
> Fixes: d376f123c7de ("target/s390x: Re-implement a few EXECUTE target insns directly")
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>   target/s390x/mem_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
> index 0108611cc9..1901e9dfc7 100644
> --- a/target/s390x/mem_helper.c
> +++ b/target/s390x/mem_helper.c
> @@ -2473,8 +2473,8 @@ void HELPER(ex)(CPUS390XState *env, uint32_t ilen, uint64_t r1, uint64_t addr)
>               uint32_t d1 = extract64(insn, 32, 12);
>               uint32_t b2 = extract64(insn, 28, 4);
>               uint32_t d2 = extract64(insn, 16, 12);
> -            uint64_t a1 = wrap_address(env, env->regs[b1] + d1);
> -            uint64_t a2 = wrap_address(env, env->regs[b2] + d2);
> +            uint64_t a1 = wrap_address(env, (b1 ? env->regs[b1] : 0) + d1);
> +            uint64_t a2 = wrap_address(env, (b2 ? env->regs[b2] : 0) + d2);

Right, "B" fields need special handling.

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

Re: [PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12

[David Hildenbrand]

On 08.01.21 20:21, Guenter Roeck wrote:
> On 1/8/21 5:20 AM, David Hildenbrand wrote:
>> This series fixes booting current upstream Linux kernel compiled by
>> clang-11 and clang-12 under TCG.
>>
>> Decided to pull in already separatly sent patches. The last patch is
>> not required to fix the boot issues, but related to patch #3.
>>
>> Latest version of the patches available at:
>> git@github.com:davidhildenbrand/qemu.git clang
>>
>> v1 -> v2:
>> - Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or
>>   "x"'
>> - Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling
>>   EXEUTE'
>> - "s390x/tcg: Fix ALGSI"
>> -- Fixup subject
>> - "s390x/tcg: Fix RISBHG"
>> -- Rephrase description, stating that it fixes clang-11
>>
>> David Hildenbrand (4):
>>   s390x/tcg: Fix ALGSI
>>   s390x/tcg: Fix RISBHG
>>   s390x/tcg: Only ignore content in r0 when specified via "b" or "x"
>>   s390x/tcg: Ignore register content if b1/b2 is zero when handling
>>     EXECUTE
>>
>>  target/s390x/insn-data.def | 10 +++++-----
>>  target/s390x/mem_helper.c  |  4 ++--
>>  target/s390x/translate.c   | 33 +++++++++++++++++----------------
>>  3 files changed, 24 insertions(+), 23 deletions(-)
>>
> 
> FWIW, for the series, with gcc 8.3.0 and 10.2.0, booting Linux kernel
> v5.11-rc2-178-gf5e6c330254a:
> 
> Tested-by: Guenter Roeck <linux@roeck-us.net>
> 
> Guenter
> 

Thanks Guenter, appreciated!

-- 
Thanks,

David / dhildenb