diff mbox series

[v3,2/4] target/s390x: Make translator stop before the end of a page

Message ID 20220808171022.49439-3-iii@linux.ibm.com
State New
Headers show
Series linux-user: Fix siginfo_t contents when jumping to non-readable pages | expand

Commit Message

Ilya Leoshkevich Aug. 8, 2022, 5:10 p.m. UTC 
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 | 15 +++++++++++----
 2 files changed, 21 insertions(+), 4 deletions(-)

Comments

Richard Henderson Aug. 10, 2022, 8:30 p.m. UTC | #1 
On 8/8/22 10:10, Ilya Leoshkevich wrote:
> 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 | 15 +++++++++++----
>   2 files changed, 21 insertions(+), 4 deletions(-)

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


r~
Richard Henderson Aug. 11, 2022, 4:08 a.m. UTC | #2 
On 8/8/22 10:10, Ilya Leoshkevich wrote:
> 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 | 15 +++++++++++----
>   2 files changed, 21 insertions(+), 4 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;
> +}

FYI, I've had occasion to pull this out to a separate patch locally.


r~
diff mbox series

Patch

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..8e45a0e0d3 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6609,6 +6609,14 @@  static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
     dc->insn_start = tcg_last_op();
 }
 
+static target_ulong get_next_pc(CPUS390XState *env, DisasContext *s,
+                                uint64_t pc)
+{
+    uint64_t insn = ld_code2(env, s, pc);
+
+    return pc + get_ilen((insn >> 8) & 0xff);
+}
+
 static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
 {
     CPUS390XState *env = cs->env_ptr;
@@ -6616,10 +6624,9 @@  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) ||
+            !is_same_page(dcbase, get_next_pc(env, dc, dc->base.pc_next)) ||
+            dc->ex_value) {
             dc->base.is_jmp = DISAS_TOO_MANY;
         }
     }