From 4d6b3c57b1a0b99409cae743a102408f23ca828b Mon Sep 17 00:00:00 2001 From: pennae Date: Thu, 20 Jul 2023 16:08:59 +0200 Subject: [PATCH 1/2] rp: fix multicore stack guard setup the region field of the register is four bits wide followed by the valid bit that causes the rnr update we rely on for the rasr write. 0x08 is just a bit short to reach the valid bit, and since rp2040 has only 8 regions it (at best) doesn't do anything at all. --- embassy-rp/src/multicore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/embassy-rp/src/multicore.rs b/embassy-rp/src/multicore.rs index 468e8470a..89a2680ad 100644 --- a/embassy-rp/src/multicore.rs +++ b/embassy-rp/src/multicore.rs @@ -74,7 +74,7 @@ fn install_stack_guard(stack_bottom: *mut usize) { let subregion_select = 0xff ^ (1 << ((addr >> 5) & 7)); unsafe { core.MPU.ctrl.write(5); // enable mpu with background default map - core.MPU.rbar.write((addr & !0xff) | 0x8); + core.MPU.rbar.write((addr & !0xff) | (1 << 4)); // set address and update RNR core.MPU.rasr.write( 1 // enable region | (0x7 << 1) // size 2^(7 + 1) = 256 From e9445ec72d8713aa7705e0a727c13c259d529e71 Mon Sep 17 00:00:00 2001 From: pennae Date: Thu, 20 Jul 2023 16:57:54 +0200 Subject: [PATCH 2/2] rp: allow for MPU-based stack guards on core 0 as well using these will require some linker script intervention. setting the core0 stack needs linker intervention anyway (to provide _stack_start), having it also provide _stack_end for the guard to use is not that much of a stretch. --- embassy-rp/src/lib.rs | 68 +++++++++++++++++++++++++++++++++++++ embassy-rp/src/multicore.rs | 33 ++++-------------- 2 files changed, 74 insertions(+), 27 deletions(-) diff --git a/embassy-rp/src/lib.rs b/embassy-rp/src/lib.rs index 4f205a16e..50f028d4c 100644 --- a/embassy-rp/src/lib.rs +++ b/embassy-rp/src/lib.rs @@ -219,6 +219,74 @@ select_bootloader! { default => BOOT_LOADER_W25Q080 } +/// Installs a stack guard for the CORE0 stack in MPU region 0. +/// Will fail if the MPU is already confgigured. This function requires +/// a `_stack_end` symbol to be defined by the linker script, and expexcts +/// `_stack_end` to be located at the lowest address (largest depth) of +/// the stack. +/// +/// This method can *only* set up stack guards on the currently +/// executing core. Stack guards for CORE1 are set up automatically, +/// only CORE0 should ever use this. +/// +/// # Usage +/// +/// ```no_run +/// #![feature(type_alias_impl_trait)] +/// use embassy_rp::install_core0_stack_guard; +/// use embassy_executor::{Executor, Spawner}; +/// +/// #[embassy_executor::main] +/// async fn main(_spawner: Spawner) { +/// // set up by the linker as follows: +/// // +/// // MEMORY { +/// // STACK0: ORIGIN = 0x20040000, LENGTH = 4K +/// // } +/// // +/// // _stack_end = ORIGIN(STACK0); +/// // _stack_start = _stack_end + LENGTH(STACK0); +/// // +/// install_core0_stack_guard().expect("MPU already configured"); +/// let p = embassy_rp::init(Default::default()); +/// +/// // ... +/// } +/// ``` +pub fn install_core0_stack_guard() -> Result<(), ()> { + extern "C" { + static mut _stack_end: usize; + } + unsafe { install_stack_guard(&mut _stack_end as *mut usize) } +} + +#[inline(always)] +fn install_stack_guard(stack_bottom: *mut usize) -> Result<(), ()> { + let core = unsafe { cortex_m::Peripherals::steal() }; + + // Fail if MPU is already configured + if core.MPU.ctrl.read() != 0 { + return Err(()); + } + + // The minimum we can protect is 32 bytes on a 32 byte boundary, so round up which will + // just shorten the valid stack range a tad. + let addr = (stack_bottom as u32 + 31) & !31; + // Mask is 1 bit per 32 bytes of the 256 byte range... clear the bit for the segment we want + let subregion_select = 0xff ^ (1 << ((addr >> 5) & 7)); + unsafe { + core.MPU.ctrl.write(5); // enable mpu with background default map + core.MPU.rbar.write((addr & !0xff) | (1 << 4)); // set address and update RNR + core.MPU.rasr.write( + 1 // enable region + | (0x7 << 1) // size 2^(7 + 1) = 256 + | (subregion_select << 8) + | 0x10000000, // XN = disable instruction fetch; no other bits means no permissions + ); + } + Ok(()) +} + pub mod config { use crate::clocks::ClockConfig; diff --git a/embassy-rp/src/multicore.rs b/embassy-rp/src/multicore.rs index 89a2680ad..915761801 100644 --- a/embassy-rp/src/multicore.rs +++ b/embassy-rp/src/multicore.rs @@ -52,41 +52,20 @@ use core::sync::atomic::{compiler_fence, AtomicBool, Ordering}; use crate::interrupt::InterruptExt; use crate::peripherals::CORE1; -use crate::{gpio, interrupt, pac}; +use crate::{gpio, install_stack_guard, interrupt, pac}; const PAUSE_TOKEN: u32 = 0xDEADBEEF; const RESUME_TOKEN: u32 = !0xDEADBEEF; static IS_CORE1_INIT: AtomicBool = AtomicBool::new(false); #[inline(always)] -fn install_stack_guard(stack_bottom: *mut usize) { - let core = unsafe { cortex_m::Peripherals::steal() }; - - // Trap if MPU is already configured - if core.MPU.ctrl.read() != 0 { +fn core1_setup(stack_bottom: *mut usize) { + if let Err(_) = install_stack_guard(stack_bottom) { + // currently only happens if the MPU was already set up, which + // would indicate that the core is already in use from outside + // embassy, somehow. trap if so since we can't deal with that. cortex_m::asm::udf(); } - - // The minimum we can protect is 32 bytes on a 32 byte boundary, so round up which will - // just shorten the valid stack range a tad. - let addr = (stack_bottom as u32 + 31) & !31; - // Mask is 1 bit per 32 bytes of the 256 byte range... clear the bit for the segment we want - let subregion_select = 0xff ^ (1 << ((addr >> 5) & 7)); - unsafe { - core.MPU.ctrl.write(5); // enable mpu with background default map - core.MPU.rbar.write((addr & !0xff) | (1 << 4)); // set address and update RNR - core.MPU.rasr.write( - 1 // enable region - | (0x7 << 1) // size 2^(7 + 1) = 256 - | (subregion_select << 8) - | 0x10000000, // XN = disable instruction fetch; no other bits means no permissions - ); - } -} - -#[inline(always)] -fn core1_setup(stack_bottom: *mut usize) { - install_stack_guard(stack_bottom); unsafe { gpio::init(); }