diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index a26cc335e..76b726b47 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -31,7 +31,7 @@ use std::io::Write; use std::num::Wrapping; use std::result; use std::sync::atomic::{AtomicU16, AtomicUsize, Ordering}; -use std::sync::{Arc, Barrier, Mutex}; +use std::sync::{atomic::AtomicBool, Arc, Barrier, Mutex}; use vm_allocator::SystemAllocator; use vm_device::interrupt::{ InterruptIndex, InterruptManager, InterruptSourceGroup, MsiIrqGroupConfig, @@ -292,7 +292,7 @@ pub struct VirtioPciDevice { // Virtio device reference and status device: Arc>, - device_activated: bool, + device_activated: Arc, // PCI interrupts. interrupt_status: Arc, @@ -323,10 +323,17 @@ pub struct VirtioPciDevice { // Details of bar regions to free bar_regions: Vec<(GuestAddress, GuestUsize, PciBarRegionType)>, + + // EventFd to signal on to request activation + activate_evt: EventFd, + + // Barrier that is used to wait on for activation + activate_barrier: Arc, } impl VirtioPciDevice { /// Constructs a new PCI transport for the given virtio device. + #[allow(clippy::too_many_arguments)] pub fn new( id: String, memory: GuestMemoryAtomic, @@ -335,6 +342,7 @@ impl VirtioPciDevice { iommu_mapping_cb: Option>, interrupt_manager: &Arc>, pci_device_bdf: u32, + activate_evt: EventFd, ) -> Result { let device_clone = device.clone(); let locked_device = device_clone.lock().unwrap(); @@ -420,7 +428,7 @@ impl VirtioPciDevice { msix_config, msix_num, device, - device_activated: false, + device_activated: Arc::new(AtomicBool::new(false)), interrupt_status: Arc::new(AtomicUsize::new(0)), virtio_interrupt: None, queues, @@ -432,6 +440,8 @@ impl VirtioPciDevice { interrupt_source_group, cap_pci_cfg_info: VirtioPciCfgCapInfo::default(), bar_regions: vec![], + activate_evt, + activate_barrier: Arc::new(Barrier::new(2)), }; if let Some(msix_config) = &virtio_pci_device.msix_config { @@ -447,14 +457,15 @@ impl VirtioPciDevice { fn state(&self) -> VirtioPciDeviceState { VirtioPciDeviceState { - device_activated: self.device_activated, + device_activated: self.device_activated.load(Ordering::Acquire), interrupt_status: self.interrupt_status.load(Ordering::Acquire), queues: self.queues.clone(), } } fn set_state(&mut self, state: &VirtioPciDeviceState) -> std::result::Result<(), Error> { - self.device_activated = state.device_activated; + self.device_activated + .store(state.device_activated, Ordering::Release); self.interrupt_status .store(state.interrupt_status, Ordering::Release); @@ -656,14 +667,19 @@ impl VirtioPciDevice { self.queue_evts.split_off(0), ) .expect("Failed to activate device"); - self.device_activated = true; + self.device_activated.store(true, Ordering::SeqCst); + info!("Waiting for barrier"); + self.activate_barrier.wait(); + info!("Barrier released"); } } } } fn needs_activation(&self) -> bool { - !self.device_activated && self.is_driver_ready() && self.are_queues_valid() + !self.device_activated.load(Ordering::SeqCst) + && self.is_driver_ready() + && self.are_queues_valid() } } @@ -1001,17 +1017,20 @@ impl PciDevice for VirtioPciDevice { }; // Try and activate the device if the driver status has changed - self.maybe_activate(); + if self.needs_activation() { + self.activate_evt.write(1).ok(); + return Some(self.activate_barrier.clone()); + } // Device has been reset by the driver - if self.device_activated && self.is_driver_init() { + if self.device_activated.load(Ordering::SeqCst) && self.is_driver_init() { let mut device = self.device.lock().unwrap(); if let Some((virtio_interrupt, mut queue_evts)) = device.reset() { // Upon reset the device returns its interrupt EventFD and it's queue EventFDs self.virtio_interrupt = Some(virtio_interrupt); self.queue_evts.append(&mut queue_evts); - self.device_activated = false; + self.device_activated.store(false, Ordering::SeqCst); // Reset queue readiness (changes queue_enable), queue sizes // and selected_queue as per spec for reset @@ -1128,7 +1147,10 @@ impl Snapshottable for VirtioPciDevice { // Then we can activate the device, as we know at this point that // the virtqueues are in the right state and the device is ready // to be activated, which will spawn each virtio worker thread. - if self.device_activated && self.is_driver_ready() && self.are_queues_valid() { + if self.device_activated.load(Ordering::SeqCst) + && self.is_driver_ready() + && self.are_queues_valid() + { if let Some(virtio_interrupt) = self.virtio_interrupt.take() { if self.memory.is_some() { let mem = self.memory.as_ref().unwrap().clone(); diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index a9687fd09..621bd526f 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -776,6 +776,10 @@ pub struct DeviceManager { // Possible handle to the virtio-balloon device balloon: Option>>, + + // Virtio Device activation EventFd to allow the VMM thread to trigger device + // activation and thus start the threads from the VMM thread + activate_evt: EventFd, } impl DeviceManager { @@ -788,6 +792,7 @@ impl DeviceManager { reset_evt: &EventFd, seccomp_action: SeccompAction, #[cfg(feature = "acpi")] numa_nodes: NumaNodes, + activate_evt: &EventFd, ) -> DeviceManagerResult>> { let device_tree = Arc::new(Mutex::new(DeviceTree::new())); @@ -844,6 +849,9 @@ impl DeviceManager { #[cfg(feature = "acpi")] numa_nodes, balloon: None, + activate_evt: activate_evt + .try_clone() + .map_err(DeviceManagerError::EventFd)?, }; #[cfg(feature = "acpi")] @@ -2739,6 +2747,9 @@ impl DeviceManager { iommu_mapping_cb, interrupt_manager, pci_device_bdf, + self.activate_evt + .try_clone() + .map_err(DeviceManagerError::EventFd)?, ) .map_err(DeviceManagerError::VirtioDevice)?; @@ -2834,6 +2845,18 @@ impl DeviceManager { Ok(()) } + pub fn activate_virtio_devices(&self) -> DeviceManagerResult<()> { + // Find virtio pci devices and activate any pending ones + for (_, any_device) in self.pci_devices.iter() { + if let Ok(virtio_pci_device) = + Arc::clone(any_device).downcast::>() + { + virtio_pci_device.lock().unwrap().maybe_activate(); + } + } + Ok(()) + } + pub fn notify_hotplug( &self, _notification_type: HotPlugNotificationFlags, diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 769833869..d4ae73a09 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -131,6 +131,10 @@ pub enum Error { /// Cannot apply seccomp filter #[error("Error applying seccomp filter: {0}")] ApplySeccompFilter(seccomp::Error), + + /// Error activating virtio devices + #[error("Error activating virtio devices: {0:?}")] + ActivateVirtioDevices(VmError), } pub type Result = result::Result; @@ -140,6 +144,7 @@ pub enum EpollDispatch { Reset, Stdin, Api, + ActivateVirtioDevices, } pub struct EpollContext { @@ -281,6 +286,7 @@ pub struct Vmm { vm_config: Option>>, seccomp_action: SeccompAction, hypervisor: Arc, + activate_evt: EventFd, } impl Vmm { @@ -293,6 +299,7 @@ impl Vmm { let mut epoll = EpollContext::new().map_err(Error::Epoll)?; let exit_evt = EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?; let reset_evt = EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?; + let activate_evt = EventFd::new(EFD_NONBLOCK).map_err(Error::EventFdCreate)?; if unsafe { libc::isatty(libc::STDIN_FILENO as i32) } != 0 { epoll.add_stdin().map_err(Error::Epoll)?; @@ -306,6 +313,10 @@ impl Vmm { .add_event(&reset_evt, EpollDispatch::Reset) .map_err(Error::Epoll)?; + epoll + .add_event(&activate_evt, EpollDispatch::ActivateVirtioDevices) + .map_err(Error::Epoll)?; + epoll .add_event(&api_evt, EpollDispatch::Api) .map_err(Error::Epoll)?; @@ -320,6 +331,7 @@ impl Vmm { vm_config: None, seccomp_action, hypervisor, + activate_evt, }) } @@ -328,6 +340,10 @@ impl Vmm { if self.vm.is_none() { let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?; let reset_evt = self.reset_evt.try_clone().map_err(VmError::EventFdClone)?; + let activate_evt = self + .activate_evt + .try_clone() + .map_err(VmError::EventFdClone)?; if let Some(ref vm_config) = self.vm_config { let vm = Vm::new( @@ -336,6 +352,7 @@ impl Vmm { reset_evt, &self.seccomp_action, self.hypervisor.clone(), + activate_evt, )?; self.vm = Some(vm); } @@ -397,6 +414,10 @@ impl Vmm { let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?; let reset_evt = self.reset_evt.try_clone().map_err(VmError::EventFdClone)?; + let activate_evt = self + .activate_evt + .try_clone() + .map_err(VmError::EventFdClone)?; let vm = Vm::new_from_snapshot( &snapshot, @@ -406,6 +427,7 @@ impl Vmm { restore_cfg.prefault, &self.seccomp_action, self.hypervisor.clone(), + activate_evt, )?; self.vm = Some(vm); @@ -443,6 +465,10 @@ impl Vmm { let exit_evt = self.exit_evt.try_clone().map_err(VmError::EventFdClone)?; let reset_evt = self.reset_evt.try_clone().map_err(VmError::EventFdClone)?; + let activate_evt = self + .activate_evt + .try_clone() + .map_err(VmError::EventFdClone)?; // The Linux kernel fires off an i8042 reset after doing the ACPI reset so there may be // an event sitting in the shared reset_evt. Without doing this we get very early reboots @@ -456,6 +482,7 @@ impl Vmm { reset_evt, &self.seccomp_action, self.hypervisor.clone(), + activate_evt, )?); } @@ -676,6 +703,10 @@ impl Vmm { let reset_evt = self.reset_evt.try_clone().map_err(|e| { MigratableError::MigrateReceive(anyhow!("Error cloning reset EventFd: {}", e)) })?; + let activate_evt = self.activate_evt.try_clone().map_err(|e| { + MigratableError::MigrateReceive(anyhow!("Error cloning activate EventFd: {}", e)) + })?; + self.vm_config = Some(Arc::new(Mutex::new(config))); let vm = Vm::new_from_migration( self.vm_config.clone().unwrap(), @@ -683,6 +714,7 @@ impl Vmm { reset_evt, &self.seccomp_action, self.hypervisor.clone(), + activate_evt, ) .map_err(|e| { MigratableError::MigrateReceive(anyhow!("Error creating VM from snapshot: {:?}", e)) @@ -1065,6 +1097,13 @@ impl Vmm { vm.handle_stdin().map_err(Error::Stdin)?; } } + EpollDispatch::ActivateVirtioDevices => { + if let Some(ref vm) = self.vm { + vm.activate_virtio_devices() + .map_err(Error::ActivateVirtioDevices)?; + } + self.activate_evt.read().map_err(Error::EventFdRead)?; + } EpollDispatch::Api => { // Consume the event. self.api_evt.read().map_err(Error::EventFdRead)?; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 0e15999bd..9b6b8aaf4 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -227,6 +227,9 @@ pub enum Error { /// Failed setting the VmmOps interface. SetVmmOpsInterface(hypervisor::HypervisorVmError), + + /// Cannot activate virtio devices + ActivateVirtioDevices(device_manager::DeviceManagerError), } pub type Result = result::Result; @@ -488,6 +491,7 @@ impl Vm { seccomp_action: &SeccompAction, hypervisor: Arc, #[cfg(feature = "kvm")] _saved_clock: Option, + activate_evt: EventFd, ) -> Result { config .lock() @@ -509,6 +513,7 @@ impl Vm { seccomp_action.clone(), #[cfg(feature = "acpi")] numa_nodes.clone(), + &activate_evt, ) .map_err(Error::DeviceManager)?; @@ -646,6 +651,7 @@ impl Vm { reset_evt: EventFd, seccomp_action: &SeccompAction, hypervisor: Arc, + activate_evt: EventFd, ) -> Result { #[cfg(all(feature = "kvm", target_arch = "x86_64"))] hypervisor.check_required_extensions().unwrap(); @@ -682,6 +688,7 @@ impl Vm { hypervisor, #[cfg(feature = "kvm")] None, + activate_evt, )?; // The device manager must create the devices from here as it is part @@ -704,6 +711,7 @@ impl Vm { prefault: bool, seccomp_action: &SeccompAction, hypervisor: Arc, + activate_evt: EventFd, ) -> Result { #[cfg(all(feature = "kvm", target_arch = "x86_64"))] hypervisor.check_required_extensions().unwrap(); @@ -746,6 +754,7 @@ impl Vm { hypervisor, #[cfg(feature = "kvm")] None, + activate_evt, ) } @@ -755,6 +764,7 @@ impl Vm { reset_evt: EventFd, seccomp_action: &SeccompAction, hypervisor: Arc, + activate_evt: EventFd, ) -> Result { #[cfg(all(feature = "kvm", target_arch = "x86_64"))] hypervisor.check_required_extensions().unwrap(); @@ -781,6 +791,7 @@ impl Vm { hypervisor, #[cfg(feature = "kvm")] None, + activate_evt, ) } @@ -1739,6 +1750,14 @@ impl Vm { pub fn device_tree(&self) -> Arc> { self.device_manager.lock().unwrap().device_tree() } + + pub fn activate_virtio_devices(&self) -> Result<()> { + self.device_manager + .lock() + .unwrap() + .activate_virtio_devices() + .map_err(Error::ActivateVirtioDevices) + } } impl Pausable for Vm {