virtio-queue: Add helpers for accessing queue

These helpers are meant to help crate's consumers getting and setting
information about the queue.

Signed-off-by: Sebastien Boeuf <[email protected]>

Author: Sebastien Boeuf

coverage_config_x86_64.json

@@ -1,5 +1,5 @@
 {
-  "coverage_score": 90.0,
+  "coverage_score": 90.3,
   "exclude_path": "crates/virtio-queue/src/mock.rs",
   "crate_features": "virtio-blk/backend-stdio"
 }

crates/virtio-queue/src/lib.rs

@@ -154,6 +154,9 @@ pub trait QueueStateT: for<'a> QueueStateGuard<'a> {
     /// Read the `idx` field from the available ring.
     fn avail_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
 
+    /// Read the `idx` field from the used ring.
+    fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error>;
+
     /// Put a used descriptor head into the used ring.
     fn add_used<M: GuestMemory>(&mut self, mem: &M, head_index: u16, len: u32)
         -> Result<(), Error>;
@@ -181,4 +184,10 @@ pub trait QueueStateT: for<'a> QueueStateGuard<'a> {
 
     /// Set the index of the next entry in the available ring.
     fn set_next_avail(&mut self, next_avail: u16);
+
+    /// Return the index for the next descriptor in the used ring.
+    fn next_used(&self) -> u16;
+
+    /// Set the index for the next descriptor in the used ring.
+    fn set_next_used(&mut self, next_used: u16);
 }

crates/virtio-queue/src/queue.rs

@@ -197,6 +197,14 @@ impl<M: GuestAddressSpace, S: QueueStateT> Queue<M, S> {
         self.state.avail_idx(self.mem.memory().deref(), order)
     }
 
+    /// Reads the `idx` field from the used ring.
+    ///
+    /// # Arguments
+    /// * `order` - the memory ordering used to access the `idx` field from memory.
+    pub fn used_idx(&self, order: Ordering) -> Result<Wrapping<u16>, Error> {
+        self.state.used_idx(self.mem.memory().deref(), order)
+    }
+
     /// Put a used descriptor head into the used ring.
     ///
     /// # Arguments
@@ -236,13 +244,26 @@ impl<M: GuestAddressSpace, S: QueueStateT> Queue<M, S> {
         self.state.next_avail()
     }
 
+    /// Returns the index for the next descriptor in the used ring.
+    pub fn next_used(&self) -> u16 {
+        self.state.next_used()
+    }
+
     /// Set the index of the next entry in the available ring.
     ///
     /// # Arguments
     /// * `next_avail` - the index of the next available ring entry.
     pub fn set_next_avail(&mut self, next_avail: u16) {
         self.state.set_next_avail(next_avail);
     }
+
+    /// Sets the index for the next descriptor in the used ring.
+    ///
+    /// # Arguments
+    /// * `next_used` - the index of the next used ring entry.
+    pub fn set_next_used(&mut self, next_used: u16) {
+        self.state.set_next_used(next_used);
+    }
 }
 
 impl<M: GuestAddressSpace> Queue<M, QueueState> {
@@ -257,7 +278,7 @@ mod tests {
     use super::*;
     use crate::defs::{
         DEFAULT_AVAIL_RING_ADDR, DEFAULT_DESC_TABLE_ADDR, DEFAULT_USED_RING_ADDR,
-        VIRTQ_DESC_F_NEXT, VIRTQ_USED_F_NO_NOTIFY,
+        VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE, VIRTQ_USED_F_NO_NOTIFY,
     };
     use crate::mock::MockSplitQueue;
     use crate::Descriptor;
@@ -348,6 +369,7 @@ mod tests {
         let vq = MockSplitQueue::new(m, 16);
         let mut q = vq.create_queue(m);
 
+        assert_eq!(q.used_idx(Ordering::Acquire).unwrap(), Wrapping(0));
         assert_eq!(u16::from_le(vq.used().idx().load()), 0);
 
         // index too large
@@ -357,6 +379,7 @@ mod tests {
         // should be ok
         q.add_used(1, 0x1000).unwrap();
         assert_eq!(q.state.next_used, Wrapping(1));
+        assert_eq!(q.used_idx(Ordering::Acquire).unwrap(), Wrapping(1));
         assert_eq!(u16::from_le(vq.used().idx().load()), 1);
 
         let x = vq.used().ring().ref_at(0).load();
@@ -377,7 +400,7 @@ mod tests {
         // Same for `event_idx_enabled`, `next_avail` `next_used` and `signalled_used`.
         q.set_event_idx(true);
         q.set_next_avail(2);
-        q.add_used(1, 200).unwrap();
+        q.set_next_used(4);
         q.state.signalled_used = Some(Wrapping(15));
         assert_eq!(q.state.size, 8);
         // `create_queue` also marks the queue as ready.
@@ -510,7 +533,7 @@ mod tests {
         for i in 0..13 {
             let flags = match i {
                 1 | 4 | 6 | 8 | 12 => 0,
-                _ => VIRTQ_DESC_F_NEXT,
+                _ => VIRTQ_DESC_F_NEXT | VIRTQ_DESC_F_WRITE,
             };
 
             let desc = Descriptor::new((0x1000 * (i + 1)) as u64, 0x1000, flags, i + 1);
@@ -533,10 +556,13 @@ mod tests {
             i += 1;
             q.disable_notification().unwrap();
 
-            while let Some(_chain) = q.iter().unwrap().next() {
-                // Here the device would consume entries from the available ring, add an entry in
-                // the used ring and optionally notify the driver. For the purpose of this test, we
-                // don't need to do anything with the chain, only consume it.
+            while let Some(chain) = q.iter().unwrap().next() {
+                // Process the descriptor chain, and then add entries to the
+                // used ring.
+                let head_index = chain.head_index();
+                let mut desc_len = 0;
+                chain.for_each(|d| desc_len += d.len());
+                q.add_used(head_index, desc_len).unwrap();
             }
             if !q.enable_notification().unwrap() {
                 break;
@@ -547,6 +573,7 @@ mod tests {
         assert_eq!(i, 1);
         // The next chain that can be consumed should have index 2.
         assert_eq!(q.next_avail(), 2);
+        assert_eq!(q.next_used(), 2);
         // Let the device know it can consume one more chain.
         vq.avail().idx().store(u16::to_le(3));
         i = 0;
@@ -555,8 +582,19 @@ mod tests {
             i += 1;
             q.disable_notification().unwrap();
 
-            while let Some(_chain) = q.iter().unwrap().next() {
-                // In a real use case, we would do something with the chain here.
+            while let Some(chain) = q.iter().unwrap().next() {
+                // Process the descriptor chain, and then add entries to the
+                // used ring.
+                let head_index = chain.head_index();
+                let mut desc_len = 0;
+                chain.for_each(|d| {
+                    if d.flags() & VIRTQ_DESC_F_WRITE == VIRTQ_DESC_F_WRITE {
+                        desc_len += d.len();
+                    }
+                });
+                if desc_len > 0 {
+                    q.add_used(head_index, desc_len).unwrap();
+                }
             }
 
             // For the simplicity of the test we are updating here the `idx` value of the available
@@ -571,21 +609,28 @@ mod tests {
         assert_eq!(i, 2);
         // The next chain that can be consumed should have index 4.
         assert_eq!(q.next_avail(), 4);
+        assert_eq!(q.next_used(), 4);
 
         // Set an `idx` that is bigger than the number of entries added in the ring.
         // This is an allowed scenario, but the indexes of the chain will have unexpected values.
         vq.avail().idx().store(u16::to_le(7));
         loop {
             q.disable_notification().unwrap();
 
-            while let Some(_chain) = q.iter().unwrap().next() {
-                // In a real use case, we would do something with the chain here.
+            while let Some(chain) = q.iter().unwrap().next() {
+                // Process the descriptor chain, and then add entries to the
+                // used ring.
+                let head_index = chain.head_index();
+                let mut desc_len = 0;
+                chain.for_each(|d| desc_len += d.len());
+                q.add_used(head_index, desc_len).unwrap();
             }
             if !q.enable_notification().unwrap() {
                 break;
             }
         }
         assert_eq!(q.next_avail(), 7);
+        assert_eq!(q.next_used(), 7);
     }
 
     #[test]
@@ -619,14 +664,18 @@ mod tests {
         vq.avail().idx().store(u16::to_le(3));
         // No descriptor chains are consumed at this point.
         assert_eq!(q.next_avail(), 0);
+        assert_eq!(q.next_used(), 0);
 
         loop {
             q.disable_notification().unwrap();
 
-            while let Some(_chain) = q.iter().unwrap().next() {
-                // Here the device would consume entries from the available ring, add an entry in
-                // the used ring and optionally notify the driver. For the purpose of this test, we
-                // don't need to do anything with the chain, only consume it.
+            while let Some(chain) = q.iter().unwrap().next() {
+                // Process the descriptor chain, and then add entries to the
+                // used ring.
+                let head_index = chain.head_index();
+                let mut desc_len = 0;
+                chain.for_each(|d| desc_len += d.len());
+                q.add_used(head_index, desc_len).unwrap();
             }
             if !q.enable_notification().unwrap() {
                 break;
@@ -635,6 +684,8 @@ mod tests {
         // The next chain that can be consumed should have index 3.
         assert_eq!(q.next_avail(), 3);
         assert_eq!(q.avail_idx(Ordering::Acquire).unwrap(), Wrapping(3));
+        assert_eq!(q.next_used(), 3);
+        assert_eq!(q.used_idx(Ordering::Acquire).unwrap(), Wrapping(3));
         assert!(q.lock().ready());
 
         // Decrement `idx` which should be forbidden. We don't enforce this thing, but we should

crates/virtio-queue/src/queue_guard.rs

@@ -138,6 +138,11 @@ where
         self.state.avail_idx(self.mem.deref(), order)
     }
 
+    /// Read the `idx` field from the used ring.
+    pub fn used_idx(&self, order: Ordering) -> Result<Wrapping<u16>, Error> {
+        self.state.used_idx(self.mem.deref(), order)
+    }
+
     /// Put a used descriptor head into the used ring.
     pub fn add_used(&mut self, head_index: u16, len: u32) -> Result<(), Error> {
         self.state.add_used(self.mem.deref(), head_index, len)
@@ -172,11 +177,21 @@ where
         self.state.next_avail()
     }
 
+    /// Return the index of the next entry in the used ring.
+    pub fn next_used(&self) -> u16 {
+        self.state.next_used()
+    }
+
     /// Set the index of the next entry in the available ring.
     pub fn set_next_avail(&mut self, next_avail: u16) {
         self.state.set_next_avail(next_avail);
     }
 
+    /// Set the index of the next entry in the used ring.
+    pub fn set_next_used(&mut self, next_used: u16) {
+        self.state.set_next_used(next_used);
+    }
+
     /// Get a consuming iterator over all available descriptor chain heads offered by the driver.
     pub fn iter(&mut self) -> Result<AvailIter<'_, M>, Error> {
         self.state.deref_mut().iter(self.mem.clone())
@@ -223,14 +238,18 @@ mod tests {
         vq.avail().idx().store(3);
         // No descriptor chains are consumed at this point.
         assert_eq!(g.next_avail(), 0);
+        assert_eq!(g.next_used(), 0);
 
         loop {
             g.disable_notification().unwrap();
 
-            while let Some(_chain) = g.iter().unwrap().next() {
-                // Here the device would consume entries from the available ring, add an entry in
-                // the used ring and optionally notify the driver. For the purpose of this test, we
-                // don't need to do anything with the chain, only consume it.
+            while let Some(chain) = g.iter().unwrap().next() {
+                // Process the descriptor chain, and then add entries to the
+                // used ring.
+                let head_index = chain.head_index();
+                let mut desc_len = 0;
+                chain.for_each(|d| desc_len += d.len());
+                g.add_used(head_index, desc_len).unwrap();
             }
             if !g.enable_notification().unwrap() {
                 break;
@@ -239,6 +258,8 @@ mod tests {
         // The next chain that can be consumed should have index 3.
         assert_eq!(g.next_avail(), 3);
         assert_eq!(g.avail_idx(Ordering::Acquire).unwrap(), Wrapping(3));
+        assert_eq!(g.next_used(), 3);
+        assert_eq!(g.used_idx(Ordering::Acquire).unwrap(), Wrapping(3));
         assert!(g.ready());
 
         // Decrement `idx` which should be forbidden. We don't enforce this thing, but we should

crates/virtio-queue/src/state.rs

@@ -311,6 +311,17 @@ impl QueueStateT for QueueState {
             .map_err(Error::GuestMemory)
     }
 
+    fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error> {
+        let addr = self
+            .used_ring
+            .checked_add(2)
+            .ok_or(Error::AddressOverflow)?;
+
+        mem.load(addr, order)
+            .map(Wrapping)
+            .map_err(Error::GuestMemory)
+    }
+
     fn add_used<M: GuestMemory>(
         &mut self,
         mem: &M,
@@ -415,7 +426,15 @@ impl QueueStateT for QueueState {
         self.next_avail.0
     }
 
+    fn next_used(&self) -> u16 {
+        self.next_used.0
+    }
+
     fn set_next_avail(&mut self, next_avail: u16) {
         self.next_avail = Wrapping(next_avail);
     }
+
+    fn set_next_used(&mut self, next_used: u16) {
+        self.next_used = Wrapping(next_used);
+    }
 }

crates/virtio-queue/src/state_sync.rs

@@ -107,6 +107,10 @@ impl QueueStateT for QueueStateSync {
         self.lock_state().avail_idx(mem, order)
     }
 
+    fn used_idx<M: GuestMemory>(&self, mem: &M, order: Ordering) -> Result<Wrapping<u16>, Error> {
+        self.lock_state().used_idx(mem, order)
+    }
+
     fn add_used<M: GuestMemory>(
         &mut self,
         mem: &M,
@@ -132,9 +136,17 @@ impl QueueStateT for QueueStateSync {
         self.lock_state().next_avail()
     }
 
+    fn next_used(&self) -> u16 {
+        self.lock_state().next_used()
+    }
+
     fn set_next_avail(&mut self, next_avail: u16) {
         self.lock_state().set_next_avail(next_avail);
     }
+
+    fn set_next_used(&mut self, next_used: u16) {
+        self.lock_state().set_next_used(next_used);
+    }
 }
 
 #[cfg(test)]
@@ -202,20 +214,30 @@ mod tests {
         assert_eq!(q.max_size(), 0x100);
         q.set_next_avail(5);
         assert_eq!(q.next_avail(), 5);
+        q.set_next_used(3);
+        assert_eq!(q.next_used(), 3);
         assert_eq!(
             q.avail_idx(m.memory(), Ordering::Acquire).unwrap(),
             Wrapping(0)
         );
+        assert_eq!(
+            q.used_idx(m.memory(), Ordering::Acquire).unwrap(),
+            Wrapping(0)
+        );
 
-        assert_eq!(q.lock_state().next_used, Wrapping(0));
+        assert_eq!(q.next_used(), 3);
 
         // index too large
         assert!(q.add_used(m.memory(), 0x200, 0x1000).is_err());
-        assert_eq!(q.lock_state().next_used, Wrapping(0));
+        assert_eq!(q.next_used(), 3);
 
         // should be ok
         q.add_used(m.memory(), 1, 0x1000).unwrap();
-        assert_eq!(q.lock_state().next_used, Wrapping(1));
+        assert_eq!(q.next_used(), 4);
+        assert_eq!(
+            q.used_idx(m.memory(), Ordering::Acquire).unwrap(),
+            Wrapping(4)
+        );
     }
 
     #[test]
@@ -228,11 +250,11 @@ mod tests {
         q.set_used_ring_address(Some(0x3000), None);
         q.set_event_idx(true);
         q.set_next_avail(2);
+        q.set_next_used(2);
         q.set_size(0x8);
         q.set_ready(true);
         assert!(q.is_valid(m.memory()));
 
-        q.add_used(m.memory(), 1, 0x100).unwrap();
         q.needs_notification(m.memory()).unwrap();
 
         assert_eq!(q.lock_state().size, 0x8);