Skip to content

virtio-queue: Add helpers for accessing queue information #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion coverage_config_x86_64.json
Original file line number Diff line number Diff line change
@@ -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"
}
9 changes: 9 additions & 0 deletions crates/virtio-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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);
}
89 changes: 75 additions & 14 deletions crates/virtio-queue/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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
Expand All @@ -571,21 +611,32 @@ 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| {
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]
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
35 changes: 30 additions & 5 deletions crates/virtio-queue/src/queue_guard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
19 changes: 19 additions & 0 deletions crates/virtio-queue/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Loading