diff --git a/pci/src/vfio.rs b/pci/src/vfio.rs index 7f97e73da..dbcf59fbe 100644 --- a/pci/src/vfio.rs +++ b/pci/src/vfio.rs @@ -1466,7 +1466,7 @@ pub struct VfioPciDevice { id: String, vm: Arc, device: Arc, - container: Arc, + vfio_ops: Arc, common: VfioCommon, iommu_attached: bool, memory_slot_allocator: MemorySlotAllocator, @@ -1481,7 +1481,7 @@ impl VfioPciDevice { id: String, vm: Arc, device: VfioDevice, - container: Arc, + vfio_ops: Arc, msi_interrupt_manager: Arc>, legacy_interrupt_group: Option>, iommu_attached: bool, @@ -1510,7 +1510,7 @@ impl VfioPciDevice { id, vm, device, - container, + vfio_ops, common, iommu_attached, memory_slot_allocator, @@ -1705,7 +1705,7 @@ impl VfioPciDevice { // user_memory_region.mapping.len() bytes of // valid memory that will only be unmapped with munmap(). unsafe { - self.container.vfio_dma_map( + self.vfio_ops.vfio_dma_map( user_memory_region.start, user_memory_region.mapping.len(), user_memory_region.mapping.addr(), @@ -1726,15 +1726,15 @@ impl VfioPciDevice { for user_memory_region in region.user_memory_regions.drain(..) { let len = user_memory_region.mapping.len(); let host_addr = user_memory_region.mapping.addr(); - // Unmap from vfio container + // Unmap MMIO region from the host IOMMU address space via VfioOps if !self.iommu_attached && let Err(e) = self - .container + .vfio_ops .vfio_dma_unmap(user_memory_region.start, len) .map_err(|e| VfioPciError::DmaUnmap(e, self.device_path.clone(), self.bdf)) { error!( - "Could not unmap mmio region from vfio container: \ + "Could not unmap MMIO region from the host IOMMU address space: \ iova 0x{:x}, size 0x{:x}: {}, ", user_memory_region.start, len, e ); @@ -1884,17 +1884,17 @@ impl PciDevice for VfioPciDevice { for user_memory_region in region.user_memory_regions.iter_mut() { let len = user_memory_region.mapping.len(); let host_addr = user_memory_region.mapping.addr(); - // Unmap the old MMIO region from vfio container + // Unmap the old MMIO region from the host IOMMU address space via VfioOps if !self.iommu_attached && let Err(e) = self - .container + .vfio_ops .vfio_dma_unmap(user_memory_region.start, len) .map_err(|e| { VfioPciError::DmaUnmap(e, self.device_path.clone(), self.bdf) }) { error!( - "Could not unmap mmio region from vfio container: \ + "Could not unmap MMIO region from the host IOMMU address space: \ iova 0x{:x}, size 0x{:x}: {}, ", user_memory_region.start, len, e ); @@ -1938,7 +1938,7 @@ iova 0x{:x}, size 0x{:x}: {}, ", } .map_err(io::Error::other)?; - // Map the moved mmio region to vfio container + // Map the moved MMIO region into the host IOMMU address space via VfioOps if !self.iommu_attached { // vfio_dma_map is unsound and ought to be marked as unsafe #[allow(unused_unsafe)] @@ -1946,13 +1946,13 @@ iova 0x{:x}, size 0x{:x}: {}, ", // host_addr points to len bytes of // valid memory that will only be unmapped with munmap(). unsafe { - self.container + self.vfio_ops .vfio_dma_map(user_memory_region.start, len, host_addr) } .map_err(|e| VfioPciError::DmaMap(e, self.device_path.clone(), self.bdf)) .map_err(|e| { io::Error::other(format!( - "Could not map mmio region to vfio container: \ + "Could not map MMIO region into the host IOMMU address space: \ iova 0x{:x}, size 0x{:x}: {}, ", user_memory_region.start, len, e )) @@ -1996,9 +1996,9 @@ impl Migratable for VfioPciDevice {} /// This structure implements the ExternalDmaMapping trait. It is meant to /// be used when the caller tries to provide a way to update the mappings -/// associated with a specific VFIO container. +/// associated with a specific VfioOps instance. pub struct VfioDmaMapping { - container: Arc, + vfio_ops: Arc, memory: Arc, mmio_regions: Arc>>, } @@ -2006,16 +2006,16 @@ pub struct VfioDmaMapping { impl VfioDmaMapping { /// Create a DmaMapping object. /// # Parameters - /// * `container`: VFIO container object. + /// * `vfio_ops`: VfioOps instance. /// * `memory`: guest memory to mmap. /// * `mmio_regions`: mmio_regions to mmap. pub fn new( - container: Arc, + vfio_ops: Arc, memory: Arc, mmio_regions: Arc>>, ) -> Self { VfioDmaMapping { - container, + vfio_ops, memory, mmio_regions, } @@ -2062,20 +2062,20 @@ impl ExternalDmaMapping for VfioDmaMapping std::result::Result<(), io::Error> { - self.container + self.vfio_ops .vfio_dma_unmap(iova, size as usize) .map_err(|e| { io::Error::other(format!( - "failed to unmap memory for VFIO container, \ + "failed to unmap memory from the host IOMMU address space, \ iova 0x{iova:x}, size 0x{size:x}: {e:?}" )) }) diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 8dc2399e3..95814cc92 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -1029,10 +1029,10 @@ pub struct DeviceManager { // Passthrough device handle passthrough_device: Option, - // VFIO container - // Only one container can be created, therefore it is stored as part of the + // VFIO operation instance + // Only one can be created, therefore it is stored as part of the // DeviceManager to be reused. - vfio_container: Option>, + vfio_ops: Option>, // Paravirtualized IOMMU iommu_device: Option>>, @@ -1350,7 +1350,7 @@ impl DeviceManager { msi_interrupt_manager, legacy_interrupt_manager: None, passthrough_device: None, - vfio_container: None, + vfio_ops: None, iommu_device: None, iommu_mapping: None, iommu_attached_devices: None, @@ -3798,7 +3798,7 @@ impl DeviceManager { self.add_vfio_device(device_cfg) } - fn create_vfio_container(&self) -> DeviceManagerResult> { + fn create_vfio_ops(&self) -> DeviceManagerResult> { let passthrough_device = self .passthrough_device .as_ref() @@ -3830,19 +3830,24 @@ impl DeviceManager { let mut needs_dma_mapping = false; - // Here we create a new VFIO container for two reasons. Either this is - // the first VFIO device, meaning we need a new VFIO container, which - // will be shared with other VFIO devices. Or the new VFIO device is - // attached to a vIOMMU, meaning we must create a dedicated VFIO - // container. In the vIOMMU use case, we can't let all devices under - // the same VFIO container since we couldn't map/unmap memory for each - // device. That's simply because the map/unmap operations happen at the - // VFIO container level. - let vfio_container = if device_cfg.iommu { - let vfio_container = self.create_vfio_container()?; + // Here we create a new VfioOps for two reasons: + // 1) This is the first VFIO device, meaning we need a new VfioOps + // which will be shared with other VFIO devices. + // 2) The new VFIO device is attached to a vIOMMU, meaning we must + // create a dedicated VfioOps. In the vIOMMU use case, we can't + // let all devices share the same VfioOps since we couldn't + // map/unmap memory for each device independently. That's simply + // because the map/unmap operations happen at the VfioOps level. + // + // Note: this is a limitation of the legacy VFIO interface using + // container/group. The VFIO cdev and iommufd do not have such a + // limitation, and this will be revised once we have VFIO cdev and + // iommufd support. + let vfio_ops = if device_cfg.iommu { + let vfio_ops = self.create_vfio_ops()?; let vfio_mapping = Arc::new(VfioDmaMapping::new( - Arc::clone(&vfio_container), + Arc::clone(&vfio_ops), Arc::new(self.memory_manager.lock().unwrap().guest_memory()), Arc::clone(&self.mmio_regions), )); @@ -3856,22 +3861,20 @@ impl DeviceManager { return Err(DeviceManagerError::MissingVirtualIommu); } - vfio_container - } else if let Some(vfio_container) = &self.vfio_container { - Arc::clone(vfio_container) + vfio_ops + } else if let Some(vfio_ops) = &self.vfio_ops { + Arc::clone(vfio_ops) } else { - let vfio_container = self.create_vfio_container()?; + let vfio_ops = self.create_vfio_ops()?; needs_dma_mapping = true; - self.vfio_container = Some(Arc::clone(&vfio_container)); + self.vfio_ops = Some(Arc::clone(&vfio_ops)); - vfio_container + vfio_ops }; - let vfio_device = VfioDevice::new( - &device_cfg.path, - Arc::clone(&vfio_container) as Arc, - ) - .map_err(DeviceManagerError::VfioCreate)?; + let vfio_device = + VfioDevice::new(&device_cfg.path, Arc::clone(&vfio_ops) as Arc) + .map_err(DeviceManagerError::VfioCreate)?; if needs_dma_mapping { // Register DMA mapping in IOMMU. @@ -3885,7 +3888,7 @@ impl DeviceManager { // to len bytes of valid memory starting at as_ptr() // that will only be freed with munmap(). unsafe { - vfio_container.vfio_dma_map( + vfio_ops.vfio_dma_map( region.start_addr().raw_value(), region.len() as usize, region.as_ptr(), @@ -3896,7 +3899,7 @@ impl DeviceManager { } let vfio_mapping = Arc::new(VfioDmaMapping::new( - Arc::clone(&vfio_container), + Arc::clone(&vfio_ops), Arc::new(self.memory_manager.lock().unwrap().guest_memory()), Arc::clone(&self.mmio_regions), )); @@ -3934,7 +3937,7 @@ impl DeviceManager { vfio_name.clone(), self.address_manager.vm.clone(), vfio_device, - vfio_container, + vfio_ops, self.msi_interrupt_manager.clone(), legacy_interrupt_group, device_cfg.iommu, @@ -4513,14 +4516,14 @@ impl DeviceManager { } // Take care of updating the memory for VFIO PCI devices. - if let Some(vfio_container) = &self.vfio_container { + if let Some(vfio_ops) = &self.vfio_ops { // vfio_dma_map is unsound and ought to be marked as unsafe #[allow(unused_unsafe)] // SAFETY: GuestMemoryMmap guarantees that region points // to len bytes of valid memory starting at as_ptr() // that will only be freed with munmap(). unsafe { - vfio_container.vfio_dma_map( + vfio_ops.vfio_dma_map( new_region.start_addr().raw_value(), new_region.len() as usize, new_region.as_ptr(), @@ -4764,7 +4767,7 @@ impl DeviceManager { let (pci_device, bus_device, virtio_device, remove_dma_handler) = match pci_device_handle { // VirtioMemMappingSource::Container cleanup is handled by - // cleanup_vfio_container when the last VFIO device is removed. + // cleanup_vfio_ops when the last VFIO device is removed. PciDeviceHandle::Vfio(vfio_pci_device) => { // Remove this device's MMIO regions from the DeviceManager's // mmio_regions list. We match on UserMemoryRegion slot numbers @@ -5154,11 +5157,11 @@ impl DeviceManager { &self.acpi_platform_addresses } - fn cleanup_vfio_container(&mut self) { - // Drop the 'vfio container' instance when "Self" is the only reference - if let Some(1) = self.vfio_container.as_ref().map(Arc::strong_count) { - debug!("Drop 'vfio container' given no active 'vfio devices'."); - self.vfio_container = None; + fn cleanup_vfio_ops(&mut self) { + // Drop the VfioOps instance when "Self" is the only reference + if let Some(1) = self.vfio_ops.as_ref().map(Arc::strong_count) { + debug!("Drop VfioOps given no active VFIO devices."); + self.vfio_ops = None; } } } @@ -5644,7 +5647,7 @@ impl BusDevice for DeviceManager { if let Err(e) = self.eject_device(self.selected_segment as u16, slot_id as u8) { error!("Failed ejecting device {slot_id}: {e:?}"); } - self.cleanup_vfio_container(); + self.cleanup_vfio_ops(); slot_bitmap &= !(1 << slot_id); } }