From 744e2cbb8a0b58eccf3a9708c8c28f1ef17e4771 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Mon, 5 Jul 2021 17:42:43 +1000 Subject: extras: Fix UB in `Peripheral` `Peripheral` assumed that interrupts can't be preempted, when they can be preempted by higher priority interrupts. So I put the interrupt handler inside a critical section, and also added checks for whether the state had been dropped before the critical section was entered. I also added a `'static` bound to `PeripheralState`, since `Pin` only guarantees that the memory it directly references will not be invalidated. It doesn't guarantee that memory its pointee references also won't be invalidated. There were already some implementations of `PeripheralState` that weren't `'static`, though, so I added an unsafe `PeripheralStateUnchecked` trait and forwarded the `unsafe` to the constructors of the implementors. --- embassy-extras/src/peripheral.rs | 50 ++++++++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 9 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 68972c543..e3e06d692 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -1,15 +1,38 @@ use core::cell::UnsafeCell; use core::marker::{PhantomData, PhantomPinned}; use core::pin::Pin; +use core::ptr; use embassy::interrupt::{Interrupt, InterruptExt}; -pub trait PeripheralState { +/// # Safety +/// When types implementing this trait are used with `Peripheral` or `PeripheralMutex`, +/// their lifetime must not end without first calling `Drop` on the `Peripheral` or `PeripheralMutex`. +pub unsafe trait PeripheralStateUnchecked { type Interrupt: Interrupt; fn on_interrupt(&mut self); } -pub struct PeripheralMutex { +// `PeripheralMutex` is safe because `Pin` guarantees that the memory it references will not be invalidated or reused +// without calling `Drop`. However, it provides no guarantees about references contained within the state still being valid, +// so this `'static` bound is necessary. +pub trait PeripheralState: 'static { + type Interrupt: Interrupt; + fn on_interrupt(&mut self); +} + +// SAFETY: `T` has to live for `'static` to implement `PeripheralState`, thus its lifetime cannot end. +unsafe impl PeripheralStateUnchecked for T +where + T: PeripheralState, +{ + type Interrupt = T::Interrupt; + fn on_interrupt(&mut self) { + self.on_interrupt() + } +} + +pub struct PeripheralMutex { state: UnsafeCell, irq_setup_done: bool, @@ -19,7 +42,7 @@ pub struct PeripheralMutex { _pinned: PhantomPinned, } -impl PeripheralMutex { +impl PeripheralMutex { pub fn new(state: S, irq: S::Interrupt) -> Self { Self { irq, @@ -39,11 +62,17 @@ impl PeripheralMutex { this.irq.disable(); this.irq.set_handler(|p| { - // Safety: it's OK to get a &mut to the state, since - // - We're in the IRQ, no one else can't preempt us - // - We can't have preempted a with() call because the irq is disabled during it. - let state = unsafe { &mut *(p as *mut S) }; - state.on_interrupt(); + critical_section::with(|_| { + if p.is_null() { + // The state was dropped, so we can't operate on it. + return; + } + // Safety: it's OK to get a &mut to the state, since + // - We're in a critical section, no one can preempt us (and call with()) + // - We can't have preempted a with() call because the irq is disabled during it. + let state = unsafe { &mut *(p as *mut S) }; + state.on_interrupt(); + }) }); this.irq .set_handler_context((&mut this.state) as *mut _ as *mut ()); @@ -67,9 +96,12 @@ impl PeripheralMutex { } } -impl Drop for PeripheralMutex { +impl Drop for PeripheralMutex { fn drop(&mut self) { self.irq.disable(); self.irq.remove_handler(); + // Set the context to null so that the interrupt will know we're dropped + // if we pre-empted it before it entered a critical section. + self.irq.set_handler_context(ptr::null_mut()); } } -- cgit From fc1ef4947d463495ccd3da1d7763dcbe95a2588f Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Mon, 5 Jul 2021 18:18:05 +1000 Subject: Fix stm32 ethernet --- embassy-extras/src/peripheral.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index e3e06d692..edc5fe955 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -6,8 +6,8 @@ use core::ptr; use embassy::interrupt::{Interrupt, InterruptExt}; /// # Safety -/// When types implementing this trait are used with `Peripheral` or `PeripheralMutex`, -/// their lifetime must not end without first calling `Drop` on the `Peripheral` or `PeripheralMutex`. +/// When types implementing this trait are used with `PeripheralMutex`, +/// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `PeripheralMutex`. pub unsafe trait PeripheralStateUnchecked { type Interrupt: Interrupt; fn on_interrupt(&mut self); -- cgit From 1b7ad7080e6a8c96f2622d75b99a4d3bdbc7c394 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Sat, 24 Jul 2021 12:53:57 +1000 Subject: Add `Send/Sync` bounds to `PeripheralState` --- embassy-extras/src/peripheral.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index edc5fe955..2358ab96f 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -5,18 +5,26 @@ use core::ptr; use embassy::interrupt::{Interrupt, InterruptExt}; +/// A version of `PeripheralState` without the `'static` bound, +/// for cases where the compiler can't statically make sure +/// that `on_interrupt` doesn't reference anything which might be invalidated. +/// /// # Safety /// When types implementing this trait are used with `PeripheralMutex`, /// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `PeripheralMutex`. -pub unsafe trait PeripheralStateUnchecked { +pub unsafe trait PeripheralStateUnchecked: Send { type Interrupt: Interrupt; fn on_interrupt(&mut self); } -// `PeripheralMutex` is safe because `Pin` guarantees that the memory it references will not be invalidated or reused -// without calling `Drop`. However, it provides no guarantees about references contained within the state still being valid, -// so this `'static` bound is necessary. -pub trait PeripheralState: 'static { +/// A type which can be used as state with `PeripheralMutex`. +/// +/// It needs to be `Send` because `&mut` references are sent back and forth between the 'thread' which owns the `PeripheralMutex` and the interrupt, +/// and `&mut T` is `Send` where `T: Send`. +/// +/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated, +/// it doesn't guarantee that the lifetime will last. +pub trait PeripheralState: Send + 'static { type Interrupt: Interrupt; fn on_interrupt(&mut self); } -- cgit From 079526559f44a0822574205e8798136f11f207d3 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Tue, 27 Jul 2021 17:28:52 +1000 Subject: Remove critical sections from `PeripheralMutex` interrupt handler by checking the interrupt's priority on startup. Since `PeripheralMutex` is the only way to safely maintain state across interrupts, and it no longer allows setting the interrupt's priority, the priority changing isn't a concern. This also prevents other causes of UB due to the interrupt being exposed during `with`, and allowing enabling the interrupt and setting its context to a bogus pointer. --- embassy-extras/src/peripheral.rs | 140 ++++++++++++++++++++++++++++++++++----- 1 file changed, 122 insertions(+), 18 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 2358ab96f..2402ba9eb 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -1,8 +1,9 @@ use core::cell::UnsafeCell; use core::marker::{PhantomData, PhantomPinned}; use core::pin::Pin; -use core::ptr; +use cortex_m::peripheral::scb::{Exception, SystemHandler, VectActive}; +use cortex_m::peripheral::{NVIC, SCB}; use embassy::interrupt::{Interrupt, InterruptExt}; /// A version of `PeripheralState` without the `'static` bound, @@ -50,8 +51,79 @@ pub struct PeripheralMutex { _pinned: PhantomPinned, } +fn exception_to_system_handler(exception: Exception) -> Option { + match exception { + Exception::NonMaskableInt | Exception::HardFault => None, + #[cfg(not(armv6m))] + Exception::MemoryManagement => Some(SystemHandler::MemoryManagement), + #[cfg(not(armv6m))] + Exception::BusFault => Some(SystemHandler::BusFault), + #[cfg(not(armv6m))] + Exception::UsageFault => Some(SystemHandler::UsageFault), + #[cfg(any(armv8m, target_arch = "x86_64"))] + Exception::SecureFault => Some(SystemHandler::SecureFault), + Exception::SVCall => Some(SystemHandler::SVCall), + #[cfg(not(armv6m))] + Exception::DebugMonitor => Some(SystemHandler::DebugMonitor), + Exception::PendSV => Some(SystemHandler::PendSV), + Exception::SysTick => Some(SystemHandler::SysTick), + } +} + +/// Whether `irq` can be preempted by the current interrupt. +pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool { + match SCB::vect_active() { + // Thread mode can't preempt each other + VectActive::ThreadMode => false, + VectActive::Exception(exception) => { + // `SystemHandler` is a subset of `Exception` for those with configurable priority. + // There's no built in way to convert between them, so `exception_to_system_handler` was necessary. + if let Some(system_handler) = exception_to_system_handler(exception) { + let current_prio = SCB::get_priority(system_handler); + let irq_prio = irq.get_priority().into(); + if current_prio < irq_prio { + true + } else if current_prio == irq_prio { + // When multiple interrupts have the same priority number, + // the pending interrupt with the lowest interrupt number takes precedence. + (exception.irqn() as i16) < irq.number() as i16 + } else { + false + } + } else { + // There's no safe way I know of to maintain `!Send` state across invocations of HardFault or NMI, so that should be fine. + false + } + } + VectActive::Interrupt { irqn } => { + #[derive(Clone, Copy)] + struct NrWrap(u16); + unsafe impl cortex_m::interrupt::InterruptNumber for NrWrap { + fn number(self) -> u16 { + self.0 + } + } + let current_prio = NVIC::get_priority(NrWrap(irqn.into())); + let irq_prio = irq.get_priority().into(); + if current_prio < irq_prio { + true + } else if current_prio == irq_prio { + // When multiple interrupts have the same priority number, + // the pending interrupt with the lowest interrupt number takes precedence. + (irqn as u16) < irq.number() + } else { + false + } + } + } +} + impl PeripheralMutex { pub fn new(state: S, irq: S::Interrupt) -> Self { + if can_be_preempted(&irq) { + panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps"); + } + Self { irq, irq_setup_done: false, @@ -70,17 +142,13 @@ impl PeripheralMutex { this.irq.disable(); this.irq.set_handler(|p| { - critical_section::with(|_| { - if p.is_null() { - // The state was dropped, so we can't operate on it. - return; - } - // Safety: it's OK to get a &mut to the state, since - // - We're in a critical section, no one can preempt us (and call with()) - // - We can't have preempted a with() call because the irq is disabled during it. - let state = unsafe { &mut *(p as *mut S) }; - state.on_interrupt(); - }) + // Safety: it's OK to get a &mut to the state, since + // - We checked that the thread owning the `PeripheralMutex` can't preempt us in `new`. + // Interrupts' priorities can only be changed with raw embassy `Interrupts`, + // which can't safely store a `PeripheralMutex` across invocations. + // - We can't have preempted a with() call because the irq is disabled during it. + let state = unsafe { &mut *(p as *mut S) }; + state.on_interrupt(); }); this.irq .set_handler_context((&mut this.state) as *mut _ as *mut ()); @@ -89,27 +157,63 @@ impl PeripheralMutex { this.irq_setup_done = true; } - pub fn with(self: Pin<&mut Self>, f: impl FnOnce(&mut S, &mut S::Interrupt) -> R) -> R { + pub fn with(self: Pin<&mut Self>, f: impl FnOnce(&mut S) -> R) -> R { let this = unsafe { self.get_unchecked_mut() }; + let was_enabled = this.irq.is_enabled(); this.irq.disable(); // Safety: it's OK to get a &mut to the state, since the irq is disabled. let state = unsafe { &mut *this.state.get() }; - let r = f(state, &mut this.irq); + let r = f(state); - this.irq.enable(); + if was_enabled { + this.irq.enable(); + } r } + + /// Enables the wrapped interrupt. + pub fn enable(&self) { + // This is fine to do before initialization, because we haven't set the handler yet. + self.irq.enable() + } + + /// Disables the wrapped interrupt. + pub fn disable(&self) { + self.irq.disable() + } + + /// Returns whether the wrapped interrupt is enabled. + pub fn is_enabled(&self) -> bool { + self.irq.is_enabled() + } + + /// Returns whether the wrapped interrupt is currently in a pending state. + pub fn is_pending(&self) -> bool { + self.irq.is_pending() + } + + /// Forces the wrapped interrupt into a pending state. + pub fn pend(&self) { + self.irq.pend() + } + + /// Forces the wrapped interrupt out of a pending state. + pub fn unpend(&self) { + self.irq.unpend() + } + + /// Gets the priority of the wrapped interrupt. + pub fn priority(&self) -> ::Priority { + self.irq.get_priority() + } } impl Drop for PeripheralMutex { fn drop(&mut self) { self.irq.disable(); self.irq.remove_handler(); - // Set the context to null so that the interrupt will know we're dropped - // if we pre-empted it before it entered a critical section. - self.irq.set_handler_context(ptr::null_mut()); } } -- cgit From 68c93256bcab8dfe0f65c694fa5fadb890fd3f00 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Wed, 28 Jul 2021 21:31:31 +1000 Subject: fix: interrupts with equal priority can't preempt each other --- embassy-extras/src/peripheral.rs | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 2402ba9eb..396ab5432 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -21,7 +21,7 @@ pub unsafe trait PeripheralStateUnchecked: Send { /// A type which can be used as state with `PeripheralMutex`. /// /// It needs to be `Send` because `&mut` references are sent back and forth between the 'thread' which owns the `PeripheralMutex` and the interrupt, -/// and `&mut T` is `Send` where `T: Send`. +/// and `&mut T` is only `Send` where `T: Send`. /// /// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated, /// it doesn't guarantee that the lifetime will last. @@ -73,23 +73,13 @@ fn exception_to_system_handler(exception: Exception) -> Option { /// Whether `irq` can be preempted by the current interrupt. pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool { match SCB::vect_active() { - // Thread mode can't preempt each other + // Thread mode can't preempt anything VectActive::ThreadMode => false, VectActive::Exception(exception) => { // `SystemHandler` is a subset of `Exception` for those with configurable priority. // There's no built in way to convert between them, so `exception_to_system_handler` was necessary. if let Some(system_handler) = exception_to_system_handler(exception) { - let current_prio = SCB::get_priority(system_handler); - let irq_prio = irq.get_priority().into(); - if current_prio < irq_prio { - true - } else if current_prio == irq_prio { - // When multiple interrupts have the same priority number, - // the pending interrupt with the lowest interrupt number takes precedence. - (exception.irqn() as i16) < irq.number() as i16 - } else { - false - } + SCB::get_priority(system_handler) < irq.get_priority().into() } else { // There's no safe way I know of to maintain `!Send` state across invocations of HardFault or NMI, so that should be fine. false @@ -103,17 +93,7 @@ pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool { self.0 } } - let current_prio = NVIC::get_priority(NrWrap(irqn.into())); - let irq_prio = irq.get_priority().into(); - if current_prio < irq_prio { - true - } else if current_prio == irq_prio { - // When multiple interrupts have the same priority number, - // the pending interrupt with the lowest interrupt number takes precedence. - (irqn as u16) < irq.number() - } else { - false - } + NVIC::get_priority(NrWrap(irqn.into())) < irq.get_priority().into() } } } -- cgit From 4d9514cbcb97343c3a75bfa565d753c44c2a0e27 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Wed, 28 Jul 2021 21:39:31 +1000 Subject: Don't allow disabling interrupts wrapped by `PeripheralMutex` --- embassy-extras/src/peripheral.rs | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 396ab5432..725a58a46 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -140,36 +140,17 @@ impl PeripheralMutex { pub fn with(self: Pin<&mut Self>, f: impl FnOnce(&mut S) -> R) -> R { let this = unsafe { self.get_unchecked_mut() }; - let was_enabled = this.irq.is_enabled(); this.irq.disable(); // Safety: it's OK to get a &mut to the state, since the irq is disabled. let state = unsafe { &mut *this.state.get() }; let r = f(state); - if was_enabled { - this.irq.enable(); - } + this.irq.enable(); r } - /// Enables the wrapped interrupt. - pub fn enable(&self) { - // This is fine to do before initialization, because we haven't set the handler yet. - self.irq.enable() - } - - /// Disables the wrapped interrupt. - pub fn disable(&self) { - self.irq.disable() - } - - /// Returns whether the wrapped interrupt is enabled. - pub fn is_enabled(&self) -> bool { - self.irq.is_enabled() - } - /// Returns whether the wrapped interrupt is currently in a pending state. pub fn is_pending(&self) -> bool { self.irq.is_pending() -- cgit From d5ba35424d7eef2cc0c501758d214ce3a6febfc1 Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 29 Jul 2021 15:11:26 +1000 Subject: Replace `PeripheralStateUnchecked` with `register_interrupt_unchecked` --- embassy-extras/src/peripheral.rs | 66 ++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 30 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 725a58a46..1868edd7d 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -6,42 +6,20 @@ use cortex_m::peripheral::scb::{Exception, SystemHandler, VectActive}; use cortex_m::peripheral::{NVIC, SCB}; use embassy::interrupt::{Interrupt, InterruptExt}; -/// A version of `PeripheralState` without the `'static` bound, -/// for cases where the compiler can't statically make sure -/// that `on_interrupt` doesn't reference anything which might be invalidated. -/// -/// # Safety -/// When types implementing this trait are used with `PeripheralMutex`, -/// no fields referenced by `on_interrupt`'s lifetimes must end without first calling `Drop` on the `PeripheralMutex`. -pub unsafe trait PeripheralStateUnchecked: Send { - type Interrupt: Interrupt; - fn on_interrupt(&mut self); -} - /// A type which can be used as state with `PeripheralMutex`. /// /// It needs to be `Send` because `&mut` references are sent back and forth between the 'thread' which owns the `PeripheralMutex` and the interrupt, /// and `&mut T` is only `Send` where `T: Send`. /// -/// It also requires `'static`, because although `Pin` guarantees that the memory of the state won't be invalidated, +/// It also requires `'static` to be used safely with `PeripheralMutex::register_interrupt`, +/// because although `Pin` guarantees that the memory of the state won't be invalidated, /// it doesn't guarantee that the lifetime will last. -pub trait PeripheralState: Send + 'static { +pub trait PeripheralState: Send { type Interrupt: Interrupt; fn on_interrupt(&mut self); } -// SAFETY: `T` has to live for `'static` to implement `PeripheralState`, thus its lifetime cannot end. -unsafe impl PeripheralStateUnchecked for T -where - T: PeripheralState, -{ - type Interrupt = T::Interrupt; - fn on_interrupt(&mut self) { - self.on_interrupt() - } -} - -pub struct PeripheralMutex { +pub struct PeripheralMutex { state: UnsafeCell, irq_setup_done: bool, @@ -98,7 +76,25 @@ pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool { } } -impl PeripheralMutex { +impl PeripheralMutex { + /// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it. + /// + /// This requires this `PeripheralMutex`'s `PeripheralState` to live for `'static`, + /// because `Pin` only guarantees that it's memory won't be repurposed, + /// not that it's lifetime will last. + /// + /// To use non-`'static` `PeripheralState`, use the unsafe `register_interrupt_unchecked`. + /// + /// Note: `'static` doesn't mean it _has_ to live for the entire program, like an `&'static T`; + /// it just means it _can_ live for the entire program - for example, `u8` lives for `'static`. + pub fn register_interrupt(self: Pin<&mut Self>) { + // SAFETY: `S: 'static`, so there's no way it's lifetime can expire. + unsafe { self.register_interrupt_unchecked() } + } +} + +impl PeripheralMutex { + /// Create a new `PeripheralMutex` wrapping `irq`, with the initial state `state`. pub fn new(state: S, irq: S::Interrupt) -> Self { if can_be_preempted(&irq) { panic!("`PeripheralMutex` cannot be created in an interrupt with higher priority than the interrupt it wraps"); @@ -114,8 +110,18 @@ impl PeripheralMutex { } } - pub fn register_interrupt(self: Pin<&mut Self>) { - let this = unsafe { self.get_unchecked_mut() }; + /// Registers `on_interrupt` as the wrapped interrupt's interrupt handler and enables it. + /// + /// # Safety + /// The lifetime of any data in `PeripheralState` that is accessed by the interrupt handler + /// must not end without `Drop` being called on this `PeripheralMutex`. + /// + /// This can be accomplished by either not accessing any data with a lifetime in `on_interrupt`, + /// or making sure that nothing like `mem::forget` is used on the `PeripheralMutex`. + + // TODO: this name isn't the best. + pub unsafe fn register_interrupt_unchecked(self: Pin<&mut Self>) { + let this = self.get_unchecked_mut(); if this.irq_setup_done { return; } @@ -172,7 +178,7 @@ impl PeripheralMutex { } } -impl Drop for PeripheralMutex { +impl Drop for PeripheralMutex { fn drop(&mut self) { self.irq.disable(); self.irq.remove_handler(); -- cgit From cd1a3fcff34943117f446e1afeb9e6d531ee577b Mon Sep 17 00:00:00 2001 From: Liam Murphy Date: Thu, 29 Jul 2021 15:19:57 +1000 Subject: Don't bother supporting creating a `PeripheralMutex` in an exception handler --- embassy-extras/src/peripheral.rs | 36 +++++------------------------------- 1 file changed, 5 insertions(+), 31 deletions(-) (limited to 'embassy-extras/src/peripheral.rs') diff --git a/embassy-extras/src/peripheral.rs b/embassy-extras/src/peripheral.rs index 1868edd7d..92512a0f6 100644 --- a/embassy-extras/src/peripheral.rs +++ b/embassy-extras/src/peripheral.rs @@ -2,7 +2,7 @@ use core::cell::UnsafeCell; use core::marker::{PhantomData, PhantomPinned}; use core::pin::Pin; -use cortex_m::peripheral::scb::{Exception, SystemHandler, VectActive}; +use cortex_m::peripheral::scb::VectActive; use cortex_m::peripheral::{NVIC, SCB}; use embassy::interrupt::{Interrupt, InterruptExt}; @@ -29,40 +29,14 @@ pub struct PeripheralMutex { _pinned: PhantomPinned, } -fn exception_to_system_handler(exception: Exception) -> Option { - match exception { - Exception::NonMaskableInt | Exception::HardFault => None, - #[cfg(not(armv6m))] - Exception::MemoryManagement => Some(SystemHandler::MemoryManagement), - #[cfg(not(armv6m))] - Exception::BusFault => Some(SystemHandler::BusFault), - #[cfg(not(armv6m))] - Exception::UsageFault => Some(SystemHandler::UsageFault), - #[cfg(any(armv8m, target_arch = "x86_64"))] - Exception::SecureFault => Some(SystemHandler::SecureFault), - Exception::SVCall => Some(SystemHandler::SVCall), - #[cfg(not(armv6m))] - Exception::DebugMonitor => Some(SystemHandler::DebugMonitor), - Exception::PendSV => Some(SystemHandler::PendSV), - Exception::SysTick => Some(SystemHandler::SysTick), - } -} - /// Whether `irq` can be preempted by the current interrupt. pub(crate) fn can_be_preempted(irq: &impl Interrupt) -> bool { match SCB::vect_active() { - // Thread mode can't preempt anything + // Thread mode can't preempt anything. VectActive::ThreadMode => false, - VectActive::Exception(exception) => { - // `SystemHandler` is a subset of `Exception` for those with configurable priority. - // There's no built in way to convert between them, so `exception_to_system_handler` was necessary. - if let Some(system_handler) = exception_to_system_handler(exception) { - SCB::get_priority(system_handler) < irq.get_priority().into() - } else { - // There's no safe way I know of to maintain `!Send` state across invocations of HardFault or NMI, so that should be fine. - false - } - } + // Exceptions don't always preempt interrupts, + // but there isn't much of a good reason to be keeping a `PeripheralMutex` in an exception anyway. + VectActive::Exception(_) => true, VectActive::Interrupt { irqn } => { #[derive(Clone, Copy)] struct NrWrap(u16); -- cgit