From c396baca46f8175cc5e958a8acf2ab4563a9f7f0 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 13 Jan 2020 18:52:19 +0100 Subject: [PATCH] vm-virtio: Modify VirtioInterrupt callback into a trait Callbacks are not the most Rust idiomatic way of programming. The right way is to use a Trait to provide multiple implementation of the same interface. Additionally, a Trait will allow for multiple functions to be defined while using callbacks means that a new callback must be introduced for each new function we want to add. For these two reasons, the current commit modifies the existing VirtioInterrupt callback into a Trait of the same name. Signed-off-by: Sebastien Boeuf --- pci/src/device.rs | 2 +- vm-virtio/src/block.rs | 16 +- vm-virtio/src/console.rs | 25 ++-- vm-virtio/src/device.rs | 16 +- vm-virtio/src/iommu.rs | 18 ++- vm-virtio/src/net.rs | 18 ++- vm-virtio/src/pmem.rs | 18 ++- vm-virtio/src/rng.rs | 18 ++- vm-virtio/src/transport/mmio.rs | 51 +++++-- vm-virtio/src/transport/pci_common_config.rs | 2 +- vm-virtio/src/transport/pci_device.rs | 150 +++++++++---------- vm-virtio/src/vhost_user/blk.rs | 6 +- vm-virtio/src/vhost_user/fs.rs | 6 +- vm-virtio/src/vhost_user/handler.rs | 6 +- vm-virtio/src/vhost_user/net.rs | 6 +- vm-virtio/src/vsock/device.rs | 30 ++-- vm-virtio/src/vsock/mod.rs | 16 +- vmm/src/device_manager.rs | 40 +---- 18 files changed, 221 insertions(+), 223 deletions(-) diff --git a/pci/src/device.rs b/pci/src/device.rs index 9df2129d9..6a4a90b33 100755 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -67,7 +67,7 @@ pub trait PciDevice: BusDevice { } /// Assign MSI-X to this device. - fn assign_msix(&mut self, _msi_cb: Arc) {} + fn assign_msix(&mut self) {} /// 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. diff --git a/vm-virtio/src/block.rs b/vm-virtio/src/block.rs index fdd839a69..3a7fb871c 100755 --- a/vm-virtio/src/block.rs +++ b/vm-virtio/src/block.rs @@ -596,7 +596,7 @@ struct BlockEpollHandler { mem: Arc>, disk_image: T, disk_nsectors: u64, - interrupt_cb: Arc, + interrupt_cb: Arc, disk_image_id: Vec, kill_evt: EventFd, pause_evt: EventFd, @@ -649,12 +649,12 @@ impl BlockEpollHandler { } fn signal_used_queue(&self, queue_index: usize) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(&self.queues[queue_index])).map_err( - |e| { + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(&self.queues[queue_index])) + .map_err(|e| { error!("Failed to signal used queue: {:?}", e); DeviceError::FailedSignalingUsedQueue(e) - }, - ) + }) } #[allow(dead_code)] @@ -774,7 +774,7 @@ pub struct Block { acked_features: u64, config_space: Vec, queue_evt: Option, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, pause_evt: Option, paused: Arc, @@ -917,7 +917,7 @@ impl VirtioDevice for Block { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -989,7 +989,7 @@ impl VirtioDevice for Block { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/console.rs b/vm-virtio/src/console.rs index ff19e578e..6927e90ed 100755 --- a/vm-virtio/src/console.rs +++ b/vm-virtio/src/console.rs @@ -59,7 +59,7 @@ unsafe impl ByteValued for VirtioConsoleConfig {} struct ConsoleEpollHandler { queues: Vec, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, in_buffer: Arc>>, out: Arc>>, input_queue_evt: EventFd, @@ -147,10 +147,12 @@ impl ConsoleEpollHandler { } fn signal_used_queue(&self) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(&self.queues[0])).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(&self.queues[0])) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } fn run(&mut self, paused: Arc) -> result::Result<(), DeviceError> { @@ -261,8 +263,9 @@ impl ConsoleEpollHandler { if let Err(e) = self.config_evt.read() { error!("Failed to get config event: {:?}", e); break 'epoll; - } else if let Err(e) = - (self.interrupt_cb)(&VirtioInterruptType::Config, None) + } else if let Err(e) = self + .interrupt_cb + .trigger(&VirtioInterruptType::Config, None) { error!("Failed to signal console driver: {:?}", e); } @@ -347,7 +350,7 @@ pub struct Console { input: Arc, out: Arc>>, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -471,7 +474,7 @@ impl VirtioDevice for Console { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -521,7 +524,7 @@ impl VirtioDevice for Console { .store(self.acked_features, Ordering::Relaxed); if (self.acked_features & (1u64 << VIRTIO_CONSOLE_F_SIZE)) != 0 { - if let Err(e) = (interrupt_cb)(&VirtioInterruptType::Config, None) { + if let Err(e) = interrupt_cb.trigger(&VirtioInterruptType::Config, None) { error!("Failed to signal console driver: {:?}", e); } } @@ -553,7 +556,7 @@ impl VirtioDevice for Console { Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/device.rs b/vm-virtio/src/device.rs index 707d34860..da9ba1c6b 100644 --- a/vm-virtio/src/device.rs +++ b/vm-virtio/src/device.rs @@ -17,11 +17,13 @@ pub enum VirtioInterruptType { Queue, } -pub type VirtioInterrupt = Box< - dyn Fn(&VirtioInterruptType, Option<&Queue>) -> std::result::Result<(), std::io::Error> - + Send - + Sync, ->; +pub trait VirtioInterrupt: Send + Sync { + fn trigger( + &self, + int_type: &VirtioInterruptType, + queue: Option<&Queue>, + ) -> std::result::Result<(), std::io::Error>; +} pub type VirtioIommuRemapping = Box std::result::Result + Send + Sync>; @@ -72,14 +74,14 @@ pub trait VirtioDevice: Send { fn activate( &mut self, mem: Arc>, - interrupt_evt: Arc, + interrupt_evt: Arc, queues: Vec, queue_evts: Vec, ) -> ActivateResult; /// Optionally deactivates this device and returns ownership of the guest memory map, interrupt /// event, and queue events. - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { None } diff --git a/vm-virtio/src/iommu.rs b/vm-virtio/src/iommu.rs index ecf9493ae..19aa0929b 100644 --- a/vm-virtio/src/iommu.rs +++ b/vm-virtio/src/iommu.rs @@ -557,7 +557,7 @@ impl Request { struct IommuEpollHandler { queues: Vec, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queue_evts: Vec, kill_evt: EventFd, pause_evt: EventFd, @@ -601,10 +601,12 @@ impl IommuEpollHandler { } fn signal_used_queue(&self, queue: &Queue) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(queue)).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(queue)) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } fn run(&mut self, paused: Arc) -> result::Result<(), DeviceError> { @@ -762,7 +764,7 @@ pub struct Iommu { mapping: Arc, ext_mapping: BTreeMap>, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -879,7 +881,7 @@ impl VirtioDevice for Iommu { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, queue_evts: Vec, ) -> ActivateResult { @@ -948,7 +950,7 @@ impl VirtioDevice for Iommu { Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/net.rs b/vm-virtio/src/net.rs index a9250c1a5..3425b7f9d 100644 --- a/vm-virtio/src/net.rs +++ b/vm-virtio/src/net.rs @@ -48,7 +48,7 @@ struct NetEpollHandler { tap: Tap, rx: RxVirtio, tx: TxVirtio, - interrupt_cb: Arc, + interrupt_cb: Arc, kill_evt: EventFd, pause_evt: EventFd, epoll_fd: RawFd, @@ -57,10 +57,12 @@ struct NetEpollHandler { impl NetEpollHandler { fn signal_used_queue(&self, queue: &Queue) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(queue)).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(queue)) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } // Copies a single frame from `self.rx.frame_buf` into the guest. Returns true @@ -299,7 +301,7 @@ pub struct Net { // or nothing, if no such address if provided. config_space: Vec, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>>, ctrl_queue_epoll_thread: Option>>, paused: Arc, @@ -449,7 +451,7 @@ impl VirtioDevice for Net { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, mut queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -562,7 +564,7 @@ impl VirtioDevice for Net { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/pmem.rs b/vm-virtio/src/pmem.rs index a50d5560e..5f2a714a7 100644 --- a/vm-virtio/src/pmem.rs +++ b/vm-virtio/src/pmem.rs @@ -160,7 +160,7 @@ struct PmemEpollHandler { queue: Queue, mem: Arc>, disk: File, - interrupt_cb: Arc, + interrupt_cb: Arc, queue_evt: EventFd, kill_evt: EventFd, pause_evt: EventFd, @@ -213,10 +213,12 @@ impl PmemEpollHandler { } fn signal_used_queue(&self) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(&self.queue)).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(&self.queue)) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } fn run(&mut self, paused: Arc) -> result::Result<(), DeviceError> { @@ -315,7 +317,7 @@ pub struct Pmem { acked_features: u64, config: VirtioPmemConfig, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -422,7 +424,7 @@ impl VirtioDevice for Pmem { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, mut queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -496,7 +498,7 @@ impl VirtioDevice for Pmem { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/rng.rs b/vm-virtio/src/rng.rs index fc2e4ff09..9c37414ae 100755 --- a/vm-virtio/src/rng.rs +++ b/vm-virtio/src/rng.rs @@ -38,7 +38,7 @@ struct RngEpollHandler { queues: Vec, mem: Arc>, random_file: File, - interrupt_cb: Arc, + interrupt_cb: Arc, queue_evt: EventFd, kill_evt: EventFd, pause_evt: EventFd, @@ -80,10 +80,12 @@ impl RngEpollHandler { } fn signal_used_queue(&self) -> result::Result<(), DeviceError> { - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(&self.queues[0])).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(&self.queues[0])) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } fn run(&mut self, paused: Arc) -> result::Result<(), DeviceError> { @@ -181,7 +183,7 @@ pub struct Rng { avail_features: u64, acked_features: u64, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -273,7 +275,7 @@ impl VirtioDevice for Rng { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -347,7 +349,7 @@ impl VirtioDevice for Rng { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/transport/mmio.rs b/vm-virtio/src/transport/mmio.rs index 42fcf6718..a9a21a77f 100644 --- a/vm-virtio/src/transport/mmio.rs +++ b/vm-virtio/src/transport/mmio.rs @@ -24,6 +24,37 @@ const VENDOR_ID: u32 = 0; const MMIO_MAGIC_VALUE: u32 = 0x7472_6976; const MMIO_VERSION: u32 = 2; +pub struct VirtioInterruptIntx { + interrupt_status: Arc, + interrupt: Box, +} + +impl VirtioInterruptIntx { + pub fn new(interrupt_status: Arc, interrupt: Box) -> Self { + VirtioInterruptIntx { + interrupt_status, + interrupt, + } + } +} + +impl VirtioInterrupt for VirtioInterruptIntx { + fn trigger( + &self, + int_type: &VirtioInterruptType, + _queue: Option<&Queue>, + ) -> std::result::Result<(), std::io::Error> { + let status = match int_type { + VirtioInterruptType::Config => INTERRUPT_STATUS_CONFIG_CHANGED, + VirtioInterruptType::Queue => INTERRUPT_STATUS_USED_RING, + }; + self.interrupt_status + .fetch_or(status as usize, Ordering::SeqCst); + + self.interrupt.deliver() + } +} + /// Implements the /// [MMIO](http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-1090002) /// transport for virtio devices. @@ -46,7 +77,7 @@ pub struct MmioDevice { acked_features_select: u32, queue_select: u32, interrupt_status: Arc, - interrupt_cb: Option>, + interrupt_cb: Option>, driver_status: u32, config_generation: u32, queues: Vec, @@ -127,20 +158,10 @@ impl MmioDevice { } pub fn assign_interrupt(&mut self, interrupt: Box) { - let interrupt_status = self.interrupt_status.clone(); - let cb = Arc::new(Box::new( - move |int_type: &VirtioInterruptType, _queue: Option<&Queue>| { - let status = match int_type { - VirtioInterruptType::Config => INTERRUPT_STATUS_CONFIG_CHANGED, - VirtioInterruptType::Queue => INTERRUPT_STATUS_USED_RING, - }; - interrupt_status.fetch_or(status as usize, Ordering::SeqCst); - - interrupt.deliver() - }, - ) as VirtioInterrupt); - - self.interrupt_cb = Some(cb); + self.interrupt_cb = Some(Arc::new(VirtioInterruptIntx::new( + self.interrupt_status.clone(), + interrupt, + ))); } } diff --git a/vm-virtio/src/transport/pci_common_config.rs b/vm-virtio/src/transport/pci_common_config.rs index eb9f3bcf7..d6cf39995 100644 --- a/vm-virtio/src/transport/pci_common_config.rs +++ b/vm-virtio/src/transport/pci_common_config.rs @@ -273,7 +273,7 @@ mod tests { fn activate( &mut self, _mem: Arc>, - _interrupt_evt: Arc, + _interrupt_evt: Arc, _queues: Vec, _queue_evts: Vec, ) -> ActivateResult { diff --git a/vm-virtio/src/transport/pci_device.rs b/vm-virtio/src/transport/pci_device.rs index fab540366..8d7f5eb55 100755 --- a/vm-virtio/src/transport/pci_device.rs +++ b/vm-virtio/src/transport/pci_device.rs @@ -18,8 +18,7 @@ use crate::transport::VirtioTransport; use crate::{ Queue, VirtioDevice, VirtioDeviceType, VirtioInterrupt, VirtioInterruptType, VirtioIommuRemapping, DEVICE_ACKNOWLEDGE, DEVICE_DRIVER, DEVICE_DRIVER_OK, DEVICE_FAILED, - DEVICE_FEATURES_OK, DEVICE_INIT, INTERRUPT_STATUS_CONFIG_CHANGED, INTERRUPT_STATUS_USED_RING, - VIRTIO_MSI_NO_VECTOR, + DEVICE_FEATURES_OK, DEVICE_INIT, VIRTIO_MSI_NO_VECTOR, }; use arc_swap::ArcSwap; use devices::BusDevice; @@ -27,10 +26,10 @@ use kvm_bindings::kvm_irq_routing_entry; use kvm_ioctls::VmFd; use libc::EFD_NONBLOCK; use pci::{ - BarReprogrammingParams, InterruptDelivery, InterruptParameters, MsixCap, MsixConfig, - PciBarConfiguration, PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode, - PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, - PciMassStorageSubclass, PciNetworkControllerSubclass, PciSubclass, + BarReprogrammingParams, InterruptDelivery, MsixCap, MsixConfig, PciBarConfiguration, + PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice, + PciDeviceError, PciHeaderType, PciInterruptPin, PciMassStorageSubclass, + PciNetworkControllerSubclass, PciSubclass, }; use std::any::Any; use std::collections::HashMap; @@ -235,7 +234,7 @@ pub struct VirtioPciDevice { // PCI interrupts. interrupt_status: Arc, - interrupt_cb: Option>, + virtio_interrupt: Option>, // virtio queues queues: Vec, @@ -342,7 +341,7 @@ impl VirtioPciDevice { device, device_activated: false, interrupt_status: Arc::new(AtomicUsize::new(0)), - interrupt_cb: None, + virtio_interrupt: None, queues, queue_evts, memory: Some(memory), @@ -469,81 +468,74 @@ impl VirtioTransport for VirtioPciDevice { } } +pub struct VirtioInterruptMsix { + msix_config: Arc>, + config_vector: Arc, +} + +impl VirtioInterruptMsix { + pub fn new(msix_config: Arc>, config_vector: Arc) -> Self { + VirtioInterruptMsix { + msix_config, + config_vector, + } + } +} + +impl VirtioInterrupt for VirtioInterruptMsix { + fn trigger( + &self, + int_type: &VirtioInterruptType, + queue: Option<&Queue>, + ) -> std::result::Result<(), std::io::Error> { + let vector = match int_type { + VirtioInterruptType::Config => self.config_vector.load(Ordering::SeqCst), + VirtioInterruptType::Queue => { + if let Some(q) = queue { + q.vector + } else { + 0 + } + } + }; + + if vector == VIRTIO_MSI_NO_VECTOR { + return Ok(()); + } + + let config = &mut self.msix_config.lock().unwrap(); + let entry = &config.table_entries[vector as usize]; + // In case the vector control register associated with the entry + // has its first bit set, this means the vector is masked and the + // device should not inject the interrupt. + // Instead, the Pending Bit Array table is updated to reflect there + // is a pending interrupt for this specific vector. + if config.masked() || entry.masked() { + config.set_pba_bit(vector, false); + return Ok(()); + } + + config.irq_routes[vector as usize].irq_fd.write(1) + } +} + impl PciDevice for VirtioPciDevice { fn assign_pin_irq( &mut self, - irq_cb: Arc, - irq_num: u32, - irq_pin: PciInterruptPin, + _irq_cb: Arc, + _irq_num: u32, + _irq_pin: PciInterruptPin, ) { - self.configuration.set_irq(irq_num as u8, irq_pin); - - let interrupt_status = self.interrupt_status.clone(); - let cb = Arc::new(Box::new( - move |int_type: &VirtioInterruptType, _queue: Option<&Queue>| { - let param = InterruptParameters { msix: None }; - - let status = match int_type { - VirtioInterruptType::Config => INTERRUPT_STATUS_CONFIG_CHANGED, - VirtioInterruptType::Queue => INTERRUPT_STATUS_USED_RING, - }; - interrupt_status.fetch_or(status as usize, Ordering::SeqCst); - - (irq_cb)(param) - }, - ) as VirtioInterrupt); - - self.interrupt_cb = Some(cb); } - fn assign_msix(&mut self, msi_cb: Arc) { + fn assign_msix(&mut self) { if let Some(msix_config) = &self.msix_config { - let msix_config_clone = msix_config.clone(); + let virtio_interrupt_msix = Arc::new(VirtioInterruptMsix::new( + msix_config.clone(), + self.common_config.msix_config.clone(), + )); - let common_config_msi_vector = self.common_config.msix_config.clone(); - let cb = Arc::new(Box::new( - move |int_type: &VirtioInterruptType, queue: Option<&Queue>| { - let vector = match int_type { - VirtioInterruptType::Config => { - common_config_msi_vector.load(Ordering::SeqCst) - } - VirtioInterruptType::Queue => { - if let Some(q) = queue { - q.vector - } else { - 0 - } - } - }; - - if vector == VIRTIO_MSI_NO_VECTOR { - return Ok(()); - } - - let config = &mut msix_config_clone.lock().unwrap(); - let entry = &config.table_entries[vector as usize]; - - // If MSI-X interrupts are not enabled for this device, then simply - // ignore the interrupt. - if !config.enabled() { - return Ok(()); - } - - // In case the vector control register associated with the entry - // has its first bit set, this means the vector is masked and the - // device should not inject the interrupt. - // Instead, the Pending Bit Array table is updated to reflect there - // is a pending interrupt for this specific vector. - if config.masked() || entry.masked() { - config.set_pba_bit(vector, false); - return Ok(()); - } - - (msi_cb)(InterruptParameters { msix: Some(entry) }) - }, - ) as VirtioInterrupt); - - self.interrupt_cb = Some(cb); + self.virtio_interrupt = Some(virtio_interrupt_msix); } } @@ -722,14 +714,14 @@ impl PciDevice for VirtioPciDevice { }; if !self.device_activated && self.is_driver_ready() && self.are_queues_valid() { - if let Some(interrupt_cb) = self.interrupt_cb.take() { + if let Some(virtio_interrupt) = self.virtio_interrupt.take() { if self.memory.is_some() { let mem = self.memory.as_ref().unwrap().clone(); let mut device = self.device.lock().unwrap(); device .activate( mem, - interrupt_cb, + virtio_interrupt, self.queues.clone(), self.queue_evts.split_off(0), ) @@ -742,9 +734,9 @@ impl PciDevice for VirtioPciDevice { // Device has been reset by the driver if self.device_activated && self.is_driver_init() { let mut device = self.device.lock().unwrap(); - if let Some((interrupt_cb, mut queue_evts)) = device.reset() { + if let Some((virtio_interrupt, mut queue_evts)) = device.reset() { // Upon reset the device returns its interrupt EventFD and it's queue EventFDs - self.interrupt_cb = Some(interrupt_cb); + self.virtio_interrupt = Some(virtio_interrupt); self.queue_evts.append(&mut queue_evts); self.device_activated = false; diff --git a/vm-virtio/src/vhost_user/blk.rs b/vm-virtio/src/vhost_user/blk.rs index e14894ed9..79a46c9b5 100644 --- a/vm-virtio/src/vhost_user/blk.rs +++ b/vm-virtio/src/vhost_user/blk.rs @@ -46,7 +46,7 @@ pub struct Blk { config_space: Vec, queue_sizes: Vec, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -220,7 +220,7 @@ impl VirtioDevice for Blk { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, queue_evts: Vec, ) -> ActivateResult { @@ -285,7 +285,7 @@ impl VirtioDevice for Blk { Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/vhost_user/fs.rs b/vm-virtio/src/vhost_user/fs.rs index c2b584d0a..1cde98e85 100644 --- a/vm-virtio/src/vhost_user/fs.rs +++ b/vm-virtio/src/vhost_user/fs.rs @@ -161,7 +161,7 @@ pub struct Fs { cache: Option<(VirtioSharedMemoryList, u64)>, slave_req_support: bool, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -333,7 +333,7 @@ impl VirtioDevice for Fs { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, queue_evts: Vec, ) -> ActivateResult { @@ -431,7 +431,7 @@ impl VirtioDevice for Fs { Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/vhost_user/handler.rs b/vm-virtio/src/vhost_user/handler.rs index 50f29ee2f..22a722aed 100644 --- a/vm-virtio/src/vhost_user/handler.rs +++ b/vm-virtio/src/vhost_user/handler.rs @@ -28,7 +28,7 @@ use vhost_rs::vhost_user::{MasterReqHandler, VhostUserMasterReqHandler}; /// * `kill_evt` - EventFd used to kill the vhost-user device. /// * `vu_interrupt_list` - virtqueue and EventFd to signal when buffer used. pub struct VhostUserEpollConfig { - pub interrupt_cb: Arc, + pub interrupt_cb: Arc, pub kill_evt: EventFd, pub pause_evt: EventFd, pub vu_interrupt_list: Vec<(EventFd, Queue)>, @@ -52,7 +52,9 @@ impl VhostUserEpollHandler { } fn signal_used_queue(&self, queue: &Queue) -> Result<()> { - (self.vu_epoll_cfg.interrupt_cb)(&VirtioInterruptType::Queue, Some(queue)) + self.vu_epoll_cfg + .interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(queue)) .map_err(Error::FailedSignalingUsedQueue) } diff --git a/vm-virtio/src/vhost_user/net.rs b/vm-virtio/src/vhost_user/net.rs index 09e0c7443..7e2c26f3b 100644 --- a/vm-virtio/src/vhost_user/net.rs +++ b/vm-virtio/src/vhost_user/net.rs @@ -44,7 +44,7 @@ pub struct Net { config_space: Vec, queue_sizes: Vec, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>>, ctrl_queue_epoll_thread: Option>>, paused: Arc, @@ -227,7 +227,7 @@ impl VirtioDevice for Net { fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, mut queues: Vec, mut queue_evts: Vec, ) -> ActivateResult { @@ -336,7 +336,7 @@ impl VirtioDevice for Net { Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; diff --git a/vm-virtio/src/vsock/device.rs b/vm-virtio/src/vsock/device.rs index 8e40edbf2..80c8584b7 100644 --- a/vm-virtio/src/vsock/device.rs +++ b/vm-virtio/src/vsock/device.rs @@ -91,7 +91,7 @@ pub struct VsockEpollHandler { pub queue_evts: Vec, pub kill_evt: EventFd, pub pause_evt: EventFd, - pub interrupt_cb: Arc, + pub interrupt_cb: Arc, pub backend: Arc>, } @@ -105,10 +105,12 @@ where fn signal_used_queue(&self, queue: &Queue) -> result::Result<(), DeviceError> { debug!("vsock: raising IRQ"); - (self.interrupt_cb)(&VirtioInterruptType::Queue, Some(queue)).map_err(|e| { - error!("Failed to signal used queue: {:?}", e); - DeviceError::FailedSignalingUsedQueue(e) - }) + self.interrupt_cb + .trigger(&VirtioInterruptType::Queue, Some(queue)) + .map_err(|e| { + error!("Failed to signal used queue: {:?}", e); + DeviceError::FailedSignalingUsedQueue(e) + }) } /// Walk the driver-provided RX queue buffers and attempt to fill them up with any data that we @@ -380,7 +382,7 @@ pub struct Vsock { avail_features: u64, acked_features: u64, queue_evts: Option>, - interrupt_cb: Option>, + interrupt_cb: Option>, epoll_thread: Option>>, paused: Arc, } @@ -497,7 +499,7 @@ where fn activate( &mut self, mem: Arc>, - interrupt_cb: Arc, + interrupt_cb: Arc, queues: Vec, queue_evts: Vec, ) -> ActivateResult { @@ -564,7 +566,7 @@ where Ok(()) } - fn reset(&mut self) -> Option<(Arc, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { // We first must resume the virtio thread if it was paused. if self.pause_evt.take().is_some() { self.resume().ok()?; @@ -595,7 +597,7 @@ impl Migratable for Vsock where B: VsockBackend + Sync + 'static {} #[cfg(test)] mod tests { - use super::super::tests::TestContext; + use super::super::tests::{NoopVirtioInterrupt, TestContext}; use super::super::*; use super::*; use crate::vsock::device::{BACKEND_EVENT, EVT_QUEUE_EVENT, RX_QUEUE_EVENT, TX_QUEUE_EVENT}; @@ -665,10 +667,7 @@ mod tests { // Test a bad activation. let bad_activate = ctx.device.activate( Arc::new(ArcSwap::from(Arc::new(ctx.mem.clone()))), - Arc::new( - Box::new(move |_: &VirtioInterruptType, _: Option<&Queue>| Ok(())) - as VirtioInterrupt, - ), + Arc::new(NoopVirtioInterrupt {}), Vec::new(), Vec::new(), ); @@ -681,10 +680,7 @@ mod tests { ctx.device .activate( Arc::new(ArcSwap::new(Arc::new(ctx.mem.clone()))), - Arc::new( - Box::new(move |_: &VirtioInterruptType, _: Option<&Queue>| Ok(())) - as VirtioInterrupt, - ), + Arc::new(NoopVirtioInterrupt {}), vec![Queue::new(256), Queue::new(256), Queue::new(256)], vec![ EventFd::new(EFD_NONBLOCK).unwrap(), diff --git a/vm-virtio/src/vsock/mod.rs b/vm-virtio/src/vsock/mod.rs index e2426af78..3feb2bff9 100644 --- a/vm-virtio/src/vsock/mod.rs +++ b/vm-virtio/src/vsock/mod.rs @@ -172,6 +172,18 @@ mod tests { use vm_memory::{GuestAddress, GuestMemoryMmap}; use vmm_sys_util::eventfd::EventFd; + pub struct NoopVirtioInterrupt {} + + impl VirtioInterrupt for NoopVirtioInterrupt { + fn trigger( + &self, + _int_type: &VirtioInterruptType, + _queue: Option<&Queue>, + ) -> std::result::Result<(), std::io::Error> { + Ok(()) + } + } + pub struct TestBackend { pub evfd: EventFd, pub rx_err: Option, @@ -291,9 +303,7 @@ mod tests { EventFd::new(EFD_NONBLOCK).unwrap(), EventFd::new(EFD_NONBLOCK).unwrap(), ]; - let interrupt_cb = Arc::new(Box::new( - move |_: &VirtioInterruptType, _: Option<&Queue>| Ok(()), - ) as VirtioInterrupt); + let interrupt_cb = Arc::new(NoopVirtioInterrupt {}); EpollHandlerContext { guest_rxvq, diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 3ee6decbe..b22312e74 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -26,11 +26,9 @@ use libc::O_TMPFILE; use libc::TIOCGWINSZ; #[cfg(feature = "pci_support")] use pci::{ - DeviceRelocation, InterruptDelivery, InterruptParameters, PciBarRegionType, PciBus, - PciConfigIo, PciConfigMmio, PciDevice, PciRoot, + DeviceRelocation, PciBarRegionType, PciBus, PciConfigIo, PciConfigMmio, PciDevice, PciRoot, }; use qcow::{self, ImageType, QcowFile}; -use std::cmp; #[cfg(feature = "pci_support")] use std::collections::HashMap; use std::fs::{File, OpenOptions}; @@ -1493,41 +1491,7 @@ impl DeviceManager { .map_err(DeviceManagerError::RegisterIoevent)?; } - let vm_fd_clone = vm_fd.clone(); - - let msi_cb = Arc::new(Box::new(move |p: InterruptParameters| { - if let Some(entry) = p.msix { - use kvm_bindings::kvm_msi; - let msi_queue = kvm_msi { - address_lo: entry.msg_addr_lo, - address_hi: entry.msg_addr_hi, - data: entry.msg_data, - flags: 0u32, - devid: 0u32, - pad: [0u8; 12], - }; - - return vm_fd_clone - .signal_msi(msi_queue) - .map_err(|e| io::Error::from_raw_os_error(e.errno())) - .map(|ret| match ret.cmp(&0) { - cmp::Ordering::Greater => { - debug!("MSI message successfully delivered"); - } - cmp::Ordering::Equal => { - warn!("failed to deliver MSI message, blocked by guest"); - } - _ => {} - }); - } - - Err(std::io::Error::new( - std::io::ErrorKind::Other, - "missing MSI-X entry", - )) - }) as InterruptDelivery); - - virtio_pci_device.assign_msix(msi_cb); + virtio_pci_device.assign_msix(); let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device));