All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb
@ 2021-06-30 18:31 Richard Henderson
  2021-06-30 18:31 ` [PATCH v2 01/28] " Richard Henderson
                   ` (28 more replies)
  0 siblings, 29 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:31 UTC (permalink / raw)
  To: qemu-devel

Based-on: <20210629185455.3131172-1-richard.henderson@linaro.org>
("[PULL 00/63] tcg patch queue")

There are a number of inconsistencies with goto_tb usage, and I
plan to make changes in order to better support breakpoints.

(1) Testing CF_LAST_IO is a hold-over from since before ba3e7926691
    ("icount: clean up cpu_can_io at the entry to the block").
    Several targets still have this test.

(2) Testing singlestep is superfluous, as it doesn't mean anything
    besides limiting max_insns to 1.

(3) Not testing page crossing for CONFIG_USER_ONLY is wrong, because
    mmap and mprotect can change page permissions.  It's a very
    uncommon case wrt executables, but it's still wrong.

(4) Not testing page crossing for non-mmu targets (where page
    permissions literally cannot change) is not currently wrong,
    but will be after the breakpoint changes.

(5) When the TB does cross two pages, considering non-page crossing
    from the second page is not currently wrong, but will be after
    the breakpoint changes.

Changes for v2:
  * Fix aarch32 ISB, SB insns vs single-stepping.
  * Drop use_goto_tb for aarch32
  * Retain use_goto_tb for aarch64.

Patches lacking review:
  02-target-alpha-Remove-use_exit_tb.patch
  03-target-alpha-Remove-in_superpage.patch
  04-target-alpha-Use-translator_use_goto_tb.patch
  05-target-arm-Use-gen_jmp-for-ISB-and-SB.patch
  06-target-arm-Use-translator_use_goto_tb-for-aarch64.patch
  07-target-arm-Use-translator_use_goto_tb-for-aarch32.patch
  08-target-avr-Use-translator_use_goto_tb.patch
  10-target-cris-Use-translator_use_goto_tb.patch
  11-target-hppa-Use-translator_use_goto_tb.patch
  12-target-i386-Use-translator_use_goto_tb.patch
  14-target-microblaze-Use-translator_use_goto_tb.patch
  15-target-mips-Use-translator_use_goto_tb.patch
  17-target-nios2-Use-translator_use_goto_tb.patch
  18-target-openrisc-Use-translator_use_goto_tb.patch
  21-target-rx-Use-translator_use_goto_tb.patch
  22-target-s390x-Use-translator_use_goto_tb.patch
  23-target-s390x-Remove-use_exit_tb.patch
  24-target-sh4-Use-translator_use_goto_tb.patch


r~


Richard Henderson (28):
  accel/tcg: Introduce translator_use_goto_tb
  target/alpha: Remove use_exit_tb
  target/alpha: Remove in_superpage
  target/alpha: Use translator_use_goto_tb
  target/arm: Use gen_jmp for ISB and SB
  target/arm: Use translator_use_goto_tb for aarch64
  target/arm: Use translator_use_goto_tb for aarch32
  target/avr: Use translator_use_goto_tb
  target/avr: Mark some helpers noreturn
  target/cris: Use translator_use_goto_tb
  target/hppa: Use translator_use_goto_tb
  target/i386: Use translator_use_goto_tb
  target/m68k: Use translator_use_goto_tb
  target/microblaze: Use translator_use_goto_tb
  target/mips: Use translator_use_goto_tb
  target/mips: Fix missing else in gen_goto_tb
  target/nios2: Use translator_use_goto_tb
  target/openrisc: Use translator_use_goto_tb
  target/ppc: Use translator_use_goto_tb
  target/riscv: Use translator_use_goto_tb
  target/rx: Use translator_use_goto_tb
  target/s390x: Use translator_use_goto_tb
  target/s390x: Remove use_exit_tb
  target/sh4: Use translator_use_goto_tb
  target/sparc: Use translator_use_goto_tb
  target/tricore: Use translator_use_goto_tb
  target/tricore: Use tcg_gen_lookup_and_goto_ptr
  target/xtensa: Use translator_use_goto_tb

 include/exec/translator.h     | 10 ++++++++
 target/avr/helper.h           |  8 +++---
 accel/tcg/translator.c        | 11 +++++++++
 target/alpha/translate.c      | 46 ++++-------------------------------
 target/arm/translate-a64.c    | 25 ++++---------------
 target/arm/translate.c        | 16 +++---------
 target/avr/translate.c        |  9 ++++---
 target/cris/translate.c       |  5 ++--
 target/hppa/translate.c       |  5 +---
 target/i386/tcg/translate.c   | 14 ++---------
 target/m68k/translate.c       | 12 +--------
 target/microblaze/translate.c | 11 +--------
 target/mips/tcg/translate.c   | 20 +++------------
 target/nios2/translate.c      | 15 +-----------
 target/openrisc/translate.c   | 15 ++++++------
 target/ppc/translate.c        | 10 +-------
 target/riscv/translate.c      | 20 +--------------
 target/rx/translate.c         | 11 +--------
 target/s390x/translate.c      | 18 +++-----------
 target/sh4/translate.c        | 11 +++------
 target/sparc/translate.c      | 19 ++++-----------
 target/tricore/translate.c    | 20 +++------------
 target/xtensa/translate.c     |  6 +----
 23 files changed, 83 insertions(+), 254 deletions(-)

-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 01/28] accel/tcg: Introduce translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:31 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 02/28] target/alpha: Remove use_exit_tb Richard Henderson
                   ` (27 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:31 UTC (permalink / raw)
  To: qemu-devel; +Cc: Max Filippov, Philippe Mathieu-Daudé

Add a generic version of the common use_goto_tb test.

Various targets avoid the page crossing test for CONFIG_USER_ONLY,
but that is wrong: mmap and mprotect can change page permissions.

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/translator.h | 10 ++++++++++
 accel/tcg/translator.c    | 11 +++++++++++
 2 files changed, 21 insertions(+)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index 24232ead41..dd9c06d40d 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -145,6 +145,16 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
 
 void translator_loop_temp_check(DisasContextBase *db);
 
+/**
+ * translator_use_goto_tb
+ * @db: Disassembly context
+ * @dest: target pc of the goto
+ *
+ * Return true if goto_tb is allowed between the current TB
+ * and the destination PC.
+ */
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
+
 /*
  * Translator Load Functions
  *
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 1d32732198..59804af37b 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -31,6 +31,17 @@ void translator_loop_temp_check(DisasContextBase *db)
     }
 }
 
+bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
+{
+    /* Suppress goto_tb in the case of single-steping.  */
+    if (db->singlestep_enabled || singlestep) {
+        return false;
+    }
+
+    /* Check for the dest on the same page as the start of the TB.  */
+    return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
+}
+
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
                      CPUState *cpu, TranslationBlock *tb, int max_insns)
 {
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 02/28] target/alpha: Remove use_exit_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
  2021-06-30 18:31 ` [PATCH v2 01/28] " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:34   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 03/28] target/alpha: Remove in_superpage Richard Henderson
                   ` (26 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel

We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block").
We do not need to use exit_tb for singlestep, which only
means generate one insn per TB.

Which leaves only singlestep_enabled, which means raise a
debug trap after every TB, which does not use exit_tb,
which would leave the function mis-named.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/translate.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index f2922f5f8c..aaedf78116 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -450,19 +450,8 @@ static bool in_superpage(DisasContext *ctx, int64_t addr)
 #endif
 }
 
-static bool use_exit_tb(DisasContext *ctx)
-{
-    return ((tb_cflags(ctx->base.tb) & CF_LAST_IO)
-            || ctx->base.singlestep_enabled
-            || singlestep);
-}
-
 static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
 {
-    /* Suppress goto_tb in the case of single-steping and IO.  */
-    if (unlikely(use_exit_tb(ctx))) {
-        return false;
-    }
 #ifndef CONFIG_USER_ONLY
     /* If the destination is in the superpage, the page perms can't change.  */
     if (in_superpage(ctx, dest)) {
@@ -1271,7 +1260,7 @@ static DisasJumpType gen_call_pal(DisasContext *ctx, int palcode)
            need the page permissions check.  We'll see the existence of
            the page when we create the TB, and we'll flush all TBs if
            we change the PAL base register.  */
-        if (!use_exit_tb(ctx)) {
+        if (!ctx->base.singlestep_enabled) {
             tcg_gen_goto_tb(0);
             tcg_gen_movi_i64(cpu_pc, entry);
             tcg_gen_exit_tb(ctx->base.tb, 0);
@@ -3095,7 +3084,7 @@ static void alpha_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
         tcg_gen_movi_i64(cpu_pc, ctx->base.pc_next);
         /* FALLTHRU */
     case DISAS_PC_UPDATED:
-        if (!use_exit_tb(ctx)) {
+        if (!ctx->base.singlestep_enabled) {
             tcg_gen_lookup_and_goto_ptr();
             break;
         }
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 03/28] target/alpha: Remove in_superpage
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
  2021-06-30 18:31 ` [PATCH v2 01/28] " Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 02/28] target/alpha: Remove use_exit_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:35   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb Richard Henderson
                   ` (25 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel

The number of links across (normal) pages using this is low,
and it will shortly violate the contract for breakpoints.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/translate.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index aaedf78116..8fa00a79d1 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -439,24 +439,9 @@ static DisasJumpType gen_store_conditional(DisasContext *ctx, int ra, int rb,
     return DISAS_NEXT;
 }
 
-static bool in_superpage(DisasContext *ctx, int64_t addr)
-{
-#ifndef CONFIG_USER_ONLY
-    return ((ctx->tbflags & ENV_FLAG_PS_USER) == 0
-            && addr >> TARGET_VIRT_ADDR_SPACE_BITS == -1
-            && ((addr >> 41) & 3) == 2);
-#else
-    return false;
-#endif
-}
-
 static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
 {
 #ifndef CONFIG_USER_ONLY
-    /* If the destination is in the superpage, the page perms can't change.  */
-    if (in_superpage(ctx, dest)) {
-        return true;
-    }
     /* Check for the dest on the same page as the start of the TB.  */
     return ((ctx->base.tb->pc ^ dest) & TARGET_PAGE_MASK) == 0;
 #else
@@ -2991,7 +2976,7 @@ static void alpha_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPUAlphaState *env = cpu->env_ptr;
-    int64_t bound, mask;
+    int64_t bound;
 
     ctx->tbflags = ctx->base.tb->flags;
     ctx->mem_idx = cpu_mmu_index(env, false);
@@ -3020,12 +3005,7 @@ static void alpha_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
     ctx->lit = NULL;
 
     /* Bound the number of insns to execute to those left on the page.  */
-    if (in_superpage(ctx, ctx->base.pc_first)) {
-        mask = -1ULL << 41;
-    } else {
-        mask = TARGET_PAGE_MASK;
-    }
-    bound = -(ctx->base.pc_first | mask) / 4;
+    bound = -(ctx->base.pc_first | TARGET_PAGE_MASK) / 4;
     ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
 }
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (2 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 03/28] target/alpha: Remove in_superpage Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:29   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB Richard Henderson
                   ` (24 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/alpha/translate.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 8fa00a79d1..de769f7633 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -441,12 +441,7 @@ static DisasJumpType gen_store_conditional(DisasContext *ctx, int ra, int rb,
 
 static bool use_goto_tb(DisasContext *ctx, uint64_t dest)
 {
-#ifndef CONFIG_USER_ONLY
-    /* Check for the dest on the same page as the start of the TB.  */
-    return ((ctx->base.tb->pc ^ dest) & TARGET_PAGE_MASK) == 0;
-#else
-    return true;
-#endif
+    return translator_use_goto_tb(&ctx->base, dest);
 }
 
 static DisasJumpType gen_bdirect(DisasContext *ctx, int ra, int32_t disp)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (3 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:05   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64 Richard Henderson
                   ` (23 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm

Using gen_goto_tb directly misses the single-step check.

Cc: qemu-arm@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index a0c6cfa902..8cd31feeaa 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -8582,7 +8582,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a)
      * self-modifying code correctly and also to take
      * any pending interrupts immediately.
      */
-    gen_goto_tb(s, 0, s->base.pc_next);
+    gen_jmp(s, s->base.pc_next);
     return true;
 }
 
@@ -8596,7 +8596,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a)
      * for TCG; MB and end the TB instead.
      */
     tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
-    gen_goto_tb(s, 0, s->base.pc_next);
+    gen_jmp(s, s->base.pc_next);
     return true;
 }
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (4 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:08   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32 Richard Henderson
                   ` (22 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm

We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block"),
and gdbstub singlestep is handled by the generic function.

Drop the unused 'n' argument to use_goto_tb.

Cc: qemu-arm@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 25 +++++--------------------
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 1a40e49db7..eb1907d049 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -386,35 +386,20 @@ static void gen_step_complete_exception(DisasContext *s)
     s->base.is_jmp = DISAS_NORETURN;
 }
 
-static inline bool use_goto_tb(DisasContext *s, int n, uint64_t dest)
+static inline bool use_goto_tb(DisasContext *s, uint64_t dest)
 {
-    /* No direct tb linking with singlestep (either QEMU's or the ARM
-     * debug architecture kind) or deterministic io
-     */
-    if (s->base.singlestep_enabled || s->ss_active ||
-        (tb_cflags(s->base.tb) & CF_LAST_IO)) {
+    if (s->ss_active) {
         return false;
     }
-
-#ifndef CONFIG_USER_ONLY
-    /* Only link tbs from inside the same guest page */
-    if ((s->base.tb->pc & TARGET_PAGE_MASK) != (dest & TARGET_PAGE_MASK)) {
-        return false;
-    }
-#endif
-
-    return true;
+    return translator_use_goto_tb(&s->base, dest);
 }
 
 static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
 {
-    const TranslationBlock *tb;
-
-    tb = s->base.tb;
-    if (use_goto_tb(s, n, dest)) {
+    if (use_goto_tb(s, dest)) {
         tcg_gen_goto_tb(n);
         gen_a64_set_pc_im(dest);
-        tcg_gen_exit_tb(tb, n);
+        tcg_gen_exit_tb(s->base.tb, n);
         s->base.is_jmp = DISAS_NORETURN;
     } else {
         gen_a64_set_pc_im(dest);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (5 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64 Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:14   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 08/28] target/avr: Use translator_use_goto_tb Richard Henderson
                   ` (21 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-arm

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: qemu-arm@nongnu.org
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 8cd31feeaa..87c3c09df5 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -2509,16 +2509,6 @@ static int disas_dsp_insn(DisasContext *s, uint32_t insn)
     return 1;
 }
 
-static inline bool use_goto_tb(DisasContext *s, target_ulong dest)
-{
-#ifndef CONFIG_USER_ONLY
-    return (s->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK) ||
-           ((s->base.pc_next - 1) & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 static void gen_goto_ptr(void)
 {
     tcg_gen_lookup_and_goto_ptr();
@@ -2530,7 +2520,7 @@ static void gen_goto_ptr(void)
  */
 static void gen_goto_tb(DisasContext *s, int n, target_ulong dest)
 {
-    if (use_goto_tb(s, dest)) {
+    if (translator_use_goto_tb(&s->base, dest)) {
         tcg_gen_goto_tb(n);
         gen_set_pc_im(s, dest);
         tcg_gen_exit_tb(s->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 08/28] target/avr: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (6 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32 Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:28   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 09/28] target/avr: Mark some helpers noreturn Richard Henderson
                   ` (20 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Rolnik

Single stepping is not the only reason not to use goto_tb.
If goto_tb is disallowed, and single-stepping is not enabled,
then use tcg_gen_lookup_and_goto_tb to indirectly chain.

Cc: Michael Rolnik <mrolnik@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/avr/translate.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/target/avr/translate.c b/target/avr/translate.c
index c06ce45bc7..8237a03c23 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1083,14 +1083,17 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
     const TranslationBlock *tb = ctx->base.tb;
 
-    if (!ctx->base.singlestep_enabled) {
+    if (translator_use_goto_tb(&ctx->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(cpu_pc, dest);
         tcg_gen_exit_tb(tb, n);
     } else {
         tcg_gen_movi_i32(cpu_pc, dest);
-        gen_helper_debug(cpu_env);
-        tcg_gen_exit_tb(NULL, 0);
+        if (ctx->base.singlestep_enabled) {
+            gen_helper_debug(cpu_env);
+        } else {
+            tcg_gen_lookup_and_goto_ptr();
+        }
     }
     ctx->base.is_jmp = DISAS_NORETURN;
 }
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 09/28] target/avr: Mark some helpers noreturn
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (7 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 08/28] target/avr: Use translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 10/28] target/cris: Use translator_use_goto_tb Richard Henderson
                   ` (19 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Michael Rolnik, Philippe Mathieu-Daudé

All of these helpers end with cpu_loop_exit.

Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/avr/helper.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/avr/helper.h b/target/avr/helper.h
index 8e1ae7fda0..4d02e648fa 100644
--- a/target/avr/helper.h
+++ b/target/avr/helper.h
@@ -19,10 +19,10 @@
  */
 
 DEF_HELPER_1(wdr, void, env)
-DEF_HELPER_1(debug, void, env)
-DEF_HELPER_1(break, void, env)
-DEF_HELPER_1(sleep, void, env)
-DEF_HELPER_1(unsupported, void, env)
+DEF_HELPER_1(debug, noreturn, env)
+DEF_HELPER_1(break, noreturn, env)
+DEF_HELPER_1(sleep, noreturn, env)
+DEF_HELPER_1(unsupported, noreturn, env)
 DEF_HELPER_3(outb, void, env, i32, i32)
 DEF_HELPER_2(inb, tl, env, i32)
 DEF_HELPER_3(fullwr, void, env, i32, i32)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 10/28] target/cris: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (8 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 09/28] target/avr: Mark some helpers noreturn Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:27   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 11/28] target/hppa: " Richard Henderson
                   ` (18 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Edgar E . Iglesias

The test for singlestepping is done in translator_use_goto_tb,
so we may elide it from cris_tr_tb_stop.

Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/cris/translate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index 4cfe5c86d9..e33a3bb326 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -482,7 +482,7 @@ static void t_gen_swapr(TCGv d, TCGv s)
 
 static bool use_goto_tb(DisasContext *dc, target_ulong dest)
 {
-    return ((dest ^ dc->base.pc_first) & TARGET_PAGE_MASK) == 0;
+    return translator_use_goto_tb(&dc->base, dest);
 }
 
 static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
@@ -3235,8 +3235,7 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
              * Use a conditional branch if either taken or not-taken path
              * can use goto_tb.  If neither can, then treat it as indirect.
              */
-            if (likely(!dc->base.singlestep_enabled)
-                && likely(!dc->cpustate_changed)
+            if (likely(!dc->cpustate_changed)
                 && (use_goto_tb(dc, dc->jmp_pc) || use_goto_tb(dc, npc))) {
                 TCGLabel *not_taken = gen_new_label();
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 11/28] target/hppa: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (9 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 10/28] target/cris: Use translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:26   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 12/28] target/i386: " Richard Henderson
                   ` (17 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/hppa/translate.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 64af1e0d5c..952cfe09a6 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -817,10 +817,7 @@ static bool gen_illegal(DisasContext *ctx)
 
 static bool use_goto_tb(DisasContext *ctx, target_ureg dest)
 {
-    /* Suppress goto_tb for page crossing, IO, or single-steping.  */
-    return !(((ctx->base.pc_first ^ dest) & TARGET_PAGE_MASK)
-             || (tb_cflags(ctx->base.tb) & CF_LAST_IO)
-             || ctx->base.singlestep_enabled);
+    return translator_use_goto_tb(&ctx->base, dest);
 }
 
 /* If the next insn is to be nullified, and it's on the same page,
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 12/28] target/i386: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (10 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 11/28] target/hppa: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:26   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 13/28] target/m68k: " Richard Henderson
                   ` (16 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Paolo Bonzini, Eduardo Habkost

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/tcg/translate.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index b21873ed23..eb9ee0296f 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2314,21 +2314,11 @@ static inline int insn_const_size(MemOp ot)
     }
 }
 
-static inline bool use_goto_tb(DisasContext *s, target_ulong pc)
-{
-#ifndef CONFIG_USER_ONLY
-    return (pc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) ||
-           (pc & TARGET_PAGE_MASK) == (s->pc_start & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
-static inline void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
+static void gen_goto_tb(DisasContext *s, int tb_num, target_ulong eip)
 {
     target_ulong pc = s->cs_base + eip;
 
-    if (use_goto_tb(s, pc))  {
+    if (translator_use_goto_tb(&s->base, pc))  {
         /* jump to same page: we can use a direct jump */
         tcg_gen_goto_tb(tb_num);
         gen_jmp_im(s, eip);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 13/28] target/m68k: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (11 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 12/28] target/i386: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:22   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 14/28] target/microblaze: " Richard Henderson
                   ` (15 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Laurent Vivier

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Acked-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/translate.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index f0c5bf9154..05b96fdda7 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1520,16 +1520,6 @@ static void gen_exit_tb(DisasContext *s)
         }                                                               \
     } while (0)
 
-static inline bool use_goto_tb(DisasContext *s, uint32_t dest)
-{
-#ifndef CONFIG_USER_ONLY
-    return (s->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK)
-        || (s->base.pc_next & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 /* Generate a jump to an immediate address.  */
 static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
 {
@@ -1537,7 +1527,7 @@ static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
         update_cc_op(s);
         tcg_gen_movi_i32(QREG_PC, dest);
         gen_singlestep_exception(s);
-    } else if (use_goto_tb(s, dest)) {
+    } else if (translator_use_goto_tb(&s->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(QREG_PC, dest);
         tcg_gen_exit_tb(s->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 14/28] target/microblaze: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (12 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 13/28] target/m68k: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:21   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 15/28] target/mips: " Richard Henderson
                   ` (14 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Edgar E . Iglesias

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/microblaze/translate.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index c1b13f4c7d..b753f080e7 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -125,15 +125,6 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t esr_ec)
     gen_raise_exception_sync(dc, EXCP_HW_EXCP);
 }
 
-static inline bool use_goto_tb(DisasContext *dc, target_ulong dest)
-{
-#ifndef CONFIG_USER_ONLY
-    return (dc->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
 {
     if (dc->base.singlestep_enabled) {
@@ -141,7 +132,7 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
         tcg_gen_movi_i32(cpu_pc, dest);
         gen_helper_raise_exception(cpu_env, tmp);
         tcg_temp_free_i32(tmp);
-    } else if (use_goto_tb(dc, dest)) {
+    } else if (translator_use_goto_tb(&dc->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(cpu_pc, dest);
         tcg_gen_exit_tb(dc->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 15/28] target/mips: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (13 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 14/28] target/microblaze: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:18   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 16/28] target/mips: Fix missing else in gen_goto_tb Richard Henderson
                   ` (13 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/translate.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index b4a454ec09..52ae88b777 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -5019,22 +5019,9 @@ static void gen_trap(DisasContext *ctx, uint32_t opc,
     tcg_temp_free(t1);
 }
 
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
+static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
-    if (unlikely(ctx->base.singlestep_enabled)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
-static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
-{
-    if (use_goto_tb(ctx, dest)) {
+    if (translator_use_goto_tb(&ctx->base, dest)) {
         tcg_gen_goto_tb(n);
         gen_save_pc(dest);
         tcg_gen_exit_tb(ctx->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 16/28] target/mips: Fix missing else in gen_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (14 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 15/28] target/mips: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb Richard Henderson
                   ` (12 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Philippe Mathieu-Daudé

Do not emit dead code for the singlestep_enabled case,
after having exited the TB with a debug exception.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 52ae88b777..17e79f3de3 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -5030,8 +5030,9 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         if (ctx->base.singlestep_enabled) {
             save_cpu_state(ctx, 0);
             gen_helper_raise_exception_debug(cpu_env);
+        } else {
+            tcg_gen_lookup_and_goto_ptr();
         }
-        tcg_gen_lookup_and_goto_ptr();
     }
 }
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (15 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 16/28] target/mips: Fix missing else in gen_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:17   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 18/28] target/openrisc: " Richard Henderson
                   ` (11 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Marek Vasut, Chris Wulff

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: Chris Wulff <crwulff@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/nios2/translate.c | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 930f3d3395..17742cebc7 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -150,24 +150,11 @@ static void t_gen_helper_raise_exception(DisasContext *dc,
     dc->base.is_jmp = DISAS_NORETURN;
 }
 
-static bool use_goto_tb(DisasContext *dc, uint32_t dest)
-{
-    if (unlikely(dc->base.singlestep_enabled)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (dc->base.pc_first & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 static void gen_goto_tb(DisasContext *dc, int n, uint32_t dest)
 {
     const TranslationBlock *tb = dc->base.tb;
 
-    if (use_goto_tb(dc, dest)) {
+    if (translator_use_goto_tb(&dc->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_tl(cpu_R[R_PC], dest);
         tcg_gen_exit_tb(tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 18/28] target/openrisc: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (16 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-07 20:59   ` Stafford Horne
  2021-06-30 18:32 ` [PATCH v2 19/28] target/ppc: " Richard Henderson
                   ` (10 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Stafford Horne

Reorder the cases in openrisc_tr_tb_stop to make this easier to read.

Cc: Stafford Horne <shorne@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/openrisc/translate.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index a9c81f8bd5..2d142d8577 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1720,16 +1720,17 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         /* fallthru */
 
     case DISAS_TOO_MANY:
-        if (unlikely(dc->base.singlestep_enabled)) {
-            tcg_gen_movi_tl(cpu_pc, jmp_dest);
-            gen_exception(dc, EXCP_DEBUG);
-        } else if ((dc->base.pc_first ^ jmp_dest) & TARGET_PAGE_MASK) {
-            tcg_gen_movi_tl(cpu_pc, jmp_dest);
-            tcg_gen_lookup_and_goto_ptr();
-        } else {
+        if (translator_use_goto_tb(&dc->base, jmp_dest)) {
             tcg_gen_goto_tb(0);
             tcg_gen_movi_tl(cpu_pc, jmp_dest);
             tcg_gen_exit_tb(dc->base.tb, 0);
+            break;
+        }
+        tcg_gen_movi_tl(cpu_pc, jmp_dest);
+        if (unlikely(dc->base.singlestep_enabled)) {
+            gen_exception(dc, EXCP_DEBUG);
+        } else {
+            tcg_gen_lookup_and_goto_ptr();
         }
         break;
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 19/28] target/ppc: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (17 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 18/28] target/openrisc: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 20/28] target/riscv: " Richard Henderson
                   ` (9 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Luis Pires

Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index f65d1e81ea..0fb09f2301 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4302,15 +4302,7 @@ static inline void gen_update_cfar(DisasContext *ctx, target_ulong nip)
 
 static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 {
-    if (unlikely(ctx->singlestep_enabled)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
+    return translator_use_goto_tb(&ctx->base, dest);
 }
 
 static void gen_lookup_and_goto_ptr(DisasContext *ctx)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 20/28] target/riscv: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (18 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 19/28] target/ppc: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 21/28] target/rx: " Richard Henderson
                   ` (8 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alistair Francis

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/translate.c | 20 +-------------------
 1 file changed, 1 insertion(+), 19 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 62a7d7e4c7..deda0c8a44 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -168,29 +168,11 @@ static void gen_exception_inst_addr_mis(DisasContext *ctx)
     generate_exception_mtval(ctx, RISCV_EXCP_INST_ADDR_MIS);
 }
 
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
-{
-    if (unlikely(ctx->base.singlestep_enabled)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
-    if (use_goto_tb(ctx, dest)) {
-        /* chaining is only allowed when the jump is to the same page */
+    if (translator_use_goto_tb(&ctx->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_tl(cpu_pc, dest);
-
-        /* No need to check for single stepping here as use_goto_tb() will
-         * return false in case of single stepping.
-         */
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
         tcg_gen_movi_tl(cpu_pc, dest);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 21/28] target/rx: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (19 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 20/28] target/riscv: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:17   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 22/28] target/s390x: " Richard Henderson
                   ` (7 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yoshinori Sato

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/translate.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/rx/translate.c b/target/rx/translate.c
index 9ea941c630..2443406de5 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -143,18 +143,9 @@ void rx_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     }
 }
 
-static bool use_goto_tb(DisasContext *dc, target_ulong dest)
-{
-    if (unlikely(dc->base.singlestep_enabled)) {
-        return false;
-    } else {
-        return true;
-    }
-}
-
 static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
 {
-    if (use_goto_tb(dc, dest)) {
+    if (translator_use_goto_tb(&dc->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(cpu_pc, dest);
         tcg_gen_exit_tb(dc->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 22/28] target/s390x: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (20 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 21/28] target/rx: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-01  7:42   ` David Hildenbrand
  2021-06-30 18:32 ` [PATCH v2 23/28] target/s390x: Remove use_exit_tb Richard Henderson
                   ` (6 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: David Hildenbrand

Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/translate.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 03dab9f350..117a890ecd 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -697,12 +697,7 @@ static bool use_goto_tb(DisasContext *s, uint64_t dest)
     if (unlikely(use_exit_tb(s))) {
         return false;
     }
-#ifndef CONFIG_USER_ONLY
-    return (dest & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) ||
-           (dest & TARGET_PAGE_MASK) == (s->base.pc_next & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
+    return translator_use_goto_tb(&s->base, dest);
 }
 
 static void account_noninline_branch(DisasContext *s, int cc_op)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 23/28] target/s390x: Remove use_exit_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (21 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 22/28] target/s390x: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-01  7:44   ` David Hildenbrand
  2021-06-30 18:32 ` [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb Richard Henderson
                   ` (5 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: David Hildenbrand

We have not needed to end a TB for I/O since ba3e7926691
("icount: clean up cpu_can_io at the entry to the block").

In use_goto_tb, the check for singlestep_enabled is in the
generic translator_use_goto_tb.  In s390x_tr_tb_stop, the
check for singlestep_enabled is in the preceeding do_debug test.

Which leaves only FLAG_MASK_PER: fold that test alone into
the two callers of use_exit tb.

Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/s390x/translate.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index 117a890ecd..4742f59ca9 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -685,16 +685,9 @@ static void gen_op_calc_cc(DisasContext *s)
     set_cc_static(s);
 }
 
-static bool use_exit_tb(DisasContext *s)
-{
-    return s->base.singlestep_enabled ||
-            (tb_cflags(s->base.tb) & CF_LAST_IO) ||
-            (s->base.tb->flags & FLAG_MASK_PER);
-}
-
 static bool use_goto_tb(DisasContext *s, uint64_t dest)
 {
-    if (unlikely(use_exit_tb(s))) {
+    if (unlikely(s->base.tb->flags & FLAG_MASK_PER)) {
         return false;
     }
     return translator_use_goto_tb(&s->base, dest);
@@ -6634,7 +6627,7 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         /* Exit the TB, either by raising a debug exception or by return.  */
         if (dc->do_debug) {
             gen_exception(EXCP_DEBUG);
-        } else if (use_exit_tb(dc) ||
+        } else if ((dc->base.tb->flags & FLAG_MASK_PER) ||
                    dc->base.is_jmp == DISAS_PC_STALE_NOCHAIN) {
             tcg_gen_exit_tb(NULL, 0);
         } else {
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (22 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 23/28] target/s390x: Remove use_exit_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-08 12:16   ` Peter Maydell
  2021-06-30 18:32 ` [PATCH v2 25/28] target/sparc: " Richard Henderson
                   ` (4 subsequent siblings)
  28 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Yoshinori Sato

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/translate.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 4dcfff81f6..db09a0bce3 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -225,17 +225,12 @@ static inline bool use_exit_tb(DisasContext *ctx)
     return (ctx->tbflags & GUSA_EXCLUSIVE) != 0;
 }
 
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
+static bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 {
-    /* Use a direct jump if in same page and singlestep not enabled */
-    if (unlikely(ctx->base.singlestep_enabled || use_exit_tb(ctx))) {
+    if (use_exit_tb(ctx)) {
         return false;
     }
-#ifndef CONFIG_USER_ONLY
-    return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
+    return translator_use_goto_tb(&ctx->base, dest);
 }
 
 static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 25/28] target/sparc: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (23 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 26/28] target/tricore: " Richard Henderson
                   ` (3 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Mark Cave-Ayland

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sparc/translate.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 4bfa3179f8..fb0c242606 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -339,23 +339,14 @@ static inline TCGv gen_dest_gpr(DisasContext *dc, int reg)
     }
 }
 
-static inline bool use_goto_tb(DisasContext *s, target_ulong pc,
-                               target_ulong npc)
+static bool use_goto_tb(DisasContext *s, target_ulong pc, target_ulong npc)
 {
-    if (unlikely(s->base.singlestep_enabled || singlestep)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (pc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) &&
-           (npc & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
+    return translator_use_goto_tb(&s->base, pc) &&
+           translator_use_goto_tb(&s->base, npc);
 }
 
-static inline void gen_goto_tb(DisasContext *s, int tb_num,
-                               target_ulong pc, target_ulong npc)
+static void gen_goto_tb(DisasContext *s, int tb_num,
+                        target_ulong pc, target_ulong npc)
 {
     if (use_goto_tb(s, pc, npc))  {
         /* jump to same page: we can use a direct jump */
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 26/28] target/tricore: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (24 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 25/28] target/sparc: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 27/28] target/tricore: Use tcg_gen_lookup_and_goto_ptr Richard Henderson
                   ` (2 subsequent siblings)
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Bastian Koppelmann

Just use translator_use_goto_tb directly at the one call site,
rather than maintaining a local wrapper.

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/tricore/translate.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 2a814263de..09465ea013 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3225,19 +3225,6 @@ static inline void gen_save_pc(target_ulong pc)
     tcg_gen_movi_tl(cpu_PC, pc);
 }
 
-static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
-{
-    if (unlikely(ctx->base.singlestep_enabled)) {
-        return false;
-    }
-
-#ifndef CONFIG_USER_ONLY
-    return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
-#else
-    return true;
-#endif
-}
-
 static void generate_qemu_excp(DisasContext *ctx, int excp)
 {
     TCGv_i32 tmp = tcg_const_i32(excp);
@@ -3246,9 +3233,9 @@ static void generate_qemu_excp(DisasContext *ctx, int excp)
     tcg_temp_free(tmp);
 }
 
-static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
+static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
-    if (use_goto_tb(ctx, dest)) {
+    if (translator_use_goto_tb(&ctx->base, dest)) {
         tcg_gen_goto_tb(n);
         gen_save_pc(dest);
         tcg_gen_exit_tb(ctx->base.tb, n);
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 27/28] target/tricore: Use tcg_gen_lookup_and_goto_ptr
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (25 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 26/28] target/tricore: " Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-06-30 18:32 ` [PATCH v2 28/28] target/xtensa: Use translator_use_goto_tb Richard Henderson
  2021-07-07 21:41 ` [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Bastian Koppelmann

The non-single-step case of gen_goto_tb may use
tcg_gen_lookup_and_goto_ptr to indirectly chain.

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/tricore/translate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 09465ea013..865020754d 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3243,8 +3243,9 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         gen_save_pc(dest);
         if (ctx->base.singlestep_enabled) {
             generate_qemu_excp(ctx, EXCP_DEBUG);
+        } else {
+            tcg_gen_lookup_and_goto_ptr();
         }
-        tcg_gen_exit_tb(NULL, 0);
     }
 }
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* [PATCH v2 28/28] target/xtensa: Use translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (26 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 27/28] target/tricore: Use tcg_gen_lookup_and_goto_ptr Richard Henderson
@ 2021-06-30 18:32 ` Richard Henderson
  2021-07-07 21:41 ` [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-06-30 18:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Max Filippov

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/xtensa/translate.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 14028d307d..ac42f5efdc 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -406,11 +406,7 @@ static void gen_jump(DisasContext *dc, TCGv dest)
 
 static int adjust_jump_slot(DisasContext *dc, uint32_t dest, int slot)
 {
-    if (((dc->base.pc_first ^ dest) & TARGET_PAGE_MASK) != 0) {
-        return -1;
-    } else {
-        return slot;
-    }
+    return translator_use_goto_tb(&dc->base, dest) ? slot : -1;
 }
 
 static void gen_jumpi(DisasContext *dc, uint32_t dest, int slot)
-- 
2.25.1



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 22/28] target/s390x: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 22/28] target/s390x: " Richard Henderson
@ 2021-07-01  7:42   ` David Hildenbrand
  0 siblings, 0 replies; 53+ messages in thread
From: David Hildenbrand @ 2021-07-01  7:42 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 30.06.21 20:32, Richard Henderson wrote:
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/translate.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 03dab9f350..117a890ecd 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -697,12 +697,7 @@ static bool use_goto_tb(DisasContext *s, uint64_t dest)
>       if (unlikely(use_exit_tb(s))) {
>           return false;
>       }
> -#ifndef CONFIG_USER_ONLY
> -    return (dest & TARGET_PAGE_MASK) == (s->base.tb->pc & TARGET_PAGE_MASK) ||
> -           (dest & TARGET_PAGE_MASK) == (s->base.pc_next & TARGET_PAGE_MASK);
> -#else
> -    return true;
> -#endif
> +    return translator_use_goto_tb(&s->base, dest);
>   }
>   
>   static void account_noninline_branch(DisasContext *s, int cc_op)
> 

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

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 23/28] target/s390x: Remove use_exit_tb
  2021-06-30 18:32 ` [PATCH v2 23/28] target/s390x: Remove use_exit_tb Richard Henderson
@ 2021-07-01  7:44   ` David Hildenbrand
  0 siblings, 0 replies; 53+ messages in thread
From: David Hildenbrand @ 2021-07-01  7:44 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 30.06.21 20:32, Richard Henderson wrote:
> We have not needed to end a TB for I/O since ba3e7926691
> ("icount: clean up cpu_can_io at the entry to the block").
> 
> In use_goto_tb, the check for singlestep_enabled is in the
> generic translator_use_goto_tb.  In s390x_tr_tb_stop, the
> check for singlestep_enabled is in the preceeding do_debug test.

s/preceeding/preceding/

> 
> Which leaves only FLAG_MASK_PER: fold that test alone into
> the two callers of use_exit tb.
> 
> Cc: David Hildenbrand <david@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/s390x/translate.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 117a890ecd..4742f59ca9 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -685,16 +685,9 @@ static void gen_op_calc_cc(DisasContext *s)
>       set_cc_static(s);
>   }
>   
> -static bool use_exit_tb(DisasContext *s)
> -{
> -    return s->base.singlestep_enabled ||
> -            (tb_cflags(s->base.tb) & CF_LAST_IO) ||
> -            (s->base.tb->flags & FLAG_MASK_PER);
> -}
> -
>   static bool use_goto_tb(DisasContext *s, uint64_t dest)
>   {
> -    if (unlikely(use_exit_tb(s))) {
> +    if (unlikely(s->base.tb->flags & FLAG_MASK_PER)) {
>           return false;
>       }
>       return translator_use_goto_tb(&s->base, dest);
> @@ -6634,7 +6627,7 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>           /* Exit the TB, either by raising a debug exception or by return.  */
>           if (dc->do_debug) {
>               gen_exception(EXCP_DEBUG);
> -        } else if (use_exit_tb(dc) ||
> +        } else if ((dc->base.tb->flags & FLAG_MASK_PER) ||
>                      dc->base.is_jmp == DISAS_PC_STALE_NOCHAIN) {
>               tcg_gen_exit_tb(NULL, 0);
>           } else {
> 

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

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 18/28] target/openrisc: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 18/28] target/openrisc: " Richard Henderson
@ 2021-07-07 20:59   ` Stafford Horne
  0 siblings, 0 replies; 53+ messages in thread
From: Stafford Horne @ 2021-07-07 20:59 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-devel

On Wed, Jun 30, 2021 at 11:32:16AM -0700, Richard Henderson wrote:
> Reorder the cases in openrisc_tr_tb_stop to make this easier to read.

Hi Richard,

For me the description is a bit misleading.  I don't see it as a simple
reorder for making things easier to read, because we need to understand
what is inside of translator_use_goto_tb now.

From the patch basically translator_use_goto_tb indicates that a jump is in
the same page and we are not single stepping.

The old code was:

  If single step
   DEBUG
  else if not same page
   tcg_gen_lookup_and_goto_ptr()
  else *same page*
   jump same page

Now:

  If translator_use_goto_tb() (same page & not single step)
    jump same page

  If single step
    DEBUG
  else *not same page*
    tcg_gen_lookup_and_goto_ptr()

It would be a bit easier to understand if the description was something like,
Reorder the control statements to allow using the page boundary check from
translator_use_goto_tb().

That said it looks correct so:

Reviewed-by: Stafford Horne <shorne@gmail.com>


> Cc: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/openrisc/translate.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index a9c81f8bd5..2d142d8577 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -1720,16 +1720,17 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>          /* fallthru */
>  
>      case DISAS_TOO_MANY:
> -        if (unlikely(dc->base.singlestep_enabled)) {
> -            tcg_gen_movi_tl(cpu_pc, jmp_dest);
> -            gen_exception(dc, EXCP_DEBUG);
> -        } else if ((dc->base.pc_first ^ jmp_dest) & TARGET_PAGE_MASK) {
> -            tcg_gen_movi_tl(cpu_pc, jmp_dest);
> -            tcg_gen_lookup_and_goto_ptr();
> -        } else {
> +        if (translator_use_goto_tb(&dc->base, jmp_dest)) {
>              tcg_gen_goto_tb(0);
>              tcg_gen_movi_tl(cpu_pc, jmp_dest);
>              tcg_gen_exit_tb(dc->base.tb, 0);
> +            break;
> +        }
> +        tcg_gen_movi_tl(cpu_pc, jmp_dest);
> +        if (unlikely(dc->base.singlestep_enabled)) {
> +            gen_exception(dc, EXCP_DEBUG);
> +        } else {
> +            tcg_gen_lookup_and_goto_ptr();
>          }
>          break;
>  
> -- 
> 2.25.1
> 


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb
  2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
                   ` (27 preceding siblings ...)
  2021-06-30 18:32 ` [PATCH v2 28/28] target/xtensa: Use translator_use_goto_tb Richard Henderson
@ 2021-07-07 21:41 ` Richard Henderson
  28 siblings, 0 replies; 53+ messages in thread
From: Richard Henderson @ 2021-07-07 21:41 UTC (permalink / raw)
  To: qemu-devel, Peter Maydell, Alex Bennée

Ping.  FWIW, the 3 target/arm patches are the only major target for which I do not have 
reviews, or am not also the maintainer.

r~

On 6/30/21 11:31 AM, Richard Henderson wrote:
> Based-on: <20210629185455.3131172-1-richard.henderson@linaro.org>
> ("[PULL 00/63] tcg patch queue")
> 
> There are a number of inconsistencies with goto_tb usage, and I
> plan to make changes in order to better support breakpoints.
> 
> (1) Testing CF_LAST_IO is a hold-over from since before ba3e7926691
>      ("icount: clean up cpu_can_io at the entry to the block").
>      Several targets still have this test.
> 
> (2) Testing singlestep is superfluous, as it doesn't mean anything
>      besides limiting max_insns to 1.
> 
> (3) Not testing page crossing for CONFIG_USER_ONLY is wrong, because
>      mmap and mprotect can change page permissions.  It's a very
>      uncommon case wrt executables, but it's still wrong.
> 
> (4) Not testing page crossing for non-mmu targets (where page
>      permissions literally cannot change) is not currently wrong,
>      but will be after the breakpoint changes.
> 
> (5) When the TB does cross two pages, considering non-page crossing
>      from the second page is not currently wrong, but will be after
>      the breakpoint changes.
> 
> Changes for v2:
>    * Fix aarch32 ISB, SB insns vs single-stepping.
>    * Drop use_goto_tb for aarch32
>    * Retain use_goto_tb for aarch64.
> 
> Patches lacking review:
>    02-target-alpha-Remove-use_exit_tb.patch
>    03-target-alpha-Remove-in_superpage.patch
>    04-target-alpha-Use-translator_use_goto_tb.patch
>    05-target-arm-Use-gen_jmp-for-ISB-and-SB.patch
>    06-target-arm-Use-translator_use_goto_tb-for-aarch64.patch
>    07-target-arm-Use-translator_use_goto_tb-for-aarch32.patch
>    08-target-avr-Use-translator_use_goto_tb.patch
>    10-target-cris-Use-translator_use_goto_tb.patch
>    11-target-hppa-Use-translator_use_goto_tb.patch
>    12-target-i386-Use-translator_use_goto_tb.patch
>    14-target-microblaze-Use-translator_use_goto_tb.patch
>    15-target-mips-Use-translator_use_goto_tb.patch
>    17-target-nios2-Use-translator_use_goto_tb.patch
>    18-target-openrisc-Use-translator_use_goto_tb.patch
>    21-target-rx-Use-translator_use_goto_tb.patch
>    22-target-s390x-Use-translator_use_goto_tb.patch
>    23-target-s390x-Remove-use_exit_tb.patch
>    24-target-sh4-Use-translator_use_goto_tb.patch
> 
> 
> r~
> 
> 
> Richard Henderson (28):
>    accel/tcg: Introduce translator_use_goto_tb
>    target/alpha: Remove use_exit_tb
>    target/alpha: Remove in_superpage
>    target/alpha: Use translator_use_goto_tb
>    target/arm: Use gen_jmp for ISB and SB
>    target/arm: Use translator_use_goto_tb for aarch64
>    target/arm: Use translator_use_goto_tb for aarch32
>    target/avr: Use translator_use_goto_tb
>    target/avr: Mark some helpers noreturn
>    target/cris: Use translator_use_goto_tb
>    target/hppa: Use translator_use_goto_tb
>    target/i386: Use translator_use_goto_tb
>    target/m68k: Use translator_use_goto_tb
>    target/microblaze: Use translator_use_goto_tb
>    target/mips: Use translator_use_goto_tb
>    target/mips: Fix missing else in gen_goto_tb
>    target/nios2: Use translator_use_goto_tb
>    target/openrisc: Use translator_use_goto_tb
>    target/ppc: Use translator_use_goto_tb
>    target/riscv: Use translator_use_goto_tb
>    target/rx: Use translator_use_goto_tb
>    target/s390x: Use translator_use_goto_tb
>    target/s390x: Remove use_exit_tb
>    target/sh4: Use translator_use_goto_tb
>    target/sparc: Use translator_use_goto_tb
>    target/tricore: Use translator_use_goto_tb
>    target/tricore: Use tcg_gen_lookup_and_goto_ptr
>    target/xtensa: Use translator_use_goto_tb
> 
>   include/exec/translator.h     | 10 ++++++++
>   target/avr/helper.h           |  8 +++---
>   accel/tcg/translator.c        | 11 +++++++++
>   target/alpha/translate.c      | 46 ++++-------------------------------
>   target/arm/translate-a64.c    | 25 ++++---------------
>   target/arm/translate.c        | 16 +++---------
>   target/avr/translate.c        |  9 ++++---
>   target/cris/translate.c       |  5 ++--
>   target/hppa/translate.c       |  5 +---
>   target/i386/tcg/translate.c   | 14 ++---------
>   target/m68k/translate.c       | 12 +--------
>   target/microblaze/translate.c | 11 +--------
>   target/mips/tcg/translate.c   | 20 +++------------
>   target/nios2/translate.c      | 15 +-----------
>   target/openrisc/translate.c   | 15 ++++++------
>   target/ppc/translate.c        | 10 +-------
>   target/riscv/translate.c      | 20 +--------------
>   target/rx/translate.c         | 11 +--------
>   target/s390x/translate.c      | 18 +++-----------
>   target/sh4/translate.c        | 11 +++------
>   target/sparc/translate.c      | 19 ++++-----------
>   target/tricore/translate.c    | 20 +++------------
>   target/xtensa/translate.c     |  6 +----
>   23 files changed, 83 insertions(+), 254 deletions(-)
> 



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-06-30 18:32 ` [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB Richard Henderson
@ 2021-07-08 12:05   ` Peter Maydell
  2021-07-08 16:04     ` Richard Henderson
  0 siblings, 1 reply; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:05 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers

On Wed, 30 Jun 2021 at 19:47, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Using gen_goto_tb directly misses the single-step check.
>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index a0c6cfa902..8cd31feeaa 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -8582,7 +8582,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a)
>       * self-modifying code correctly and also to take
>       * any pending interrupts immediately.
>       */
> -    gen_goto_tb(s, 0, s->base.pc_next);
> +    gen_jmp(s, s->base.pc_next);
>      return true;
>  }
>
> @@ -8596,7 +8596,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a)
>       * for TCG; MB and end the TB instead.
>       */
>      tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> -    gen_goto_tb(s, 0, s->base.pc_next);
> +    gen_jmp(s, s->base.pc_next);
>      return true;

Why isn't it enough here just to set is_jmp to DISAS_NEXT ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64
  2021-06-30 18:32 ` [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64 Richard Henderson
@ 2021-07-08 12:08   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:08 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers

On Wed, 30 Jun 2021 at 19:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have not needed to end a TB for I/O since ba3e7926691
> ("icount: clean up cpu_can_io at the entry to the block"),
> and gdbstub singlestep is handled by the generic function.
>
> Drop the unused 'n' argument to use_goto_tb.
>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32
  2021-06-30 18:32 ` [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32 Richard Henderson
@ 2021-07-08 12:14   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:14 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers

On Wed, 30 Jun 2021 at 19:36, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/translate.c | 12 +-----------
>  1 file changed, 1 insertion(+), 11 deletions(-)

Now we end up checking for the single-stepping case twice: once in
gen_jmp_tb() etc, and then again indirectly in gen_goto_tb(), because
all the callsites to gen_goto_tb() carefully avoid calling it for
the is-singlestepping case. That suggests there's more cleanup
possible here.

For this patch, I guess
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb Richard Henderson
@ 2021-07-08 12:16   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:16 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Yoshinori Sato

On Wed, 30 Jun 2021 at 19:56, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/sh4/translate.c | 11 +++--------

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 21/28] target/rx: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 21/28] target/rx: " Richard Henderson
@ 2021-07-08 12:17   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:17 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Yoshinori Sato

On Wed, 30 Jun 2021 at 19:50, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb Richard Henderson
@ 2021-07-08 12:17   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:17 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Marek Vasut, Chris Wulff, QEMU Developers

On Wed, 30 Jun 2021 at 19:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: Chris Wulff <crwulff@gmail.com>
> Cc: Marek Vasut <marex@denx.de>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 15/28] target/mips: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 15/28] target/mips: " Richard Henderson
@ 2021-07-08 12:18   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:18 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Philippe Mathieu-Daudé

On Wed, 30 Jun 2021 at 19:48, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/mips/tcg/translate.c | 17 ++---------------

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 14/28] target/microblaze: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 14/28] target/microblaze: " Richard Henderson
@ 2021-07-08 12:21   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:21 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Edgar E . Iglesias, QEMU Developers

On Wed, 30 Jun 2021 at 19:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/microblaze/translate.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 13/28] target/m68k: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 13/28] target/m68k: " Richard Henderson
@ 2021-07-08 12:22   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:22 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers, Laurent Vivier

On Wed, 30 Jun 2021 at 19:44, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Acked-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 12/28] target/i386: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 12/28] target/i386: " Richard Henderson
@ 2021-07-08 12:26   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:26 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Paolo Bonzini, QEMU Developers, Eduardo Habkost

On Wed, 30 Jun 2021 at 19:39, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Just use translator_use_goto_tb directly at the one call site,
> rather than maintaining a local wrapper.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 11/28] target/hppa: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 11/28] target/hppa: " Richard Henderson
@ 2021-07-08 12:26   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:26 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On Wed, 30 Jun 2021 at 19:40, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/hppa/translate.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 10/28] target/cris: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 10/28] target/cris: Use translator_use_goto_tb Richard Henderson
@ 2021-07-08 12:27   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:27 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Edgar E . Iglesias, QEMU Developers

On Wed, 30 Jun 2021 at 19:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The test for singlestepping is done in translator_use_goto_tb,
> so we may elide it from cris_tr_tb_stop.
>
> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/cris/translate.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/target/cris/translate.c b/target/cris/translate.c
> index 4cfe5c86d9..e33a3bb326 100644
> --- a/target/cris/translate.c
> +++ b/target/cris/translate.c
> @@ -482,7 +482,7 @@ static void t_gen_swapr(TCGv d, TCGv s)
>
>  static bool use_goto_tb(DisasContext *dc, target_ulong dest)
>  {
> -    return ((dest ^ dc->base.pc_first) & TARGET_PAGE_MASK) == 0;
> +    return translator_use_goto_tb(&dc->base, dest);
>  }

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 08/28] target/avr: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 08/28] target/avr: Use translator_use_goto_tb Richard Henderson
@ 2021-07-08 12:28   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:28 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Michael Rolnik, QEMU Developers

On Wed, 30 Jun 2021 at 19:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Single stepping is not the only reason not to use goto_tb.
> If goto_tb is disallowed, and single-stepping is not enabled,
> then use tcg_gen_lookup_and_goto_tb to indirectly chain.
>
> Cc: Michael Rolnik <mrolnik@gmail.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb
  2021-06-30 18:32 ` [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb Richard Henderson
@ 2021-07-08 12:29   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:29 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On Wed, 30 Jun 2021 at 19:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/alpha/translate.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 02/28] target/alpha: Remove use_exit_tb
  2021-06-30 18:32 ` [PATCH v2 02/28] target/alpha: Remove use_exit_tb Richard Henderson
@ 2021-07-08 12:34   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:34 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On Wed, 30 Jun 2021 at 19:39, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> We have not needed to end a TB for I/O since ba3e7926691
> ("icount: clean up cpu_can_io at the entry to the block").
> We do not need to use exit_tb for singlestep, which only
> means generate one insn per TB.
>
> Which leaves only singlestep_enabled, which means raise a
> debug trap after every TB, which does not use exit_tb,
> which would leave the function mis-named.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

I wonder if we should rename the 'singlestep' global to
'tcg_one_insn_per_tb' or something (and perhaps also provide a
new command line flag, with the old one deprecated). It's very
misleadingly named...

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 03/28] target/alpha: Remove in_superpage
  2021-06-30 18:32 ` [PATCH v2 03/28] target/alpha: Remove in_superpage Richard Henderson
@ 2021-07-08 12:35   ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 12:35 UTC (permalink / raw)
  To: Richard Henderson; +Cc: QEMU Developers

On Wed, 30 Jun 2021 at 19:43, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The number of links across (normal) pages using this is low,
> and it will shortly violate the contract for breakpoints.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-07-08 12:05   ` Peter Maydell
@ 2021-07-08 16:04     ` Richard Henderson
  2021-07-08 16:11       ` Peter Maydell
  0 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-07-08 16:04 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, QEMU Developers

On 7/8/21 5:05 AM, Peter Maydell wrote:
> On Wed, 30 Jun 2021 at 19:47, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Using gen_goto_tb directly misses the single-step check.
>>
>> Cc: qemu-arm@nongnu.org
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/translate.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/arm/translate.c b/target/arm/translate.c
>> index a0c6cfa902..8cd31feeaa 100644
>> --- a/target/arm/translate.c
>> +++ b/target/arm/translate.c
>> @@ -8582,7 +8582,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a)
>>        * self-modifying code correctly and also to take
>>        * any pending interrupts immediately.
>>        */
>> -    gen_goto_tb(s, 0, s->base.pc_next);
>> +    gen_jmp(s, s->base.pc_next);
>>       return true;
>>   }
>>
>> @@ -8596,7 +8596,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a)
>>        * for TCG; MB and end the TB instead.
>>        */
>>       tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
>> -    gen_goto_tb(s, 0, s->base.pc_next);
>> +    gen_jmp(s, s->base.pc_next);
>>       return true;
> 
> Why isn't it enough here just to set is_jmp to DISAS_NEXT ?

You mean DISAS_TOO_MANY?  That would work, yes.
At the time I was just thinking of replacing one jump with another.


r~



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-07-08 16:04     ` Richard Henderson
@ 2021-07-08 16:11       ` Peter Maydell
  2021-07-08 16:17         ` Richard Henderson
  0 siblings, 1 reply; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 16:11 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers

On Thu, 8 Jul 2021 at 17:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/8/21 5:05 AM, Peter Maydell wrote:
> > On Wed, 30 Jun 2021 at 19:47, Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> Using gen_goto_tb directly misses the single-step check.
> >>
> >> Cc: qemu-arm@nongnu.org
> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> >> ---
> >>   target/arm/translate.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/arm/translate.c b/target/arm/translate.c
> >> index a0c6cfa902..8cd31feeaa 100644
> >> --- a/target/arm/translate.c
> >> +++ b/target/arm/translate.c
> >> @@ -8582,7 +8582,7 @@ static bool trans_ISB(DisasContext *s, arg_ISB *a)
> >>        * self-modifying code correctly and also to take
> >>        * any pending interrupts immediately.
> >>        */
> >> -    gen_goto_tb(s, 0, s->base.pc_next);
> >> +    gen_jmp(s, s->base.pc_next);
> >>       return true;
> >>   }
> >>
> >> @@ -8596,7 +8596,7 @@ static bool trans_SB(DisasContext *s, arg_SB *a)
> >>        * for TCG; MB and end the TB instead.
> >>        */
> >>       tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> >> -    gen_goto_tb(s, 0, s->base.pc_next);
> >> +    gen_jmp(s, s->base.pc_next);
> >>       return true;
> >
> > Why isn't it enough here just to set is_jmp to DISAS_NEXT ?
>
> You mean DISAS_TOO_MANY?  That would work, yes.
> At the time I was just thinking of replacing one jump with another.

You've implicitly answered my question, which is that the main
translator loop code treats DISAS_NEXT as "keep adding insns to
the TB" :-)

It feels slightly like misuse to use DISAS_TOO_MANY, unless we
renamed it to something like DISAS_END_TB (which is what it's
actually doing).

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-07-08 16:11       ` Peter Maydell
@ 2021-07-08 16:17         ` Richard Henderson
  2021-07-08 17:47           ` Peter Maydell
  0 siblings, 1 reply; 53+ messages in thread
From: Richard Henderson @ 2021-07-08 16:17 UTC (permalink / raw)
  To: Peter Maydell; +Cc: qemu-arm, QEMU Developers

On 7/8/21 9:11 AM, Peter Maydell wrote:
>>> Why isn't it enough here just to set is_jmp to DISAS_NEXT ?
>>
>> You mean DISAS_TOO_MANY?  That would work, yes.
>> At the time I was just thinking of replacing one jump with another.
> 
> You've implicitly answered my question, which is that the main
> translator loop code treats DISAS_NEXT as "keep adding insns to
> the TB" :-)
> 
> It feels slightly like misuse to use DISAS_TOO_MANY, unless we
> renamed it to something like DISAS_END_TB (which is what it's
> actually doing).

Yeah, better naming would have been a good.  In this instance I think I chose an odd 
colour for the bike shed.

The problem with just DISAS_END_TB is that there are many ways to end a tb, with at least: 
(1) goto_tb next, (2) goto_ptr next, (3) exit_tb.  We wind up replicating these three 
across many of the targets, so it would be a really nice cleanup to standardize, and with 
good names.


r~



^ permalink raw reply	[flat|nested] 53+ messages in thread

* Re: [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB
  2021-07-08 16:17         ` Richard Henderson
@ 2021-07-08 17:47           ` Peter Maydell
  0 siblings, 0 replies; 53+ messages in thread
From: Peter Maydell @ 2021-07-08 17:47 UTC (permalink / raw)
  To: Richard Henderson; +Cc: qemu-arm, QEMU Developers

On Thu, 8 Jul 2021 at 17:17, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 7/8/21 9:11 AM, Peter Maydell wrote:
> >>> Why isn't it enough here just to set is_jmp to DISAS_NEXT ?
> >>
> >> You mean DISAS_TOO_MANY?  That would work, yes.
> >> At the time I was just thinking of replacing one jump with another.
> >
> > You've implicitly answered my question, which is that the main
> > translator loop code treats DISAS_NEXT as "keep adding insns to
> > the TB" :-)
> >
> > It feels slightly like misuse to use DISAS_TOO_MANY, unless we
> > renamed it to something like DISAS_END_TB (which is what it's
> > actually doing).
>
> Yeah, better naming would have been a good.  In this instance I think I chose an odd
> colour for the bike shed.
>
> The problem with just DISAS_END_TB is that there are many ways to end a tb, with at least:
> (1) goto_tb next, (2) goto_ptr next, (3) exit_tb.  We wind up replicating these three
> across many of the targets, so it would be a really nice cleanup to standardize, and with
> good names.

Mmm. Anyway, for this change

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM


^ permalink raw reply	[flat|nested] 53+ messages in thread

end of thread, other threads:[~2021-07-08 18:14 UTC | newest]

Thread overview: 53+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-30 18:31 [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson
2021-06-30 18:31 ` [PATCH v2 01/28] " Richard Henderson
2021-06-30 18:32 ` [PATCH v2 02/28] target/alpha: Remove use_exit_tb Richard Henderson
2021-07-08 12:34   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 03/28] target/alpha: Remove in_superpage Richard Henderson
2021-07-08 12:35   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 04/28] target/alpha: Use translator_use_goto_tb Richard Henderson
2021-07-08 12:29   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 05/28] target/arm: Use gen_jmp for ISB and SB Richard Henderson
2021-07-08 12:05   ` Peter Maydell
2021-07-08 16:04     ` Richard Henderson
2021-07-08 16:11       ` Peter Maydell
2021-07-08 16:17         ` Richard Henderson
2021-07-08 17:47           ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 06/28] target/arm: Use translator_use_goto_tb for aarch64 Richard Henderson
2021-07-08 12:08   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 07/28] target/arm: Use translator_use_goto_tb for aarch32 Richard Henderson
2021-07-08 12:14   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 08/28] target/avr: Use translator_use_goto_tb Richard Henderson
2021-07-08 12:28   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 09/28] target/avr: Mark some helpers noreturn Richard Henderson
2021-06-30 18:32 ` [PATCH v2 10/28] target/cris: Use translator_use_goto_tb Richard Henderson
2021-07-08 12:27   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 11/28] target/hppa: " Richard Henderson
2021-07-08 12:26   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 12/28] target/i386: " Richard Henderson
2021-07-08 12:26   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 13/28] target/m68k: " Richard Henderson
2021-07-08 12:22   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 14/28] target/microblaze: " Richard Henderson
2021-07-08 12:21   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 15/28] target/mips: " Richard Henderson
2021-07-08 12:18   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 16/28] target/mips: Fix missing else in gen_goto_tb Richard Henderson
2021-06-30 18:32 ` [PATCH v2 17/28] target/nios2: Use translator_use_goto_tb Richard Henderson
2021-07-08 12:17   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 18/28] target/openrisc: " Richard Henderson
2021-07-07 20:59   ` Stafford Horne
2021-06-30 18:32 ` [PATCH v2 19/28] target/ppc: " Richard Henderson
2021-06-30 18:32 ` [PATCH v2 20/28] target/riscv: " Richard Henderson
2021-06-30 18:32 ` [PATCH v2 21/28] target/rx: " Richard Henderson
2021-07-08 12:17   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 22/28] target/s390x: " Richard Henderson
2021-07-01  7:42   ` David Hildenbrand
2021-06-30 18:32 ` [PATCH v2 23/28] target/s390x: Remove use_exit_tb Richard Henderson
2021-07-01  7:44   ` David Hildenbrand
2021-06-30 18:32 ` [PATCH v2 24/28] target/sh4: Use translator_use_goto_tb Richard Henderson
2021-07-08 12:16   ` Peter Maydell
2021-06-30 18:32 ` [PATCH v2 25/28] target/sparc: " Richard Henderson
2021-06-30 18:32 ` [PATCH v2 26/28] target/tricore: " Richard Henderson
2021-06-30 18:32 ` [PATCH v2 27/28] target/tricore: Use tcg_gen_lookup_and_goto_ptr Richard Henderson
2021-06-30 18:32 ` [PATCH v2 28/28] target/xtensa: Use translator_use_goto_tb Richard Henderson
2021-07-07 21:41 ` [PATCH v2 00/28] accel/tcg: Introduce translator_use_goto_tb Richard Henderson

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.