Skip to content

Commit

Permalink
Optimize critical_section: use MSR instruction
Browse files Browse the repository at this point in the history
To avoid branches and unnecessary code in the acquire and release
functions, simply write back the original value to the PRIMASK register
using the MSR instruction.

This changes the critical-section dependency to use `u32` as the
`RawRestoreState` type instead of `bool`.

The `register::primask` module now defines a `struct(pub u32)` instead of an
`enum`, and additionally a `write` function for use with the critical
section.
  • Loading branch information
sw committed Jan 20, 2025
1 parent b02ec57 commit 02ec04a
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 25 deletions.
2 changes: 1 addition & 1 deletion cortex-m/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
16 changes: 7 additions & 9 deletions cortex-m/src/critical_section.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use core::sync::atomic::{compiler_fence, Ordering};
use critical_section::{set_impl, Impl, RawRestoreState};

use crate::interrupt;
Expand All @@ -8,18 +9,15 @@ set_impl!(SingleCoreCriticalSection);

unsafe impl Impl for SingleCoreCriticalSection {
unsafe fn acquire() -> RawRestoreState {
let was_active = primask::read().is_active();
let restore_state = primask::read();
// NOTE: Fence guarantees are provided by interrupt::disable(), which performs a `compiler_fence(SeqCst)`.
interrupt::disable();
was_active
restore_state.0
}

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) {
// Ensure no preceeding memory accesses are reordered to after interrupts are enabled.
compiler_fence(Ordering::SeqCst);
primask::write(restore_state);
}
}
27 changes: 12 additions & 15 deletions cortex-m/src/register/primask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,20 @@
#[cfg(cortex_m)]
use core::arch::asm;

/// All exceptions with configurable priority are ...
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Primask {
/// Active
Active,
/// Inactive
Inactive,
}
/// Priority mask register
pub struct Primask(pub u32);

impl Primask {
/// All exceptions with configurable priority are active
#[inline]
pub fn is_active(self) -> bool {
self == Primask::Active
!self.is_inactive()
}

/// All exceptions with configurable priority are inactive
#[inline]
pub fn is_inactive(self) -> bool {
self == Primask::Inactive
self.0 & (1 << 0) == (1 << 0)
}
}

Expand All @@ -32,9 +26,12 @@ impl Primask {
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) {
Primask::Inactive
} else {
Primask::Active
}
Primask(r)
}

/// Writes the CPU register
#[cfg(cortex_m)]
#[inline]
pub fn write(r: u32) {
unsafe { asm!("msr PRIMASK, {}", in(reg) r, options(nomem, nostack, preserves_flags)) };
}
21 changes: 21 additions & 0 deletions testsuite/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#![no_std]

extern crate cortex_m_rt;
use core::sync::atomic::{AtomicBool, Ordering};

#[cfg(target_env = "")] // appease clippy
#[panic_handler]
Expand All @@ -11,8 +12,16 @@ fn panic(info: &core::panic::PanicInfo) -> ! {
minitest::fail()
}

static CRITICAL_SECTION_FLAG: AtomicBool = AtomicBool::new(false);

#[cortex_m_rt::exception]
fn PendSV() {
CRITICAL_SECTION_FLAG.store(true, Ordering::SeqCst);
}

#[minitest::tests]
mod tests {
use crate::{Ordering, CRITICAL_SECTION_FLAG};
use minitest::log;

#[init]
Expand Down Expand Up @@ -51,4 +60,16 @@ mod tests {
assert!(!p.DWT.has_cycle_counter());
}
}

#[test]
fn critical_section_nesting() {
critical_section::with(|_| {
critical_section::with(|_| {
cortex_m::peripheral::SCB::set_pendsv();
assert!(!CRITICAL_SECTION_FLAG.load(Ordering::SeqCst));
});
assert!(!CRITICAL_SECTION_FLAG.load(Ordering::SeqCst));
});
assert!(CRITICAL_SECTION_FLAG.load(Ordering::SeqCst));
}
}

0 comments on commit 02ec04a

Please sign in to comment.