From 7385d61153aff3236d7083d143bbeef9c2eb7326 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Thu, 13 Jul 2023 18:25:46 +0100 Subject: [PATCH 1/3] Add support for VIRTIO_F_EVENT_IDX buffer notification suppression. --- README.md | 2 +- src/device/blk.rs | 4 ++- src/device/console.rs | 20 ++++++++++++--- src/device/gpu.rs | 20 ++++++++++++--- src/device/input.rs | 22 +++++++++++++---- src/device/net.rs | 22 ++++++++++++++--- src/device/socket/vsock.rs | 28 ++++++++++++++++----- src/queue.rs | 50 +++++++++++++++++++++++++++----------- 8 files changed, 129 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 82842b4c..fc87378e 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ VirtIO guest drivers in Rust. For **no_std** environment. | Feature flag | Supported | | | ---------------------------- | --------- | --------------------------------------- | | `VIRTIO_F_INDIRECT_DESC` | ✅ | Indirect descriptors | -| `VIRTIO_F_EVENT_IDX` | ❌ | `avail_event` and `used_event` fields | +| `VIRTIO_F_EVENT_IDX` | ✅ | `avail_event` and `used_event` fields | | `VIRTIO_F_VERSION_1` | TODO | VirtIO version 1 compliance | | `VIRTIO_F_ACCESS_PLATFORM` | ❌ | Limited device access to memory | | `VIRTIO_F_RING_PACKED` | ❌ | Packed virtqueue layout | diff --git a/src/device/blk.rs b/src/device/blk.rs index 43931705..719a037f 100644 --- a/src/device/blk.rs +++ b/src/device/blk.rs @@ -13,7 +13,8 @@ const QUEUE: u16 = 0; const QUEUE_SIZE: u16 = 16; const SUPPORTED_FEATURES: BlkFeature = BlkFeature::RO .union(BlkFeature::FLUSH) - .union(BlkFeature::RING_INDIRECT_DESC); + .union(BlkFeature::RING_INDIRECT_DESC) + .union(BlkFeature::RING_EVENT_IDX); /// Driver for a VirtIO block device. /// @@ -74,6 +75,7 @@ impl VirtIOBlk { &mut transport, QUEUE, negotiated_features.contains(BlkFeature::RING_INDIRECT_DESC), + negotiated_features.contains(BlkFeature::RING_EVENT_IDX), )?; transport.finish_init(); diff --git a/src/device/console.rs b/src/device/console.rs index 38aede46..635ffcd5 100644 --- a/src/device/console.rs +++ b/src/device/console.rs @@ -13,6 +13,7 @@ use log::info; const QUEUE_RECEIVEQ_PORT_0: u16 = 0; const QUEUE_TRANSMITQ_PORT_0: u16 = 1; const QUEUE_SIZE: usize = 2; +const SUPPORTED_FEATURES: Features = Features::RING_EVENT_IDX; /// Driver for a VirtIO console device. /// @@ -65,15 +66,26 @@ pub struct ConsoleInfo { impl VirtIOConsole { /// Creates a new VirtIO console driver. pub fn new(mut transport: T) -> Result { + let mut negotiated_features = Features::empty(); transport.begin_init(|features| { let features = Features::from_bits_truncate(features); info!("Device features {:?}", features); - let supported_features = Features::empty(); - (features & supported_features).bits() + negotiated_features = features & SUPPORTED_FEATURES; + negotiated_features.bits() }); let config_space = transport.config_space::()?; - let receiveq = VirtQueue::new(&mut transport, QUEUE_RECEIVEQ_PORT_0, false)?; - let transmitq = VirtQueue::new(&mut transport, QUEUE_TRANSMITQ_PORT_0, false)?; + let receiveq = VirtQueue::new( + &mut transport, + QUEUE_RECEIVEQ_PORT_0, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; + let transmitq = VirtQueue::new( + &mut transport, + QUEUE_TRANSMITQ_PORT_0, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; // Safe because no alignment or initialisation is required for [u8], the DMA buffer is // dereferenceable, and the lifetime of the reference matches the lifetime of the DMA buffer diff --git a/src/device/gpu.rs b/src/device/gpu.rs index 7e5de9b3..6541c8d4 100644 --- a/src/device/gpu.rs +++ b/src/device/gpu.rs @@ -11,6 +11,7 @@ use log::info; use zerocopy::{AsBytes, FromBytes}; const QUEUE_SIZE: u16 = 2; +const SUPPORTED_FEATURES: Features = Features::RING_EVENT_IDX; /// A virtio based graphics adapter. /// @@ -39,11 +40,12 @@ pub struct VirtIOGpu { impl VirtIOGpu { /// Create a new VirtIO-Gpu driver. pub fn new(mut transport: T) -> Result { + let mut negotiated_features = Features::empty(); transport.begin_init(|features| { let features = Features::from_bits_truncate(features); info!("Device features {:?}", features); - let supported_features = Features::empty(); - (features & supported_features).bits() + negotiated_features = features & SUPPORTED_FEATURES; + negotiated_features.bits() }); // read configuration space @@ -57,8 +59,18 @@ impl VirtIOGpu { ); } - let control_queue = VirtQueue::new(&mut transport, QUEUE_TRANSMIT, false)?; - let cursor_queue = VirtQueue::new(&mut transport, QUEUE_CURSOR, false)?; + let control_queue = VirtQueue::new( + &mut transport, + QUEUE_TRANSMIT, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; + let cursor_queue = VirtQueue::new( + &mut transport, + QUEUE_CURSOR, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; let queue_buf_send = FromBytes::new_box_slice_zeroed(PAGE_SIZE); let queue_buf_recv = FromBytes::new_box_slice_zeroed(PAGE_SIZE); diff --git a/src/device/input.rs b/src/device/input.rs index 0314d4fb..a9d7ec9c 100644 --- a/src/device/input.rs +++ b/src/device/input.rs @@ -28,18 +28,29 @@ impl VirtIOInput { /// Create a new VirtIO-Input driver. pub fn new(mut transport: T) -> Result { let mut event_buf = Box::new([InputEvent::default(); QUEUE_SIZE]); + + let mut negotiated_features = Feature::empty(); transport.begin_init(|features| { let features = Feature::from_bits_truncate(features); info!("Device features: {:?}", features); - // negotiate these flags only - let supported_features = Feature::empty(); - (features & supported_features).bits() + negotiated_features = features & SUPPORTED_FEATURES; + negotiated_features.bits() }); let config = transport.config_space::()?; - let mut event_queue = VirtQueue::new(&mut transport, QUEUE_EVENT, false)?; - let status_queue = VirtQueue::new(&mut transport, QUEUE_STATUS, false)?; + let mut event_queue = VirtQueue::new( + &mut transport, + QUEUE_EVENT, + false, + negotiated_features.contains(Feature::RING_EVENT_IDX), + )?; + let status_queue = VirtQueue::new( + &mut transport, + QUEUE_STATUS, + false, + negotiated_features.contains(Feature::RING_EVENT_IDX), + )?; for (i, event) in event_buf.as_mut().iter_mut().enumerate() { // Safe because the buffer lasts as long as the queue. let token = unsafe { event_queue.add(&[], &mut [event.as_bytes_mut()])? }; @@ -193,6 +204,7 @@ pub struct InputEvent { const QUEUE_EVENT: u16 = 0; const QUEUE_STATUS: u16 = 1; +const SUPPORTED_FEATURES: Feature = Feature::RING_EVENT_IDX; // a parameter that can change const QUEUE_SIZE: usize = 32; diff --git a/src/device/net.rs b/src/device/net.rs index 3d25dd45..b922b374 100644 --- a/src/device/net.rs +++ b/src/device/net.rs @@ -112,11 +112,12 @@ pub struct VirtIONet { impl VirtIONet { /// Create a new VirtIO-Net driver. pub fn new(mut transport: T, buf_len: usize) -> Result { + let mut negotiated_features = Features::empty(); transport.begin_init(|features| { let features = Features::from_bits_truncate(features); info!("Device features {:?}", features); - let supported_features = Features::MAC | Features::STATUS; - (features & supported_features).bits() + negotiated_features = features & SUPPORTED_FEATURES; + negotiated_features.bits() }); // read configuration space let config = transport.config_space::()?; @@ -139,8 +140,18 @@ impl VirtIONet return Err(Error::InvalidParam); } - let send_queue = VirtQueue::new(&mut transport, QUEUE_TRANSMIT, false)?; - let mut recv_queue = VirtQueue::new(&mut transport, QUEUE_RECEIVE, false)?; + let send_queue = VirtQueue::new( + &mut transport, + QUEUE_TRANSMIT, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; + let mut recv_queue = VirtQueue::new( + &mut transport, + QUEUE_RECEIVE, + false, + negotiated_features.contains(Features::RING_EVENT_IDX), + )?; const NONE_BUF: Option = None; let mut rx_buffers = [NONE_BUF; QUEUE_SIZE]; @@ -403,3 +414,6 @@ impl GsoType { const QUEUE_RECEIVE: u16 = 0; const QUEUE_TRANSMIT: u16 = 1; +const SUPPORTED_FEATURES: Features = Features::MAC + .union(Features::STATUS) + .union(Features::RING_EVENT_IDX); diff --git a/src/device/socket/vsock.rs b/src/device/socket/vsock.rs index 4c365e8e..a1ea4304 100644 --- a/src/device/socket/vsock.rs +++ b/src/device/socket/vsock.rs @@ -19,6 +19,7 @@ pub(crate) const TX_QUEUE_IDX: u16 = 1; const EVENT_QUEUE_IDX: u16 = 2; pub(crate) const QUEUE_SIZE: usize = 8; +const SUPPORTED_FEATURES: Feature = Feature::RING_EVENT_IDX; /// The size in bytes of each buffer used in the RX virtqueue. This must be bigger than size_of::(). const RX_BUFFER_SIZE: usize = 512; @@ -241,12 +242,12 @@ impl Drop for VirtIOSocket { impl VirtIOSocket { /// Create a new VirtIO Vsock driver. pub fn new(mut transport: T) -> Result { + let mut negotiated_features = Feature::empty(); transport.begin_init(|features| { let features = Feature::from_bits_truncate(features); debug!("Device features: {:?}", features); - // negotiate these flags only - let supported_features = Feature::empty(); - (features & supported_features).bits() + negotiated_features = features & SUPPORTED_FEATURES; + negotiated_features.bits() }); let config = transport.config_space::()?; @@ -257,9 +258,24 @@ impl VirtIOSocket { }; debug!("guest cid: {guest_cid:?}"); - let mut rx = VirtQueue::new(&mut transport, RX_QUEUE_IDX, false)?; - let tx = VirtQueue::new(&mut transport, TX_QUEUE_IDX, false)?; - let event = VirtQueue::new(&mut transport, EVENT_QUEUE_IDX, false)?; + let mut rx = VirtQueue::new( + &mut transport, + RX_QUEUE_IDX, + false, + negotiated_features.contains(Feature::RING_EVENT_IDX), + )?; + let tx = VirtQueue::new( + &mut transport, + TX_QUEUE_IDX, + false, + negotiated_features.contains(Feature::RING_EVENT_IDX), + )?; + let event = VirtQueue::new( + &mut transport, + EVENT_QUEUE_IDX, + false, + negotiated_features.contains(Feature::RING_EVENT_IDX), + )?; // Allocate and add buffers for the RX queue. let mut rx_queue_buffers = [null_mut(); QUEUE_SIZE]; diff --git a/src/queue.rs b/src/queue.rs index dd13c87f..854a6099 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -52,6 +52,8 @@ pub struct VirtQueue { /// Our trusted copy of `avail.idx`. avail_idx: u16, last_used_idx: u16, + /// Whether the `VIRTIO_F_EVENT_IDX` feature has been negotiated. + event_idx: bool, #[cfg(feature = "alloc")] indirect: bool, #[cfg(feature = "alloc")] @@ -59,8 +61,19 @@ pub struct VirtQueue { } impl VirtQueue { - /// Create a new VirtQueue. - pub fn new(transport: &mut T, idx: u16, indirect: bool) -> Result { + /// Creates a new VirtQueue. + /// + /// * `indirect`: Whether to use indirect descriptors. This should be set if the + /// `VIRTIO_F_INDIRECT_DESC` feature has been negotiated with the device. + /// * `event_idx`: Whether to use the `used_event` and `avail_event` fields for notification + /// suppression. This should be set if the `VIRTIO_F_EVENT_IDX` feature has been negotiated + /// with the device. + pub fn new( + transport: &mut T, + idx: u16, + indirect: bool, + event_idx: bool, + ) -> Result { if transport.queue_used(idx) { return Err(Error::AlreadyUsed); } @@ -115,6 +128,7 @@ impl VirtQueue { desc_shadow, avail_idx: 0, last_used_idx: 0, + event_idx, #[cfg(feature = "alloc")] indirect, #[cfg(feature = "alloc")] @@ -310,9 +324,16 @@ impl VirtQueue { // Read barrier, so we read a fresh value from the device. fence(Ordering::SeqCst); - // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable - // instance of UsedRing. - unsafe { (*self.used.as_ptr()).flags & 0x0001 == 0 } + if self.event_idx { + // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable + // instance of UsedRing. + let avail_event = unsafe { (*self.used.as_ptr()).avail_event }; + self.avail_idx >= avail_event.wrapping_add(1) + } else { + // Safe because self.used points to a valid, aligned, initialised, dereferenceable, readable + // instance of UsedRing. + unsafe { (*self.used.as_ptr()).flags & 0x0001 == 0 } + } } /// Copies the descriptor at the given index from `desc_shadow` to `desc`, so it can be seen by @@ -735,7 +756,8 @@ struct UsedRing { flags: u16, idx: u16, ring: [UsedElem; SIZE], - avail_event: u16, // unused + /// Only used if `VIRTIO_F_EVENT_IDX` is negotiated. + avail_event: u16, } #[repr(C)] @@ -928,7 +950,7 @@ mod tests { let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); // Size not a power of 2. assert_eq!( - VirtQueue::::new(&mut transport, 0, false).unwrap_err(), + VirtQueue::::new(&mut transport, 0, false, false).unwrap_err(), Error::InvalidParam ); } @@ -938,7 +960,7 @@ mod tests { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); assert_eq!( - VirtQueue::::new(&mut transport, 0, false).unwrap_err(), + VirtQueue::::new(&mut transport, 0, false, false).unwrap_err(), Error::InvalidParam ); } @@ -947,9 +969,9 @@ mod tests { fn queue_already_used() { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); - VirtQueue::::new(&mut transport, 0, false).unwrap(); + VirtQueue::::new(&mut transport, 0, false, false).unwrap(); assert_eq!( - VirtQueue::::new(&mut transport, 0, false).unwrap_err(), + VirtQueue::::new(&mut transport, 0, false, false).unwrap_err(), Error::AlreadyUsed ); } @@ -958,7 +980,7 @@ mod tests { fn add_empty() { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); - let mut queue = VirtQueue::::new(&mut transport, 0, false).unwrap(); + let mut queue = VirtQueue::::new(&mut transport, 0, false, false).unwrap(); assert_eq!( unsafe { queue.add(&[], &mut []) }.unwrap_err(), Error::InvalidParam @@ -969,7 +991,7 @@ mod tests { fn add_too_many() { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); - let mut queue = VirtQueue::::new(&mut transport, 0, false).unwrap(); + let mut queue = VirtQueue::::new(&mut transport, 0, false, false).unwrap(); assert_eq!(queue.available_desc(), 4); assert_eq!( unsafe { queue.add(&[&[], &[], &[]], &mut [&mut [], &mut []]) }.unwrap_err(), @@ -981,7 +1003,7 @@ mod tests { fn add_buffers() { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); - let mut queue = VirtQueue::::new(&mut transport, 0, false).unwrap(); + let mut queue = VirtQueue::::new(&mut transport, 0, false, false).unwrap(); assert_eq!(queue.available_desc(), 4); // Add a buffer chain consisting of two device-readable parts followed by two @@ -1044,7 +1066,7 @@ mod tests { let mut header = VirtIOHeader::make_fake_header(MODERN_VERSION, 1, 0, 0, 4); let mut transport = unsafe { MmioTransport::new(NonNull::from(&mut header)) }.unwrap(); - let mut queue = VirtQueue::::new(&mut transport, 0, true).unwrap(); + let mut queue = VirtQueue::::new(&mut transport, 0, true, false).unwrap(); assert_eq!(queue.available_desc(), 4); // Add a buffer chain consisting of two device-readable parts followed by two From 961938ab196b6901d70ee24675205440107a383b Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Mon, 17 Jul 2023 15:36:40 +0100 Subject: [PATCH 2/3] Add tests for notifying device about queue. These cover suppressing notifications either using flags or with avail_event. --- src/queue.rs | 84 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/src/queue.rs b/src/queue.rs index 854a6099..dcafcbca 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -939,10 +939,16 @@ pub(crate) fn fake_read_write_queue( mod tests { use super::*; use crate::{ + device::common::Feature, hal::fake::FakeHal, - transport::mmio::{MmioTransport, VirtIOHeader, MODERN_VERSION}, + transport::{ + fake::{FakeTransport, QueueStatus, State}, + mmio::{MmioTransport, VirtIOHeader, MODERN_VERSION}, + DeviceStatus, DeviceType, + }, }; use core::ptr::NonNull; + use std::sync::{Arc, Mutex}; #[test] fn invalid_queue_size() { @@ -1111,4 +1117,80 @@ mod tests { assert_eq!((*indirect_descriptors)[3].flags, DescFlags::WRITE); } } + + /// Tests that the queue notifies the device about added buffers, if it hasn't suppressed + /// notifications. + #[test] + fn add_notify() { + let mut config_space = (); + let state = Arc::new(Mutex::new(State { + queues: vec![QueueStatus::default()], + ..Default::default() + })); + let mut transport = FakeTransport { + device_type: DeviceType::Block, + max_queue_size: 4, + device_features: 0, + config_space: NonNull::from(&mut config_space), + state: state.clone(), + }; + let mut queue = VirtQueue::::new(&mut transport, 0, false, false).unwrap(); + + // Add a buffer chain with a single device-readable part. + unsafe { queue.add(&[&[42]], &mut []) }.unwrap(); + + // Check that the transport would be notified. + assert_eq!(queue.should_notify(), true); + + // SAFETY: the various parts of the queue are properly aligned, dereferenceable and + // initialised, and nothing else is accessing them at the same time. + unsafe { + // Suppress notifications. + (*queue.used.as_ptr()).flags = 0x01; + } + + // Check that the transport would not be notified. + assert_eq!(queue.should_notify(), false); + } + + /// Tests that the queue notifies the device about added buffers, if it hasn't suppressed + /// notifications with the `avail_event` index. + #[test] + fn add_notify_event_idx() { + let mut config_space = (); + let state = Arc::new(Mutex::new(State { + queues: vec![QueueStatus::default()], + ..Default::default() + })); + let mut transport = FakeTransport { + device_type: DeviceType::Block, + max_queue_size: 4, + device_features: Feature::RING_EVENT_IDX.bits(), + config_space: NonNull::from(&mut config_space), + state: state.clone(), + }; + let mut queue = VirtQueue::::new(&mut transport, 0, false, true).unwrap(); + + // Add a buffer chain with a single device-readable part. + assert_eq!(unsafe { queue.add(&[&[42]], &mut []) }.unwrap(), 0); + + // Check that the transport would be notified. + assert_eq!(queue.should_notify(), true); + + // SAFETY: the various parts of the queue are properly aligned, dereferenceable and + // initialised, and nothing else is accessing them at the same time. + unsafe { + // Suppress notifications. + (*queue.used.as_ptr()).avail_event = 1; + } + + // Check that the transport would not be notified. + assert_eq!(queue.should_notify(), false); + + // Add another buffer chain. + assert_eq!(unsafe { queue.add(&[&[42]], &mut []) }.unwrap(), 1); + + // Check that the transport should be notified again now. + assert_eq!(queue.should_notify(), true); + } } From 6dae798a6c7941e24c736aa0052d896ddd72c003 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Mon, 17 Jul 2023 16:19:00 +0100 Subject: [PATCH 3/3] Deduplicate feature negotiation. Rather than each device driver passing a closure to negotiate features, just pass a set of supported features. The closure was unnecessarily complicated. --- src/device/blk.rs | 10 +--------- src/device/console.rs | 9 +-------- src/device/gpu.rs | 8 +------- src/device/input.rs | 9 +-------- src/device/net.rs | 10 ++-------- src/device/socket/vsock.rs | 8 +------- src/transport/mod.rs | 21 ++++++++++++++++----- 7 files changed, 23 insertions(+), 52 deletions(-) diff --git a/src/device/blk.rs b/src/device/blk.rs index 719a037f..9b7bf36b 100644 --- a/src/device/blk.rs +++ b/src/device/blk.rs @@ -52,15 +52,7 @@ pub struct VirtIOBlk { impl VirtIOBlk { /// Create a new VirtIO-Blk driver. pub fn new(mut transport: T) -> Result { - let mut negotiated_features = BlkFeature::empty(); - - transport.begin_init(|features| { - let features = BlkFeature::from_bits_truncate(features); - info!("device features: {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - // Negotiate these features only. - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); // Read configuration space. let config = transport.config_space::()?; diff --git a/src/device/console.rs b/src/device/console.rs index 635ffcd5..65282766 100644 --- a/src/device/console.rs +++ b/src/device/console.rs @@ -8,7 +8,6 @@ use crate::{Result, PAGE_SIZE}; use alloc::boxed::Box; use bitflags::bitflags; use core::ptr::NonNull; -use log::info; const QUEUE_RECEIVEQ_PORT_0: u16 = 0; const QUEUE_TRANSMITQ_PORT_0: u16 = 1; @@ -66,13 +65,7 @@ pub struct ConsoleInfo { impl VirtIOConsole { /// Creates a new VirtIO console driver. pub fn new(mut transport: T) -> Result { - let mut negotiated_features = Features::empty(); - transport.begin_init(|features| { - let features = Features::from_bits_truncate(features); - info!("Device features {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); let config_space = transport.config_space::()?; let receiveq = VirtQueue::new( &mut transport, diff --git a/src/device/gpu.rs b/src/device/gpu.rs index 6541c8d4..e19b7807 100644 --- a/src/device/gpu.rs +++ b/src/device/gpu.rs @@ -40,13 +40,7 @@ pub struct VirtIOGpu { impl VirtIOGpu { /// Create a new VirtIO-Gpu driver. pub fn new(mut transport: T) -> Result { - let mut negotiated_features = Features::empty(); - transport.begin_init(|features| { - let features = Features::from_bits_truncate(features); - info!("Device features {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); // read configuration space let config_space = transport.config_space::()?; diff --git a/src/device/input.rs b/src/device/input.rs index a9d7ec9c..c277b649 100644 --- a/src/device/input.rs +++ b/src/device/input.rs @@ -8,7 +8,6 @@ use crate::volatile::{volread, volwrite, ReadOnly, WriteOnly}; use crate::Result; use alloc::boxed::Box; use core::ptr::NonNull; -use log::info; use zerocopy::{AsBytes, FromBytes}; /// Virtual human interface devices such as keyboards, mice and tablets. @@ -29,13 +28,7 @@ impl VirtIOInput { pub fn new(mut transport: T) -> Result { let mut event_buf = Box::new([InputEvent::default(); QUEUE_SIZE]); - let mut negotiated_features = Feature::empty(); - transport.begin_init(|features| { - let features = Feature::from_bits_truncate(features); - info!("Device features: {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); let config = transport.config_space::()?; diff --git a/src/device/net.rs b/src/device/net.rs index b922b374..522997e4 100644 --- a/src/device/net.rs +++ b/src/device/net.rs @@ -8,7 +8,7 @@ use crate::{Error, Result}; use alloc::{vec, vec::Vec}; use bitflags::bitflags; use core::{convert::TryInto, mem::size_of}; -use log::{debug, info, warn}; +use log::{debug, warn}; use zerocopy::{AsBytes, FromBytes}; const MAX_BUFFER_LEN: usize = 65535; @@ -112,13 +112,7 @@ pub struct VirtIONet { impl VirtIONet { /// Create a new VirtIO-Net driver. pub fn new(mut transport: T, buf_len: usize) -> Result { - let mut negotiated_features = Features::empty(); - transport.begin_init(|features| { - let features = Features::from_bits_truncate(features); - info!("Device features {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); // read configuration space let config = transport.config_space::()?; let mac; diff --git a/src/device/socket/vsock.rs b/src/device/socket/vsock.rs index a1ea4304..2e9978a7 100644 --- a/src/device/socket/vsock.rs +++ b/src/device/socket/vsock.rs @@ -242,13 +242,7 @@ impl Drop for VirtIOSocket { impl VirtIOSocket { /// Create a new VirtIO Vsock driver. pub fn new(mut transport: T) -> Result { - let mut negotiated_features = Feature::empty(); - transport.begin_init(|features| { - let features = Feature::from_bits_truncate(features); - debug!("Device features: {:?}", features); - negotiated_features = features & SUPPORTED_FEATURES; - negotiated_features.bits() - }); + let negotiated_features = transport.begin_init(SUPPORTED_FEATURES); let config = transport.config_space::()?; debug!("config: {:?}", config); diff --git a/src/transport/mod.rs b/src/transport/mod.rs index 3157e81a..f6e9eae4 100644 --- a/src/transport/mod.rs +++ b/src/transport/mod.rs @@ -6,8 +6,9 @@ pub mod mmio; pub mod pci; use crate::{PhysAddr, Result, PAGE_SIZE}; -use bitflags::bitflags; -use core::ptr::NonNull; +use bitflags::{bitflags, Flags}; +use core::{fmt::Debug, ops::BitAnd, ptr::NonNull}; +use log::debug; /// A VirtIO transport layer. pub trait Transport { @@ -64,17 +65,27 @@ pub trait Transport { /// Begins initializing the device. /// /// Ref: virtio 3.1.1 Device Initialization - fn begin_init(&mut self, negotiate_features: impl FnOnce(u64) -> u64) { + /// + /// Returns the negotiated set of features. + fn begin_init + BitAnd + Debug>( + &mut self, + supported_features: F, + ) -> F { self.set_status(DeviceStatus::empty()); self.set_status(DeviceStatus::ACKNOWLEDGE | DeviceStatus::DRIVER); - let features = self.read_device_features(); - self.write_driver_features(negotiate_features(features)); + let device_features = F::from_bits_truncate(self.read_device_features()); + debug!("Device features: {:?}", device_features); + let negotiated_features = device_features & supported_features; + self.write_driver_features(negotiated_features.bits()); + self.set_status( DeviceStatus::ACKNOWLEDGE | DeviceStatus::DRIVER | DeviceStatus::FEATURES_OK, ); self.set_guest_page_size(PAGE_SIZE as u32); + + negotiated_features } /// Finishes initializing the device.