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);