From 0076579e9553eeb3efaecd279148a80fec294da4 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 1 Jul 2022 13:53:24 -0700 Subject: [PATCH] arch: allocate reserved PCI addresses first Add a new PciDevice::preferred_address() function, which allows devices to report a PCI address where they would prefer to be allocated. This lets the arch layer pick out the devices that need to have their addresses reserved before allocating the remaining addresses to device that don't care about their assigned PCI address. For now, the returned preferred_address value is not actually used aside from checking is_some(), but it will be used in upcoming cleanups. BUG=b:237415650 TEST=Run x86-64 bzImage Change-Id: Ia323274dcf801a8fe1959287bcc791ca32ce4e18 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3781441 Tested-by: Daniel Verkamp Reviewed-by: Keiichi Watanabe Commit-Queue: Daniel Verkamp --- ARCHITECTURE.md | 2 ++ arch/src/lib.rs | 30 +++++++++++++++++++++++++ devices/src/pci/pci_device.rs | 12 ++++++++++ devices/src/pci/stub.rs | 4 ++++ devices/src/virtio/virtio_pci_device.rs | 4 ++++ src/crosvm/sys/unix.rs | 2 ++ 6 files changed, 54 insertions(+) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f888b7f8e5..12bd880eb1 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -18,6 +18,8 @@ VM. Broken down into rough steps: 1. `Arch::build_vm` will itself invoke the provided `create_devices` function from `linux/mod.rs` 1. `create_devices` creates every PCI device, including the virtio devices, that were configured in `Config`, along with matching [minijail] configs for each. +1. `Arch::assign_pci_addresses` assigns an address to each PCI device, prioritizing devices that + report a preferred slot by implementing the `PciDevice` trait's `preferred_address` function. 1. `Arch::generate_pci_root`, using a list of every PCI device with optional `Minijail`, will finally jail the PCI devices and construct a `PciRoot` that communicates with them. 1. Once the VM has been built, it's contained within a `RunnableLinuxVm` object that is used by the diff --git a/arch/src/lib.rs b/arch/src/lib.rs index f2055598be..0f2f125ac0 100644 --- a/arch/src/lib.rs +++ b/arch/src/lib.rs @@ -593,6 +593,36 @@ pub fn generate_pci_topology( Ok((bar_ranges, subordinate_bus)) } +/// Ensure all PCI devices have an assigned PCI address. +pub fn assign_pci_addresses( + devices: &mut [(Box, Option)], + resources: &mut SystemAllocator, +) -> Result<(), DeviceRegistrationError> { + // First allocate devices with a preferred address. + for pci_device in devices + .iter_mut() + .filter_map(|(device, _jail)| device.as_pci_device_mut()) + .filter(|pci_device| pci_device.preferred_address().is_some()) + { + let _ = pci_device + .allocate_address(resources) + .map_err(DeviceRegistrationError::AllocateDeviceAddrs)?; + } + + // Then allocate addresses for the remaining devices. + for pci_device in devices + .iter_mut() + .filter_map(|(device, _jail)| device.as_pci_device_mut()) + .filter(|pci_device| pci_device.preferred_address().is_none()) + { + let _ = pci_device + .allocate_address(resources) + .map_err(DeviceRegistrationError::AllocateDeviceAddrs)?; + } + + Ok(()) +} + /// Creates a root PCI device for use by this Vm. pub fn generate_pci_root( mut devices: Vec<(Box, Option)>, diff --git a/devices/src/pci/pci_device.rs b/devices/src/pci/pci_device.rs index de59f14cae..7a88429898 100644 --- a/devices/src/pci/pci_device.rs +++ b/devices/src/pci/pci_device.rs @@ -287,8 +287,17 @@ impl PciBus { pub trait PciDevice: Send { /// Returns a label suitable for debug output. fn debug_label(&self) -> String; + + /// Preferred PCI address for this device, if any. + fn preferred_address(&self) -> Option { + None + } + /// Allocate and return an unique bus, device and function number for this device. + /// May be called multiple times; on subsequent calls, the device should return the same + /// address it returned from the first call. fn allocate_address(&mut self, resources: &mut SystemAllocator) -> Result; + /// A vector of device-specific file descriptors that must be kept open /// after jailing. Must be called before the process is jailed. fn keep_rds(&self) -> Vec; @@ -571,6 +580,9 @@ impl PciDevice for Box { fn debug_label(&self) -> String { (**self).debug_label() } + fn preferred_address(&self) -> Option { + (**self).preferred_address() + } fn allocate_address(&mut self, resources: &mut SystemAllocator) -> Result { (**self).allocate_address(resources) } diff --git a/devices/src/pci/stub.rs b/devices/src/pci/stub.rs index 0643150c93..29ef1d0549 100644 --- a/devices/src/pci/stub.rs +++ b/devices/src/pci/stub.rs @@ -93,6 +93,10 @@ impl PciDevice for StubPciDevice { "Stub".to_owned() } + fn preferred_address(&self) -> Option { + Some(self.requested_address) + } + fn allocate_address(&mut self, resources: &mut SystemAllocator) -> Result { if self.assigned_address.is_none() { if resources.reserve_pci( diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index beb357edef..d0e071ac6d 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/devices/src/virtio/virtio_pci_device.rs @@ -540,6 +540,10 @@ impl PciDevice for VirtioPciDevice { format!("pci{}", self.device.debug_label()) } + fn preferred_address(&self) -> Option { + self.preferred_address + } + fn allocate_address( &mut self, resources: &mut SystemAllocator, diff --git a/src/crosvm/sys/unix.rs b/src/crosvm/sys/unix.rs index a891c6dfa3..09573422fd 100644 --- a/src/crosvm/sys/unix.rs +++ b/src/crosvm/sys/unix.rs @@ -1610,6 +1610,8 @@ where )?; } + arch::assign_pci_addresses(&mut devices, &mut sys_allocator)?; + let (translate_response_senders, request_rx) = setup_virtio_access_platform( &mut sys_allocator, &mut iommu_attached_endpoints,