virtio-devices, vmm: Optimised async virtio device activation

In order to ensure that the virtio device thread is spawned from the vmm
thread we use an asynchronous activation mechanism for the virtio
devices. This change optimises that code so that we do not need to
iterate through all virtio devices on the platform in order to find the
one that requires activation. We solve this by creating a separate short
lived VirtioPciDeviceActivator that holds the required state for the
activation (e.g. the clones of the queues) this can then be stored onto
the device manager ready for asynchronous activation.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>

virtio-devices/src/transport/mod.rs

@@ -6,7 +6,7 @@ use vmm_sys_util::eventfd::EventFd;
 mod pci_common_config;
 mod pci_device;
 pub use pci_common_config::VirtioPciCommonConfig;
-pub use pci_device::VirtioPciDevice;
+pub use pci_device::{VirtioPciDevice, VirtioPciDeviceActivator};
 
 pub trait VirtioTransport {
     fn ioeventfds(&self, base_addr: u64) -> Vec<(&EventFd, u64)>;

virtio-devices/src/transport/pci_device.rs

@@ -287,6 +287,37 @@ struct VirtioPciDeviceState {
 
 impl VersionMapped for VirtioPciDeviceState {}
 
+pub struct VirtioPciDeviceActivator {
+    interrupt: Option<Arc<dyn VirtioInterrupt>>,
+    memory: Option<GuestMemoryAtomic<GuestMemoryMmap>>,
+    device: Arc<Mutex<dyn VirtioDevice>>,
+    device_activated: Arc<AtomicBool>,
+    queues: Option<Vec<Queue<GuestMemoryAtomic<GuestMemoryMmap>>>>,
+    queue_evts: Option<Vec<EventFd>>,
+    barrier: Option<Arc<Barrier>>,
+    id: String,
+}
+
+impl VirtioPciDeviceActivator {
+    pub fn activate(&mut self) -> ActivateResult {
+        self.device.lock().unwrap().activate(
+            self.memory.take().unwrap(),
+            self.interrupt.take().unwrap(),
+            self.queues.take().unwrap(),
+            self.queue_evts.take().unwrap(),
+        )?;
+        self.device_activated.store(true, Ordering::SeqCst);
+
+        if let Some(barrier) = self.barrier.take() {
+            info!("{}: Waiting for barrier", self.id);
+            barrier.wait();
+            info!("{}: Barrier released", self.id);
+        }
+
+        Ok(())
+    }
+}
+
 pub struct VirtioPciDevice {
     id: String,
 
@@ -338,11 +369,11 @@ pub struct VirtioPciDevice {
     // EventFd to signal on to request activation
     activate_evt: EventFd,
 
-    // Barrier that is used to wait on for activation
-    activate_barrier: Arc<Barrier>,
-
     // Optional DMA handler
     dma_handler: Option<Arc<dyn ExternalDmaMapping>>,
+
+    // Pending activations
+    pending_activations: Arc<Mutex<Vec<VirtioPciDeviceActivator>>>,
 }
 
 impl VirtioPciDevice {
@@ -359,6 +390,7 @@ impl VirtioPciDevice {
         activate_evt: EventFd,
         use_64bit_bar: bool,
         dma_handler: Option<Arc<dyn ExternalDmaMapping>>,
+        pending_activations: Arc<Mutex<Vec<VirtioPciDeviceActivator>>>,
     ) -> Result<Self> {
         let device_clone = device.clone();
         let mut locked_device = device_clone.lock().unwrap();
@@ -458,8 +490,8 @@ impl VirtioPciDevice {
             cap_pci_cfg_info: VirtioPciCfgCapInfo::default(),
             bar_regions: vec![],
             activate_evt,
-            activate_barrier: Arc::new(Barrier::new(2)),
             dma_handler,
+            pending_activations,
         };
 
         if let Some(msix_config) = &virtio_pci_device.msix_config {
@@ -666,37 +698,37 @@ impl VirtioPciDevice {
         self.device.clone()
     }
 
-    fn activate(&mut self) -> ActivateResult {
-        if let Some(virtio_interrupt) = self.virtio_interrupt.take() {
-            if self.memory.is_some() {
-                let mem = self.memory.as_ref().unwrap().clone();
-                let mut device = self.device.lock().unwrap();
-                let mut queue_evts = Vec::new();
-                let mut queues: Vec<Queue<GuestMemoryAtomic<GuestMemoryMmap>>> =
-                    self.queues.iter().map(vm_virtio::clone_queue).collect();
-                queues.retain(|q| q.state.ready);
-                for (i, queue) in queues.iter().enumerate() {
-                    queue_evts.push(self.queue_evts[i].try_clone().unwrap());
-                    if !queue.is_valid() {
-                        error!("Queue {} is not valid", i);
-                    }
-                }
-                return device.activate(mem, virtio_interrupt, queues, queue_evts);
+    fn prepare_activator(&mut self, barrier: Option<Arc<Barrier>>) -> VirtioPciDeviceActivator {
+        let mut queue_evts = Vec::new();
+        let mut queues: Vec<Queue<GuestMemoryAtomic<GuestMemoryMmap>>> =
+            self.queues.iter().map(vm_virtio::clone_queue).collect();
+        queues.retain(|q| q.state.ready);
+        for (i, queue) in queues.iter().enumerate() {
+            queue_evts.push(self.queue_evts[i].try_clone().unwrap());
+            if !queue.is_valid() {
+                error!("Queue {} is not valid", i);
             }
         }
-        Ok(())
+
+        VirtioPciDeviceActivator {
+            interrupt: self.virtio_interrupt.take(),
+            memory: self.memory.clone(),
+            device: self.device.clone(),
+            queues: Some(queues),
+            device_activated: self.device_activated.clone(),
+            queue_evts: Some(
+                queue_evts
+                    .iter()
+                    .map(|fd| fd.try_clone().unwrap())
+                    .collect(),
+            ),
+            barrier,
+            id: self.id.clone(),
+        }
     }
 
-    pub fn maybe_activate(&mut self) {
-        if self.needs_activation() {
-            self.activate().expect("Failed to activate device");
-            self.device_activated.store(true, Ordering::SeqCst);
-            info!("{}: Waiting for barrier", self.id);
-            self.activate_barrier.wait();
-            info!("{}: Barrier released", self.id);
-        } else {
-            info!("{}: Device does not need activation", self.id)
-        }
+    fn activate(&mut self) -> ActivateResult {
+        self.prepare_activator(None).activate()
     }
 
     fn needs_activation(&self) -> bool {
@@ -1058,13 +1090,16 @@ impl PciDevice for VirtioPciDevice {
 
         // Try and activate the device if the driver status has changed
         if self.needs_activation() {
+            let barrier = Arc::new(Barrier::new(2));
+            let activator = self.prepare_activator(Some(barrier.clone()));
+            self.pending_activations.lock().unwrap().push(activator);
             info!(
                 "{}: Needs activation; writing to activate event fd",
                 self.id
             );
             self.activate_evt.write(1).ok();
             info!("{}: Needs activation; returning barrier", self.id);
-            return Some(self.activate_barrier.clone());
+            return Some(barrier);
         }
 
         // Device has been reset by the driver

vmm/src/device_manager.rs

@@ -79,10 +79,12 @@ use std::result;
 use std::sync::{Arc, Mutex};
 use std::time::Instant;
 use vfio_ioctls::{VfioContainer, VfioDevice};
-use virtio_devices::transport::VirtioPciDevice;
 use virtio_devices::transport::VirtioTransport;
+use virtio_devices::transport::{VirtioPciDevice, VirtioPciDeviceActivator};
 use virtio_devices::vhost_user::VhostUserConfig;
-use virtio_devices::{AccessPlatformMapping, VdpaDmaMapping, VirtioMemMappingSource};
+use virtio_devices::{
+    AccessPlatformMapping, ActivateError, VdpaDmaMapping, VirtioMemMappingSource,
+};
 use virtio_devices::{Endpoint, IommuMapping};
 use virtio_devices::{VirtioSharedMemory, VirtioSharedMemoryList};
 use vm_allocator::{AddressAllocator, SystemAllocator};
@@ -476,6 +478,9 @@ pub enum DeviceManagerError {
 
     /// Invalid identifier
     InvalidIdentifier(String),
+
+    /// Error activating virtio device
+    VirtioActivate(ActivateError),
 }
 pub type DeviceManagerResult<T> = result::Result<T, DeviceManagerError>;
 
@@ -939,6 +944,9 @@ pub struct DeviceManager {
 
     // Start time of the VM
     timestamp: Instant,
+
+    // Pending activations
+    pending_activations: Arc<Mutex<Vec<VirtioPciDeviceActivator>>>,
 }
 
 impl DeviceManager {
@@ -1080,6 +1088,7 @@ impl DeviceManager {
             io_uring_supported: None,
             boot_id_list,
             timestamp,
+            pending_activations: Arc::new(Mutex::new(Vec::default())),
         };
 
         let device_manager = Arc::new(Mutex::new(device_manager));
@@ -3520,6 +3529,7 @@ impl DeviceManager {
                 // The exception being if not on the default PCI segment.
                 pci_segment_id > 0 || device_type != VirtioDeviceType::Block as u32,
                 dma_handler,
+                self.pending_activations.clone(),
             )
             .map_err(DeviceManagerError::VirtioDevice)?,
         ));
@@ -3674,17 +3684,10 @@ impl DeviceManager {
     }
 
     pub fn activate_virtio_devices(&self) -> DeviceManagerResult<()> {
-        // Find virtio pci devices and activate any pending ones
-        let device_tree = self.device_tree.lock().unwrap();
-        for pci_device_node in device_tree.pci_devices() {
-            #[allow(irrefutable_let_patterns)]
-            if let PciDeviceHandle::Virtio(virtio_pci_device) = &pci_device_node
-                .pci_device_handle
-                .as_ref()
-                .ok_or(DeviceManagerError::MissingPciDevice)?
-            {
-                virtio_pci_device.lock().unwrap().maybe_activate();
-            }
+        for mut activator in self.pending_activations.lock().unwrap().drain(..) {
+            activator
+                .activate()
+                .map_err(DeviceManagerError::VirtioActivate)?;
         }
         Ok(())
     }