pci, vmm: Manage VFIO DMA mapping from DeviceManager

Instead of letting the VfioPciDevice take the decision on how/when to
perform the DMA mapping/unmapping, we move this to the DeviceManager
instead.

The point is to let the DeviceManager choose which guest memory regions
should be mapped or not. In particular, we don't want the virtio-mem
region to be mapped/unmapped as it will be virtio-mem device
responsibility to do so.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>

pci/src/vfio.rs

@@ -12,7 +12,6 @@ use crate::{
 };
 use byteorder::{ByteOrder, LittleEndian};
 use std::any::Any;
-use std::ops::Deref;
 use std::os::unix::io::AsRawFd;
 use std::ptr::null_mut;
 use std::sync::{Arc, Barrier};
@@ -24,15 +23,14 @@ use vm_device::interrupt::{
     InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig,
 };
 use vm_device::BusDevice;
-use vm_memory::{
+use vm_memory::{Address, GuestAddress, GuestUsize};
-    Address, GuestAddress, GuestAddressSpace, GuestMemoryAtomic, GuestMemoryMmap,
-    GuestMemoryRegion, GuestRegionMmap, GuestUsize,
-};
 use vmm_sys_util::eventfd::EventFd;
 
 #[derive(Debug)]
 pub enum VfioPciError {
     AllocateGsi,
+    DmaMap(VfioError),
+    DmaUnmap(VfioError),
     EnableIntx(VfioError),
     EnableMsi(VfioError),
     EnableMsix(VfioError),
@@ -45,7 +43,6 @@ pub enum VfioPciError {
     MsixNotConfigured,
     NewVfioPciDevice,
     SetGsiRouting(hypervisor::HypervisorVmError),
-    UpdateMemory(VfioError),
 }
 pub type Result<T> = std::result::Result<T, VfioPciError>;
 
@@ -53,6 +50,8 @@ impl fmt::Display for VfioPciError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             VfioPciError::AllocateGsi => write!(f, "failed to allocate GSI"),
+            VfioPciError::DmaMap(e) => write!(f, "failed to DMA map: {}", e),
+            VfioPciError::DmaUnmap(e) => write!(f, "failed to DMA unmap: {}", e),
             VfioPciError::EnableIntx(e) => write!(f, "failed to enable INTx: {}", e),
             VfioPciError::EnableMsi(e) => write!(f, "failed to enable MSI: {}", e),
             VfioPciError::EnableMsix(e) => write!(f, "failed to enable MSI-X: {}", e),
@@ -69,7 +68,6 @@ impl fmt::Display for VfioPciError {
             VfioPciError::MsixNotConfigured => write!(f, "MSI-X interrupt not yet configured"),
             VfioPciError::NewVfioPciDevice => write!(f, "failed to create VFIO PCI device"),
             VfioPciError::SetGsiRouting(e) => write!(f, "failed to set GSI routes: {}", e),
-            VfioPciError::UpdateMemory(e) => write!(f, "failed to update memory: {}", e),
         }
     }
 }
@@ -303,7 +301,6 @@ pub struct VfioPciDevice {
     configuration: PciConfiguration,
     mmio_regions: Vec<MmioRegion>,
     interrupt: Interrupt,
-    mem: GuestMemoryAtomic<GuestMemoryMmap>,
     iommu_attached: bool,
 }
 
@@ -315,7 +312,6 @@ impl VfioPciDevice {
         container: Arc<VfioContainer>,
         msi_interrupt_manager: &Arc<dyn InterruptManager<GroupConfig = MsiIrqGroupConfig>>,
         legacy_interrupt_group: Option<Arc<Box<dyn InterruptSourceGroup>>>,
-        mem: GuestMemoryAtomic<GuestMemoryMmap>,
         iommu_attached: bool,
     ) -> Result<Self> {
         let device = Arc::new(device);
@@ -348,7 +344,6 @@ impl VfioPciDevice {
                 msi: None,
                 msix: None,
             },
-            mem,
             iommu_attached,
         };
 
@@ -427,7 +422,7 @@ impl VfioPciDevice {
 
             self.device
                 .enable_msix(irq_fds.iter().collect())
-                .map_err(VfioPciError::EnableMsi)?;
+                .map_err(VfioPciError::EnableMsix)?;
         }
 
         Ok(())
@@ -728,15 +723,21 @@ impl VfioPciDevice {
         }
     }
 
-    pub fn update_memory(&self, new_region: &Arc<GuestRegionMmap>) -> Result<()> {
+    pub fn dma_map(&self, iova: u64, size: u64, user_addr: u64) -> Result<()> {
         if !self.iommu_attached {
             self.container
-                .vfio_dma_map(
+                .vfio_dma_map(iova, size, user_addr)
-                    new_region.start_addr().raw_value(),
+                .map_err(VfioPciError::DmaMap)?;
-                    new_region.len() as u64,
+        }
-                    new_region.as_ptr() as u64,
+
-                )
+        Ok(())
-                .map_err(VfioPciError::UpdateMemory)?;
+    }
+
+    pub fn dma_unmap(&self, iova: u64, size: u64) -> Result<()> {
+        if !self.iommu_attached {
+            self.container
+                .vfio_dma_unmap(iova, size)
+                .map_err(VfioPciError::DmaUnmap)?;
         }
 
         Ok(())
@@ -766,15 +767,6 @@ impl Drop for VfioPciDevice {
         if self.interrupt.intx_in_use() {
             self.disable_intx();
         }
-
-        if !self.iommu_attached
-            && self
-                .container
-                .vfio_unmap_guest_memory(self.mem.memory().deref())
-                .is_err()
-        {
-            error!("failed to remove all guest memory regions from iommu table");
-        }
     }
 }
 
@@ -978,15 +970,6 @@ impl PciDevice for VfioPciDevice {
             }
         }
 
-        if !self.iommu_attached
-            && self
-                .container
-                .vfio_map_guest_memory(self.mem.memory().deref())
-                .is_err()
-        {
-            error!("failed to add all guest memory regions into iommu table");
-        }
-
         Ok(ranges)
     }
 

vmm/src/device_manager.rs

@@ -93,8 +93,11 @@ use vm_device::interrupt::{
 };
 use vm_device::{Bus, BusDevice, Resource};
 use vm_memory::guest_memory::FileOffset;
+#[cfg(feature = "cmos")]
+use vm_memory::GuestMemory;
 use vm_memory::{
-    Address, GuestAddress, GuestAddressSpace, GuestRegionMmap, GuestUsize, MmapRegion,
+    Address, GuestAddress, GuestAddressSpace, GuestMemoryRegion, GuestRegionMmap, GuestUsize,
+    MmapRegion,
 };
 use vm_migration::{
     Migratable, MigratableError, Pausable, Snapshot, SnapshotDataSection, Snapshottable,
@@ -255,6 +258,12 @@ pub enum DeviceManagerError {
     /// Failed to map VFIO MMIO region.
     VfioMapRegion(pci::VfioPciError),
 
+    /// Failed to DMA map VFIO device.
+    VfioDmaMap(pci::VfioPciError),
+
+    /// Failed to DMA unmap VFIO device.
+    VfioDmaUnmap(pci::VfioPciError),
+
     /// Failed to create the passthrough device.
     CreatePassthroughDevice(anyhow::Error),
 
@@ -1415,7 +1424,6 @@ impl DeviceManager {
         #[cfg(feature = "cmos")]
         {
             // Add a CMOS emulated device
-            use vm_memory::GuestMemory;
             let mem_size = self
                 .memory_manager
                 .lock()
@@ -2723,14 +2731,12 @@ impl DeviceManager {
             None
         };
 
-        let memory = self.memory_manager.lock().unwrap().guest_memory();
         let mut vfio_pci_device = VfioPciDevice::new(
             &self.address_manager.vm,
             vfio_device,
             vfio_container,
             &self.msi_interrupt_manager,
             legacy_interrupt_group,
-            memory,
             device_cfg.iommu,
         )
         .map_err(DeviceManagerError::VfioPciCreate)?;
@@ -2762,6 +2768,21 @@ impl DeviceManager {
             });
         }
 
+        // Register DMA mapping in IOMMU.
+        // Do not register virtio-mem regions, as they are handled directly by
+        // virtio-mem device itself.
+        for (_, zone) in self.memory_manager.lock().unwrap().memory_zones().iter() {
+            for region in zone.regions() {
+                vfio_pci_device
+                    .dma_map(
+                        region.start_addr().raw_value(),
+                        region.len() as u64,
+                        region.as_ptr() as u64,
+                    )
+                    .map_err(DeviceManagerError::VfioDmaMap)?;
+            }
+        }
+
         let vfio_pci_device = Arc::new(Mutex::new(vfio_pci_device));
 
         self.add_pci_device(
@@ -3017,7 +3038,7 @@ impl DeviceManager {
         self.cmdline_additions.as_slice()
     }
 
-    pub fn update_memory(&self, _new_region: &Arc<GuestRegionMmap>) -> DeviceManagerResult<()> {
+    pub fn update_memory(&self, new_region: &Arc<GuestRegionMmap>) -> DeviceManagerResult<()> {
         let memory = self.memory_manager.lock().unwrap().guest_memory();
         for (virtio_device, _, _) in self.virtio_devices.iter() {
             virtio_device
@@ -3033,7 +3054,11 @@ impl DeviceManager {
                 vfio_pci_device
                     .lock()
                     .unwrap()
-                    .update_memory(_new_region)
+                    .dma_map(
+                        new_region.start_addr().raw_value(),
+                        new_region.len() as u64,
+                        new_region.as_ptr() as u64,
+                    )
                     .map_err(DeviceManagerError::UpdateMemoryForVfioPciDevice)?;
             }
         }
@@ -3173,6 +3198,19 @@ impl DeviceManager {
             let (pci_device, bus_device, virtio_device) = if let Ok(vfio_pci_device) =
                 any_device.clone().downcast::<Mutex<VfioPciDevice>>()
             {
+                // Unregister DMA mapping in IOMMU.
+                // Do not unregister the virtio-mem region, as it is directly
+                // handled by the virtio-mem device.
+                {
+                    let dev = vfio_pci_device.lock().unwrap();
+                    for (_, zone) in self.memory_manager.lock().unwrap().memory_zones().iter() {
+                        for region in zone.regions() {
+                            dev.dma_unmap(region.start_addr().raw_value(), region.len() as u64)
+                                .map_err(DeviceManagerError::VfioDmaUnmap)?;
+                        }
+                    }
+                }
+
                 (
                     Arc::clone(&vfio_pci_device) as Arc<Mutex<dyn PciDevice>>,
                     Arc::clone(&vfio_pci_device) as Arc<Mutex<dyn BusDevice>>,