From d3c7b45542503f619fc5b675576a8c8734739cab Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Mon, 3 Jun 2019 13:57:26 -0700 Subject: [PATCH] interrupt: Make IRQ delivery generic Because we cannot always assume the irq fd will be the way to send an IRQ to the guest, this means we cannot make the assumption that every virtio device implementation should expect an EventFd to trigger an IRQ. This commit organizes the code related to virtio devices so that it now expects a Rust closure instead of a known EventFd. This lets the caller decide what should be done whenever a device needs to trigger an interrupt to the guest. The closure will allow for other type of interrupt mechanism such as MSI to be implemented. From the device perspective, it could be a pin based interrupt or an MSI, it does not matter since the device will simply call into the provided callback, passing the appropriate Queue as a reference. This design keeps the device model generic. Signed-off-by: Sebastien Boeuf --- pci/src/device.rs | 5 +++- pci/src/lib.rs | 2 +- vm-virtio/src/block.rs | 33 ++++++++++---------------- vm-virtio/src/device.rs | 6 +++-- vm-virtio/src/net.rs | 17 +++++++------- vm-virtio/src/rng.rs | 10 ++++---- vm-virtio/src/transport/pci_device.rs | 34 +++++++++++++-------------- vmm/src/vm.rs | 8 +++++-- 8 files changed, 58 insertions(+), 57 deletions(-) diff --git a/pci/src/device.rs b/pci/src/device.rs index f32db4d55..b84f5a44d 100755 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -7,10 +7,13 @@ use crate::PciInterruptPin; use devices::BusDevice; use std; use std::fmt::{self, Display}; +use std::sync::Arc; use vm_allocator::SystemAllocator; use vm_memory::{GuestAddress, GuestUsize}; use vmm_sys_util::EventFd; +pub type IrqClosure = Box std::result::Result<(), std::io::Error> + Send + Sync>; + #[derive(Debug)] pub enum Error { /// Setup of the device capabilities failed. @@ -41,7 +44,7 @@ impl Display for Error { pub trait PciDevice: BusDevice { /// Assign a legacy PCI IRQ to this device. /// The device may write to `irq_evt` to trigger an interrupt. - fn assign_irq(&mut self, _irq_evt: EventFd, _irq_num: u32, _irq_pin: PciInterruptPin) {} + fn assign_irq(&mut self, _irq_cb: Arc, _irq_num: u32, _irq_pin: PciInterruptPin) {} /// 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/pci/src/lib.rs b/pci/src/lib.rs index d90b11b7d..b5eb07797 100644 --- a/pci/src/lib.rs +++ b/pci/src/lib.rs @@ -20,7 +20,7 @@ pub use self::configuration::{ PciSubclass, }; pub use self::device::Error as PciDeviceError; -pub use self::device::PciDevice; +pub use self::device::{IrqClosure, PciDevice}; pub use self::root::{PciConfigIo, PciConfigMmio, PciRoot, PciRootError}; /// PCI has four interrupt pins A->D. diff --git a/vm-virtio/src/block.rs b/vm-virtio/src/block.rs index 681750166..084889fd6 100755 --- a/vm-virtio/src/block.rs +++ b/vm-virtio/src/block.rs @@ -26,6 +26,7 @@ use super::{ ActivateError, ActivateResult, DescriptorChain, DeviceEventT, Queue, VirtioDevice, VirtioDeviceType, INTERRUPT_STATUS_USED_RING, }; +use crate::VirtioInterrupt; use virtio_bindings::virtio_blk::*; use vm_memory::{Bytes, GuestAddress, GuestMemory, GuestMemoryError, GuestMemoryMmap}; use vmm_sys_util::EventFd; @@ -325,7 +326,7 @@ struct BlockEpollHandler { disk_image: T, disk_nsectors: u64, interrupt_status: Arc, - interrupt_evt: EventFd, + interrupt_cb: Arc, disk_image_id: Vec, } @@ -374,10 +375,10 @@ impl BlockEpollHandler { used_count > 0 } - fn signal_used_queue(&self) -> result::Result<(), DeviceError> { + fn signal_used_queue(&self, queue_index: usize) -> result::Result<(), DeviceError> { self.interrupt_status .fetch_or(INTERRUPT_STATUS_USED_RING as usize, Ordering::SeqCst); - self.interrupt_evt.write(1).map_err(|e| { + (self.interrupt_cb)(&self.queues[queue_index]).map_err(|e| { error!("Failed to signal used queue: {:?}", e); DeviceError::FailedSignalingUsedQueue(e) }) @@ -435,7 +436,7 @@ impl BlockEpollHandler { error!("Failed to get queue event: {:?}", e); break 'epoll; } else if self.process_queue(0) { - if let Err(e) = self.signal_used_queue() { + if let Err(e) = self.signal_used_queue(0) { error!("Failed to signal used queue: {:?}", e); break 'epoll; } @@ -466,7 +467,7 @@ pub struct Block { acked_features: u64, config_space: Vec, queue_evt: Option, - interrupt_evt: Option, + interrupt_cb: Option>, } pub fn build_config_space(disk_size: u64) -> Vec { @@ -514,7 +515,7 @@ impl Block { acked_features: 0u64, config_space: build_config_space(disk_size), queue_evt: None, - interrupt_evt: None, + interrupt_cb: None, }) } } @@ -598,7 +599,7 @@ impl VirtioDevice for Block { fn activate( &mut self, mem: GuestMemoryMmap, - interrupt_evt: EventFd, + interrupt_cb: Arc, status: Arc, queues: Vec, mut queue_evts: Vec, @@ -627,16 +628,8 @@ impl VirtioDevice for Block { // Save the interrupt EventFD as we need to return it on reset // but clone it to pass into the thread. - self.interrupt_evt = Some(interrupt_evt); - let interrupt_evt = self - .interrupt_evt - .as_ref() - .unwrap() - .try_clone() - .map_err(|e| { - error!("failed to clone interrupt EventFd: {}", e); - ActivateError::BadActivate - })?; + self.interrupt_cb = Some(interrupt_cb); + let interrupt_cb = self.interrupt_cb.as_ref().unwrap().clone(); // Save the queue EventFD as we need to return it on reset // but clone it to pass into the thread. @@ -652,7 +645,7 @@ impl VirtioDevice for Block { disk_image, disk_nsectors: self.disk_nsectors, interrupt_status: status, - interrupt_evt, + interrupt_cb, disk_image_id, }; @@ -670,7 +663,7 @@ impl VirtioDevice for Block { Err(ActivateError::BadActivate) } - fn reset(&mut self) -> Option<(EventFd, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { if let Some(kill_evt) = self.kill_evt.take() { // Ignore the result because there is nothing we can do about it. let _ = kill_evt.write(1); @@ -678,7 +671,7 @@ impl VirtioDevice for Block { // Return the interrupt and queue EventFDs Some(( - self.interrupt_evt.take().unwrap(), + self.interrupt_cb.take().unwrap(), vec![self.queue_evt.take().unwrap()], )) } diff --git a/vm-virtio/src/device.rs b/vm-virtio/src/device.rs index 52ca6a5ef..5fcb4d318 100644 --- a/vm-virtio/src/device.rs +++ b/vm-virtio/src/device.rs @@ -13,6 +13,8 @@ use std::sync::Arc; use vm_memory::GuestMemoryMmap; use vmm_sys_util::EventFd; +pub type VirtioInterrupt = Box std::result::Result<(), std::io::Error> + Send + Sync>; + /// Trait for virtio devices to be driven by a virtio transport. /// /// The lifecycle of a virtio device is to be moved to a virtio transport, which will then query the @@ -46,7 +48,7 @@ pub trait VirtioDevice: Send { fn activate( &mut self, mem: GuestMemoryMmap, - interrupt_evt: EventFd, + interrupt_evt: Arc, status: Arc, queues: Vec, queue_evts: Vec, @@ -54,7 +56,7 @@ pub trait VirtioDevice: Send { /// Optionally deactivates this device and returns ownership of the guest memory map, interrupt /// event, and queue events. - fn reset(&mut self) -> Option<(EventFd, Vec)> { + fn reset(&mut self) -> Option<(Arc, Vec)> { None } diff --git a/vm-virtio/src/net.rs b/vm-virtio/src/net.rs index 799b9607b..b704b0edf 100644 --- a/vm-virtio/src/net.rs +++ b/vm-virtio/src/net.rs @@ -28,6 +28,7 @@ use super::{ ActivateError, ActivateResult, DeviceEventT, Queue, VirtioDevice, VirtioDeviceType, INTERRUPT_STATUS_USED_RING, }; +use crate::VirtioInterrupt; use net_util::{MacAddr, Tap, TapError, MAC_ADDR_LEN}; use virtio_bindings::virtio_net::*; use vm_memory::{Bytes, GuestAddress, GuestMemoryMmap}; @@ -121,15 +122,15 @@ struct NetEpollHandler { rx: RxVirtio, tx: TxVirtio, interrupt_status: Arc, - interrupt_evt: EventFd, + interrupt_cb: Arc, kill_evt: EventFd, } impl NetEpollHandler { - fn signal_used_queue(&self) -> result::Result<(), DeviceError> { + fn signal_used_queue(&self, queue: &Queue) -> result::Result<(), DeviceError> { self.interrupt_status .fetch_or(INTERRUPT_STATUS_USED_RING as usize, Ordering::SeqCst); - self.interrupt_evt.write(1).map_err(|e| { + (self.interrupt_cb)(queue).map_err(|e| { error!("Failed to signal used queue: {:?}", e); DeviceError::FailedSignalingUsedQueue(e) }) @@ -219,7 +220,7 @@ impl NetEpollHandler { } if self.rx.deferred_irqs { self.rx.deferred_irqs = false; - self.signal_used_queue() + self.signal_used_queue(&self.rx.queue) } else { Ok(()) } @@ -234,7 +235,7 @@ impl NetEpollHandler { self.process_rx() } else if self.rx.deferred_irqs { self.rx.deferred_irqs = false; - self.signal_used_queue() + self.signal_used_queue(&self.rx.queue) } else { Ok(()) } @@ -374,7 +375,7 @@ impl NetEpollHandler { self.process_rx().unwrap(); } else if self.rx.deferred_irqs { self.rx.deferred_irqs = false; - self.signal_used_queue().unwrap(); + self.signal_used_queue(&self.rx.queue).unwrap(); } } else { self.process_rx().unwrap(); @@ -538,7 +539,7 @@ impl VirtioDevice for Net { fn activate( &mut self, mem: GuestMemoryMmap, - interrupt_evt: EventFd, + interrupt_cb: Arc, status: Arc, mut queues: Vec, mut queue_evts: Vec, @@ -573,7 +574,7 @@ impl VirtioDevice for Net { rx: RxVirtio::new(rx_queue, rx_queue_evt), tx: TxVirtio::new(tx_queue, tx_queue_evt), interrupt_status: status, - interrupt_evt, + interrupt_cb, kill_evt, }; diff --git a/vm-virtio/src/rng.rs b/vm-virtio/src/rng.rs index fa6a9e20d..78a9f451c 100755 --- a/vm-virtio/src/rng.rs +++ b/vm-virtio/src/rng.rs @@ -18,7 +18,7 @@ use super::{ ActivateError, ActivateResult, DeviceEventT, Queue, VirtioDevice, VirtioDeviceType, INTERRUPT_STATUS_USED_RING, VIRTIO_F_VERSION_1, }; - +use crate::VirtioInterrupt; use vm_memory::{Bytes, GuestMemoryMmap}; use vmm_sys_util::EventFd; @@ -36,7 +36,7 @@ struct RngEpollHandler { mem: GuestMemoryMmap, random_file: File, interrupt_status: Arc, - interrupt_evt: EventFd, + interrupt_cb: Arc, queue_evt: EventFd, kill_evt: EventFd, } @@ -79,7 +79,7 @@ impl RngEpollHandler { fn signal_used_queue(&self) -> result::Result<(), DeviceError> { self.interrupt_status .fetch_or(INTERRUPT_STATUS_USED_RING as usize, Ordering::SeqCst); - self.interrupt_evt.write(1).map_err(|e| { + (self.interrupt_cb)(&self.queues[0]).map_err(|e| { error!("Failed to signal used queue: {:?}", e); DeviceError::FailedSignalingUsedQueue(e) }) @@ -228,7 +228,7 @@ impl VirtioDevice for Rng { fn activate( &mut self, mem: GuestMemoryMmap, - interrupt_evt: EventFd, + interrupt_cb: Arc, status: Arc, queues: Vec, mut queue_evts: Vec, @@ -258,7 +258,7 @@ impl VirtioDevice for Rng { mem, random_file, interrupt_status: status, - interrupt_evt, + interrupt_cb, queue_evt: queue_evts.remove(0), kill_evt, }; diff --git a/vm-virtio/src/transport/pci_device.rs b/vm-virtio/src/transport/pci_device.rs index 899e4824c..6c2187d4a 100755 --- a/vm-virtio/src/transport/pci_device.rs +++ b/vm-virtio/src/transport/pci_device.rs @@ -19,8 +19,8 @@ use std::sync::Arc; use devices::BusDevice; use pci::{ - PciBarConfiguration, PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice, - PciDeviceError, PciHeaderType, PciInterruptPin, PciSubclass, + IrqClosure, PciBarConfiguration, PciCapability, PciCapabilityID, PciClassCode, + PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciInterruptPin, PciSubclass, }; use vm_allocator::SystemAllocator; use vm_memory::{Address, ByteValued, GuestAddress, GuestMemoryMmap, GuestUsize, Le32}; @@ -28,8 +28,8 @@ use vmm_sys_util::{EventFd, Result}; use super::VirtioPciCommonConfig; use crate::{ - Queue, VirtioDevice, DEVICE_ACKNOWLEDGE, DEVICE_DRIVER, DEVICE_DRIVER_OK, DEVICE_FAILED, - DEVICE_FEATURES_OK, DEVICE_INIT, + Queue, VirtioDevice, VirtioInterrupt, DEVICE_ACKNOWLEDGE, DEVICE_DRIVER, DEVICE_DRIVER_OK, + DEVICE_FAILED, DEVICE_FEATURES_OK, DEVICE_INIT, }; #[allow(clippy::enum_variant_names)] @@ -163,7 +163,7 @@ pub struct VirtioPciDevice { // PCI interrupts. interrupt_status: Arc, - interrupt_evt: Option, + interrupt_cb: Option>, // virtio queues queues: Vec, @@ -214,7 +214,7 @@ impl VirtioPciDevice { device, device_activated: false, interrupt_status: Arc::new(AtomicUsize::new(0)), - interrupt_evt: None, + interrupt_cb: None, queues, queue_evts, memory: Some(memory), @@ -229,11 +229,6 @@ impl VirtioPciDevice { self.queue_evts.as_slice() } - /// Gets the event this device uses to interrupt the VM when the used queue is changed. - pub fn interrupt_evt(&self) -> Option<&EventFd> { - self.interrupt_evt.as_ref() - } - fn is_driver_ready(&self) -> bool { let ready_bits = (DEVICE_ACKNOWLEDGE | DEVICE_DRIVER | DEVICE_DRIVER_OK | DEVICE_FEATURES_OK) as u8; @@ -313,9 +308,12 @@ impl VirtioPciDevice { } impl PciDevice for VirtioPciDevice { - fn assign_irq(&mut self, irq_evt: EventFd, irq_num: u32, irq_pin: PciInterruptPin) { + fn assign_irq(&mut self, irq_cb: Arc, irq_num: u32, irq_pin: PciInterruptPin) { self.configuration.set_irq(irq_num as u8, irq_pin); - self.interrupt_evt = Some(irq_evt); + + let cb = Arc::new(Box::new(move |_queue: &Queue| (irq_cb)()) as VirtioInterrupt); + + self.interrupt_cb = Some(cb); } fn config_registers(&self) -> &PciConfiguration { @@ -440,18 +438,18 @@ impl PciDevice for VirtioPciDevice { }; if !self.device_activated && self.is_driver_ready() && self.are_queues_valid() { - if let Some(interrupt_evt) = self.interrupt_evt.take() { + if let Some(interrupt_cb) = self.interrupt_cb.take() { if self.memory.is_some() { let mem = self.memory.as_ref().unwrap().clone(); self.device .activate( mem, - interrupt_evt, + interrupt_cb, self.interrupt_status.clone(), self.queues.clone(), self.queue_evts.split_off(0), ) - .expect("Failed to activate device");; + .expect("Failed to activate device"); self.device_activated = true; } } @@ -459,9 +457,9 @@ impl PciDevice for VirtioPciDevice { // Device has been reset by the driver if self.device_activated && self.is_driver_init() { - if let Some((interrupt_evt, mut queue_evts)) = self.device.reset() { + if let Some((interrupt_cb, mut queue_evts)) = self.device.reset() { // Upon reset the device returns its interrupt EventFD and it's queue EventFDs - self.interrupt_evt = Some(interrupt_evt); + self.interrupt_cb = Some(interrupt_cb); self.queue_evts.append(&mut queue_evts); self.device_activated = false; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 34f19e880..cc7d96709 100755 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -27,7 +27,7 @@ use kvm_ioctls::*; use libc::{c_void, siginfo_t, EFD_NONBLOCK}; use linux_loader::loader::KernelLoader; use net_util::Tap; -use pci::{PciConfigIo, PciDevice, PciInterruptPin, PciRoot}; +use pci::{IrqClosure, PciConfigIo, PciDevice, PciInterruptPin, PciRoot}; use qcow::{self, ImageType, QcowFile}; use std::ffi::CString; use std::fs::{File, OpenOptions}; @@ -423,6 +423,7 @@ impl DeviceManager { ) -> DeviceManagerResult<()> { let mut virtio_pci_device = VirtioPciDevice::new(memory, virtio_device) .map_err(DeviceManagerError::VirtioDevice)?; + let bars = virtio_pci_device .allocate_bars(allocator) .map_err(DeviceManagerError::AllocateBars)?; @@ -442,8 +443,11 @@ impl DeviceManager { vm_fd .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's use irq line INTA for now. - virtio_pci_device.assign_irq(irqfd, irq_num as u32, PciInterruptPin::IntA); + virtio_pci_device.assign_irq(irq_cb, irq_num as u32, PciInterruptPin::IntA); let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device));