Fix avr_cpu_tlb_fill use of probe argument

Fix skip instructions being separated from the next insn (#1118)
 -----BEGIN PGP SIGNATURE-----
 
 iQFRBAABCgA7FiEEekgeeIaLTbaoWgXAZN846K9+IV8FAmMQRs4dHHJpY2hhcmQu
 aGVuZGVyc29uQGxpbmFyby5vcmcACgkQZN846K9+IV+7cAgAtlUxw9kNnIdrz1HG
 mkXO1kOfj0si8OHeAddy221lOL7zUm/Tw6vOdqxBsUjzkERLTNC6MhtVu6s3msyP
 Yi+Hh1lC9tk+YTYNnIeMqgEQYno3RFGAIaDHHRGQn8ha9PWWr0yGGaWTOZjm3Idf
 QYvFxiKfgTOEVekP4GYwkMsM02ItHu0hLLUUryKrQrCISNYzkF7AEtPxfxG4eDIr
 kN0QQndN5pfhRWnV6cvo6VVmAGz70YfKnlJgAFveeCZETYNpHP1npcsc4uj52JGk
 o0jxUSbZEzIbqLWSHqxa3KXydx/070sh0qmTmCzJSU7hOfmYpBHnT4ApHkijrIGI
 3lrrJw==
 =5lX1
 -----END PGP SIGNATURE-----

Merge tag 'pull-avr-20220901' of https://gitlab.com/rth7680/qemu into staging

Fix avr_cpu_tlb_fill use of probe argument
Fix skip instructions being separated from the next insn (#1118)

# -----BEGIN PGP SIGNATURE-----
#
# iQFRBAABCgA7FiEEekgeeIaLTbaoWgXAZN846K9+IV8FAmMQRs4dHHJpY2hhcmQu
# aGVuZGVyc29uQGxpbmFyby5vcmcACgkQZN846K9+IV+7cAgAtlUxw9kNnIdrz1HG
# mkXO1kOfj0si8OHeAddy221lOL7zUm/Tw6vOdqxBsUjzkERLTNC6MhtVu6s3msyP
# Yi+Hh1lC9tk+YTYNnIeMqgEQYno3RFGAIaDHHRGQn8ha9PWWr0yGGaWTOZjm3Idf
# QYvFxiKfgTOEVekP4GYwkMsM02ItHu0hLLUUryKrQrCISNYzkF7AEtPxfxG4eDIr
# kN0QQndN5pfhRWnV6cvo6VVmAGz70YfKnlJgAFveeCZETYNpHP1npcsc4uj52JGk
# o0jxUSbZEzIbqLWSHqxa3KXydx/070sh0qmTmCzJSU7hOfmYpBHnT4ApHkijrIGI
# 3lrrJw==
# =5lX1
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 01 Sep 2022 01:44:46 EDT
# gpg:                using RSA key 7A481E78868B4DB6A85A05C064DF38E8AF7E215F
# gpg:                issuer "richard.henderson@linaro.org"
# gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>" [full]
# Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A  05C0 64DF 38E8 AF7E 215F

* tag 'pull-avr-20220901' of https://gitlab.com/rth7680/qemu:
  target/avr: Disable interrupts when env->skip set
  target/avr: Only execute one interrupt at a time
  target/avr: Call avr_cpu_do_interrupt directly
  target/avr: Support probe argument to tlb_fill

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

target/avr/helper.c

@@ -28,36 +28,41 @@
 
 bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 {
-    bool ret = false;
-    CPUClass *cc = CPU_GET_CLASS(cs);
     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;
-            cc->tcg_ops->do_interrupt(cs);
+            avr_cpu_do_interrupt(cs);
 
             cs->interrupt_request &= ~CPU_INTERRUPT_RESET;
-
+            return true;
-            ret = true;
         }
     }
     if (interrupt_request & CPU_INTERRUPT_HARD) {
         if (cpu_interrupts_enabled(env) && env->intsrc != 0) {
             int index = ctz32(env->intsrc);
             cs->exception_index = EXCP_INT(index);
-            cc->tcg_ops->do_interrupt(cs);
+            avr_cpu_do_interrupt(cs);
 
             env->intsrc &= env->intsrc - 1; /* clear the interrupt */
             if (!env->intsrc) {
                 cs->interrupt_request &= ~CPU_INTERRUPT_HARD;
             }
-
+            return true;
-            ret = true;
         }
     }
-    return ret;
+    return false;
 }
 
 void avr_cpu_do_interrupt(CPUState *cs)
@@ -102,38 +107,50 @@ bool avr_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr)
 {
-    int prot = 0;
+    int prot, page_size = TARGET_PAGE_SIZE;
-    MemTxAttrs attrs = {};
     uint32_t paddr;
 
     address &= TARGET_PAGE_MASK;
 
     if (mmu_idx == MMU_CODE_IDX) {
-        /* access to code in flash */
+        /* Access to code in flash. */
         paddr = OFFSET_CODE + address;
         prot = PAGE_READ | PAGE_EXEC;
-        if (paddr + TARGET_PAGE_SIZE > OFFSET_DATA) {
+        if (paddr >= OFFSET_DATA) {
+            /*
+             * This should not be possible via any architectural operations.
+             * There is certainly not an exception that we can deliver.
+             * Accept probing that might come from generic code.
+             */
+            if (probe) {
+                return false;
+            }
             error_report("execution left flash memory");
             abort();
         }
-    } else if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
-        /*
-         * access to CPU registers, exit and rebuilt this TB to use full access
-         * incase it touches specially handled registers like SREG or SP
-         */
-        AVRCPU *cpu = AVR_CPU(cs);
-        CPUAVRState *env = &cpu->env;
-        env->fullacc = 1;
-        cpu_loop_exit_restore(cs, retaddr);
     } else {
-        /* access to memory. nothing special */
+        /* Access to memory. */
         paddr = OFFSET_DATA + address;
         prot = PAGE_READ | PAGE_WRITE;
+        if (address < NUMBER_OF_CPU_REGISTERS + NUMBER_OF_IO_REGISTERS) {
+            /*
+             * Access to CPU registers, exit and rebuilt this TB to use
+             * full access in case it touches specially handled registers
+             * like SREG or SP.  For probing, set page_size = 1, in order
+             * to force tlb_fill to be called for the next access.
+             */
+            if (probe) {
+                page_size = 1;
+            } else {
+                AVRCPU *cpu = AVR_CPU(cs);
+                CPUAVRState *env = &cpu->env;
+                env->fullacc = 1;
+                cpu_loop_exit_restore(cs, retaddr);
+            }
+        }
     }
 
-    tlb_set_page_with_attrs(cs, address, paddr, attrs, prot,
+    tlb_set_page(cs, address, paddr, prot, mmu_idx, page_size);
-                            mmu_idx, TARGET_PAGE_SIZE);
-
     return true;
 }
 

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();
+        if (!force_exit) {
-        break;
+            tcg_gen_lookup_and_goto_ptr();
+            break;
+        }
+        /* fall through */
     case DISAS_EXIT:
         tcg_gen_exit_tb(NULL, 0);
         break;