From c9460d75c59fc6b6f110156e4c1035e9ae9bd611 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 15:12:12 -1000 Subject: [PATCH 01/24] accel/tcg: Handle gdb singlestep in cpu_tb_exec 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 --- 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 5fd1ed3422..c9764c1325 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; } From 1760e4abf11223535b92e25a7fbd928b29b57233 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 15:25:13 -1000 Subject: [PATCH 02/24] target/alpha: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 0eee3a1bcc..a4c3f43e72 100644 --- a/target/alpha/translate.c +++ b/target/alpha/translate.c @@ -3005,17 +3005,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(); From a893daa936161b8f45b2567875922343e388de9c Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 20:56:46 -1000 Subject: [PATCH 03/24] target/avr: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Tested-by: Michael Rolnik Reviewed-by: Michael Rolnik Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 438e7b13c1..af8a3e0f9c 100644 --- a/target/avr/translate.c +++ b/target/avr/translate.c @@ -1087,11 +1087,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; } @@ -3009,17 +3005,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(); From e3774881b5b651da7b44e5e98c5c8cea610e35bc Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 20:58:53 -1000 Subject: [PATCH 04/24] target/cris: Drop checks for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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); From db07bd026ee6f5ea4fe32c503645813252bae2c6 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 21:02:03 -1000 Subject: [PATCH 05/24] target/hexagon: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 4f05ce3388..159931e8ee 100644 --- a/target/hexagon/translate.c +++ b/target/hexagon/translate.c @@ -69,11 +69,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; } @@ -614,11 +610,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; From 364caea70f965a4fc81395863a3bae6bc16ce240 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 21:12:59 -1000 Subject: [PATCH 06/24] target/arm: Drop checks for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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 717afd481c..cec672f229 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; @@ -14879,7 +14877,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 @@ -14891,11 +14889,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 f7086c66a5..d6af5b1b03 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -341,7 +341,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. @@ -357,30 +357,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) { /* @@ -837,7 +813,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); @@ -2606,7 +2582,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; @@ -9459,7 +9435,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; } @@ -9794,7 +9770,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: @@ -9889,7 +9865,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 { From 8532a14e413b3395c3ee2f13285462259858f433 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 21:19:26 -1000 Subject: [PATCH 07/24] target/hppa: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 c3698cf067..3b9744deb4 100644 --- a/target/hppa/translate.c +++ b/target/hppa/translate.c @@ -814,11 +814,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(); } } @@ -2346,11 +2342,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); @@ -4274,10 +4266,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: From 9ef6c6ec08b8ecd89a797fa6819b0c1d121e24d7 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 15:59:08 -1000 Subject: [PATCH 08/24] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt We were using singlestep_enabled as a proxy for whether translator_use_goto_tb would always return false. Signed-off-by: Richard Henderson --- 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 a46be75b00..c8d919bc3f 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(); From 4bc4c3135be4b258ae8143b1f8982fe76cba460a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 16:04:29 -1000 Subject: [PATCH 09/24] target/i386: Drop check for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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 c8d919bc3f..e9e1451540 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) { From 661da0f63f973b84755423ae9e919582ff330a0c Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 18 Jul 2021 21:27:53 -1000 Subject: [PATCH 10/24] target/m68k: Drop checks for singlestep_enabled GDB single-stepping is now handled generically. Acked-by: Laurent Vivier Signed-off-by: Richard Henderson --- 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 50a55f949c..af43c8eab8 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); } From fbafb3a4d2fc90efdba05ec0d1f2ad7c4f68e9a5 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 06:16:42 -1000 Subject: [PATCH 11/24] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP We were using singlestep_enabled as a proxy for whether translator_use_goto_tb would always return false. Signed-off-by: Richard Henderson --- 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); From 66345580255f4a9f382cb9e8321a2590025eb1b4 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 06:17:20 -1000 Subject: [PATCH 12/24] target/microblaze: Drop checks for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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: From ef00cd4a22701923583862c48448b43bbcdaca0f Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:01:49 -1000 Subject: [PATCH 13/24] target/mips: Fix single stepping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 148afec9dc..f239f9ffc0 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -16016,6 +16016,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); } @@ -16085,17 +16095,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; } } From 1a79c413999e241bbc24487b4db08faec2493c9d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:04:32 -1000 Subject: [PATCH 14/24] target/mips: Drop exit checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 f239f9ffc0..0e59b97190 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -4823,12 +4823,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(); } } @@ -11788,10 +11783,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: @@ -16111,28 +16102,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(); } } From b21fce536c5e673b1f3faab6db9bd7a7aec88cc1 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:07:10 -1000 Subject: [PATCH 15/24] target/openrisc: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 5f3d430245..ca79e609da 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(); From 9498d1032c107166ffb72939f99929a4dd3f600a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:26:48 -1000 Subject: [PATCH 16/24] target/ppc: Drop exit checks for singlestep_enabled GDB single-stepping is now handled generically. Reuse gen_debug_exception to handle architectural debug exceptions. Signed-off-by: Richard Henderson --- 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 9ca78ee156..c3c6cb9589 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 */ @@ -333,7 +332,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; } @@ -4309,15 +4308,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(); } @@ -8563,17 +8555,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) @@ -8642,7 +8628,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. */ @@ -8650,8 +8635,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: @@ -8665,15 +8650,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) { From ea06a006525181826b1197a987d474bdaeb0acf7 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:34:13 -1000 Subject: [PATCH 17/24] target/riscv: Remove dead code after exception We have already set DISAS_NORETURN in generate_exception, which makes the exit_tb unreachable. Reviewed-by: Alistair Francis Signed-off-by: Richard Henderson --- 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; } From 273b68b10428117ffb1917d4081cf8ab4cc940ca Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:35:18 -1000 Subject: [PATCH 18/24] target/riscv: Remove exit_tb and lookup_and_goto_ptr GDB single-stepping is now handled generically, which means we don't need to do anything in the wrappers. Reviewed-by: Alistair Francis Signed-off-by: Richard Henderson --- .../riscv/insn_trans/trans_privileged.c.inc | 4 +-- target/riscv/insn_trans/trans_rvi.c.inc | 8 +++--- target/riscv/insn_trans/trans_rvv.c.inc | 2 +- target/riscv/translate.c | 27 +------------------ 4 files changed, 7 insertions(+), 34 deletions(-) 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 920ae0edb3..a6a57c94bb 100644 --- a/target/riscv/insn_trans/trans_rvi.c.inc +++ b/target/riscv/insn_trans/trans_rvi.c.inc @@ -71,9 +71,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); } - - /* No chaining with JALR. */ - lookup_and_goto_ptr(ctx); + tcg_gen_lookup_and_goto_ptr(); if (misaligned) { gen_set_label(misaligned); @@ -421,7 +419,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; } @@ -430,7 +428,7 @@ static bool do_csr_post(DisasContext *ctx) { /* We may have changed important cpu state -- exit to main loop. */ 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; } diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc index fa451938f1..081a5ca34d 100644 --- a/target/riscv/insn_trans/trans_rvv.c.inc +++ b/target/riscv/insn_trans/trans_rvv.c.inc @@ -41,7 +41,7 @@ static bool trans_vsetvl(DisasContext *ctx, arg_vsetvl *a) gen_set_gpr(ctx, 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; return true; } diff --git a/target/riscv/translate.c b/target/riscv/translate.c index d2442f0cf5..6d7fbca1fa 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -149,31 +149,6 @@ static void generate_exception_mtval(DisasContext *ctx, int excp) ctx->base.is_jmp = DISAS_NORETURN; } -static void gen_exception_debug(void) -{ - gen_helper_raise_exception(cpu_env, tcg_constant_i32(EXCP_DEBUG)); -} - -/* 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); @@ -192,7 +167,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(); } } From b6509e35b9e3a3fa90cdafcb8d53bf75306f427b Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:41:55 -1000 Subject: [PATCH 19/24] target/rx: Drop checks for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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); From 2df7a3618aacaa0f057d7b05444a21974a157263 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:52:20 -1000 Subject: [PATCH 20/24] target/s390x: Drop check for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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 a2d6fa5cca..dcc249a197 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -149,7 +149,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. */ @@ -6544,7 +6543,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) @@ -6596,10 +6594,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(); From 52df5adce9c028cec91cc0c10f7012b17212ea35 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:54:55 -1000 Subject: [PATCH 21/24] target/sh4: Drop check for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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 d363050272..ce5d674a52 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: From 1b55c52d607043210ed4579009191f99a5cf8f71 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 14:59:06 -1000 Subject: [PATCH 22/24] target/tricore: Drop check for singlestep_enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GDB single-stepping is now handled generically. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- 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(); } } From 02bf7fa022feae36b0f818b5ee30daccef8615fd Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 15:02:11 -1000 Subject: [PATCH 23/24] target/xtensa: Drop check for singlestep_enabled GDB single-stepping is now handled generically. Signed-off-by: Richard Henderson --- 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 dcf6b500ef..09430c1bf9 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; } @@ -1293,12 +1289,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(); From 995b87dedc78b0467f5f18bbc3546072ba97516a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sun, 22 Aug 2021 00:25:28 -0700 Subject: [PATCH 24/24] Revert "cpu: Move cpu_common_props to hw/core/cpu.c" This reverts commit 1b36e4f5a5de585210ea95f2257839c2312be28f. Despite a comment saying why cpu_common_props cannot be placed in a file that is compiled once, it was moved anyway. Revert that. Since then, Property is not defined in hw/core/cpu.h, so it is now easier to declare a function to install the properties rather than the Property array itself. Cc: Eduardo Habkost Suggested-by: Peter Maydell Signed-off-by: Richard Henderson --- cpu.c | 21 +++++++++++++++++++++ hw/core/cpu-common.c | 17 +---------------- include/hw/core/cpu.h | 1 + 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/cpu.c b/cpu.c index e1799a15bc..9bce67ef55 100644 --- a/cpu.c +++ b/cpu.c @@ -179,6 +179,27 @@ void cpu_exec_unrealizefn(CPUState *cpu) cpu_list_remove(cpu); } +static Property cpu_common_props[] = { +#ifndef CONFIG_USER_ONLY + /* + * Create a memory property for softmmu CPU object, + * so users can wire up its memory. (This can't go in hw/core/cpu.c + * because that file is compiled only once for both user-mode + * and system builds.) The default if no link is set up is to use + * the system address space. + */ + DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION, + MemoryRegion *), +#endif + DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false), + DEFINE_PROP_END_OF_LIST(), +}; + +void cpu_class_init_props(DeviceClass *dc) +{ + device_class_set_props(dc, cpu_common_props); +} + void cpu_exec_initfn(CPUState *cpu) { cpu->as = NULL; diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index e2f5a64604..9e3241b430 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -257,21 +257,6 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu) return cpu->cpu_index; } -static Property cpu_common_props[] = { -#ifndef CONFIG_USER_ONLY - /* Create a memory property for softmmu CPU object, - * so users can wire up its memory. (This can't go in hw/core/cpu.c - * because that file is compiled only once for both user-mode - * and system builds.) The default if no link is set up is to use - * the system address space. - */ - DEFINE_PROP_LINK("memory", CPUState, memory, TYPE_MEMORY_REGION, - MemoryRegion *), -#endif - DEFINE_PROP_BOOL("start-powered-off", CPUState, start_powered_off, false), - DEFINE_PROP_END_OF_LIST(), -}; - static void cpu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -286,7 +271,7 @@ static void cpu_class_init(ObjectClass *klass, void *data) dc->realize = cpu_common_realizefn; dc->unrealize = cpu_common_unrealizefn; dc->reset = cpu_common_reset; - device_class_set_props(dc, cpu_common_props); + cpu_class_init_props(dc); /* * Reason: CPUs still need special care by board code: wiring up * IRQs, adding reset handlers, halting non-first CPUs, ... diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index b7d5bc1200..1a10497af3 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1008,6 +1008,7 @@ void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...) GCC_FMT_ATTR(2, 3); /* $(top_srcdir)/cpu.c */ +void cpu_class_init_props(DeviceClass *dc); void cpu_exec_initfn(CPUState *cpu); void cpu_exec_realizefn(CPUState *cpu, Error **errp); void cpu_exec_unrealizefn(CPUState *cpu);