diff --git a/virtio-devices/src/transport/mod.rs b/virtio-devices/src/transport/mod.rs index cb38c6fb6..7ef5d3772 100644 --- a/virtio-devices/src/transport/mod.rs +++ b/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, VirtioPciDeviceActivator}; +pub use pci_device::VirtioPciDevice; pub trait VirtioTransport { fn ioeventfds(&self, base_addr: u64) -> Vec<(&EventFd, u64)>; diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index 76062c14e..97b764bda 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -287,37 +287,6 @@ struct VirtioPciDeviceState { impl VersionMapped for VirtioPciDeviceState {} -pub struct VirtioPciDeviceActivator { - interrupt: Option>, - memory: Option>, - device: Arc>, - device_activated: Arc, - queues: Option>>>, - queue_evts: Option>, - barrier: Option>, - 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, @@ -369,11 +338,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, + // Optional DMA handler dma_handler: Option>, - - // Pending activations - pending_activations: Arc>>, } impl VirtioPciDevice { @@ -390,7 +359,6 @@ impl VirtioPciDevice { activate_evt: EventFd, use_64bit_bar: bool, dma_handler: Option>, - pending_activations: Arc>>, ) -> Result { let device_clone = device.clone(); let mut locked_device = device_clone.lock().unwrap(); @@ -490,8 +458,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 { @@ -698,37 +666,37 @@ impl VirtioPciDevice { self.device.clone() } - fn prepare_activator(&mut self, barrier: Option>) -> VirtioPciDeviceActivator { - let mut queue_evts = Vec::new(); - let mut queues: Vec>> = - 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); + 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>> = + 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); } } - - 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(), - } + Ok(()) } - fn activate(&mut self) -> ActivateResult { - self.prepare_activator(None).activate() + 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 needs_activation(&self) -> bool { @@ -1090,16 +1058,13 @@ 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(barrier); + return Some(self.activate_barrier.clone()); } // Device has been reset by the driver diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index 44030c7bf..d781cfda8 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -79,12 +79,10 @@ 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, ActivateError, VdpaDmaMapping, VirtioMemMappingSource, -}; +use virtio_devices::{AccessPlatformMapping, VdpaDmaMapping, VirtioMemMappingSource}; use virtio_devices::{Endpoint, IommuMapping}; use virtio_devices::{VirtioSharedMemory, VirtioSharedMemoryList}; use vm_allocator::{AddressAllocator, SystemAllocator}; @@ -478,9 +476,6 @@ pub enum DeviceManagerError { /// Invalid identifier InvalidIdentifier(String), - - /// Error activating virtio device - VirtioActivate(ActivateError), } pub type DeviceManagerResult = result::Result; @@ -944,9 +939,6 @@ pub struct DeviceManager { // Start time of the VM timestamp: Instant, - - // Pending activations - pending_activations: Arc>>, } impl DeviceManager { @@ -1088,7 +1080,6 @@ 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)); @@ -3529,7 +3520,6 @@ 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)?, )); @@ -3684,10 +3674,17 @@ impl DeviceManager { } pub fn activate_virtio_devices(&self) -> DeviceManagerResult<()> { - for mut activator in self.pending_activations.lock().unwrap().drain(..) { - activator - .activate() - .map_err(DeviceManagerError::VirtioActivate)?; + // 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(); + } } Ok(()) }