From 81ba70a497597e729cc263e1da026c018cfbfe99 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Wed, 8 Jun 2022 16:41:24 +0200 Subject: [PATCH] pci, vmm: Defer mapping VFIO MMIO regions on restore When restoring a VM, the restore codepath will take care of mapping the MMIO regions based on the information from the snapshot, rather than having the mapping being performed during device creation. When the device is created, information such as which BARs contain the MSI-X tables are missing, preventing to perform the mapping of the MMIO regions. Signed-off-by: Sebastien Boeuf --- pci/src/vfio.rs | 25 ++++++++++++---------- pci/src/vfio_user.rs | 27 +++++++++++++---------- vmm/src/device_manager.rs | 45 +++++++++++++++++++++++---------------- 3 files changed, 57 insertions(+), 40 deletions(-) diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 91ca831f0..26e72f7db 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -1139,6 +1139,7 @@ pub struct VfioPciDevice { container: Arc, common: VfioCommon, iommu_attached: bool, + memory_slot: Arc u32 + Send + Sync>, } impl VfioPciDevice { @@ -1154,6 +1155,7 @@ impl VfioPciDevice { iommu_attached: bool, bdf: PciBdf, restoring: bool, + memory_slot: Arc u32 + Send + Sync>, ) -> Result { let device = Arc::new(device); device.reset(); @@ -1201,6 +1203,7 @@ impl VfioPciDevice { container, common, iommu_attached, + memory_slot, }; Ok(vfio_pci_device) @@ -1310,14 +1313,7 @@ impl VfioPciDevice { /// * `vm` - The VM object. It is used to set the VFIO MMIO regions /// as user memory regions. /// * `mem_slot` - The closure to return a memory slot. - pub fn map_mmio_regions( - &mut self, - vm: &Arc, - mem_slot: F, - ) -> Result<(), VfioPciError> - where - F: Fn() -> u32, - { + pub fn map_mmio_regions(&mut self) -> Result<(), VfioPciError> { let fd = self.device.as_raw_fd(); for region in self.common.mmio_regions.iter_mut() { @@ -1383,7 +1379,7 @@ impl VfioPciDevice { } let user_memory_region = UserMemoryRegion { - slot: mem_slot(), + slot: (self.memory_slot)(), start: region.start.0 + area.offset, size: area.size, host_addr: host_addr as u64, @@ -1391,7 +1387,7 @@ impl VfioPciDevice { region.user_memory_regions.push(user_memory_region); - let mem_region = vm.make_user_memory_region( + let mem_region = self.vm.make_user_memory_region( user_memory_region.slot, user_memory_region.start, user_memory_region.size, @@ -1400,7 +1396,8 @@ impl VfioPciDevice { false, ); - vm.create_user_memory_region(mem_region) + self.vm + .create_user_memory_region(mem_region) .map_err(VfioPciError::CreateUserMemoryRegion)?; } } @@ -1645,6 +1642,12 @@ impl Snapshottable for VfioPciDevice { // Restore VfioCommon if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) { self.common.restore(*vfio_common_snapshot.clone())?; + self.map_mmio_regions().map_err(|e| { + MigratableError::Restore(anyhow!( + "Could not map MMIO regions for VfioPciDevice on restore {:?}", + e + )) + })?; } Ok(()) diff --git a/pci/src/vfio_user.rs b/pci/src/vfio_user.rs index 3eb4b53d6..10d00c7e8 100644 --- a/pci/src/vfio_user.rs +++ b/pci/src/vfio_user.rs @@ -8,6 +8,7 @@ use crate::{BarReprogrammingParams, PciBarConfiguration, VfioPciError}; use crate::{ PciBdf, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciSubclass, }; +use anyhow::anyhow; use hypervisor::HypervisorVmError; use std::any::Any; use std::os::unix::prelude::AsRawFd; @@ -34,6 +35,7 @@ pub struct VfioUserPciDevice { vm: Arc, client: Arc>, common: VfioCommon, + memory_slot: Arc u32 + Send + Sync>, } #[derive(Error, Debug)] @@ -62,6 +64,7 @@ impl PciSubclass for PciVfioUserSubclass { } impl VfioUserPciDevice { + #[allow(clippy::too_many_arguments)] pub fn new( id: String, vm: &Arc, @@ -70,6 +73,7 @@ impl VfioUserPciDevice { legacy_interrupt_group: Option>, bdf: PciBdf, restoring: bool, + memory_slot: Arc u32 + Send + Sync>, ) -> Result { // This is used for the BAR and capabilities only let configuration = PciConfiguration::new( @@ -125,17 +129,11 @@ impl VfioUserPciDevice { vm: vm.clone(), client, common, + memory_slot, }) } - pub fn map_mmio_regions( - &mut self, - vm: &Arc, - mem_slot: F, - ) -> Result<(), VfioUserPciDeviceError> - where - F: Fn() -> u32, - { + pub fn map_mmio_regions(&mut self) -> Result<(), VfioUserPciDeviceError> { for mmio_region in &mut self.common.mmio_regions { let region_flags = self .client @@ -202,7 +200,7 @@ impl VfioUserPciDevice { } let user_memory_region = UserMemoryRegion { - slot: mem_slot(), + slot: (self.memory_slot)(), start: mmio_region.start.0 + s.offset, size: s.size, host_addr: host_addr as u64, @@ -210,7 +208,7 @@ impl VfioUserPciDevice { mmio_region.user_memory_regions.push(user_memory_region); - let mem_region = vm.make_user_memory_region( + let mem_region = self.vm.make_user_memory_region( user_memory_region.slot, user_memory_region.start, user_memory_region.size, @@ -219,7 +217,8 @@ impl VfioUserPciDevice { false, ); - vm.create_user_memory_region(mem_region) + self.vm + .create_user_memory_region(mem_region) .map_err(VfioUserPciDeviceError::MapRegionGuest)?; } } @@ -578,6 +577,12 @@ impl Snapshottable for VfioUserPciDevice { // Restore VfioCommon if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) { self.common.restore(*vfio_common_snapshot.clone())?; + self.map_mmio_regions().map_err(|e| { + MigratableError::Restore(anyhow!( + "Could not map MMIO regions for VfioUserPciDevice on restore {:?}", + e + )) + })?; } Ok(()) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 37d272056..bfc207cec 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -16,8 +16,7 @@ use crate::config::{ use crate::device_tree::{DeviceNode, DeviceTree}; use crate::interrupt::LegacyUserspaceInterruptManager; use crate::interrupt::MsiInterruptManager; -use crate::memory_manager::MEMORY_MANAGER_ACPI_SIZE; -use crate::memory_manager::{Error as MemoryManagerError, MemoryManager}; +use crate::memory_manager::{Error as MemoryManagerError, MemoryManager, MEMORY_MANAGER_ACPI_SIZE}; use crate::pci_segment::PciSegment; use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::serial_manager::{Error as SerialManagerError, SerialManager}; @@ -3074,6 +3073,8 @@ impl DeviceManager { None }; + let memory_manager = self.memory_manager.clone(); + let vfio_pci_device = VfioPciDevice::new( vfio_name.clone(), &self.address_manager.vm, @@ -3084,6 +3085,7 @@ impl DeviceManager { device_cfg.iommu, pci_device_bdf, self.restoring, + Arc::new(move || memory_manager.lock().unwrap().allocate_memory_slot()), ) .map_err(DeviceManagerError::VfioPciCreate)?; @@ -3097,13 +3099,15 @@ impl DeviceManager { resources, )?; - vfio_pci_device - .lock() - .unwrap() - .map_mmio_regions(&self.address_manager.vm, || { - self.memory_manager.lock().unwrap().allocate_memory_slot() - }) - .map_err(DeviceManagerError::VfioMapRegion)?; + // When restoring a VM, the restore codepath will take care of mapping + // the MMIO regions based on the information from the snapshot. + if !self.restoring { + vfio_pci_device + .lock() + .unwrap() + .map_mmio_regions() + .map_err(DeviceManagerError::VfioMapRegion)?; + } let mut node = device_node!(vfio_name, vfio_pci_device); @@ -3230,6 +3234,8 @@ impl DeviceManager { .map_err(DeviceManagerError::VfioUserCreateClient)?, )); + let memory_manager = self.memory_manager.clone(); + let mut vfio_user_pci_device = VfioUserPciDevice::new( vfio_user_name.clone(), &self.address_manager.vm, @@ -3238,6 +3244,7 @@ impl DeviceManager { legacy_interrupt_group, pci_device_bdf, self.restoring, + Arc::new(move || memory_manager.lock().unwrap().allocate_memory_slot()), ) .map_err(DeviceManagerError::VfioUserCreate)?; @@ -3272,15 +3279,17 @@ impl DeviceManager { resources, )?; - // Note it is required to call 'add_pci_device()' in advance to have the list of - // mmio regions provisioned correctly - vfio_user_pci_device - .lock() - .unwrap() - .map_mmio_regions(&self.address_manager.vm, || { - self.memory_manager.lock().unwrap().allocate_memory_slot() - }) - .map_err(DeviceManagerError::VfioUserMapRegion)?; + // When restoring a VM, the restore codepath will take care of mapping + // the MMIO regions based on the information from the snapshot. + if !self.restoring { + // Note it is required to call 'add_pci_device()' in advance to have the list of + // mmio regions provisioned correctly + vfio_user_pci_device + .lock() + .unwrap() + .map_mmio_regions() + .map_err(DeviceManagerError::VfioUserMapRegion)?; + } let mut node = device_node!(vfio_user_name, vfio_user_pci_device);