From 6d18397d006ac321dee093f44568f245b479da9c Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Thu, 21 Apr 2022 09:39:23 +0200 Subject: [PATCH 1/9] Fix `interrupt::free`. --- src/interrupt.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/interrupt.rs b/src/interrupt.rs index 72450c4b..709e86f5 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -60,14 +60,14 @@ pub unsafe fn enable() { #[inline] pub fn free(f: F) -> R where - F: FnOnce(&CriticalSection) -> R, + F: FnOnce(CriticalSection<'_>) -> R, { let primask = crate::register::primask::read(); // disable interrupts disable(); - let r = f(unsafe { &CriticalSection::new() }); + let r = f(unsafe { CriticalSection::new() }); // If the interrupts were active before our `disable` call, then re-enable // them. Otherwise, keep them disabled @@ -85,7 +85,7 @@ where #[inline] pub fn free(_: F) -> R where - F: FnOnce(&CriticalSection) -> R, + F: FnOnce(CriticalSection<'_>) -> R, { panic!("cortex_m::interrupt::free() is only functional on cortex-m platforms"); } From d77f65e4f47514d2052ebd971c9878fab3c879ea Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 4 May 2022 02:57:34 +0200 Subject: [PATCH 2/9] Don't pass `CriticalSection` in `interrupt::free`. --- cortex-m-semihosting/src/export.rs | 8 ++++---- src/interrupt.rs | 11 ++++------- src/macros.rs | 2 +- src/peripheral/mod.rs | 2 +- src/peripheral/sau.rs | 4 ++-- 5 files changed, 12 insertions(+), 15 deletions(-) diff --git a/cortex-m-semihosting/src/export.rs b/cortex-m-semihosting/src/export.rs index 0bbd09f5..dc76d62c 100644 --- a/cortex-m-semihosting/src/export.rs +++ b/cortex-m-semihosting/src/export.rs @@ -9,7 +9,7 @@ use crate::hio::{self, HostStream}; static mut HSTDOUT: Option = None; pub fn hstdout_str(s: &str) { - let _result = interrupt::free(|_| unsafe { + let _result = interrupt::free(|| unsafe { if HSTDOUT.is_none() { HSTDOUT = Some(hio::hstdout()?); } @@ -19,7 +19,7 @@ pub fn hstdout_str(s: &str) { } pub fn hstdout_fmt(args: fmt::Arguments) { - let _result = interrupt::free(|_| unsafe { + let _result = interrupt::free(|| unsafe { if HSTDOUT.is_none() { HSTDOUT = Some(hio::hstdout()?); } @@ -31,7 +31,7 @@ pub fn hstdout_fmt(args: fmt::Arguments) { static mut HSTDERR: Option = None; pub fn hstderr_str(s: &str) { - let _result = interrupt::free(|_| unsafe { + let _result = interrupt::free(|| unsafe { if HSTDERR.is_none() { HSTDERR = Some(hio::hstderr()?); } @@ -41,7 +41,7 @@ pub fn hstderr_str(s: &str) { } pub fn hstderr_fmt(args: fmt::Arguments) { - let _result = interrupt::free(|_| unsafe { + let _result = interrupt::free(|| unsafe { if HSTDERR.is_none() { HSTDERR = Some(hio::hstderr()?); } diff --git a/src/interrupt.rs b/src/interrupt.rs index 709e86f5..c0ddb703 100644 --- a/src/interrupt.rs +++ b/src/interrupt.rs @@ -1,6 +1,5 @@ //! Interrupts -pub use bare_metal::{CriticalSection, Mutex}; #[cfg(cortex_m)] use core::arch::asm; #[cfg(cortex_m)] @@ -54,20 +53,18 @@ pub unsafe fn enable() { } /// Execute closure `f` in an interrupt-free context. -/// -/// This as also known as a "critical section". #[cfg(cortex_m)] #[inline] pub fn free(f: F) -> R where - F: FnOnce(CriticalSection<'_>) -> R, + F: FnOnce() -> R, { let primask = crate::register::primask::read(); // disable interrupts 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 @@ -83,9 +80,9 @@ where #[doc(hidden)] #[cfg(not(cortex_m))] #[inline] -pub fn free(_: F) -> R +pub fn free(_f: F) -> R where - F: FnOnce(CriticalSection<'_>) -> R, + F: FnOnce() -> R, { panic!("cortex_m::interrupt::free() is only functional on cortex-m platforms"); } diff --git a/src/macros.rs b/src/macros.rs index 512c9323..83ef5648 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -62,7 +62,7 @@ macro_rules! iprintln { #[macro_export] macro_rules! singleton { ($name:ident: $ty:ty = $expr:expr) => { - $crate::interrupt::free(|_| { + $crate::interrupt::free(|| { // this is a tuple of a MaybeUninit and a bool because using an Option here is // problematic: Due to niche-optimization, an Option could end up producing a non-zero // initializer value which would move the entire static from `.bss` into `.data`... diff --git a/src/peripheral/mod.rs b/src/peripheral/mod.rs index c3168863..c6a0e257 100644 --- a/src/peripheral/mod.rs +++ b/src/peripheral/mod.rs @@ -164,7 +164,7 @@ impl Peripherals { /// Returns all the core peripherals *once* #[inline] pub fn take() -> Option { - interrupt::free(|_| { + interrupt::free(|| { if unsafe { TAKEN } { None } else { diff --git a/src/peripheral/sau.rs b/src/peripheral/sau.rs index da91aca9..b2d236a8 100644 --- a/src/peripheral/sau.rs +++ b/src/peripheral/sau.rs @@ -162,7 +162,7 @@ impl SAU { /// This function is executed under a critical section to prevent having inconsistent results. #[inline] pub fn set_region(&mut self, region_number: u8, region: SauRegion) -> Result<(), SauError> { - interrupt::free(|_| { + interrupt::free(|| { let base_address = region.base_address; let limit_address = region.limit_address; let attribute = region.attribute; @@ -215,7 +215,7 @@ impl SAU { /// This function is executed under a critical section to prevent having inconsistent results. #[inline] pub fn get_region(&mut self, region_number: u8) -> Result { - interrupt::free(|_| { + interrupt::free(|| { if region_number >= self.region_numbers() { Err(SauError::RegionNumberTooBig) } else { From d2210f4465ebd9b9867c6ba35bd73587ec493cbe Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 4 May 2022 02:58:07 +0200 Subject: [PATCH 3/9] Add single-core implementation for `critical_section` crate. --- Cargo.toml | 8 ++++++-- src/critical_section.rs | 27 +++++++++++++++++++++++++++ src/lib.rs | 5 ++--- 3 files changed, 35 insertions(+), 5 deletions(-) create mode 100644 src/critical_section.rs diff --git a/Cargo.toml b/Cargo.toml index b4f23c0f..8635331e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,10 +17,10 @@ rust-version = "1.59" links = "cortex-m" # prevent multiple versions of this crate to be linked together [dependencies] -bare-metal = "1" -volatile-register = "0.2.0" bitfield = "0.13.2" +critical-section = { version = "0.2", optional = true } embedded-hal = "0.2.4" +volatile-register = "0.2.0" [dependencies.serde] version = "1" @@ -32,6 +32,7 @@ cm7 = [] cm7-r0p1 = ["cm7"] linker-plugin-lto = [] std = [] +single-core-critical-section = ["critical-section", "critical-section/custom-impl"] [workspace] members = [ @@ -55,3 +56,6 @@ targets = [ "thumbv8m.base-none-eabi", "thumbv8m.main-none-eabi" ] + +[patch.crates-io] +critical-section = { git = "https://github.com/reitermarkus/critical-section", branch = "cortex-m" } diff --git a/src/critical_section.rs b/src/critical_section.rs new file mode 100644 index 00000000..81a2af82 --- /dev/null +++ b/src/critical_section.rs @@ -0,0 +1,27 @@ +use crate::interrupt; +use crate::register::primask::{self, Primask}; + +struct CriticalSection; +critical_section::custom_impl!(CriticalSection); + +const TOKEN_IGNORE: u8 = 0; +const TOKEN_REENABLE: u8 = 1; + +unsafe impl critical_section::Impl for CriticalSection { + unsafe fn acquire() -> u8 { + match primask::read() { + Primask::Active => { + interrupt::disable(); + TOKEN_REENABLE + } + Primask::Inactive => TOKEN_IGNORE, + } + } + + unsafe fn release(token: u8) { + // Only re-enable interrupts if they were enabled before the critical section. + if token == TOKEN_REENABLE { + interrupt::enable() + } + } +} diff --git a/src/lib.rs b/src/lib.rs index e430dd85..e5f80345 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,15 +43,14 @@ // Don't warn about feature(asm) being stable on Rust >= 1.59.0 #![allow(stable_features)] -extern crate bare_metal; -extern crate volatile_register; - #[macro_use] mod macros; pub mod asm; #[cfg(armv8m)] pub mod cmse; +#[cfg(feature = "single-core-critical-section")] +mod critical_section; pub mod delay; pub mod interrupt; #[cfg(all(not(armv6m), not(armv8m_base)))] From ea80d605147532e453385f9e08e89ec878fd69de Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 3 May 2022 07:42:31 +0200 Subject: [PATCH 4/9] Add changelog entry. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23496078..e95d2bd9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - TPIU: add `swo_supports` for checking what SWO configurations the target supports. (#381) - Add `std` and `serde` crate features for improved host-side ITM decode functionality when working with the downstream `itm`, `cargo-rtic-scope` crates (#363, #366). - Added the ability to name the statics generated by `singleton!()` for better debuggability (#364, #380). +- Added `single-core-critical-section` feature which implements the trait for the `critical_section` crate. (#433) ### Fixed - Fixed `singleton!()` statics sometimes ending up in `.data` instead of `.bss` (#364, #380). +- `interrupt::free` no longer wrongly (on multi-core systems) hands out `CriticalSection`s. (#433) ### Changed - Inline assembly is now always used, requiring Rust 1.59. From 3ced1f2d660d7dadfb9deab97d40806c420869dd Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 4 May 2022 21:46:04 +0200 Subject: [PATCH 5/9] Add patch note. --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 8635331e..d4295a78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,4 +58,5 @@ targets = [ ] [patch.crates-io] +# See https://github.com/embassy-rs/critical-section/pull/13. critical-section = { git = "https://github.com/reitermarkus/critical-section", branch = "cortex-m" } From 40b83528ab4d66651ce3e4079fca7101b1abd92c Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 4 May 2022 22:41:24 +0200 Subject: [PATCH 6/9] Fix tests. --- .github/workflows/on-target.yml | 4 ++-- src/lib.rs | 2 +- testsuite/Cargo.toml | 1 + testsuite/minitest/macros/src/lib.rs | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/on-target.yml b/.github/workflows/on-target.yml index 437c0ed9..e484ad7d 100644 --- a/.github/workflows/on-target.yml +++ b/.github/workflows/on-target.yml @@ -22,7 +22,7 @@ jobs: - name: Build testsuite env: RUSTFLAGS: -C link-arg=-Tlink.x -D warnings - run: cargo build -p testsuite --target thumbv7m-none-eabi --features testsuite/semihosting + run: cargo build -p testsuite --target thumbv7m-none-eabi --features semihosting,cortex-m/single-core-critical-section - name: Install QEMU run: sudo apt-get update && sudo apt-get install qemu qemu-system-arm - name: Run testsuite @@ -51,7 +51,7 @@ jobs: - name: Build testsuite env: RUSTFLAGS: -C link-arg=-Tlink.x -D warnings - run: cargo build -p testsuite --target thumbv6m-none-eabi --features testsuite/rtt + run: cargo build -p testsuite --target thumbv6m-none-eabi --features rtt,cortex-m/single-core-critical-section - name: Upload testsuite binaries uses: actions/upload-artifact@v3 with: diff --git a/src/lib.rs b/src/lib.rs index e5f80345..979dd1cd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,7 +49,7 @@ mod macros; pub mod asm; #[cfg(armv8m)] pub mod cmse; -#[cfg(feature = "single-core-critical-section")] +#[cfg(all(cortex_m, feature = "single-core-critical-section"))] mod critical_section; pub mod delay; pub mod interrupt; diff --git a/testsuite/Cargo.toml b/testsuite/Cargo.toml index 17f15626..53fda102 100644 --- a/testsuite/Cargo.toml +++ b/testsuite/Cargo.toml @@ -13,6 +13,7 @@ semihosting = ["cortex-m-semihosting", "minitest/semihosting"] cortex-m-rt.path = "../cortex-m-rt" cortex-m.path = ".." minitest.path = "minitest" +critical-section = "0.2" [dependencies.rtt-target] version = "0.3.1" diff --git a/testsuite/minitest/macros/src/lib.rs b/testsuite/minitest/macros/src/lib.rs index 65705022..e8a1087a 100644 --- a/testsuite/minitest/macros/src/lib.rs +++ b/testsuite/minitest/macros/src/lib.rs @@ -215,8 +215,8 @@ fn tests_impl(args: TokenStream, input: TokenStream) -> parse::Result Date: Thu, 5 May 2022 14:37:40 +0200 Subject: [PATCH 7/9] Replace `interrupt::free` with `critical_section::with`. --- .github/workflows/ci.yml | 2 +- .github/workflows/clippy.yml | 9 ++----- .github/workflows/rt-ci.yml | 14 +++++----- cortex-m-rt/ci/script.sh | 31 ++++++++++++---------- cortex-m-semihosting/Cargo.toml | 1 + cortex-m-semihosting/src/export.rs | 10 +++----- src/critical_section.rs | 41 +++++++++++++++++------------- src/lib.rs | 7 +++-- src/macros.rs | 2 +- src/peripheral/mod.rs | 3 +-- src/peripheral/sau.rs | 5 ++-- xtask/tests/ci.rs | 7 +++++ 12 files changed, 71 insertions(+), 61 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa88a56c..f668b890 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,6 +29,6 @@ jobs: toolchain: ${{ matrix.rust }} override: true - name: Run tests - run: cargo test --all --exclude cortex-m-rt --exclude testsuite + run: cargo test --all --exclude cortex-m-rt --exclude testsuite --features cortex-m/single-core-critical-section # FIXME: test on macOS and Windows diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 75c61dc2..50e7794b 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -1,7 +1,7 @@ on: push: branches: [ staging, trying, master ] - pull_request_target: + pull_request: name: Clippy check jobs: @@ -9,11 +9,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - if: github.event_name == 'pull_request_target' - with: - ref: refs/pull/${{ github.event.number }}/head - - uses: actions/checkout@v3 - if: github.event_name != 'pull_request_target' - uses: actions-rs/toolchain@v1 with: profile: minimal @@ -23,4 +18,4 @@ jobs: - uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --all + args: --all --features cortex-m/single-core-critical-section diff --git a/.github/workflows/rt-ci.yml b/.github/workflows/rt-ci.yml index c3efb0cc..4326e849 100644 --- a/.github/workflows/rt-ci.yml +++ b/.github/workflows/rt-ci.yml @@ -69,18 +69,18 @@ jobs: - name: Install all Rust targets run: rustup target install thumbv6m-none-eabi thumbv7m-none-eabi thumbv7em-none-eabi thumbv7em-none-eabihf thumbv8m.base-none-eabi thumbv8m.main-none-eabi thumbv8m.main-none-eabihf - name: Build examples for thumbv6m-none-eabi - run: cargo build --target=thumbv6m-none-eabi --examples + run: cargo build --target=thumbv6m-none-eabi --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv7m-none-eabi - run: cargo build --target=thumbv7m-none-eabi --examples + run: cargo build --target=thumbv7m-none-eabi --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv7em-none-eabi - run: cargo build --target=thumbv7em-none-eabi --examples + run: cargo build --target=thumbv7em-none-eabi --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv7em-none-eabihf - run: cargo build --target=thumbv7em-none-eabihf --examples + run: cargo build --target=thumbv7em-none-eabihf --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv8m.base-none-eabi - run: cargo build --target=thumbv8m.base-none-eabi --examples + run: cargo build --target=thumbv8m.base-none-eabi --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv8m.main-none-eabi - run: cargo build --target=thumbv8m.main-none-eabi --examples + run: cargo build --target=thumbv8m.main-none-eabi --features cortex-m/single-core-critical-section --examples - name: Build examples for thumbv8m.main-none-eabihf - run: cargo build --target=thumbv8m.main-none-eabihf --examples + run: cargo build --target=thumbv8m.main-none-eabihf --features cortex-m/single-core-critical-section --examples - name: Build crate for host OS run: cargo build diff --git a/cortex-m-rt/ci/script.sh b/cortex-m-rt/ci/script.sh index 4683566e..8160125e 100755 --- a/cortex-m-rt/ci/script.sh +++ b/cortex-m-rt/ci/script.sh @@ -7,10 +7,13 @@ main() { cargo check --target "$TARGET" --features device + # A `critical_section` implementation is always needed. + needed_features=cortex-m/single-core-critical-section + if [ "$TARGET" = x86_64-unknown-linux-gnu ] && [ "$TRAVIS_RUST_VERSION" = stable ]; then ( cd macros && cargo check && cargo test ) - cargo test --features device --test compiletest + cargo test --features "device,${needed_features}" --test compiletest fi local examples=( @@ -43,25 +46,25 @@ main() { if [ "$TARGET" != x86_64-unknown-linux-gnu ]; then # Only test on stable and nightly, not MSRV. if [ "$TRAVIS_RUST_VERSION" = stable ] || [ "$TRAVIS_RUST_VERSION" = nightly ]; then - RUSTDOCFLAGS="-Cpanic=abort" cargo test --doc + RUSTDOCFLAGS="-Cpanic=abort" cargo test --features "${needed_features}" --doc fi for linker in "${linkers[@]}"; do for ex in "${examples[@]}"; do - cargo rustc --target "$TARGET" --example "$ex" -- $linker - cargo rustc --target "$TARGET" --example "$ex" --release -- $linker + cargo rustc --target "$TARGET" --example "$ex" --features "${needed_features}" -- $linker + cargo rustc --target "$TARGET" --example "$ex" --features "${needed_features}" --release -- $linker done for ex in "${fail_examples[@]}"; do - ! cargo rustc --target "$TARGET" --example "$ex" -- $linker - ! cargo rustc --target "$TARGET" --example "$ex" --release -- $linker + ! cargo rustc --target "$TARGET" --example "$ex" --features "${needed_features}" -- $linker + ! cargo rustc --target "$TARGET" --example "$ex" --features "${needed_features}" --release -- $linker done - cargo rustc --target "$TARGET" --example device --features device -- $linker - cargo rustc --target "$TARGET" --example device --features device --release -- $linker + cargo rustc --target "$TARGET" --example device --features "device,${needed_features}" -- $linker + cargo rustc --target "$TARGET" --example device --features "device,${needed_features}" --release -- $linker - cargo rustc --target "$TARGET" --example minimal --features set-sp -- $linker - cargo rustc --target "$TARGET" --example minimal --features set-sp --release -- $linker - cargo rustc --target "$TARGET" --example minimal --features set-vtor -- $linker - cargo rustc --target "$TARGET" --example minimal --features set-vtor --release -- $linker + cargo rustc --target "$TARGET" --example minimal --features "set-sp,${needed_features}" -- $linker + cargo rustc --target "$TARGET" --example minimal --features "set-sp,${needed_features}" --release -- $linker + cargo rustc --target "$TARGET" --example minimal --features "set-vtor,${needed_features}" -- $linker + cargo rustc --target "$TARGET" --example minimal --features "set-vtor,${needed_features}" --release -- $linker done fi @@ -69,9 +72,9 @@ main() { thumbv6m-none-eabi|thumbv7m-none-eabi) for linker in "${linkers[@]}"; do env RUSTFLAGS="$linker -C link-arg=-Tlink.x" cargo run \ - --target "$TARGET" --example qemu | grep "x = 42" + --target "$TARGET" --features "${needed_features}" --example qemu | grep "x = 42" env RUSTFLAGS="$linker -C link-arg=-Tlink.x" cargo run \ - --target "$TARGET" --example qemu --release | grep "x = 42" + --target "$TARGET" --features "${needed_features}" --example qemu --release | grep "x = 42" done ;; diff --git a/cortex-m-semihosting/Cargo.toml b/cortex-m-semihosting/Cargo.toml index 5afe0ac2..fdb6c5c3 100644 --- a/cortex-m-semihosting/Cargo.toml +++ b/cortex-m-semihosting/Cargo.toml @@ -21,3 +21,4 @@ no-semihosting = [] [dependencies] cortex-m = { path = "..", version = ">= 0.5.8, < 0.8" } +critical-section = "0.2" diff --git a/cortex-m-semihosting/src/export.rs b/cortex-m-semihosting/src/export.rs index dc76d62c..46e70e79 100644 --- a/cortex-m-semihosting/src/export.rs +++ b/cortex-m-semihosting/src/export.rs @@ -2,14 +2,12 @@ use core::fmt::{self, Write}; -use cortex_m::interrupt; - use crate::hio::{self, HostStream}; static mut HSTDOUT: Option = None; pub fn hstdout_str(s: &str) { - let _result = interrupt::free(|| unsafe { + let _result = critical_section::with(|_| unsafe { if HSTDOUT.is_none() { HSTDOUT = Some(hio::hstdout()?); } @@ -19,7 +17,7 @@ pub fn hstdout_str(s: &str) { } pub fn hstdout_fmt(args: fmt::Arguments) { - let _result = interrupt::free(|| unsafe { + let _result = critical_section::with(|_| unsafe { if HSTDOUT.is_none() { HSTDOUT = Some(hio::hstdout()?); } @@ -31,7 +29,7 @@ pub fn hstdout_fmt(args: fmt::Arguments) { static mut HSTDERR: Option = None; pub fn hstderr_str(s: &str) { - let _result = interrupt::free(|| unsafe { + let _result = critical_section::with(|_| unsafe { if HSTDERR.is_none() { HSTDERR = Some(hio::hstderr()?); } @@ -41,7 +39,7 @@ pub fn hstderr_str(s: &str) { } pub fn hstderr_fmt(args: fmt::Arguments) { - let _result = interrupt::free(|| unsafe { + let _result = critical_section::with(|_| unsafe { if HSTDERR.is_none() { HSTDERR = Some(hio::hstderr()?); } diff --git a/src/critical_section.rs b/src/critical_section.rs index 81a2af82..06b6b83a 100644 --- a/src/critical_section.rs +++ b/src/critical_section.rs @@ -1,27 +1,32 @@ -use crate::interrupt; -use crate::register::primask::{self, Primask}; +#[cfg(all(cortex_m, feature = "single-core-critical-section"))] +mod single_core_critical_section { + use crate::interrupt; + use crate::register::primask::{self, Primask}; -struct CriticalSection; -critical_section::custom_impl!(CriticalSection); + struct CriticalSection; + critical_section::custom_impl!(CriticalSection); -const TOKEN_IGNORE: u8 = 0; -const TOKEN_REENABLE: u8 = 1; + const TOKEN_IGNORE: u8 = 0; + const TOKEN_REENABLE: u8 = 1; -unsafe impl critical_section::Impl for CriticalSection { - unsafe fn acquire() -> u8 { - match primask::read() { - Primask::Active => { - interrupt::disable(); - TOKEN_REENABLE + unsafe impl critical_section::Impl for CriticalSection { + unsafe fn acquire() -> u8 { + match primask::read() { + Primask::Active => { + interrupt::disable(); + TOKEN_REENABLE + } + Primask::Inactive => TOKEN_IGNORE, } - Primask::Inactive => TOKEN_IGNORE, } - } - unsafe fn release(token: u8) { - // Only re-enable interrupts if they were enabled before the critical section. - if token == TOKEN_REENABLE { - interrupt::enable() + unsafe fn release(token: u8) { + // Only re-enable interrupts if they were enabled before the critical section. + if token == TOKEN_REENABLE { + interrupt::enable() + } } } } + +pub use critical_section::with; diff --git a/src/lib.rs b/src/lib.rs index 979dd1cd..b0a0ba04 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,8 +49,11 @@ mod macros; pub mod asm; #[cfg(armv8m)] pub mod cmse; -#[cfg(all(cortex_m, feature = "single-core-critical-section"))] -mod critical_section; +// This is only public so the `singleton` macro does not require depending on +// the `critical-section` crate separately. +#[doc(hidden)] +#[cfg(feature = "critical-section")] +pub mod critical_section; pub mod delay; pub mod interrupt; #[cfg(all(not(armv6m), not(armv8m_base)))] diff --git a/src/macros.rs b/src/macros.rs index 83ef5648..21bf78b3 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -62,7 +62,7 @@ macro_rules! iprintln { #[macro_export] macro_rules! singleton { ($name:ident: $ty:ty = $expr:expr) => { - $crate::interrupt::free(|| { + $crate::critical_section::with(|_| { // this is a tuple of a MaybeUninit and a bool because using an Option here is // problematic: Due to niche-optimization, an Option could end up producing a non-zero // initializer value which would move the entire static from `.bss` into `.data`... diff --git a/src/peripheral/mod.rs b/src/peripheral/mod.rs index c6a0e257..bf18151f 100644 --- a/src/peripheral/mod.rs +++ b/src/peripheral/mod.rs @@ -57,7 +57,6 @@ //! //! - ARMv7-M Architecture Reference Manual (Issue E.b) - Chapter B3 -use crate::interrupt; use core::marker::PhantomData; use core::ops; @@ -164,7 +163,7 @@ impl Peripherals { /// Returns all the core peripherals *once* #[inline] pub fn take() -> Option { - interrupt::free(|| { + critical_section::with(|_| { if unsafe { TAKEN } { None } else { diff --git a/src/peripheral/sau.rs b/src/peripheral/sau.rs index b2d236a8..6b8477f3 100644 --- a/src/peripheral/sau.rs +++ b/src/peripheral/sau.rs @@ -7,7 +7,6 @@ //! //! For reference please check the section B8.3 of the Armv8-M Architecture Reference Manual. -use crate::interrupt; use crate::peripheral::SAU; use bitfield::bitfield; use volatile_register::{RO, RW}; @@ -162,7 +161,7 @@ impl SAU { /// This function is executed under a critical section to prevent having inconsistent results. #[inline] pub fn set_region(&mut self, region_number: u8, region: SauRegion) -> Result<(), SauError> { - interrupt::free(|| { + critical_section::with(|_| { let base_address = region.base_address; let limit_address = region.limit_address; let attribute = region.attribute; @@ -215,7 +214,7 @@ impl SAU { /// This function is executed under a critical section to prevent having inconsistent results. #[inline] pub fn get_region(&mut self, region_number: u8) -> Result { - interrupt::free(|| { + critical_section::with(|_| { if region_number >= self.region_numbers() { Err(SauError::RegionNumberTooBig) } else { diff --git a/xtask/tests/ci.rs b/xtask/tests/ci.rs index 603491c7..a25c633a 100644 --- a/xtask/tests/ci.rs +++ b/xtask/tests/ci.rs @@ -32,6 +32,13 @@ fn build(package: &str, target: &str, features: &[&str]) { cargo.args(&["--features", *feat]); } + // A `critical_section` implementation is always needed. + if package == "cortex-m" { + cargo.args(&["--features", "single-core-critical-section"]); + } else { + cargo.args(&["--features", "cortex-m/single-core-critical-section"]); + } + // Cargo features don't work right when invoked from the workspace root, so change to the // package's directory when necessary. if package != "cortex-m" { From 4b01e89ea93b14103e0cf929f6dee1cfc9169c11 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 8 May 2022 13:01:59 +0200 Subject: [PATCH 8/9] Update `critical-section` crate. --- Cargo.toml | 6 +++--- cortex-m-semihosting/Cargo.toml | 2 +- src/critical_section.rs | 21 ++++++++++----------- testsuite/Cargo.toml | 2 +- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d4295a78..f0c7ff07 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ links = "cortex-m" # prevent multiple versions of this crate to be linked toget [dependencies] bitfield = "0.13.2" -critical-section = { version = "0.2", optional = true } +critical-section = { version = "0.3", optional = true } embedded-hal = "0.2.4" volatile-register = "0.2.0" @@ -32,7 +32,7 @@ cm7 = [] cm7-r0p1 = ["cm7"] linker-plugin-lto = [] std = [] -single-core-critical-section = ["critical-section", "critical-section/custom-impl"] +single-core-critical-section = ["critical-section", "critical-section/token-bool"] [workspace] members = [ @@ -59,4 +59,4 @@ targets = [ [patch.crates-io] # See https://github.com/embassy-rs/critical-section/pull/13. -critical-section = { git = "https://github.com/reitermarkus/critical-section", branch = "cortex-m" } +critical-section = { git = "https://github.com/embassy-rs/critical-section" } diff --git a/cortex-m-semihosting/Cargo.toml b/cortex-m-semihosting/Cargo.toml index fdb6c5c3..f089a4c6 100644 --- a/cortex-m-semihosting/Cargo.toml +++ b/cortex-m-semihosting/Cargo.toml @@ -21,4 +21,4 @@ no-semihosting = [] [dependencies] cortex-m = { path = "..", version = ">= 0.5.8, < 0.8" } -critical-section = "0.2" +critical-section = "0.3" diff --git a/src/critical_section.rs b/src/critical_section.rs index 06b6b83a..6f785ae6 100644 --- a/src/critical_section.rs +++ b/src/critical_section.rs @@ -1,28 +1,27 @@ #[cfg(all(cortex_m, feature = "single-core-critical-section"))] mod single_core_critical_section { + use critical_section::{set_impl, Impl, RawToken}; + use crate::interrupt; use crate::register::primask::{self, Primask}; - struct CriticalSection; - critical_section::custom_impl!(CriticalSection); - - const TOKEN_IGNORE: u8 = 0; - const TOKEN_REENABLE: u8 = 1; + struct SingleCoreCriticalSection; + set_impl!(SingleCoreCriticalSection); - unsafe impl critical_section::Impl for CriticalSection { - unsafe fn acquire() -> u8 { + unsafe impl Impl for SingleCoreCriticalSection { + unsafe fn acquire() -> RawToken { match primask::read() { Primask::Active => { interrupt::disable(); - TOKEN_REENABLE + true } - Primask::Inactive => TOKEN_IGNORE, + Primask::Inactive => false, } } - unsafe fn release(token: u8) { + unsafe fn release(primask_was_active: RawToken) { // Only re-enable interrupts if they were enabled before the critical section. - if token == TOKEN_REENABLE { + if primask_was_active { interrupt::enable() } } diff --git a/testsuite/Cargo.toml b/testsuite/Cargo.toml index 53fda102..d4e131ee 100644 --- a/testsuite/Cargo.toml +++ b/testsuite/Cargo.toml @@ -13,7 +13,7 @@ semihosting = ["cortex-m-semihosting", "minitest/semihosting"] cortex-m-rt.path = "../cortex-m-rt" cortex-m.path = ".." minitest.path = "minitest" -critical-section = "0.2" +critical-section = "0.3" [dependencies.rtt-target] version = "0.3.1" From 183e7e8c7ae4214b9f39fbed1f81c736f074c884 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 14 Jun 2022 21:01:17 +0200 Subject: [PATCH 9/9] Update `critical_section` dependency. --- Cargo.toml | 4 ++-- src/critical_section.rs | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f0c7ff07..da4d6b18 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ links = "cortex-m" # prevent multiple versions of this crate to be linked toget [dependencies] bitfield = "0.13.2" -critical-section = { version = "0.3", optional = true } +critical-section = "0.3" embedded-hal = "0.2.4" volatile-register = "0.2.0" @@ -32,7 +32,7 @@ cm7 = [] cm7-r0p1 = ["cm7"] linker-plugin-lto = [] std = [] -single-core-critical-section = ["critical-section", "critical-section/token-bool"] +single-core-critical-section = ["critical-section/token-u8"] [workspace] members = [ diff --git a/src/critical_section.rs b/src/critical_section.rs index 6f785ae6..27bf4025 100644 --- a/src/critical_section.rs +++ b/src/critical_section.rs @@ -8,20 +8,23 @@ mod single_core_critical_section { struct SingleCoreCriticalSection; set_impl!(SingleCoreCriticalSection); + const TOKEN_IGNORE: RawToken = 0; + const TOKEN_REENABLE: RawToken = 1; + unsafe impl Impl for SingleCoreCriticalSection { unsafe fn acquire() -> RawToken { match primask::read() { Primask::Active => { interrupt::disable(); - true + TOKEN_REENABLE } - Primask::Inactive => false, + Primask::Inactive => TOKEN_IGNORE, } } - unsafe fn release(primask_was_active: RawToken) { + unsafe fn release(token: RawToken) { // Only re-enable interrupts if they were enabled before the critical section. - if primask_was_active { + if token == TOKEN_REENABLE { interrupt::enable() } }