From 6aa5e21212cd5d626840b21682f4c24977c29344 Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf <sebastien.boeuf@intel.com>
Date: Mon, 5 Oct 2020 19:13:28 +0200
Subject: [PATCH] vmm: device_manager: Fix PCI device unplug issues

Because of the PCI refactoring that happened in the previous commit
d793cc4da365ef960a239ff32589efc2635072bd, the ability to fully remove a
PCI device was altered.

The refactoring was correct, but the usage of a generic function to pass
the same reference for both BusDevice, PciDevice and Any + Send + Sync
causes the Arc::ptr_eq() function to behave differently than expected,
as it does not match the references later in the code. That means we
were not able to remove the device reference from the MMIO and/or PIO
buses, which was leading to some bus range overlapping error once we
were trying to add a device again to the previous range that should have
been removed.

Fixes #1802

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 vmm/src/device_manager.rs | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs
index c224b891f..247a6e1d3 100644
--- a/vmm/src/device_manager.rs
+++ b/vmm/src/device_manager.rs
@@ -2703,9 +2703,13 @@ impl DeviceManager {
             })
             .map_err(DeviceManagerError::VfioMapRegion)?;
 
+        let vfio_pci_device = Arc::new(Mutex::new(vfio_pci_device));
+
         self.add_pci_device(
             pci,
-            Arc::new(Mutex::new(vfio_pci_device)),
+            vfio_pci_device.clone(),
+            vfio_pci_device.clone(),
+            vfio_pci_device,
             pci_device_bdf,
             vfio_name.clone(),
         )?;
@@ -2714,34 +2718,31 @@ impl DeviceManager {
     }
 
     #[cfg(feature = "pci_support")]
-    fn add_pci_device<T>(
+    fn add_pci_device(
         &mut self,
         pci_bus: &mut PciBus,
-        device: Arc<Mutex<T>>,
+        bus_device: Arc<Mutex<dyn BusDevice>>,
+        pci_device: Arc<Mutex<dyn PciDevice>>,
+        any_device: Arc<dyn Any + Send + Sync>,
         bdf: u32,
         device_id: String,
-    ) -> DeviceManagerResult<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>>
-    where
-        T: BusDevice + PciDevice + Any + Send + Sync,
-    {
-        let bars = device
+    ) -> DeviceManagerResult<Vec<(GuestAddress, GuestUsize, PciBarRegionType)>> {
+        let bars = pci_device
             .lock()
             .unwrap()
             .allocate_bars(&mut self.address_manager.allocator.lock().unwrap())
             .map_err(DeviceManagerError::AllocateBars)?;
 
         pci_bus
-            .add_device(bdf, device.clone())
+            .add_device(bdf, pci_device)
             .map_err(DeviceManagerError::AddPciDevice)?;
 
-        self.pci_devices
-            .insert(bdf, Arc::clone(&device) as Arc<dyn Any + Send + Sync>);
-        self.bus_devices
-            .push(Arc::clone(&device) as Arc<Mutex<dyn BusDevice>>);
+        self.pci_devices.insert(bdf, any_device);
+        self.bus_devices.push(Arc::clone(&bus_device));
 
         pci_bus
             .register_mapping(
-                device,
+                bus_device,
                 #[cfg(target_arch = "x86_64")]
                 self.address_manager.io_bus.as_ref(),
                 self.address_manager.mmio_bus.as_ref(),
@@ -2901,6 +2902,8 @@ impl DeviceManager {
         let bars = self.add_pci_device(
             pci,
             virtio_pci_device.clone(),
+            virtio_pci_device.clone(),
+            virtio_pci_device.clone(),
             pci_device_bdf,
             virtio_device_id,
         )?;