Fix a few silly mistakes

esp-backtrace/src/xtensa.rs

@@ -20,15 +20,15 @@ fn sp() -> u32 {
         asm!(
             "mov {0}, a1", // current stack pointer
             // Spill registers, otherwise `sp - 12` will not contain the previous stack pointer
-            "add a12,a12,a12",
+            "and a12,a12,a12",
             "rotw 3",
-            "add a12,a12,a12",
+            "and a12,a12,a12",
             "rotw 3",
-            "add a12,a12,a12",
+            "and a12,a12,a12",
             "rotw 3",
-            "add a12,a12,a12",
+            "and a12,a12,a12",
             "rotw 3",
-            "add a12,a12,a12",
+            "and a12,a12,a12",
             "rotw 4",
             out(reg) sp
         );

esp-rtos/src/lib.rs

@@ -412,7 +412,7 @@ pub fn start_with_idle_hook(
         let stack_bottom = (&raw const _stack_end_cpu0).cast::<MaybeUninit<u32>>();
         let stack_slice = core::ptr::slice_from_raw_parts_mut(
             stack_bottom.cast_mut(),
-            stack_top as usize - stack_bottom as usize,
+            (stack_top as usize - stack_bottom as usize) / 4,
         );
 
         task::allocate_main_task(
@@ -482,7 +482,7 @@ pub fn start_second_core_with_stack_guard_offset<const STACK_SIZE: usize>(
     let stack_ptrs = SecondCoreStack {
         stack: core::ptr::slice_from_raw_parts_mut(
             stack.bottom().cast::<MaybeUninit<u32>>(),
-            STACK_SIZE,
+            STACK_SIZE / 4,
         ),
     };
 

esp-rtos/src/scheduler.rs

@@ -65,6 +65,13 @@ pub(crate) struct CpuState {
     /// Pointer to the task that is scheduled for deletion.
     pub(crate) to_delete: TaskList<TaskDeleteListElement>,
 
+    /// Set while the CPU executes the idle context.
+    ///
+    /// The idle context has no `Task`, so the thread pointer is null while it runs. A task that
+    /// has deleted itself also has a null thread pointer, so the flag is needed to tell the two
+    /// apart.
+    idle: bool,
+
     // This context will be filled out by the first context switch.
     // We allocate the main task statically, because there is always a main task. If deleted, we
     // simply don't deallocate this.
@@ -77,6 +84,7 @@ impl CpuState {
             initialized: false,
             idle_context: CpuContext::new(),
             to_delete: TaskList::new(),
+            idle: false,
 
             #[cfg(multi_core)]
             current_task: core::ptr::null_mut(),
@@ -227,19 +235,34 @@ impl SchedulerState {
         }
 
         let current_task = NonNull::new(read_thread_pointer());
-        if let Some(current_task) = current_task {
+
+        // The idle task has no Task structure, and it has no stack of its own - it runs on the
+        // main task's stack. Check the main task in that case, so that a deep idle hook cannot
+        // overflow the main stack unnoticed. Before the main task is set up, there is no stack
+        // guard to check. A task that deleted itself also has no thread pointer, but it still runs
+        // on its own stack, which is about to be freed - there is nothing to check for it.
+        let stack_owner = match current_task {
+            Some(current_task) => Some(current_task),
+            None if self.per_cpu[current_cpu].idle => {
+                Some(NonNull::from(&self.per_cpu[current_cpu].main_task))
+            }
+            None => None,
+        };
+        if let Some(stack_owner) = stack_owner {
             unsafe {
-                current_task
+                stack_owner
                     .as_ref()
                     .ensure_no_stack_overflow(current_sp as usize)
             };
+        }
 
-            if current_task.state() == TaskState::Ready {
-                // Current task is still ready, mark it as such.
-                debug!("re-queueing current task: {:?}", current_task);
-                self.run_queue.mark_task_ready(&self.per_cpu, current_task);
-            }
-        };
+        if let Some(current_task) = current_task
+            && current_task.state() == TaskState::Ready
+        {
+            // Current task is still ready, mark it as such.
+            debug!("re-queueing current task: {:?}", current_task);
+            self.run_queue.mark_task_ready(&self.per_cpu, current_task);
+        }
 
         let mut arm_next_timeslice_tick = false;
         let next_task = self.run_queue.pop();
@@ -311,6 +334,8 @@ impl SchedulerState {
                 &raw mut self.per_cpu[current_cpu].idle_context
             };
 
+            self.per_cpu[current_cpu].idle = next_task.is_none();
+
             task_switch(current_context, next_context);
 
             // If we went to idle, this will be None and we won't mess up the main task's stack.
@@ -454,7 +479,8 @@ impl SchedulerState {
     #[cfg(all(multi_core, sleep_light_sleep))]
     pub(crate) fn cpu_idle(&self, cpu: Cpu) -> bool {
         let per_cpu = &self.per_cpu[cpu as usize];
-        !per_cpu.initialized || per_cpu.current_task.is_null()
+        // A CPU that never started the scheduler has no work to do, so it counts as idle.
+        !per_cpu.initialized || per_cpu.idle
     }
 }
 

esp-rtos/src/task/xtensa.rs

@@ -50,7 +50,8 @@ extern "C" fn idle_entry() -> ! {
 // Exception mode. Setting this bit prevents interrupts below EXCMLEVEL. Cleared by `rfe` at the end
 // of the Level 1 interrupt handler.
 const PS_EXCM: u32 = 1 << 4;
-// User mode. This bit doesn't matter for us yet, we don't have separate kernel mode exceptions.
+// User mode. Selects the user exception vector, instead of the kernel one. Both vectors point at
+// the same handler, but tasks must run with this bit set, because the interrupt handlers do, too.
 const PS_UM: u32 = 1 << 5;
 // Windowed mode.
 const PS_WOE: u32 = 1 << 18;

xtensa-lx-rt/src/exception/asm.rs

@@ -87,7 +87,7 @@ global_asm!(
     .set PS_INTLEVEL_EXCM, 3	        // interrupt handlers above this level shouldn't be written in high level languages
     .set PS_INTLEVEL_MASK, 0x0000000f
     .set PS_EXCM,          0x00000010
-    .set PS_UM,            0x00000020
+    .set PS_UM,            0x00000020      // user mode: general exceptions use the user, not the kernel vector
     .set PS_WOE,           0x00040000
 
     .set EXCCAUSE_LEVEL1_INTERRUPT, 4
@@ -193,7 +193,7 @@ global_asm!(
     .macro HANDLE_INTERRUPT_LEVEL level
     SAVE_CONTEXT \\level
 
-    movi    a0, (\\level | PS_WOE)
+    movi    a0, (\\level | PS_WOE | PS_UM)
     wsr     a0, PS
     rsync
 
@@ -343,6 +343,8 @@ save_context:
 
     // SPILL_REGISTERS macro requires window overflow exceptions to be enabled,
     // i.e. PS.EXCM cleared and PS.WOE set.
+    // We also set PS.UM, so that an exception during the spill uses the same
+    // vector as the rest of the handler.
     // Since we are going to clear PS.EXCM, we also need to increase INTLEVEL
     // at least to XCHAL_EXCM_LEVEL. This matches that value of effective INTLEVEL
     // at entry (CINTLEVEL=max(PS.INTLEVEL, XCHAL_EXCM_LEVEL) when PS.EXCM is set.
@@ -362,7 +364,7 @@ save_context:
     bgeui   a3, +PS_INTLEVEL_EXCM, 1f  // calculate max(INTLEVEL, XCHAL_EXCM_LEVEL) - 3 = XCHAL_EXCM_LEVEL
     movi    a3, PS_INTLEVEL_EXCM
     1:
-    movi    a0, PS_WOE       // clear EXCM, enable window overflow, set new INTLEVEL
+    movi    a0, (PS_WOE | PS_UM)   // clear EXCM, enable window overflow, set user mode and new INTLEVEL
     or      a3, a3, a0
     wsr     a3, ps
     rsr     a0, EPC1
@@ -518,7 +520,7 @@ __default_naked_exception:
 
     bnei    a6, EXCCAUSE_LEVEL1_INTERRUPT, .HandleException   // Handle exception elsewhere
 
-    movi    a0, (1 | PS_WOE)          // set PS.INTLEVEL accordingly
+    movi    a0, (1 | PS_WOE | PS_UM)  // set PS.INTLEVEL, and run the handler in user mode
     wsr     a0, PS
     rsync
     mov     a6, sp                    // put address of save frame in a6=a2 in callee
@@ -533,7 +535,7 @@ __default_naked_exception:
 .HandleException:
     mov     a7, sp                    // put address of save frame in a7=a3 in callee
 
-    movi    a0, (PS_INTLEVEL_EXCM | PS_WOE)
+    movi    a0, (PS_INTLEVEL_EXCM | PS_WOE | PS_UM)
     wsr     a0, PS
     rsync