From a321d2424976a86b7007871d631ae66bb6d11f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kornel=20Lesi=C5=84ski?= Date: Mon, 11 Dec 2023 15:51:34 +0100 Subject: [PATCH] vqueue abstractions: use iterators for VirtioDevice.queues() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kornel Lesiński --- src/vmm/src/device_manager/mmio.rs | 10 +-- src/vmm/src/devices/virtio/balloon/device.rs | 10 +-- src/vmm/src/devices/virtio/balloon/persist.rs | 2 +- src/vmm/src/devices/virtio/device.rs | 11 ++-- src/vmm/src/devices/virtio/mmio.rs | 65 ++++++++++++------- src/vmm/src/devices/virtio/net/device.rs | 24 ++++--- src/vmm/src/devices/virtio/persist.rs | 2 +- src/vmm/src/devices/virtio/queue.rs | 3 + src/vmm/src/devices/virtio/rng/device.rs | 24 +++++-- .../devices/virtio/vhost_user_block/device.rs | 10 +-- .../src/devices/virtio/virtio_block/device.rs | 10 +-- .../devices/virtio/virtio_block/persist.rs | 2 +- src/vmm/src/devices/virtio/vsock/device.rs | 10 +-- 13 files changed, 105 insertions(+), 78 deletions(-) diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index bc95bd8822e..4c347f58b9c 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -467,7 +467,7 @@ mod tests { use super::*; use crate::devices::virtio::device::VirtioDevice; - use crate::devices::virtio::queue::Queue; + use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut}; use crate::devices::virtio::ActivateError; use crate::vstate::memory::{GuestAddress, GuestMemoryExtension, GuestMemoryMmap}; use crate::{builder, Vm}; @@ -535,12 +535,12 @@ mod tests { 0 } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { diff --git a/src/vmm/src/devices/virtio/balloon/device.rs b/src/vmm/src/devices/virtio/balloon/device.rs index 541af3e5080..427c1348455 100644 --- a/src/vmm/src/devices/virtio/balloon/device.rs +++ b/src/vmm/src/devices/virtio/balloon/device.rs @@ -14,7 +14,6 @@ use utils::eventfd::EventFd; use utils::u64_to_usize; use super::super::device::{DeviceState, VirtioDevice}; -use super::super::queue::Queue; use super::super::{ActivateError, TYPE_BALLOON}; use super::metrics::METRICS; use super::util::{compact_page_frame_numbers, remove_range}; @@ -30,6 +29,7 @@ use super::{ use crate::devices::virtio::balloon::BalloonError; use crate::devices::virtio::device::{IrqTrigger, IrqType}; use crate::devices::virtio::gen::virtio_blk::VIRTIO_F_VERSION_1; +use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut}; use crate::logger::IncMetric; use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap}; @@ -573,12 +573,12 @@ impl VirtioDevice for Balloon { TYPE_BALLOON } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { diff --git a/src/vmm/src/devices/virtio/balloon/persist.rs b/src/vmm/src/devices/virtio/balloon/persist.rs index cd201fe6589..bf9eb6e1bd9 100644 --- a/src/vmm/src/devices/virtio/balloon/persist.rs +++ b/src/vmm/src/devices/virtio/balloon/persist.rs @@ -209,7 +209,7 @@ mod tests { assert_eq!(restored_balloon.acked_features, balloon.acked_features); assert_eq!(restored_balloon.avail_features, balloon.avail_features); assert_eq!(restored_balloon.config_space, balloon.config_space); - assert_eq!(restored_balloon.queues(), balloon.queues()); + assert!(restored_balloon.queues().eq(balloon.queues())); assert_eq!( restored_balloon.interrupt_status().load(Ordering::Relaxed), balloon.interrupt_status().load(Ordering::Relaxed) diff --git a/src/vmm/src/devices/virtio/device.rs b/src/vmm/src/devices/virtio/device.rs index 84756c03096..4f68fa5723a 100644 --- a/src/vmm/src/devices/virtio/device.rs +++ b/src/vmm/src/devices/virtio/device.rs @@ -12,8 +12,8 @@ use std::sync::Arc; use utils::eventfd::EventFd; use super::mmio::{VIRTIO_MMIO_INT_CONFIG, VIRTIO_MMIO_INT_VRING}; -use super::queue::Queue; use super::ActivateError; +use crate::devices::virtio::queue::{QueueIter, QueueIterMut}; use crate::devices::virtio::AsAny; use crate::logger::{error, warn}; use crate::vstate::memory::GuestMemoryMmap; @@ -111,10 +111,10 @@ pub trait VirtioDevice: AsAny + Send { fn device_type(&self) -> u32; /// Returns the device queues. - fn queues(&self) -> &[Queue]; + fn queues(&self) -> QueueIter; /// Returns a mutable reference to the device queues. - fn queues_mut(&mut self) -> &mut [Queue]; + fn queues_mut(&mut self) -> QueueIterMut; /// Returns the device queues event fds. fn queue_events(&self) -> &[EventFd]; @@ -190,6 +190,7 @@ impl fmt::Debug for dyn VirtioDevice { #[cfg(test)] pub(crate) mod tests { use super::*; + use crate::devices::virtio::queue::{QueueIter, QueueIterMut}; impl IrqTrigger { pub fn has_pending_irq(&self, irq_type: IrqType) -> bool { @@ -254,11 +255,11 @@ pub(crate) mod tests { todo!() } - fn queues(&self) -> &[Queue] { + fn queues(&self) -> QueueIter { todo!() } - fn queues_mut(&mut self) -> &mut [Queue] { + fn queues_mut(&mut self) -> QueueIterMut { todo!() } diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs index 1d53210ab9f..d8edc7c2ad5 100644 --- a/src/vmm/src/devices/virtio/mmio.rs +++ b/src/vmm/src/devices/virtio/mmio.rs @@ -97,10 +97,7 @@ impl MmioTransport { } fn are_queues_valid(&self) -> bool { - self.locked_device() - .queues() - .iter() - .all(|q| q.is_valid(&self.mem)) + self.locked_device().queues().all(|q| q.is_valid(&self.mem)) } fn with_queue(&self, d: U, f: F) -> U @@ -111,7 +108,7 @@ impl MmioTransport { match self .locked_device() .queues() - .get(self.queue_select as usize) + .nth(self.queue_select as usize) { Some(queue) => f(queue), None => d, @@ -122,7 +119,7 @@ impl MmioTransport { if let Some(queue) = self .locked_device() .queues_mut() - .get_mut(self.queue_select as usize) + .nth(self.queue_select as usize) { f(queue); true @@ -363,6 +360,7 @@ pub(crate) mod tests { use utils::u64_to_usize; use super::*; + use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut}; use crate::devices::virtio::ActivateError; use crate::vstate::memory::{GuestMemoryExtension, GuestMemoryMmap}; @@ -417,12 +415,12 @@ pub(crate) mod tests { 123 } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { @@ -482,7 +480,11 @@ pub(crate) mod tests { assert_eq!(d.with_queue(0, |q| q.max_size()), 16); assert!(d.with_queue_mut(|q| q.set_size(16))); assert_eq!( - d.locked_device().queues()[d.queue_select as usize].size(), + d.locked_device() + .queues() + .nth(d.queue_select as usize) + .unwrap() + .size(), 16 ); @@ -490,7 +492,11 @@ pub(crate) mod tests { assert_eq!(d.with_queue(0, |q| q.max_size()), 32); assert!(d.with_queue_mut(|q| q.set_size(16))); assert_eq!( - d.locked_device().queues()[d.queue_select as usize].size(), + d.locked_device() + .queues() + .nth(d.queue_select as usize) + .unwrap() + .size(), 16 ); @@ -673,43 +679,52 @@ pub(crate) mod tests { assert_eq!(d.queue_select, 3); d.queue_select = 0; - assert_eq!(d.locked_device().queues()[0].size(), 0); + assert_eq!(d.locked_device().queues().nth(0).unwrap().size(), 0); write_le_u32(&mut buf[..], 16); d.bus_write(0x38, &buf[..]); - assert_eq!(d.locked_device().queues()[0].size(), 16); + assert_eq!(d.locked_device().queues().nth(0).unwrap().size(), 16); - assert!(!d.locked_device().queues()[0].ready()); + assert!(!d.locked_device().queues().nth(0).unwrap().ready()); write_le_u32(&mut buf[..], 1); d.bus_write(0x44, &buf[..]); - assert!(d.locked_device().queues()[0].ready()); + assert!(d.locked_device().queues().nth(0).unwrap().ready()); - assert_eq!(d.locked_device().queues()[0].desc_table().0, 0); + assert_eq!(d.locked_device().queues().nth(0).unwrap().desc_table().0, 0); write_le_u32(&mut buf[..], 123); d.bus_write(0x80, &buf[..]); - assert_eq!(d.locked_device().queues()[0].desc_table().0, 123); + assert_eq!( + d.locked_device().queues().nth(0).unwrap().desc_table().0, + 123 + ); d.bus_write(0x84, &buf[..]); assert_eq!( - d.locked_device().queues()[0].desc_table().0, + d.locked_device().queues().nth(0).unwrap().desc_table().0, 123 + (123 << 32) ); - assert_eq!(d.locked_device().queues()[0].avail_ring().0, 0); + assert_eq!(d.locked_device().queues().nth(0).unwrap().avail_ring().0, 0); write_le_u32(&mut buf[..], 124); d.bus_write(0x90, &buf[..]); - assert_eq!(d.locked_device().queues()[0].avail_ring().0, 124); + assert_eq!( + d.locked_device().queues().nth(0).unwrap().avail_ring().0, + 124 + ); d.bus_write(0x94, &buf[..]); assert_eq!( - d.locked_device().queues()[0].avail_ring().0, + d.locked_device().queues().nth(0).unwrap().avail_ring().0, 124 + (124 << 32) ); - assert_eq!(d.locked_device().queues()[0].used_ring().0, 0); + assert_eq!(d.locked_device().queues().nth(0).unwrap().used_ring().0, 0); write_le_u32(&mut buf[..], 125); d.bus_write(0xa0, &buf[..]); - assert_eq!(d.locked_device().queues()[0].used_ring().0, 125); + assert_eq!( + d.locked_device().queues().nth(0).unwrap().used_ring().0, + 125 + ); d.bus_write(0xa4, &buf[..]); assert_eq!( - d.locked_device().queues()[0].used_ring().0, + d.locked_device().queues().nth(0).unwrap().used_ring().0, 125 + (125 << 32) ); diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 798134f0218..d37f2d316ea 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -33,7 +33,7 @@ use crate::devices::virtio::net::tap::Tap; use crate::devices::virtio::net::{ gen, NetError, NetQueue, MAX_BUFFER_SIZE, NET_QUEUE_SIZES, RX_INDEX, TX_INDEX, }; -use crate::devices::virtio::queue::{DescriptorChain, Queue}; +use crate::devices::virtio::queue::{DescriptorChain, Queue, QueueIter, QueueIterMut}; use crate::devices::virtio::{ActivateError, TYPE_NET}; use crate::devices::{report_net_event_fail, DeviceError}; use crate::dumbo::pdu::arp::ETH_IPV4_FRAME_LEN; @@ -787,12 +787,12 @@ impl VirtioDevice for Net { TYPE_NET } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { @@ -845,7 +845,7 @@ impl VirtioDevice for Net { fn activate(&mut self, mem: GuestMemoryMmap) -> Result<(), ActivateError> { let event_idx = self.has_feature(u64::from(VIRTIO_RING_F_EVENT_IDX)); if event_idx { - for queue in &mut self.queues { + for queue in self.queues_mut() { queue.enable_notif_suppression(); } } @@ -2007,10 +2007,9 @@ pub mod tests { let net = th.net.lock().unwrap(); // Test queues count (TX and RX). - let queues = net.queues(); - assert_eq!(queues.len(), NET_QUEUE_SIZES.len()); - assert_eq!(queues[RX_INDEX].size(), th.rxq.size()); - assert_eq!(queues[TX_INDEX].size(), th.txq.size()); + assert_eq!(net.queues().count(), NET_QUEUE_SIZES.len()); + assert_eq!(net.queues().nth(RX_INDEX).unwrap().size(), th.rxq.size()); + assert_eq!(net.queues().nth(TX_INDEX).unwrap().size(), th.txq.size()); // Test corresponding queues events. assert_eq!(net.queue_events().len(), NET_QUEUE_SIZES.len()); @@ -2028,8 +2027,7 @@ pub mod tests { th.activate_net(); let net = th.net(); - let queues = net.queues(); - assert!(queues[RX_INDEX].uses_notif_suppression()); - assert!(queues[TX_INDEX].uses_notif_suppression()); + assert!(net.queues().nth(RX_INDEX).unwrap().uses_notif_suppression()); + assert!(net.queues().nth(TX_INDEX).unwrap().uses_notif_suppression()); } } diff --git a/src/vmm/src/devices/virtio/persist.rs b/src/vmm/src/devices/virtio/persist.rs index 31a83513dba..a984300a070 100644 --- a/src/vmm/src/devices/virtio/persist.rs +++ b/src/vmm/src/devices/virtio/persist.rs @@ -118,7 +118,7 @@ impl VirtioDeviceState { device_type: device.device_type(), avail_features: device.avail_features(), acked_features: device.acked_features(), - queues: device.queues().iter().map(Persist::save).collect(), + queues: device.queues().map(Persist::save).collect(), interrupt_status: device.interrupt_status().load(Ordering::Relaxed), interrupt_status_old: device.interrupt_status().load(Ordering::Relaxed) as usize, activated: device.is_activated(), diff --git a/src/vmm/src/devices/virtio/queue.rs b/src/vmm/src/devices/virtio/queue.rs index 0be48b07449..f2dbd8e5b14 100644 --- a/src/vmm/src/devices/virtio/queue.rs +++ b/src/vmm/src/devices/virtio/queue.rs @@ -36,6 +36,9 @@ pub enum QueueError { UsedRing(#[from] vm_memory::GuestMemoryError), } +pub type QueueIter<'a> = std::slice::Iter<'a, Queue>; +pub type QueueIterMut<'a> = std::slice::IterMut<'a, Queue>; + /// A virtio descriptor constraints with C representative. #[repr(C)] #[derive(Default, Clone, Copy)] diff --git a/src/vmm/src/devices/virtio/rng/device.rs b/src/vmm/src/devices/virtio/rng/device.rs index 9f7ec9274d9..c183ef43db1 100644 --- a/src/vmm/src/devices/virtio/rng/device.rs +++ b/src/vmm/src/devices/virtio/rng/device.rs @@ -14,7 +14,7 @@ use super::{RNG_NUM_QUEUES, RNG_QUEUE}; use crate::devices::virtio::device::{DeviceState, IrqTrigger, IrqType, VirtioDevice}; use crate::devices::virtio::gen::virtio_rng::VIRTIO_F_VERSION_1; use crate::devices::virtio::iovec::IoVecBufferMut; -use crate::devices::virtio::queue::{Queue, FIRECRACKER_MAX_QUEUE_SIZE}; +use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut, FIRECRACKER_MAX_QUEUE_SIZE}; use crate::devices::virtio::{ActivateError, TYPE_RNG}; use crate::devices::DeviceError; use crate::logger::{debug, error, IncMetric}; @@ -246,12 +246,12 @@ impl VirtioDevice for Entropy { TYPE_RNG } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { @@ -430,14 +430,24 @@ mod tests { let mut entropy_dev = th.device(); // This should succeed, we just added two descriptors - let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap(); + let desc = entropy_dev + .queues_mut() + .nth(RNG_QUEUE) + .unwrap() + .pop(&mem) + .unwrap(); assert!(matches!( IoVecBufferMut::from_descriptor_chain(&mem, desc,), Err(crate::devices::virtio::iovec::IoVecError::ReadOnlyDescriptor) )); // This should succeed, we should have one more descriptor - let desc = entropy_dev.queues_mut()[RNG_QUEUE].pop(&mem).unwrap(); + let desc = entropy_dev + .queues_mut() + .nth(RNG_QUEUE) + .unwrap() + .pop(&mem) + .unwrap(); let mut iovec = IoVecBufferMut::from_descriptor_chain(&mem, desc).unwrap(); assert!(entropy_dev.handle_one(&mut iovec).is_ok()); } diff --git a/src/vmm/src/devices/virtio/vhost_user_block/device.rs b/src/vmm/src/devices/virtio/vhost_user_block/device.rs index 65178785a2f..e44c4274676 100644 --- a/src/vmm/src/devices/virtio/vhost_user_block/device.rs +++ b/src/vmm/src/devices/virtio/vhost_user_block/device.rs @@ -22,7 +22,7 @@ use crate::devices::virtio::gen::virtio_blk::{ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_F_VERSION_1, }; use crate::devices::virtio::gen::virtio_ring::VIRTIO_RING_F_EVENT_IDX; -use crate::devices::virtio::queue::Queue; +use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut}; use crate::devices::virtio::vhost_user::{VhostUserHandleBackend, VhostUserHandleImpl}; use crate::devices::virtio::vhost_user_metrics::{ VhostUserDeviceMetrics, VhostUserMetricsPerDevice, @@ -301,12 +301,12 @@ impl VirtioDevice for VhostUserBlock TYPE_BLOCK } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { diff --git a/src/vmm/src/devices/virtio/virtio_block/device.rs b/src/vmm/src/devices/virtio/virtio_block/device.rs index 81d3a54b661..b913739d72d 100644 --- a/src/vmm/src/devices/virtio/virtio_block/device.rs +++ b/src/vmm/src/devices/virtio/virtio_block/device.rs @@ -32,7 +32,7 @@ use crate::devices::virtio::gen::virtio_blk::{ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES, VIRTIO_F_VERSION_1, }; use crate::devices::virtio::gen::virtio_ring::VIRTIO_RING_F_EVENT_IDX; -use crate::devices::virtio::queue::Queue; +use crate::devices::virtio::queue::{Queue, QueueIter, QueueIterMut}; use crate::devices::virtio::virtio_block::metrics::{BlockDeviceMetrics, BlockMetricsPerDevice}; use crate::devices::virtio::{ActivateError, TYPE_BLOCK}; use crate::logger::{error, warn, IncMetric}; @@ -596,12 +596,12 @@ impl VirtioDevice for VirtioBlock { TYPE_BLOCK } - fn queues(&self) -> &[Queue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [Queue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] { diff --git a/src/vmm/src/devices/virtio/virtio_block/persist.rs b/src/vmm/src/devices/virtio/virtio_block/persist.rs index 3fd7e91d470..515016c61bc 100644 --- a/src/vmm/src/devices/virtio/virtio_block/persist.rs +++ b/src/vmm/src/devices/virtio/virtio_block/persist.rs @@ -328,7 +328,7 @@ mod tests { assert_eq!(restored_block.device_type(), TYPE_BLOCK); assert_eq!(restored_block.avail_features(), block.avail_features()); assert_eq!(restored_block.acked_features(), block.acked_features()); - assert_eq!(restored_block.queues(), block.queues()); + assert!(restored_block.queues().eq(block.queues())); assert_eq!( restored_block.interrupt_status().load(Ordering::Relaxed), block.interrupt_status().load(Ordering::Relaxed) diff --git a/src/vmm/src/devices/virtio/vsock/device.rs b/src/vmm/src/devices/virtio/vsock/device.rs index 873a5cf2fb0..89cf228455c 100644 --- a/src/vmm/src/devices/virtio/vsock/device.rs +++ b/src/vmm/src/devices/virtio/vsock/device.rs @@ -32,7 +32,7 @@ use super::defs::uapi; use super::packet::{VsockPacket, VSOCK_PKT_HDR_SIZE}; use super::{defs, VsockBackend}; use crate::devices::virtio::device::{DeviceState, IrqTrigger, IrqType, VirtioDevice}; -use crate::devices::virtio::queue::Queue as VirtQueue; +use crate::devices::virtio::queue::{Queue as VirtQueue, QueueIter, QueueIterMut}; use crate::devices::virtio::vsock::metrics::METRICS; use crate::devices::virtio::vsock::VsockError; use crate::devices::virtio::ActivateError; @@ -276,12 +276,12 @@ where uapi::VIRTIO_ID_VSOCK } - fn queues(&self) -> &[VirtQueue] { - &self.queues + fn queues(&self) -> QueueIter { + self.queues.iter() } - fn queues_mut(&mut self) -> &mut [VirtQueue] { - &mut self.queues + fn queues_mut(&mut self) -> QueueIterMut { + self.queues.iter_mut() } fn queue_events(&self) -> &[EventFd] {