+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.hender...@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é <f4...@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?
