interrupt: Use a single closure to describe pin based and MSI-X

In order to factorize the complexity brought by closures, this commit
merges IrqClosure and MsixClosure into a generic InterruptDelivery one.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

pci/src/device.rs

@@ -13,9 +13,12 @@ use vm_allocator::SystemAllocator;
 use vm_memory::{GuestAddress, GuestUsize};
 use vmm_sys_util::EventFd;
 
-pub type IrqClosure = Box<Fn() -> std::result::Result<(), std::io::Error> + Send + Sync>;
+pub struct InterruptParameters {
-pub type MsixClosure =
+    pub msix: Option<MsixTableEntry>,
-    Box<Fn(MsixTableEntry) -> std::result::Result<(), std::io::Error> + Send + Sync>;
+}
+
+pub type InterruptDelivery =
+    Box<Fn(InterruptParameters) -> std::result::Result<(), std::io::Error> + Send + Sync>;
 
 #[derive(Debug)]
 pub enum Error {
@@ -49,14 +52,14 @@ pub trait PciDevice: BusDevice {
     /// The device may write to `irq_evt` to trigger an interrupt.
     fn assign_pin_irq(
         &mut self,
-        _irq_cb: Arc<IrqClosure>,
+        _irq_cb: Arc<InterruptDelivery>,
         _irq_num: u32,
         _irq_pin: PciInterruptPin,
     ) {
     }
 
     /// Assign MSI-X to this device.
-    fn assign_msix(&mut self, _msi_cb: Arc<MsixClosure>) {}
+    fn assign_msix(&mut self, _msi_cb: Arc<InterruptDelivery>) {}
 
     /// Allocates the needed PCI BARs space using the `allocate` function which takes a size and
     /// returns an address. Returns a Vec of (GuestAddress, GuestUsize) tuples.

pci/src/lib.rs

@@ -21,7 +21,7 @@ pub use self::configuration::{
     PciSubclass,
 };
 pub use self::device::Error as PciDeviceError;
-pub use self::device::{IrqClosure, MsixClosure, PciDevice};
+pub use self::device::{InterruptDelivery, InterruptParameters, PciDevice};
 pub use self::msix::{MsixCap, MsixConfig, MsixTableEntry};
 pub use self::root::{PciConfigIo, PciConfigMmio, PciRoot, PciRootError};
 

vm-virtio/src/transport/pci_device.rs

@@ -20,9 +20,9 @@ use std::sync::Mutex;
 
 use devices::BusDevice;
 use pci::{
-    IrqClosure, MsixCap, MsixClosure, MsixConfig, PciBarConfiguration, PciCapability,
+    InterruptDelivery, InterruptParameters, MsixCap, MsixConfig, PciBarConfiguration,
-    PciCapabilityID, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType,
+    PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice, PciDeviceError,
-    PciInterruptPin, PciSubclass,
+    PciHeaderType, PciInterruptPin, PciSubclass,
 };
 use vm_allocator::SystemAllocator;
 use vm_memory::{Address, ByteValued, GuestAddress, GuestMemoryMmap, GuestUsize, Le32};
@@ -333,20 +333,33 @@ impl VirtioPciDevice {
 }
 
 impl PciDevice for VirtioPciDevice {
-    fn assign_pin_irq(&mut self, irq_cb: Arc<IrqClosure>, irq_num: u32, irq_pin: PciInterruptPin) {
+    fn assign_pin_irq(
+        &mut self,
+        irq_cb: Arc<InterruptDelivery>,
+        irq_num: u32,
+        irq_pin: PciInterruptPin,
+    ) {
         self.configuration.set_irq(irq_num as u8, irq_pin);
 
-        let cb = Arc::new(Box::new(move |_queue: &Queue| (irq_cb)()) as VirtioInterrupt);
+        let cb = Arc::new(Box::new(move |_queue: &Queue| {
+            let param = InterruptParameters { msix: None };
+            (irq_cb)(param)
+        }) as VirtioInterrupt);
 
         self.interrupt_cb = Some(cb);
     }
 
-    fn assign_msix(&mut self, msi_cb: Arc<MsixClosure>) {
+    fn assign_msix(&mut self, msi_cb: Arc<InterruptDelivery>) {
         let msix_config = self.msix_config.clone();
 
         let cb = Arc::new(Box::new(move |queue: &Queue| {
-            (msi_cb)(msix_config.lock().unwrap().table_entries[queue.vector as usize].clone())
+            let param = InterruptParameters {
-        }) as VirtioInterruptClosure);
+                msix: Some(
+                    msix_config.lock().unwrap().table_entries[queue.vector as usize].clone(),
+                ),
+            };
+            (msi_cb)(param)
+        }) as VirtioInterrupt);
 
         self.interrupt_cb = Some(cb);
     }

vmm/src/vm.rs

@@ -28,7 +28,7 @@ use libc::{c_void, siginfo_t, EFD_NONBLOCK};
 use linux_loader::loader::KernelLoader;
 use net_util::Tap;
 use pci::{
-    IrqClosure, MsixClosure, MsixTableEntry, PciConfigIo, PciDevice, PciInterruptPin, PciRoot,
+    InterruptDelivery, InterruptParameters, PciConfigIo, PciDevice, PciInterruptPin, PciRoot,
 };
 use qcow::{self, ImageType, QcowFile};
 use std::ffi::CString;
@@ -447,24 +447,31 @@ impl DeviceManager {
         if msi_capable {
             let vm_fd_clone = vm_fd.clone();
 
-            let msi_cb = Arc::new(Box::new(move |entry: MsixTableEntry| {
+            let msi_cb = Arc::new(Box::new(move |p: InterruptParameters| {
-                let msi_queue = kvm_msi {
+                if let Some(entry) = p.msix {
-                    address_lo: entry.msg_addr_lo,
+                    let msi_queue = kvm_msi {
-                    address_hi: entry.msg_addr_hi,
+                        address_lo: entry.msg_addr_lo,
-                    data: entry.msg_data,
+                        address_hi: entry.msg_addr_hi,
-                    flags: 0u32,
+                        data: entry.msg_data,
-                    devid: 0u32,
+                        flags: 0u32,
-                    pad: [0u8; 12],
+                        devid: 0u32,
-                };
+                        pad: [0u8; 12],
+                    };
 
-                vm_fd_clone.signal_msi(msi_queue).map(|ret| {
+                    return vm_fd_clone.signal_msi(msi_queue).map(|ret| {
-                    if ret > 0 {
+                        if ret > 0 {
-                        debug!("MSI message successfully delivered");
+                            debug!("MSI message successfully delivered");
-                    } else if ret == 0 {
+                        } else if ret == 0 {
-                        warn!("failed to deliver MSI message, blocked by guest");
+                            warn!("failed to deliver MSI message, blocked by guest");
-                    }
+                        }
-                })
+                    });
-            }) as MsixClosure);
+                }
+
+                Err(std::io::Error::new(
+                    std::io::ErrorKind::Other,
+                    "missing MSI-X entry",
+                ))
+            }) as InterruptDelivery);
 
             virtio_pci_device.assign_msix(msi_cb);
         } else {
@@ -477,7 +484,9 @@ impl DeviceManager {
                 .register_irqfd(irqfd.as_raw_fd(), irq_num)
                 .map_err(DeviceManagerError::Irq)?;
 
-            let irq_cb = Arc::new(Box::new(move || irqfd.write(1)) as IrqClosure);
+            let irq_cb = Arc::new(
+                Box::new(move |_p: InterruptParameters| irqfd.write(1)) as InterruptDelivery
+            );
             virtio_pci_device.assign_pin_irq(irq_cb, irq_num as u32, PciInterruptPin::IntA);
         }