core: Don't sleep with pending interrupts
authorJakob Gruber <jakob.gruber@gmail.com>
Wed, 1 Aug 2012 15:01:37 +0000 (17:01 +0200)
committerJakob Gruber <jakob.gruber@gmail.com>
Sun, 5 Aug 2012 11:00:53 +0000 (13:00 +0200)
This avoids an issue in which we would incorrectly enter a state in
which the CPU was sleeping and interrupts were disabled. It occurred
when an interrupt was raised and a SLEEP instruction processed before
the interrupt could be serviced (due to pending wait).

There are several disadvantages: if somebody adds new code which
switches the CPU to sleeping, there's a good chance of them forgetting
to add this check. Preventing sleep even if the interrupt is masked is
probably harmless but incorrect. Finally, on a real AVR it _is_ possible
to trigger an interrupt and then go to sleep in the next instruction
before the interrupt is serviced.

I'd actually prefer fixing this by moving the CPU interrupt wakeup from
avr_raise_interrupt() to avr_service_interrupts(), but that requires
further changes to avoid sleeping during avr->run() while an interrupt
is about to be serviced.

simavr/sim/sim_core.c

index 33073c8..757ed66 100644 (file)
@@ -820,7 +820,12 @@ avr_flashaddr_t avr_run_one(avr_t * avr)
                        } else switch (opcode) {
                                case 0x9588: { // SLEEP
                                        STATE("sleep\n");
-                                       avr->state = cpu_Sleeping;
+                                       /* Don't sleep if there are interrupts about to be serviced.
+                                        * Without this check, it was possible to incorrectly enter a state
+                                        * in which the cpu was sleeping and interrupts were disabled. For more
+                                        * details, see the commit message. */
+                                       if (!avr_has_pending_interrupts(avr) || !avr->sreg[S_I])
+                                               avr->state = cpu_Sleeping;
                                }       break;
                                case 0x9598: { // BREAK
                                        STATE("break\n");