Reviewed-by: Michael Rolnik <mrol...@gmail.com> On Fri, Aug 26, 2022 at 11:55 PM Richard Henderson < richard.hender...@linaro.org> wrote:
> This bit is not saved across interrupts, so we must > delay delivering the interrupt until the skip has > been processed. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1118 > Signed-off-by: Richard Henderson <richard.hender...@linaro.org> > --- > target/avr/helper.c | 9 +++++++++ > target/avr/translate.c | 26 ++++++++++++++++++++++---- > 2 files changed, 31 insertions(+), 4 deletions(-) > > diff --git a/target/avr/helper.c b/target/avr/helper.c > index 34f1cbffb2..156dde4e92 100644 > --- a/target/avr/helper.c > +++ b/target/avr/helper.c > @@ -31,6 +31,15 @@ bool avr_cpu_exec_interrupt(CPUState *cs, int > interrupt_request) > AVRCPU *cpu = AVR_CPU(cs); > CPUAVRState *env = &cpu->env; > > + /* > + * We cannot separate a skip from the next instruction, > + * as the skip would not be preserved across the interrupt. > + * Separating the two insn normally only happens at page boundaries. > + */ > + if (env->skip) { > + return false; > + } > + > if (interrupt_request & CPU_INTERRUPT_RESET) { > if (cpu_interrupts_enabled(env)) { > cs->exception_index = EXCP_RESET; > diff --git a/target/avr/translate.c b/target/avr/translate.c > index dc9c3d6bcc..026753c963 100644 > --- a/target/avr/translate.c > +++ b/target/avr/translate.c > @@ -2971,8 +2971,18 @@ static void avr_tr_translate_insn(DisasContextBase > *dcbase, CPUState *cs) > if (skip_label) { > canonicalize_skip(ctx); > gen_set_label(skip_label); > - if (ctx->base.is_jmp == DISAS_NORETURN) { > + > + switch (ctx->base.is_jmp) { > + case DISAS_NORETURN: > ctx->base.is_jmp = DISAS_CHAIN; > + break; > + case DISAS_NEXT: > + if (ctx->base.tb->flags & TB_FLAGS_SKIP) { > + ctx->base.is_jmp = DISAS_TOO_MANY; > + } > + break; > + default: > + break; > } > } > > @@ -2989,6 +2999,11 @@ static void avr_tr_tb_stop(DisasContextBase > *dcbase, CPUState *cs) > { > DisasContext *ctx = container_of(dcbase, DisasContext, base); > bool nonconst_skip = canonicalize_skip(ctx); > + /* > + * Because we disable interrupts while env->skip is set, > + * we must return to the main loop to re-evaluate afterward. > + */ > + bool force_exit = ctx->base.tb->flags & TB_FLAGS_SKIP; > > switch (ctx->base.is_jmp) { > case DISAS_NORETURN: > @@ -2997,7 +3012,7 @@ static void avr_tr_tb_stop(DisasContextBase *dcbase, > CPUState *cs) > case DISAS_NEXT: > case DISAS_TOO_MANY: > case DISAS_CHAIN: > - if (!nonconst_skip) { > + if (!nonconst_skip && !force_exit) { > /* Note gen_goto_tb checks singlestep. */ > gen_goto_tb(ctx, 1, ctx->npc); > break; > @@ -3005,8 +3020,11 @@ static void avr_tr_tb_stop(DisasContextBase > *dcbase, CPUState *cs) > tcg_gen_movi_tl(cpu_pc, ctx->npc); > /* fall through */ > case DISAS_LOOKUP: > - tcg_gen_lookup_and_goto_ptr(); > - break; > + if (!force_exit) { > + tcg_gen_lookup_and_goto_ptr(); > + break; > + } > + /* fall through */ > case DISAS_EXIT: > tcg_gen_exit_tb(NULL, 0); > break; > -- > 2.34.1 > > -- Best Regards, Michael Rolnik
