[05/26] target/arm: Use translator_use_goto_tb

Commit Message

Richard Henderson June 21, 2021, 1:34 a.m. UTC

Put a wrapper in translate.h, which also checks for ss_active.
The ss_active test was incorrectly missing from the a32 version.

Cc: [e-mail]
Signed-off-by: Richard Henderson <[e-mail]>
---
 target/arm/translate.h     | 13 +++++++++++++
 target/arm/translate-a64.c | 22 +---------------------
 target/arm/translate.c     | 10 ----------
 3 files changed, 14 insertions(+), 31 deletions(-)

Comments

Peter Maydell June 21, 2021, 10:20 a.m. UTC | #1

On Mon, 21 Jun 2021 at 02:42, Richard Henderson
<[e-mail]> wrote:
>
> Put a wrapper in translate.h, which also checks for ss_active.
> The ss_active test was incorrectly missing from the a32 version.

True, but we almost never call gen_goto_tb() when ss_active
(which makes sense because we generate code to raise a singlestep
exception instead). The only cases I think where we do call
gen_goto_tb() are ISB and SB; everything else calls gen_jmp_tb()
or otherwise handles the is_singlestepping() case.

thanks
-- PMM

Patch

diff --git a/target/arm/translate.h b/target/arm/translate.h
index 2821b325e3..3a62f50faf 100644
--- a/target/arm/translate.h
+++ b/target/arm/translate.h
@@ -116,6 +116,19 @@  typedef struct DisasContext {
     TCGv_i64 tmp_a64[TMP_A64_MAX];
 } DisasContext;
 
+static inline bool use_goto_tb(DisasContext *s, target_ulong dest)
+{
+    /*
+     * No direct tb linking with singlestep.
+     * This handles the ARM debug architecture kind; the QEMU kind
+     * is handled inside translator_use_goto_tb.
+     */
+    if (s->ss_active) {
+        return false;
+    }
+    return translator_use_goto_tb(&s->base, dest);
+}
+
 typedef struct DisasCompare {
     TCGCond cond;
     TCGv_i32 value;
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 7f74d0e81a..ac58a86e59 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -386,32 +386,12 @@  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)
-{
-    /* 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)) {
-        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;
-}
-
 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);
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9e2cca7707..3ac7943d86 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -2511,16 +2511,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();