From 9da2343cb7fb7a383b6f5dfd2966e2c4ad4c3672 Mon Sep 17 00:00:00 2001
From: Jing Liu <jing2.liu@linux.intel.com>
Date: Wed, 5 Jun 2019 17:24:18 +0800
Subject: [PATCH] device: Improvement for BusDevice trait and PciDevice trait

BusDevice includes two methods which are only for PCI devices, which should
be as members of PciDevice trait for a better clean high level APIs.

Signed-off-by: Jing Liu <jing2.liu@linux.intel.com>
---
 devices/src/bus.rs                    | 10 ----------
 pci/src/device.rs                     |  7 +++++++
 pci/src/root.rs                       | 17 ++++++++++-------
 vm-virtio/src/transport/pci_device.rs | 18 +++++++++---------
 vmm/src/vm.rs                         |  6 +++++-
 5 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/devices/src/bus.rs b/devices/src/bus.rs
index 0d544bafc..9c6a011f6 100644
--- a/devices/src/bus.rs
+++ b/devices/src/bus.rs
@@ -24,16 +24,6 @@ pub trait BusDevice: Send {
     fn write(&mut self, offset: u64, data: &[u8]) {}
     /// Triggers the `irq_mask` interrupt on this device
     fn interrupt(&self, irq_mask: u32) {}
-
-    /// Sets a register in the configuration space. Only used by PCI.
-    /// * `reg_idx` - The index of the config register to modify.
-    /// * `offset` - Offset in to the register.
-    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {}
-    /// Gets a register from the configuration space. Only used by PCI.
-    /// * `reg_idx` - The index of the config register to read.
-    fn read_config_register(&self, reg_idx: usize) -> u32 {
-        0
-    }
 }
 
 #[derive(Debug)]
diff --git a/pci/src/device.rs b/pci/src/device.rs
index cf26731e9..275a65b7c 100755
--- a/pci/src/device.rs
+++ b/pci/src/device.rs
@@ -80,6 +80,13 @@ pub trait PciDevice: BusDevice {
     fn ioeventfds(&self) -> Vec<(&EventFd, u64, u64)> {
         Vec::new()
     }
+    /// Sets a register in the configuration space.
+    /// * `reg_idx` - The index of the config register to modify.
+    /// * `offset` - Offset in to the register.
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]);
+    /// Gets a register from the configuration space.
+    /// * `reg_idx` - The index of the config register to read.
+    fn read_config_register(&self, reg_idx: usize) -> u32;
     /// Reads from a BAR region mapped in to the device.
     /// * `addr` - The guest address inside the BAR.
     /// * `data` - Filled with the data from `addr`.
diff --git a/pci/src/root.rs b/pci/src/root.rs
index 9620434c0..adb917f69 100755
--- a/pci/src/root.rs
+++ b/pci/src/root.rs
@@ -3,7 +3,7 @@
 // found in the LICENSE-BSD-3-Clause file.
 
 use crate::configuration::{PciBridgeSubclass, PciClassCode, PciConfiguration, PciHeaderType};
-use crate::device::Error as PciDeviceError;
+use crate::device::{Error as PciDeviceError, PciDevice};
 use byteorder::{ByteOrder, LittleEndian};
 use devices::BusDevice;
 use std;
@@ -31,7 +31,7 @@ pub struct PciRoot {
     /// Bus configuration for the root device.
     configuration: PciConfiguration,
     /// Devices attached to this bridge.
-    devices: Vec<Arc<Mutex<dyn BusDevice>>>,
+    devices: Vec<Arc<Mutex<dyn PciDevice>>>,
 }
 
 impl PciRoot {
@@ -61,8 +61,14 @@ impl PciRoot {
     }
 
     /// Add a `device` to this root PCI bus.
-    pub fn add_device(
-        &mut self,
+    pub fn add_device(&mut self, pci_device: Arc<Mutex<dyn PciDevice>>) -> Result<()> {
+        self.devices.push(pci_device);
+        Ok(())
+    }
+
+    /// Register Guest Address mapping of a `device` to IO bus.
+    pub fn register_mapping(
+        &self,
         device: Arc<Mutex<dyn BusDevice>>,
         bus: &mut devices::Bus,
         bars: Vec<(GuestAddress, GuestUsize)>,
@@ -71,11 +77,8 @@ impl PciRoot {
             bus.insert(device.clone(), address.raw_value(), size)
                 .map_err(PciRootError::MmioInsert)?;
         }
-
-        self.devices.push(device);
         Ok(())
     }
-
     pub fn config_space_read(
         &self,
         bus: usize,
diff --git a/vm-virtio/src/transport/pci_device.rs b/vm-virtio/src/transport/pci_device.rs
index a516b7da0..1031b601a 100755
--- a/vm-virtio/src/transport/pci_device.rs
+++ b/vm-virtio/src/transport/pci_device.rs
@@ -397,6 +397,15 @@ impl PciDevice for VirtioPciDevice {
         }
     }
 
+    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
+        self.configuration
+            .write_config_register(reg_idx, offset, data);
+    }
+
+    fn read_config_register(&self, reg_idx: usize) -> u32 {
+        self.configuration.read_reg(reg_idx)
+    }
+
     fn ioeventfds(&self) -> Vec<(&EventFd, u64, u64)> {
         let bar0 = self
             .configuration
@@ -589,13 +598,4 @@ impl BusDevice for VirtioPciDevice {
     fn write(&mut self, offset: u64, data: &[u8]) {
         self.write_bar(offset, data)
     }
-
-    fn write_config_register(&mut self, reg_idx: usize, offset: u64, data: &[u8]) {
-        self.configuration
-            .write_config_register(reg_idx, offset, data);
-    }
-
-    fn read_config_register(&self, reg_idx: usize) -> u32 {
-        self.configuration.read_config_register(reg_idx)
-    }
 }
diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs
index 0b412fcf5..ed3628c50 100755
--- a/vmm/src/vm.rs
+++ b/vmm/src/vm.rs
@@ -692,7 +692,11 @@ impl DeviceManager {
         let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device));
 
         pci_root
-            .add_device(virtio_pci_device.clone(), mmio_bus, bars)
+            .add_device(virtio_pci_device.clone())
+            .map_err(DeviceManagerError::AddPciDevice)?;
+
+        pci_root
+            .register_mapping(virtio_pci_device.clone(), mmio_bus, bars)
             .map_err(DeviceManagerError::AddPciDevice)?;
 
         Ok(())