diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 6bd41ddb5..6a95a51d3 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -13,7 +13,9 @@ extern crate vm_device; use crate::config::ConsoleOutputMode; use crate::config::VmConfig; -use crate::interrupt::{KvmInterruptManager, KvmRoutingEntry}; +use crate::interrupt::{ + KvmLegacyUserspaceInterruptManager, KvmMsiInterruptManager, KvmRoutingEntry, +}; use crate::memory_manager::{Error as MemoryManagerError, MemoryManager}; #[cfg(feature = "acpi")] use acpi_tables::{aml, aml::Aml}; @@ -428,38 +430,26 @@ impl DeviceManager { let kvm_gsi_msi_routes: Arc>> = Arc::new(Mutex::new(HashMap::new())); - // Here we create a first interrupt manager that will be directly - // passed down to the Ioapic. The reason we need this extra interrupt - // manager is because the more global one will need a handler onto the - // Ioapic itself. We didn't want to solve this problem by adding some - // setter to the KvmInterruptManager as this would have required the - // interrupt manager to be mutable. - // - // One thing to note, it is safe to create two interrupt managers - // without risking some concurrency between the two since the list - // of GSI routes is shared and protected by a Mutex. - let ioapic_interrupt_manager: Arc = - Arc::new(KvmInterruptManager::new( + // First we create the MSI interrupt manager, the legacy one is created + // later, after the IOAPIC device creation. + // The reason we create the MSI one first is because the IOAPIC needs it, + // and then the legacy interrupt manager needs an IOAPIC. So we're + // handling a linear dependency chain: + // msi_interrupt_manager <- IOAPIC <- legacy_interrupt_manager. + let msi_interrupt_manager: Arc = + Arc::new(KvmMsiInterruptManager::new( Arc::clone(&address_manager.allocator), - vm_fd.clone(), + vm_fd, Arc::clone(&kvm_gsi_msi_routes), - None, )); - let ioapic = DeviceManager::add_ioapic(&address_manager, ioapic_interrupt_manager)?; + let ioapic = + DeviceManager::add_ioapic(&address_manager, Arc::clone(&msi_interrupt_manager))?; - // Creation of the global interrupt manager, which can take a hold onto - // the brand new Ioapic. - // - // Note the list of GSI routes is Arc cloned, the same way it was Arc - // cloned for the interrupt manager dedicated to the Ioapic. That's how - // both interrupt managers are going to share the list correctly. - let interrupt_manager: Arc = Arc::new(KvmInterruptManager::new( - Arc::clone(&address_manager.allocator), - vm_fd, - Arc::clone(&kvm_gsi_msi_routes), - Some(ioapic.clone()), - )); + // Now we can create the legacy interrupt manager, which needs the freshly + // formed IOAPIC device. + let legacy_interrupt_manager: Arc = + Arc::new(KvmLegacyUserspaceInterruptManager::new(ioapic.clone())); #[cfg(feature = "acpi")] address_manager @@ -494,22 +484,22 @@ impl DeviceManager { #[cfg(feature = "acpi")] { device_manager.ged_notification_device = device_manager.add_acpi_devices( - &interrupt_manager, + &legacy_interrupt_manager, reset_evt.try_clone().map_err(DeviceManagerError::EventFd)?, _exit_evt.try_clone().map_err(DeviceManagerError::EventFd)?, )?; } device_manager.console = - device_manager.add_console_device(&interrupt_manager, &mut virtio_devices)?; + device_manager.add_console_device(&legacy_interrupt_manager, &mut virtio_devices)?; #[cfg(any(feature = "pci_support", feature = "mmio_support"))] virtio_devices.append(&mut device_manager.make_virtio_devices()?); if cfg!(feature = "pci_support") { - device_manager.add_pci_devices(virtio_devices, &interrupt_manager)?; + device_manager.add_pci_devices(virtio_devices, &msi_interrupt_manager)?; } else if cfg!(feature = "mmio_support") { - device_manager.add_mmio_devices(virtio_devices, &interrupt_manager)?; + device_manager.add_mmio_devices(virtio_devices, &legacy_interrupt_manager)?; } Ok(device_manager) diff --git a/vmm/src/interrupt.rs b/vmm/src/interrupt.rs index 115277b02..e8a690900 100644 --- a/vmm/src/interrupt.rs +++ b/vmm/src/interrupt.rs @@ -272,30 +272,37 @@ impl InterruptSourceGroup for LegacyUserspaceInterruptGroup { } } -pub struct KvmInterruptManager { +pub struct KvmLegacyUserspaceInterruptManager { + ioapic: Arc>, +} + +pub struct KvmMsiInterruptManager { allocator: Arc>, vm_fd: Arc, gsi_msi_routes: Arc>>, - ioapic: Option>>, } -impl KvmInterruptManager { +impl KvmLegacyUserspaceInterruptManager { + pub fn new(ioapic: Arc>) -> Self { + KvmLegacyUserspaceInterruptManager { ioapic } + } +} + +impl KvmMsiInterruptManager { pub fn new( allocator: Arc>, vm_fd: Arc, gsi_msi_routes: Arc>>, - ioapic: Option>>, ) -> Self { - KvmInterruptManager { + KvmMsiInterruptManager { allocator, vm_fd, gsi_msi_routes, - ioapic, } } } -impl InterruptManager for KvmInterruptManager { +impl InterruptManager for KvmLegacyUserspaceInterruptManager { fn create_group( &self, interrupt_type: InterruptType, @@ -311,18 +318,35 @@ impl InterruptManager for KvmInterruptManager { )); } - if let Some(ioapic) = &self.ioapic { - Arc::new(Box::new(LegacyUserspaceInterruptGroup::new( - ioapic.clone(), - base as u32, - ))) - } else { - return Err(io::Error::new( - io::ErrorKind::Other, - "No IOAPIC configured, cannot create legacy interrupt group", - )); - } + Arc::new(Box::new(LegacyUserspaceInterruptGroup::new( + self.ioapic.clone(), + base as u32, + ))) } + _ => { + return Err(io::Error::new( + io::ErrorKind::Other, + "Interrupt type not supported", + )) + } + }; + + Ok(interrupt_source_group) + } + + fn destroy_group(&self, _group: Arc>) -> Result<()> { + Ok(()) + } +} + +impl InterruptManager for KvmMsiInterruptManager { + fn create_group( + &self, + interrupt_type: InterruptType, + base: InterruptIndex, + count: InterruptIndex, + ) -> Result>> { + let interrupt_source_group: Arc> = match interrupt_type { PCI_MSI_IRQ => { let mut allocator = self.allocator.lock().unwrap(); let mut irq_routes: HashMap =