From ab86d52fba27db7f84bb8b259145fc9e83a17996 Mon Sep 17 00:00:00 2001 From: Tomasz Nowicki Date: Wed, 22 Sep 2021 05:50:46 +0000 Subject: [PATCH] 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 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2961210 Tested-by: kokoro Commit-Queue: Tomasz Nowicki Reviewed-by: Daniel Verkamp --- aarch64/src/lib.rs | 13 +++++++++++-- arch/src/lib.rs | 10 +++++----- devices/src/bus.rs | 17 ++++++++++++++++- devices/src/lib.rs | 4 ++-- devices/src/pci/pci_device.rs | 14 +++++++++++++- src/linux.rs | 31 +++++++++++++++++-------------- x86_64/src/lib.rs | 14 ++++++++++++-- 7 files changed, 76 insertions(+), 27 deletions(-) diff --git a/aarch64/src/lib.rs b/aarch64/src/lib.rs index fa9a59d761..b1df863e07 100644 --- a/aarch64/src/lib.rs +++ b/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, Option), mut vm: V, ramoops_region: Option, - pci_devices: Vec<(Box, Option)>, + devs: Vec<(Box, Option)>, irq_chip: &mut dyn IrqChipAArch64, ) -> std::result::Result, 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(), diff --git a/arch/src/lib.rs b/arch/src/lib.rs index 4e3ca29819..889ef0ee1a 100644 --- a/arch/src/lib.rs +++ b/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( components: VmComponents, @@ -180,7 +180,7 @@ pub trait LinuxArch { battery: (&Option, Option), vm: V, ramoops_region: Option, - pci_devices: Vec<(Box, Option)>, + devices: Vec<(Box, Option)>, irq_chip: &mut dyn IrqChipArch, ) -> std::result::Result, Self::Error> where diff --git a/devices/src/bus.rs b/devices/src/bus.rs index fc130f95de..46db2c09a0 100644 --- a/devices/src/bus.rs +++ b/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; } +/// 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) -> Option> { + None + } +} + #[sorted] #[derive(Error, Debug)] pub enum Error { diff --git a/devices/src/lib.rs b/devices/src/lib.rs index 466ac4498c..e3c9b25a78 100644 --- a/devices/src/lib.rs +++ b/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")] diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index 99b6ab1ada..3b9d2c2f8d 100644 --- a/devices/src/pci/pci_device.rs +++ b/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 PciDevice for Box { } } +impl 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) -> Option> { + Some(self) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/linux.rs b/src/linux.rs index d7d5d229a0..38ce3970b1 100644 --- a/src/linux.rs +++ b/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, #[cfg(feature = "usb")] usb_provider: HostBackendDeviceProvider, map_request: Arc>>, -) -> DeviceResult, Option)>> { +) -> DeviceResult, Option)>> { 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; - pci_devices.push((dev, stub.jail)); + let dev = Box::new(dev) as Box; + 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)?; diff --git a/x86_64/src/lib.rs b/x86_64/src/lib.rs index 20ee02b278..e80e61f41e 100644 --- a/x86_64/src/lib.rs +++ b/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, Option), mut vm: V, ramoops_region: Option, - pci_devices: Vec<(Box, Option)>, + devs: Vec<(Box, Option)>, irq_chip: &mut dyn IrqChipX86_64, ) -> std::result::Result, 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(),