Fix interrupt::free() unsoundness on multicore systems.

.github/workflows/ci.yaml

@@ -38,6 +38,14 @@ jobs:
         run: cargo check --target riscv64imac-unknown-none-elf
       - name: Run CI script for riscv64gc-unknown-none-elf under ${{ matrix.rust }}
         run: cargo check --target riscv64gc-unknown-none-elf
+      - name: Run CI script for x86_64-unknown-linux-gnu under ${{ matrix.rust }} with critical-section-single-core
+        run: cargo check --target x86_64-unknown-linux-gnu --features critical-section-single-core
+      - name: Run CI script for riscv32imac-unknown-none-elf under ${{ matrix.rust }} with critical-section-single-core
+        run: cargo check --target riscv32imac-unknown-none-elf --features critical-section-single-core
+      - name: Run CI script for riscv64imac-unknown-none-elf under ${{ matrix.rust }} with critical-section-single-core
+        run: cargo check --target riscv64imac-unknown-none-elf --features critical-section-single-core
+      - name: Run CI script for riscv64gc-unknown-none-elf under ${{ matrix.rust }} with critical-section-single-core
+        run: cargo check --target riscv64gc-unknown-none-elf --features critical-section-single-core
 
   # On macOS and Windows, we at least make sure that the crate builds and links.
   build-other:
@@ -56,4 +64,4 @@ jobs:
           toolchain: stable
           override: true
       - name: Build crate for host OS
-        run: cargo build
+        run: cargo build --features critical-section-single-core

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Added
+
+- Added `critical-section-single-core` feature which provides an implementation for the `critical_section` crate for single-core systems, based on disabling all interrupts.
+
 ### Fixed
 
 - Fix `asm::delay()` to ensure count register is always reloaded

Cargo.toml

@@ -1,6 +1,7 @@
 [package]
 name = "riscv"
 version = "0.8.0"
+edition = "2021"
 rust-version = "1.59"
 repository = "https://github.com/rust-embedded/riscv"
 authors = ["The RISC-V Team <[email protected]>"]
@@ -16,7 +17,10 @@ targets = [
     "riscv64imac-unknown-none-elf", "riscv64gc-unknown-none-elf",
 ]
 
+[features]
+critical-section-single-core = ["critical-section/restore-state-bool"]
+
 [dependencies]
-bare-metal = "1.0.0"
 bit_field = "0.10.0"
+critical-section = "1.1.0"
 embedded-hal = "0.2.6"

src/critical_section.rs

@@ -0,0 +1,22 @@
+use critical_section::{set_impl, Impl, RawRestoreState};
+
+use crate::interrupt;
+use crate::register::mstatus;
+
+struct SingleCoreCriticalSection;
+set_impl!(SingleCoreCriticalSection);
+
+unsafe impl Impl for SingleCoreCriticalSection {
+    unsafe fn acquire() -> RawRestoreState {
+        let was_active = mstatus::read().mie();
+        interrupt::disable();
+        was_active
+    }
+
+    unsafe fn release(was_active: RawRestoreState) {
+        // Only re-enable interrupts if they were enabled before the critical section.
+        if was_active {
+            interrupt::enable()
+        }
+    }
+}

src/interrupt.rs

@@ -1,10 +1,9 @@
 //! Interrupts
 
 // NOTE: Adapted from cortex-m/src/interrupt.rs
-pub use bare_metal::{CriticalSection, Mutex};
-use register::mstatus;
+use crate::register::mstatus;
 
-/// Disables all interrupts
+/// Disables all interrupts in the current core.
 #[inline]
 pub unsafe fn disable() {
     match () {
@@ -15,11 +14,11 @@ pub unsafe fn disable() {
     }
 }
 
-/// Enables all the interrupts
+/// Enables all the interrupts in the current core.
 ///
 /// # Safety
 ///
-/// - Do not call this function inside an `interrupt::free` critical section
+/// - Do not call this function inside a critical section.
 #[inline]
 pub unsafe fn enable() {
     match () {
@@ -30,13 +29,18 @@ pub unsafe fn enable() {
     }
 }
 
-/// Execute closure `f` in an interrupt-free context.
+/// Execute closure `f` with interrupts disabled in the current core.
 ///
-/// This as also known as a "critical section".
+/// This method does not synchronise multiple cores, so it is not suitable for
+/// using as a critical section. See the `critical-section` crate for a cross-platform
+/// way to enter a critical section which provides a `CriticalSection` token.
+///
+/// This crate provides an implementation for `critical-section` suitable for single-core systems,
+/// based on disabling all interrupts. It can be enabled with the `critical-section-single-core` feature.
 #[inline]
 pub fn free<F, R>(f: F) -> R
 where
-    F: FnOnce(&CriticalSection) -> R,
+    F: FnOnce() -> R,
 {
     let mstatus = mstatus::read();
 
@@ -45,7 +49,7 @@ where
         disable();
     }
 
-    let r = f(unsafe { &CriticalSection::new() });
+    let r = f();
 
     // If the interrupts were active before our `disable` call, then re-enable
     // them. Otherwise, keep them disabled

src/lib.rs

@@ -15,14 +15,20 @@
 
 #![no_std]
 
-extern crate bare_metal;
-extern crate bit_field;
-extern crate embedded_hal;
-
 pub mod asm;
 pub mod delay;
 pub mod interrupt;
 pub mod register;
 
 #[macro_use]
 mod macros;
+
+#[cfg(all(riscv, feature = "critical-section-single-core"))]
+mod critical_section;
+
+/// Used to reexport items for use in macros. Do not use directly.
+/// Not covered by semver guarantees.
+#[doc(hidden)]
+pub mod _export {
+    pub use critical_section;
+}

src/macros.rs

@@ -6,7 +6,10 @@
 /// at most once in the whole lifetime of the program.
 ///
 /// # Note
-/// this macro is unsound on multi-core systems
+///
+/// this macro requires a `critical-section` implementation to be set. For single core systems, you can
+/// enable the `critical-section-single-core` feature for this crate. For multi core systems, you
+/// have to provide one from elsewhere, typically your chip's HAL crate.
 ///
 /// # Example
 ///
@@ -29,7 +32,7 @@
 #[macro_export]
 macro_rules! singleton {
     (: $ty:ty = $expr:expr) => {
-        $crate::interrupt::free(|_| {
+        $crate::_export::critical_section::with(|_| {
             static mut VAR: Option<$ty> = None;
 
             #[allow(unsafe_code)]