diff --git a/book/src/by-example/tips.md b/book/src/by-example/tips.md index 0e3d47b7ad..c1633288ed 100644 --- a/book/src/by-example/tips.md +++ b/book/src/by-example/tips.md @@ -1,5 +1,25 @@ # Tips & tricks +## Generics + +Resources shared between two or more tasks implement the `Mutex` trait in *all* +contexts, even on those where a critical section is not required to access the +data. This lets you easily write generic code that operates on resources and can +be called from different tasks. Here's one such example: + +``` rust +{{#include ../../../examples/generics.rs}} +``` + +``` console +$ cargo run --example generics +{{#include ../../../ci/expected/generics.run}}``` + +This also lets you change the static priorities of tasks without having to +rewrite code. If you consistently use `lock`s to access the data behind shared +resources then your code will continue to compile when you change the priority +of tasks. + ## Running tasks from RAM The main goal of moving the specification of RTFM applications to attributes in diff --git a/ci/expected/generics.run b/ci/expected/generics.run new file mode 100644 index 0000000000..7fa97758e1 --- /dev/null +++ b/ci/expected/generics.run @@ -0,0 +1,6 @@ +UART1(STATE = 0) +SHARED: 0 -> 1 +UART0(STATE = 0) +SHARED: 1 -> 2 +UART1(STATE = 1) +SHARED: 2 -> 4 diff --git a/ci/script.sh b/ci/script.sh index 4c86d144ab..eb582d4c4c 100644 --- a/ci/script.sh +++ b/ci/script.sh @@ -57,6 +57,7 @@ main() { not-send not-sync + generics ramfunc ) diff --git a/examples/capacity.rs b/examples/capacity.rs index 2dea2c300e..fca7fe2bdf 100644 --- a/examples/capacity.rs +++ b/examples/capacity.rs @@ -23,7 +23,7 @@ macro_rules! println { #[app(device = lm3s6965)] const APP: () = { - #[init(spawn = [foo])] + #[init] fn init() { rtfm::pend(Interrupt::UART0); } diff --git a/examples/generics.rs b/examples/generics.rs new file mode 100644 index 0000000000..aee70611cd --- /dev/null +++ b/examples/generics.rs @@ -0,0 +1,74 @@ +//! examples/generics.rs + +#![deny(unsafe_code)] +#![deny(warnings)] +#![no_main] +#![no_std] + +extern crate panic_semihosting; + +use cortex_m_semihosting::debug; +use lm3s6965::Interrupt; +use rtfm::{app, Mutex}; + +// NOTE: This convenience macro will appear in all the other examples and +// will always look the same +macro_rules! println { + ($($tt:tt)*) => { + if let Ok(mut stdout) = cortex_m_semihosting::hio::hstdout() { + use core::fmt::Write; + + writeln!(stdout, $($tt)*).ok(); + } + }; +} + +#[app(device = lm3s6965)] +const APP: () = { + static mut SHARED: u32 = 0; + + #[init] + fn init() { + rtfm::pend(Interrupt::UART0); + rtfm::pend(Interrupt::UART1); + } + + #[interrupt(resources = [SHARED])] + fn UART0() { + static mut STATE: u32 = 0; + + println!("UART0(STATE = {})", *STATE); + + advance(STATE, resources.SHARED); + + rtfm::pend(Interrupt::UART1); + + debug::exit(debug::EXIT_SUCCESS); + } + + #[interrupt(priority = 2, resources = [SHARED])] + fn UART1() { + static mut STATE: u32 = 0; + + println!("UART1(STATE = {})", *STATE); + + // just to show that `SHARED` can be accessed directly and .. + *resources.SHARED += 0; + // .. also through a (no-op) `lock` + resources.SHARED.lock(|shared| *shared += 0); + + advance(STATE, resources.SHARED); + } +}; + +fn advance(state: &mut u32, mut shared: impl Mutex) { + *state += 1; + + let (old, new) = shared.lock(|shared| { + let old = *shared; + *shared += *state; + (old, *shared) + }); + + println!("SHARED: {} -> {}", old, new); +} diff --git a/examples/types.rs b/examples/types.rs index 660620858f..c1b8cd6923 100644 --- a/examples/types.rs +++ b/examples/types.rs @@ -8,7 +8,7 @@ extern crate panic_semihosting; use cortex_m_semihosting::debug; -use rtfm::{app, Instant}; +use rtfm::{app, Exclusive, Instant}; #[app(device = lm3s6965)] const APP: () = { @@ -43,6 +43,7 @@ const APP: () = { #[task(priority = 2, resources = [SHARED], schedule = [foo], spawn = [foo])] fn foo() { let _: Instant = scheduled; + let _: Exclusive = resources.SHARED; let _: foo::Resources = resources; let _: foo::Schedule = schedule; let _: foo::Spawn = spawn; diff --git a/macros/src/analyze.rs b/macros/src/analyze.rs index 04b462fa7a..869b5d2060 100644 --- a/macros/src/analyze.rs +++ b/macros/src/analyze.rs @@ -30,13 +30,14 @@ pub struct Analysis { pub enum Ownership { // NOTE priorities and ceilings are "logical" (0 = lowest priority, 255 = highest priority) Owned { priority: u8 }, + CoOwned { priority: u8 }, Shared { ceiling: u8 }, } impl Ownership { pub fn needs_lock(&self, priority: u8) -> bool { match *self { - Ownership::Owned { .. } => false, + Ownership::Owned { .. } | Ownership::CoOwned { .. } => false, Ownership::Shared { ceiling } => { debug_assert!(ceiling >= priority); @@ -44,6 +45,13 @@ impl Ownership { } } } + + pub fn is_owned(&self) -> bool { + match *self { + Ownership::Owned { .. } => true, + _ => false, + } + } } pub struct Dispatcher { @@ -72,18 +80,24 @@ pub fn app(app: &App) -> Analysis { for (priority, res) in app.resource_accesses() { if let Some(ownership) = ownerships.get_mut(res) { match *ownership { - Ownership::Owned { priority: ceiling } | Ownership::Shared { ceiling } => { - if priority != ceiling { - *ownership = Ownership::Shared { - ceiling: cmp::max(ceiling, priority), - }; + Ownership::Owned { priority: ceiling } + | Ownership::CoOwned { priority: ceiling } + | Ownership::Shared { ceiling } + if priority != ceiling => + { + *ownership = Ownership::Shared { + ceiling: cmp::max(ceiling, priority), + }; - let res = &app.resources[res]; - if res.mutability.is_none() { - assert_sync.insert(res.ty.clone()); - } + let res = &app.resources[res]; + if res.mutability.is_none() { + assert_sync.insert(res.ty.clone()); } } + Ownership::Owned { priority: ceiling } if ceiling == priority => { + *ownership = Ownership::CoOwned { priority }; + } + _ => {} } continue; diff --git a/macros/src/codegen.rs b/macros/src/codegen.rs index ff1062ae4b..6d01d32d11 100644 --- a/macros/src/codegen.rs +++ b/macros/src/codegen.rs @@ -676,6 +676,7 @@ fn prelude( } } else { let ownership = &analysis.ownerships[name]; + let mut exclusive = false; if ownership.needs_lock(logical_prio) { may_call_lock = true; @@ -710,28 +711,53 @@ fn prelude( exprs.push(quote!(#name: <#name as owned_singleton::Singleton>::new())); } else { needs_unsafe = true; - defs.push(quote!(pub #name: &'a mut #name)); - exprs.push( - quote!(#name: &mut <#name as owned_singleton::Singleton>::new()), - ); + if ownership.is_owned() { + defs.push(quote!(pub #name: &'a mut #name)); + exprs.push(quote!( + #name: &mut <#name as owned_singleton::Singleton>::new() + )); + } else { + may_call_lock = true; + defs.push(quote!(pub #name: rtfm::Exclusive<'a, #name>)); + exprs.push(quote!( + #name: rtfm::Exclusive( + &mut <#name as owned_singleton::Singleton>::new() + ) + )); + } } continue; } else { - defs.push(quote!(pub #name: &#lt #mut_ #ty)); + if ownership.is_owned() || mut_.is_none() { + defs.push(quote!(pub #name: &#lt #mut_ #ty)); + } else { + exclusive = true; + may_call_lock = true; + defs.push(quote!(pub #name: rtfm::Exclusive<#lt, #ty>)); + } } } let alias = &ctxt.statics[name]; needs_unsafe = true; if initialized { - exprs.push(quote!(#name: &#mut_ #alias)); + if exclusive { + exprs.push(quote!(#name: rtfm::Exclusive(&mut #alias))); + } else { + exprs.push(quote!(#name: &#mut_ #alias)); + } } else { let method = if mut_.is_some() { quote!(get_mut) } else { quote!(get_ref) }; - exprs.push(quote!(#name: #alias.#method() )); + + if exclusive { + exprs.push(quote!(#name: rtfm::Exclusive(#alias.#method()) )); + } else { + exprs.push(quote!(#name: #alias.#method() )); + } } } } @@ -1655,19 +1681,23 @@ fn mk_resource( }; items.push(quote!( - unsafe impl<'a> rtfm::Mutex for #path<'a> { - const CEILING: u8 = #ceiling; - const NVIC_PRIO_BITS: u8 = #device::NVIC_PRIO_BITS; - type Data = #ty; + impl<'a> rtfm::Mutex for #path<'a> { + type T = #ty; - #[inline(always)] - unsafe fn priority(&self) -> &core::cell::Cell { - &self.#priority - } - - #[inline(always)] - fn ptr(&self) -> *mut Self::Data { - unsafe { #ptr } + #[inline] + fn lock(&mut self, f: F) -> R + where + F: FnOnce(&mut Self::T) -> R, + { + unsafe { + rtfm::export::claim( + #ptr, + &self.#priority, + #ceiling, + #device::NVIC_PRIO_BITS, + f, + ) + } } } )); diff --git a/src/export.rs b/src/export.rs index cb63e0cee3..200c69d95e 100644 --- a/src/export.rs +++ b/src/export.rs @@ -1,5 +1,8 @@ -/// IMPLEMENTATION DETAILS. DO NOT USE ANYTHING IN THIS MODULE -use core::{hint, ptr}; +//! IMPLEMENTATION DETAILS. DO NOT USE ANYTHING IN THIS MODULE + +#[cfg(not(debug_assertions))] +use core::hint; +use core::{cell::Cell, ptr, u8}; #[cfg(armv7m)] use cortex_m::register::basepri; @@ -52,7 +55,13 @@ impl MaybeUninit { if let Some(x) = self.value.as_ref() { x } else { - hint::unreachable_unchecked() + match () { + // Try to catch UB when compiling in release with debug assertions enabled + #[cfg(debug_assertions)] + () => unreachable!(), + #[cfg(not(debug_assertions))] + () => hint::unreachable_unchecked(), + } } } @@ -60,12 +69,19 @@ impl MaybeUninit { if let Some(x) = self.value.as_mut() { x } else { - hint::unreachable_unchecked() + match () { + // Try to catch UB when compiling in release with debug assertions enabled + #[cfg(debug_assertions)] + () => unreachable!(), + #[cfg(not(debug_assertions))] + () => hint::unreachable_unchecked(), + } } } pub fn set(&mut self, val: T) { - unsafe { ptr::write(&mut self.value, Some(val)) } + // NOTE(volatile) we have observed UB when this uses a plain `ptr::write` + unsafe { ptr::write_volatile(&mut self.value, Some(val)) } } } @@ -82,3 +98,66 @@ where T: Sync, { } + +#[cfg(armv7m)] +#[inline(always)] +pub unsafe fn claim( + ptr: *mut T, + priority: &Cell, + ceiling: u8, + nvic_prio_bits: u8, + f: F, +) -> R +where + F: FnOnce(&mut T) -> R, +{ + let current = priority.get(); + + if priority.get() < ceiling { + if ceiling == (1 << nvic_prio_bits) { + priority.set(u8::MAX); + let r = interrupt::free(|_| f(&mut *ptr)); + priority.set(current); + r + } else { + priority.set(ceiling); + basepri::write(logical2hw(ceiling, nvic_prio_bits)); + let r = f(&mut *ptr); + basepri::write(logical2hw(current, nvic_prio_bits)); + priority.set(current); + r + } + } else { + f(&mut *ptr) + } +} + +#[cfg(not(armv7m))] +#[inline(always)] +pub unsafe fn claim( + ptr: *mut T, + priority: &Cell, + ceiling: u8, + _nvic_prio_bits: u8, + f: F, +) -> R +where + F: FnOnce(&mut T) -> R, +{ + let current = priority.get(); + + if priority.get() < ceiling { + priority.set(u8::MAX); + let r = interrupt::free(|_| f(&mut *ptr)); + priority.set(current); + r + } else { + f(&mut *ptr) + } +} + +#[cfg(armv7m)] +#[inline] +fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 { + ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits) +} diff --git a/src/lib.rs b/src/lib.rs index 836654cba7..943413fd01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,16 +26,14 @@ #![deny(warnings)] #![no_std] -use core::{cell::Cell, u8}; #[cfg(feature = "timer-queue")] -use core::{cmp::Ordering, ops}; +use core::cmp::Ordering; +use core::{fmt, ops}; #[cfg(not(feature = "timer-queue"))] use cortex_m::peripheral::SYST; -#[cfg(armv7m)] -use cortex_m::register::basepri; use cortex_m::{ - interrupt::{self, Nr}, + interrupt::Nr, peripheral::{CBP, CPUID, DCB, DWT, FPB, FPU, ITM, MPU, NVIC, SCB, TPIU}, }; pub use cortex_m_rtfm_macros::app; @@ -253,81 +251,76 @@ impl U32Ext for u32 { /// [BASEPRI]) of the current context. /// /// [BASEPRI]: https://developer.arm.com/products/architecture/cpu-architecture/m-profile/docs/100701/latest/special-purpose-mask-registers -pub unsafe trait Mutex { - /// IMPLEMENTATION DETAIL. DO NOT USE THIS CONSTANT - #[doc(hidden)] - const CEILING: u8; - - /// IMPLEMENTATION DETAIL. DO NOT USE THIS CONSTANT - #[doc(hidden)] - const NVIC_PRIO_BITS: u8; - +pub trait Mutex { /// Data protected by the mutex - type Data: Send; - - /// IMPLEMENTATION DETAIL. DO NOT USE THIS METHOD - #[doc(hidden)] - unsafe fn priority(&self) -> &Cell; - - /// IMPLEMENTATION DETAIL. DO NOT USE THIS METHOD - #[doc(hidden)] - fn ptr(&self) -> *mut Self::Data; + type T; /// Creates a critical section and grants temporary access to the protected data - #[inline(always)] - #[cfg(armv7m)] fn lock(&mut self, f: F) -> R where - F: FnOnce(&mut Self::Data) -> R, - { - unsafe { - let current = self.priority().get(); + F: FnOnce(&mut Self::T) -> R; +} - if self.priority().get() < Self::CEILING { - if Self::CEILING == (1 << Self::NVIC_PRIO_BITS) { - self.priority().set(u8::MAX); - let r = interrupt::free(|_| f(&mut *self.ptr())); - self.priority().set(current); - r - } else { - self.priority().set(Self::CEILING); - basepri::write(logical2hw(Self::CEILING, Self::NVIC_PRIO_BITS)); - let r = f(&mut *self.ptr()); - basepri::write(logical2hw(current, Self::NVIC_PRIO_BITS)); - self.priority().set(current); - r - } - } else { - f(&mut *self.ptr()) - } - } - } +impl<'a, M> Mutex for &'a mut M +where + M: Mutex, +{ + type T = M::T; - /// Creates a critical section and grants temporary access to the protected data - #[cfg(not(armv7m))] fn lock(&mut self, f: F) -> R where - F: FnOnce(&mut Self::Data) -> R, + F: FnOnce(&mut Self::T) -> R, { - unsafe { - let current = self.priority().get(); - - if self.priority().get() < Self::CEILING { - self.priority().set(u8::MAX); - let r = interrupt::free(|_| f(&mut *self.ptr())); - self.priority().set(current); - r - } else { - f(&mut *self.ptr()) - } - } + (**self).lock(f) } } -#[cfg(armv7m)] -#[inline] -fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 { - ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits) +/// Newtype over `&'a mut T` that implements the `Mutex` trait +/// +/// The `Mutex` implementation for this type is a no-op, no critical section is created +pub struct Exclusive<'a, T>(pub &'a mut T); + +impl<'a, T> Mutex for Exclusive<'a, T> { + type T = T; + + fn lock(&mut self, f: F) -> R + where + F: FnOnce(&mut Self::T) -> R, + { + f(self.0) + } +} + +impl<'a, T> fmt::Debug for Exclusive<'a, T> +where + T: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + (**self).fmt(f) + } +} + +impl<'a, T> fmt::Display for Exclusive<'a, T> +where + T: fmt::Display, +{ + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + (**self).fmt(f) + } +} + +impl<'a, T> ops::Deref for Exclusive<'a, T> { + type Target = T; + + fn deref(&self) -> &T { + self.0 + } +} + +impl<'a, T> ops::DerefMut for Exclusive<'a, T> { + fn deref_mut(&mut self) -> &mut T { + self.0 + } } /// Sets the given `interrupt` as pending diff --git a/src/tq.rs b/src/tq.rs index 88db2f84a2..8d52051855 100644 --- a/src/tq.rs +++ b/src/tq.rs @@ -91,7 +91,7 @@ where #[inline(always)] pub fn isr(mut tq: TQ, mut f: F) where - TQ: Mutex>, + TQ: Mutex>, T: Copy + Send, N: ArrayLength>, F: FnMut(T, u8),