diff --git a/crosvm-fuzz/virtqueue_fuzzer.rs b/crosvm-fuzz/virtqueue_fuzzer.rs index bdc121a830..1926efd4a6 100644 --- a/crosvm-fuzz/virtqueue_fuzzer.rs +++ b/crosvm-fuzz/virtqueue_fuzzer.rs @@ -73,9 +73,11 @@ fuzz_target!(|data: &[u8]| { q.set_ready(true); GUEST_MEM.with(|mem| { - if !q.ready() { + let mut q = if let Ok(q) = q.activate() { + q + } else { return; - } + }; // First zero out all of the memory. let vs = mem diff --git a/devices/src/virtio/queue.rs b/devices/src/virtio/queue.rs index 9455928196..79074d4763 100644 --- a/devices/src/virtio/queue.rs +++ b/devices/src/virtio/queue.rs @@ -296,14 +296,11 @@ impl<'a, 'b> Iterator for AvailIter<'a, 'b> { } } -#[derive(Clone)] /// A virtio queue's parameters. -/// -/// WARNING: it is NOT safe to clone and then use n>1 Queue(s) to interact with the same virtqueue. -/// That will prevent descriptor index tracking from being accurate, which can cause incorrect -/// interrupt masking. -/// TODO(b/201119859) drop Clone from this struct. pub struct Queue { + /// Whether this queue has already been activated. + activated: bool, + /// The maximal size in elements offered by the device max_size: u16, @@ -365,6 +362,7 @@ impl Queue { pub fn new(max_size: u16) -> Queue { assert!(max_size.is_power_of_two()); Queue { + activated: false, max_size, size: max_size, ready: false, @@ -453,6 +451,39 @@ impl Queue { self.ready = enable; } + /// Convert the queue configuration into an active queue. + pub fn activate(&mut self) -> Result { + if !self.ready { + bail!("attempted to activate a non-ready queue"); + } + + if self.activated { + bail!("queue is already activated"); + } + + self.activated = true; + + let queue = Queue { + activated: self.activated, + max_size: self.max_size, + size: self.size, + ready: self.ready, + vector: self.vector, + desc_table: self.desc_table, + avail_ring: self.avail_ring, + used_ring: self.used_ring, + next_avail: self.next_avail, + next_used: self.next_used, + features: self.features, + last_used: self.last_used, + iommu: self.iommu.as_ref().map(Arc::clone), + exported_desc_table: self.exported_desc_table.clone(), + exported_avail_ring: self.exported_avail_ring.clone(), + exported_used_ring: self.exported_used_ring.clone(), + }; + Ok(queue) + } + // Return `index` modulo the currently configured queue size. fn wrap_queue_index(&self, index: Wrapping) -> u16 { // We know that `self.size` is a power of two (enforced by `set_size()`), so the modulus can @@ -463,6 +494,7 @@ impl Queue { /// Reset queue to a clean state pub fn reset(&mut self) { + self.activated = false; self.ready = false; self.size = self.max_size; self.vector = VIRTIO_MSI_NO_VECTOR; diff --git a/devices/src/virtio/vhost/user/device/handler.rs b/devices/src/virtio/vhost/user/device/handler.rs index d215e126d6..5ec15fd644 100644 --- a/devices/src/virtio/vhost/user/device/handler.rs +++ b/devices/src/virtio/vhost/user/device/handler.rs @@ -569,7 +569,14 @@ impl VhostUserSlaveReqHandlerMut for DeviceRequestHandl vring.queue.ack_features(self.backend.acked_features()); vring.queue.set_ready(true); - let queue = vring.queue.clone(); + let queue = match vring.queue.activate() { + Ok(queue) => queue, + Err(e) => { + error!("failed to activate vring: {:#}", e); + return Err(VhostError::SlaveInternalError); + } + }; + let doorbell = vring.doorbell.clone().ok_or(VhostError::InvalidOperation)?; let mem = self .mem diff --git a/devices/src/virtio/virtio_mmio_device.rs b/devices/src/virtio/virtio_mmio_device.rs index 6d378b7980..17138fa296 100644 --- a/devices/src/virtio/virtio_mmio_device.rs +++ b/devices/src/virtio/virtio_mmio_device.rs @@ -152,12 +152,12 @@ impl VirtioMmioDevice { // Use ready queues and their events. let queues = self .queues - .iter() + .iter_mut() .zip(self.queue_evts.iter()) .filter(|(q, _)| q.ready()) .map(|(queue, evt)| { Ok(( - queue.clone(), + queue.activate().context("failed to activate queue")?, evt.try_clone().context("failed to clone queue_evt")?, )) }) diff --git a/devices/src/virtio/virtio_pci_device.rs b/devices/src/virtio/virtio_pci_device.rs index 669af82c7b..5b6dc753af 100644 --- a/devices/src/virtio/virtio_pci_device.rs +++ b/devices/src/virtio/virtio_pci_device.rs @@ -491,12 +491,12 @@ impl VirtioPciDevice { // Use ready queues and their events. let queues = self .queues - .iter() + .iter_mut() .zip(self.queue_evts.iter()) .filter(|(q, _)| q.ready()) .map(|(queue, evt)| { Ok(( - queue.clone(), + queue.activate().context("failed to activate queue")?, evt.try_clone().context("failed to clone queue_evt")?, )) })