diff --git a/cortex-m/src/critical_section.rs b/cortex-m/src/critical_section.rs index 7a30d16e..6bedfffa 100644 --- a/cortex-m/src/critical_section.rs +++ b/cortex-m/src/critical_section.rs @@ -1,4 +1,3 @@ -use core::sync::atomic::{compiler_fence, Ordering}; use critical_section::{set_impl, Impl, RawRestoreState}; use crate::interrupt; @@ -9,15 +8,17 @@ set_impl!(SingleCoreCriticalSection); unsafe impl Impl for SingleCoreCriticalSection { unsafe fn acquire() -> RawRestoreState { - let restore_state = 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 restore_state = primask::read_raw(); // NOTE: Fence guarantees are provided by interrupt::disable(), which performs a `compiler_fence(SeqCst)`. interrupt::disable(); - restore_state.0 + restore_state } unsafe fn release(restore_state: RawRestoreState) { - // Ensure no preceeding memory accesses are reordered to after interrupts are enabled. - compiler_fence(Ordering::SeqCst); - primask::write(restore_state); + // 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..c8914f60 100644 --- a/cortex-m/src/interrupt.rs +++ b/cortex-m/src/interrupt.rs @@ -66,18 +66,17 @@ 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() } - } + crate::register::primask::write_raw(primask); r } diff --git a/cortex-m/src/register/primask.rs b/cortex-m/src/register/primask.rs index 988d1ff8..89178de9 100644 --- a/cortex-m/src/register/primask.rs +++ b/cortex-m/src/register/primask.rs @@ -2,36 +2,60 @@ #[cfg(cortex_m)] use core::arch::asm; +use core::sync::atomic::{compiler_fence, Ordering}; -/// Priority mask register -pub struct Primask(pub u32); +/// All exceptions with configurable priority are ... +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub enum Primask { + /// Active + Active, + /// Inactive + Inactive, +} impl Primask { /// All exceptions with configurable priority are active #[inline] pub fn is_active(self) -> bool { - !self.is_inactive() + self == Primask::Active } /// All exceptions with configurable priority are inactive #[inline] pub fn is_inactive(self) -> bool { - self.0 & (1 << 0) == (1 << 0) + self == Primask::Inactive } } -/// Reads the CPU register +/// Reads the prioritizable interrupt mask #[cfg(cortex_m)] #[inline] pub fn read() -> Primask { + 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)) }; - Primask(r) + r } -/// Writes the CPU register +/// Writes the entire PRIMASK register +/// Note that bits [31:1] are reserved and SBZP (Should-Be-Zero-or-Preserved) #[cfg(cortex_m)] #[inline] -pub fn write(r: u32) { +pub 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 569e1540..d742c160 100644 --- a/testsuite/src/main.rs +++ b/testsuite/src/main.rs @@ -12,16 +12,16 @@ fn panic(info: &core::panic::PanicInfo) -> ! { minitest::fail() } -static CRITICAL_SECTION_FLAG: AtomicBool = AtomicBool::new(false); +static EXCEPTION_FLAG: AtomicBool = AtomicBool::new(false); #[cortex_m_rt::exception] fn PendSV() { - CRITICAL_SECTION_FLAG.store(true, Ordering::SeqCst); + EXCEPTION_FLAG.store(true, Ordering::SeqCst); } #[minitest::tests] mod tests { - use crate::{Ordering, CRITICAL_SECTION_FLAG}; + use crate::{Ordering, EXCEPTION_FLAG}; use minitest::log; #[init] @@ -63,13 +63,27 @@ mod tests { #[test] fn critical_section_nesting() { + EXCEPTION_FLAG.store(false, Ordering::SeqCst); critical_section::with(|_| { critical_section::with(|_| { cortex_m::peripheral::SCB::set_pendsv(); - assert!(!CRITICAL_SECTION_FLAG.load(Ordering::SeqCst)); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); }); - assert!(!CRITICAL_SECTION_FLAG.load(Ordering::SeqCst)); + assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst)); }); - assert!(CRITICAL_SECTION_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)); } }