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.henderson@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.1Reviewed-by: Michael Rolnik <mrolnik@gmail.com>
On Fri, Aug 26, 2022 at 11:55 PM Richard Henderson <
richard.henderson@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.henderson@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
© 2016 - 2026 Red Hat, Inc.