Skip to content

Commit

Permalink
Fix core1 startup
Browse files Browse the repository at this point in the history
The original code tried to set the stack pointer and call the closure
_in the same stack frame_. This isn't possible, so the actual stack
being used was the 8K stack setup by ROM code. If this stack overflowed,
it could corrupt the ROM function .bss and .data which is why I was
seeing intermittent panic messages. I still don't understand all the
failure cases, but I know that now we are using the correct stack
properly.
  • Loading branch information
MabezDev committed Mar 13, 2024
1 parent 465a876 commit 19c3401
Showing 1 changed file with 25 additions and 9 deletions.
34 changes: 25 additions & 9 deletions esp-hal/src/soc/esp32s3/cpu_control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ use core::{
mem::{ManuallyDrop, MaybeUninit},
};

use xtensa_lx::set_stack_pointer;

use crate::Cpu;
use crate::{prelude::*, Cpu};

/// Data type for a properly aligned stack of N bytes
// Xtensa ISA 10.5: [B]y default, the
Expand Down Expand Up @@ -103,8 +101,7 @@ 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;
static mut APP_CORE_STACK_TOP: *mut u32 = core::ptr::null_mut();

/// Will park the APP (second) core when dropped
#[must_use]
Expand Down Expand Up @@ -196,6 +193,12 @@ 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.
#[ram]
unsafe fn start_core1_init<F>() -> !
where
F: FnOnce(),
Expand All @@ -208,9 +211,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;
}
Expand All @@ -221,6 +221,22 @@ impl CpuControl {
core::arch::asm!("wsr.vecbase {0}", in(reg) base, options(nostack));
}

// switch to new stack
xtensa_lx::set_stack_pointer(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.
#[ram]
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>>()) };
Expand Down Expand Up @@ -275,7 +291,7 @@ impl CpuControl {

let entry_fn = entry_dst.cast::<()>();
START_CORE1_FUNCTION = Some(entry_fn);
APP_CORE_STACK_TOP = Some(stack.top());
APP_CORE_STACK_TOP = stack.top();
}

// TODO there is no boot_addr register in SVD or TRM - ESP-IDF uses a ROM
Expand Down

0 comments on commit 19c3401

Please sign in to comment.