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 <sebastien.boeuf@intel.com>
This commit is contained in:
Sebastien Boeuf 2022-06-08 16:41:24 +02:00
parent 7df7061610
commit 81ba70a497
3 changed files with 57 additions and 40 deletions

View File

@ -1139,6 +1139,7 @@ pub struct VfioPciDevice {
container: Arc<VfioContainer>, container: Arc<VfioContainer>,
common: VfioCommon, common: VfioCommon,
iommu_attached: bool, iommu_attached: bool,
memory_slot: Arc<dyn Fn() -> u32 + Send + Sync>,
} }
impl VfioPciDevice { impl VfioPciDevice {
@ -1154,6 +1155,7 @@ impl VfioPciDevice {
iommu_attached: bool, iommu_attached: bool,
bdf: PciBdf, bdf: PciBdf,
restoring: bool, restoring: bool,
memory_slot: Arc<dyn Fn() -> u32 + Send + Sync>,
) -> Result<Self, VfioPciError> { ) -> Result<Self, VfioPciError> {
let device = Arc::new(device); let device = Arc::new(device);
device.reset(); device.reset();
@ -1201,6 +1203,7 @@ impl VfioPciDevice {
container, container,
common, common,
iommu_attached, iommu_attached,
memory_slot,
}; };
Ok(vfio_pci_device) Ok(vfio_pci_device)
@ -1310,14 +1313,7 @@ impl VfioPciDevice {
/// * `vm` - The VM object. It is used to set the VFIO MMIO regions /// * `vm` - The VM object. It is used to set the VFIO MMIO regions
/// as user memory regions. /// as user memory regions.
/// * `mem_slot` - The closure to return a memory slot. /// * `mem_slot` - The closure to return a memory slot.
pub fn map_mmio_regions<F>( pub fn map_mmio_regions(&mut self) -> Result<(), VfioPciError> {
&mut self,
vm: &Arc<dyn hypervisor::Vm>,
mem_slot: F,
) -> Result<(), VfioPciError>
where
F: Fn() -> u32,
{
let fd = self.device.as_raw_fd(); let fd = self.device.as_raw_fd();
for region in self.common.mmio_regions.iter_mut() { for region in self.common.mmio_regions.iter_mut() {
@ -1383,7 +1379,7 @@ impl VfioPciDevice {
} }
let user_memory_region = UserMemoryRegion { let user_memory_region = UserMemoryRegion {
slot: mem_slot(), slot: (self.memory_slot)(),
start: region.start.0 + area.offset, start: region.start.0 + area.offset,
size: area.size, size: area.size,
host_addr: host_addr as u64, host_addr: host_addr as u64,
@ -1391,7 +1387,7 @@ impl VfioPciDevice {
region.user_memory_regions.push(user_memory_region); 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.slot,
user_memory_region.start, user_memory_region.start,
user_memory_region.size, user_memory_region.size,
@ -1400,7 +1396,8 @@ impl VfioPciDevice {
false, false,
); );
vm.create_user_memory_region(mem_region) self.vm
.create_user_memory_region(mem_region)
.map_err(VfioPciError::CreateUserMemoryRegion)?; .map_err(VfioPciError::CreateUserMemoryRegion)?;
} }
} }
@ -1645,6 +1642,12 @@ impl Snapshottable for VfioPciDevice {
// Restore VfioCommon // Restore VfioCommon
if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) { if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) {
self.common.restore(*vfio_common_snapshot.clone())?; 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(()) Ok(())

View File

@ -8,6 +8,7 @@ use crate::{BarReprogrammingParams, PciBarConfiguration, VfioPciError};
use crate::{ use crate::{
PciBdf, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciSubclass, PciBdf, PciClassCode, PciConfiguration, PciDevice, PciDeviceError, PciHeaderType, PciSubclass,
}; };
use anyhow::anyhow;
use hypervisor::HypervisorVmError; use hypervisor::HypervisorVmError;
use std::any::Any; use std::any::Any;
use std::os::unix::prelude::AsRawFd; use std::os::unix::prelude::AsRawFd;
@ -34,6 +35,7 @@ pub struct VfioUserPciDevice {
vm: Arc<dyn hypervisor::Vm>, vm: Arc<dyn hypervisor::Vm>,
client: Arc<Mutex<Client>>, client: Arc<Mutex<Client>>,
common: VfioCommon, common: VfioCommon,
memory_slot: Arc<dyn Fn() -> u32 + Send + Sync>,
} }
#[derive(Error, Debug)] #[derive(Error, Debug)]
@ -62,6 +64,7 @@ impl PciSubclass for PciVfioUserSubclass {
} }
impl VfioUserPciDevice { impl VfioUserPciDevice {
#[allow(clippy::too_many_arguments)]
pub fn new( pub fn new(
id: String, id: String,
vm: &Arc<dyn hypervisor::Vm>, vm: &Arc<dyn hypervisor::Vm>,
@ -70,6 +73,7 @@ impl VfioUserPciDevice {
legacy_interrupt_group: Option<Arc<dyn InterruptSourceGroup>>, legacy_interrupt_group: Option<Arc<dyn InterruptSourceGroup>>,
bdf: PciBdf, bdf: PciBdf,
restoring: bool, restoring: bool,
memory_slot: Arc<dyn Fn() -> u32 + Send + Sync>,
) -> Result<Self, VfioUserPciDeviceError> { ) -> Result<Self, VfioUserPciDeviceError> {
// This is used for the BAR and capabilities only // This is used for the BAR and capabilities only
let configuration = PciConfiguration::new( let configuration = PciConfiguration::new(
@ -125,17 +129,11 @@ impl VfioUserPciDevice {
vm: vm.clone(), vm: vm.clone(),
client, client,
common, common,
memory_slot,
}) })
} }
pub fn map_mmio_regions<F>( pub fn map_mmio_regions(&mut self) -> Result<(), VfioUserPciDeviceError> {
&mut self,
vm: &Arc<dyn hypervisor::Vm>,
mem_slot: F,
) -> Result<(), VfioUserPciDeviceError>
where
F: Fn() -> u32,
{
for mmio_region in &mut self.common.mmio_regions { for mmio_region in &mut self.common.mmio_regions {
let region_flags = self let region_flags = self
.client .client
@ -202,7 +200,7 @@ impl VfioUserPciDevice {
} }
let user_memory_region = UserMemoryRegion { let user_memory_region = UserMemoryRegion {
slot: mem_slot(), slot: (self.memory_slot)(),
start: mmio_region.start.0 + s.offset, start: mmio_region.start.0 + s.offset,
size: s.size, size: s.size,
host_addr: host_addr as u64, host_addr: host_addr as u64,
@ -210,7 +208,7 @@ impl VfioUserPciDevice {
mmio_region.user_memory_regions.push(user_memory_region); 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.slot,
user_memory_region.start, user_memory_region.start,
user_memory_region.size, user_memory_region.size,
@ -219,7 +217,8 @@ impl VfioUserPciDevice {
false, false,
); );
vm.create_user_memory_region(mem_region) self.vm
.create_user_memory_region(mem_region)
.map_err(VfioUserPciDeviceError::MapRegionGuest)?; .map_err(VfioUserPciDeviceError::MapRegionGuest)?;
} }
} }
@ -578,6 +577,12 @@ impl Snapshottable for VfioUserPciDevice {
// Restore VfioCommon // Restore VfioCommon
if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) { if let Some(vfio_common_snapshot) = snapshot.snapshots.get(&self.common.id()) {
self.common.restore(*vfio_common_snapshot.clone())?; 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(()) Ok(())

View File

@ -16,8 +16,7 @@ use crate::config::{
use crate::device_tree::{DeviceNode, DeviceTree}; use crate::device_tree::{DeviceNode, DeviceTree};
use crate::interrupt::LegacyUserspaceInterruptManager; use crate::interrupt::LegacyUserspaceInterruptManager;
use crate::interrupt::MsiInterruptManager; use crate::interrupt::MsiInterruptManager;
use crate::memory_manager::MEMORY_MANAGER_ACPI_SIZE; use crate::memory_manager::{Error as MemoryManagerError, MemoryManager, MEMORY_MANAGER_ACPI_SIZE};
use crate::memory_manager::{Error as MemoryManagerError, MemoryManager};
use crate::pci_segment::PciSegment; use crate::pci_segment::PciSegment;
use crate::seccomp_filters::{get_seccomp_filter, Thread}; use crate::seccomp_filters::{get_seccomp_filter, Thread};
use crate::serial_manager::{Error as SerialManagerError, SerialManager}; use crate::serial_manager::{Error as SerialManagerError, SerialManager};
@ -3074,6 +3073,8 @@ impl DeviceManager {
None None
}; };
let memory_manager = self.memory_manager.clone();
let vfio_pci_device = VfioPciDevice::new( let vfio_pci_device = VfioPciDevice::new(
vfio_name.clone(), vfio_name.clone(),
&self.address_manager.vm, &self.address_manager.vm,
@ -3084,6 +3085,7 @@ impl DeviceManager {
device_cfg.iommu, device_cfg.iommu,
pci_device_bdf, pci_device_bdf,
self.restoring, self.restoring,
Arc::new(move || memory_manager.lock().unwrap().allocate_memory_slot()),
) )
.map_err(DeviceManagerError::VfioPciCreate)?; .map_err(DeviceManagerError::VfioPciCreate)?;
@ -3097,13 +3099,15 @@ impl DeviceManager {
resources, resources,
)?; )?;
vfio_pci_device // When restoring a VM, the restore codepath will take care of mapping
.lock() // the MMIO regions based on the information from the snapshot.
.unwrap() if !self.restoring {
.map_mmio_regions(&self.address_manager.vm, || { vfio_pci_device
self.memory_manager.lock().unwrap().allocate_memory_slot() .lock()
}) .unwrap()
.map_err(DeviceManagerError::VfioMapRegion)?; .map_mmio_regions()
.map_err(DeviceManagerError::VfioMapRegion)?;
}
let mut node = device_node!(vfio_name, vfio_pci_device); let mut node = device_node!(vfio_name, vfio_pci_device);
@ -3230,6 +3234,8 @@ impl DeviceManager {
.map_err(DeviceManagerError::VfioUserCreateClient)?, .map_err(DeviceManagerError::VfioUserCreateClient)?,
)); ));
let memory_manager = self.memory_manager.clone();
let mut vfio_user_pci_device = VfioUserPciDevice::new( let mut vfio_user_pci_device = VfioUserPciDevice::new(
vfio_user_name.clone(), vfio_user_name.clone(),
&self.address_manager.vm, &self.address_manager.vm,
@ -3238,6 +3244,7 @@ impl DeviceManager {
legacy_interrupt_group, legacy_interrupt_group,
pci_device_bdf, pci_device_bdf,
self.restoring, self.restoring,
Arc::new(move || memory_manager.lock().unwrap().allocate_memory_slot()),
) )
.map_err(DeviceManagerError::VfioUserCreate)?; .map_err(DeviceManagerError::VfioUserCreate)?;
@ -3272,15 +3279,17 @@ impl DeviceManager {
resources, resources,
)?; )?;
// Note it is required to call 'add_pci_device()' in advance to have the list of // When restoring a VM, the restore codepath will take care of mapping
// mmio regions provisioned correctly // the MMIO regions based on the information from the snapshot.
vfio_user_pci_device if !self.restoring {
.lock() // Note it is required to call 'add_pci_device()' in advance to have the list of
.unwrap() // mmio regions provisioned correctly
.map_mmio_regions(&self.address_manager.vm, || { vfio_user_pci_device
self.memory_manager.lock().unwrap().allocate_memory_slot() .lock()
}) .unwrap()
.map_err(DeviceManagerError::VfioUserMapRegion)?; .map_mmio_regions()
.map_err(DeviceManagerError::VfioUserMapRegion)?;
}
let mut node = device_node!(vfio_user_name, vfio_user_pci_device); let mut node = device_node!(vfio_user_name, vfio_user_pci_device);