From a78ec87e1349ecb0e0533096e53055ad652ae10e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kornel=20Lesi=C5=84ski?= Date: Mon, 11 Dec 2023 15:22:11 +0100 Subject: [PATCH 1/2] vqueue encapsulation: make virtio.Queue fields private, expose as functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kornel Lesiński --- src/vmm/src/devices/virtio/mmio.rs | 73 ++++++++++-------- src/vmm/src/devices/virtio/net/device.rs | 8 +- src/vmm/src/devices/virtio/persist.rs | 14 ++-- src/vmm/src/devices/virtio/queue.rs | 96 ++++++++++++++++++------ src/vmm/src/devices/virtio/test_utils.rs | 10 +-- src/vmm/src/devices/virtio/vhost_user.rs | 14 ++-- 6 files changed, 136 insertions(+), 79 deletions(-) diff --git a/src/vmm/src/devices/virtio/mmio.rs b/src/vmm/src/devices/virtio/mmio.rs index cd9c1a5e033..1d53210ab9f 100644 --- a/src/vmm/src/devices/virtio/mmio.rs +++ b/src/vmm/src/devices/virtio/mmio.rs @@ -158,7 +158,7 @@ impl MmioTransport { // eventfds, but nothing will happen other than supurious wakeups. // . Do not reset config_generation and keep it monotonically increasing for queue in self.locked_device().queues_mut() { - *queue = Queue::new(queue.get_max_size()); + *queue = Queue::new(queue.max_size()); } } @@ -243,8 +243,8 @@ impl MmioTransport { } features } - 0x34 => self.with_queue(0, |q| u32::from(q.get_max_size())), - 0x44 => self.with_queue(0, |q| u32::from(q.ready)), + 0x34 => self.with_queue(0, |q| u32::from(q.max_size())), + 0x44 => self.with_queue(0, |q| u32::from(q.ready())), 0x60 => { // For vhost-user backed devices we need some additional // logic to differentiate between `VIRTIO_MMIO_INT_VRING` @@ -319,20 +319,20 @@ impl MmioTransport { } 0x24 => self.acked_features_select = v, 0x30 => self.queue_select = v, - 0x38 => self.update_queue_field(|q| q.size = (v & 0xffff) as u16), - 0x44 => self.update_queue_field(|q| q.ready = v == 1), + 0x38 => self.update_queue_field(|q| q.set_size(v as u16)), + 0x44 => self.update_queue_field(|q| q.set_ready(v == 1)), 0x64 => { if self.check_device_status(device_status::DRIVER_OK, 0) { self.interrupt_status.fetch_and(!v, Ordering::SeqCst); } } 0x70 => self.set_device_status(v), - 0x80 => self.update_queue_field(|q| lo(&mut q.desc_table, v)), - 0x84 => self.update_queue_field(|q| hi(&mut q.desc_table, v)), - 0x90 => self.update_queue_field(|q| lo(&mut q.avail_ring, v)), - 0x94 => self.update_queue_field(|q| hi(&mut q.avail_ring, v)), - 0xa0 => self.update_queue_field(|q| lo(&mut q.used_ring, v)), - 0xa4 => self.update_queue_field(|q| hi(&mut q.used_ring, v)), + 0x80 => self.update_queue_field(|q| lo(q.desc_table_mut(), v)), + 0x84 => self.update_queue_field(|q| hi(q.desc_table_mut(), v)), + 0x90 => self.update_queue_field(|q| lo(q.avail_ring_mut(), v)), + 0x94 => self.update_queue_field(|q| hi(q.avail_ring_mut(), v)), + 0xa0 => self.update_queue_field(|q| lo(q.used_ring_mut(), v)), + 0xa4 => self.update_queue_field(|q| hi(q.used_ring_mut(), v)), _ => { warn!("unknown virtio mmio register write: 0x{:x}", offset); } @@ -479,18 +479,24 @@ pub(crate) mod tests { assert!(!d.are_queues_valid()); d.queue_select = 0; - assert_eq!(d.with_queue(0, Queue::get_max_size), 16); - assert!(d.with_queue_mut(|q| q.size = 16)); - assert_eq!(d.locked_device().queues()[d.queue_select as usize].size, 16); + 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(), + 16 + ); d.queue_select = 1; - assert_eq!(d.with_queue(0, Queue::get_max_size), 32); - assert!(d.with_queue_mut(|q| q.size = 16)); - assert_eq!(d.locked_device().queues()[d.queue_select as usize].size, 16); + 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(), + 16 + ); d.queue_select = 2; - assert_eq!(d.with_queue(0, Queue::get_max_size), 0); - assert!(!d.with_queue_mut(|q| q.size = 16)); + assert_eq!(d.with_queue(0, |q| q.max_size()), 0); + assert!(!d.with_queue_mut(|q| q.set_size(16))); assert!(!d.are_queues_valid()); } @@ -667,42 +673,45 @@ 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()[0].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()[0].size(), 16); - assert!(!d.locked_device().queues()[0].ready); + assert!(!d.locked_device().queues()[0].ready()); write_le_u32(&mut buf[..], 1); d.bus_write(0x44, &buf[..]); - assert!(d.locked_device().queues()[0].ready); + assert!(d.locked_device().queues()[0].ready()); - assert_eq!(d.locked_device().queues()[0].desc_table.0, 0); + assert_eq!(d.locked_device().queues()[0].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()[0].desc_table().0, 123); d.bus_write(0x84, &buf[..]); assert_eq!( - d.locked_device().queues()[0].desc_table.0, + d.locked_device().queues()[0].desc_table().0, 123 + (123 << 32) ); - assert_eq!(d.locked_device().queues()[0].avail_ring.0, 0); + assert_eq!(d.locked_device().queues()[0].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()[0].avail_ring().0, 124); d.bus_write(0x94, &buf[..]); assert_eq!( - d.locked_device().queues()[0].avail_ring.0, + d.locked_device().queues()[0].avail_ring().0, 124 + (124 << 32) ); - assert_eq!(d.locked_device().queues()[0].used_ring.0, 0); + assert_eq!(d.locked_device().queues()[0].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()[0].used_ring().0, 125); d.bus_write(0xa4, &buf[..]); - assert_eq!(d.locked_device().queues()[0].used_ring.0, 125 + (125 << 32)); + assert_eq!( + d.locked_device().queues()[0].used_ring().0, + 125 + (125 << 32) + ); set_device_status( &mut d, diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index cc11ad42b23..798134f0218 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -2009,8 +2009,8 @@ pub mod tests { // 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!(queues[RX_INDEX].size(), th.rxq.size()); + assert_eq!(queues[TX_INDEX].size(), th.txq.size()); // Test corresponding queues events. assert_eq!(net.queue_events().len(), NET_QUEUE_SIZES.len()); @@ -2029,7 +2029,7 @@ pub mod tests { let net = th.net(); let queues = net.queues(); - assert!(queues[RX_INDEX].uses_notif_suppression); - assert!(queues[TX_INDEX].uses_notif_suppression); + assert!(queues[RX_INDEX].uses_notif_suppression()); + assert!(queues[TX_INDEX].uses_notif_suppression()); } } diff --git a/src/vmm/src/devices/virtio/persist.rs b/src/vmm/src/devices/virtio/persist.rs index dba9ba1bc41..31a83513dba 100644 --- a/src/vmm/src/devices/virtio/persist.rs +++ b/src/vmm/src/devices/virtio/persist.rs @@ -61,12 +61,12 @@ impl Persist<'_> for Queue { fn save(&self) -> Self::State { QueueState { - max_size: self.max_size, - size: self.size, - ready: self.ready, - desc_table: self.desc_table.0, - avail_ring: self.avail_ring.0, - used_ring: self.used_ring.0, + max_size: self.max_size(), + size: self.size(), + ready: self.ready(), + desc_table: self.desc_table().0, + avail_ring: self.avail_ring().0, + used_ring: self.used_ring().0, next_avail: self.next_avail, next_used: self.next_used, num_added: self.num_added, @@ -161,7 +161,7 @@ impl VirtioDeviceState { for q in &queues { // Sanity check queue size and queue max size. - if q.max_size != expected_queue_max_size || q.size > expected_queue_max_size { + if q.max_size() != expected_queue_max_size || q.size() > expected_queue_max_size { return Err(PersistError::InvalidInput); } // Snapshot can happen at any time, including during device configuration/activation diff --git a/src/vmm/src/devices/virtio/queue.rs b/src/vmm/src/devices/virtio/queue.rs index 45cf6349f7e..0be48b07449 100644 --- a/src/vmm/src/devices/virtio/queue.rs +++ b/src/vmm/src/devices/virtio/queue.rs @@ -183,19 +183,19 @@ pub struct Queue { pub(crate) max_size: u16, /// The queue size in elements the driver selected - pub size: u16, + pub(crate) size: u16, /// Indicates if the queue is finished with configuration - pub ready: bool, + pub(crate) ready: bool, /// Guest physical address of the descriptor table - pub desc_table: GuestAddress, + pub(crate) desc_table: GuestAddress, /// Guest physical address of the available ring - pub avail_ring: GuestAddress, + pub(crate) avail_ring: GuestAddress, /// Guest physical address of the used ring - pub used_ring: GuestAddress, + pub(crate) used_ring: GuestAddress, pub(crate) next_avail: Wrapping, pub(crate) next_used: Wrapping, @@ -225,10 +225,54 @@ impl Queue { } /// Maximum size of the queue. - pub fn get_max_size(&self) -> u16 { + pub fn max_size(&self) -> u16 { self.max_size } + pub fn size(&self) -> u16 { + self.size + } + + pub fn ready(&self) -> bool { + self.ready + } + + pub fn desc_table(&self) -> GuestAddress { + self.desc_table + } + + pub fn avail_ring(&self) -> GuestAddress { + self.avail_ring + } + + pub fn used_ring(&self) -> GuestAddress { + self.used_ring + } + + pub fn desc_table_mut(&mut self) -> &mut GuestAddress { + &mut self.desc_table + } + + pub fn avail_ring_mut(&mut self) -> &mut GuestAddress { + &mut self.avail_ring + } + + pub fn used_ring_mut(&mut self) -> &mut GuestAddress { + &mut self.used_ring + } + + pub fn set_max_size(&mut self, val: u16) { + self.max_size = val; + } + + pub fn set_size(&mut self, val: u16) { + self.size = val; + } + + pub fn set_ready(&mut self, val: bool) { + self.ready = val; + } + /// Return the actual size of the queue, as the driver may not set up a /// queue as big as the device allows. pub fn actual_size(&self) -> u16 { @@ -347,7 +391,7 @@ impl Queue { &mut self, mem: &'b M, ) -> Option> { - if !self.uses_notif_suppression { + if !self.uses_notif_suppression() { return self.pop(mem); } @@ -499,7 +543,7 @@ impl Queue { // If the device doesn't use notification suppression, we'll continue to get notifications // no matter what. - if !self.uses_notif_suppression { + if !self.uses_notif_suppression() { return true; } @@ -533,6 +577,10 @@ impl Queue { self.uses_notif_suppression = true; } + pub fn uses_notif_suppression(&self) -> bool { + self.uses_notif_suppression + } + /// Check if we need to kick the guest. /// /// Please note this method has side effects: once it returns `true`, it considers the @@ -544,7 +592,7 @@ impl Queue { debug_assert!(self.is_layout_valid(mem)); // If the device doesn't use notification suppression, always return true - if !self.uses_notif_suppression { + if !self.uses_notif_suppression() { return true; } @@ -959,8 +1007,8 @@ mod verification { fn verify_actual_size() { let ProofContext(queue, _) = kani::any(); - assert!(queue.actual_size() <= queue.get_max_size()); - assert!(queue.actual_size() <= queue.size); + assert!(queue.actual_size() <= queue.max_size()); + assert!(queue.actual_size() <= queue.size()); } #[kani::proof] @@ -1075,7 +1123,7 @@ mod tests { impl Queue { fn avail_event(&self, mem: &GuestMemoryMmap) -> u16 { let avail_event_addr = self - .used_ring + .used_ring() .unchecked_add(u64::from(4 + 8 * self.actual_size())); mem.read_obj::(avail_event_addr).unwrap() @@ -1171,7 +1219,7 @@ mod tests { assert!(!q.is_valid(m)); m.write_obj::(5_u16, q.avail_ring.unchecked_add(2)) .unwrap(); - q.max_size = 2; + q.set_max_size(2); assert!(!q.is_valid(m)); // reset dirtied values @@ -1182,23 +1230,23 @@ mod tests { // or if the various addresses are off - q.desc_table = GuestAddress(0xffff_ffff); + *q.desc_table_mut() = GuestAddress(0xffff_ffff); assert!(!q.is_valid(m)); - q.desc_table = GuestAddress(0x1001); + *q.desc_table_mut() = GuestAddress(0x1001); assert!(!q.is_valid(m)); - q.desc_table = vq.dtable_start(); + *q.desc_table_mut() = vq.dtable_start(); - q.avail_ring = GuestAddress(0xffff_ffff); + *q.avail_ring_mut() = GuestAddress(0xffff_ffff); assert!(!q.is_valid(m)); - q.avail_ring = GuestAddress(0x1001); + *q.avail_ring_mut() = GuestAddress(0x1001); assert!(!q.is_valid(m)); - q.avail_ring = vq.avail_start(); + *q.avail_ring_mut() = vq.avail_start(); - q.used_ring = GuestAddress(0xffff_ffff); + *q.used_ring_mut() = GuestAddress(0xffff_ffff); assert!(!q.is_valid(m)); - q.used_ring = GuestAddress(0x1001); + *q.used_ring_mut() = GuestAddress(0x1001); assert!(!q.is_valid(m)); - q.used_ring = vq.used_start(); + *q.used_ring_mut() = vq.used_start(); } #[test] @@ -1351,7 +1399,7 @@ mod tests { let vq = VirtQueue::new(GuestAddress(0), m, 4); let mut q = vq.create_queue(); - q.uses_notif_suppression = true; + q.enable_notif_suppression(); // Create 1 descriptor chain of 4. for j in 0..4 { @@ -1476,7 +1524,7 @@ mod tests { let vq = VirtQueue::new(GuestAddress(0), m, 16); let mut q = vq.create_queue(); - q.ready = true; + q.set_ready(true); // We create a simple descriptor chain vq.dtable[0].set(0x1000_u64, 0x1000, 0, 0); diff --git a/src/vmm/src/devices/virtio/test_utils.rs b/src/vmm/src/devices/virtio/test_utils.rs index 96971574c3d..7b96eb62e2e 100644 --- a/src/vmm/src/devices/virtio/test_utils.rs +++ b/src/vmm/src/devices/virtio/test_utils.rs @@ -294,11 +294,11 @@ impl<'a> VirtQueue<'a> { pub fn create_queue(&self) -> Queue { let mut q = Queue::new(self.size()); - q.size = self.size(); - q.ready = true; - q.desc_table = self.dtable_start(); - q.avail_ring = self.avail_start(); - q.used_ring = self.used_start(); + q.set_size(self.size()); + q.set_ready(true); + *q.desc_table_mut() = self.dtable_start(); + *q.avail_ring_mut() = self.avail_start(); + *q.used_ring_mut() = self.used_start(); q } diff --git a/src/vmm/src/devices/virtio/vhost_user.rs b/src/vmm/src/devices/virtio/vhost_user.rs index bdb12015fcd..a6a1af8b68d 100644 --- a/src/vmm/src/devices/virtio/vhost_user.rs +++ b/src/vmm/src/devices/virtio/vhost_user.rs @@ -417,18 +417,18 @@ impl VhostUserHandleImpl { for (queue_index, queue, queue_evt) in queues.iter() { let config_data = VringConfigData { - queue_max_size: queue.get_max_size(), + queue_max_size: queue.max_size(), queue_size: queue.actual_size(), flags: 0u32, desc_table_addr: mem - .get_host_address(queue.desc_table) + .get_host_address(queue.desc_table()) .map_err(VhostUserError::DescriptorTableAddress)? as u64, used_ring_addr: mem - .get_host_address(queue.used_ring) + .get_host_address(queue.used_ring()) .map_err(VhostUserError::UsedAddress)? as u64, avail_ring_addr: mem - .get_host_address(queue.avail_ring) + .get_host_address(queue.avail_ring()) .map_err(VhostUserError::AvailAddress)? as u64, log_addr: None, }; @@ -910,9 +910,9 @@ mod tests { queue_max_size: 69, queue_size: 0, flags: 0, - desc_table_addr: guest_memory.get_host_address(queue.desc_table).unwrap() as u64, - used_ring_addr: guest_memory.get_host_address(queue.used_ring).unwrap() as u64, - avail_ring_addr: guest_memory.get_host_address(queue.avail_ring).unwrap() as u64, + desc_table_addr: guest_memory.get_host_address(queue.desc_table()).unwrap() as u64, + used_ring_addr: guest_memory.get_host_address(queue.used_ring()).unwrap() as u64, + avail_ring_addr: guest_memory.get_host_address(queue.avail_ring()).unwrap() as u64, log_addr: None, }, base: queue.avail_idx(&guest_memory).0, 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 2/2] 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] {