refactor: remove actual_size from Queue

The size of queue set by the driver must be always less or equal to the
queue size in FC. This is checked before device activation.
This removes the need for `actual_size` function.

Signed-off-by: Egor Lazarchuk <[email protected]>

Author: ShadowCurse

src/vmm/src/devices/virtio/queue.rs

@@ -5,7 +5,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the THIRD-PARTY file.
 
-use std::cmp::min;
 use std::num::Wrapping;
 use std::sync::atomic::{Ordering, fence};
 
@@ -462,12 +461,6 @@ impl Queue {
         }
     }
 
-    /// 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 {
-        min(self.size, self.max_size)
-    }
-
     /// Validates the queue's in-memory layout is correct.
     pub fn is_valid<M: GuestMemory>(&self, mem: &M) -> bool {
         let desc_table = self.desc_table_address;
@@ -548,9 +541,9 @@ impl Queue {
         // once. Checking and reporting such incorrect driver behavior
         // can prevent potential hanging and Denial-of-Service from
         // happening on the VMM side.
-        if len > self.actual_size() {
+        if self.size < len {
             return Err(InvalidAvailIdx {
-                queue_size: self.actual_size(),
+                queue_size: self.size,
                 reported_len: len,
             });
         }
@@ -602,17 +595,15 @@ impl Queue {
         //
         // We use `self.next_avail` to store the position, in `ring`, of the next available
         // descriptor index, with a twist: we always only increment `self.next_avail`, so the
-        // actual position will be `self.next_avail % self.actual_size()`.
-        let idx = self.next_avail.0 % self.actual_size();
+        // actual position will be `self.next_avail % self.size`.
+        let idx = self.next_avail.0 % self.size;
         // SAFETY:
         // index is bound by the queue size
         let desc_index = unsafe { self.avail_ring_ring_get(usize::from(idx)) };
 
-        DescriptorChain::checked_new(self.desc_table_ptr, self.actual_size(), desc_index).inspect(
-            |_| {
-                self.next_avail += Wrapping(1);
-            },
-        )
+        DescriptorChain::checked_new(self.desc_table_ptr, self.size, desc_index).inspect(|_| {
+            self.next_avail += Wrapping(1);
+        })
     }
 
     /// Undo the effects of the last `self.pop()` call.
@@ -630,15 +621,15 @@ impl Queue {
         desc_index: u16,
         len: u32,
     ) -> Result<(), QueueError> {
-        if self.actual_size() <= desc_index {
+        if self.size <= desc_index {
             error!(
                 "attempted to add out of bounds descriptor to used ring: {}",
                 desc_index
             );
             return Err(QueueError::DescIndexOutOfBounds(desc_index));
         }
 
-        let next_used = (self.next_used + Wrapping(ring_index_offset)).0 % self.actual_size();
+        let next_used = (self.next_used + Wrapping(ring_index_offset)).0 % self.size;
         let used_element = UsedElement {
             id: u32::from(desc_index),
             len,
@@ -684,9 +675,9 @@ impl Queue {
         if len != 0 {
             // The number of descriptor chain heads to process should always
             // be smaller or equal to the queue size.
-            if len > self.actual_size() {
+            if len > self.size {
                 return Err(InvalidAvailIdx {
-                    queue_size: self.actual_size(),
+                    queue_size: self.size,
                     reported_len: len,
                 });
             }
@@ -1086,7 +1077,7 @@ mod verification {
             // done. This is relying on implementation details of add_used, namely that
             // the check for out-of-bounds descriptor index happens at the very beginning of the
             // function.
-            assert!(used_desc_table_index >= queue.actual_size());
+            assert!(used_desc_table_index >= queue.size);
         }
     }
 
@@ -1123,11 +1114,11 @@ mod verification {
 
     #[kani::proof]
     #[kani::unwind(0)]
-    fn verify_actual_size() {
+    fn verify_size() {
         let ProofContext(queue, _) = kani::any();
 
-        assert!(queue.actual_size() <= queue.max_size);
-        assert!(queue.actual_size() <= queue.size);
+        assert!(queue.size <= queue.max_size);
+        assert!(queue.size <= queue.size);
     }
 
     #[kani::proof]
@@ -1192,7 +1183,7 @@ mod verification {
         // is called when the queue is being initialized, e.g. empty. We compute it using
         // local variables here to make things easier on kani: One less roundtrip through vm-memory.
         let queue_len = queue.len();
-        kani::assume(queue_len <= queue.actual_size());
+        kani::assume(queue_len <= queue.size);
 
         let next_avail = queue.next_avail;
 
@@ -1210,7 +1201,7 @@ mod verification {
         let ProofContext(mut queue, _) = kani::any();
 
         // See verify_pop for explanation
-        kani::assume(queue.len() <= queue.actual_size());
+        kani::assume(queue.len() <= queue.size);
 
         let queue_clone = queue.clone();
         if let Some(_) = queue.pop().unwrap() {
@@ -1226,7 +1217,7 @@ mod verification {
     fn verify_try_enable_notification() {
         let ProofContext(mut queue, _) = ProofContext::bounded_queue();
 
-        kani::assume(queue.len() <= queue.actual_size());
+        kani::assume(queue.len() <= queue.size);
 
         if queue.try_enable_notification().unwrap() && queue.uses_notif_suppression {
             // We only require new notifications if the queue is empty (e.g. we've processed
@@ -1244,10 +1235,9 @@ mod verification {
         let ProofContext(queue, mem) = kani::any();
 
         let index = kani::any();
-        let maybe_chain =
-            DescriptorChain::checked_new(queue.desc_table_ptr, queue.actual_size(), index);
+        let maybe_chain = DescriptorChain::checked_new(queue.desc_table_ptr, queue.size, index);
 
-        if index >= queue.actual_size() {
+        if index >= queue.size {
             assert!(maybe_chain.is_none())
         } else {
             // If the index was in-bounds for the descriptor table, we at least should be
@@ -1262,7 +1252,7 @@ mod verification {
             match maybe_chain {
                 None => {
                     // This assert is the negation of the "is_valid" check in checked_new
-                    assert!(desc.flags & VIRTQ_DESC_F_NEXT == 1 && desc.next >= queue.actual_size())
+                    assert!(desc.flags & VIRTQ_DESC_F_NEXT == 1 && desc.next >= queue.size)
                 }
                 Some(head) => {
                     assert!(head.is_valid())

src/vmm/src/devices/virtio/vhost_user.rs

@@ -410,14 +410,14 @@ impl<T: VhostUserHandleBackend> VhostUserHandleImpl<T> {
         // at early stage.
         for (queue_index, queue, _) in queues.iter() {
             self.vu
-                .set_vring_num(*queue_index, queue.actual_size())
+                .set_vring_num(*queue_index, queue.size)
                 .map_err(VhostUserError::VhostUserSetVringNum)?;
         }
 
         for (queue_index, queue, queue_evt) in queues.iter() {
             let config_data = VringConfigData {
                 queue_max_size: queue.max_size,
-                queue_size: queue.actual_size(),
+                queue_size: queue.size,
                 flags: 0u32,
                 desc_table_addr: mem
                     .get_host_address(queue.desc_table_address)