PLIC refactors and performance improvements

platforms/allwinner-d1/d1-core/src/plic.rs

@@ -54,16 +54,14 @@ impl Plic {
     ///
     /// May effect normal interrupt processing
     pub unsafe fn unmask(&self, interrupt: Interrupt) {
-        let nr = interrupt.into_bits() as usize;
-        let (reg_offset, irq_en) = (nr / 32, 1 << (nr % 32));
-        self.plic.mie[reg_offset].modify(|r, w| w.bits(r.bits() | irq_en));
+        let (mie, irq_en) = self.index_mie(interrupt);
+        mie.modify(|r, w| w.bits(r.bits() | irq_en));
     }
 
     /// Disable an interrupt
     pub fn mask(&self, interrupt: Interrupt) {
-        let nr = interrupt.into_bits() as usize;
-        let (reg_offset, irq_en) = (nr / 32, 1 << (nr % 32));
-        self.plic.mie[reg_offset].modify(|r, w| unsafe { w.bits(r.bits() & !irq_en) });
+        let (mie, irq_en) = self.index_mie(interrupt);
+        mie.modify(|r, w| unsafe { w.bits(r.bits() | irq_en) });
     }
 
     /// Globally set priority for one interrupt
@@ -100,14 +98,23 @@ impl Plic {
         let claim_u16 = claim as u16;
 
         // Is this a known interrupt?
-        let handler = INTERRUPT_ARRAY.iter().find(|i| i.id == claim_u16);
-        if let Some(Vectored { id: _id, handler }) = handler {
-            let ptr = handler.load(Ordering::SeqCst); // todo: ordering
-            if !ptr.is_null() {
-                let hdlr: fn() = unsafe { core::mem::transmute(ptr) };
-                (hdlr)();
-            } // TODO: panic on else?
-        } // TODO: panic on else?
+        // N.B. that the index is always in bounds, because `claim()` has
+        // panicked already if the interrupt is not a valid interrupt number.
+        // Theoretically, we could probably optimize this to use
+        // `get_unchecked`, if we cared to.
+        let &Vectored { id, ref handler } = &INTERRUPT_ARRAY[claim_u16 as usize];
+        debug_assert_eq!(
+            id,
+            Some(claim),
+            "FLAGRANT ERROR: interrupt ID {id:?} does not match index \
+                ({claim_u16}); perhaps the interrupt dispatch table has \
+                somehow been corrupted?"
+        );
+        let ptr = handler.load(Ordering::SeqCst); // todo: ordering
+        if !ptr.is_null() {
+            let hdlr: fn() = unsafe { core::mem::transmute(ptr) };
+            (hdlr)();
+        } // otherwise, the ISR hasn't been registered yet; just do nothing.
 
         // Release claim
         self.complete(claim);
@@ -119,11 +126,20 @@ impl Plic {
             .write(|w| w.mclaim().variant(interrupt.into_bits() as u16));
     }
 
+    #[track_caller]
     pub unsafe fn register(&self, interrupt: Interrupt, new_hdl: fn()) {
-        let v = INTERRUPT_ARRAY.iter().find(|v| v.id == interrupt as u16);
-        if let Some(Vectored { id: _id, handler }) = v {
-            handler.store(new_hdl as *mut fn() as *mut (), Ordering::Release);
-        }
+        let idx = interrupt as usize;
+        let Some(&Vectored { id, ref handler }) = INTERRUPT_ARRAY.get(idx) else {
+            panic!("interrupt not found in dispatch table: {interrupt:?} (index {idx})")
+        };
+        assert_eq!(
+            Some(interrupt),
+            id,
+            "FLAGRANT ERROR: interrupt ID for {interrupt:?} does not \
+            match index {idx}; perhaps the interrupt dispatch table has \
+            somehow been corrupted?"
+        );
+        handler.store(new_hdl as *mut fn() as *mut (), Ordering::Release);
     }
 
     pub unsafe fn activate(&self, interrupt: Interrupt, prio: Priority) -> Result<(), MaskError> {
@@ -140,16 +156,25 @@ impl Plic {
     }
 
     fn can_mask(&self, interrupt: Interrupt) -> Result<(), MaskError> {
-        let v = INTERRUPT_ARRAY
-            .iter()
-            .find(|v| v.id == interrupt as u16)
+        let &Vectored { id, ref handler } = INTERRUPT_ARRAY
+            .get(interrupt as usize)
             .ok_or(MaskError::NotFound(interrupt))?;
 
-        if v.handler.load(Ordering::SeqCst).is_null() {
-            Err(MaskError::NoHandler(interrupt))
-        } else {
-            Ok(())
+        if id != Some(interrupt) {
+            return Err(MaskError::NotFound(interrupt));
         }
+
+        if handler.load(Ordering::SeqCst).is_null() {
+            return Err(MaskError::NoHandler(interrupt));
+        }
+
+        Ok(())
+    }
+
+    #[inline(always)]
+    fn index_mie(&self, interrupt: Interrupt) -> (&plic::MIE, u32) {
+        let nr = interrupt.into_bits() as usize;
+        (&self.plic.mie[nr / 32], 1 << (nr % 32))
     }
 }
 
@@ -217,20 +242,23 @@ impl TryFromBits for Priority {
 }
 
 struct Vectored {
-    id: u16,
+    id: Option<Interrupt>,
     handler: AtomicPtr<()>,
 }
 
 impl Vectored {
-    const fn new(id: u16) -> Self {
+    const fn new(interrupt: Interrupt) -> Self {
         Self {
-            id,
+            id: Some(interrupt),
             handler: AtomicPtr::new(null_mut()),
         }
     }
 
-    const fn from_interrupt(i: Interrupt) -> Self {
-        Self::new(i as u16)
+    const fn none() -> Self {
+        Self {
+            id: None,
+            handler: AtomicPtr::new(no_such_interrupt as *mut fn() as *mut ()),
+        }
     }
 }
 
@@ -293,21 +321,44 @@ const INTERRUPT_LIST: &[Interrupt] = &[
     Interrupt::IR_RX,
 ];
 
-const fn lister() -> [Vectored; INTERRUPT_LIST.len()] {
-    // This constant is used as an initializer. The fact that each time it's
-    // used, a new instance of the interior mutable value is created is the
-    // *correct* behavior here. I hate this clippy lint so goddamn much...
+const N_INTERRUPTS: usize = {
+    let mut max = 0;
+    // INTERRUPT_LIST is sorted, but --- because we're doing this in a const fn
+    // --- we may as well just find the actual max vector, instead of taking the
+    // vector of the last element in the array; this will *always* be correct,
+    // even if the interrupt list is ever out of order.
+    //
+    // Unfortunately, as you probably already know, const fn. So, we do this
+    // with a goofy while loop, instead of `iter().max()` --- you can pretend it
+    // says that, if you like.
+    let mut i = 0;
+    while i < INTERRUPT_LIST.len() {
+        let vector = INTERRUPT_LIST[i] as usize;
+        if vector > max {
+            max = vector;
+        }
+        i += 1;
+    }
+    max + 1
+};
+
+static INTERRUPT_ARRAY: [Vectored; N_INTERRUPTS] = {
+    // This is a static initializer, ignore the stupid goddamn clippy lint.
     #[allow(clippy::declare_interior_mutable_const)]
-    const ONE: Vectored = Vectored::new(0);
+    const TRAP: Vectored = Vectored::none();
+    let mut array = [TRAP; N_INTERRUPTS];
 
-    let mut arr = [ONE; INTERRUPT_LIST.len()];
+    // Populate the real interrupt vectors.
     let mut i = 0;
     while i < INTERRUPT_LIST.len() {
-        // Just take the ID,
-        arr[i] = Vectored::from_interrupt(INTERRUPT_LIST[i]);
+        let interrupt = INTERRUPT_LIST[i];
+        let vector = interrupt as usize;
+        array[vector] = Vectored::new(interrupt);
         i += 1;
     }
-    arr
-}
+    array
+};
 
-static INTERRUPT_ARRAY: [Vectored; INTERRUPT_LIST.len()] = lister();
+fn no_such_interrupt() {
+    panic!("no such interrupt!");
+}