All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ilya Leoshkevich <iii@linux.ibm.com>
To: "Laurent Vivier" <laurent@vivier.eu>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Yanan Wang" <wangyanan55@huawei.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"David Hildenbrand" <david@redhat.com>,
	"Peter Maydell" <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org, qemu-s390x@nongnu.org,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Ilya Leoshkevich <iii@linux.ibm.com>
Subject: [PATCH v2 2/4] target/s390x: Make translator stop before the end of a page
Date: Fri,  5 Aug 2022 18:09:12 +0200	[thread overview]
Message-ID: <20220805160914.1106091-3-iii@linux.ibm.com> (raw)
In-Reply-To: <20220805160914.1106091-1-iii@linux.ibm.com>

Right now translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 include/exec/translator.h    | 10 ++++++++++
 target/s390x/tcg/translate.c | 35 ++++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index 7db6845535..d27f8c33b6 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -187,4 +187,14 @@ FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
 
 #undef GEN_TRANSLATOR_LD
 
+/*
+ * Return whether addr is on the same page as where disassembly started.
+ * Translators can use this to enforce the rule that only single-insn
+ * translation blocks are allowed to cross page boundaries.
+ */
+static inline bool is_same_page(DisasContextBase *db, target_ulong addr)
+{
+    return ((addr ^ db->pc_first) & TARGET_PAGE_MASK) == 0;
+}
+
 #endif /* EXEC__TRANSLATOR_H */
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index e2ee005671..0cd0c932fb 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6305,14 +6305,13 @@ static void extract_field(DisasFields *o, const DisasField *f, uint64_t insn)
     o->c[f->indexC] = r;
 }
 
-/* Lookup the insn at the current PC, extracting the operands into O and
-   returning the info struct for the insn.  Returns NULL for invalid insn.  */
+/* Lookup the insn at the current PC, filling the info struct.  */
 
-static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
+static DisasJumpType extract_insn(CPUS390XState *env, DisasContext *s,
+                                  const DisasInsn **info)
 {
     uint64_t insn, pc = s->base.pc_next;
     int op, op2, ilen;
-    const DisasInsn *info;
 
     if (unlikely(s->ex_value)) {
         /* Drop the EX data now, so that it's clear on exception paths.  */
@@ -6325,9 +6324,13 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
         ilen = s->ex_value & 0xf;
         op = insn >> 56;
     } else {
+        assert(s->base.num_insns == 1 || is_same_page(&s->base, pc));
         insn = ld_code2(env, s, pc);
         op = (insn >> 8) & 0xff;
         ilen = get_ilen(op);
+        if (s->base.num_insns > 1 && !is_same_page(&s->base, pc + ilen - 1)) {
+            return DISAS_TOO_MANY;
+        }
         switch (ilen) {
         case 2:
             insn = insn << 48;
@@ -6394,19 +6397,19 @@ static const DisasInsn *extract_insn(CPUS390XState *env, DisasContext *s)
     s->fields.op2 = op2;
 
     /* Lookup the instruction.  */
-    info = lookup_opc(op << 8 | op2);
-    s->insn = info;
+    *info = lookup_opc(op << 8 | op2);
+    s->insn = *info;
 
     /* If we found it, extract the operands.  */
-    if (info != NULL) {
-        DisasFormat fmt = info->fmt;
+    if (*info != NULL) {
+        DisasFormat fmt = (*info)->fmt;
         int i;
 
         for (i = 0; i < NUM_C_FIELD; ++i) {
             extract_field(&s->fields, &format_info[fmt].op[i], insn);
         }
     }
-    return info;
+    return DISAS_NEXT;
 }
 
 static bool is_afp_reg(int reg)
@@ -6423,12 +6426,17 @@ static bool is_fp_pair(int reg)
 static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
 {
     const DisasInsn *insn;
-    DisasJumpType ret = DISAS_NEXT;
+    DisasJumpType ret;
     DisasOps o = {};
     bool icount = false;
 
     /* Search for the insn in the table.  */
-    insn = extract_insn(env, s);
+    ret = extract_insn(env, s, &insn);
+
+    /* This is a subsequent insn that crosses a page boundary.  */
+    if (ret == DISAS_TOO_MANY) {
+        goto out;
+    }
 
     /* Update insn_start now that we know the ILEN.  */
     tcg_set_insn_start_param(s->insn_start, 2, s->ilen);
@@ -6616,10 +6624,7 @@ static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 
     dc->base.is_jmp = translate_one(env, dc);
     if (dc->base.is_jmp == DISAS_NEXT) {
-        uint64_t page_start;
-
-        page_start = dc->base.pc_first & TARGET_PAGE_MASK;
-        if (dc->base.pc_next - page_start >= TARGET_PAGE_SIZE || dc->ex_value) {
+        if (!is_same_page(dcbase, dc->base.pc_next) || dc->ex_value) {
             dc->base.is_jmp = DISAS_TOO_MANY;
         }
     }
-- 
2.35.3



  parent reply	other threads:[~2022-08-05 18:40 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-05 16:09 [PATCH v2 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
2022-08-05 16:09 ` [PATCH v2 1/4] accel/tcg: Invalidate translations when clearing PAGE_READ Ilya Leoshkevich
2022-08-05 17:42   ` Peter Maydell
2022-08-05 17:55   ` Richard Henderson
2022-08-05 16:09 ` Ilya Leoshkevich [this message]
2022-08-05 19:13   ` [PATCH v2 2/4] target/s390x: Make translator stop before the end of a page Richard Henderson
2022-08-05 16:09 ` [PATCH v2 3/4] target/i386: " Ilya Leoshkevich
2022-08-05 20:19   ` Richard Henderson
2022-08-05 16:09 ` [PATCH v2 4/4] tests/tcg: Test siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich

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=20220805160914.1106091-3-iii@linux.ibm.com \
    --to=iii@linux.ibm.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=david@redhat.com \
    --cc=eduardo@habkost.net \
    --cc=f4bug@amsat.org \
    --cc=laurent@vivier.eu \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=wangyanan55@huawei.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.