Improve startup interrupt initialization.

nvmed/src/main.rs

@@ -1,4 +1,6 @@
-use std::{env, slice, usize};
+use std::{slice, usize};
+use std::collections::BTreeMap;
+use std::convert::TryInto;
 use std::fs::File;
 use std::io::{ErrorKind, Read, Write};
 use std::ptr::NonNull;
@@ -12,13 +14,13 @@ use syscall::io::Mmio;
 use arrayvec::ArrayVec;
 use log::{debug, error, info, warn, trace};
 
-use self::nvme::{InterruptMethod, Nvme};
+use self::nvme::{InterruptMethod, InterruptSources, Nvme};
 use self::scheme::DiskScheme;
 
 mod nvme;
 mod scheme;
 
-#[derive(Default)]
+/// A wrapper for a BAR allocation.
 pub struct Bar {
     ptr: NonNull<u8>,
     physical: usize,
@@ -40,11 +42,15 @@ impl Drop for Bar {
     }
 }
 
+/// The PCI BARs that may be allocated.
 #[derive(Default)]
 pub struct AllocatedBars(pub [Mutex<Option<Bar>>; 6]);
 
-/// Get the most optimal yet functional interrupt 
-fn get_int_method(pcid_handle: &mut PcidServerHandle, function: &PciFunction, nvme: &mut Nvme, allocated_bars: &AllocatedBars) -> Result<InterruptMethod> {
+/// Get the most optimal yet functional interrupt mechanism: either (in the order preference):
+/// MSI-X, MSI, and INTx# pin. Returns both runtime interrupt structures (MSI/MSI-X capability
+/// structures), and the handles to the interrupts.
+fn get_int_method(pcid_handle: &mut PcidServerHandle, function: &PciFunction, allocated_bars: &AllocatedBars) -> Result<(InterruptMethod, InterruptSources)> {
+    use pcid_interface::irq_helpers;
 
     let features = pcid_handle.fetch_all_features().unwrap();
 
@@ -97,25 +103,74 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, function: &PciFunction, nv
         pcid_handle.enable_feature(PciFeature::MsiX).unwrap();
         capability_struct.set_msix_enabled(true); // only affects our local mirror of the cap
 
-        // We don't allocate any vectors yet; that's later done when we get into
-        // submission/completion queues.
+        let (msix_vector_number, irq_handle) = {
+            use pcid_interface::msi::x86_64 as msi_x86_64;
+            use msi_x86_64::DeliveryMode;
 
-        Ok(InterruptMethod::MsiX(MsixCfg {
+            let entry: &mut MsixTableEntry = &mut table_entries[0];
+
+            let bsp_cpu_id = irq_helpers::read_bsp_apic_id().expect("nvmed: failed to read APIC ID");
+            let bsp_lapic_id = bsp_cpu_id.try_into().expect("nvmed: BSP local apic ID couldn't fit inside u8");
+            let (vector, irq_handle) = irq_helpers::allocate_single_interrupt_vector(bsp_cpu_id).expect("nvmed: failed to allocate single MSI-X interrupt vector").expect("nvmed: no interrupt vectors left on BSP");
+
+            let msg_addr = msi_x86_64::message_address(bsp_lapic_id, false, false);
+            let msg_data = msi_x86_64::message_data_edge_triggered(DeliveryMode::Fixed, vector);
+
+            entry.set_addr_lo(msg_addr);
+            entry.set_msg_data(msg_data);
+            entry.unmask();
+
+            (0, irq_handle)
+        };
+
+        let interrupt_method = InterruptMethod::MsiX(MsixCfg {
             cap: capability_struct,
             table: table_entries,
             pba: pba_entries,
-        }))
+        });
+        let interrupt_sources = InterruptSources::MsiX(std::iter::once((msix_vector_number, irq_handle)).collect());
+
+        Ok((interrupt_method, interrupt_sources))
     } else if has_msi {
         // Message signaled interrupts.
         let capability_struct = match pcid_handle.feature_info(PciFeature::Msi).unwrap() {
             PciFeatureInfo::Msi(msi) => msi,
             _ => unreachable!(),
         };
-        // We don't enable MSI until needed.
-        Ok(InterruptMethod::Msi(capability_struct))
+
+        let (msi_vector_number, irq_handle) = {
+            use pcid_interface::{MsiSetFeatureInfo, SetFeatureInfo};
+            use pcid_interface::msi::x86_64 as msi_x86_64;
+            use msi_x86_64::DeliveryMode;
+
+            let bsp_cpu_id = irq_helpers::read_bsp_apic_id().expect("nvmed: failed to read BSP APIC ID");
+            let bsp_lapic_id = bsp_cpu_id.try_into().expect("nvmed: BSP local apic ID couldn't fit inside u8");
+            let (vector, irq_handle) = irq_helpers::allocate_single_interrupt_vector(bsp_cpu_id).expect("nvmed: failed to allocate single MSI interrupt vector").expect("nvmed: no interrupt vectors left on BSP");
+
+            let msg_addr = msi_x86_64::message_address(bsp_lapic_id, false, false);
+            let msg_data = msi_x86_64::message_data_edge_triggered(DeliveryMode::Fixed, vector) as u16;
+
+            pcid_handle.set_feature_info(SetFeatureInfo::Msi(MsiSetFeatureInfo {
+                message_address: Some(msg_addr),
+                message_upper_address: Some(0),
+                message_data: Some(msg_data),
+                multi_message_enable: Some(0), // enable 2^0=1 vectors
+                mask_bits: None,
+            }));
+
+            (0, irq_handle)
+        };
+
+        let interrupt_method = InterruptMethod::Msi(capability_struct);
+        let interrupt_sources = InterruptSources::Msi(std::iter::once((msi_vector_number, irq_handle)).collect());
+
+        pcid_handle.enable_feature(PciFeature::Msi).unwrap();
+
+        Ok((interrupt_method, interrupt_sources))
     } else if function.legacy_interrupt_pin.is_some() {
         // INTx# pin based interrupts.
-        Ok(InterruptMethod::Intx)
+        let irq_handle = File::open(format!("irq:{}", function.legacy_interrupt_line)).expect("nvmed: failed to open INTx# interrupt line");
+        Ok((InterruptMethod::Intx, InterruptSources::Intx(irq_handle)))
     } else {
         // No interrupts at all
         todo!("handling of no interrupts")
@@ -155,37 +210,30 @@ fn main() {
             .expect("nvmed: failed to open event queue");
         let mut event_file = unsafe { File::from_raw_fd(event_fd as RawFd) };
 
-        let irq_fd = syscall::open(
-            &format!("irq:{}", irq),
-            syscall::O_RDWR | syscall::O_NONBLOCK | syscall::O_CLOEXEC
-        ).expect("nvmed: failed to open irq file");
-        syscall::write(event_fd, &syscall::Event {
-            id: irq_fd,
-            flags: syscall::EVENT_READ,
-            data: 0,
-        }).expect("nvmed: failed to watch irq file events");
-        let mut irq_file = unsafe { File::from_raw_fd(irq_fd as RawFd) };
-
         let scheme_name = format!("disk/{}", name);
         let socket_fd = syscall::open(
             &format!(":{}", scheme_name),
             syscall::O_RDWR | syscall::O_CREAT | syscall::O_NONBLOCK | syscall::O_CLOEXEC
         ).expect("nvmed: failed to create disk scheme");
+
         syscall::write(event_fd, &syscall::Event {
             id: socket_fd,
             flags: syscall::EVENT_READ,
-            data: 1,
+            data: 0,
         }).expect("nvmed: failed to watch disk scheme events");
+
         let mut socket_file = unsafe { File::from_raw_fd(socket_fd as RawFd) };
 
         syscall::setrens(0, 0).expect("nvmed: failed to enter null namespace");
 
         let (reactor_sender, reactor_receiver) = crossbeam_channel::unbounded();
+        let (interrupt_method, interrupt_sources) = get_int_method(&mut pcid_handle, &pci_config.func, &allocated_bars).expect("nvmed: failed to find a suitable interrupt method");
         let mut nvme = Nvme::new(address, interrupt_method, pcid_handle, reactor_sender).expect("nvmed: failed to allocate driver data");
         let nvme = Arc::new(nvme);
         unsafe { nvme.init() }
-        nvme::cq_reactor::start_cq_reactor_thread(nvme);
+        nvme::cq_reactor::start_cq_reactor_thread(nvme, interrupt_sources, reactor_receiver);
         let namespaces = unsafe { nvme.init_with_queues() };
+
         let mut scheme = DiskScheme::new(scheme_name, nvme, namespaces);
         let mut todo = Vec::new();
         'events: loop {
@@ -195,15 +243,7 @@ fn main() {
             }
 
             match event.data {
-                0 => {
-                    let mut irq = [0; 8];
-                    if irq_file.read(&mut irq).expect("nvmed: failed to read irq file") >= irq.len() {
-                        if scheme.irq() {
-                            irq_file.write(&irq).expect("nvmed: failed to write irq file");
-                        }
-                    }
-                },
-                1 => loop {
+                0 => loop {
                     let mut packet = Packet::default();
                     match socket_file.read(&mut packet) {
                         Ok(0) => break 'events,

nvmed/src/nvme/cq_reactor.rs

@@ -5,23 +5,14 @@ use std::io::prelude::*;
 use std::pin::Pin;
 use std::sync::{Arc, Mutex};
 use std::{io, task, thread};
-use std::os::unix::io::{FromRawFd, RawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
 use syscall::Result;
+use syscall::data::Event;
 
 use crossbeam_channel::{Sender, Receiver};
 
-use crate::nvme::{InterruptMethod, Nvme, NvmeComp};
-
-/// A source of interrupts. The NVME spec splits the definition of MSI into "single msi" and "multi
-/// msi".
-#[derive(Debug)]
-pub enum IntSources {
-    Intx(File),
-    SingleMsi(File),
-    MultiMsi(BTreeMap<u8, File>),
-    MsiX(BTreeMap<u16, File>),
-}
+use crate::nvme::{InterruptMethod, InterruptSources, Nvme, NvmeComp};
 
 /// A notification request, sent by the future in order to tell the completion thread that the
 /// current task wants a notification when a matching completion queue entry has been seen.
@@ -40,26 +31,54 @@ struct PendingReq {
     queue_id: usize,
 }
 struct CqReactor {
-    int_sources: Option<IntSources>,
+    int_sources: InterruptSources,
     nvme: Arc<Nvme>,
     pending_reqs: Vec<PendingReq>,
     receiver: Receiver<NotifReq>,
     event_queue: File,
 }
 impl CqReactor {
-    fn create_event_queue() -> Result<File> {
+    fn create_event_queue(int_sources: &InterruptSources) -> Result<File> {
         use syscall::flag::*;
         let fd = syscall::open("event:", O_CLOEXEC | O_RDWR)?;
         let mut file = unsafe { File::from_raw_fd(fd as RawFd) };
-        todo!()
+
+        let mut msix_iter;
+        let mut msi_iter;
+        let mut intx_iter;
+
+        let iter: &mut dyn Iterator<Item = (u16, &File)> = match int_sources {
+            InterruptSources::MsiX(ref btree) => {
+                msix_iter = btree.iter().map(|(&n, f)| (n, f));
+                &mut msix_iter
+            }
+            InterruptSources::Msi(ref btree) => {
+                msi_iter = btree.iter().map(|(&n, f)| (u16::from(n), f));
+                &mut msi_iter
+            }
+            InterruptSources::Intx(ref file) => {
+                intx_iter = std::iter::once((0, file));
+                &mut intx_iter
+            }
+        };
+        for (num, irq_handle) in iter {
+            if file.write(&Event {
+                id: irq_handle.as_raw_fd() as usize,
+                flags: syscall::EVENT_READ,
+                data: num as usize,
+            }).unwrap() == 0 {
+                panic!("Failed to setup event queue for {} {:?}", num, irq_handle);
+            }
+        }
+        Ok(file)
     }
-    fn new(nvme: Arc<Nvme>, receiver: Receiver<NotifReq>) -> Result<Self> {
+    fn new(nvme: Arc<Nvme>, int_sources: InterruptSources, receiver: Receiver<NotifReq>) -> Result<Self> {
         Ok(Self {
-            int_sources: None, // TODO
+            event_queue: Self::create_event_queue(&int_sources)?,
+            int_sources,
             nvme,
             pending_reqs: Vec::new(),
             receiver,
-            event_queue: Self::create_event_queue()?,
         })
     }
     fn handle_notif_reqs(&mut self) {
@@ -73,19 +92,27 @@ impl CqReactor {
             }
         }
     }
+    fn block_on_new_irq(&mut self) -> Event {
+        let mut event = Event::default();
+        self.event_queue.read(&mut event);
+        event
+    }
     fn run(mut self) -> ! {
         loop {
             self.handle_notif_reqs();
+            let event = self.block_on_new_irq();
         }
     }
 }
 
-pub fn start_cq_reactor_thread(nvme: Arc<Nvme>, receiver: Receiver<NotifReq>) -> thread::JoinHandle<()> {
+pub fn start_cq_reactor_thread(nvme: Arc<Nvme>, interrupt_sources: InterruptSources, receiver: Receiver<NotifReq>) -> thread::JoinHandle<()> {
     // Actually, nothing prevents us from spawning additional threads. the channel is MPMC and
     // everything is properly synchronized. I'm not saying this is strictly required, but with
-    // multiple completion queues it might actually be worth considering.
+    // multiple completion queues it might actually be worth considering. The IRQ subsystem might
+    // be improved to lower the latency, but MSI-X allows multiple vectors to point to different
+    // CPUs, so that the load is balanced across the logical processors.
     thread::spawn(move || {
-        CqReactor::new(nvme, receiver)
+        CqReactor::new(nvme, interrupt_sources, receiver)
             .expect("nvmed: failed to setup CQ reactor")
             .run()
     })

nvmed/src/nvme/mod.rs

@@ -1,7 +1,8 @@
-use std::ptr;
 use std::collections::BTreeMap;
+use std::fs::File;
 use std::sync::{Mutex, RwLock};
 use std::sync::atomic::{AtomicUsize, Ordering};
+use std::ptr;
 
 use crossbeam_channel::Sender;
 
@@ -9,19 +10,44 @@ use syscall::io::{Dma, Io, Mmio};
 use syscall::error::{Error, Result, EINVAL};
 
 pub mod cq_reactor;
-use self::cq_reactor::{self, NotifReq};
+use self::cq_reactor::NotifReq;
 
 use pcid_interface::msi::{MsiCapability, MsixCapability, MsixTableEntry};
 use pcid_interface::PcidServerHandle;
 
 #[derive(Debug)]
-pub enum InterruptMethod {
-    Intx,
-    Msi(MsiCapability),
-    MsiX(MsixCfg),
+pub enum InterruptSources {
+    MsiX(BTreeMap<u16, File>),
+    Msi(BTreeMap<u8, File>),
+    Intx(File),
+}
+
+pub enum InterruptMethod {
+    /// INTx# interrupt pins
+    Intx,
+    /// Message signaled interrupts
+    Msi(MsiCapability),
+    /// Extended message signaled interrupts
+    MsiX(MsixCfg),
+}
+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 }
+    }
 }
 
-#[derive(Debug)]
 pub struct MsixCfg {
     pub cap: MsixCapability,
     pub table: &'static mut [MsixTableEntry],
@@ -307,6 +333,8 @@ pub struct Nvme {
     reactor_sender: Sender<cq_reactor::NotifReq>,
     next_cid: AtomicUsize,
 }
+unsafe impl Send for Nvme {}
+unsafe impl Sync for Nvme {}
 
 impl Nvme {
     pub fn new(address: usize, interrupt_method: InterruptMethod, pcid_interface: PcidServerHandle, reactor_sender: Sender<NotifReq>) -> Result<Self> {
@@ -326,7 +354,7 @@ impl Nvme {
         let mut regs_guard = self.regs.lock().unwrap();
 
         let dstrd = ((regs_guard.cap.read() >> 32) & 0b1111) as usize;
-        let addr = (regs_guard as *mut u8 as usize)
+        let addr = ((*regs_guard) as *mut NvmeRegs as usize)
             + 0x1000
             + index * (4 << dstrd);
         (&mut *(addr as *mut Mmio<u32>)).write(value);
@@ -368,7 +396,10 @@ impl Nvme {
         }
 
         // println!("  - Mask all interrupts");
-        self.regs.get_mut().unwrap().intms.write(0xFFFFFFFF); // TODO: Don't mask
+        if !self.interrupt_method.get_mut().unwrap().is_msix() {
+            self.regs.get_mut().unwrap().intms.write(0xFFFFFFFF);
+            self.regs.get_mut().unwrap().intmc.write(0xFFFFFFFE);
+        }
 
         for (qid, queue) in self.completion_queues.get_mut().unwrap().iter().enumerate() {
             let data = &queue.get_mut().unwrap().data;

pcid/src/pci/msi.rs

@@ -385,12 +385,11 @@ pub mod x86_64 {
     }
 
     // TODO: should the reserved field be preserved?
-    pub const fn message_address(destination_id: u8, rh: bool, dm: bool, xx: u8) -> u32 {
+    pub const fn message_address(destination_id: u8, rh: bool, dm: bool) -> u32 {
         0xFEE0_0000u32
             | ((destination_id as u32) << 12)
             | ((rh as u32) << 3)
             | ((dm as u32) << 2)
-            | xx as u32
     }
     pub const fn message_data(trigger_mode: TriggerMode, level_trigger_mode: LevelTriggerMode, delivery_mode: DeliveryMode, vector: u8) -> u32 {
         ((trigger_mode as u32) << 15)
@@ -413,12 +412,21 @@ impl MsixTableEntry {
     pub fn addr_hi(&self) -> u32 {
         self.addr_hi.read()
     }
+    pub fn set_addr_lo(&mut self, value: u32) {
+        self.addr_lo.write(value);
+    }
+    pub fn set_addr_hi(&mut self, value: u32) {
+        self.addr_hi.write(value);
+    }
     pub fn msg_data(&self) -> u32 {
         self.msg_data.read()
     }
     pub fn vec_ctl(&self) -> u32 {
         self.vec_ctl.read()
     }
+    pub fn set_msg_data(&mut self, value: u32) {
+        self.msg_data.write(value);
+    }
     pub fn addr(&self) -> u64 {
         u64::from(self.addr_lo()) | (u64::from(self.addr_hi()) << 32)
     }