diff --git a/cortex-m/Cargo.toml b/cortex-m/Cargo.toml index d90dc376..0c317c64 100644 --- a/cortex-m/Cargo.toml +++ b/cortex-m/Cargo.toml @@ -33,7 +33,7 @@ cm7 = [] cm7-r0p1 = ["cm7"] linker-plugin-lto = [] std = [] -critical-section-single-core = ["critical-section/restore-state-bool"] +critical-section-single-core = ["critical-section/restore-state-u32"] [package.metadata.docs.rs] targets = [ diff --git a/cortex-m/src/critical_section.rs b/cortex-m/src/critical_section.rs index ecf0681f..6bedfffa 100644 --- a/cortex-m/src/critical_section.rs +++ b/cortex-m/src/critical_section.rs @@ -8,18 +8,17 @@ set_impl!(SingleCoreCriticalSection); unsafe impl Impl for SingleCoreCriticalSection { unsafe fn acquire() -> RawRestoreState { - let was_active = primask::read().is_active(); + // Backup previous state of PRIMASK register. We access the entire register directly as a + // u32 instead of using the primask::read() function to minimize the number of processor + // cycles during which interrupts are disabled. + let restore_state = primask::read_raw(); // NOTE: Fence guarantees are provided by interrupt::disable(), which performs a `compiler_fence(SeqCst)`. interrupt::disable(); - was_active + restore_state } - unsafe fn release(was_active: RawRestoreState) { - // Only re-enable interrupts if they were enabled before the critical section. - if was_active { - // NOTE: Fence guarantees are provided by interrupt::enable(), which performs a - // `compiler_fence(SeqCst)`. - interrupt::enable() - } + unsafe fn release(restore_state: RawRestoreState) { + // NOTE: Fence guarantees are provided by primask::write_raw(), which performs a `compiler_fence(SeqCst)`. + primask::write_raw(restore_state); } } diff --git a/cortex-m/src/interrupt.rs b/cortex-m/src/interrupt.rs index 6b8f9e96..aa792201 100644 --- a/cortex-m/src/interrupt.rs +++ b/cortex-m/src/interrupt.rs @@ -66,17 +66,18 @@ pub fn free(f: F) -> R where F: FnOnce() -> R, { - let primask = crate::register::primask::read(); + // Backup previous state of PRIMASK register. We access the entire register directly as a + // u32 instead of using the primask::read() function to minimize the number of processor + // cycles during which interrupts are disabled. + let primask = crate::register::primask::read_raw(); // disable interrupts disable(); let r = f(); - // If the interrupts were active before our `disable` call, then re-enable - // them. Otherwise, keep them disabled - if primask.is_active() { - unsafe { enable() } + unsafe { + crate::register::primask::write_raw(primask); } r diff --git a/cortex-m/src/lib.rs b/cortex-m/src/lib.rs index b4ae0157..b0ca35c9 100644 --- a/cortex-m/src/lib.rs +++ b/cortex-m/src/lib.rs @@ -19,6 +19,11 @@ //! or critical sections are managed as part of an RTOS. In these cases, you should use //! a target-specific implementation instead, typically provided by a HAL or RTOS crate. //! +//! The critical section has been optimized to block interrupts for as few cycles as possible, +//! but -- due to `critical-section` implementation details -- incurs branches in a normal build +//! configuration. For minimal interrupt latency, you can achieve inlining by enabling +//! [linker-plugin-based LTO](https://doc.rust-lang.org/rustc/linker-plugin-lto.html). +//! //! ## `cm7-r0p1` //! //! This feature enables workarounds for errata found on Cortex-M7 chips with revision r0p1. Some diff --git a/cortex-m/src/register/primask.rs b/cortex-m/src/register/primask.rs index e95276fa..58b8c287 100644 --- a/cortex-m/src/register/primask.rs +++ b/cortex-m/src/register/primask.rs @@ -2,6 +2,8 @@ #[cfg(cortex_m)] use core::arch::asm; +#[cfg(cortex_m)] +use core::sync::atomic::{compiler_fence, Ordering}; /// All exceptions with configurable priority are ... #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -26,15 +28,42 @@ impl Primask { } } -/// Reads the CPU register +/// Reads the prioritizable interrupt mask #[cfg(cortex_m)] #[inline] pub fn read() -> Primask { - let r: u32; - unsafe { asm!("mrs {}, PRIMASK", out(reg) r, options(nomem, nostack, preserves_flags)) }; - if r & (1 << 0) == (1 << 0) { + if read_raw() & (1 << 0) == (1 << 0) { Primask::Inactive } else { Primask::Active } } + +/// Reads the entire PRIMASK register +/// Note that bits [31:1] are reserved and UNK (Unknown) +#[cfg(cortex_m)] +#[inline] +pub fn read_raw() -> u32 { + let r: u32; + unsafe { asm!("mrs {}, PRIMASK", out(reg) r, options(nomem, nostack, preserves_flags)) }; + r +} + +/// Writes the entire PRIMASK register +/// Note that bits [31:1] are reserved and SBZP (Should-Be-Zero-or-Preserved) +/// +/// # Safety +/// +/// This method is unsafe as other unsafe code may rely on interrupts remaining disabled, for +/// example during a critical section, and being able to safely re-enable them would lead to +/// undefined behaviour. Do not call this function in a context where interrupts are expected to +/// remain disabled -- for example, in the midst of a critical section or `interrupt::free()` call. +#[cfg(cortex_m)] +#[inline] +pub unsafe fn write_raw(r: u32) { + // Ensure no preceeding memory accesses are reordered to after interrupts are possibly enabled. + compiler_fence(Ordering::SeqCst); + unsafe { asm!("msr PRIMASK, {}", in(reg) r, options(nomem, nostack, preserves_flags)) }; + // Ensure no subsequent memory accesses are reordered to before interrupts are possibly disabled. + compiler_fence(Ordering::SeqCst); +} diff --git a/testsuite/src/main.rs b/testsuite/src/main.rs index f6718b36..d742c160 100644 --- a/testsuite/src/main.rs +++ b/testsuite/src/main.rs @@ -2,6 +2,7 @@ #![no_std] extern crate cortex_m_rt; +use core::sync::atomic::{AtomicBool, Ordering}; #[cfg(target_env = "")] // appease clippy #[panic_handler] @@ -11,8 +12,16 @@ fn panic(info: &core::panic::PanicInfo) -> ! { minitest::fail() } +static EXCEPTION_FLAG: AtomicBool = AtomicBool::new(false); + +#[cortex_m_rt::exception] +fn PendSV() { + EXCEPTION_FLAG.store(true, Ordering::SeqCst); +} + #[minitest::tests] mod tests { + use crate::{Ordering, EXCEPTION_FLAG}; use minitest::log; #[init] @@ -51,4 +60,30 @@ mod tests { assert!(!p.DWT.has_cycle_counter()); } } + + #[test] + fn critical_section_nesting() { + EXCEPTION_FLAG.store(false, Ordering::SeqCst); + critical_section::with(|_| { + critical_section::with(|_| { + cortex_m::peripheral::SCB::set_pendsv(); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); + }); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); + }); + assert!(EXCEPTION_FLAG.load(Ordering::SeqCst)); + } + + #[test] + fn interrupt_free_nesting() { + EXCEPTION_FLAG.store(false, Ordering::SeqCst); + cortex_m::interrupt::free(|| { + cortex_m::interrupt::free(|| { + cortex_m::peripheral::SCB::set_pendsv(); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); + }); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); + }); + assert!(EXCEPTION_FLAG.load(Ordering::SeqCst)); + } }