Fix core1 startup

esp-hal/CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fixing `esp-wifi` + `TRNG` issue on `ESP32-S2` (#1272)
+- Fixed core1 startup using the wrong stack on the esp32 and esp32s3 (#1286).
 
 ### Changed
 

esp-hal/src/soc/esp32/cpu_control.rs

@@ -53,8 +53,6 @@ use core::{
     mem::{ManuallyDrop, MaybeUninit},
 };
 
-use xtensa_lx::set_stack_pointer;
-
 use crate::Cpu;
 
 /// Data type for a properly aligned stack of N bytes
@@ -260,6 +258,11 @@ impl CpuControl {
         };
     }
 
+    /// When we get here, the core is out of reset, with a stack setup by ROM
+    /// code
+    ///
+    /// We need to initialize the CPU fully, and setup the new stack to use the
+    /// stack provided by the user.
     unsafe fn start_core1_init<F>() -> !
     where
         F: FnOnce(),
@@ -272,9 +275,6 @@ impl CpuControl {
         xtensa_lx::timer::set_ccompare1(0);
         xtensa_lx::timer::set_ccompare2(0);
 
-        // set stack pointer to end of memory: no need to retain stack up to this point
-        set_stack_pointer(unsafe { unwrap!(APP_CORE_STACK_TOP) });
-
         extern "C" {
             static mut _init_start: u32;
         }
@@ -285,6 +285,22 @@ impl CpuControl {
             core::arch::asm!("wsr.vecbase {0}", in(reg) base, options(nostack));
         }
 
+        // switch to new stack
+        xtensa_lx::set_stack_pointer(unwrap!(APP_CORE_STACK_TOP));
+
+        // Trampoline to run from the new stack.
+        // start_core1_run should _NEVER_ be inlined
+        // as we rely on the function call to use
+        // the new stack.
+        Self::start_core1_run::<F>()
+    }
+
+    /// Run the core1 closure.
+    #[inline(never)]
+    unsafe fn start_core1_run<F>() -> !
+    where
+        F: FnOnce(),
+    {
         match START_CORE1_FUNCTION.take() {
             Some(entry) => {
                 let entry = unsafe { ManuallyDrop::take(&mut *entry.cast::<ManuallyDrop<F>>()) };

esp-hal/src/soc/esp32s3/cpu_control.rs

@@ -53,8 +53,6 @@ use core::{
     mem::{ManuallyDrop, MaybeUninit},
 };
 
-use xtensa_lx::set_stack_pointer;
-
 use crate::Cpu;
 
 /// Data type for a properly aligned stack of N bytes
@@ -103,7 +101,6 @@ impl<const SIZE: usize> Stack<SIZE> {
 // Pointer to the closure that will be executed on the second core. The closure
 // is copied to the core's stack.
 static mut START_CORE1_FUNCTION: Option<*mut ()> = None;
-
 static mut APP_CORE_STACK_TOP: Option<*mut u32> = None;
 
 /// Will park the APP (second) core when dropped
@@ -196,6 +193,11 @@ impl CpuControl {
         }
     }
 
+    /// When we get here, the core is out of reset, with a stack setup by ROM
+    /// code
+    ///
+    /// We need to initialize the CPU fully, and setup the new stack to use the
+    /// stack provided by the user.
     unsafe fn start_core1_init<F>() -> !
     where
         F: FnOnce(),
@@ -208,9 +210,6 @@ impl CpuControl {
         xtensa_lx::timer::set_ccompare1(0);
         xtensa_lx::timer::set_ccompare2(0);
 
-        // set stack pointer to end of memory: no need to retain stack up to this point
-        set_stack_pointer(unsafe { unwrap!(APP_CORE_STACK_TOP) });
-
         extern "C" {
             static mut _init_start: u32;
         }
@@ -221,6 +220,22 @@ impl CpuControl {
             core::arch::asm!("wsr.vecbase {0}", in(reg) base, options(nostack));
         }
 
+        // switch to new stack
+        xtensa_lx::set_stack_pointer(unwrap!(APP_CORE_STACK_TOP));
+
+        // Trampoline to run from the new stack.
+        // start_core1_run should _NEVER_ be inlined
+        // as we rely on the function call to use
+        // the new stack.
+        Self::start_core1_run::<F>()
+    }
+
+    /// Run the core1 closure.
+    #[inline(never)]
+    unsafe fn start_core1_run<F>() -> !
+    where
+        F: FnOnce(),
+    {
         match START_CORE1_FUNCTION.take() {
             Some(entry) => {
                 let entry = unsafe { ManuallyDrop::take(&mut *entry.cast::<ManuallyDrop<F>>()) };

examples/src/bin/embassy_multicore.rs

@@ -64,14 +64,14 @@ async fn main(_spawner: Spawner) {
     let led_ctrl_signal = &*make_static!(Signal::new());
 
     let led = io.pins.gpio0.into_push_pull_output();
-    let cpu1_fnctn = move || {
-        let executor = make_static!(Executor::new());
-        executor.run(|spawner| {
-            spawner.spawn(control_led(led, led_ctrl_signal)).ok();
-        });
-    };
+
     let _guard = cpu_control
-        .start_app_core(unsafe { &mut APP_CORE_STACK }, cpu1_fnctn)
+        .start_app_core(unsafe { &mut APP_CORE_STACK }, move || {
+            let executor = make_static!(Executor::new());
+            executor.run(|spawner| {
+                spawner.spawn(control_led(led, led_ctrl_signal)).ok();
+            });
+        })
         .unwrap();
 
     // Sends periodic messages to control_led, enabling or disabling it.

examples/src/bin/multicore.rs

@@ -15,9 +15,9 @@ use esp_backtrace as _;
 use esp_hal::{
     clock::ClockControl,
     cpu_control::{CpuControl, Stack},
-    peripherals::{Peripherals, TIMG1},
+    peripherals::Peripherals,
     prelude::*,
-    timer::{Timer, Timer0, TimerGroup},
+    timer::TimerGroup,
 };
 use esp_println::println;
 use nb::block;
@@ -39,14 +39,20 @@ fn main() -> ! {
     timer0.start(1u64.secs());
     timer1.start(500u64.millis());
 
-    let counter = Mutex::new(RefCell::new(0));
+    let counter = Mutex::new(RefCell::new(0u32));
 
     let mut cpu_control = CpuControl::new(system.cpu_control);
-    let cpu1_fnctn = || {
-        cpu1_task(&mut timer1, &counter);
-    };
     let _guard = cpu_control
-        .start_app_core(unsafe { &mut APP_CORE_STACK }, cpu1_fnctn)
+        .start_app_core(unsafe { &mut APP_CORE_STACK }, || {
+            println!("Hello World - Core 1!");
+            loop {
+                block!(timer1.wait()).unwrap();
+                critical_section::with(|cs| {
+                    let mut val = counter.borrow_ref_mut(cs);
+                    *val = val.wrapping_add(1);
+                });
+            }
+        })
         .unwrap();
 
     loop {
@@ -56,15 +62,3 @@ fn main() -> ! {
         println!("Hello World - Core 0! Counter is {}", count);
     }
 }
-
-fn cpu1_task(timer: &mut Timer<Timer0<TIMG1>>, counter: &Mutex<RefCell<i32>>) -> ! {
-    println!("Hello World - Core 1!");
-    loop {
-        block!(timer.wait()).unwrap();
-
-        critical_section::with(|cs| {
-            let new_val = counter.borrow_ref_mut(cs).wrapping_add(1);
-            *counter.borrow_ref_mut(cs) = new_val;
-        });
-    }
-}