From cd9d1cf8fc9723dfa61dd1a027779335a15f55c1 Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Fri, 15 Oct 2021 11:38:06 +0100 Subject: [PATCH] pci, virtio-devices, vmm: Allocate PCI 64-bit bars per segment Since each segment must have a non-overlapping memory range associated with it the device memory must be equally divided amongst all segments. A new allocator is used for each segment to ensure that BARs are allocated from the correct address ranges. This requires changes to PciDevice::allocate/free_bars to take that allocator and when reallocating BARs the correct allocator must be identified from the ranges. Signed-off-by: Rob Bradford --- pci/src/device.rs | 9 +- pci/src/vfio.rs | 22 ++-- pci/src/vfio_user.rs | 14 ++- virtio-devices/src/transport/pci_device.rs | 10 +- vmm/src/device_manager.rs | 113 ++++++++++++++------- vmm/src/pci_segment.rs | 32 +++--- 6 files changed, 127 insertions(+), 73 deletions(-) diff --git a/pci/src/device.rs b/pci/src/device.rs index 429458c6a..99a4602b4 100644 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -7,7 +7,7 @@ use std::any::Any; use std::fmt::{self, Display}; use std::sync::{Arc, Barrier}; use std::{self, io, result}; -use vm_allocator::SystemAllocator; +use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::BusDevice; use vm_memory::{GuestAddress, GuestUsize}; @@ -52,12 +52,17 @@ pub trait PciDevice: BusDevice { fn allocate_bars( &mut self, _allocator: &mut SystemAllocator, + _mmio_allocator: &mut AddressAllocator, ) -> Result> { Ok(Vec::new()) } /// Frees the PCI BARs previously allocated with a call to allocate_bars(). - fn free_bars(&mut self, _allocator: &mut SystemAllocator) -> Result<()> { + fn free_bars( + &mut self, + _allocator: &mut SystemAllocator, + _mmio_allocator: &mut AddressAllocator, + ) -> Result<()> { Ok(()) } diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 3ce1dd13f..9c813c3f2 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -19,7 +19,7 @@ use std::sync::{Arc, Barrier}; use thiserror::Error; use vfio_bindings::bindings::vfio::*; use vfio_ioctls::{VfioContainer, VfioDevice, VfioIrq}; -use vm_allocator::SystemAllocator; +use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, }; @@ -361,6 +361,7 @@ impl VfioCommon { pub(crate) fn allocate_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, vfio_wrapper: &dyn Vfio, ) -> Result, PciDeviceError> { let mut ranges = Vec::new(); @@ -458,8 +459,8 @@ impl VfioCommon { region_size = (!combined_size + 1) as u64; // BAR allocation must be naturally aligned - bar_addr = allocator - .allocate_mmio_addresses(None, region_size, Some(region_size)) + bar_addr = mmio_allocator + .allocate(None, region_size, Some(region_size)) .ok_or(PciDeviceError::IoAllocationFailed(region_size))?; } else { // Mask out flag bits (lowest 4 for memory bars) @@ -526,6 +527,7 @@ impl VfioCommon { pub(crate) fn free_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, ) -> Result<(), PciDeviceError> { for region in self.mmio_regions.iter() { match region.type_ { @@ -539,7 +541,7 @@ impl VfioCommon { allocator.free_mmio_hole_addresses(region.start, region.length); } PciBarRegionType::Memory64BitRegion => { - allocator.free_mmio_addresses(region.start, region.length); + mmio_allocator.free(region.start, region.length); } } } @@ -1293,12 +1295,18 @@ impl PciDevice for VfioPciDevice { fn allocate_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, ) -> Result, PciDeviceError> { - self.common.allocate_bars(allocator, &self.vfio_wrapper) + self.common + .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper) } - fn free_bars(&mut self, allocator: &mut SystemAllocator) -> Result<(), PciDeviceError> { - self.common.free_bars(allocator) + fn free_bars( + &mut self, + allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, + ) -> Result<(), PciDeviceError> { + self.common.free_bars(allocator, mmio_allocator) } fn write_config_register( diff --git a/pci/src/vfio_user.rs b/pci/src/vfio_user.rs index 29887de0a..b8c867201 100644 --- a/pci/src/vfio_user.rs +++ b/pci/src/vfio_user.rs @@ -18,7 +18,7 @@ use thiserror::Error; use vfio_bindings::bindings::vfio::*; use vfio_ioctls::VfioIrq; use vfio_user::{Client, Error as VfioUserError}; -use vm_allocator::SystemAllocator; +use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::dma_mapping::ExternalDmaMapping; use vm_device::interrupt::{InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig}; use vm_device::BusDevice; @@ -234,12 +234,18 @@ impl PciDevice for VfioUserPciDevice { fn allocate_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, ) -> Result, PciDeviceError> { - self.common.allocate_bars(allocator, &self.vfio_wrapper) + self.common + .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper) } - fn free_bars(&mut self, allocator: &mut SystemAllocator) -> Result<(), PciDeviceError> { - self.common.free_bars(allocator) + fn free_bars( + &mut self, + allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, + ) -> Result<(), PciDeviceError> { + self.common.free_bars(allocator, mmio_allocator) } fn as_any(&mut self) -> &mut dyn Any { diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 7da606ba7..498d5d8af 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -31,7 +31,7 @@ use versionize::{VersionMap, Versionize, VersionizeResult}; use versionize_derive::Versionize; use virtio_queue::AccessPlatform; use virtio_queue::{defs::VIRTQ_MSI_NO_VECTOR, Error as QueueError, Queue}; -use vm_allocator::SystemAllocator; +use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, }; @@ -842,6 +842,7 @@ impl PciDevice for VirtioPciDevice { fn allocate_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, ) -> std::result::Result, PciDeviceError> { let mut ranges = Vec::new(); @@ -852,8 +853,8 @@ impl PciDevice for VirtioPciDevice { // See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004 let (virtio_pci_bar_addr, region_type) = if self.use_64bit_bar { let region_type = PciBarRegionType::Memory64BitRegion; - let addr = allocator - .allocate_mmio_addresses( + let addr = mmio_allocator + .allocate( self.settings_bar_addr, CAPABILITY_BAR_SIZE, Some(CAPABILITY_BAR_SIZE), @@ -925,6 +926,7 @@ impl PciDevice for VirtioPciDevice { fn free_bars( &mut self, allocator: &mut SystemAllocator, + mmio_allocator: &mut AddressAllocator, ) -> std::result::Result<(), PciDeviceError> { for (addr, length, type_) in self.bar_regions.drain(..) { match type_ { @@ -932,7 +934,7 @@ impl PciDevice for VirtioPciDevice { allocator.free_mmio_hole_addresses(addr, length); } PciBarRegionType::Memory64BitRegion => { - allocator.free_mmio_addresses(addr, length); + mmio_allocator.free(addr, length); } _ => error!("Unexpected PCI bar type"), } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index c91685139..3c9f732ad 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -93,7 +93,7 @@ use virtio_devices::{AccessPlatformMapping, VirtioMemMappingSource}; use virtio_devices::{Endpoint, IommuMapping}; use virtio_devices::{VirtioSharedMemory, VirtioSharedMemoryList}; use virtio_queue::AccessPlatform; -use vm_allocator::SystemAllocator; +use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::dma_mapping::vfio::VfioDmaMapping; use vm_device::interrupt::{ InterruptIndex, InterruptManager, LegacyIrqGroupConfig, MsiIrqGroupConfig, @@ -540,6 +540,7 @@ pub(crate) struct AddressManager { pub(crate) mmio_bus: Arc, vm: Arc, device_tree: Arc>, + pci_mmio_allocators: Vec>>, } impl DeviceRelocation for AddressManager { @@ -604,25 +605,35 @@ impl DeviceRelocation for AddressManager { ) })?; } else { - self.allocator - .lock() - .unwrap() - .free_mmio_addresses(GuestAddress(old_base), len as GuestUsize); + // Find the specific allocator that this BAR was allocated from and use it for new one + for allocator in &self.pci_mmio_allocators { + let allocator_base = allocator.lock().unwrap().base(); + let allocator_end = allocator.lock().unwrap().end(); - self.allocator - .lock() - .unwrap() - .allocate_mmio_addresses( - Some(GuestAddress(new_base)), - len as GuestUsize, - Some(len), - ) - .ok_or_else(|| { - io::Error::new( - io::ErrorKind::Other, - "failed allocating new 64 bits MMIO range", - ) - })?; + if old_base >= allocator_base.0 && old_base <= allocator_end.0 { + allocator + .lock() + .unwrap() + .free(GuestAddress(old_base), len as GuestUsize); + + allocator + .lock() + .unwrap() + .allocate( + Some(GuestAddress(new_base)), + len as GuestUsize, + Some(len), + ) + .ok_or_else(|| { + io::Error::new( + io::ErrorKind::Other, + "failed allocating new 64 bits MMIO range", + ) + })?; + + break; + } + } } // Update MMIO bus @@ -916,6 +927,30 @@ impl DeviceManager { ) -> DeviceManagerResult>> { let device_tree = Arc::new(Mutex::new(DeviceTree::new())); + let num_pci_segments = + if let Some(platform_config) = config.lock().unwrap().platform.as_ref() { + platform_config.num_pci_segments + } else { + 1 + }; + + let start_of_device_area = memory_manager.lock().unwrap().start_of_device_area().0; + let end_of_device_area = memory_manager.lock().unwrap().end_of_device_area().0; + + // Start each PCI segment range on a 4GiB boundary + let pci_segment_size = (end_of_device_area - start_of_device_area + 1) + / ((4 << 30) * num_pci_segments as u64) + * (4 << 30); + + let mut pci_mmio_allocators = vec![]; + for i in 0..num_pci_segments as u64 { + let mmio_start = start_of_device_area + i * pci_segment_size; + let allocator = Arc::new(Mutex::new( + AddressAllocator::new(GuestAddress(mmio_start), pci_segment_size).unwrap(), + )); + pci_mmio_allocators.push(allocator) + } + let address_manager = Arc::new(AddressManager { allocator: memory_manager.lock().unwrap().allocator(), #[cfg(target_arch = "x86_64")] @@ -923,6 +958,7 @@ impl DeviceManager { mmio_bus: Arc::new(Bus::new()), vm: vm.clone(), device_tree: Arc::clone(&device_tree), + pci_mmio_allocators, }); // First we create the MSI interrupt manager, the legacy one is created @@ -945,9 +981,6 @@ impl DeviceManager { .allocate_mmio_addresses(None, DEVICE_MANAGER_ACPI_SIZE as u64, None) .ok_or(DeviceManagerError::AllocateIoPort)?; - let start_of_device_area = memory_manager.lock().unwrap().start_of_device_area().0; - let end_of_device_area = memory_manager.lock().unwrap().end_of_device_area().0; - let mut pci_irq_slots = [0; 32]; PciSegment::reserve_legacy_interrupts_for_pci_devices( &address_manager, @@ -956,21 +989,17 @@ impl DeviceManager { let mut pci_segments = vec![PciSegment::new_default_segment( &address_manager, - start_of_device_area, - end_of_device_area, + Arc::clone(&address_manager.pci_mmio_allocators[0]), &pci_irq_slots, )?]; - if let Some(platform_config) = config.lock().unwrap().platform.as_ref() { - for i in 1..platform_config.num_pci_segments { - pci_segments.push(PciSegment::new( - i as u16, - &address_manager, - start_of_device_area, - end_of_device_area, - &pci_irq_slots, - )?); - } + for i in 1..num_pci_segments as usize { + pci_segments.push(PciSegment::new( + i as u16, + &address_manager, + Arc::clone(&address_manager.pci_mmio_allocators[i]), + &pci_irq_slots, + )?); } let device_manager = DeviceManager { @@ -3013,7 +3042,13 @@ impl DeviceManager { let bars = pci_device .lock() .unwrap() - .allocate_bars(&mut self.address_manager.allocator.lock().unwrap()) + .allocate_bars( + &mut self.address_manager.allocator.lock().unwrap(), + &mut self.pci_segments[segment_id as usize] + .allocator + .lock() + .unwrap(), + ) .map_err(DeviceManagerError::AllocateBars)?; let mut pci_bus = self.pci_segments[segment_id as usize] @@ -3597,7 +3632,13 @@ impl DeviceManager { pci_device .lock() .unwrap() - .free_bars(&mut self.address_manager.allocator.lock().unwrap()) + .free_bars( + &mut self.address_manager.allocator.lock().unwrap(), + &mut self.pci_segments[pci_segment_id as usize] + .allocator + .lock() + .unwrap(), + ) .map_err(DeviceManagerError::FreePciBars)?; // Remove the device from the PCI bus diff --git a/vmm/src/pci_segment.rs b/vmm/src/pci_segment.rs index 89865b435..2ea40aaa3 100644 --- a/vmm/src/pci_segment.rs +++ b/vmm/src/pci_segment.rs @@ -19,6 +19,7 @@ use pci::{PciConfigIo, PCI_CONFIG_IO_PORT, PCI_CONFIG_IO_PORT_SIZE}; use std::sync::{Arc, Mutex}; #[cfg(feature = "acpi")] use uuid::Uuid; +use vm_allocator::AddressAllocator; use vm_device::BusDevice; // One bus with potentially 256 devices (32 slots x 8 functions). @@ -43,14 +44,15 @@ pub(crate) struct PciSegment { // Device memory covered by this segment pub(crate) start_of_device_area: u64, pub(crate) end_of_device_area: u64, + + pub(crate) allocator: Arc>, } impl PciSegment { pub(crate) fn new( id: u16, address_manager: &Arc, - start_of_device_area: u64, - end_of_device_area: u64, + allocator: Arc>, pci_irq_slots: &[u8; 32], ) -> DeviceManagerResult { let pci_root = PciRoot::new(None); @@ -71,6 +73,9 @@ impl PciSegment { ) .map_err(DeviceManagerError::BusError)?; + let start_of_device_area = allocator.lock().unwrap().base().0; + let end_of_device_area = allocator.lock().unwrap().end().0; + let segment = PciSegment { id, pci_bus, @@ -80,6 +85,7 @@ impl PciSegment { pci_devices_down: 0, #[cfg(target_arch = "x86_64")] pci_config_io: None, + allocator, start_of_device_area, end_of_device_area, pci_irq_slots: *pci_irq_slots, @@ -95,17 +101,10 @@ impl PciSegment { #[cfg(target_arch = "x86_64")] pub(crate) fn new_default_segment( address_manager: &Arc, - start_of_device_area: u64, - end_of_device_area: u64, + allocator: Arc>, pci_irq_slots: &[u8; 32], ) -> DeviceManagerResult { - let mut segment = Self::new( - 0, - address_manager, - start_of_device_area, - end_of_device_area, - pci_irq_slots, - )?; + let mut segment = Self::new(0, address_manager, allocator, pci_irq_slots)?; let pci_config_io = Arc::new(Mutex::new(PciConfigIo::new(Arc::clone(&segment.pci_bus)))); address_manager @@ -125,17 +124,10 @@ impl PciSegment { #[cfg(target_arch = "aarch64")] pub(crate) fn new_default_segment( address_manager: &Arc, - start_of_device_area: u64, - end_of_device_area: u64, + allocator: Arc>, pci_irq_slots: &[u8; 32], ) -> DeviceManagerResult { - Self::new( - 0, - address_manager, - start_of_device_area, - end_of_device_area, - pci_irq_slots, - ) + Self::new(0, address_manager, allocator, pci_irq_slots) } pub(crate) fn next_device_bdf(&self) -> DeviceManagerResult {