Software interrupts runtime binding (#1398)

* Split software interrupts

* Make swint0 unavailable on a multi_core target when using thread-executor

* Clarify why embassy_multiprio example needs two executors

* Make interrupt-executor require a SoftwareInterrupt<n>

* Improve code

* CHANGELOG.md

* Don't use `#[interrupt]` in thread-executor

* More docs

* Typo fixed

esp-hal/CHANGELOG.md

@@ -55,6 +55,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - SYSTIMER and TIMG instances can now be created in async or blocking mode (#1348)
 - Runtime ISR binding for TWAI (#1384)
 - Runtime ISR binding for assist_debug (#1395)
+- Runtime ISR binding for software interrupts, software interrupts are split now, interrupt-executor takes the software interrupt to use, interrupt-executor is easier to use (#1398)
 
 ### Removed
 

esp-hal/src/embassy/executor/interrupt.rs

@@ -1,128 +1,96 @@
 //! Interrupt-mode executor.
-use core::{cell::UnsafeCell, marker::PhantomData, mem::MaybeUninit};
+use core::{cell::UnsafeCell, mem::MaybeUninit};
 
 use embassy_executor::{raw, SendSpawner};
-#[cfg(any(esp32c6, esp32h2))]
-use peripherals::INTPRI as SystemPeripheral;
-#[cfg(not(any(esp32c6, esp32h2)))]
-use peripherals::SYSTEM as SystemPeripheral;
 use portable_atomic::{AtomicUsize, Ordering};
 
-use crate::{get_core, interrupt, peripherals};
+use crate::{
+    get_core,
+    interrupt::{self, InterruptHandler},
+    system::SoftwareInterrupt,
+};
 
-static FROM_CPU_IRQ_USED: AtomicUsize = AtomicUsize::new(0);
-
-pub trait SwPendableInterrupt {
-    fn enable(priority: interrupt::Priority);
-    fn number() -> usize;
-    fn pend();
-    fn clear();
-}
-
-macro_rules! from_cpu {
-    ($irq:literal) => {
-        paste::paste! {
-            pub struct [<FromCpu $irq>];
-
-            impl [<FromCpu $irq>] {
-                fn set_bit(value: bool) {
-                    let system = unsafe { &*SystemPeripheral::PTR };
-
-                    system
-                        .[<cpu_intr_from_cpu_ $irq>]()
-                        .write(|w| w.[<cpu_intr_from_cpu_ $irq>]().bit(value));
-                }
-            }
-
-            impl SwPendableInterrupt for [<FromCpu $irq>] {
-                fn enable(priority: interrupt::Priority) {
-                    let mask = 1 << $irq;
-                    // We don't allow using the same interrupt for multiple executors.
-                    if FROM_CPU_IRQ_USED.fetch_or(mask, Ordering::SeqCst) & mask != 0 {
-                        panic!("FROM_CPU_{} is already used by a different executor.", $irq);
-                    }
-
-                    unwrap!(interrupt::enable(peripherals::Interrupt::[<FROM_CPU_INTR $irq>], priority));
-                }
-
-                fn number() -> usize {
-                    $irq
-                }
-
-                fn pend() {
-                    Self::set_bit(true);
-                }
-
-                fn clear() {
-                    Self::set_bit(false);
-                }
-            }
-        }
-    };
-}
-
-// from_cpu!(0); // reserve 0 for thread mode & multi-core
-from_cpu!(1);
-from_cpu!(2);
-from_cpu!(3);
+static mut EXECUTORS: [CallbackContext; 4] = [
+    CallbackContext::new(),
+    CallbackContext::new(),
+    CallbackContext::new(),
+    CallbackContext::new(),
+];
 
 /// Interrupt mode executor.
 ///
 /// This executor runs tasks in interrupt mode. The interrupt handler is set up
 /// to poll tasks, and when a task is woken the interrupt is pended from
 /// software.
-///
-/// # Interrupt requirements
-///
-/// You must write the interrupt handler yourself, and make it call
-/// [`Self::on_interrupt()`]
-///
-/// ```rust,ignore
-/// #[interrupt]
-/// fn FROM_CPU_INTR1() {
-///     unsafe { INT_EXECUTOR.on_interrupt() }
-/// }
-/// ```
-pub struct InterruptExecutor<SWI>
-where
-    SWI: SwPendableInterrupt,
-{
+pub struct InterruptExecutor<const SWI: u8> {
     core: AtomicUsize,
     executor: UnsafeCell<MaybeUninit<raw::Executor>>,
-    _interrupt: PhantomData<SWI>,
+    interrupt: SoftwareInterrupt<SWI>,
+}
+
+unsafe impl<const SWI: u8> Send for InterruptExecutor<SWI> {}
+unsafe impl<const SWI: u8> Sync for InterruptExecutor<SWI> {}
+
+struct CallbackContext {
+    raw_executor: UnsafeCell<*mut raw::Executor>,
 }
 
-unsafe impl<SWI: SwPendableInterrupt> Send for InterruptExecutor<SWI> {}
-unsafe impl<SWI: SwPendableInterrupt> Sync for InterruptExecutor<SWI> {}
+impl CallbackContext {
+    const fn new() -> Self {
+        Self {
+            raw_executor: UnsafeCell::new(core::ptr::null_mut()),
+        }
+    }
+
+    fn get(&self) -> *mut raw::Executor {
+        unsafe { (*self.raw_executor.get()) as *mut raw::Executor }
+    }
+
+    fn set(&self, executor: *mut raw::Executor) {
+        unsafe {
+            self.raw_executor.get().write(executor);
+        }
+    }
+}
+
+fn handle_interrupt<const NUM: u8>() {
+    let mut swi = unsafe { SoftwareInterrupt::<NUM>::steal() };
+    swi.reset();
 
-impl<SWI> InterruptExecutor<SWI>
-where
-    SWI: SwPendableInterrupt,
-{
+    unsafe {
+        let executor = EXECUTORS[NUM as usize].get().as_mut().unwrap();
+        executor.poll();
+    }
+}
+
+extern "C" fn swi_handler0() {
+    handle_interrupt::<0>();
+}
+
+extern "C" fn swi_handler1() {
+    handle_interrupt::<1>();
+}
+
+extern "C" fn swi_handler2() {
+    handle_interrupt::<2>();
+}
+
+extern "C" fn swi_handler3() {
+    handle_interrupt::<3>();
+}
+
+impl<const SWI: u8> InterruptExecutor<SWI> {
     /// Create a new `InterruptExecutor`.
+    /// This takes the software interrupt to be used internally.
     #[inline]
-    pub const fn new() -> Self {
+    pub const fn new(interrupt: SoftwareInterrupt<SWI>) -> Self {
         Self {
             core: AtomicUsize::new(usize::MAX),
             executor: UnsafeCell::new(MaybeUninit::uninit()),
-            _interrupt: PhantomData,
+            interrupt,
         }
     }
 
-    /// Executor interrupt callback.
-    ///
-    /// # Safety
-    ///
-    /// You MUST call this from the interrupt handler, and from nowhere else.
-    // TODO: it would be pretty sweet if we could register our own interrupt handler
-    // when vectoring is enabled. The user shouldn't need to provide the handler for
-    // us.
-    pub unsafe fn on_interrupt(&'static self) {
-        SWI::clear();
-        let executor = unsafe { (*self.executor.get()).assume_init_ref() };
-        executor.poll();
-    }
-
     /// Start the executor at the given priority level.
     ///
     /// This initializes the executor, enables the interrupt, and returns.
@@ -135,18 +103,7 @@ where
     ///
     /// To obtain a [`Spawner`] for this executor, use
     /// [`Spawner::for_current_executor()`] from a task running in it.
-    ///
-    /// # Interrupt requirements
-    ///
-    /// You must write the interrupt handler yourself, and make it call
-    /// [`Self::on_interrupt()`]
-    ///
-    /// This method already enables (unmasks) the interrupt, you must NOT do it
-    /// yourself.
-    ///
-    /// [`Spawner`]: embassy_executor::Spawner
-    /// [`Spawner::for_current_executor()`]: embassy_executor::Spawner::for_current_executor()
-    pub fn start(&'static self, priority: interrupt::Priority) -> SendSpawner {
+    pub fn start(&'static mut self, priority: interrupt::Priority) -> SendSpawner {
         if self
             .core
             .compare_exchange(
@@ -163,10 +120,21 @@ where
         unsafe {
             (*self.executor.get())
                 .as_mut_ptr()
-                .write(raw::Executor::new(SWI::number() as *mut ()))
+                .write(raw::Executor::new(SWI as *mut ()));
+
+            EXECUTORS[SWI as usize].set((*self.executor.get()).as_mut_ptr());
         }
 
-        SWI::enable(priority);
+        let swi_handler = match SWI {
+            0 => swi_handler0,
+            1 => swi_handler1,
+            2 => swi_handler2,
+            3 => swi_handler3,
+            _ => unreachable!(),
+        };
+
+        self.interrupt
+            .set_interrupt_handler(InterruptHandler::new(swi_handler, priority));
 
         let executor = unsafe { (*self.executor.get()).assume_init_ref() };
         executor.spawner().make_send()

esp-hal/src/embassy/executor/mod.rs

@@ -16,16 +16,22 @@ pub use interrupt::*;
 ))]
 #[export_name = "__pender"]
 fn __pender(context: *mut ()) {
+    #[cfg(feature = "embassy-executor-interrupt")]
+    use crate::system::SoftwareInterrupt;
+
     let context = (context as usize).to_le_bytes();
 
     cfg_if::cfg_if! {
         if #[cfg(feature = "embassy-executor-interrupt")] {
             match context[0] {
                 #[cfg(feature = "embassy-executor-thread")]
                 0 => thread::pend_thread_mode(context[1] as usize),
-                1 => FromCpu1::pend(),
-                2 => FromCpu2::pend(),
-                3 => FromCpu3::pend(),
+
+                #[cfg(not(feature = "embassy-executor-thread"))]
+                0 => unsafe { SoftwareInterrupt::<0>::steal().raise() },
+                1 => unsafe { SoftwareInterrupt::<1>::steal().raise() },
+                2 => unsafe { SoftwareInterrupt::<2>::steal().raise() },
+                3 => unsafe { SoftwareInterrupt::<3>::steal().raise() },
                 _ => {}
             }
         } else {

esp-hal/src/embassy/executor/thread.rs

@@ -3,13 +3,12 @@ use core::marker::PhantomData;
 
 use embassy_executor::{raw, Spawner};
 use portable_atomic::{AtomicBool, Ordering};
+#[cfg(multi_core)]
+use procmacros::handler;
 
-use crate::{get_core, prelude::interrupt};
+use crate::get_core;
 #[cfg(multi_core)]
-use crate::{
-    interrupt,
-    peripherals::{self, SYSTEM},
-};
+use crate::peripherals::SYSTEM;
 
 /// global atomic used to keep track of whether there is work to do since sev()
 /// is not available on either Xtensa or RISC-V
@@ -18,19 +17,17 @@ static SIGNAL_WORK_THREAD_MODE: [AtomicBool; 1] = [AtomicBool::new(false)];
 #[cfg(multi_core)]
 static SIGNAL_WORK_THREAD_MODE: [AtomicBool; 2] = [AtomicBool::new(false), AtomicBool::new(false)];
 
-#[interrupt]
-fn FROM_CPU_INTR0() {
-    #[cfg(multi_core)]
-    {
-        // This interrupt is fired when the thread-mode executor's core needs to be
-        // woken. It doesn't matter which core handles this interrupt first, the
-        // point is just to wake up the core that is currently executing
-        // `waiti`.
-        let system = unsafe { &*SYSTEM::PTR };
-        system
-            .cpu_intr_from_cpu_0()
-            .write(|w| w.cpu_intr_from_cpu_0().bit(false));
-    }
+#[cfg(multi_core)]
+#[handler]
+fn software0_interrupt() {
+    // This interrupt is fired when the thread-mode executor's core needs to be
+    // woken. It doesn't matter which core handles this interrupt first, the
+    // point is just to wake up the core that is currently executing
+    // `waiti`.
+    let system = unsafe { &*SYSTEM::PTR };
+    system
+        .cpu_intr_from_cpu_0()
+        .write(|w| w.cpu_intr_from_cpu_0().bit(false));
 }
 
 pub(crate) fn pend_thread_mode(core: usize) {
@@ -60,12 +57,15 @@ pub struct Executor {
 
 impl Executor {
     /// Create a new Executor.
+    ///
+    /// On multi_core systems this will use software-interrupt 0 which isn't
+    /// available for anything else.
     pub fn new() -> Self {
         #[cfg(multi_core)]
-        unwrap!(interrupt::enable(
-            peripherals::Interrupt::FROM_CPU_INTR0,
-            interrupt::Priority::Priority1,
-        ));
+        unsafe {
+            crate::system::SoftwareInterrupt::<0>::steal()
+                .set_interrupt_handler(software0_interrupt)
+        }
 
         Self {
             inner: raw::Executor::new(usize::from_le_bytes([0, get_core() as u8, 0, 0]) as *mut ()),
@@ -107,6 +107,7 @@ impl Executor {
         }
     }
 
+    #[doc(hidden)]
     #[cfg(xtensa)]
     pub fn wait_impl(cpu: usize) {
         // Manual critical section implementation that only masks interrupts handlers.
@@ -138,6 +139,7 @@ impl Executor {
         }
     }
 
+    #[doc(hidden)]
     #[cfg(riscv)]
     pub fn wait_impl(cpu: usize) {
         // we do not care about race conditions between the load and store operations,

esp-hal/src/peripheral.rs

@@ -46,7 +46,7 @@ use core::{
 /// dedicated struct is memory efficiency:
 ///
 /// Peripheral singletons are typically either zero-sized (for concrete
-/// peripehrals like `PA9` or `Spi4`) or very small (for example `AnyPin` which
+/// peripherals like `PA9` or `Spi4`) or very small (for example `AnyPin` which
 /// is 1 byte). However `&mut T` is always 4 bytes for 32-bit targets, even if T
 /// is zero-sized. PeripheralRef stores a copy of `T` instead, so it's the same
 /// size.