diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index c183b320..397c04c6 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 90.6, + "coverage_score": 91.0, "exclude_path": "", "crate_features": "virtio-blk/backend-stdio" } diff --git a/crates/virtio-device/src/lib.rs b/crates/virtio-device/src/lib.rs index bab27416..363129a7 100644 --- a/crates/virtio-device/src/lib.rs +++ b/crates/virtio-device/src/lib.rs @@ -284,7 +284,7 @@ mod tests { assert_eq!(d.cfg.device_features & (1 << VIRTIO_F_RING_EVENT_IDX), 0); for q in d.cfg.queues.iter() { - assert_eq!(q.event_idx_enabled, false); + assert_eq!(q.state.event_idx_enabled, false); } // Revert status. @@ -302,7 +302,7 @@ mod tests { assert_eq!(d.cfg.device_status, status); for q in d.cfg.queues.iter() { - assert_eq!(q.event_idx_enabled, true); + assert_eq!(q.state.event_idx_enabled, true); } } diff --git a/crates/virtio-device/src/mmio.rs b/crates/virtio-device/src/mmio.rs index 2b3afdb5..ee024c84 100644 --- a/crates/virtio-device/src/mmio.rs +++ b/crates/virtio-device/src/mmio.rs @@ -10,7 +10,7 @@ use std::convert::TryInto; use std::sync::atomic::Ordering; use log::warn; -use vm_memory::{GuestAddress, GuestAddressSpace}; +use vm_memory::GuestAddressSpace; use crate::{status, WithDriverSelect}; use virtio_queue::Queue; @@ -51,16 +51,6 @@ where } } -// Helper function that rewrites the most significant 4 bytes of the provided `GuestAddress`. -fn set_high(v: &mut GuestAddress, hi: u32) { - *v = (*v & 0xffff_ffff) | (u64::from(hi) << 32) -} - -// Helper function that rewrites the least significant 4 bytes of the provided `GuestAddress`. -fn set_low(v: &mut GuestAddress, lo: u32) { - *v = (*v & !0xffff_ffff) | u64::from(lo) -} - /// A common interface for Virtio devices that use the MMIO transport, which also provides a /// default implementation of read and write operations from/to the device registers and /// configuration space. @@ -99,7 +89,7 @@ pub trait VirtioMmioDevice: WithDriverSelect { .into(), 0x44 => self .selected_queue() - .map(|q| q.ready) + .map(|q| q.ready()) .unwrap_or(false) .into(), 0x60 => self.interrupt_status().load(Ordering::SeqCst).into(), @@ -159,8 +149,8 @@ pub trait VirtioMmioDevice: WithDriverSelect { // data type specified by the virtio standard (we simply use `as` conversion // for now). 0x30 => self.set_queue_select(v as u16), - 0x38 => update_queue_field(self, |q| q.size = v as u16), - 0x44 => update_queue_field(self, |q| q.ready = v == 1), + 0x38 => update_queue_field(self, |q| q.set_size(v as u16)), + 0x44 => update_queue_field(self, |q| q.set_ready(v == 1)), 0x50 => self.queue_notify(v), 0x64 => { if self.check_device_status(status::DRIVER_OK, 0) { @@ -169,12 +159,12 @@ pub trait VirtioMmioDevice: WithDriverSelect { } } 0x70 => self.ack_device_status(v as u8), - 0x80 => update_queue_field(self, |q| set_low(&mut q.desc_table, v)), - 0x84 => update_queue_field(self, |q| set_high(&mut q.desc_table, v)), - 0x90 => update_queue_field(self, |q| set_low(&mut q.avail_ring, v)), - 0x94 => update_queue_field(self, |q| set_high(&mut q.avail_ring, v)), - 0xa0 => update_queue_field(self, |q| set_low(&mut q.used_ring, v)), - 0xa4 => update_queue_field(self, |q| set_high(&mut q.used_ring, v)), + 0x80 => update_queue_field(self, |q| q.set_desc_table_address(Some(v), None)), + 0x84 => update_queue_field(self, |q| q.set_desc_table_address(None, Some(v))), + 0x90 => update_queue_field(self, |q| q.set_avail_ring_address(Some(v), None)), + 0x94 => update_queue_field(self, |q| q.set_avail_ring_address(None, Some(v))), + 0xa0 => update_queue_field(self, |q| q.set_used_ring_address(Some(v), None)), + 0xa4 => update_queue_field(self, |q| q.set_used_ring_address(None, Some(v))), _ => { warn!("unknown virtio mmio register write: 0x{:x}", offset); } @@ -258,16 +248,16 @@ mod tests { // The max size for the queue in `Dummy` is 256. assert_eq!(mmio_read(&d, 0x34), 256); - assert_eq!(d.cfg.queues[0].size, 256); + assert_eq!(d.cfg.queues[0].state.size, 256); d.write(0x38, &32u32.to_le_bytes()); // Updating the queue field has no effect due to invalid device status. - assert_eq!(d.cfg.queues[0].size, 256); + assert_eq!(d.cfg.queues[0].state.size, 256); d.cfg.device_status |= status::FEATURES_OK; // Let's try the update again. d.write(0x38, &32u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].size, 32); + assert_eq!(d.cfg.queues[0].state.size, 32); // The queue in `Dummy` is not ready yet. assert_eq!(mmio_read(&d, 0x44), 0); @@ -281,23 +271,23 @@ mod tests { d.write(0x50, &2u32.to_le_bytes()); assert_eq!(d.last_queue_notify, 2); - assert_eq!(d.cfg.queues[0].desc_table.0, 0); + assert_eq!(d.cfg.queues[0].state.desc_table.0, 0); d.write(0x80, &1u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].desc_table.0, 1); + assert_eq!(d.cfg.queues[0].state.desc_table.0, 1); d.write(0x84, &2u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].desc_table.0, (2 << 32) + 1); + assert_eq!(d.cfg.queues[0].state.desc_table.0, (2 << 32) + 1); - assert_eq!(d.cfg.queues[0].avail_ring.0, 0); + assert_eq!(d.cfg.queues[0].state.avail_ring.0, 0); d.write(0x90, &1u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].avail_ring.0, 1); + assert_eq!(d.cfg.queues[0].state.avail_ring.0, 1); d.write(0x94, &2u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].avail_ring.0, (2 << 32) + 1); + assert_eq!(d.cfg.queues[0].state.avail_ring.0, (2 << 32) + 1); - assert_eq!(d.cfg.queues[0].used_ring.0, 0); + assert_eq!(d.cfg.queues[0].state.used_ring.0, 0); d.write(0xa0, &1u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].used_ring.0, 1); + assert_eq!(d.cfg.queues[0].state.used_ring.0, 1); d.write(0xa4, &2u32.to_le_bytes()); - assert_eq!(d.cfg.queues[0].used_ring.0, (2 << 32) + 1); + assert_eq!(d.cfg.queues[0].state.used_ring.0, (2 << 32) + 1); // Let's select a non-existent queue. d.write(0x30, &1u32.to_le_bytes()); diff --git a/crates/virtio-queue/src/lib.rs b/crates/virtio-queue/src/lib.rs index f3e92c9d..880af89d 100644 --- a/crates/virtio-queue/src/lib.rs +++ b/crates/virtio-queue/src/lib.rs @@ -6,11 +6,11 @@ // // Copyright © 2019 Intel Corporation // -// Copyright (C) 2020 Alibaba Cloud. All rights reserved. +// Copyright (C) 2020-2021 Alibaba Cloud. All rights reserved. // // SPDX-License-Identifier: Apache-2.0 AND BSD-3-Clause -//! A crate that exposes the virtio queue API. +//! Virtio queue API for backend device drivers to access virtio queues. #![deny(missing_docs)] @@ -21,21 +21,23 @@ pub mod mock; use std::cmp::min; use std::fmt::{self, Debug, Display}; +use std::marker::PhantomData; use std::mem::size_of; use std::num::Wrapping; +use std::ops::{Deref, DerefMut}; use std::sync::atomic::{fence, Ordering}; +use std::sync::{Arc, Mutex, MutexGuard}; -use defs::{ - VIRTQ_AVAIL_ELEMENT_SIZE, VIRTQ_AVAIL_RING_HEADER_SIZE, VIRTQ_AVAIL_RING_META_SIZE, - VIRTQ_DESCRIPTOR_SIZE, VIRTQ_DESC_F_INDIRECT, VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE, - VIRTQ_USED_ELEMENT_SIZE, VIRTQ_USED_F_NO_NOTIFY, VIRTQ_USED_RING_META_SIZE, -}; - +use log::error; use vm_memory::{ Address, ByteValued, Bytes, GuestAddress, GuestAddressSpace, GuestMemory, GuestMemoryError, }; -use log::error; +use self::defs::{ + VIRTQ_AVAIL_ELEMENT_SIZE, VIRTQ_AVAIL_RING_HEADER_SIZE, VIRTQ_AVAIL_RING_META_SIZE, + VIRTQ_DESCRIPTOR_SIZE, VIRTQ_DESC_F_INDIRECT, VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE, + VIRTQ_USED_ELEMENT_SIZE, VIRTQ_USED_F_NO_NOTIFY, VIRTQ_USED_RING_META_SIZE, +}; /// Virtio Queue related errors. #[derive(Debug)] @@ -68,7 +70,7 @@ impl Display for Error { impl std::error::Error for Error {} -/// A virtio descriptor constraints with C representation +/// A virtio descriptor constraints with C representation. #[repr(C)] #[derive(Default, Clone, Copy, Debug)] pub struct Descriptor { @@ -253,6 +255,8 @@ impl Iterator for DescriptorChain { .desc_table .unchecked_add(self.next_index as u64 * size_of::() as u64); + // The guest device driver should not touch the descriptor once submitted, so it's safe + // to use read_obj() here. let desc = self.mem.read_obj::(desc_addr).ok()?; if desc.is_indirect() { @@ -327,6 +331,19 @@ pub struct AvailIter<'b, M: GuestAddressSpace> { next_avail: &'b mut Wrapping, } +impl<'b, M: GuestAddressSpace> AvailIter<'b, M> { + /// Goes back one position in the available descriptor chain offered by the driver. + /// + /// Rust does not support bidirectional iterators. This is the only way to revert the effect + /// of an iterator increment on the queue. + /// + /// Note: this method assumes there's only one thread manipulating the queue, so it should only + /// be invoked in single-threaded context. + pub fn go_to_previous_position(&mut self) { + *self.next_avail -= Wrapping(1); + } +} + impl<'b, M: GuestAddressSpace> Iterator for AvailIter<'b, M> { type Item = DescriptorChain; @@ -349,7 +366,7 @@ impl<'b, M: GuestAddressSpace> Iterator for AvailIter<'b, M> { let addr = self.avail_ring.unchecked_add(offset); let head_index: u16 = self .mem - .read_obj(addr) + .load(addr, Ordering::Acquire) .map_err(|_| error!("Failed to read from memory {:x}", addr.raw_value())) .ok()?; @@ -384,22 +401,143 @@ impl VirtqUsedElem { unsafe impl ByteValued for VirtqUsedElem {} -#[derive(Clone, Debug)] -/// A virtio queue's parameters. -pub struct Queue { - mem: M, +/// Struct to hold an exclusive reference to the underlying `QueueState` object. +pub enum QueueStateGuard<'a, M: GuestAddressSpace> { + /// A reference to a `QueueState` object. + StateObject(&'a mut QueueState), + /// A `MutexGuard` for a `QueueState` object. + MutexGuard(MutexGuard<'a, QueueState>), +} + +impl<'a, M: GuestAddressSpace> Deref for QueueStateGuard<'a, M> { + type Target = QueueState; + + fn deref(&self) -> &Self::Target { + match self { + QueueStateGuard::StateObject(v) => v, + QueueStateGuard::MutexGuard(v) => v.deref(), + } + } +} + +impl<'a, M: GuestAddressSpace> DerefMut for QueueStateGuard<'a, M> { + fn deref_mut(&mut self) -> &mut Self::Target { + match self { + QueueStateGuard::StateObject(v) => v, + QueueStateGuard::MutexGuard(v) => v.deref_mut(), + } + } +} + +/// Trait to access and manipulate a virtio queue. +/// +/// To optimize for performance, different implementations of the `QueueStateT` trait may be +/// provided for single-threaded context and multi-threaded context. +pub trait QueueStateT { + /// Construct an empty virtio queue state object with the given `max_size`. + fn new(max_size: u16) -> Self; + + /// Check whether the queue configuration is valid. + fn is_valid(&self, mem: &M::T) -> bool; + + /// Reset the queue to the initial state. + fn reset(&mut self); + + /// Get an exclusive reference to the underlying `QueueState` object. + /// + /// Logically this method will acquire the underlying lock protecting the `QueueState` Object. + /// The lock will be released when the returned object gets dropped. + fn lock(&mut self) -> QueueStateGuard<'_, M>; + + /// Get the maximum size of the virtio queue. + fn max_size(&self) -> u16; + + /// Return the actual size of the queue. + /// + /// The virtio driver may configure queue size smaller than the value reported by `max_size()`. + fn actual_size(&self) -> u16; + + /// Configure the queue size for the virtio queue. + /// + /// The `size` should power of two and less than or equal to value reported by `max_size()`, + /// otherwise it will panic. + fn set_size(&mut self, size: u16); + + /// Check whether the queue is ready to be processed. + fn ready(&self) -> bool; + + /// Configure the queue to ready for processing. + fn set_ready(&mut self, ready: bool); + + /// Set descriptor table address for the queue. + /// + /// The descriptor table address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + fn set_desc_table_address(&mut self, low: Option, high: Option); + + /// Set available ring address for the queue. + /// + /// The available ring address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + fn set_avail_ring_address(&mut self, low: Option, high: Option); + + /// Set used ring address for the queue. + /// + /// The used ring address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + fn set_used_ring_address(&mut self, low: Option, high: Option); + + /// Enable/disable the VIRTIO_F_RING_EVENT_IDX feature for interrupt coalescing. + fn set_event_idx(&mut self, enabled: bool); + + /// Read the `idx` field from the available ring. + fn avail_idx(&self, mem: &M::T, order: Ordering) -> Result, Error>; + + /// Put a used descriptor head into the used ring. + fn add_used(&mut self, mem: &M::T, head_index: u16, len: u32) -> Result<(), Error>; + + /// Enable notification events from the guest driver. + /// + /// Return true if one or more descriptors can be consumed from the available ring after + /// notifications were enabled (and thus it's possible there will be no corresponding + /// notification). + fn enable_notification(&mut self, mem: &M::T) -> Result; + + /// Disable notification events from the guest driver. + fn disable_notification(&mut self, mem: &M::T) -> Result<(), Error>; + + /// Check whether a notification to the guest is needed. + /// + /// Please note this method has side effects: once it returns `true`, it considers the + /// driver will actually be notified, remember the associated index in the used ring, and + /// won't return `true` again until the driver updates `used_event` and/or the notification + /// conditions hold once more. + fn needs_notification(&mut self, mem: &M::T) -> Result; + + /// Return the index for the next descriptor in the available ring. + fn next_avail(&self) -> u16; + /// Set the index for the next descriptor in the available ring. + fn set_next_avail(&mut self, next_avail: u16); +} + +/// Struct to maintain information and manipulate state of a virtio queue. +#[derive(Clone, Debug)] +pub struct QueueState { /// The maximal size in elements offered by the device - max_size: u16, + pub max_size: u16, - next_avail: Wrapping, - next_used: Wrapping, + /// Tail position of the available ring. + pub next_avail: Wrapping, + + /// Head position of the used ring. + pub next_used: Wrapping, /// VIRTIO_F_RING_EVENT_IDX negotiated pub event_idx_enabled: bool, /// The last used value when using EVENT_IDX - signalled_used: Option>, + pub signalled_used: Option>, /// The queue size in elements the driver selected pub size: u16, @@ -415,13 +553,87 @@ pub struct Queue { /// Guest physical address of the used ring pub used_ring: GuestAddress, + + phantom: PhantomData, } -impl Queue { - /// Constructs an empty virtio queue with the given `max_size`. - pub fn new(mem: M, max_size: u16) -> Queue { - Queue { - mem, +impl QueueState { + /// Get a consuming iterator over all available descriptor chain heads offered by the driver. + pub fn iter(&mut self, mem: M::T) -> Result, Error> { + self.avail_idx(&mem, Ordering::Acquire) + .map(move |idx| AvailIter { + mem, + desc_table: self.desc_table, + avail_ring: self.avail_ring, + last_index: idx, + queue_size: self.actual_size(), + next_avail: &mut self.next_avail, + }) + } + + // Helper method that writes `val` to the `avail_event` field of the used ring, using + // the provided ordering. + fn set_avail_event(&self, mem: &M::T, val: u16, order: Ordering) -> Result<(), Error> { + let offset = (4 + self.actual_size() * 8) as u64; + let addr = self.used_ring.unchecked_add(offset); + + mem.store(val, addr, order).map_err(Error::GuestMemory) + } + + // Set the value of the `flags` field of the used ring, applying the specified ordering. + fn set_used_flags(&mut self, mem: &M::T, val: u16, order: Ordering) -> Result<(), Error> { + mem.store(val, self.used_ring, order) + .map_err(Error::GuestMemory) + } + + // Write the appropriate values to enable or disable notifications from the driver. + // + // Every access in this method uses `Relaxed` ordering because a fence is added by the caller + // when appropriate. + fn set_notification(&mut self, mem: &M::T, enable: bool) -> Result<(), Error> { + if enable { + if self.event_idx_enabled { + // We call `set_avail_event` using the `next_avail` value, instead of reading + // and using the current `avail_idx` to avoid missing notifications. More + // details in `enable_notification`. + self.set_avail_event(mem, self.next_avail.0, Ordering::Relaxed) + } else { + self.set_used_flags(mem, 0, Ordering::Relaxed) + } + } else if !self.event_idx_enabled { + self.set_used_flags(mem, VIRTQ_USED_F_NO_NOTIFY, Ordering::Relaxed) + } else { + // Notifications are effectively disabled by default after triggering once when + // `VIRTIO_F_EVENT_IDX` is negotiated, so we don't do anything in that case. + Ok(()) + } + } + + /// Return the value present in the used_event field of the avail ring. + /// + /// If the VIRTIO_F_EVENT_IDX feature bit is not negotiated, the flags field in the available + /// ring offers a crude mechanism for the driver to inform the device that it doesn’t want + /// interrupts when buffers are used. Otherwise virtq_avail.used_event is a more performant + /// alternative where the driver specifies how far the device can progress before interrupting. + /// + /// Neither of these interrupt suppression methods are reliable, as they are not synchronized + /// with the device, but they serve as useful optimizations. So we only ensure access to the + /// virtq_avail.used_event is atomic, but do not need to synchronize with other memory accesses. + fn used_event(&self, mem: &M::T, order: Ordering) -> Result, Error> { + // Safe because we have validated the queue and access guest memory through GuestMemory interfaces. + let used_event_addr = self + .avail_ring + .unchecked_add((4 + self.actual_size() * 2) as u64); + + mem.load(used_event_addr, order) + .map(Wrapping) + .map_err(Error::GuestMemory) + } +} + +impl QueueStateT for QueueState { + fn new(max_size: u16) -> Self { + QueueState { max_size, size: max_size, ready: false, @@ -432,42 +644,11 @@ impl Queue { next_used: Wrapping(0), event_idx_enabled: false, signalled_used: None, + phantom: PhantomData, } } - /// Gets the virtio queue maximum size. - pub fn max_size(&self) -> u16 { - self.max_size - } - - /// 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) - } - - /// Reset the queue to a state that is acceptable for a device reset - pub fn reset(&mut self) { - self.ready = false; - self.size = self.max_size; - self.desc_table = GuestAddress(0); - self.avail_ring = GuestAddress(0); - self.used_ring = GuestAddress(0); - self.next_avail = Wrapping(0); - self.next_used = Wrapping(0); - self.signalled_used = None; - self.event_idx_enabled = false; - } - - /// Enable/disable the VIRTIO_F_RING_EVENT_IDX feature. - pub fn set_event_idx(&mut self, enabled: bool) { - self.signalled_used = None; - self.event_idx_enabled = enabled; - } - - /// Check if the virtio queue configuration is valid. - pub fn is_valid(&self) -> bool { - let mem = self.mem.memory(); + fn is_valid(&self, mem: &M::T) -> bool { let queue_size = self.actual_size() as u64; let desc_table = self.desc_table; let desc_table_size = size_of::() as u64 * queue_size; @@ -526,30 +707,77 @@ impl Queue { } } - /// Reads the `idx` field from the available ring. - pub fn avail_idx(&self, order: Ordering) -> Result, Error> { + fn reset(&mut self) { + self.ready = false; + self.size = self.max_size; + self.desc_table = GuestAddress(0); + self.avail_ring = GuestAddress(0); + self.used_ring = GuestAddress(0); + self.next_avail = Wrapping(0); + self.next_used = Wrapping(0); + self.signalled_used = None; + self.event_idx_enabled = false; + } + + fn lock(&mut self) -> QueueStateGuard<'_, M> { + QueueStateGuard::StateObject(self) + } + + fn max_size(&self) -> u16 { + self.max_size + } + + fn actual_size(&self) -> u16 { + min(self.size, self.max_size) + } + + fn set_size(&mut self, size: u16) { + self.size = size; + } + + fn ready(&self) -> bool { + self.ready + } + + fn set_ready(&mut self, ready: bool) { + self.ready = ready; + } + + fn set_desc_table_address(&mut self, low: Option, high: Option) { + let low = low.unwrap_or(self.desc_table.0 as u32) as u64; + let high = high.unwrap_or((self.desc_table.0 >> 32) as u32) as u64; + + self.desc_table = GuestAddress((high << 32) | low); + } + + fn set_avail_ring_address(&mut self, low: Option, high: Option) { + let low = low.unwrap_or(self.avail_ring.0 as u32) as u64; + let high = high.unwrap_or((self.avail_ring.0 >> 32) as u32) as u64; + + self.avail_ring = GuestAddress((high << 32) | low); + } + + fn set_used_ring_address(&mut self, low: Option, high: Option) { + let low = low.unwrap_or(self.used_ring.0 as u32) as u64; + let high = high.unwrap_or((self.used_ring.0 >> 32) as u32) as u64; + + self.used_ring = GuestAddress((high << 32) | low); + } + + fn set_event_idx(&mut self, enabled: bool) { + self.signalled_used = None; + self.event_idx_enabled = enabled; + } + + fn avail_idx(&self, mem: &M::T, order: Ordering) -> Result, Error> { let addr = self.avail_ring.unchecked_add(2); - self.mem - .memory() - .load(addr, order) + + mem.load(addr, order) .map(Wrapping) .map_err(Error::GuestMemory) } - /// A consuming iterator over all available descriptor chain heads offered by the driver. - pub fn iter(&mut self) -> Result, Error> { - self.avail_idx(Ordering::Acquire).map(move |idx| AvailIter { - mem: self.mem.memory(), - desc_table: self.desc_table, - avail_ring: self.avail_ring, - last_index: idx, - queue_size: self.actual_size(), - next_avail: &mut self.next_avail, - }) - } - - /// Puts an available descriptor head into the used ring for use by the guest. - pub fn add_used(&mut self, head_index: u16, len: u32) -> Result<(), Error> { + fn add_used(&mut self, mem: &M::T, head_index: u16, len: u32) -> Result<(), Error> { if head_index >= self.actual_size() { error!( "attempted to add out of bounds descriptor to used ring: {}", @@ -558,7 +786,6 @@ impl Queue { return Err(Error::InvalidDescriptorIndex); } - let mem = self.mem.memory(); let next_used_index = u64::from(self.next_used.0 % self.actual_size()); let addr = self.used_ring.unchecked_add(4 + next_used_index * 8); mem.write_obj(VirtqUsedElem::new(head_index, len), addr) @@ -574,51 +801,6 @@ impl Queue { .map_err(Error::GuestMemory) } - // Helper method that writes `val` to the `avail_event` field of the used ring, using - // the provided ordering. - fn set_avail_event(&self, val: u16, order: Ordering) -> Result<(), Error> { - let offset = (4 + self.actual_size() * 8) as u64; - let addr = self.used_ring.unchecked_add(offset); - self.mem - .memory() - .store(val, addr, order) - .map_err(Error::GuestMemory) - } - - // Set the value of the `flags` field of the used ring, applying the specified ordering. - fn set_used_flags(&mut self, val: u16, order: Ordering) -> Result<(), Error> { - self.mem - .memory() - .store(val, self.used_ring, order) - .map_err(Error::GuestMemory) - } - - // Write the appropriate values to enable or disable notifications from the driver. Every - // access in this method uses `Relaxed` ordering because a fence is added by the caller - // when appropriate. - fn set_notification(&mut self, enable: bool) -> Result<(), Error> { - if enable { - if self.event_idx_enabled { - // We call `set_avail_event` using the `next_avail` value, instead of reading - // and using the current `avail_idx` to avoid missing notifications. More - // details in `enable_notification`. - self.set_avail_event(self.next_avail.0, Ordering::Relaxed)?; - } else { - self.set_used_flags(0, Ordering::Relaxed)?; - } - } - // Notifications are effectively disabled by default after triggering once when - // `VIRTIO_F_EVENT_IDX` is negotiated, so we don't do anything in that case. - else if !self.event_idx_enabled { - self.set_used_flags(VIRTQ_USED_F_NO_NOTIFY, Ordering::Relaxed)?; - } - Ok(()) - } - - /// Enable notification events from the guest driver. Returns true if one or more descriptors - /// can be consumed from the available ring after notifications were enabled (and thus it's - /// possible there will be no corresponding notification). - // TODO: Turn this into a doc comment/example. // With the current implementation, a common way of consuming entries from the available ring // while also leveraging notification suppression is to use a loop, for example: @@ -639,9 +821,8 @@ impl Queue { // break; // } // } - #[inline] - pub fn enable_notification(&mut self) -> Result { - self.set_notification(true)?; + fn enable_notification(&mut self, mem: &M::T) -> Result { + self.set_notification(mem, true)?; // Ensures the following read is not reordered before any previous write operation. fence(Ordering::SeqCst); @@ -651,46 +832,15 @@ impl Queue { // entries. There are situations where we intentionally avoid processing everything in the // available ring (which will cause this method to return `true`), but in that case we'll // probably not re-enable notifications as we already know there are pending entries. - self.avail_idx(Ordering::Relaxed) + self.avail_idx(mem, Ordering::Relaxed) .map(|idx| idx != self.next_avail) } - /// Disable notification events from the guest driver. - #[inline] - pub fn disable_notification(&mut self) -> Result<(), Error> { - self.set_notification(false) - } - - /// Return the value present in the used_event field of the avail ring. - /// - /// If the VIRTIO_F_EVENT_IDX feature bit is not negotiated, the flags field in the available - /// ring offers a crude mechanism for the driver to inform the device that it doesn’t want - /// interrupts when buffers are used. Otherwise virtq_avail.used_event is a more performant - /// alternative where the driver specifies how far the device can progress before interrupting. - /// - /// Neither of these interrupt suppression methods are reliable, as they are not synchronized - /// with the device, but they serve as useful optimizations. So we only ensure access to the - /// virtq_avail.used_event is atomic, but do not need to synchronize with other memory accesses. - fn used_event(&self, order: Ordering) -> Result, Error> { - // Safe because we have validated the queue and access guest memory through GuestMemory - // interfaces. - let mem = self.mem.memory(); - let used_event_addr = self - .avail_ring - .unchecked_add((4 + self.actual_size() * 2) as u64); - - mem.load(used_event_addr, order) - .map(Wrapping) - .map_err(Error::GuestMemory) + fn disable_notification(&mut self, mem: &M::T) -> Result<(), Error> { + self.set_notification(mem, false) } - /// Check whether a notification to the guest is needed. - /// - /// Please note this method has side effects: once it returns `true`, it considers the - /// driver will actually be notified, remember the associated index in the used ring, and - /// won't return `true` again until the driver updates `used_event` and/or the notification - /// conditions hold once more. - pub fn needs_notification(&mut self) -> Result { + fn needs_notification(&mut self, mem: &M::T) -> Result { let used_idx = self.next_used; // Complete all the writes in add_used() before reading the event. @@ -699,7 +849,7 @@ impl Queue { // The VRING_AVAIL_F_NO_INTERRUPT flag isn't supported yet. if self.event_idx_enabled { if let Some(old_idx) = self.signalled_used.replace(used_idx) { - let used_event = self.used_event(Ordering::Relaxed)?; + let used_event = self.used_event(mem, Ordering::Relaxed)?; // This check looks at `used_idx`, `used_event`, and `old_idx` as if they are on // an axis that wraps around. If `used_idx - used_used - Wrapping(1)` is greater // than or equal to the difference between `used_idx` and `old_idx`, then @@ -714,21 +864,250 @@ impl Queue { Ok(true) } - /// Goes back one position in the available descriptor chain offered by the driver. - /// Rust does not support bidirectional iterators. This is the only way to revert the effect - /// of an iterator increment on the queue. - pub fn go_to_previous_position(&mut self) { - self.next_avail -= Wrapping(1); + fn next_avail(&self) -> u16 { + self.next_avail.0 + } + + fn set_next_avail(&mut self, next_avail: u16) { + self.next_avail = Wrapping(next_avail); + } +} + +/// Struct to maintain information and manipulate state of a virtio queue for multi-threaded +/// context. +#[derive(Clone, Debug)] +pub struct QueueStateSync { + state: Arc>>, +} + +impl QueueStateT for QueueStateSync { + fn new(max_size: u16) -> Self { + QueueStateSync { + state: Arc::new(Mutex::new(QueueState::new(max_size))), + } + } + + fn is_valid(&self, mem: &M::T) -> bool { + self.state.lock().unwrap().is_valid(mem) + } + + fn reset(&mut self) { + self.state.lock().unwrap().reset(); + } + + fn lock(&mut self) -> QueueStateGuard<'_, M> { + QueueStateGuard::MutexGuard(self.state.lock().unwrap()) + } + + fn max_size(&self) -> u16 { + self.state.lock().unwrap().max_size() + } + + fn actual_size(&self) -> u16 { + self.state.lock().unwrap().actual_size() + } + + fn set_size(&mut self, size: u16) { + self.state.lock().unwrap().set_size(size) + } + + fn ready(&self) -> bool { + self.state.lock().unwrap().ready + } + + fn set_ready(&mut self, ready: bool) { + self.state.lock().unwrap().set_ready(ready) + } + + fn set_desc_table_address(&mut self, low: Option, high: Option) { + self.state.lock().unwrap().set_desc_table_address(low, high); + } + + fn set_avail_ring_address(&mut self, low: Option, high: Option) { + self.state.lock().unwrap().set_avail_ring_address(low, high); + } + + fn set_used_ring_address(&mut self, low: Option, high: Option) { + self.state.lock().unwrap().set_used_ring_address(low, high); + } + + fn set_event_idx(&mut self, enabled: bool) { + self.state.lock().unwrap().set_event_idx(enabled); + } + + fn avail_idx(&self, mem: &M::T, order: Ordering) -> Result, Error> { + self.state.lock().unwrap().avail_idx(mem, order) + } + + fn add_used(&mut self, mem: &M::T, head_index: u16, len: u32) -> Result<(), Error> { + self.state.lock().unwrap().add_used(mem, head_index, len) + } + + fn enable_notification(&mut self, mem: &M::T) -> Result { + self.state.lock().unwrap().enable_notification(mem) + } + + fn disable_notification(&mut self, mem: &M::T) -> Result<(), Error> { + self.state.lock().unwrap().disable_notification(mem) + } + + fn needs_notification(&mut self, mem: &M::T) -> Result { + self.state.lock().unwrap().needs_notification(mem) + } + + fn next_avail(&self) -> u16 { + self.state.lock().unwrap().next_avail() + } + + fn set_next_avail(&mut self, next_avail: u16) { + self.state.lock().unwrap().set_next_avail(next_avail); + } +} + +/// A convenient wrapper struct for a virtio queue, with associated GuestMemory object. +#[derive(Clone, Debug)] +pub struct Queue = QueueState> { + /// Guest memory object associated with the queue. + pub mem: M, + /// Virtio queue state. + pub state: S, +} + +impl> Queue { + /// Construct an empty virtio queue with the given `max_size`. + pub fn new(mem: M, max_size: u16) -> Self { + Queue { + mem, + state: S::new(max_size), + } + } + + /// Check whether the queue configuration is valid. + pub fn is_valid(&self) -> bool { + self.state.is_valid(&self.mem.memory()) + } + + /// Reset the queue to the initial state. + pub fn reset(&mut self) { + self.state.reset() + } + + /// Get an exclusive reference to the underlying `QueueState` object. + /// + /// Logically this method will acquire the underlying lock protecting the `QueueState` Object. + /// The lock will be released when the returned object gets dropped. + pub fn lock(&mut self) -> QueueStateGuard<'_, M> { + self.state.lock() + } + + /// Get the maximum size of the virtio queue. + pub fn max_size(&self) -> u16 { + self.state.max_size() + } + + /// Return the actual size of the queue. + /// + /// The virtio driver may configure queue size smaller than the value reported by `max_size()`. + pub fn actual_size(&self) -> u16 { + self.state.actual_size() + } + + /// Configure the queue size for the virtio queue. + /// + /// The `size` should power of two and less than or equal to value reported by `max_size()`, + /// otherwise it will panic. + pub fn set_size(&mut self, size: u16) { + self.state.set_size(size) + } + + /// Check whether the queue is ready to be processed. + pub fn ready(&self) -> bool { + self.state.ready() + } + + /// Configure the queue to ready for processing. + pub fn set_ready(&mut self, ready: bool) { + self.state.set_ready(ready) + } + + /// Set descriptor table address for the queue. + /// + /// The descriptor table address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + pub fn set_desc_table_address(&mut self, low: Option, high: Option) { + self.state.set_desc_table_address(low, high); + } + + /// Set available ring address for the queue. + /// + /// The available ring address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + pub fn set_avail_ring_address(&mut self, low: Option, high: Option) { + self.state.set_avail_ring_address(low, high); + } + + /// Set used ring address for the queue. + /// + /// The used ring address is 64-bit, the corresponding part will be updated if 'low' + /// and/or `high` is valid. + pub fn set_used_ring_address(&mut self, low: Option, high: Option) { + self.state.set_used_ring_address(low, high) + } + + /// Enable/disable the VIRTIO_F_RING_EVENT_IDX feature for interrupt coalescing. + pub fn set_event_idx(&mut self, enabled: bool) { + self.state.set_event_idx(enabled) + } + + /// Read the `idx` field from the available ring. + pub fn avail_idx(&self, order: Ordering) -> Result, Error> { + self.state.avail_idx(&self.mem.memory(), 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.memory(), head_index, len) + } + + /// Enable notification events from the guest driver. + /// + /// Return true if one or more descriptors can be consumed from the available ring after + /// notifications were enabled (and thus it's possible there will be no corresponding + /// notification). + pub fn enable_notification(&mut self) -> Result { + self.state.enable_notification(&self.mem.memory()) + } + + /// Disable notification events from the guest driver. + pub fn disable_notification(&mut self) -> Result<(), Error> { + self.state.disable_notification(&self.mem.memory()) + } + + /// Check whether a notification to the guest is needed. + /// + /// Please note this method has side effects: once it returns `true`, it considers the + /// driver will actually be notified, remember the associated index in the used ring, and + /// won't return `true` again until the driver updates `used_event` and/or the notification + /// conditions hold once more. + pub fn needs_notification(&mut self) -> Result { + self.state.needs_notification(&self.mem.memory()) } - /// Returns the index for the next descriptor in the available ring. + /// Return the index for the next descriptor in the available ring. pub fn next_avail(&self) -> u16 { - self.next_avail.0 + self.state.next_avail() } - /// Sets the index for the next descriptor in the available ring. + /// Set the index for the next descriptor in the available ring. pub fn set_next_avail(&mut self, next_avail: u16) { - self.next_avail = Wrapping(next_avail); + self.state.set_next_avail(next_avail); + } +} + +impl Queue> { + /// A consuming iterator over all available descriptor chain heads offered by the driver. + pub fn iter(&mut self) -> Result, Error> { + self.state.iter(self.mem.memory()) } } @@ -905,53 +1284,53 @@ mod tests { assert!(q.is_valid()); // shouldn't be valid when not marked as ready - q.ready = false; + q.state.ready = false; assert!(!q.is_valid()); - q.ready = true; + q.state.ready = true; // or when size > max_size - q.size = q.max_size << 1; + q.state.size = q.state.max_size << 1; assert!(!q.is_valid()); - q.size = q.max_size; + q.state.size = q.state.max_size; // or when size is 0 - q.size = 0; + q.state.size = 0; assert!(!q.is_valid()); - q.size = q.max_size; + q.state.size = q.state.max_size; // or when size is not a power of 2 - q.size = 11; + q.state.size = 11; assert!(!q.is_valid()); - q.size = q.max_size; + q.state.size = q.state.max_size; // or if the various addresses are off - q.desc_table = GuestAddress(0xffff_ffff); + q.state.desc_table = GuestAddress(0xffff_ffff); assert!(!q.is_valid()); - q.desc_table = GuestAddress(0x1001); + q.state.desc_table = GuestAddress(0x1001); assert!(!q.is_valid()); - q.desc_table = vq.desc_table_addr(); + q.state.desc_table = vq.desc_table_addr(); - q.avail_ring = GuestAddress(0xffff_ffff); + q.state.avail_ring = GuestAddress(0xffff_ffff); assert!(!q.is_valid()); - q.avail_ring = GuestAddress(0x1001); + q.state.avail_ring = GuestAddress(0x1001); assert!(!q.is_valid()); - q.avail_ring = vq.avail_addr(); + q.state.avail_ring = vq.avail_addr(); - q.used_ring = GuestAddress(0xffff_ffff); + q.state.used_ring = GuestAddress(0xffff_ffff); assert!(!q.is_valid()); - q.used_ring = GuestAddress(0x1001); + q.state.used_ring = GuestAddress(0x1001); assert!(!q.is_valid()); - q.used_ring = vq.used_addr(); + q.state.used_ring = vq.used_addr(); { // an invalid queue should return an iterator with no next - q.ready = false; + q.state.ready = false; let mut i = q.iter().unwrap(); assert!(i.next().is_none()); } - q.ready = true; + q.state.ready = true; // now let's create two simple descriptor chains // the chains are (0, 1) and (2, 3, 4) @@ -992,17 +1371,17 @@ mod tests { assert!(c.next().is_none()); assert_eq!(c.head_index(), 2); } - } - // also test go_to_previous_position() works as expected - { - assert!(q.iter().unwrap().next().is_none()); - q.go_to_previous_position(); - let mut c = q.iter().unwrap().next().unwrap(); - c.next().unwrap(); - c.next().unwrap(); - c.next().unwrap(); - assert!(c.next().is_none()); + // also test go_to_previous_position() works as expected + { + assert!(i.next().is_none()); + i.go_to_previous_position(); + let mut c = q.iter().unwrap().next().unwrap(); + c.next().unwrap(); + c.next().unwrap(); + c.next().unwrap(); + assert!(c.next().is_none()); + } } } @@ -1083,7 +1462,7 @@ mod tests { // should be ok q.add_used(1, 0x1000).unwrap(); - assert_eq!(q.next_used, Wrapping(1)); + assert_eq!(q.state.next_used, Wrapping(1)); assert_eq!(vq.used().idx().load(), 1); let x = vq.used().ring().ref_at(0).load(); @@ -1097,11 +1476,11 @@ mod tests { let vq = MockSplitQueue::new(m, 16); let mut q = vq.create_queue(m); - q.size = 8; - q.ready = true; - q.reset(); - assert_eq!(q.size, 16); - assert_eq!(q.ready, false); + q.state.size = 8; + q.state.ready = true; + q.state.reset(); + assert_eq!(q.state.size, 16); + assert_eq!(q.state.ready, false); } #[test] @@ -1115,21 +1494,21 @@ mod tests { // It should always return true when EVENT_IDX isn't enabled. for i in 0..qsize { - q.next_used = Wrapping(i); + q.state.next_used = Wrapping(i); assert_eq!(q.needs_notification().unwrap(), true); } m.write_obj::(4, avail_addr.unchecked_add(4 + qsize as u64 * 2)) .unwrap(); - q.set_event_idx(true); + q.state.set_event_idx(true); // Incrementing up to this value causes an `u16` to wrap back to 0. let wrap = u32::from(u16::MAX) + 1; for i in 0..wrap + 12 { - q.next_used = Wrapping(i as u16); + q.state.next_used = Wrapping(i as u16); // Let's test wrapping around the maximum index value as well. - let expected = i == 5 || i == (5 + wrap) || q.signalled_used.is_none(); + let expected = i == 5 || i == (5 + wrap) || q.state.signalled_used.is_none(); assert_eq!(q.needs_notification().unwrap(), expected); } @@ -1143,9 +1522,9 @@ mod tests { .unwrap(); assert_eq!(q.needs_notification().unwrap(), false); - q.next_used = Wrapping(15); + q.state.next_used = Wrapping(15); assert_eq!(q.needs_notification().unwrap(), false); - q.next_used = Wrapping(0); + q.state.next_used = Wrapping(0); assert_eq!(q.needs_notification().unwrap(), true); assert_eq!(q.needs_notification().unwrap(), false); } @@ -1158,7 +1537,7 @@ mod tests { let mut q = vq.create_queue(m); let used_addr = vq.used_addr(); - assert_eq!(q.event_idx_enabled, false); + assert_eq!(q.state.event_idx_enabled, false); q.enable_notification().unwrap(); let v = m.read_obj::(used_addr).unwrap(); @@ -1177,13 +1556,13 @@ mod tests { m.write_obj::(2, avail_addr.unchecked_add(2)).unwrap(); assert_eq!(q.enable_notification().unwrap(), true); - q.next_avail = Wrapping(2); + q.state.next_avail = Wrapping(2); assert_eq!(q.enable_notification().unwrap(), false); m.write_obj::(8, avail_addr.unchecked_add(2)).unwrap(); assert_eq!(q.enable_notification().unwrap(), true); - q.next_avail = Wrapping(8); + q.state.next_avail = Wrapping(8); assert_eq!(q.enable_notification().unwrap(), false); } } diff --git a/crates/virtio-queue/src/mock.rs b/crates/virtio-queue/src/mock.rs index 4e130142..6320928b 100644 --- a/crates/virtio-queue/src/mock.rs +++ b/crates/virtio-queue/src/mock.rs @@ -12,7 +12,7 @@ use vm_memory::{ }; use crate::defs::{VIRTQ_DESC_F_INDIRECT, VIRTQ_DESC_F_NEXT}; -use crate::{Descriptor, Queue}; +use crate::{Descriptor, Queue, QueueState}; /// Wrapper struct used for accesing a particular address of a GuestMemory area. pub struct Ref<'a, M, T> { @@ -366,14 +366,14 @@ impl<'a, M: GuestMemory> MockSplitQueue<'a, M> { /// Creates a new `Queue`, using the underlying memory regions represented /// by the `MockSplitQueue`. - pub fn create_queue(&self, a: A) -> Queue { - let mut q = Queue::new(a, self.len); - - q.size = self.len; - q.ready = true; - q.desc_table = self.desc_table_addr; - q.avail_ring = self.avail_addr; - q.used_ring = self.used_addr; + pub fn create_queue(&self, a: A) -> Queue> { + let mut q = Queue::>::new(a, self.len); + + q.state.size = self.len; + q.state.ready = true; + q.state.desc_table = self.desc_table_addr; + q.state.avail_ring = self.avail_addr; + q.state.used_ring = self.used_addr; q } }