Merge pull request #576 from sw/cs-msr

Optimize critical_section: use MSR instruction

Author: adamgreig

cortex-m/Cargo.toml

@@ -38,7 +38,7 @@ cm7-r0p1 = ["cm7"]
 inline-asm = []
 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 = [

cortex-m/src/critical_section.rs

@@ -10,16 +10,18 @@ mod single_core_critical_section {
 
     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 {
-                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);
         }
     }
 }

cortex-m/src/interrupt.rs

@@ -56,17 +56,18 @@ pub fn free<F, R>(f: F) -> R
 where
     F: FnOnce(&CriticalSection) -> 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(unsafe { &CriticalSection::new() });
 
-    // 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

cortex-m/src/lib.rs

@@ -35,6 +35,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

cortex-m/src/register/primask.rs

@@ -1,5 +1,10 @@
 //! Priority mask register
 
+#[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)]
 pub enum Primask {
@@ -23,13 +28,42 @@ impl Primask {
     }
 }
 
-/// Reads the CPU register
+/// Reads the prioritizable interrupt mask
+#[cfg(cortex_m)]
 #[inline]
 pub fn read() -> Primask {
-    let r: u32 = call_asm!(__primask_r() -> u32);
-    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);
+}

testsuite/src/main.rs

@@ -77,8 +77,8 @@ mod tests {
     #[test]
     fn interrupt_free_nesting() {
         EXCEPTION_FLAG.store(false, Ordering::SeqCst);
-        cortex_m::interrupt::free(|_| {
-            cortex_m::interrupt::free(|_| {
+        cortex_m::interrupt::free(|| {
+            cortex_m::interrupt::free(|| {
                 cortex_m::peripheral::SCB::set_pendsv();
                 assert!(!EXCEPTION_FLAG.load(Ordering::SeqCst));
             });