altos: Leave interrupts disabled while checking for task to run

Otherwise, we run the risk of an interrupt waking a task after we've
decided to idle the CPU.

Signed-off-by: Keith Packard <keithp@keithp.com>

diff --git a/src/avr/ao_arch.h b/src/avr/ao_arch.h
index c82612a8ca12bf98666cc7f2970b147229ca6657..d626e830ee97bf4bdb7bf760ef2832fff698853a 100644 (file)
--- a/src/avr/ao_arch.h
+++ b/src/avr/ao_arch.h
@@ -112,7 +112,6 @@ extern uint8_t      ao_cpu_sleep_disable;
                asm("push r9" "\n\t" "push r8" "\n\t" "push r7" "\n\t" "push r6" "\n\t" "push r5"); \
                asm("push r4" "\n\t" "push r3" "\n\t" "push r2" "\n\t" "push r1" "\n\t" "push r0"); \
                asm("in r0, __SREG__" "\n\t" "push r0");                \
-               sei();                                                  \
        } while (0)
 
 #define ao_arch_save_stack() do {                                      \
@@ -124,16 +123,28 @@ extern uint8_t    ao_cpu_sleep_disable;
 
 #define ao_arch_isr_stack()    /* nothing */
 
-#define ao_arch_cpu_idle() do {                        \
-               if (!ao_cpu_sleep_disable)      \
+/* Idle the CPU (if possible) waiting for an interrupt. Enabling
+ * interrupts and sleeping the CPU must be adjacent to eliminate race
+ * conditions. In all cases, we execute a single nop with interrupts
+ * enabled
+ */
+#define ao_arch_wait_interrupt() do {          \
+               if (!ao_cpu_sleep_disable) {    \
+                       sleep_enable();         \
+                       sei();                  \
                        sleep_cpu();            \
+                       sleep_disable();        \
+               } else {                        \
+                       sei();                  \
+               }                               \
+               ao_arch_nop();                  \
+               cli();                          \
        } while (0)
 
 #define ao_arch_restore_stack() do { \
                uint8_t sp_l, sp_h;                                     \
                sp_l = (uint16_t) ao_cur_task->sp;                      \
                sp_h = ((uint16_t) ao_cur_task->sp) >> 8;               \
-               cli();                                                  \
                asm("out __SP_H__,%0" : : "r" (sp_h) );                 \
                asm("out __SP_L__,%0" : : "r" (sp_l) );                 \
                asm("pop r0"    "\n\t"                                  \

diff --git a/src/cc1111/ao_arch.h b/src/cc1111/ao_arch.h
index 39468e061c5ed64b4586db9ca7a14cdb87c7efd1..9097557ffd3460359f148aa8343dba3851b0a462 100644 (file)
--- a/src/cc1111/ao_arch.h
+++ b/src/cc1111/ao_arch.h
@@ -112,9 +112,7 @@ extern AO_ROMCONFIG_SYMBOL(0x00a6) uint32_t ao_radio_cal;
        /* Push ACC first, as when restoring the context it must be restored \
         * last (it is used to set the IE register). */                 \
        push    ACC                                                     \
-       /* Store the IE register then enable interrupts. */             \
        push    _IEN0                                                   \
-       setb    _EA                                                     \
        push    DPL                                                     \
        push    DPH                                                     \
        push    b                                                       \
@@ -150,11 +148,16 @@ extern AO_ROMCONFIG_SYMBOL(0x00a6) uint32_t ao_radio_cal;
 /* Empty the stack; might as well let interrupts have the whole thing */
 #define ao_arch_isr_stack()            (SP = AO_STACK_START - 1)
 
-/* Idle the CPU, waking when an interrupt occurs */
-#define ao_arch_cpu_idle()             (PCON = PCON_IDLE)
 #define ao_arch_block_interrupts()     __asm clr _EA __endasm
 #define ao_arch_release_interrupts()   __asm setb _EA __endasm
 
+/* Idle the CPU, waking when an interrupt occurs */
+#define ao_arch_wait_interrupt() do {          \
+               ao_arch_release_interrupts();   \
+               (PCON = PCON_IDLE);             \
+               ao_arch_block_interrupts();     \
+       } while (0)
+
 #define ao_arch_restore_stack() {                                      \
                uint8_t stack_len;                                      \
                __data uint8_t *stack_ptr;                              \

diff --git a/src/core/ao_task.c b/src/core/ao_task.c
index a11979f0a81fb78a5063ee9066189647c649285e..985c37fa79953849f6237dd475879e92aa7231cb 100644 (file)
--- a/src/core/ao_task.c
+++ b/src/core/ao_task.c
@@ -331,6 +331,7 @@ ao_yield(void) ao_arch_naked_define
        }
 
        ao_arch_isr_stack();
+       ao_arch_block_interrupts();
 
 #if AO_CHECK_STACK
        in_yield = 1;
@@ -339,21 +340,19 @@ ao_yield(void) ao_arch_naked_define
         * this loop will run forever, which is just fine
         */
 #if HAS_TASK_QUEUE
-       if (ao_cur_task->wchan == NULL) {
-               uint32_t flags;
-               flags = ao_arch_irqsave();
+       /* If the current task is running, move it to the
+        * end of the queue to allow other tasks a chance
+        */
+       if (ao_cur_task->wchan == NULL)
                ao_task_to_run_queue(ao_cur_task);
-               ao_arch_irqrestore(flags);
-       }
        ao_cur_task = NULL;
-
        for (;;) {
                ao_arch_memory_barrier();
                if (!ao_list_is_empty(&run_queue))
                        break;
-               ao_arch_cpu_idle();
+               /* Wait for interrupts when there's nothing ready */
+               ao_arch_wait_interrupt();
        }
-               
        ao_cur_task = ao_list_first_entry(&run_queue, struct ao_task, queue);
 #else
        {
@@ -374,20 +373,19 @@ ao_yield(void) ao_arch_naked_define
                            (int16_t) (ao_time() - ao_cur_task->alarm) >= 0)
                                break;
 
-                       /* Enter lower power mode when there isn't anything to do */
+                       /* Wait for interrupts when there's nothing ready */
                        if (ao_cur_task_index == ao_last_task_index)
-                               ao_arch_cpu_idle();
+                               ao_arch_wait_interrupt();
                }
-#if HAS_SAMPLE_PROFILE
-               ao_cur_task->start = ao_sample_profile_timer_value();
-#endif
        }
 #endif
+#if HAS_SAMPLE_PROFILE
+       ao_cur_task->start = ao_sample_profile_timer_value();
+#endif
 #if HAS_STACK_GUARD
        ao_mpu_stack_guard(ao_cur_task->stack);
 #endif
 #if AO_CHECK_STACK
-       ao_arch_block_interrupts();
        in_yield = 0;
 #endif
        ao_arch_restore_stack();
@@ -519,7 +517,7 @@ ao_task_info(void)
                       task->name,
                       (int) task->wchan);
        }
-#if HAS_TASK_QUEUE
+#if HAS_TASK_QUEUE && DEBUG
        ao_task_validate();
 #endif
 }

diff --git a/src/stm/ao_arch_funcs.h b/src/stm/ao_arch_funcs.h
index ca451a53239cf34c805bcfc7f89e1c3db1aae4bd..d6ab1465c73cd1b9a5f0c46080ba1ef780afd1aa 100644 (file)
--- a/src/stm/ao_arch_funcs.h
+++ b/src/stm/ao_arch_funcs.h
@@ -299,8 +299,10 @@ static inline void ao_arch_restore_stack(void) {
 
 #define ao_arch_isr_stack()
 
-#define ao_arch_cpu_idle() do {                        \
+#define ao_arch_wait_interrupt() do {                  \
                asm(".global ao_idle_loc\n\twfi\nao_idle_loc:");        \
+               ao_arch_release_interrupts();                           \
+               ao_arch_block_interrupts();                             \
        } while (0)
 
 #define ao_arch_critical(b) do {                               \