All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-6.2 00/23] tcg: gdb singlestep reorg
@ 2021-07-21  6:41 Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 01/23] accel/tcg: Handle gdb singlestep in cpu_tb_exec Richard Henderson
                   ` (22 more replies)
  0 siblings, 23 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

Based-on: <20210720195439.626594-1-richard.henderson@linaro.org>
("tcg: breakpoint reorg")

Consolidate all handling for gdb singlestep to 4 lines in cpu_tb_exec.
Drop all of the code from target/*.  Note that nios2 and sparc, lacked
support for gdb singlestep entirely, which was a bit of a surprise.


r~


Richard Henderson (23):
  accel/tcg: Handle gdb singlestep in cpu_tb_exec
  target/alpha: Drop checks for singlestep_enabled
  target/avr: Drop checks for singlestep_enabled
  target/cris: Drop checks for singlestep_enabled
  target/hexagon: Drop checks for singlestep_enabled
  target/arm: Drop checks for singlestep_enabled
  target/hppa: Drop checks for singlestep_enabled
  target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt
  target/i386: Drop check for singlestep_enabled
  target/m68k: Drop checks for singlestep_enabled
  target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP
  target/microblaze: Drop checks for singlestep_enabled
  target/mips: Fix single stepping
  target/mips: Drop exit checks for singlestep_enabled
  target/openrisc: Drop checks for singlestep_enabled
  target/ppc: Drop exit checks for singlestep_enabled
  target/riscv: Remove dead code after exception
  target/riscv: Remove exit_tb and lookup_and_goto_ptr
  target/rx: Drop checks for singlestep_enabled
  target/s390x: Drop check for singlestep_enabled
  target/sh4: Drop check for singlestep_enabled
  target/tricore: Drop check for singlestep_enabled
  target/xtensa: Drop check for singlestep_enabled

 target/i386/helper.h                          |  1 -
 target/rx/helper.h                            |  1 -
 target/sh4/helper.h                           |  1 -
 target/tricore/helper.h                       |  1 -
 accel/tcg/cpu-exec.c                          | 11 +++
 target/alpha/translate.c                      | 13 +---
 target/arm/translate-a64.c                    | 10 +--
 target/arm/translate.c                        | 36 ++-------
 target/avr/translate.c                        | 19 +----
 target/cris/translate.c                       | 16 ----
 target/hexagon/translate.c                    | 12 +--
 target/hppa/translate.c                       | 17 +----
 target/i386/tcg/misc_helper.c                 |  8 --
 target/i386/tcg/translate.c                   |  9 +--
 target/m68k/translate.c                       | 44 +++--------
 target/microblaze/translate.c                 | 18 +----
 target/mips/tcg/translate.c                   | 75 +++++++++----------
 target/openrisc/translate.c                   | 18 +----
 target/ppc/translate.c                        | 38 ++--------
 target/riscv/translate.c                      | 29 +------
 target/rx/op_helper.c                         |  8 --
 target/rx/translate.c                         | 12 +--
 target/s390x/tcg/translate.c                  |  8 +-
 target/sh4/op_helper.c                        |  5 --
 target/sh4/translate.c                        | 14 +---
 target/tricore/op_helper.c                    |  7 --
 target/tricore/translate.c                    | 14 +---
 target/xtensa/translate.c                     | 25 ++-----
 .../riscv/insn_trans/trans_privileged.c.inc   | 10 +--
 target/riscv/insn_trans/trans_rvi.c.inc       |  6 +-
 target/riscv/insn_trans/trans_rvv.c.inc       |  2 +-
 31 files changed, 118 insertions(+), 370 deletions(-)

-- 
2.25.1



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

* [PATCH for-6.2 01/23] accel/tcg: Handle gdb singlestep in cpu_tb_exec
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 02/23] target/alpha: Drop checks for singlestep_enabled Richard Henderson
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

Currently the change in cpu_tb_exec is masked by the debug exception
being raised by the translators.  But this allows us to remove that code.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/cpu-exec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index fc895cf51e..313f0b748e 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -383,6 +383,17 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)
             cc->set_pc(cpu, last_tb->pc);
         }
     }
+
+    /*
+     * If gdb single-step, and we haven't raised another exception,
+     * raise a debug exception.  Single-step with another exception
+     * is handled in cpu_handle_exception.
+     */
+    if (unlikely(cpu->singlestep_enabled) && cpu->exception_index == -1) {
+        cpu->exception_index = EXCP_DEBUG;
+        cpu_loop_exit(cpu);
+    }
+
     return last_tb;
 }
 
-- 
2.25.1



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

* [PATCH for-6.2 02/23] target/alpha: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 01/23] accel/tcg: Handle gdb singlestep in cpu_tb_exec Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 03/23] target/avr: " Richard Henderson
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index de6c0a8439..cfb0c3d675 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -2998,17 +2998,10 @@ 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 (!ctx->base.singlestep_enabled) {
-            tcg_gen_lookup_and_goto_ptr();
-            break;
-        }
-        /* FALLTHRU */
+        tcg_gen_lookup_and_goto_ptr();
+        break;
     case DISAS_PC_UPDATED_NOCHAIN:
-        if (ctx->base.singlestep_enabled) {
-            gen_excp_1(EXCP_DEBUG, 0);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
         break;
     default:
         g_assert_not_reached();
-- 
2.25.1



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

* [PATCH for-6.2 03/23] target/avr: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 01/23] accel/tcg: Handle gdb singlestep in cpu_tb_exec Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 02/23] target/alpha: Drop checks for singlestep_enabled Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21 18:00   ` Philippe Mathieu-Daudé
  2021-07-21  6:41 ` [PATCH for-6.2 04/23] target/cris: " Richard Henderson
                   ` (19 subsequent siblings)
  22 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/avr/translate.c b/target/avr/translate.c
index 1111e08b83..0403470dd8 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -1089,11 +1089,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         tcg_gen_exit_tb(tb, n);
     } else {
         tcg_gen_movi_i32(cpu_pc, dest);
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
     }
     ctx->base.is_jmp = DISAS_NORETURN;
 }
@@ -3011,17 +3007,10 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         tcg_gen_movi_tl(cpu_pc, ctx->npc);
         /* fall through */
     case DISAS_LOOKUP:
-        if (!ctx->base.singlestep_enabled) {
-            tcg_gen_lookup_and_goto_ptr();
-            break;
-        }
-        /* fall through */
+        tcg_gen_lookup_and_goto_ptr();
+        break;
     case DISAS_EXIT:
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
         break;
     default:
         g_assert_not_reached();
-- 
2.25.1



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

* [PATCH for-6.2 04/23] target/cris: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (2 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 03/23] target/avr: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 05/23] target/hexagon: " Richard Henderson
                   ` (18 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/cris/translate.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/target/cris/translate.c b/target/cris/translate.c
index a84b753349..59325b388a 100644
--- a/target/cris/translate.c
+++ b/target/cris/translate.c
@@ -3249,22 +3249,6 @@ static void cris_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
         }
     }
 
-    if (unlikely(dc->base.singlestep_enabled)) {
-        switch (is_jmp) {
-        case DISAS_TOO_MANY:
-        case DISAS_UPDATE_NEXT:
-            tcg_gen_movi_tl(env_pc, npc);
-            /* fall through */
-        case DISAS_JUMP:
-        case DISAS_UPDATE:
-            t_gen_raise_exception(EXCP_DEBUG);
-            return;
-        default:
-            break;
-        }
-        g_assert_not_reached();
-    }
-
     switch (is_jmp) {
     case DISAS_TOO_MANY:
         gen_goto_tb(dc, 0, npc);
-- 
2.25.1



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

* [PATCH for-6.2 05/23] target/hexagon: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (3 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 04/23] target/cris: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 06/23] target/arm: " Richard Henderson
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/hexagon/translate.c b/target/hexagon/translate.c
index 54fdcaa5e8..606fae6b06 100644
--- a/target/hexagon/translate.c
+++ b/target/hexagon/translate.c
@@ -71,11 +71,7 @@ static void gen_end_tb(DisasContext *ctx)
 {
     gen_exec_counters(ctx);
     tcg_gen_mov_tl(hex_gpr[HEX_REG_PC], hex_next_PC);
-    if (ctx->base.singlestep_enabled) {
-        gen_exception_raw(EXCP_DEBUG);
-    } else {
-        tcg_gen_exit_tb(NULL, 0);
-    }
+    tcg_gen_exit_tb(NULL, 0);
     ctx->base.is_jmp = DISAS_NORETURN;
 }
 
@@ -591,11 +587,7 @@ static void hexagon_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
     case DISAS_TOO_MANY:
         gen_exec_counters(ctx);
         tcg_gen_movi_tl(hex_gpr[HEX_REG_PC], ctx->base.pc_next);
-        if (ctx->base.singlestep_enabled) {
-            gen_exception_raw(EXCP_DEBUG);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
         break;
     case DISAS_NORETURN:
         break;
-- 
2.25.1



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

* [PATCH for-6.2 06/23] target/arm: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (4 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 05/23] target/hexagon: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 07/23] target/hppa: " Richard Henderson
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/translate-a64.c | 10 ++--------
 target/arm/translate.c     | 36 ++++++------------------------------
 2 files changed, 8 insertions(+), 38 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 422e2ac0c9..a7da3c5d45 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -404,8 +404,6 @@ static inline void gen_goto_tb(DisasContext *s, int n, uint64_t dest)
         gen_a64_set_pc_im(dest);
         if (s->ss_active) {
             gen_step_complete_exception(s);
-        } else if (s->base.singlestep_enabled) {
-            gen_exception_internal(EXCP_DEBUG);
         } else {
             tcg_gen_lookup_and_goto_ptr();
             s->base.is_jmp = DISAS_NORETURN;
@@ -14874,7 +14872,7 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
-    if (unlikely(dc->base.singlestep_enabled || dc->ss_active)) {
+    if (unlikely(dc->ss_active)) {
         /* Note that this means single stepping WFI doesn't halt the CPU.
          * For conditional branch insns this is harmless unreachable code as
          * gen_goto_tb() has already handled emitting the debug exception
@@ -14886,11 +14884,7 @@ static void aarch64_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
             /* fall through */
         case DISAS_EXIT:
         case DISAS_JUMP:
-            if (dc->base.singlestep_enabled) {
-                gen_exception_internal(EXCP_DEBUG);
-            } else {
-                gen_step_complete_exception(dc);
-            }
+            gen_step_complete_exception(dc);
             break;
         case DISAS_NORETURN:
             break;
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 351afa43a2..4fb2b846a3 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -338,7 +338,7 @@ static void gen_exception_internal(int excp)
     tcg_temp_free_i32(tcg_excp);
 }
 
-static void gen_step_complete_exception(DisasContext *s)
+static void gen_singlestep_exception(DisasContext *s)
 {
     /* We just completed step of an insn. Move from Active-not-pending
      * to Active-pending, and then also take the swstep exception.
@@ -354,30 +354,6 @@ static void gen_step_complete_exception(DisasContext *s)
     s->base.is_jmp = DISAS_NORETURN;
 }
 
-static void gen_singlestep_exception(DisasContext *s)
-{
-    /* Generate the right kind of exception for singlestep, which is
-     * either the architectural singlestep or EXCP_DEBUG for QEMU's
-     * gdb singlestepping.
-     */
-    if (s->ss_active) {
-        gen_step_complete_exception(s);
-    } else {
-        gen_exception_internal(EXCP_DEBUG);
-    }
-}
-
-static inline bool is_singlestepping(DisasContext *s)
-{
-    /* Return true if we are singlestepping either because of
-     * architectural singlestep or QEMU gdbstub singlestep. This does
-     * not include the command line '-singlestep' mode which is rather
-     * misnamed as it only means "one instruction per TB" and doesn't
-     * affect the code we generate.
-     */
-    return s->base.singlestep_enabled || s->ss_active;
-}
-
 void clear_eci_state(DisasContext *s)
 {
     /*
@@ -834,7 +810,7 @@ static inline void gen_bx_excret_final_code(DisasContext *s)
     /* Is the new PC value in the magic range indicating exception return? */
     tcg_gen_brcondi_i32(TCG_COND_GEU, cpu_R[15], min_magic, excret_label);
     /* No: end the TB as we would for a DISAS_JMP */
-    if (is_singlestepping(s)) {
+    if (s->ss_active) {
         gen_singlestep_exception(s);
     } else {
         tcg_gen_exit_tb(NULL, 0);
@@ -2603,7 +2579,7 @@ static void gen_goto_tb(DisasContext *s, int n, target_ulong dest)
 /* Jump, specifying which TB number to use if we gen_goto_tb() */
 static inline void gen_jmp_tb(DisasContext *s, uint32_t dest, int tbno)
 {
-    if (unlikely(is_singlestepping(s))) {
+    if (unlikely(s->ss_active)) {
         /* An indirect jump so that we still trigger the debug exception.  */
         gen_set_pc_im(s, dest);
         s->base.is_jmp = DISAS_JUMP;
@@ -9360,7 +9336,7 @@ static void arm_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     dc->page_start = dc->base.pc_first & TARGET_PAGE_MASK;
 
     /* If architectural single step active, limit to 1.  */
-    if (is_singlestepping(dc)) {
+    if (dc->ss_active) {
         dc->base.max_insns = 1;
     }
 
@@ -9684,7 +9660,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
          * insn codepath itself.
          */
         gen_bx_excret_final_code(dc);
-    } else if (unlikely(is_singlestepping(dc))) {
+    } else if (unlikely(dc->ss_active)) {
         /* Unconditional and "condition passed" instruction codepath. */
         switch (dc->base.is_jmp) {
         case DISAS_SWI:
@@ -9779,7 +9755,7 @@ static void arm_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
         /* "Condition failed" instruction codepath for the branch/trap insn */
         gen_set_label(dc->condlabel);
         gen_set_condexec(dc);
-        if (unlikely(is_singlestepping(dc))) {
+        if (unlikely(dc->ss_active)) {
             gen_set_pc_im(dc, dc->base.pc_next);
             gen_singlestep_exception(dc);
         } else {
-- 
2.25.1



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

* [PATCH for-6.2 07/23] target/hppa: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (5 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 06/23] target/arm: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21 17:50   ` Philippe Mathieu-Daudé
  2021-07-21  6:41 ` [PATCH for-6.2 08/23] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt Richard Henderson
                   ` (15 subsequent siblings)
  22 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index b18150ef8d..9e0524efef 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -817,11 +817,7 @@ static void gen_goto_tb(DisasContext *ctx, int which,
     } else {
         copy_iaoq_entry(cpu_iaoq_f, f, cpu_iaoq_b);
         copy_iaoq_entry(cpu_iaoq_b, b, ctx->iaoq_n_var);
-        if (ctx->base.singlestep_enabled) {
-            gen_excp_1(EXCP_DEBUG);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
     }
 }
 
@@ -2349,11 +2345,7 @@ static bool do_rfi(DisasContext *ctx, bool rfi_r)
         gen_helper_rfi(cpu_env);
     }
     /* Exit the TB to recognize new interrupts.  */
-    if (ctx->base.singlestep_enabled) {
-        gen_excp_1(EXCP_DEBUG);
-    } else {
-        tcg_gen_exit_tb(NULL, 0);
-    }
+    tcg_gen_exit_tb(NULL, 0);
     ctx->base.is_jmp = DISAS_NORETURN;
 
     return nullify_end(ctx);
@@ -4277,10 +4269,9 @@ static void hppa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         nullify_save(ctx);
         /* FALLTHRU */
     case DISAS_IAQ_N_UPDATED:
-        if (ctx->base.singlestep_enabled) {
-            gen_excp_1(EXCP_DEBUG);
-        } else if (is_jmp != DISAS_IAQ_N_STALE_EXIT) {
+        if (is_jmp != DISAS_IAQ_N_STALE_EXIT) {
             tcg_gen_lookup_and_goto_ptr();
+            break;
         }
         /* FALLTHRU */
     case DISAS_EXIT:
-- 
2.25.1



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

* [PATCH for-6.2 08/23] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (6 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 07/23] target/hppa: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 09/23] target/i386: Drop check for singlestep_enabled Richard Henderson
                   ` (14 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

We were using singlestep_enabled as a proxy for whether
translator_use_goto_tb would always return false.

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

diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index aacb605eee..4bd947cf86 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -8556,6 +8556,7 @@ static void i386_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
     DisasContext *dc = container_of(dcbase, DisasContext, base);
     CPUX86State *env = cpu->env_ptr;
     uint32_t flags = dc->base.tb->flags;
+    uint32_t cflags = tb_cflags(dc->base.tb);
     int cpl = (flags >> HF_CPL_SHIFT) & 3;
     int iopl = (flags >> IOPL_SHIFT) & 3;
 
@@ -8593,14 +8594,14 @@ static void i386_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
     dc->cpuid_ext3_features = env->features[FEAT_8000_0001_ECX];
     dc->cpuid_7_0_ebx_features = env->features[FEAT_7_0_EBX];
     dc->cpuid_xsave_features = env->features[FEAT_XSAVE];
-    dc->jmp_opt = !(dc->base.singlestep_enabled ||
+    dc->jmp_opt = !((cflags & CF_NO_GOTO_TB) ||
                     (flags & (HF_TF_MASK | HF_INHIBIT_IRQ_MASK)));
     /*
      * If jmp_opt, we want to handle each string instruction individually.
      * For icount also disable repz optimization so that each iteration
      * is accounted separately.
      */
-    dc->repz_opt = !dc->jmp_opt && !(tb_cflags(dc->base.tb) & CF_USE_ICOUNT);
+    dc->repz_opt = !dc->jmp_opt && !(cflags & CF_USE_ICOUNT);
 
     dc->T0 = tcg_temp_new();
     dc->T1 = tcg_temp_new();
-- 
2.25.1



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

* [PATCH for-6.2 09/23] target/i386: Drop check for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (7 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 08/23] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 10/23] target/m68k: Drop checks " Richard Henderson
                   ` (13 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/helper.h          | 1 -
 target/i386/tcg/misc_helper.c | 8 --------
 target/i386/tcg/translate.c   | 4 +---
 3 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/target/i386/helper.h b/target/i386/helper.h
index 574ff75615..ac3b4d1ee3 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -56,7 +56,6 @@ DEF_HELPER_2(syscall, void, env, int)
 DEF_HELPER_2(sysret, void, env, int)
 #endif
 DEF_HELPER_FLAGS_2(pause, TCG_CALL_NO_WG, noreturn, env, int)
-DEF_HELPER_FLAGS_1(debug, TCG_CALL_NO_WG, noreturn, env)
 DEF_HELPER_1(reset_rf, void, env)
 DEF_HELPER_FLAGS_3(raise_interrupt, TCG_CALL_NO_WG, noreturn, env, int, int)
 DEF_HELPER_FLAGS_2(raise_exception, TCG_CALL_NO_WG, noreturn, env, int)
diff --git a/target/i386/tcg/misc_helper.c b/target/i386/tcg/misc_helper.c
index baffa5d7ba..5769db5ace 100644
--- a/target/i386/tcg/misc_helper.c
+++ b/target/i386/tcg/misc_helper.c
@@ -110,14 +110,6 @@ void QEMU_NORETURN helper_pause(CPUX86State *env, int next_eip_addend)
     do_pause(env);
 }
 
-void QEMU_NORETURN helper_debug(CPUX86State *env)
-{
-    CPUState *cs = env_cpu(env);
-
-    cs->exception_index = EXCP_DEBUG;
-    cpu_loop_exit(cs);
-}
-
 uint64_t helper_rdpkru(CPUX86State *env, uint32_t ecx)
 {
     if ((env->cr[4] & CR4_PKE_MASK) == 0) {
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 4bd947cf86..0a200d2f68 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2660,9 +2660,7 @@ do_gen_eob_worker(DisasContext *s, bool inhibit, bool recheck_tf, bool jr)
     if (s->base.tb->flags & HF_RF_MASK) {
         gen_helper_reset_rf(cpu_env);
     }
-    if (s->base.singlestep_enabled) {
-        gen_helper_debug(cpu_env);
-    } else if (recheck_tf) {
+    if (recheck_tf) {
         gen_helper_rechecking_single_step(cpu_env);
         tcg_gen_exit_tb(NULL, 0);
     } else if (s->flags & HF_TF_MASK) {
-- 
2.25.1



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

* [PATCH for-6.2 10/23] target/m68k: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (8 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 09/23] target/i386: Drop check for singlestep_enabled Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  8:54   ` Laurent Vivier
  2021-07-21  6:41 ` [PATCH for-6.2 11/23] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP Richard Henderson
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/translate.c | 44 +++++++++--------------------------------
 1 file changed, 9 insertions(+), 35 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index c34d9aed61..c3d281a5f2 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -194,18 +194,6 @@ static void do_writebacks(DisasContext *s)
     }
 }
 
-static bool is_singlestepping(DisasContext *s)
-{
-    /*
-     * Return true if we are singlestepping either because of
-     * architectural singlestep or QEMU gdbstub singlestep. This does
-     * not include the command line '-singlestep' mode which is rather
-     * misnamed as it only means "one instruction per TB" and doesn't
-     * affect the code we generate.
-     */
-    return s->base.singlestep_enabled || s->ss_active;
-}
-
 /* is_jmp field values */
 #define DISAS_JUMP      DISAS_TARGET_0 /* only pc was modified dynamically */
 #define DISAS_EXIT      DISAS_TARGET_1 /* cpu state was modified dynamically */
@@ -320,20 +308,6 @@ static void gen_exception(DisasContext *s, uint32_t dest, int nr)
     s->base.is_jmp = DISAS_NORETURN;
 }
 
-static void gen_singlestep_exception(DisasContext *s)
-{
-    /*
-     * Generate the right kind of exception for singlestep, which is
-     * either the architectural singlestep or EXCP_DEBUG for QEMU's
-     * gdb singlestepping.
-     */
-    if (s->ss_active) {
-        gen_raise_exception(EXCP_TRACE);
-    } else {
-        gen_raise_exception(EXCP_DEBUG);
-    }
-}
-
 static inline void gen_addr_fault(DisasContext *s)
 {
     gen_exception(s, s->base.pc_next, EXCP_ADDRESS);
@@ -1522,10 +1496,10 @@ static void gen_exit_tb(DisasContext *s)
 /* Generate a jump to an immediate address.  */
 static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
 {
-    if (unlikely(is_singlestepping(s))) {
+    if (unlikely(s->ss_active)) {
         update_cc_op(s);
         tcg_gen_movi_i32(QREG_PC, dest);
-        gen_singlestep_exception(s);
+        gen_raise_exception(EXCP_TRACE);
     } else if (translator_use_goto_tb(&s->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(QREG_PC, dest);
@@ -6193,7 +6167,7 @@ static void m68k_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
 
     dc->ss_active = (M68K_SR_TRACE(env->sr) == M68K_SR_TRACE_ANY_INS);
     /* If architectural single step active, limit to 1 */
-    if (is_singlestepping(dc)) {
+    if (dc->ss_active) {
         dc->base.max_insns = 1;
     }
 }
@@ -6252,17 +6226,17 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
         break;
     case DISAS_TOO_MANY:
         update_cc_op(dc);
-        if (is_singlestepping(dc)) {
+        if (dc->ss_active) {
             tcg_gen_movi_i32(QREG_PC, dc->pc);
-            gen_singlestep_exception(dc);
+            gen_raise_exception(EXCP_TRACE);
         } else {
             gen_jmp_tb(dc, 0, dc->pc);
         }
         break;
     case DISAS_JUMP:
         /* We updated CC_OP and PC in gen_jmp/gen_jmp_im.  */
-        if (is_singlestepping(dc)) {
-            gen_singlestep_exception(dc);
+        if (dc->ss_active) {
+            gen_raise_exception(EXCP_TRACE);
         } else {
             tcg_gen_lookup_and_goto_ptr();
         }
@@ -6272,8 +6246,8 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
          * We updated CC_OP and PC in gen_exit_tb, but also modified
          * other state that may require returning to the main loop.
          */
-        if (is_singlestepping(dc)) {
-            gen_singlestep_exception(dc);
+        if (dc->ss_active) {
+            gen_raise_exception(EXCP_TRACE);
         } else {
             tcg_gen_exit_tb(NULL, 0);
         }
-- 
2.25.1



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

* [PATCH for-6.2 11/23] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (9 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 10/23] target/m68k: Drop checks " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 12/23] target/microblaze: Drop checks for singlestep_enabled Richard Henderson
                   ` (11 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

We were using singlestep_enabled as a proxy for whether
translator_use_goto_tb would always return false.

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

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index a14ffed784..7e465b629a 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1779,7 +1779,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
         break;
 
     case DISAS_JUMP:
-        if (dc->jmp_dest != -1 && !cs->singlestep_enabled) {
+        if (dc->jmp_dest != -1 && !(tb_cflags(dc->base.tb) & CF_NO_GOTO_TB)) {
             /* Direct jump. */
             tcg_gen_discard_i32(cpu_btarget);
 
@@ -1804,7 +1804,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
             return;
         }
 
-        /* Indirect jump (or direct jump w/ singlestep) */
+        /* Indirect jump (or direct jump w/ goto_tb disabled) */
         tcg_gen_mov_i32(cpu_pc, cpu_btarget);
         tcg_gen_discard_i32(cpu_btarget);
 
-- 
2.25.1



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

* [PATCH for-6.2 12/23] target/microblaze: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (10 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 11/23] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 13/23] target/mips: Fix single stepping Richard Henderson
                   ` (10 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 7e465b629a..437bbed6d6 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -126,12 +126,7 @@ static void gen_raise_hw_excp(DisasContext *dc, uint32_t esr_ec)
 
 static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
 {
-    if (dc->base.singlestep_enabled) {
-        TCGv_i32 tmp = tcg_const_i32(EXCP_DEBUG);
-        tcg_gen_movi_i32(cpu_pc, dest);
-        gen_helper_raise_exception(cpu_env, tmp);
-        tcg_temp_free_i32(tmp);
-    } else if (translator_use_goto_tb(&dc->base, 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);
@@ -1807,12 +1802,7 @@ static void mb_tr_tb_stop(DisasContextBase *dcb, CPUState *cs)
         /* Indirect jump (or direct jump w/ goto_tb disabled) */
         tcg_gen_mov_i32(cpu_pc, cpu_btarget);
         tcg_gen_discard_i32(cpu_btarget);
-
-        if (unlikely(cs->singlestep_enabled)) {
-            gen_raise_exception(dc, EXCP_DEBUG);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
         return;
 
     default:
-- 
2.25.1



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

* [PATCH for-6.2 13/23] target/mips: Fix single stepping
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (11 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 12/23] target/microblaze: Drop checks for singlestep_enabled Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled Richard Henderson
                   ` (9 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

As per an ancient comment in mips_tr_translate_insn about the
expectations of gdb, when restarting the insn in a delay slot
we also re-execute the branch.  Which means that we are
expected to execute two insns in this case.

This has been broken since 8b86d6d2580, where we forced max_insns
to 1 while single-stepping.  This resulted in an exit from the
translator loop after the branch but before the delay slot is
translated.

Increase the max_insns to 2 for this case.  In addition, bypass
the end-of-page check, for when the branch itself ends the page.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/translate.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 5b03545f09..4d1e08cfb1 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16162,6 +16162,16 @@ static void mips_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->default_tcg_memop_mask = (ctx->insn_flags & (ISA_MIPS_R6 |
                                   INSN_LOONGSON3A)) ? MO_UNALN : MO_ALIGN;
 
+    /*
+     * Execute a branch and its delay slot as a single instruction.
+     * This is what GDB expects and is consistent with what the
+     * hardware does (e.g. if a delay slot instruction faults, the
+     * reported PC is the PC of the branch).
+     */
+    if (ctx->base.singlestep_enabled && (ctx->hflags & MIPS_HFLAG_BMASK)) {
+        ctx->base.max_insns = 2;
+    }
+
     LOG_DISAS("\ntb %p idx %d hflags %04x\n", ctx->base.tb, ctx->mem_idx,
               ctx->hflags);
 }
@@ -16231,17 +16241,14 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     if (ctx->base.is_jmp != DISAS_NEXT) {
         return;
     }
+
     /*
-     * Execute a branch and its delay slot as a single instruction.
-     * This is what GDB expects and is consistent with what the
-     * hardware does (e.g. if a delay slot instruction faults, the
-     * reported PC is the PC of the branch).
+     * End the TB on (most) page crossings.
+     * See mips_tr_init_disas_context about single-stepping a branch
+     * together with its delay slot.
      */
-    if (ctx->base.singlestep_enabled &&
-        (ctx->hflags & MIPS_HFLAG_BMASK) == 0) {
-        ctx->base.is_jmp = DISAS_TOO_MANY;
-    }
-    if (ctx->base.pc_next - ctx->page_start >= TARGET_PAGE_SIZE) {
+    if (ctx->base.pc_next - ctx->page_start >= TARGET_PAGE_SIZE
+        && !ctx->base.singlestep_enabled) {
         ctx->base.is_jmp = DISAS_TOO_MANY;
     }
 }
-- 
2.25.1



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

* [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (12 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 13/23] target/mips: Fix single stepping Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21 17:51   ` Philippe Mathieu-Daudé
  2021-07-21  6:41 ` [PATCH for-6.2 15/23] target/openrisc: Drop " Richard Henderson
                   ` (8 subsequent siblings)
  22 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/translate.c | 50 +++++++++++++------------------------
 1 file changed, 18 insertions(+), 32 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 4d1e08cfb1..cc24d443cf 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -4954,12 +4954,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
         gen_save_pc(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();
     }
 }
 
@@ -11929,10 +11924,6 @@ static void gen_branch(DisasContext *ctx, int insn_bytes)
             } else {
                 tcg_gen_mov_tl(cpu_PC, btarget);
             }
-            if (ctx->base.singlestep_enabled) {
-                save_cpu_state(ctx, 0);
-                gen_helper_raise_exception_debug(cpu_env);
-            }
             tcg_gen_lookup_and_goto_ptr();
             break;
         default:
@@ -16257,28 +16248,23 @@ static void mips_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
-    if (ctx->base.singlestep_enabled && ctx->base.is_jmp != DISAS_NORETURN) {
-        save_cpu_state(ctx, ctx->base.is_jmp != DISAS_EXIT);
-        gen_helper_raise_exception_debug(cpu_env);
-    } else {
-        switch (ctx->base.is_jmp) {
-        case DISAS_STOP:
-            gen_save_pc(ctx->base.pc_next);
-            tcg_gen_lookup_and_goto_ptr();
-            break;
-        case DISAS_NEXT:
-        case DISAS_TOO_MANY:
-            save_cpu_state(ctx, 0);
-            gen_goto_tb(ctx, 0, ctx->base.pc_next);
-            break;
-        case DISAS_EXIT:
-            tcg_gen_exit_tb(NULL, 0);
-            break;
-        case DISAS_NORETURN:
-            break;
-        default:
-            g_assert_not_reached();
-        }
+    switch (ctx->base.is_jmp) {
+    case DISAS_STOP:
+        gen_save_pc(ctx->base.pc_next);
+        tcg_gen_lookup_and_goto_ptr();
+        break;
+    case DISAS_NEXT:
+    case DISAS_TOO_MANY:
+        save_cpu_state(ctx, 0);
+        gen_goto_tb(ctx, 0, ctx->base.pc_next);
+        break;
+    case DISAS_EXIT:
+        tcg_gen_exit_tb(NULL, 0);
+        break;
+    case DISAS_NORETURN:
+        break;
+    default:
+        g_assert_not_reached();
     }
 }
 
-- 
2.25.1



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

* [PATCH for-6.2 15/23] target/openrisc: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (13 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 16/23] target/ppc: Drop exit " Richard Henderson
                   ` (7 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
index d6ea536744..7e1aace63a 100644
--- a/target/openrisc/translate.c
+++ b/target/openrisc/translate.c
@@ -1659,11 +1659,7 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
             /* The jump destination is indirect/computed; use jmp_pc.  */
             tcg_gen_mov_tl(cpu_pc, jmp_pc);
             tcg_gen_discard_tl(jmp_pc);
-            if (unlikely(dc->base.singlestep_enabled)) {
-                gen_exception(dc, EXCP_DEBUG);
-            } else {
-                tcg_gen_lookup_and_goto_ptr();
-            }
+            tcg_gen_lookup_and_goto_ptr();
             break;
         }
         /* The jump destination is direct; use jmp_pc_imm.
@@ -1680,19 +1676,11 @@ static void openrisc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
             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();
-        }
+        tcg_gen_lookup_and_goto_ptr();
         break;
 
     case DISAS_EXIT:
-        if (unlikely(dc->base.singlestep_enabled)) {
-            gen_exception(dc, EXCP_DEBUG);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
         break;
     default:
         g_assert_not_reached();
-- 
2.25.1



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

* [PATCH for-6.2 16/23] target/ppc: Drop exit checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (14 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 15/23] target/openrisc: Drop " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 17/23] target/riscv: Remove dead code after exception Richard Henderson
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.
Reuse gen_debug_exception to handle architectural debug exceptions.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/translate.c | 38 ++++++++------------------------------
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 171b216e17..70dbb61a15 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -42,7 +42,6 @@
 
 #define CPU_SINGLE_STEP 0x1
 #define CPU_BRANCH_STEP 0x2
-#define GDBSTUB_SINGLE_STEP 0x4
 
 /* Include definitions for instructions classes and implementations flags */
 /* #define PPC_DEBUG_DISAS */
@@ -332,7 +331,7 @@ static uint32_t gen_prep_dbgex(DisasContext *ctx)
 
 static void gen_debug_exception(DisasContext *ctx)
 {
-    gen_helper_raise_exception(cpu_env, tcg_constant_i32(EXCP_DEBUG));
+    gen_helper_raise_exception(cpu_env, tcg_constant_i32(gen_prep_dbgex(ctx)));
     ctx->base.is_jmp = DISAS_NORETURN;
 }
 
@@ -4306,15 +4305,8 @@ static inline bool use_goto_tb(DisasContext *ctx, target_ulong dest)
 
 static void gen_lookup_and_goto_ptr(DisasContext *ctx)
 {
-    int sse = ctx->singlestep_enabled;
-    if (unlikely(sse)) {
-        if (sse & GDBSTUB_SINGLE_STEP) {
-            gen_debug_exception(ctx);
-        } else if (sse & (CPU_SINGLE_STEP | CPU_BRANCH_STEP)) {
-            gen_helper_raise_exception(cpu_env, tcg_constant_i32(gen_prep_dbgex(ctx)));
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+    if (unlikely(ctx->singlestep_enabled)) {
+        gen_debug_exception(ctx);
     } else {
         tcg_gen_lookup_and_goto_ptr();
     }
@@ -8543,17 +8535,11 @@ static void ppc_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->singlestep_enabled = 0;
     if ((hflags >> HFLAGS_SE) & 1) {
         ctx->singlestep_enabled |= CPU_SINGLE_STEP;
+        ctx->base.max_insns = 1;
     }
     if ((hflags >> HFLAGS_BE) & 1) {
         ctx->singlestep_enabled |= CPU_BRANCH_STEP;
     }
-    if (unlikely(ctx->base.singlestep_enabled)) {
-        ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
-    }
-
-    if (ctx->singlestep_enabled & (CPU_SINGLE_STEP | GDBSTUB_SINGLE_STEP)) {
-        ctx->base.max_insns = 1;
-    }
 }
 
 static void ppc_tr_tb_start(DisasContextBase *db, CPUState *cs)
@@ -8621,7 +8607,6 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     DisasJumpType is_jmp = ctx->base.is_jmp;
     target_ulong nip = ctx->base.pc_next;
-    int sse;
 
     if (is_jmp == DISAS_NORETURN) {
         /* We have already exited the TB. */
@@ -8629,8 +8614,8 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
     }
 
     /* Honor single stepping. */
-    sse = ctx->singlestep_enabled & (CPU_SINGLE_STEP | GDBSTUB_SINGLE_STEP);
-    if (unlikely(sse)) {
+    if (unlikely(ctx->singlestep_enabled & CPU_SINGLE_STEP)
+        && (nip <= 0x100 || nip > 0xf00)) {
         switch (is_jmp) {
         case DISAS_TOO_MANY:
         case DISAS_EXIT_UPDATE:
@@ -8644,15 +8629,8 @@ static void ppc_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
             g_assert_not_reached();
         }
 
-        if (sse & GDBSTUB_SINGLE_STEP) {
-            gen_debug_exception(ctx);
-            return;
-        }
-        /* else CPU_SINGLE_STEP... */
-        if (nip <= 0x100 || nip > 0xf00) {
-            gen_helper_raise_exception(cpu_env, tcg_constant_i32(gen_prep_dbgex(ctx)));
-            return;
-        }
+        gen_debug_exception(ctx);
+        return;
     }
 
     switch (is_jmp) {
-- 
2.25.1



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

* [PATCH for-6.2 17/23] target/riscv: Remove dead code after exception
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (15 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 16/23] target/ppc: Drop exit " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 18/23] target/riscv: Remove exit_tb and lookup_and_goto_ptr Richard Henderson
                   ` (5 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

We have already set DISAS_NORETURN in generate_exception,
which makes the exit_tb unreachable.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/insn_trans/trans_privileged.c.inc | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index 32312be202..a7afcb15ce 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -22,8 +22,6 @@ static bool trans_ecall(DisasContext *ctx, arg_ecall *a)
 {
     /* always generates U-level ECALL, fixed in do_interrupt handler */
     generate_exception(ctx, RISCV_EXCP_U_ECALL);
-    exit_tb(ctx); /* no chaining */
-    ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 }
 
@@ -60,13 +58,11 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
         post   = opcode_at(&ctx->base, post_addr);
     }
 
-    if  (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
+    if (pre == 0x01f01013 && ebreak == 0x00100073 && post == 0x40705013) {
         generate_exception(ctx, RISCV_EXCP_SEMIHOST);
     } else {
         generate_exception(ctx, RISCV_EXCP_BREAKPOINT);
     }
-    exit_tb(ctx); /* no chaining */
-    ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 }
 
-- 
2.25.1



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

* [PATCH for-6.2 18/23] target/riscv: Remove exit_tb and lookup_and_goto_ptr
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (16 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 17/23] target/riscv: Remove dead code after exception Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 19/23] target/rx: Drop checks for singlestep_enabled Richard Henderson
                   ` (4 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically, which means
we don't need to do anything in the wrappers.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/translate.c                      | 29 +------------------
 .../riscv/insn_trans/trans_privileged.c.inc   |  4 +--
 target/riscv/insn_trans/trans_rvi.c.inc       |  6 ++--
 target/riscv/insn_trans/trans_rvv.c.inc       |  2 +-
 4 files changed, 7 insertions(+), 34 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 6983be5723..b4cc79a494 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -131,33 +131,6 @@ static void generate_exception_mtval(DisasContext *ctx, int excp)
     ctx->base.is_jmp = DISAS_NORETURN;
 }
 
-static void gen_exception_debug(void)
-{
-    TCGv_i32 helper_tmp = tcg_const_i32(EXCP_DEBUG);
-    gen_helper_raise_exception(cpu_env, helper_tmp);
-    tcg_temp_free_i32(helper_tmp);
-}
-
-/* Wrapper around tcg_gen_exit_tb that handles single stepping */
-static void exit_tb(DisasContext *ctx)
-{
-    if (ctx->base.singlestep_enabled) {
-        gen_exception_debug();
-    } else {
-        tcg_gen_exit_tb(NULL, 0);
-    }
-}
-
-/* Wrapper around tcg_gen_lookup_and_goto_ptr that handles single stepping */
-static void lookup_and_goto_ptr(DisasContext *ctx)
-{
-    if (ctx->base.singlestep_enabled) {
-        gen_exception_debug();
-    } else {
-        tcg_gen_lookup_and_goto_ptr();
-    }
-}
-
 static void gen_exception_illegal(DisasContext *ctx)
 {
     generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST);
@@ -176,7 +149,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
         tcg_gen_movi_tl(cpu_pc, dest);
-        lookup_and_goto_ptr(ctx);
+        tcg_gen_lookup_and_goto_ptr();
     }
 }
 
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc b/target/riscv/insn_trans/trans_privileged.c.inc
index a7afcb15ce..75c6ef80a6 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -78,7 +78,7 @@ static bool trans_sret(DisasContext *ctx, arg_sret *a)
 
     if (has_ext(ctx, RVS)) {
         gen_helper_sret(cpu_pc, cpu_env, cpu_pc);
-        exit_tb(ctx); /* no chaining */
+        tcg_gen_exit_tb(NULL, 0); /* no chaining */
         ctx->base.is_jmp = DISAS_NORETURN;
     } else {
         return false;
@@ -94,7 +94,7 @@ static bool trans_mret(DisasContext *ctx, arg_mret *a)
 #ifndef CONFIG_USER_ONLY
     tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
     gen_helper_mret(cpu_pc, cpu_env, cpu_pc);
-    exit_tb(ctx); /* no chaining */
+    tcg_gen_exit_tb(NULL, 0); /* no chaining */
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 #else
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc b/target/riscv/insn_trans/trans_rvi.c.inc
index 6e736c9d0d..aef3e84ca8 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -72,7 +72,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
     if (a->rd != 0) {
         tcg_gen_movi_tl(cpu_gpr[a->rd], ctx->pc_succ_insn);
     }
-    lookup_and_goto_ptr(ctx);
+    tcg_gen_lookup_and_goto_ptr();
 
     if (misaligned) {
         gen_set_label(misaligned);
@@ -461,7 +461,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
      * however we need to end the translation block
      */
     tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
-    exit_tb(ctx);
+    tcg_gen_exit_tb(NULL, 0);
     ctx->base.is_jmp = DISAS_NORETURN;
     return true;
 }
@@ -481,7 +481,7 @@ static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 #define RISCV_OP_CSR_POST do {\
     gen_set_gpr(a->rd, dest); \
     tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn); \
-    exit_tb(ctx); \
+    tcg_gen_exit_tb(NULL, 0); \
     ctx->base.is_jmp = DISAS_NORETURN; \
     tcg_temp_free(source1); \
     tcg_temp_free(csr_store); \
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc
index 83d9a285ba..56eb775cef 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -42,7 +42,7 @@ static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl *a)
     gen_helper_vsetvl(dst, cpu_env, s1, s2);
     gen_set_gpr(a->rd, dst);
     tcg_gen_movi_tl(cpu_pc, ctx->pc_succ_insn);
-    lookup_and_goto_ptr(ctx);
+    tcg_gen_lookup_and_goto_ptr();
     ctx->base.is_jmp = DISAS_NORETURN;
 
     tcg_temp_free(s1);
-- 
2.25.1



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

* [PATCH for-6.2 19/23] target/rx: Drop checks for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (17 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 18/23] target/riscv: Remove exit_tb and lookup_and_goto_ptr Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 20/23] target/s390x: Drop check " Richard Henderson
                   ` (3 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/rx/helper.h    |  1 -
 target/rx/op_helper.c |  8 --------
 target/rx/translate.c | 12 ++----------
 3 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/target/rx/helper.h b/target/rx/helper.h
index f0b7ebbbf7..ebb4739474 100644
--- a/target/rx/helper.h
+++ b/target/rx/helper.h
@@ -2,7 +2,6 @@ DEF_HELPER_1(raise_illegal_instruction, noreturn, env)
 DEF_HELPER_1(raise_access_fault, noreturn, env)
 DEF_HELPER_1(raise_privilege_violation, noreturn, env)
 DEF_HELPER_1(wait, noreturn, env)
-DEF_HELPER_1(debug, noreturn, env)
 DEF_HELPER_2(rxint, noreturn, env, i32)
 DEF_HELPER_1(rxbrk, noreturn, env)
 DEF_HELPER_FLAGS_3(fadd, TCG_CALL_NO_WG, f32, env, f32, f32)
diff --git a/target/rx/op_helper.c b/target/rx/op_helper.c
index 4d315b4449..11f952d340 100644
--- a/target/rx/op_helper.c
+++ b/target/rx/op_helper.c
@@ -451,14 +451,6 @@ void QEMU_NORETURN helper_wait(CPURXState *env)
     raise_exception(env, EXCP_HLT, 0);
 }
 
-void QEMU_NORETURN helper_debug(CPURXState *env)
-{
-    CPUState *cs = env_cpu(env);
-
-    cs->exception_index = EXCP_DEBUG;
-    cpu_loop_exit(cs);
-}
-
 void QEMU_NORETURN helper_rxint(CPURXState *env, uint32_t vec)
 {
     raise_exception(env, 0x100 + vec, 0);
diff --git a/target/rx/translate.c b/target/rx/translate.c
index a3cf720455..5db8f79a82 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -150,11 +150,7 @@ static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
         tcg_gen_exit_tb(dc->base.tb, n);
     } else {
         tcg_gen_movi_i32(cpu_pc, dest);
-        if (dc->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
     }
     dc->base.is_jmp = DISAS_NORETURN;
 }
@@ -2331,11 +2327,7 @@ static void rx_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         gen_goto_tb(ctx, 0, dcbase->pc_next);
         break;
     case DISAS_JUMP:
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
         break;
     case DISAS_UPDATE:
         tcg_gen_movi_i32(cpu_pc, ctx->base.pc_next);
-- 
2.25.1



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

* [PATCH for-6.2 20/23] target/s390x: Drop check for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (18 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 19/23] target/rx: Drop checks for singlestep_enabled Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 21/23] target/sh4: " Richard Henderson
                   ` (2 subsequent siblings)
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

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

diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 0632b0374b..1894ed7a66 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -148,7 +148,6 @@ struct DisasContext {
     uint64_t pc_tmp;
     uint32_t ilen;
     enum cc_op cc_op;
-    bool do_debug;
 };
 
 /* Information carried about a condition to be evaluated.  */
@@ -6541,7 +6540,6 @@ static void s390x_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 
     dc->cc_op = CC_OP_DYNAMIC;
     dc->ex_value = dc->base.tb->cs_base;
-    dc->do_debug = dc->base.singlestep_enabled;
 }
 
 static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs)
@@ -6588,10 +6586,8 @@ static void s390x_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
         /* FALLTHRU */
     case DISAS_PC_CC_UPDATED:
         /* Exit the TB, either by raising a debug exception or by return.  */
-        if (dc->do_debug) {
-            gen_exception(EXCP_DEBUG);
-        } else if ((dc->base.tb->flags & FLAG_MASK_PER) ||
-                   dc->base.is_jmp == DISAS_PC_STALE_NOCHAIN) {
+        if ((dc->base.tb->flags & FLAG_MASK_PER) ||
+             dc->base.is_jmp == DISAS_PC_STALE_NOCHAIN) {
             tcg_gen_exit_tb(NULL, 0);
         } else {
             tcg_gen_lookup_and_goto_ptr();
-- 
2.25.1



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

* [PATCH for-6.2 21/23] target/sh4: Drop check for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (19 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 20/23] target/s390x: Drop check " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21 17:51   ` Philippe Mathieu-Daudé
  2021-07-21  6:41 ` [PATCH for-6.2 22/23] target/tricore: " Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 23/23] target/xtensa: " Richard Henderson
  22 siblings, 1 reply; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/sh4/helper.h    |  1 -
 target/sh4/op_helper.c |  5 -----
 target/sh4/translate.c | 14 +++-----------
 3 files changed, 3 insertions(+), 17 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index 1e768fcbc7..8d792f6b55 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -3,7 +3,6 @@ DEF_HELPER_1(raise_illegal_instruction, noreturn, env)
 DEF_HELPER_1(raise_slot_illegal_instruction, noreturn, env)
 DEF_HELPER_1(raise_fpu_disable, noreturn, env)
 DEF_HELPER_1(raise_slot_fpu_disable, noreturn, env)
-DEF_HELPER_1(debug, noreturn, env)
 DEF_HELPER_1(sleep, noreturn, env)
 DEF_HELPER_2(trapa, noreturn, env, i32)
 DEF_HELPER_1(exclusive, noreturn, env)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index c0cbb95382..c996dce7df 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -81,11 +81,6 @@ void helper_raise_slot_fpu_disable(CPUSH4State *env)
     raise_exception(env, 0x820, 0);
 }
 
-void helper_debug(CPUSH4State *env)
-{
-    raise_exception(env, EXCP_DEBUG, 0);
-}
-
 void helper_sleep(CPUSH4State *env)
 {
     CPUState *cs = env_cpu(env);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 8704fea1ca..3ce35b9e4c 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -240,9 +240,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
         tcg_gen_movi_i32(cpu_pc, dest);
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else if (use_exit_tb(ctx)) {
+        if (use_exit_tb(ctx)) {
             tcg_gen_exit_tb(NULL, 0);
         } else {
             tcg_gen_lookup_and_goto_ptr();
@@ -258,9 +256,7 @@ static void gen_jump(DisasContext * ctx)
 	   delayed jump as immediate jump are conditinal jumps */
 	tcg_gen_mov_i32(cpu_pc, cpu_delayed_pc);
         tcg_gen_discard_i32(cpu_delayed_pc);
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else if (use_exit_tb(ctx)) {
+        if (use_exit_tb(ctx)) {
             tcg_gen_exit_tb(NULL, 0);
         } else {
             tcg_gen_lookup_and_goto_ptr();
@@ -2324,11 +2320,7 @@ static void sh4_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
     switch (ctx->base.is_jmp) {
     case DISAS_STOP:
         gen_save_cpu_state(ctx, true);
-        if (ctx->base.singlestep_enabled) {
-            gen_helper_debug(cpu_env);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
         break;
     case DISAS_NEXT:
     case DISAS_TOO_MANY:
-- 
2.25.1



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

* [PATCH for-6.2 22/23] target/tricore: Drop check for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (20 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 21/23] target/sh4: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  2021-07-21  6:41 ` [PATCH for-6.2 23/23] target/xtensa: " Richard Henderson
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/tricore/helper.h    |  1 -
 target/tricore/op_helper.c |  7 -------
 target/tricore/translate.c | 14 +-------------
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/target/tricore/helper.h b/target/tricore/helper.h
index 78176aa17a..b64780c37d 100644
--- a/target/tricore/helper.h
+++ b/target/tricore/helper.h
@@ -153,4 +153,3 @@ DEF_HELPER_2(psw_write, void, env, i32)
 DEF_HELPER_1(psw_read, i32, env)
 /* Exceptions */
 DEF_HELPER_3(raise_exception_sync, noreturn, env, i32, i32)
-DEF_HELPER_2(qemu_excp, noreturn, env, i32)
diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c
index 32c2bc1699..9476d10d00 100644
--- a/target/tricore/op_helper.c
+++ b/target/tricore/op_helper.c
@@ -107,13 +107,6 @@ static void raise_exception_sync_helper(CPUTriCoreState *env, uint32_t class,
     raise_exception_sync_internal(env, class, tin, pc, 0);
 }
 
-void helper_qemu_excp(CPUTriCoreState *env, uint32_t excp)
-{
-    CPUState *cs = env_cpu(env);
-    cs->exception_index = excp;
-    cpu_loop_exit(cs);
-}
-
 /* Addressing mode helper */
 
 static uint16_t reverse16(uint16_t val)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index a0cc0f1cb3..07084407cb 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -3225,14 +3225,6 @@ static inline void gen_save_pc(target_ulong pc)
     tcg_gen_movi_tl(cpu_PC, pc);
 }
 
-static void generate_qemu_excp(DisasContext *ctx, int excp)
-{
-    TCGv_i32 tmp = tcg_const_i32(excp);
-    gen_helper_qemu_excp(cpu_env, tmp);
-    ctx->base.is_jmp = DISAS_NORETURN;
-    tcg_temp_free(tmp);
-}
-
 static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
 {
     if (translator_use_goto_tb(&ctx->base, dest)) {
@@ -3241,11 +3233,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
         tcg_gen_exit_tb(ctx->base.tb, n);
     } else {
         gen_save_pc(dest);
-        if (ctx->base.singlestep_enabled) {
-            generate_qemu_excp(ctx, EXCP_DEBUG);
-        } else {
-            tcg_gen_lookup_and_goto_ptr();
-        }
+        tcg_gen_lookup_and_goto_ptr();
     }
 }
 
-- 
2.25.1



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

* [PATCH for-6.2 23/23] target/xtensa: Drop check for singlestep_enabled
  2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
                   ` (21 preceding siblings ...)
  2021-07-21  6:41 ` [PATCH for-6.2 22/23] target/tricore: " Richard Henderson
@ 2021-07-21  6:41 ` Richard Henderson
  22 siblings, 0 replies; 30+ messages in thread
From: Richard Henderson @ 2021-07-21  6:41 UTC (permalink / raw)
  To: qemu-devel

GDB single-stepping is now handled generically.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/xtensa/translate.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/target/xtensa/translate.c b/target/xtensa/translate.c
index 20399d6a04..3fbf76f4dd 100644
--- a/target/xtensa/translate.c
+++ b/target/xtensa/translate.c
@@ -382,18 +382,14 @@ static void gen_jump_slot(DisasContext *dc, TCGv dest, int slot)
     if (dc->icount) {
         tcg_gen_mov_i32(cpu_SR[ICOUNT], dc->next_icount);
     }
-    if (dc->base.singlestep_enabled) {
-        gen_exception(dc, EXCP_DEBUG);
+    if (dc->op_flags & XTENSA_OP_POSTPROCESS) {
+        slot = gen_postprocess(dc, slot);
+    }
+    if (slot >= 0) {
+        tcg_gen_goto_tb(slot);
+        tcg_gen_exit_tb(dc->base.tb, slot);
     } else {
-        if (dc->op_flags & XTENSA_OP_POSTPROCESS) {
-            slot = gen_postprocess(dc, slot);
-        }
-        if (slot >= 0) {
-            tcg_gen_goto_tb(slot);
-            tcg_gen_exit_tb(dc->base.tb, slot);
-        } else {
-            tcg_gen_exit_tb(NULL, 0);
-        }
+        tcg_gen_exit_tb(NULL, 0);
     }
     dc->base.is_jmp = DISAS_NORETURN;
 }
@@ -1292,12 +1288,7 @@ static void xtensa_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
     case DISAS_NORETURN:
         break;
     case DISAS_TOO_MANY:
-        if (dc->base.singlestep_enabled) {
-            tcg_gen_movi_i32(cpu_pc, dc->pc);
-            gen_exception(dc, EXCP_DEBUG);
-        } else {
-            gen_jumpi(dc, dc->pc, 0);
-        }
+        gen_jumpi(dc, dc->pc, 0);
         break;
     default:
         g_assert_not_reached();
-- 
2.25.1



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

* Re: [PATCH for-6.2 10/23] target/m68k: Drop checks for singlestep_enabled
  2021-07-21  6:41 ` [PATCH for-6.2 10/23] target/m68k: Drop checks " Richard Henderson
@ 2021-07-21  8:54   ` Laurent Vivier
  0 siblings, 0 replies; 30+ messages in thread
From: Laurent Vivier @ 2021-07-21  8:54 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 21/07/2021 à 08:41, Richard Henderson a écrit :
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/translate.c | 44 +++++++++--------------------------------
>  1 file changed, 9 insertions(+), 35 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index c34d9aed61..c3d281a5f2 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -194,18 +194,6 @@ static void do_writebacks(DisasContext *s)
>      }
>  }
>  
> -static bool is_singlestepping(DisasContext *s)
> -{
> -    /*
> -     * Return true if we are singlestepping either because of
> -     * architectural singlestep or QEMU gdbstub singlestep. This does
> -     * not include the command line '-singlestep' mode which is rather
> -     * misnamed as it only means "one instruction per TB" and doesn't
> -     * affect the code we generate.
> -     */
> -    return s->base.singlestep_enabled || s->ss_active;
> -}
> -
>  /* is_jmp field values */
>  #define DISAS_JUMP      DISAS_TARGET_0 /* only pc was modified dynamically */
>  #define DISAS_EXIT      DISAS_TARGET_1 /* cpu state was modified dynamically */
> @@ -320,20 +308,6 @@ static void gen_exception(DisasContext *s, uint32_t dest, int nr)
>      s->base.is_jmp = DISAS_NORETURN;
>  }
>  
> -static void gen_singlestep_exception(DisasContext *s)
> -{
> -    /*
> -     * Generate the right kind of exception for singlestep, which is
> -     * either the architectural singlestep or EXCP_DEBUG for QEMU's
> -     * gdb singlestepping.
> -     */
> -    if (s->ss_active) {
> -        gen_raise_exception(EXCP_TRACE);
> -    } else {
> -        gen_raise_exception(EXCP_DEBUG);
> -    }
> -}
> -
>  static inline void gen_addr_fault(DisasContext *s)
>  {
>      gen_exception(s, s->base.pc_next, EXCP_ADDRESS);
> @@ -1522,10 +1496,10 @@ static void gen_exit_tb(DisasContext *s)
>  /* Generate a jump to an immediate address.  */
>  static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
>  {
> -    if (unlikely(is_singlestepping(s))) {
> +    if (unlikely(s->ss_active)) {
>          update_cc_op(s);
>          tcg_gen_movi_i32(QREG_PC, dest);
> -        gen_singlestep_exception(s);
> +        gen_raise_exception(EXCP_TRACE);
>      } else if (translator_use_goto_tb(&s->base, dest)) {
>          tcg_gen_goto_tb(n);
>          tcg_gen_movi_i32(QREG_PC, dest);
> @@ -6193,7 +6167,7 @@ static void m68k_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>  
>      dc->ss_active = (M68K_SR_TRACE(env->sr) == M68K_SR_TRACE_ANY_INS);
>      /* If architectural single step active, limit to 1 */
> -    if (is_singlestepping(dc)) {
> +    if (dc->ss_active) {
>          dc->base.max_insns = 1;
>      }
>  }
> @@ -6252,17 +6226,17 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>          break;
>      case DISAS_TOO_MANY:
>          update_cc_op(dc);
> -        if (is_singlestepping(dc)) {
> +        if (dc->ss_active) {
>              tcg_gen_movi_i32(QREG_PC, dc->pc);
> -            gen_singlestep_exception(dc);
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              gen_jmp_tb(dc, 0, dc->pc);
>          }
>          break;
>      case DISAS_JUMP:
>          /* We updated CC_OP and PC in gen_jmp/gen_jmp_im.  */
> -        if (is_singlestepping(dc)) {
> -            gen_singlestep_exception(dc);
> +        if (dc->ss_active) {
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              tcg_gen_lookup_and_goto_ptr();
>          }
> @@ -6272,8 +6246,8 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>           * We updated CC_OP and PC in gen_exit_tb, but also modified
>           * other state that may require returning to the main loop.
>           */
> -        if (is_singlestepping(dc)) {
> -            gen_singlestep_exception(dc);
> +        if (dc->ss_active) {
> +            gen_raise_exception(EXCP_TRACE);
>          } else {
>              tcg_gen_exit_tb(NULL, 0);
>          }
> 

Acked-by: Laurent Vivier <laurent@vivier.eu>


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

* Re: [PATCH for-6.2 07/23] target/hppa: Drop checks for singlestep_enabled
  2021-07-21  6:41 ` [PATCH for-6.2 07/23] target/hppa: " Richard Henderson
@ 2021-07-21 17:50   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-21 17:50 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 7/21/21 8:41 AM, Richard Henderson wrote:
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/hppa/translate.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled
  2021-07-21  6:41 ` [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled Richard Henderson
@ 2021-07-21 17:51   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-21 17:51 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 7/21/21 8:41 AM, Richard Henderson wrote:
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/mips/tcg/translate.c | 50 +++++++++++++------------------------
>  1 file changed, 18 insertions(+), 32 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH for-6.2 21/23] target/sh4: Drop check for singlestep_enabled
  2021-07-21  6:41 ` [PATCH for-6.2 21/23] target/sh4: " Richard Henderson
@ 2021-07-21 17:51   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-21 17:51 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

On 7/21/21 8:41 AM, Richard Henderson wrote:
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/sh4/helper.h    |  1 -
>  target/sh4/op_helper.c |  5 -----
>  target/sh4/translate.c | 14 +++-----------
>  3 files changed, 3 insertions(+), 17 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


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

* Re: [PATCH for-6.2 03/23] target/avr: Drop checks for singlestep_enabled
  2021-07-21  6:41 ` [PATCH for-6.2 03/23] target/avr: " Richard Henderson
@ 2021-07-21 18:00   ` Philippe Mathieu-Daudé
  2021-07-22 11:18     ` Michael Rolnik
  0 siblings, 1 reply; 30+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-21 18:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Michael Rolnik
  Cc: Alex Bennée, Pavel Dovgalyuk

+Michael/Alex/Pavel

On 7/21/21 8:41 AM, Richard Henderson wrote:
> GDB single-stepping is now handled generically.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/avr/translate.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 1111e08b83..0403470dd8 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -1089,11 +1089,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest)
>          tcg_gen_exit_tb(tb, n);
>      } else {
>          tcg_gen_movi_i32(cpu_pc, dest);
> -        if (ctx->base.singlestep_enabled) {
> -            gen_helper_debug(cpu_env);
> -        } else {
> -            tcg_gen_lookup_and_goto_ptr();
> -        }
> +        tcg_gen_lookup_and_goto_ptr();
>      }
>      ctx->base.is_jmp = DISAS_NORETURN;
>  }
> @@ -3011,17 +3007,10 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, CPUState *cs)
>          tcg_gen_movi_tl(cpu_pc, ctx->npc);
>          /* fall through */
>      case DISAS_LOOKUP:
> -        if (!ctx->base.singlestep_enabled) {
> -            tcg_gen_lookup_and_goto_ptr();
> -            break;
> -        }
> -        /* fall through */
> +        tcg_gen_lookup_and_goto_ptr();
> +        break;
>      case DISAS_EXIT:
> -        if (ctx->base.singlestep_enabled) {
> -            gen_helper_debug(cpu_env);
> -        } else {
> -            tcg_gen_exit_tb(NULL, 0);
> -        }
> +        tcg_gen_exit_tb(NULL, 0);
>          break;
>      default:
>          g_assert_not_reached();
> 

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Not related to this patch, but looking at the last
gen_helper_debug() use:

/*
 *  The BREAK instruction is used by the On-chip Debug system, and is
 *  normally not used in the application software. When the BREAK
instruction is
 *  executed, the AVR CPU is set in the Stopped Mode. This gives the On-chip
 *  Debugger access to internal resources.  If any Lock bits are set, or
either
 *  the JTAGEN or OCDEN Fuses are unprogrammed, the CPU will treat the BREAK
 *  instruction as a NOP and will not enter the Stopped mode.  This
instruction
 *  is not available in all devices. Refer to the device specific
instruction
 *  set summary.
 */
static bool trans_BREAK(DisasContext *ctx, arg_BREAK *a)
{
    if (!avr_have_feature(ctx, AVR_FEATURE_BREAK)) {
        return true;
    }

#ifdef BREAKPOINT_ON_BREAK
    tcg_gen_movi_tl(cpu_pc, ctx->npc - 1);
    gen_helper_debug(cpu_env);
    ctx->base.is_jmp = DISAS_EXIT;
#else
    /* NOP */
#endif

    return true;
}

Shouldn't we have a generic 'bool gdbstub_is_attached()' in
"exec/gdbstub.h", then use it in replay_gdb_attached() and
trans_BREAK() instead of this BREAKPOINT_ON_BREAK build-time
definitions?


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

* Re: [PATCH for-6.2 03/23] target/avr: Drop checks for singlestep_enabled
  2021-07-21 18:00   ` Philippe Mathieu-Daudé
@ 2021-07-22 11:18     ` Michael Rolnik
  0 siblings, 0 replies; 30+ messages in thread
From: Michael Rolnik @ 2021-07-22 11:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alex Bennée, Richard Henderson, QEMU Developers, Pavel Dovgalyuk

[-- Attachment #1: Type: text/plain, Size: 3339 bytes --]

Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
Tested-by: Michael Rolnik <mrolnik@gmail.com>

On Wed, Jul 21, 2021 at 9:00 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> +Michael/Alex/Pavel
>
> On 7/21/21 8:41 AM, Richard Henderson wrote:
> > GDB single-stepping is now handled generically.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  target/avr/translate.c | 19 ++++---------------
> >  1 file changed, 4 insertions(+), 15 deletions(-)
> >
> > diff --git a/target/avr/translate.c b/target/avr/translate.c
> > index 1111e08b83..0403470dd8 100644
> > --- a/target/avr/translate.c
> > +++ b/target/avr/translate.c
> > @@ -1089,11 +1089,7 @@ static void gen_goto_tb(DisasContext *ctx, int n,
> target_ulong dest)
> >          tcg_gen_exit_tb(tb, n);
> >      } else {
> >          tcg_gen_movi_i32(cpu_pc, dest);
> > -        if (ctx->base.singlestep_enabled) {
> > -            gen_helper_debug(cpu_env);
> > -        } else {
> > -            tcg_gen_lookup_and_goto_ptr();
> > -        }
> > +        tcg_gen_lookup_and_goto_ptr();
> >      }
> >      ctx->base.is_jmp = DISAS_NORETURN;
> >  }
> > @@ -3011,17 +3007,10 @@ static void avr_tr_tb_stop(DisasContextBase
> *dcbase, CPUState *cs)
> >          tcg_gen_movi_tl(cpu_pc, ctx->npc);
> >          /* fall through */
> >      case DISAS_LOOKUP:
> > -        if (!ctx->base.singlestep_enabled) {
> > -            tcg_gen_lookup_and_goto_ptr();
> > -            break;
> > -        }
> > -        /* fall through */
> > +        tcg_gen_lookup_and_goto_ptr();
> > +        break;
> >      case DISAS_EXIT:
> > -        if (ctx->base.singlestep_enabled) {
> > -            gen_helper_debug(cpu_env);
> > -        } else {
> > -            tcg_gen_exit_tb(NULL, 0);
> > -        }
> > +        tcg_gen_exit_tb(NULL, 0);
> >          break;
> >      default:
> >          g_assert_not_reached();
> >
>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> Not related to this patch, but looking at the last
> gen_helper_debug() use:
>
> /*
>  *  The BREAK instruction is used by the On-chip Debug system, and is
>  *  normally not used in the application software. When the BREAK
> instruction is
>  *  executed, the AVR CPU is set in the Stopped Mode. This gives the
> On-chip
>  *  Debugger access to internal resources.  If any Lock bits are set, or
> either
>  *  the JTAGEN or OCDEN Fuses are unprogrammed, the CPU will treat the
> BREAK
>  *  instruction as a NOP and will not enter the Stopped mode.  This
> instruction
>  *  is not available in all devices. Refer to the device specific
> instruction
>  *  set summary.
>  */
> static bool trans_BREAK(DisasContext *ctx, arg_BREAK *a)
> {
>     if (!avr_have_feature(ctx, AVR_FEATURE_BREAK)) {
>         return true;
>     }
>
> #ifdef BREAKPOINT_ON_BREAK
>     tcg_gen_movi_tl(cpu_pc, ctx->npc - 1);
>     gen_helper_debug(cpu_env);
>     ctx->base.is_jmp = DISAS_EXIT;
> #else
>     /* NOP */
> #endif
>
>     return true;
> }
>
> Shouldn't we have a generic 'bool gdbstub_is_attached()' in
> "exec/gdbstub.h", then use it in replay_gdb_attached() and
> trans_BREAK() instead of this BREAKPOINT_ON_BREAK build-time
> definitions?
>


-- 
Best Regards,
Michael Rolnik

[-- Attachment #2: Type: text/html, Size: 4445 bytes --]

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

end of thread, other threads:[~2021-07-22 11:20 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21  6:41 [PATCH for-6.2 00/23] tcg: gdb singlestep reorg Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 01/23] accel/tcg: Handle gdb singlestep in cpu_tb_exec Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 02/23] target/alpha: Drop checks for singlestep_enabled Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 03/23] target/avr: " Richard Henderson
2021-07-21 18:00   ` Philippe Mathieu-Daudé
2021-07-22 11:18     ` Michael Rolnik
2021-07-21  6:41 ` [PATCH for-6.2 04/23] target/cris: " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 05/23] target/hexagon: " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 06/23] target/arm: " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 07/23] target/hppa: " Richard Henderson
2021-07-21 17:50   ` Philippe Mathieu-Daudé
2021-07-21  6:41 ` [PATCH for-6.2 08/23] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 09/23] target/i386: Drop check for singlestep_enabled Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 10/23] target/m68k: Drop checks " Richard Henderson
2021-07-21  8:54   ` Laurent Vivier
2021-07-21  6:41 ` [PATCH for-6.2 11/23] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 12/23] target/microblaze: Drop checks for singlestep_enabled Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 13/23] target/mips: Fix single stepping Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 14/23] target/mips: Drop exit checks for singlestep_enabled Richard Henderson
2021-07-21 17:51   ` Philippe Mathieu-Daudé
2021-07-21  6:41 ` [PATCH for-6.2 15/23] target/openrisc: Drop " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 16/23] target/ppc: Drop exit " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 17/23] target/riscv: Remove dead code after exception Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 18/23] target/riscv: Remove exit_tb and lookup_and_goto_ptr Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 19/23] target/rx: Drop checks for singlestep_enabled Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 20/23] target/s390x: Drop check " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 21/23] target/sh4: " Richard Henderson
2021-07-21 17:51   ` Philippe Mathieu-Daudé
2021-07-21  6:41 ` [PATCH for-6.2 22/23] target/tricore: " Richard Henderson
2021-07-21  6:41 ` [PATCH for-6.2 23/23] target/xtensa: " 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.