From fae19287974a54421603a66a68051404849ff391 Mon Sep 17 00:00:00 2001 From: Sebastien Boeuf Date: Tue, 21 Dec 2021 09:49:49 +0100 Subject: [PATCH] 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 --- coverage_config_x86_64.json | 2 +- crates/virtio-queue/src/lib.rs | 9 +++ crates/virtio-queue/src/queue.rs | 89 ++++++++++++++++++++++---- crates/virtio-queue/src/queue_guard.rs | 35 ++++++++-- crates/virtio-queue/src/state.rs | 19 ++++++ crates/virtio-queue/src/state_sync.rs | 30 +++++++-- 6 files changed, 160 insertions(+), 24 deletions(-) diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 661fa44a..cb4e99ae 100644 --- a/coverage_config_x86_64.json +++ b/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" } diff --git a/crates/virtio-queue/src/lib.rs b/crates/virtio-queue/src/lib.rs index 5467c28e..fc35c364 100644 --- a/crates/virtio-queue/src/lib.rs +++ b/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(&self, mem: &M, order: Ordering) -> Result, Error>; + /// Read the `idx` field from the used ring. + fn used_idx(&self, mem: &M, order: Ordering) -> Result, Error>; + /// Put a used descriptor head into the used ring. fn add_used(&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); } diff --git a/crates/virtio-queue/src/queue.rs b/crates/virtio-queue/src/queue.rs index 9e60459c..ac2b77de 100644 --- a/crates/virtio-queue/src/queue.rs +++ b/crates/virtio-queue/src/queue.rs @@ -197,6 +197,14 @@ impl Queue { 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, Error> { + self.state.used_idx(self.mem.memory().deref(), order) + } + /// Put a used descriptor head into the used ring. /// /// # Arguments @@ -236,6 +244,11 @@ impl Queue { 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 @@ -243,6 +256,14 @@ impl Queue { 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 Queue { @@ -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. @@ -533,10 +556,17 @@ 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| { + if d.flags() & VIRTQ_DESC_F_WRITE == VIRTQ_DESC_F_WRITE { + desc_len += d.len(); + } + }); + q.add_used(head_index, desc_len).unwrap(); } if !q.enable_notification().unwrap() { break; @@ -547,6 +577,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 +586,17 @@ 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(); + } + }); + 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,6 +611,7 @@ 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. @@ -578,14 +619,24 @@ mod tests { 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| { + if d.flags() & VIRTQ_DESC_F_WRITE == VIRTQ_DESC_F_WRITE { + 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 +670,22 @@ 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| { + if d.flags() & VIRTQ_DESC_F_WRITE == VIRTQ_DESC_F_WRITE { + desc_len += d.len(); + } + }); + q.add_used(head_index, desc_len).unwrap(); } if !q.enable_notification().unwrap() { break; @@ -635,6 +694,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 diff --git a/crates/virtio-queue/src/queue_guard.rs b/crates/virtio-queue/src/queue_guard.rs index b9fa284c..fad10d2f 100644 --- a/crates/virtio-queue/src/queue_guard.rs +++ b/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, 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, Error> { self.state.deref_mut().iter(self.mem.clone()) @@ -186,7 +201,7 @@ where #[cfg(test)] mod tests { use super::*; - use crate::defs::VIRTQ_DESC_F_NEXT; + use crate::defs::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE}; use crate::mock::MockSplitQueue; use crate::Descriptor; @@ -223,14 +238,22 @@ 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| { + if d.flags() & VIRTQ_DESC_F_WRITE == VIRTQ_DESC_F_WRITE { + desc_len += d.len(); + } + }); + g.add_used(head_index, desc_len).unwrap(); } if !g.enable_notification().unwrap() { break; @@ -239,6 +262,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 diff --git a/crates/virtio-queue/src/state.rs b/crates/virtio-queue/src/state.rs index 3559ce43..dfae9991 100644 --- a/crates/virtio-queue/src/state.rs +++ b/crates/virtio-queue/src/state.rs @@ -311,6 +311,17 @@ impl QueueStateT for QueueState { .map_err(Error::GuestMemory) } + fn used_idx(&self, mem: &M, order: Ordering) -> Result, 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( &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); + } } diff --git a/crates/virtio-queue/src/state_sync.rs b/crates/virtio-queue/src/state_sync.rs index f9277326..4a965c84 100644 --- a/crates/virtio-queue/src/state_sync.rs +++ b/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(&self, mem: &M, order: Ordering) -> Result, Error> { + self.lock_state().used_idx(mem, order) + } + fn add_used( &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);