arch: Generalize PCI device box for build_vm

Before we call build_vm we are creating devices and there is no reason
to assume those have to be PCI only. In preparation for VFIO platform
device support, add super trait which allows to pass generic device
structure around and still be able get back to our original type.

BUG=b:185504618
TEST=manatee PCI device passthrough boots/works

Change-Id: I500f44af430f5f06299f20fc4ca17ca008a7e0c5
Signed-off-by: Tomasz Nowicki <tn@semihalf.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2961210
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Tomasz Nowicki <tnowicki@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

aarch64/src/lib.rs

@@ -12,7 +12,8 @@ use arch::{get_serial_cmdline, GetSerialCmdlineError, RunnableLinuxVm, VmCompone
 use base::{Event, MemoryMappingBuilder};
 use devices::serial_device::{SerialHardware, SerialParameters};
 use devices::{
-    Bus, BusError, IrqChip, IrqChipAArch64, PciAddress, PciConfigMmio, PciDevice, ProtectionType,
+    Bus, BusDeviceObj, BusError, IrqChip, IrqChipAArch64, PciAddress, PciConfigMmio, PciDevice,
+    ProtectionType,
 };
 use hypervisor::{DeviceKind, Hypervisor, HypervisorCap, VcpuAArch64, VcpuFeature, VmAArch64};
 use minijail::Minijail;
@@ -256,7 +257,7 @@ impl arch::LinuxArch for AArch64 {
         _battery: (&Option<BatteryType>, Option<Minijail>),
         mut vm: V,
         ramoops_region: Option<arch::pstore::RamoopsRegion>,
-        pci_devices: Vec<(Box<dyn PciDevice>, Option<Minijail>)>,
+        devs: Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)>,
         irq_chip: &mut dyn IrqChipAArch64,
     ) -> std::result::Result<RunnableLinuxVm<V, Vcpu>, Self::Error>
     where
@@ -336,6 +337,14 @@ impl arch::LinuxArch for AArch64 {
         // guest OS is trying to suspend.
         let suspend_evt = Event::new().map_err(Error::CreateEvent)?;
 
+        let (pci_devices, _others): (Vec<_>, Vec<_>) = devs
+            .into_iter()
+            .partition(|(dev, _)| dev.as_pci_device().is_some());
+
+        let pci_devices = pci_devices
+            .into_iter()
+            .map(|(dev, jail_orig)| (dev.into_pci_device().unwrap(), jail_orig))
+            .collect();
         let (pci, pci_irqs, pid_debug_label_map) = arch::generate_pci_root(
             pci_devices,
             irq_chip.as_irq_chip_mut(),

arch/src/lib.rs

@@ -19,9 +19,9 @@ use acpi_tables::sdt::SDT;
 use base::{syslog, AsRawDescriptor, AsRawDescriptors, Event, Tube};
 use devices::virtio::VirtioDevice;
 use devices::{
-    Bus, BusDevice, BusError, BusResumeDevice, HotPlugBus, IrqChip, PciAddress, PciDevice,
-    PciDeviceError, PciInterruptPin, PciRoot, ProtectionType, ProxyDevice, SerialHardware,
-    SerialParameters,
+    Bus, BusDevice, BusDeviceObj, BusError, BusResumeDevice, HotPlugBus, IrqChip, PciAddress,
+    PciDevice, PciDeviceError, PciInterruptPin, PciRoot, ProtectionType, ProxyDevice,
+    SerialHardware, SerialParameters,
 };
 use hypervisor::{IoEventAddress, Vm};
 use minijail::Minijail;
@@ -169,7 +169,7 @@ pub trait LinuxArch {
     /// * `battery` - Defines what battery device will be created.
     /// * `vm` - A VM implementation to build upon.
     /// * `ramoops_region` - Region allocated for ramoops.
-    /// * `pci_devices` - The PCI devices to be built into the VM.
+    /// * `devices` - The devices to be built into the VM.
     /// * `irq_chip` - The IRQ chip implemention for the VM.
     fn build_vm<V, Vcpu>(
         components: VmComponents,
@@ -180,7 +180,7 @@ pub trait LinuxArch {
         battery: (&Option<BatteryType>, Option<Minijail>),
         vm: V,
         ramoops_region: Option<pstore::RamoopsRegion>,
-        pci_devices: Vec<(Box<dyn PciDevice>, Option<Minijail>)>,
+        devices: Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)>,
         irq_chip: &mut dyn IrqChipArch,
     ) -> std::result::Result<RunnableLinuxVm<V, Vcpu>, Self::Error>
     where

devices/src/bus.rs

@@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize};
 use sync::Mutex;
 use thiserror::Error;
 
-use crate::PciAddress;
+use crate::{PciAddress, PciDevice};
 
 /// Information about how a device was accessed.
 #[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
@@ -122,6 +122,21 @@ pub trait HotPlugBus {
     fn get_hotplug_device(&self, host_key: HostHotPlugKey) -> Option<PciAddress>;
 }
 
+/// Trait for generic device abstraction, that is, all devices that reside on BusDevice and want
+/// to be converted back to its original type. Each new foo device must provide
+/// as_foo_device() + as_foo_device_mut() + into_foo_device(), default impl methods return None.
+pub trait BusDeviceObj {
+    fn as_pci_device(&self) -> Option<&dyn PciDevice> {
+        None
+    }
+    fn as_pci_device_mut(&mut self) -> Option<&mut dyn PciDevice> {
+        None
+    }
+    fn into_pci_device(self: Box<Self>) -> Option<Box<dyn PciDevice>> {
+        None
+    }
+}
+
 #[sorted]
 #[derive(Error, Debug)]
 pub enum Error {

devices/src/lib.rs

@@ -35,8 +35,8 @@ pub use self::acpi::ACPIPMResource;
 pub use self::bat::{BatteryError, GoldfishBattery};
 pub use self::bus::Error as BusError;
 pub use self::bus::{
-    Bus, BusAccessInfo, BusDevice, BusDeviceSync, BusRange, BusResumeDevice, HostHotPlugKey,
-    HotPlugBus,
+    Bus, BusAccessInfo, BusDevice, BusDeviceObj, BusDeviceSync, BusRange, BusResumeDevice,
+    HostHotPlugKey, HotPlugBus,
 };
 pub use self::cmos::Cmos;
 #[cfg(feature = "direct")]

devices/src/pci/pci_device.rs

@@ -10,7 +10,7 @@ use remain::sorted;
 use resources::{Error as SystemAllocatorFaliure, SystemAllocator};
 use thiserror::Error;
 
-use crate::bus::ConfigWriteResult;
+use crate::bus::{BusDeviceObj, ConfigWriteResult};
 use crate::pci::pci_configuration::{
     self, PciBarConfiguration, COMMAND_REG, COMMAND_REG_IO_SPACE_MASK,
     COMMAND_REG_MEMORY_SPACE_MASK,
@@ -249,6 +249,18 @@ impl<T: PciDevice + ?Sized> PciDevice for Box<T> {
     }
 }
 
+impl<T: 'static + PciDevice> BusDeviceObj for T {
+    fn as_pci_device(&self) -> Option<&dyn PciDevice> {
+        Some(self)
+    }
+    fn as_pci_device_mut(&mut self) -> Option<&mut dyn PciDevice> {
+        Some(self)
+    }
+    fn into_pci_device(self: Box<Self>) -> Option<Box<dyn PciDevice>> {
+        Some(self)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

src/linux.rs

@@ -47,8 +47,8 @@ use devices::virtio::{
 use devices::Ac97Dev;
 use devices::ProtectionType;
 use devices::{
-    self, HostHotPlugKey, IrqChip, IrqEventIndex, KvmKernelIrqChip, PciAddress, PciDevice,
-    VcpuRunState, VfioContainer, VfioDevice, VfioPciDevice, VirtioPciDevice,
+    self, BusDeviceObj, HostHotPlugKey, IrqChip, IrqEventIndex, KvmKernelIrqChip, PciAddress,
+    PciDevice, VcpuRunState, VfioContainer, VfioDevice, VfioPciDevice, VirtioPciDevice,
 };
 #[cfg(feature = "usb")]
 use devices::{HostBackendDeviceProvider, XhciController};
@@ -1608,7 +1608,7 @@ fn create_devices(
     fs_device_tubes: &mut Vec<Tube>,
     #[cfg(feature = "usb")] usb_provider: HostBackendDeviceProvider,
     map_request: Arc<Mutex<Option<ExternalMapping>>>,
-) -> DeviceResult<Vec<(Box<dyn PciDevice>, Option<Minijail>)>> {
+) -> DeviceResult<Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)>> {
     let stubs = create_virtio_devices(
         cfg,
         vm,
@@ -1624,15 +1624,15 @@ fn create_devices(
         fs_device_tubes,
     )?;
 
-    let mut pci_devices = Vec::new();
+    let mut devices = Vec::new();
 
     for stub in stubs {
         let (msi_host_tube, msi_device_tube) = Tube::pair().map_err(Error::CreateTube)?;
         control_tubes.push(TaggedControlTube::VmIrq(msi_host_tube));
         let dev = VirtioPciDevice::new(vm.get_memory().clone(), stub.dev, msi_device_tube)
             .map_err(Error::VirtioPciDev)?;
-        let dev = Box::new(dev) as Box<dyn PciDevice>;
-        pci_devices.push((dev, stub.jail));
+        let dev = Box::new(dev) as Box<dyn BusDeviceObj>;
+        devices.push((dev, stub.jail));
     }
 
     #[cfg(feature = "audio")]
@@ -1640,14 +1640,14 @@ fn create_devices(
         let dev = Ac97Dev::try_new(vm.get_memory().clone(), ac97_param.clone())
             .map_err(Error::CreateAc97)?;
         let jail = simple_jail(cfg, dev.minijail_policy())?;
-        pci_devices.push((Box::new(dev), jail));
+        devices.push((Box::new(dev), jail));
     }
 
     #[cfg(feature = "usb")]
     {
         // Create xhci controller.
         let usb_controller = Box::new(XhciController::new(vm.get_memory().clone(), usb_provider));
-        pci_devices.push((usb_controller, simple_jail(cfg, "xhci")?));
+        devices.push((usb_controller, simple_jail(cfg, "xhci")?));
     }
 
     if !cfg.vfio.is_empty() {
@@ -1666,7 +1666,7 @@ fn create_devices(
                 *enable_iommu,
             )?;
 
-            pci_devices.push((vfio_pci_device, jail));
+            devices.push((vfio_pci_device, jail));
         }
 
         if !iommu_attached_endpoints.is_empty() {
@@ -1681,11 +1681,11 @@ fn create_devices(
             dev.allocate_address(resources)
                 .map_err(|_| Error::VirtioPciDev(base::Error::new(EINVAL)))?;
             let dev = Box::new(dev);
-            pci_devices.push((dev, iommu_dev.jail));
+            devices.push((dev, iommu_dev.jail));
         }
     }
 
-    Ok(pci_devices)
+    Ok(devices)
 }
 
 #[derive(Copy, Clone)]
@@ -2496,7 +2496,7 @@ where
     };
 
     let phys_max_addr = Arch::get_phys_max_addr();
-    let mut pci_devices = create_devices(
+    let mut devices = create_devices(
         &cfg,
         &mut vm,
         &mut sys_allocator,
@@ -2516,7 +2516,10 @@ where
     )?;
 
     #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
-    for (device, _jail) in pci_devices.iter_mut() {
+    for device in devices
+        .iter_mut()
+        .filter_map(|(dev, _)| dev.as_pci_device_mut())
+    {
         let sdts = device
             .generate_acpi(components.acpi_sdts)
             .or_else(|| {
@@ -2537,7 +2540,7 @@ where
         battery,
         vm,
         ramoops_region,
-        pci_devices,
+        devices,
         irq_chip,
     )
     .map_err(Error::BuildVm)?;

x86_64/src/lib.rs

@@ -62,7 +62,8 @@ use arch::{
 use base::Event;
 use devices::serial_device::{SerialHardware, SerialParameters};
 use devices::{
-    BusResumeDevice, IrqChip, IrqChipX86_64, PciAddress, PciConfigIo, PciDevice, ProtectionType,
+    BusDeviceObj, BusResumeDevice, IrqChip, IrqChipX86_64, PciAddress, PciConfigIo, PciDevice,
+    ProtectionType,
 };
 use hypervisor::{HypervisorX86_64, VcpuX86_64, VmX86_64};
 use minijail::Minijail;
@@ -386,7 +387,7 @@ impl arch::LinuxArch for X8664arch {
         battery: (&Option<BatteryType>, Option<Minijail>),
         mut vm: V,
         ramoops_region: Option<arch::pstore::RamoopsRegion>,
-        pci_devices: Vec<(Box<dyn PciDevice>, Option<Minijail>)>,
+        devs: Vec<(Box<dyn BusDeviceObj>, Option<Minijail>)>,
         irq_chip: &mut dyn IrqChipX86_64,
     ) -> std::result::Result<RunnableLinuxVm<V, Vcpu>, Self::Error>
     where
@@ -407,6 +408,15 @@ impl arch::LinuxArch for X8664arch {
         let mut mmio_bus = devices::Bus::new();
         let mut io_bus = devices::Bus::new();
 
+        let (pci_devices, _others): (Vec<_>, Vec<_>) = devs
+            .into_iter()
+            .partition(|(dev, _)| dev.as_pci_device().is_some());
+
+        let pci_devices = pci_devices
+            .into_iter()
+            .map(|(dev, jail_orig)| (dev.into_pci_device().unwrap(), jail_orig))
+            .collect();
+
         let (pci, pci_irqs, pid_debug_label_map) = arch::generate_pci_root(
             pci_devices,
             irq_chip.as_irq_chip_mut(),