vqueue encapsulation: make virtio.Queue fields private, expose as fun…

…ctions

Signed-off-by: Kornel Lesiński <kornel@cloudflare.com>

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,

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());
  }
 }

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

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<u16>,
  pub(crate) next_used: Wrapping<u16>,
@@ -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<DescriptorChain<'b, M>> {
- 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::<u16>(avail_event_addr).unwrap()
@@ -1171,7 +1219,7 @@ mod tests {
  assert!(!q.is_valid(m));
  m.write_obj::<u16>(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);