From a105089702a1913ed0014ad713eb97a999a7997b Mon Sep 17 00:00:00 2001 From: Rob Bradford Date: Mon, 18 Jan 2021 17:52:45 +0000 Subject: [PATCH] virtio-devices: Support driver programming fewer queues It is permissable for the driver to program fewer queues than offered by the device. Filter the queues so that only the ready ones are included and check that they have valid addresses configured. Fixes: #2136 Signed-off-by: Rob Bradford --- virtio-devices/src/device.rs | 6 +- virtio-devices/src/transport/pci_device.rs | 84 ++++++++-------------- 2 files changed, 34 insertions(+), 56 deletions(-) diff --git a/virtio-devices/src/device.rs b/virtio-devices/src/device.rs index 6b095b6d4..4c33195ac 100644 --- a/virtio-devices/src/device.rs +++ b/virtio-devices/src/device.rs @@ -250,10 +250,10 @@ impl VirtioCommon { queue_evts: &[EventFd], interrupt_cb: &Arc, ) -> ActivateResult { - if queues.len() != self.queue_sizes.len() || queue_evts.len() != self.queue_sizes.len() { + if queues.len() != queue_evts.len() { error!( - "Cannot perform activate. Expected {} queue(s), got {}", - self.queue_sizes.len(), + "Cannot activate: length mismatch: queue_evts={} queues={}", + queue_evts.len(), queues.len() ); return Err(ActivateError::BadActivate); diff --git a/virtio-devices/src/transport/pci_device.rs b/virtio-devices/src/transport/pci_device.rs index f33dd6cf9..3397f4c83 100644 --- a/virtio-devices/src/transport/pci_device.rs +++ b/virtio-devices/src/transport/pci_device.rs @@ -14,7 +14,7 @@ extern crate vmm_sys_util; use super::VirtioPciCommonConfig; use crate::transport::VirtioTransport; use crate::{ - Queue, VirtioDevice, VirtioDeviceType, VirtioInterrupt, VirtioInterruptType, + ActivateResult, Queue, VirtioDevice, VirtioDeviceType, VirtioInterrupt, VirtioInterruptType, DEVICE_ACKNOWLEDGE, DEVICE_DRIVER, DEVICE_DRIVER_OK, DEVICE_FAILED, DEVICE_FEATURES_OK, DEVICE_INIT, VIRTIO_MSI_NO_VECTOR, }; @@ -515,14 +515,6 @@ impl VirtioPciDevice { self.common_config.driver_status == DEVICE_INIT as u8 } - fn are_queues_valid(&self) -> bool { - if let Some(mem) = self.memory.as_ref() { - self.queues.iter().all(|q| q.is_valid(&mem.memory())) - } else { - false - } - } - // This function is used by the caller to provide the expected base address // for the virtio-pci configuration BAR. pub fn set_config_bar_addr(&mut self, bar_addr: u64) { @@ -653,35 +645,40 @@ 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 = self.queues.clone(); + queues.retain(|q| q.ready); + for (i, queue) in queues.iter().enumerate() { + queue_evts.push(self.queue_evts[i].try_clone().unwrap()); + if !queue.is_valid(&mem.memory()) { + error!("Queue {} is not valid", i); + } + } + return device.activate(mem, virtio_interrupt, queues, queue_evts); + } + } + Ok(()) + } + pub fn maybe_activate(&mut self) { if self.needs_activation() { - 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 queues = self.queues.clone(); - for i in 0..queues.len() { - queue_evts.push(self.queue_evts[i].try_clone().unwrap()); - } - device - .activate(mem, virtio_interrupt, queues, queue_evts) - .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); - } - } + 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 { - !self.device_activated.load(Ordering::SeqCst) - && self.is_driver_ready() - && self.are_queues_valid() + !self.device_activated.load(Ordering::SeqCst) && self.is_driver_ready() } } @@ -1152,29 +1149,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.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(); - let mut device = self.device.lock().unwrap(); - let mut queue_evts = Vec::new(); - let queues = self.queues.clone(); - for i in 0..queues.len() { - queue_evts.push(self.queue_evts[i].try_clone().unwrap()); - } - device - .activate(mem, virtio_interrupt, queues, queue_evts) - .map_err(|e| { - MigratableError::Restore(anyhow!( - "Failed activating the device: {:?}", - e - )) - })?; - } - } + if self.device_activated.load(Ordering::SeqCst) && self.is_driver_ready() { + self.activate().map_err(|e| { + MigratableError::Restore(anyhow!("Failed activating the device: {:?}", e)) + })?; } return Ok(());