diff --git a/book/en/src/internals/targets.md b/book/en/src/internals/targets.md index 210b3814bd42..fbd11514e2a8 100644 --- a/book/en/src/internals/targets.md +++ b/book/en/src/internals/targets.md @@ -2,17 +2,17 @@ ## Cortex-M Devices -While RTIC can currently target all Cortex-m devices there are some key architecture differences that +While RTIC can currently target all Cortex-M devices there are some key architecture differences that users should be aware of. Namely, the absence of Base Priority Mask Register (`BASEPRI`) which lends itself exceptionally well to the hardware priority ceiling support used in RTIC, in the ARMv6-M and ARMv8-M-base architectures, which forces RTIC to use source masking instead. For each implementation -of lock and a detailed commentary of pros and cons, see the implementation of -[lock in src/export.rs][src_export]. +of lock and a detailed commentary of pros and cons, see the implementations of +[lock under rtic/src/export][src_export]. -[src_export]: https://github.com/rtic-rs/rtic/blob/master/src/export.rs +[src_export]: https://github.com/rtic-rs/rtic/tree/master/rtic/src/export These differences influence how critical sections are realized, but functionality should be the same -except that ARMv6-M/ARMv8-M-base cannot have tasks with shared resources bound to exception +except that ARMv6-M/ARMv8-M-base will fall back to a global critical section for tasks with shared resources bound to exception handlers, as these cannot be masked in hardware. Table 1 below shows a list of Cortex-m processors and which type of critical section they employ. @@ -67,7 +67,7 @@ priority than task B, it immediately preempts task B and is free to use the shar risk of data race conditions. At time *t4*, task A completes and returns the execution context to B. Since source masking relies on use of the NVIC, core exception sources such as HardFault, SVCall, -PendSV, and SysTick cannot share data with other tasks. +PendSV, and SysTick will fall back to a global critical section for locking when sharing data with other tasks. ## RISC-V Devices diff --git a/rtic-macros/CHANGELOG.md b/rtic-macros/CHANGELOG.md index 3390c531d492..5369622cfb78 100644 --- a/rtic-macros/CHANGELOG.md +++ b/rtic-macros/CHANGELOG.md @@ -7,6 +7,11 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ## [Unreleased] +### Fixed + +- Fixed race condition when using shared resources in exception handlers on ARMv6-M and ARMv8-M-base. + Note that on ARMv6-M and ARMv8-M-base accessing resources shared with exception handlers now uses a global critical section. + ## [v2.2.0] - 2025-06-22 ### Added diff --git a/rtic-macros/src/codegen/bindings/cortex.rs b/rtic-macros/src/codegen/bindings/cortex.rs index 5c56261db5d4..a6394aec00cd 100644 --- a/rtic-macros/src/codegen/bindings/cortex.rs +++ b/rtic-macros/src/codegen/bindings/cortex.rs @@ -75,17 +75,19 @@ mod source_masking { app: &App, analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, name: &Ident, + path: &TokenStream2, ty: &TokenStream2, - ceiling: u8, + mut ceiling: u8, ptr: &TokenStream2, ) -> TokenStream2 { - let path = if resources_prefix { - quote!(shared_resources::#name) - } else { - quote!(#name) - }; + // If resource is shared with an exception handler then boost ceiling to `u8::MAX` + // This forces usage of global critical section in `rtic::export::lock(..)` + if app.hardware_tasks.values().any(|task| { + is_exception(&task.args.binds) && task.args.shared_resources.contains_key(name) + }) { + ceiling = u8::MAX; + } // Computing mapping of used interrupts to masks let interrupt_ids = analysis.interrupts.iter().map(|(p, (id, _))| (p, id)); @@ -164,18 +166,12 @@ mod basepri { app: &App, _analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, - name: &Ident, + _name: &Ident, + path: &TokenStream2, ty: &TokenStream2, ceiling: u8, ptr: &TokenStream2, ) -> TokenStream2 { - let path = if resources_prefix { - quote!(shared_resources::#name) - } else { - quote!(#name) - }; - let device = &app.args.device; quote!( #(#cfgs)* diff --git a/rtic-macros/src/codegen/bindings/esp32c3.rs b/rtic-macros/src/codegen/bindings/esp32c3.rs index 444f4e3a8b6a..10bc25ba5ace 100644 --- a/rtic-macros/src/codegen/bindings/esp32c3.rs +++ b/rtic-macros/src/codegen/bindings/esp32c3.rs @@ -19,17 +19,12 @@ mod esp32c3 { _app: &App, _analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, - name: &Ident, + _name: &Ident, + path: &TokenStream2, ty: &TokenStream2, ceiling: u8, ptr: &TokenStream2, ) -> TokenStream2 { - let path = if resources_prefix { - quote!(shared_resources::#name) - } else { - quote!(#name) - }; quote!( #(#cfgs)* impl<'a> rtic::Mutex for #path<'a> { diff --git a/rtic-macros/src/codegen/bindings/esp32c6.rs b/rtic-macros/src/codegen/bindings/esp32c6.rs index 263651b2d9ad..492c5d0ca44c 100644 --- a/rtic-macros/src/codegen/bindings/esp32c6.rs +++ b/rtic-macros/src/codegen/bindings/esp32c6.rs @@ -24,17 +24,12 @@ mod esp32c6 { _app: &App, _analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, - name: &Ident, + _name: &Ident, + path: &TokenStream2, ty: &TokenStream2, ceiling: u8, ptr: &TokenStream2, ) -> TokenStream2 { - let path = if resources_prefix { - quote!(shared_resources::#name) - } else { - quote!(#name) - }; quote!( #(#cfgs)* impl<'a> rtic::Mutex for #path<'a> { diff --git a/rtic-macros/src/codegen/bindings/riscv_slic.rs b/rtic-macros/src/codegen/bindings/riscv_slic.rs index 30bf8fb6306f..afa729ca1c4b 100644 --- a/rtic-macros/src/codegen/bindings/riscv_slic.rs +++ b/rtic-macros/src/codegen/bindings/riscv_slic.rs @@ -27,18 +27,12 @@ pub fn impl_mutex( _app: &App, _analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, - name: &Ident, + _name: &Ident, + path: &TokenStream2, ty: &TokenStream2, ceiling: u8, ptr: &TokenStream2, ) -> TokenStream2 { - let path = if resources_prefix { - quote!(shared_resources::#name) - } else { - quote!(#name) - }; - quote!( #(#cfgs)* impl<'a> rtic::Mutex for #path<'a> { diff --git a/rtic-macros/src/codegen/bindings/template.rs b/rtic-macros/src/codegen/bindings/template.rs index be41aa205be8..b4d67aedc967 100644 --- a/rtic-macros/src/codegen/bindings/template.rs +++ b/rtic-macros/src/codegen/bindings/template.rs @@ -21,8 +21,8 @@ pub fn impl_mutex( app: &App, analysis: &CodegenAnalysis, cfgs: &[Attribute], - resources_prefix: bool, name: &Ident, + path: &TokenStream2, ty: &TokenStream2, ceiling: u8, ptr: &TokenStream2, diff --git a/rtic-macros/src/codegen/shared_resources.rs b/rtic-macros/src/codegen/shared_resources.rs index 5e8ca9ae77ca..5dffbd7c3dd4 100644 --- a/rtic-macros/src/codegen/shared_resources.rs +++ b/rtic-macros/src/codegen/shared_resources.rs @@ -92,8 +92,8 @@ pub fn codegen(app: &App, analysis: &Analysis) -> TokenStream2 { app, analysis, cfgs, - true, - &shared_name, + name, + "e!(shared_resources::#shared_name), "e!(#ty), ceiling, &ptr, diff --git a/rtic-sync/src/signal.rs b/rtic-sync/src/signal.rs index d43e9d5da372..737615ae76e1 100644 --- a/rtic-sync/src/signal.rs +++ b/rtic-sync/src/signal.rs @@ -48,7 +48,7 @@ impl Signal { } /// Split the signal into a writer and reader. - pub fn split(&self) -> (SignalWriter, SignalReader) { + pub fn split(&self) -> (SignalWriter<'_, T>, SignalReader<'_, T>) { (SignalWriter { parent: self }, SignalReader { parent: self }) } } diff --git a/rtic/CHANGELOG.md b/rtic/CHANGELOG.md index 12f5cf004891..e11bee2de343 100644 --- a/rtic/CHANGELOG.md +++ b/rtic/CHANGELOG.md @@ -20,6 +20,11 @@ Example: ## [Unreleased] +### Fixed + +- Fixed race condition when using shared resources in exception handlers on ARMv6-M and ARMv8-M-base. + Note that on ARMv6-M and ARMv8-M-base accessing resources shared with exception handlers now uses a global critical section. + ## [v2.2.0] - 2025-06-22 ### Added diff --git a/rtic/src/export/cortex_source_mask.rs b/rtic/src/export/cortex_source_mask.rs index 4fb62a403b14..c802fd7aed3a 100644 --- a/rtic/src/export/cortex_source_mask.rs +++ b/rtic/src/export/cortex_source_mask.rs @@ -100,7 +100,7 @@ where /// (Sub)-zero as: /// - Either zero OH (lock optimized out), or /// - Amounting to an optimal assembly implementation -/// - if ceiling == (1 << nvic_prio_bits) +/// - if ceiling >= 4 /// - we execute the closure in a global critical section (interrupt free) /// - CS entry cost, single write to core register /// - CS exit cost, single write to core register @@ -112,11 +112,10 @@ where /// - On par or better than any hand written implementation of SRP /// /// Limitations: -/// Current implementation does not allow for tasks with shared resources -/// to be bound to exception handlers, as these cannot be masked in HW. +/// Current implementation always uses a global critical section when +/// one of the tasks is bound to an exception handler (this is indicated by `ceiling == u8::MAX`). /// /// Possible solutions: -/// - Mask exceptions by global critical sections (interrupt::free) /// - Temporary lower exception priority /// /// These possible solutions are set goals for future work @@ -129,10 +128,10 @@ pub unsafe fn lock( ) -> R { unsafe { if ceiling >= 4 { - // safe to manipulate outside critical section - // execute closure under protection of raised system ceiling + // note: `ceiling == u8::MAX` is used to indicate that one of the tasks is bound to an exception handler + // exception handlers can't be masked with NVIC, so we need to use a global critical section - // safe to manipulate outside critical section + // execute closure under protection of global critical section critical_section::with(|_| f(&mut *ptr)) } else { // safe to manipulate outside critical section diff --git a/xtask/src/cargo_command.rs b/xtask/src/cargo_command.rs index f73f47a4a0f2..e00224cdd2c9 100644 --- a/xtask/src/cargo_command.rs +++ b/xtask/src/cargo_command.rs @@ -700,7 +700,7 @@ impl<'a> CargoCommand<'a> { } } - fn target(&self) -> Option<&Target> { + fn target(&self) -> Option<&Target<'_>> { match self { CargoCommand::Run { target, .. } | CargoCommand::Qemu { target, .. }