Merge branch 'pci_interrupts_rework3' into 'master'

jackpot51 · jackpot51 · commit 157750676544 · 2025-08-31T09:49:08.000-06:00
Various interrupt handling improvements

See merge request redox-os/drivers!282
diff --git a/storage/nvmed/src/main.rs b/storage/nvmed/src/main.rs
@@ -65,18 +65,11 @@ fn get_int_method(
         Ok((interrupt_method, interrupt_sources))
     } else if has_msi {
         // Message signaled interrupts.
-        let msi_info = match pcid_handle.feature_info(PciFeature::Msi) {
-            PciFeatureInfo::Msi(msi) => msi,
-            _ => unreachable!(),
-        };
 
         let msi_vector_number = 0;
         let irq_handle = irq_helpers::allocate_first_msi_interrupt_on_bsp(pcid_handle);
 
-        let interrupt_method = InterruptMethod::Msi {
-            msi_info,
-            log2_multiple_message_enabled: 0,
-        };
+        let interrupt_method = InterruptMethod::Msi;
         let interrupt_sources =
             InterruptSources::Msi(std::iter::once((msi_vector_number, irq_handle)).collect());
 
diff --git a/storage/nvmed/src/nvme/mod.rs b/storage/nvmed/src/nvme/mod.rs
@@ -21,7 +21,7 @@ pub mod queues;
 use self::executor::NvmeExecutor;
 pub use self::queues::{NvmeCmd, NvmeCmdQueue, NvmeComp, NvmeCompQueue};
 
-use pcid_interface::msi::{MappedMsixRegs, MsiInfo};
+use pcid_interface::msi::MappedMsixRegs;
 use pcid_interface::PciFunctionHandle;
 
 /// Used in conjunction with `InterruptMethod`, primarily by the CQ executor.
@@ -31,83 +31,17 @@ pub enum InterruptSources {
     Msi(BTreeMap<u8, File>),
     Intx(File),
 }
-impl InterruptSources {
-    pub fn iter_mut(&mut self) -> impl Iterator<Item = (u16, &mut File)> {
-        use std::collections::btree_map::IterMut as BTreeIterMut;
-        use std::iter::Once;
-
-        enum IterMut<'a> {
-            Msi(BTreeIterMut<'a, u8, File>),
-            MsiX(BTreeIterMut<'a, u16, File>),
-            Intx(Once<&'a mut File>),
-        }
-        impl<'a> Iterator for IterMut<'a> {
-            type Item = (u16, &'a mut File);
-
-            fn next(&mut self) -> Option<Self::Item> {
-                match self {
-                    &mut Self::Msi(ref mut iter) => iter
-                        .next()
-                        .map(|(&vector, handle)| (u16::from(vector), handle)),
-                    &mut Self::MsiX(ref mut iter) => {
-                        iter.next().map(|(&vector, handle)| (vector, handle))
-                    }
-                    &mut Self::Intx(ref mut iter) => iter.next().map(|handle| (0u16, handle)),
-                }
-            }
-            fn size_hint(&self) -> (usize, Option<usize>) {
-                match self {
-                    &Self::Msi(ref iter) => iter.size_hint(),
-                    &Self::MsiX(ref iter) => iter.size_hint(),
-                    &Self::Intx(ref iter) => iter.size_hint(),
-                }
-            }
-        }
-
-        match self {
-            &mut Self::MsiX(ref mut map) => IterMut::MsiX(map.iter_mut()),
-            &mut Self::Msi(ref mut map) => IterMut::Msi(map.iter_mut()),
-            &mut Self::Intx(ref mut single) => IterMut::Intx(std::iter::once(single)),
-        }
-    }
-}
 
 /// The way interrupts are sent. Unlike other PCI-based interfaces, like XHCI, it doesn't seem like
 /// NVME supports operating with interrupts completely disabled.
 pub enum InterruptMethod {
     /// Traditional level-triggered, INTx# interrupt pins.
     Intx,
     /// Message signaled interrupts
-    Msi {
-        msi_info: MsiInfo,
-        log2_multiple_message_enabled: u8,
-    },
+    Msi,
     /// Extended message signaled interrupts
     MsiX(MappedMsixRegs),
 }
-impl InterruptMethod {
-    fn is_intx(&self) -> bool {
-        if let Self::Intx = self {
-            true
-        } else {
-            false
-        }
-    }
-    fn is_msi(&self) -> bool {
-        if let Self::Msi { .. } = self {
-            true
-        } else {
-            false
-        }
-    }
-    fn is_msix(&self) -> bool {
-        if let Self::MsiX(_) = self {
-            true
-        } else {
-            false
-        }
-    }
-}
 
 #[repr(C, packed)]
 pub struct NvmeRegs {
@@ -373,18 +307,11 @@ impl Nvme {
                     self.regs.write().intmc.write(0x0000_0001);
                 }
             }
-            &mut InterruptMethod::Msi {
-                msi_info: _,
-                log2_multiple_message_enabled: log2_enabled_messages,
-            } => {
+            &mut InterruptMethod::Msi => {
                 let mut to_mask = 0x0000_0000;
                 let mut to_clear = 0x0000_0000;
 
                 for (vector, mask) in vectors {
-                    assert!(
-                        vector < (1 << log2_enabled_messages),
-                        "nvmed: internal error: MSI vector out of range"
-                    );
                     let vector = vector as u8;
 
                     if mask {
diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs
@@ -18,7 +18,6 @@ pub struct Device {
 unsafe impl Send for Device {}
 unsafe impl Sync for Device {}
 
-
 pub const MSIX_PRIMARY_VECTOR: u16 = 0;
 
 /// VirtIO Device Probe
diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs
@@ -26,7 +26,7 @@
 extern crate bitflags;
 
 use std::fs::File;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
 
 use pcid_interface::irq_helpers::read_bsp_apic_id;
 #[cfg(target_arch = "x86_64")]
@@ -78,10 +78,7 @@ fn get_int_method(pcid_handle: &mut PciFunctionHandle) -> (Option<File>, Interru
             table_entry_pointer.write_addr_and_data(msg_addr_and_data);
             table_entry_pointer.unmask();
 
-            (
-                Some(interrupt_handle),
-                InterruptMethod::MsiX(Mutex::new(info)),
-            )
+            (Some(interrupt_handle), InterruptMethod::Msi)
         };
 
         pcid_handle.enable_feature(PciFeature::MsiX);
diff --git a/xhcid/src/xhci/irq_reactor.rs b/xhcid/src/xhci/irq_reactor.rs
@@ -121,7 +121,7 @@ impl IrqReactor {
     fn pause(&self) {
         std::thread::sleep(std::time::Duration::from_millis(2));
     }
-    fn run_polling(mut self) {
+    fn run_polling(mut self) -> ! {
         debug!("Running IRQ reactor in polling mode.");
         let hci_clone = Arc::clone(&self.hci);
 
@@ -204,11 +204,11 @@ impl IrqReactor {
         run.ints[0].iman.writef(1 << 1, true);
     }
 
-    fn run_with_irq_file(mut self) {
+    fn run_with_irq_file(mut self) -> ! {
         debug!("Running IRQ reactor with IRQ file and event queue");
 
         let hci_clone = Arc::clone(&self.hci);
-        let mut event_queue =
+        let event_queue =
             RawEventQueue::new().expect("xhcid irq_reactor: failed to create IRQ event queue");
         let irq_fd = self.irq_file.as_ref().unwrap().as_raw_fd();
         event_queue
@@ -226,7 +226,8 @@ impl IrqReactor {
         };
 
         trace!("IRQ reactor has grabbed the next index in the event ring.");
-        for _event in event_queue {
+        'trb_loop: loop {
+            let _event = event_queue.next_event().unwrap();
             trace!("IRQ event queue notified");
             let mut buffer = [0u8; 8];
 
@@ -240,7 +241,7 @@ impl IrqReactor {
             if !self.hci.received_irq() {
                 // continue only when an IRQ to this device was received
                 trace!("no interrupt pending");
-                break;
+                continue 'trb_loop;
             }
 
             self.mask_interrupts();
@@ -265,7 +266,7 @@ impl IrqReactor {
                     }
                     //hci_clone.event_handler_finished();
                     self.unmask_interrupts();
-                    return;
+                    continue 'trb_loop;
                 } else {
                     count += 1
                 }
@@ -287,7 +288,7 @@ impl IrqReactor {
                         debug!("The interrupt bit is no longer pending.");
                     }
                     self.unmask_interrupts();
-                    return;
+                    continue 'trb_loop;
                 }
                 self.handle_requests();
 
@@ -309,9 +310,7 @@ impl IrqReactor {
 
                 event_trb_index = event_ring.ring.next_index();
             }
-            trace!("Exited event loop!");
         }
-        trace!("IRQ Reactor has finished handling the interrupt");
     }
 
     /// Handles device attach/detach events as indicated by a PortStatusChange
@@ -538,7 +537,7 @@ impl IrqReactor {
         error!("TODO: grow event ring");
     }
 
-    pub fn run(mut self) {
+    pub fn run(self) -> ! {
         if self.irq_file.is_some() {
             self.run_with_irq_file();
         } else {
diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs
@@ -13,10 +13,9 @@ use std::collections::BTreeMap;
 use std::convert::TryFrom;
 use std::fs::File;
 use std::sync::atomic::AtomicUsize;
-use std::sync::{Arc, Mutex, MutexGuard};
+use std::sync::{Arc, Mutex};
 
 use std::{mem, process, slice, thread};
-use pcid_interface::msi::MappedMsixRegs;
 use syscall::error::{Error, Result, EBADF, EBADMSG, EIO, ENOENT};
 use syscall::{EAGAIN, PAGE_SIZE};
 
@@ -70,11 +69,8 @@ pub enum InterruptMethod {
     /// Legacy PCI INTx# interrupt pin.
     Intx,
 
-    /// Message signaled interrupts.
+    /// (Extended) Message signaled interrupts.
     Msi,
-
-    /// Extended message signaled interrupts.
-    MsiX(Mutex<MappedMsixRegs>),
 }
 
 impl Xhci {
@@ -1142,53 +1138,29 @@ impl Xhci {
         Ok(ring)
     }
 
-    pub fn uses_msi(&self) -> bool {
-        if let InterruptMethod::Msi = self.interrupt_method {
-            true
-        } else {
-            false
-        }
-    }
-    pub fn uses_msix(&self) -> bool {
-        if let InterruptMethod::MsiX(_) = self.interrupt_method {
-            true
-        } else {
-            false
-        }
-    }
-    // TODO: Perhaps use an rwlock?
-    pub fn msix_info(&self) -> Option<MutexGuard<'_, MappedMsixRegs>> {
-        match self.interrupt_method {
-            InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()),
-            _ => None,
-        }
-    }
-    pub fn msix_info_mut(&self) -> Option<MutexGuard<'_, MappedMsixRegs>> {
-        match self.interrupt_method {
-            InterruptMethod::MsiX(ref info) => Some(info.lock().unwrap()),
-            _ => None,
-        }
+    fn uses_msi_interrupts(&self) -> bool {
+        matches!(self.interrupt_method, InterruptMethod::Msi)
     }
 
     /// Checks whether an IRQ has been received from *this* device, in case of an interrupt. Always
     /// true when using MSI/MSI-X.
     pub fn received_irq(&self) -> bool {
         let mut runtime_regs = self.run.lock().unwrap();
 
-        if self.uses_msi() || self.uses_msix() {
+        if self.uses_msi_interrupts() {
             // Since using MSI and MSI-X implies having no IRQ sharing whatsoever, the IP bit
             // doesn't have to be touched.
             trace!(
                 "Successfully received MSI/MSI-X interrupt, IP={}, EHB={}",
                 runtime_regs.ints[0].iman.readf(1),
-                runtime_regs.ints[0].erdp_low.readf(3)
+                runtime_regs.ints[0].erdp_low.readf(1 << 3)
             );
             true
         } else if runtime_regs.ints[0].iman.readf(1) {
             trace!(
                 "Successfully received INTx# interrupt, IP={}, EHB={}",
                 runtime_regs.ints[0].iman.readf(1),
-                runtime_regs.ints[0].erdp_low.readf(3)
+                runtime_regs.ints[0].erdp_low.readf(1 << 3)
             );
             // If MSI and/or MSI-X are not used, the interrupt might have to be shared, and thus there is
             // a special register to specify whether the IRQ actually came from the xHC.
