pcie: hotplug in interrupt for mutli devices

When multi pcie endpoint devices are connected to one virtual pcie
root port as pcie multi function device, virtual pcie root port couldn't
inject hotplug in interrupt for each added pcie device. Only one hotplug
in interrupt is needed for all the added pcie devices.

So this commit adds a hp_interupt field into VfioCommandRequest, only
the last added pcie device sets hp_interrupt to true and requests virtual
pcie root port to inject hotplug in interrupt. Ohter added pcie devices
just create and configure vfio-pci device.

BUG=b:185084350
TEST=Check TBT dock's hotplug in function, TBT dock has multi pcie
devices.

Change-Id: I5a68ec38af9a32de9d4233b48e0aa3681e5a3ba3
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3604290
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

devices/src/pci/pcie/pcie_host.rs

@@ -160,7 +160,24 @@ impl HotplugWorker {
         let mut children: Vec<PathBuf> = Vec::new();
         visit_children(host_sysfs.as_path(), &mut children)?;
 
-        for child in &children {
+        // Without reverse children, physical larger BDF device is at the top, it will be
+        // added into guest first with smaller virtual function number, so physical smaller
+        // BDF device has larger virtual function number, phyiscal larger BDF device has
+        // smaller virtual function number. During hotplug out process, host pcie root port
+        // driver remove physical smaller BDF pcie endpoint device first, so host vfio-pci
+        // driver send plug out event first for smaller BDF device and wait for this device
+        // removed from crosvm, when crosvm receives this plug out event, crosvm will remove
+        // all the children devices, crosvm remove smaller virtual function number device
+        // first, this isn't the target device which host vfio-pci driver is waiting for.
+        // Host vfio-pci driver holds a lock when it is waiting, when crosvm remove another
+        // device throgh vfio-pci which try to get the same lock, so deadlock happens in
+        // host kernel.
+        //
+        // In order to fix the deadlock, children is reversed, so physical smaller BDF
+        // device has smaller virtual function number, and it will have the same order
+        // between host kernel and crosvm during hotplug out process.
+        children.reverse();
+        while let Some(child) = children.pop() {
             // In order to bind device to vfio-pci driver, get device VID and DID
             let vendor_path = child.join("vendor");
             let vendor_id = read(vendor_path.as_path())
@@ -199,6 +216,7 @@ impl HotplugWorker {
             let request = VmRequest::VfioCommand {
                 vfio_path: child.clone(),
                 add: true,
+                hp_interrupt: children.is_empty(),
             };
             vm_socket.lock().send(&request).with_context(|| {
                 format!("failed to send hotplug request for {}", child.display())
@@ -206,10 +224,10 @@ impl HotplugWorker {
             vm_socket.lock().recv::<VmResponse>().with_context(|| {
                 format!("failed to receive hotplug response for {}", child.display())
             })?;
-        }
 
-        if !children.is_empty() {
-            *child_exist = true;
+            if !*child_exist {
+                *child_exist = true;
+            }
         }
 
         Ok(())

devices/src/pci/vfio_pci.rs

@@ -629,6 +629,7 @@ impl VfioPciWorker {
                         let request = VmRequest::VfioCommand {
                             vfio_path: sysfs_path,
                             add: false,
+                            hp_interrupt: true,
                         };
                         if self.vm_socket.send(&request).is_ok() {
                             if let Err(e) = self.vm_socket.recv::<VmResponse>() {

src/linux/mod.rs

@@ -1514,6 +1514,7 @@ fn add_vfio_device<V: VmArch, Vcpu: VcpuArch>(
     control_tubes: &mut Vec<TaggedControlTube>,
     iommu_host_tube: &Option<Tube>,
     vfio_path: &Path,
+    hp_interrupt: bool,
 ) -> Result<()> {
     let host_os_str = vfio_path
         .file_name()
@@ -1572,7 +1573,9 @@ fn add_vfio_device<V: VmArch, Vcpu: VcpuArch>(
     let host_key = HostHotPlugKey::Vfio { host_addr };
     let mut hp_bus = hp_bus.lock();
     hp_bus.add_hotplug_device(host_key, pci_address);
-    hp_bus.hot_plug(pci_address);
+    if hp_interrupt {
+        hp_bus.hot_plug(pci_address);
+    }
     Ok(())
 }
 
@@ -1581,6 +1584,7 @@ fn remove_vfio_device<V: VmArch, Vcpu: VcpuArch>(
     sys_allocator: &mut SystemAllocator,
     iommu_host_tube: &Option<Tube>,
     vfio_path: &Path,
+    _hp_interrupt: bool,
 ) -> Result<()> {
     let host_os_str = vfio_path
         .file_name()
@@ -1623,6 +1627,7 @@ fn handle_vfio_command<V: VmArch, Vcpu: VcpuArch>(
     iommu_host_tube: &Option<Tube>,
     vfio_path: &Path,
     add: bool,
+    hp_interrupt: bool,
 ) -> VmResponse {
     let ret = if add {
         add_vfio_device(
@@ -1632,9 +1637,16 @@ fn handle_vfio_command<V: VmArch, Vcpu: VcpuArch>(
             add_tubes,
             iommu_host_tube,
             vfio_path,
+            hp_interrupt,
         )
     } else {
-        remove_vfio_device(linux, sys_allocator, iommu_host_tube, vfio_path)
+        remove_vfio_device(
+            linux,
+            sys_allocator,
+            iommu_host_tube,
+            vfio_path,
+            hp_interrupt,
+        )
     };
 
     match ret {
@@ -1952,17 +1964,20 @@ fn run_control<V: VmArch + 'static, Vcpu: VcpuArch + 'static>(
                                 Ok(request) => {
                                     let mut run_mode_opt = None;
                                     let response = match request {
-                                        VmRequest::VfioCommand { vfio_path, add } => {
-                                            handle_vfio_command(
-                                                &mut linux,
-                                                &mut sys_allocator,
-                                                &cfg,
-                                                &mut add_tubes,
-                                                &iommu_host_tube,
-                                                &vfio_path,
-                                                add,
-                                            )
-                                        }
+                                        VmRequest::VfioCommand {
+                                            vfio_path,
+                                            add,
+                                            hp_interrupt,
+                                        } => handle_vfio_command(
+                                            &mut linux,
+                                            &mut sys_allocator,
+                                            &cfg,
+                                            &mut add_tubes,
+                                            &iommu_host_tube,
+                                            &vfio_path,
+                                            add,
+                                            hp_interrupt,
+                                        ),
                                         _ => request.execute(
                                             &mut run_mode_opt,
                                             balloon_host_tube.as_ref(),

src/main.rs

@@ -3273,7 +3273,12 @@ fn modify_vfio(mut args: std::env::Args) -> std::result::Result<(), ()> {
         }
     };
 
-    let request = VmRequest::VfioCommand { vfio_path, add };
+    let hp_interrupt = true;
+    let request = VmRequest::VfioCommand {
+        vfio_path,
+        add,
+        hp_interrupt,
+    };
     handle_request(&request, socket_path)?;
     Ok(())
 }

vm_control/src/lib.rs

@@ -991,7 +991,11 @@ pub enum VmRequest {
     /// Command to set battery.
     BatCommand(BatteryType, BatControlCommand),
     /// Command to add/remove vfio pci device
-    VfioCommand { vfio_path: PathBuf, add: bool },
+    VfioCommand {
+        vfio_path: PathBuf,
+        add: bool,
+        hp_interrupt: bool,
+    },
 }
 
 fn map_descriptor(
@@ -1218,6 +1222,7 @@ impl VmRequest {
             VmRequest::VfioCommand {
                 vfio_path: _,
                 add: _,
+                hp_interrupt: _,
             } => VmResponse::Ok,
         }
     }