From 368ab1d4fb780386f3162c7a84502e301af7f3b0 Mon Sep 17 00:00:00 2001 From: David Watson Date: Sun, 3 Jul 2022 12:24:11 -0400 Subject: [PATCH] Remove use of basepri register on thumbv8m.base The basepri register appears to be aviable on thumbv8m.main but not thumbv8m.base. At the very least, attempting to compile against a Cortex-M23 based Microchip ATSAML10E16A generates an error: ``` error[E0432]: unresolved import `cortex_m::register::basepri` --> /Users/dwatson/.cargo/registry/src/github.com-1ecc6299db9ec823/cortex-m-rtic-1.1.3/src/export.rs:25:5 | 25 | use cortex_m::register::basepri; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `basepri` in `register` ``` This is an attempt to account for the fact that thumbv8m.base (M23) MCUs don't have the BASEPRI register but have more than 32 interrupts. This moves away from the architecture specific config flags and switches to a more functional flag. Make the mask size depend on the max interrupt id Rather than assuming a fixed interrupt count of 32 this code uses an array of u32 bitmasks to calculate the priority mask. The size of this array is calculated at compile time based on the size of the largest interrupt id being used in the target code. For thumbv6m this should be equivalent to the previous version that used a single u32 mask. For thumbv8m.base it will be larger depending on the interrupts used. Don't write 0s to the ISER and ICER registers Writing 0s to these registers is a no-op. Since these masks should be calculated at compile time, this conditional should result in writes being optimized out of the code. Prevent panic on non-arm targets Panicking on unknown targets was breaking things like the doc build on linux. This change should only panic when building on unknown arm targets. --- CHANGELOG.md | 2 + build.rs | 18 ++- macros/src/codegen/assertions.rs | 11 +- macros/src/codegen/shared_resources.rs | 19 +++- macros/src/codegen/util.rs | 5 + src/export.rs | 149 ++++++++++++++++++------- 6 files changed, 146 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68af76937c..5ebd381151 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ### Fixed +- Distinguish between thumbv8m.base and thumbv8m.main for basepri usage. + ### Changed ## [v1.1.3] - 2022-06-23 diff --git a/build.rs b/build.rs index 7c518c90d4..ff9ebe35e7 100644 --- a/build.rs +++ b/build.rs @@ -7,15 +7,21 @@ fn main() { println!("cargo:rustc-cfg=rustc_is_nightly"); } - if target.starts_with("thumbv6m") { - println!("cargo:rustc-cfg=armv6m"); - } - + // These targets all have know support for the BASEPRI register. if target.starts_with("thumbv7m") | target.starts_with("thumbv7em") - | target.starts_with("thumbv8m") + | target.starts_with("thumbv8m.main") { - println!("cargo:rustc-cfg=armv7m"); + println!("cargo:rustc-cfg=have_basepri"); + + // These targets are all known to _not_ have the BASEPRI register. + } else if target.starts_with("thumb") + && !(target.starts_with("thumbv6m") | target.starts_with("thumbv8m.base")) + { + panic!( + "Unknown target '{}'. Need to update BASEPRI logic in build.rs.", + target + ); } println!("cargo:rerun-if-changed=build.rs"); diff --git a/macros/src/codegen/assertions.rs b/macros/src/codegen/assertions.rs index f6a098b5e6..66e5409504 100644 --- a/macros/src/codegen/assertions.rs +++ b/macros/src/codegen/assertions.rs @@ -22,15 +22,16 @@ pub fn codegen(app: &App, analysis: &Analysis, extra: &Extra) -> Vec = app + let chunks_name = util::priority_mask_chunks_ident(); + let no_basepri_checks: Vec<_> = app .hardware_tasks .iter() .filter_map(|(_, task)| { if !util::is_exception(&task.args.binds) { let interrupt_name = &task.args.binds; Some(quote!( - if (#device::Interrupt::#interrupt_name as u32) > 31 { - ::core::panic!("An interrupt above value 31 is used while in armv6"); + if (#device::Interrupt::#interrupt_name as usize) >= (#chunks_name * 32) { + ::core::panic!("An interrupt out of range is used while in armv6 or armv8m.base"); } )) } else { @@ -41,8 +42,8 @@ pub fn codegen(app: &App, analysis: &Analysis, extra: &Extra) -> Vec; 3] = [#(#mask_arr),*]; )); if uses_exceptions_with_resources { mod_app.push(quote!( #[doc(hidden)] #[allow(non_upper_case_globals)] - const __rtic_internal_V6_ERROR: () = rtic::export::v6_panic(); + const __rtic_internal_V6_ERROR: () = rtic::export::no_basepri_panic(); )); } diff --git a/macros/src/codegen/util.rs b/macros/src/codegen/util.rs index 0f3dca7c47..0a3edc20d2 100644 --- a/macros/src/codegen/util.rs +++ b/macros/src/codegen/util.rs @@ -253,6 +253,11 @@ pub fn static_shared_resource_ident(name: &Ident) -> Ident { mark_internal_name(&format!("shared_resource_{}", name)) } +/// Generates an Ident for the number of 32 bit chunks used for Mask storage. +pub fn priority_mask_chunks_ident() -> Ident { + mark_internal_name("MASK_CHUNKS") +} + pub fn priority_masks_ident() -> Ident { mark_internal_name("MASKS") } diff --git a/src/export.rs b/src/export.rs index a4d76b5357..6f2a1b63c1 100644 --- a/src/export.rs +++ b/src/export.rs @@ -21,10 +21,43 @@ pub use rtic_monotonic as monotonic; pub type SCFQ = Queue; pub type SCRQ = Queue<(T, u8), N>; -#[cfg(armv7m)] +/// Mask is used to store interrupt masks on systems without a BASEPRI register (M0, M0+, M23). +/// It needs to be large enough to cover all the relevant interrupts in use. +/// For M0/M0+ there are only 32 interrupts so we only need one u32 value. +/// For M23 there can be as many as 480 interrupts. +/// Rather than providing space for all possible interrupts, we just detect the highest interrupt in +/// use at compile time and allocate enough u32 chunks to cover them. +#[derive(Copy, Clone)] +pub struct Mask([u32; M]); + +impl core::ops::BitOrAssign for Mask { + fn bitor_assign(&mut self, rhs: Self) { + for i in 0..M { + self.0[i] |= rhs.0[i]; + } + } +} + +#[cfg(not(have_basepri))] +impl Mask { + /// Set a bit inside a Mask. + const fn set_bit(mut self, bit: u32) -> Self { + let block = bit / 32; + + if block as usize >= M { + panic!("Generating masks for thumbv6/thumbv8m.base failed! Are you compiling for thumbv6 on an thumbv7 MCU or using an unsupported thumbv8m.base MCU?"); + } + + let offset = bit - (block * 32); + self.0[block as usize] |= 1 << offset; + self + } +} + +#[cfg(have_basepri)] use cortex_m::register::basepri; -#[cfg(armv7m)] +#[cfg(have_basepri)] #[inline(always)] pub fn run(priority: u8, f: F) where @@ -41,7 +74,7 @@ where } } -#[cfg(not(armv7m))] +#[cfg(not(have_basepri))] #[inline(always)] pub fn run(_priority: u8, f: F) where @@ -105,16 +138,16 @@ impl Priority { } /// Const helper to check architecture -pub const fn is_armv6() -> bool { - #[cfg(not(armv6m))] - { - false - } - - #[cfg(armv6m)] +pub const fn have_basepri() -> bool { + #[cfg(have_basepri)] { true } + + #[cfg(not(have_basepri))] + { + false + } } #[inline(always)] @@ -172,14 +205,14 @@ where /// Total OH of per task is max 2 clock cycles, negligible in practice /// but can in theory be fixed. /// -#[cfg(armv7m)] +#[cfg(have_basepri)] #[inline(always)] -pub unsafe fn lock( +pub unsafe fn lock( ptr: *mut T, priority: &Priority, ceiling: u8, nvic_prio_bits: u8, - _mask: &[u32; 3], + _mask: &[Mask; 3], f: impl FnOnce(&mut T) -> R, ) -> R { let current = priority.get(); @@ -247,14 +280,14 @@ pub unsafe fn lock( /// - Temporary lower exception priority /// /// These possible solutions are set goals for future work -#[cfg(not(armv7m))] +#[cfg(not(have_basepri))] #[inline(always)] -pub unsafe fn lock( +pub unsafe fn lock( ptr: *mut T, priority: &Priority, ceiling: u8, _nvic_prio_bits: u8, - masks: &[u32; 3], + masks: &[Mask; 3], f: impl FnOnce(&mut T) -> R, ) -> R { let current = priority.get(); @@ -288,28 +321,38 @@ pub unsafe fn lock( } } -#[cfg(not(armv7m))] +#[cfg(not(have_basepri))] #[inline(always)] -fn compute_mask(from_prio: u8, to_prio: u8, masks: &[u32; 3]) -> u32 { - let mut res = 0; +fn compute_mask(from_prio: u8, to_prio: u8, masks: &[Mask; 3]) -> Mask { + let mut res = Mask([0; M]); masks[from_prio as usize..to_prio as usize] .iter() - .for_each(|m| res |= m); + .for_each(|m| res |= *m); res } // enables interrupts -#[cfg(not(armv7m))] +#[cfg(not(have_basepri))] #[inline(always)] -unsafe fn set_enable_mask(mask: u32) { - (*NVIC::PTR).iser[0].write(mask) +unsafe fn set_enable_mask(mask: Mask) { + for i in 0..M { + // This check should involve compile time constants and be optimized out. + if mask.0[i] != 0 { + (*NVIC::PTR).iser[i].write(mask.0[i]); + } + } } // disables interrupts -#[cfg(not(armv7m))] +#[cfg(not(have_basepri))] #[inline(always)] -unsafe fn clear_enable_mask(mask: u32) { - (*NVIC::PTR).icer[0].write(mask) +unsafe fn clear_enable_mask(mask: Mask) { + for i in 0..M { + // This check should involve compile time constants and be optimized out. + if mask.0[i] != 0 { + (*NVIC::PTR).icer[i].write(mask.0[i]); + } + } } #[inline] @@ -318,36 +361,56 @@ pub fn logical2hw(logical: u8, nvic_prio_bits: u8) -> u8 { ((1 << nvic_prio_bits) - logical) << (8 - nvic_prio_bits) } -#[cfg(not(armv6m))] -pub const fn create_mask(_: [u32; N]) -> u32 { - 0 +#[cfg(have_basepri)] +pub const fn create_mask(_: [u32; N]) -> Mask { + Mask([0; M]) } -#[cfg(armv6m)] -pub const fn create_mask(list_of_shifts: [u32; N]) -> u32 { - let mut mask = 0; +#[cfg(not(have_basepri))] +pub const fn create_mask(list_of_shifts: [u32; N]) -> Mask { + let mut mask = Mask([0; M]); let mut i = 0; while i < N { let shift = list_of_shifts[i]; i += 1; - - if shift > 31 { - panic!("Generating masks for thumbv6 failed! Are you compiling for thumbv6 on an thumbv7 MCU?"); - } - - mask |= 1 << shift; + mask = mask.set_bit(shift); } mask } -#[cfg(not(armv6m))] -pub const fn v6_panic() { +#[cfg(have_basepri)] +pub const fn compute_mask_chunks(_: [u32; L]) -> usize { + 0 +} + +/// Compute the number of u32 chunks needed to store the Mask value. +/// On M0, M0+ this should always end up being 1. +/// On M23 we will pick a number that allows us to store the highest index used by the code. +/// This means the amount of overhead will vary based on the actually interrupts used by the code. +#[cfg(not(have_basepri))] +pub const fn compute_mask_chunks(ids: [u32; L]) -> usize { + let mut max: usize = 0; + let mut i = 0; + + while i < L { + let id = ids[i] as usize; + i += 1; + + if id > max { + max = id; + } + } + (max + 32) / 32 +} + +#[cfg(have_basepri)] +pub const fn no_basepri_panic() { // For non-v6 all is fine } -#[cfg(armv6m)] -pub const fn v6_panic() { - panic!("Exceptions with shared resources are not allowed when compiling for thumbv6. Use local resources or `#[lock_free]` shared resources"); +#[cfg(not(have_basepri))] +pub const fn no_basepri_panic() { + panic!("Exceptions with shared resources are not allowed when compiling for thumbv6 or thumbv8m.base. Use local resources or `#[lock_free]` shared resources"); }