From: David Hildenbrand <david@redhat.com>
To: qemu-devel@nongnu.org
Cc: Thomas Huth <thuth@redhat.com>,
David Hildenbrand <david@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
Heiko Carstens <hca@linux.ibm.com>,
Cornelia Huck <cohuck@redhat.com>,
Nick Desaulniers <ndesaulniers@google.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
qemu-s390x@nongnu.org, Guenter Roeck <linux@roeck-us.net>
Subject: [PATCH v3 3/5] s390x/tcg: Don't ignore content in r0 when not specified via "b" or "x"
Date: Mon, 11 Jan 2021 17:38:43 +0100 [thread overview]
Message-ID: <20210111163845.18148-4-david@redhat.com> (raw)
In-Reply-To: <20210111163845.18148-1-david@redhat.com>
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>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
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
next prev parent reply other threads:[~2021-01-11 16:42 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 16:38 [PATCH v3 0/5] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12 David Hildenbrand
2021-01-11 16:38 ` [PATCH v3 1/5] s390x/tcg: Fix ALGSI David Hildenbrand
2021-01-11 16:38 ` [PATCH v3 2/5] s390x/tcg: Fix RISBHG David Hildenbrand
2021-01-11 16:38 ` David Hildenbrand [this message]
2021-01-11 16:38 ` [PATCH v3 4/5] tests/tcg/s390x: Fix EXRL tests David Hildenbrand
2021-01-12 7:41 ` Thomas Huth
2021-01-12 7:47 ` David Hildenbrand
2021-01-12 8:16 ` Thomas Huth
2021-01-12 10:04 ` David Hildenbrand
2021-01-11 16:38 ` [PATCH v3 5/5] s390x/tcg: Ignore register content if b1/b2 is zero when handling EXECUTE David Hildenbrand
2021-01-12 10:40 ` [PATCH v3 0/5] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12 Cornelia Huck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210111163845.18148-4-david@redhat.com \
--to=david@redhat.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=hca@linux.ibm.com \
--cc=linux@roeck-us.net \
--cc=ndesaulniers@google.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.