From d75714ae6f1d4e6733d1b65296242b78f56cfcc4 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Sun, 21 Aug 2022 23:51:39 +0200 Subject: [PATCH 1/3] Update to edition 2021. This shouldn't be a breaking change since Edition 2021 came out in Rust 1.56, and MSRV is already higher than that (Rust 1.59) --- Cargo.toml | 1 + src/interrupt.rs | 2 +- src/lib.rs | 4 ---- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 25a14779..97888342 100644 --- a/Cargo.toml +++ b/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 "] diff --git a/src/interrupt.rs b/src/interrupt.rs index d43fd987..14fc5d96 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -1,8 +1,8 @@ //! Interrupts // NOTE: Adapted from cortex-m/src/interrupt.rs +use crate::register::mstatus; pub use bare_metal::{CriticalSection, Mutex}; -use register::mstatus; /// Disables all interrupts #[inline] diff --git a/src/lib.rs b/src/lib.rs index 5590cd03..f3e58f3c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,10 +15,6 @@ #![no_std] -extern crate bare_metal; -extern crate bit_field; -extern crate embedded_hal; - pub mod asm; pub mod delay; pub mod interrupt; From 8429f8c4ed4446abe8a00e860d59364df2b1bfd2 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 22 Aug 2022 00:01:55 +0200 Subject: [PATCH 2/3] Add implementation for critical-section 1.0 for single-core chips. --- .github/workflows/ci.yaml | 10 +++++++++- CHANGELOG.md | 4 ++++ Cargo.toml | 4 ++++ src/critical_section.rs | 22 ++++++++++++++++++++++ src/lib.rs | 3 +++ 5 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 src/critical_section.rs diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 70ebc99c..bf626f77 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index c2082d1c..08519742 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/Cargo.toml b/Cargo.toml index 97888342..8144d2df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,11 @@ 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" diff --git a/src/critical_section.rs b/src/critical_section.rs new file mode 100644 index 00000000..63928849 --- /dev/null +++ b/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() + } + } +} diff --git a/src/lib.rs b/src/lib.rs index f3e58f3c..24f3d188 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,3 +22,6 @@ pub mod register; #[macro_use] mod macros; + +#[cfg(all(riscv, feature = "critical-section-single-core"))] +mod critical_section; From e38d775761b5b2ffb296359bc4a5c9e9f04e0796 Mon Sep 17 00:00:00 2001 From: Dario Nieuwenhuis Date: Mon, 22 Aug 2022 00:23:42 +0200 Subject: [PATCH 3/3] Fix `interrupt::free()` unsoundness on multicore systems. This is unsound on multicore systems because it only disables interrupts in the current core. For multicore chips, a chip-specific critical section implementation is needed instead. Unsoundness is fixed by not returning the `CriticalSection` token. This is a breaking change. --- Cargo.toml | 1 - src/interrupt.rs | 20 ++++++++++++-------- src/lib.rs | 7 +++++++ src/macros.rs | 7 +++++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 8144d2df..1135f56a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,6 @@ targets = [ 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" diff --git a/src/interrupt.rs b/src/interrupt.rs index 14fc5d96..cfe05fdd 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -2,9 +2,8 @@ // NOTE: Adapted from cortex-m/src/interrupt.rs use crate::register::mstatus; -pub use bare_metal::{CriticalSection, Mutex}; -/// 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: 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 diff --git a/src/lib.rs b/src/lib.rs index 24f3d188..a6d9e066 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,3 +25,10 @@ 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; +} diff --git a/src/macros.rs b/src/macros.rs index 9600b3cc..6cad0941 100644 --- a/src/macros.rs +++ b/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)]