From 99f39291fd6a08ef76ebe45209ea2d47c201109a Mon Sep 17 00:00:00 2001
From: Sebastien Boeuf <sebastien.boeuf@intel.com>
Date: Mon, 20 Jan 2020 16:33:14 +0100
Subject: [PATCH] pci: Simplify PciDevice trait

There's no need for assign_irq() or assign_msix() functions from the
PciDevice trait, as we can see it's never used anywhere in the codebase.
That's why it's better to remove these methods from the trait, and
slightly adapt the existing code.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
---
 pci/src/device.rs                     | 15 ----------
 vm-virtio/src/transport/pci_device.rs | 41 ++++++++++-----------------
 vmm/src/device_manager.rs             |  2 --
 3 files changed, 15 insertions(+), 43 deletions(-)

diff --git a/pci/src/device.rs b/pci/src/device.rs
index 6a4a90b33..b20f61789 100755
--- a/pci/src/device.rs
+++ b/pci/src/device.rs
@@ -4,11 +4,9 @@
 
 use crate::configuration::{self, PciBarRegionType};
 use crate::msix::MsixTableEntry;
-use crate::PciInterruptPin;
 use devices::BusDevice;
 use std::any::Any;
 use std::fmt::{self, Display};
-use std::sync::Arc;
 use std::{self, io, result};
 use vm_allocator::SystemAllocator;
 use vm_memory::{GuestAddress, GuestUsize};
@@ -56,19 +54,6 @@ pub struct BarReprogrammingParams {
 }
 
 pub trait PciDevice: BusDevice {
-    /// Assign a legacy PCI IRQ to this device.
-    /// The device may write to `irq_evt` to trigger an interrupt.
-    fn assign_pin_irq(
-        &mut self,
-        _irq_cb: Arc<InterruptDelivery>,
-        _irq_num: u32,
-        _irq_pin: PciInterruptPin,
-    ) {
-    }
-
-    /// Assign MSI-X to this device.
-    fn assign_msix(&mut self) {}
-
     /// Allocates the needed PCI BARs space using the `allocate` function which takes a size and
     /// returns an address. Returns a Vec of (GuestAddress, GuestUsize) tuples.
     fn allocate_bars(
diff --git a/vm-virtio/src/transport/pci_device.rs b/vm-virtio/src/transport/pci_device.rs
index de2f40cfa..bb81a9faa 100755
--- a/vm-virtio/src/transport/pci_device.rs
+++ b/vm-virtio/src/transport/pci_device.rs
@@ -24,10 +24,9 @@ use arc_swap::ArcSwap;
 use devices::BusDevice;
 use libc::EFD_NONBLOCK;
 use pci::{
-    BarReprogrammingParams, InterruptDelivery, MsixCap, MsixConfig, PciBarConfiguration,
-    PciBarRegionType, PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice,
-    PciDeviceError, PciHeaderType, PciInterruptPin, PciMassStorageSubclass,
-    PciNetworkControllerSubclass, PciSubclass,
+    BarReprogrammingParams, MsixCap, MsixConfig, PciBarConfiguration, PciBarRegionType,
+    PciCapability, PciCapabilityID, PciClassCode, PciConfiguration, PciDevice, PciDeviceError,
+    PciHeaderType, PciMassStorageSubclass, PciNetworkControllerSubclass, PciSubclass,
 };
 use std::any::Any;
 use std::result;
@@ -324,7 +323,7 @@ impl VirtioPciDevice {
             msix_config_clone,
         );
 
-        Ok(VirtioPciDevice {
+        let mut virtio_pci_device = VirtioPciDevice {
             configuration,
             common_config: VirtioPciCommonConfig {
                 driver_status: 0,
@@ -346,7 +345,17 @@ impl VirtioPciDevice {
             settings_bar: 0,
             use_64bit_bar,
             interrupt_source_group,
-        })
+        };
+
+        if let Some(msix_config) = &virtio_pci_device.msix_config {
+            virtio_pci_device.virtio_interrupt = Some(Arc::new(VirtioInterruptMsix::new(
+                msix_config.clone(),
+                virtio_pci_device.common_config.msix_config.clone(),
+                virtio_pci_device.interrupt_source_group.clone(),
+            )));
+        }
+
+        Ok(virtio_pci_device)
     }
 
     /// Gets the list of queue events that must be triggered whenever the VM writes to
@@ -542,26 +551,6 @@ impl VirtioInterrupt for VirtioInterruptMsix {
 }
 
 impl PciDevice for VirtioPciDevice {
-    fn assign_pin_irq(
-        &mut self,
-        _irq_cb: Arc<InterruptDelivery>,
-        _irq_num: u32,
-        _irq_pin: PciInterruptPin,
-    ) {
-    }
-
-    fn assign_msix(&mut self) {
-        if let Some(msix_config) = &self.msix_config {
-            let virtio_interrupt_msix = Arc::new(VirtioInterruptMsix::new(
-                msix_config.clone(),
-                self.common_config.msix_config.clone(),
-                self.interrupt_source_group.clone(),
-            ));
-
-            self.virtio_interrupt = Some(virtio_interrupt_msix);
-        }
-    }
-
     fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
         self.configuration
             .write_config_register(reg_idx, offset, data);
diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs
index ff4bb175b..88d65ec61 100644
--- a/vmm/src/device_manager.rs
+++ b/vmm/src/device_manager.rs
@@ -1473,8 +1473,6 @@ impl DeviceManager {
                 .map_err(DeviceManagerError::RegisterIoevent)?;
         }
 
-        virtio_pci_device.assign_msix();
-
         let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device));
 
         pci.add_device(virtio_pci_device.clone())