interrupt: Interrupt manager split

We create 2 different interrupt managers for separately handling
creation of legacy and MSI interrupt groups.
Doing so allows us to have a cleaner interrupt manager and IOAPIC
initialization path. It also prepares for an InterruptManager trait
design improvement where we remove the interrupt source type dependency
by associating an interrupt configuration type to the trait.

Signed-off-by: Samuel Ortiz <sameo@linux.intel.com>

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<Mutex<HashMap<u32, KvmRoutingEntry>>> =
             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<dyn InterruptManager> =
-            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<dyn InterruptManager> =
+            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<dyn InterruptManager> = 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<dyn InterruptManager> =
+            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)

vmm/src/interrupt.rs

@@ -272,30 +272,37 @@ impl InterruptSourceGroup for LegacyUserspaceInterruptGroup {
     }
 }
 
-pub struct KvmInterruptManager {
+pub struct KvmLegacyUserspaceInterruptManager {
+    ioapic: Arc<Mutex<ioapic::Ioapic>>,
+}
+
+pub struct KvmMsiInterruptManager {
     allocator: Arc<Mutex<SystemAllocator>>,
     vm_fd: Arc<VmFd>,
     gsi_msi_routes: Arc<Mutex<HashMap<u32, KvmRoutingEntry>>>,
-    ioapic: Option<Arc<Mutex<ioapic::Ioapic>>>,
 }
 
-impl KvmInterruptManager {
+impl KvmLegacyUserspaceInterruptManager {
+    pub fn new(ioapic: Arc<Mutex<ioapic::Ioapic>>) -> Self {
+        KvmLegacyUserspaceInterruptManager { ioapic }
+    }
+}
+
+impl KvmMsiInterruptManager {
     pub fn new(
         allocator: Arc<Mutex<SystemAllocator>>,
         vm_fd: Arc<VmFd>,
         gsi_msi_routes: Arc<Mutex<HashMap<u32, KvmRoutingEntry>>>,
-        ioapic: Option<Arc<Mutex<ioapic::Ioapic>>>,
     ) -> 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<Box<dyn InterruptSourceGroup>>) -> Result<()> {
+        Ok(())
+    }
+}
+
+impl InterruptManager for KvmMsiInterruptManager {
+    fn create_group(
+        &self,
+        interrupt_type: InterruptType,
+        base: InterruptIndex,
+        count: InterruptIndex,
+    ) -> Result<Arc<Box<dyn InterruptSourceGroup>>> {
+        let interrupt_source_group: Arc<Box<dyn InterruptSourceGroup>> = match interrupt_type {
             PCI_MSI_IRQ => {
                 let mut allocator = self.allocator.lock().unwrap();
                 let mut irq_routes: HashMap<InterruptIndex, InterruptRoute> =