diff --git a/pci/src/lib.rs b/pci/src/lib.rs index 5ab87cf19..17c3ab723 100644 --- a/pci/src/lib.rs +++ b/pci/src/lib.rs @@ -32,7 +32,10 @@ pub use self::device::{ BarReprogrammingParams, DeviceRelocation, Error as PciDeviceError, PciDevice, }; pub use self::msi::{MsiCap, MsiConfig, msi_num_enabled_vectors}; -pub use self::msix::{MSIX_CONFIG_ID, MSIX_TABLE_ENTRY_SIZE, MsixCap, MsixConfig, MsixTableEntry}; +pub use self::msix::{ + MSIX_CONFIG_ID, MSIX_TABLE_ENTRY_SIZE, MaybeMutInterruptSourceGroup, MsixCap, MsixConfig, + MsixTableEntry, +}; pub use self::vfio::{MmioRegion, VfioDmaMapping, VfioPciDevice, VfioPciError}; pub use self::vfio_user::{VfioUserDmaMapping, VfioUserPciDevice, VfioUserPciDeviceError}; diff --git a/pci/src/msix.rs b/pci/src/msix.rs index 9bc5e63f3..49b379b02 100644 --- a/pci/src/msix.rs +++ b/pci/src/msix.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause // -use std::sync::Arc; +use std::sync::{Arc, Mutex}; use std::{io, result}; use byteorder::{ByteOrder, LittleEndian}; @@ -15,6 +15,7 @@ use vm_device::interrupt::{ }; use vm_memory::ByteValued; use vm_migration::{MigratableError, Pausable, Snapshot, Snapshottable}; +use vmm_sys_util::eventfd::EventFd; use crate::{PciCapability, PciCapabilityId}; @@ -72,11 +73,66 @@ pub struct MsixConfigState { enabled: bool, } +#[derive(Clone)] +pub enum MaybeMutInterruptSourceGroup { + Immutable(Arc), + Mutable(Arc>), +} + +macro_rules! impl_method { + ($( + fn $i: ident(&self $(,$index:ident : $InterruptIndex:ty)*$(,)?) -> $r: ty; + )*) => { + $( + fn $i(&self $(,$index: $InterruptIndex)*) -> $r { + match self { + Self::Immutable(source) => source.$i($($index),*), + Self::Mutable(source) => source.lock().unwrap().$i($($index),*), + } + } + )* + }; +} + +impl InterruptSourceGroup for MaybeMutInterruptSourceGroup { + impl_method! { + fn trigger(&self, index: InterruptIndex) -> vm_device::interrupt::Result<()>; + + fn notifier(&self, index: InterruptIndex) -> Option; + + fn update( + &self, + index: InterruptIndex, + config: InterruptSourceConfig, + masked: bool, + set_gsi: bool, + ) -> vm_device::interrupt::Result<()>; + + fn set_gsi(&self) -> vm_device::interrupt::Result<()>; + } +} + +impl MaybeMutInterruptSourceGroup { + pub fn set_notifier( + &self, + index: InterruptIndex, + eventfd: Option, + vm: &dyn hypervisor::Vm, + ) -> std::io::Result<()> { + match self { + Self::Immutable(_) => panic!( + "Attempted to set a notifier of an immutable source. You must mark your device as needing a mutable source by having sets_irqfd() return true." + ), + Self::Mutable(source) => source.lock().unwrap().set_notifier(index, eventfd, vm), + } + } +} + pub struct MsixConfig { pub table_entries: Vec, pub pba_entries: Vec, pub devid: u32, - interrupt_source_group: Arc, + interrupt_source_group: MaybeMutInterruptSourceGroup, masked: bool, enabled: bool, } @@ -84,7 +140,7 @@ pub struct MsixConfig { impl MsixConfig { pub fn new( msix_vectors: u16, - interrupt_source_group: Arc, + interrupt_source_group: MaybeMutInterruptSourceGroup, devid: u32, state: Option, ) -> result::Result { diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 9e8e7e316..e46e276aa 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -37,7 +37,7 @@ use vmm_sys_util::eventfd::EventFd; use crate::mmap::MmapRegion; use crate::msi::{MSI_CONFIG_ID, MsiConfigState}; -use crate::msix::MsixConfigState; +use crate::msix::{MaybeMutInterruptSourceGroup, MsixConfigState}; use crate::{ BarReprogrammingParams, MSIX_CONFIG_ID, MSIX_TABLE_ENTRY_SIZE, MsiCap, MsiConfig, MsixCap, MsixConfig, PCI_CONFIGURATION_ID, PciBarConfiguration, PciBarPrefetchable, PciBarRegionType, @@ -863,7 +863,7 @@ impl VfioCommon { let msix_config = MsixConfig::new( msix_cap.table_size(), - interrupt_source_group.clone(), + MaybeMutInterruptSourceGroup::Immutable(interrupt_source_group.clone()), bdf.into(), state, ) diff --git a/virtio-devices/src/device.rs b/virtio-devices/src/device.rs index 91b742a0b..f0673f561 100644 --- a/virtio-devices/src/device.rs +++ b/virtio-devices/src/device.rs @@ -80,6 +80,18 @@ pub trait VirtioDevice: Send { /// The maximum size of each queue that this device supports. fn queue_max_sizes(&self) -> &[u16]; + /// Whether the device needs to register extra irqfds at runtime + /// from external sources. + /// The default is false. If this is true, locking is required for + /// most operations involving interrupts (but not for sending) + /// interrupts from external irqfds). + /// + /// If the device claims to not need to register irqfds, but + /// attempts to do so, a panic will ensue. + fn interrupt_source_mutable(&self) -> bool { + false + } + /// The set of feature bits that this device supports. fn features(&self) -> u64 { 0 diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 70e03d028..3e2a96ccd 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -17,9 +17,10 @@ use anyhow::anyhow; use libc::EFD_NONBLOCK; use log::{error, info}; use pci::{ - BarReprogrammingParams, MsixCap, MsixConfig, PciBarConfiguration, PciBarRegionType, - PciCapability, PciCapabilityId, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, - PciHeaderType, PciMassStorageSubclass, PciNetworkControllerSubclass, PciSubclass, + BarReprogrammingParams, MaybeMutInterruptSourceGroup, MsixCap, MsixConfig, PciBarConfiguration, + PciBarRegionType, PciCapability, PciCapabilityId, PciClassCode, PciConfiguration, PciDevice, + PciDeviceError, PciHeaderType, PciMassStorageSubclass, PciNetworkControllerSubclass, + PciSubclass, }; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -359,7 +360,7 @@ pub struct VirtioPciDevice { // PCI interrupts. interrupt_status: Arc, virtio_interrupt: Option>, - interrupt_source_group: Arc, + interrupt_source_group: MaybeMutInterruptSourceGroup, // virtio queues queues: Vec, @@ -433,17 +434,26 @@ impl VirtioPciDevice { let pci_device_id = VIRTIO_PCI_DEVICE_ID_BASE + locked_device.device_type() as u16; - let interrupt_source_group = interrupt_manager - .create_group(MsiIrqGroupConfig { + let interrupt_source_group: MaybeMutInterruptSourceGroup = { + let config = MsiIrqGroupConfig { base: 0, count: msix_num as InterruptIndex, + }; + (if locked_device.interrupt_source_mutable() { + interrupt_manager + .create_group_mut(config) + .map(MaybeMutInterruptSourceGroup::Mutable) + } else { + interrupt_manager + .create_group(config) + .map(MaybeMutInterruptSourceGroup::Immutable) }) .map_err(|e| { VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( "Failed creating MSI interrupt group: {e}" )) - })?; - + })? + }; let msix_state = vm_migration::state_from_id(snapshot, pci::MSIX_CONFIG_ID).map_err(|e| { VirtioPciDeviceError::CreateVirtioPciDevice(anyhow!( @@ -452,14 +462,11 @@ impl VirtioPciDevice { })?; let (msix_config, msix_config_clone) = if msix_num > 0 { + let interrupt_source_group: MaybeMutInterruptSourceGroup = + interrupt_source_group.clone(); let msix_config = Arc::new(Mutex::new( - MsixConfig::new( - msix_num, - interrupt_source_group.clone(), - pci_device_bdf, - msix_state, - ) - .unwrap(), + MsixConfig::new(msix_num, interrupt_source_group, pci_device_bdf, msix_state) + .unwrap(), )); let msix_config_clone = msix_config.clone(); (Some(msix_config), Some(msix_config_clone)) @@ -598,7 +605,7 @@ impl VirtioPciDevice { memory, settings_bar: 0, use_64bit_bar, - interrupt_source_group, + interrupt_source_group: interrupt_source_group.clone(), cap_pci_cfg_info, bar_regions: vec![], activate_evt, @@ -855,7 +862,7 @@ pub struct VirtioInterruptMsix { msix_config: Arc>, config_vector: Arc, queues_vectors: Arc>>, - interrupt_source_group: Arc, + interrupt_source_group: MaybeMutInterruptSourceGroup, } impl VirtioInterruptMsix { @@ -863,7 +870,7 @@ impl VirtioInterruptMsix { msix_config: Arc>, config_vector: Arc, queues_vectors: Arc>>, - interrupt_source_group: Arc, + interrupt_source_group: MaybeMutInterruptSourceGroup, ) -> Self { VirtioInterruptMsix { msix_config, @@ -917,11 +924,12 @@ impl VirtioInterrupt for VirtioInterruptMsix { fn set_notifier( &self, - _interrupt: u32, - _eventfd: Option, - _vm: &dyn hypervisor::Vm, + interrupt: u32, + eventfd: Option, + vm: &dyn hypervisor::Vm, ) -> std::io::Result<()> { - unimplemented!() + self.interrupt_source_group + .set_notifier(interrupt, eventfd, vm) } } diff --git a/vm-device/src/interrupt/mod.rs b/vm-device/src/interrupt/mod.rs index 342cbe063..e9b0180d2 100644 --- a/vm-device/src/interrupt/mod.rs +++ b/vm-device/src/interrupt/mod.rs @@ -57,7 +57,8 @@ //! * The virtual device backend requests the interrupt manager to create an interrupt group //! according to guest configuration information -use std::sync::Arc; +use std::io::{Error, ErrorKind}; +use std::sync::{Arc, Mutex}; pub use hypervisor::{InterruptSourceConfig, LegacyIrqSourceConfig, MsiIrqSourceConfig}; use vmm_sys_util::eventfd::EventFd; @@ -107,6 +108,30 @@ pub trait InterruptManager: Send + Sync { /// * count: number of Interrupt Sources to be managed by the group object. fn create_group(&self, config: Self::GroupConfig) -> Result>; + /// Create an [InterruptSourceGroup](trait.InterruptSourceGroup.html) object to manage + /// interrupt sources for a virtual device + /// + /// An [InterruptSourceGroup](trait.InterruptSourceGroup.html) object manages all interrupt + /// sources of the same type for a virtual device. + /// + /// This is the same as [`Self::create_group`], except that the returned + /// [`InterruptSourceGroup`] allows setting the irqfd used as notifier via + /// [`InterruptSourceGroup::set_notifier`]. + /// + /// # Arguments + /// * interrupt_type: type of interrupt source. + /// * base: base Interrupt Source ID to be managed by the group object. + /// * count: number of Interrupt Sources to be managed by the group object. + fn create_group_mut( + &self, + _config: Self::GroupConfig, + ) -> Result>> { + Err(Error::new( + ErrorKind::Unsupported, + "setting notifiers not supported", + )) + } + /// Destroy an [InterruptSourceGroup](trait.InterruptSourceGroup.html) object created by /// [create_group()](trait.InterruptManager.html#tymethod.create_group). /// @@ -137,7 +162,7 @@ pub trait InterruptSourceGroup: Send + Sync { /// Returns an interrupt notifier from this interrupt. /// /// An interrupt notifier allows for external components and processes - /// to inject interrupts into a guest, by writing to the file returned + /// to inject interrupts into a guest, by writing to the [`EventFd`] returned /// by this method. #[allow(unused_variables)] fn notifier(&self, index: InterruptIndex) -> Option; @@ -159,4 +184,17 @@ pub trait InterruptSourceGroup: Send + Sync { /// Set the interrupt group GSI routing table. fn set_gsi(&self) -> Result<()>; + + /// Sets the [`EventFd`] used to trigger interrupts. + fn set_notifier( + &mut self, + _index: InterruptIndex, + _eventfd: Option, + _vm: &dyn hypervisor::Vm, + ) -> Result<()> { + Err(Error::new( + ErrorKind::Unsupported, + "setting notifiers not supported", + )) + } } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index e560b02d7..958d3086b 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -4201,8 +4201,8 @@ impl DeviceManager { return Err(DeviceManagerError::MissingNode); } - // Allows support for one MSI-X vector per queue. It also adds 1 - // as we need to take into account the dedicated vector to notify + // Allows support for one MSI-X vector per interrupt needed by the device. + // It also adds 1 as we need to take into account the dedicated vector to notify // about a virtio config change. let msix_num = (virtio_device.lock().unwrap().queue_max_sizes().len() + 1) as u16; diff --git a/vmm/src/interrupt.rs b/vmm/src/interrupt.rs index 07727d1ad..0995d8356 100644 --- a/vmm/src/interrupt.rs +++ b/vmm/src/interrupt.rs @@ -86,6 +86,9 @@ impl InterruptRoute { ) } + // This is currently not used, but the upcoming vhost-guest feature + // will use it. Use #[allow(dead_code)] to suppress a compiler + // warning. #[allow(dead_code)] pub fn set_notifier( &mut self, @@ -96,7 +99,7 @@ impl InterruptRoute { if self.registered { if let Some(ref irq_fd) = self.irq_fd { vm.register_irqfd(irq_fd, self.gsi) - .map_err(|e| io::Error::other(format!("Failed registering irq_fd: {e}")))? + .map_err(|e| io::Error::other(format!("Failed registering irq_fd: {e}")))?; } // If the irqfd cannot be unregistered, what to do? Spin? // Returning an error isn't helpful as the new irqfd is already registered. @@ -235,6 +238,19 @@ impl InterruptSourceGroup for MsiInterruptGroup { let routes = self.gsi_msi_routes.lock().unwrap(); self.set_gsi_routes(&routes) } + + fn set_notifier( + &mut self, + index: InterruptIndex, + eventfd: Option, + vm: &dyn hypervisor::Vm, + ) -> Result<()> { + if let Some(route) = self.irq_routes.get(&index) { + return route.lock().unwrap().set_notifier(eventfd, vm); + } + + Ok(()) + } } pub struct LegacyUserspaceInterruptGroup { @@ -323,6 +339,26 @@ impl InterruptManager for LegacyUserspaceInterruptManager { } } +impl MsiInterruptManager { + fn create_group_raw( + &self, + config: ::GroupConfig, + ) -> Result { + let mut allocator = self.allocator.lock().unwrap(); + let mut irq_routes: HashMap> = + HashMap::with_capacity(config.count as usize); + for i in config.base..config.base + config.count { + irq_routes.insert(i, Mutex::new(InterruptRoute::new(&mut allocator)?)); + } + + Ok(MsiInterruptGroup::new( + self.vm.clone(), + self.gsi_msi_routes.clone(), + irq_routes, + )) + } +} + impl InterruptManager for MsiInterruptManager { type GroupConfig = MsiIrqGroupConfig; @@ -341,6 +377,14 @@ impl InterruptManager for MsiInterruptManager { ))) } + fn create_group_mut( + &self, + config: Self::GroupConfig, + ) -> vm_device::interrupt::Result>> { + let r = self.create_group_raw(config)?; + Ok(Arc::new(Mutex::new(r))) + } + fn destroy_group(&self, _group: Arc) -> Result<()> { Ok(()) }