From 6e084572d432d7ac2ee9e3bafbd4cbf9b540ce82 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Thu, 7 Apr 2022 14:22:07 +0200 Subject: [PATCH] pci, virtio: Make virtio-pci BAR restoration more generic Updating the way of restoring BAR addresses for virtio-pci by providing a more generic approach that will be reused for other PciDevice implementations (i.e VfioPcidevice and VfioUserPciDevice). Signed-off-by: Sebastien Boeuf --- pci/src/device.rs | 9 ++- pci/src/vfio.rs | 6 +- pci/src/vfio_user.rs | 5 +- virtio-devices/src/transport/pci_device.rs | 26 ++++---- vmm/src/device_manager.rs | 77 +++++++++------------- 5 files changed, 61 insertions(+), 62 deletions(-) diff --git a/pci/src/device.rs b/pci/src/device.rs index 8f0b955fc..407d31360 100644 --- a/pci/src/device.rs +++ b/pci/src/device.rs @@ -8,7 +8,7 @@ use std::fmt::{self, Display}; use std::sync::{Arc, Barrier, Mutex}; use std::{self, io, result}; use vm_allocator::{AddressAllocator, SystemAllocator}; -use vm_device::BusDevice; +use vm_device::{BusDevice, Resource}; use vm_memory::{GuestAddress, GuestUsize}; #[derive(Debug)] @@ -19,6 +19,10 @@ pub enum Error { IoAllocationFailed(u64), /// Registering an IO BAR failed. IoRegistrationFailed(u64, configuration::Error), + /// Not enough resources + MissingResource, + /// Invalid type of resource + InvalidResourceType, } pub type Result = std::result::Result; @@ -34,6 +38,8 @@ impl Display for Error { IoRegistrationFailed(addr, e) => { write!(f, "failed to register an IO BAR, addr={} err={}", addr, e) } + MissingResource => write!(f, "Missing resource"), + InvalidResourceType => write!(f, "Invalid type of resource"), } } } @@ -53,6 +59,7 @@ pub trait PciDevice: BusDevice { &mut self, _allocator: &Arc>, _mmio_allocator: &mut AddressAllocator, + _resources: Option>, ) -> Result> { Ok(Vec::new()) } diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 07e20cb89..ea9f622b5 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -23,7 +23,7 @@ use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, }; -use vm_device::BusDevice; +use vm_device::{BusDevice, Resource}; use vm_memory::{Address, GuestAddress, GuestUsize}; use vmm_sys_util::eventfd::EventFd; @@ -363,6 +363,7 @@ impl VfioCommon { allocator: &Arc>, mmio_allocator: &mut AddressAllocator, vfio_wrapper: &dyn Vfio, + _resources: Option>, ) -> Result, PciDeviceError> { let mut ranges = Vec::new(); let mut bar_id = VFIO_PCI_BAR0_REGION_INDEX as u32; @@ -1326,9 +1327,10 @@ impl PciDevice for VfioPciDevice { &mut self, allocator: &Arc>, mmio_allocator: &mut AddressAllocator, + resources: Option>, ) -> Result, PciDeviceError> { self.common - .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper) + .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper, resources) } fn free_bars( diff --git a/pci/src/vfio_user.rs b/pci/src/vfio_user.rs index 348a10891..0a572b4ec 100644 --- a/pci/src/vfio_user.rs +++ b/pci/src/vfio_user.rs @@ -21,7 +21,7 @@ use vfio_user::{Client, Error as VfioUserError}; use vm_allocator::{AddressAllocator, SystemAllocator}; use vm_device::dma_mapping::ExternalDmaMapping; use vm_device::interrupt::{InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig}; -use vm_device::BusDevice; +use vm_device::{BusDevice, Resource}; use vm_memory::bitmap::AtomicBitmap; use vm_memory::{ Address, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryRegion, GuestRegionMmap, @@ -392,9 +392,10 @@ impl PciDevice for VfioUserPciDevice { &mut self, allocator: &Arc>, mmio_allocator: &mut AddressAllocator, + resources: Option>, ) -> Result, PciDeviceError> { self.common - .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper) + .allocate_bars(allocator, mmio_allocator, &self.vfio_wrapper, resources) } fn free_bars( diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index a0772c7cb..bcf88911b 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -35,7 +35,7 @@ use vm_device::dma_mapping::ExternalDmaMapping; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, }; -use vm_device::BusDevice; +use vm_device::{BusDevice, Resource}; use vm_memory::{Address, ByteValued, GuestAddress, GuestMemoryAtomic, GuestUsize, Le32}; use vm_migration::{ Migratable, MigratableError, Pausable, Snapshot, Snapshottable, Transportable, VersionMapped, @@ -318,7 +318,6 @@ pub struct VirtioPciDevice { // Settings PCI BAR settings_bar: u8, - settings_bar_addr: Option, // Whether to use 64-bit bar location or 32-bit use_64bit_bar: bool, @@ -452,7 +451,6 @@ impl VirtioPciDevice { queue_evts, memory: Some(memory), settings_bar: 0, - settings_bar_addr: None, use_64bit_bar, interrupt_source_group, cap_pci_cfg_info: VirtioPciCfgCapInfo::default(), @@ -542,12 +540,6 @@ impl VirtioPciDevice { self.common_config.driver_status == DEVICE_INIT as u8 } - // This function is used by the caller to provide the expected base address - // for the virtio-pci configuration BAR. - pub fn set_config_bar_addr(&mut self, bar_addr: u64) { - self.settings_bar_addr = Some(GuestAddress(bar_addr)); - } - pub fn config_bar_addr(&self) -> u64 { self.configuration.get_bar_addr(self.settings_bar as usize) } @@ -847,11 +839,23 @@ impl PciDevice for VirtioPciDevice { &mut self, allocator: &Arc>, mmio_allocator: &mut AddressAllocator, + resources: Option>, ) -> std::result::Result, PciDeviceError> { let mut ranges = Vec::new(); let device_clone = self.device.clone(); let device = device_clone.lock().unwrap(); + let settings_bar_addr = if let Some(resources) = &resources { + if resources.is_empty() { + return Err(PciDeviceError::MissingResource); + } + match resources[0] { + Resource::MmioAddressRange { base, .. } => Some(GuestAddress(base)), + _ => return Err(PciDeviceError::InvalidResourceType), + } + } else { + None + }; // Allocate the virtio-pci capability BAR. // See http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-740004 @@ -859,7 +863,7 @@ impl PciDevice for VirtioPciDevice { let region_type = PciBarRegionType::Memory64BitRegion; let addr = mmio_allocator .allocate( - self.settings_bar_addr, + settings_bar_addr, CAPABILITY_BAR_SIZE, Some(CAPABILITY_BAR_SIZE), ) @@ -872,7 +876,7 @@ impl PciDevice for VirtioPciDevice { .lock() .unwrap() .allocate_mmio_hole_addresses( - self.settings_bar_addr, + settings_bar_addr, CAPABILITY_BAR_SIZE, Some(CAPABILITY_BAR_SIZE), ) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index aaf697f51..b0612e6be 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -399,9 +399,6 @@ pub enum DeviceManagerError { /// Resource was already found. ResourceAlreadyExists, - /// Expected resources for virtio-pci could not be found. - MissingVirtioPciResources, - /// Expected resources for virtio-pmem could not be found. MissingVirtioPmemResources, @@ -3204,6 +3201,7 @@ impl DeviceManager { vfio_pci_device.clone(), pci_segment_id, pci_device_bdf, + None, )?; vfio_pci_device @@ -3240,6 +3238,7 @@ impl DeviceManager { pci_device: Arc>, segment_id: u16, bdf: PciBdf, + resources: Option>, ) -> DeviceManagerResult> { let bars = pci_device .lock() @@ -3250,6 +3249,7 @@ impl DeviceManager { .allocator .lock() .unwrap(), + resources, ) .map_err(DeviceManagerError::AllocateBars)?; @@ -3378,6 +3378,7 @@ impl DeviceManager { vfio_user_pci_device.clone(), pci_segment_id, pci_device_bdf, + None, )?; let mut node = device_node!(vfio_user_name); @@ -3424,7 +3425,7 @@ impl DeviceManager { // Look for the id in the device tree. If it can be found, that means // the device is being restored, otherwise it's created from scratch. - let (pci_segment_id, pci_device_bdf, config_bar_addr) = if let Some(node) = + let (pci_segment_id, pci_device_bdf, resources) = if let Some(node) = self.device_tree.lock().unwrap().get(&id) { info!("Restoring virtio-pci {} resources", id); @@ -3440,21 +3441,7 @@ impl DeviceManager { .get_device_id(pci_device_bdf.device() as usize) .map_err(DeviceManagerError::GetPciDeviceId)?; - if node.resources.is_empty() { - return Err(DeviceManagerError::MissingVirtioPciResources); - } - - // We know the configuration BAR address is stored on the first - // resource in the list. - let config_bar_addr = match node.resources[0] { - Resource::MmioAddressRange { base, .. } => Some(base), - _ => { - error!("Unexpected resource {:?} for {}", node.resources[0], id); - return Err(DeviceManagerError::MissingVirtioPciResources); - } - }; - - (pci_segment_id, pci_device_bdf, config_bar_addr) + (pci_segment_id, pci_device_bdf, Some(node.resources.clone())) } else { let pci_device_bdf = self.pci_segments[pci_segment_id as usize].next_device_bdf()?; @@ -3529,38 +3516,34 @@ impl DeviceManager { } let device_type = virtio_device.lock().unwrap().device_type(); - let mut virtio_pci_device = VirtioPciDevice::new( - id.clone(), - memory, - virtio_device, - msix_num, - access_platform, - &self.msi_interrupt_manager, - pci_device_bdf.into(), - self.activate_evt - .try_clone() - .map_err(DeviceManagerError::EventFd)?, - // All device types *except* virtio block devices should be allocated a 64-bit bar - // The block devices should be given a 32-bit BAR so that they are easily accessible - // to firmware without requiring excessive identity mapping. - // The exception being if not on the default PCI segment. - pci_segment_id > 0 || device_type != VirtioDeviceType::Block as u32, - dma_handler, - ) - .map_err(DeviceManagerError::VirtioDevice)?; + let virtio_pci_device = Arc::new(Mutex::new( + VirtioPciDevice::new( + id.clone(), + memory, + virtio_device, + msix_num, + access_platform, + &self.msi_interrupt_manager, + pci_device_bdf.into(), + self.activate_evt + .try_clone() + .map_err(DeviceManagerError::EventFd)?, + // All device types *except* virtio block devices should be allocated a 64-bit bar + // The block devices should be given a 32-bit BAR so that they are easily accessible + // to firmware without requiring excessive identity mapping. + // The exception being if not on the default PCI segment. + pci_segment_id > 0 || device_type != VirtioDeviceType::Block as u32, + dma_handler, + ) + .map_err(DeviceManagerError::VirtioDevice)?, + )); - // This is important as this will set the BAR address if it exists, - // which is mandatory on the restore path. - if let Some(addr) = config_bar_addr { - virtio_pci_device.set_config_bar_addr(addr); - } - - let virtio_pci_device = Arc::new(Mutex::new(virtio_pci_device)); let bars = self.add_pci_device( virtio_pci_device.clone(), virtio_pci_device.clone(), pci_segment_id, pci_device_bdf, + resources, )?; let bar_addr = virtio_pci_device.lock().unwrap().config_bar_addr(); @@ -3573,12 +3556,14 @@ impl DeviceManager { } // Update the device tree with correct resource information. + let mut new_resources = Vec::new(); for pci_bar in bars.iter() { - node.resources.push(Resource::MmioAddressRange { + new_resources.push(Resource::MmioAddressRange { base: pci_bar.0.raw_value(), size: pci_bar.1 as u64, }); } + node.resources = new_resources; node.migratable = Some(Arc::clone(&virtio_pci_device) as Arc>); node.pci_bdf = Some(pci_device_bdf); node.pci_device_handle = Some(PciDeviceHandle::Virtio(virtio_pci_device));