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.
rcore-os · Jul 24, 2023 · abd9fb5 · abd9fb5
1 parent c970b4c
commit abd9fb5
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 52 deletions.
diff --git a/src/device/blk.rs b/src/device/blk.rs
@@ -52,15 +52,7 @@ pub struct VirtIOBlk<H: Hal, T: Transport> {
 impl<H: Hal, T: Transport> VirtIOBlk<H, T> {
     /// Create a new VirtIO-Blk driver.
     pub fn new(mut transport: T) -> Result<Self> {
-        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::<BlkConfig>()?;

diff --git 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<H: Hal, T: Transport> VirtIOConsole<H, T> {
     /// Creates a new VirtIO console driver.
     pub fn new(mut transport: T) -> Result<Self> {
-        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::<Config>()?;
         let receiveq = VirtQueue::new(
             &mut transport,

diff --git a/src/device/gpu.rs b/src/device/gpu.rs
@@ -40,13 +40,7 @@ pub struct VirtIOGpu<H: Hal, T: Transport> {
 impl<H: Hal, T: Transport> VirtIOGpu<H, T> {
     /// Create a new VirtIO-Gpu driver.
     pub fn new(mut transport: T) -> Result<Self> {
-        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::<Config>()?;

diff --git 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<H: Hal, T: Transport> VirtIOInput<H, T> {
     pub fn new(mut transport: T) -> Result<Self> {
         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::<Config>()?;
 

diff --git 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<H: Hal, T: Transport, const QUEUE_SIZE: usize> {
 impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONet<H, T, QUEUE_SIZE> {
     /// Create a new VirtIO-Net driver.
     pub fn new(mut transport: T, buf_len: usize) -> Result<Self> {
-        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::<Config>()?;
         let mac;

diff --git a/src/device/socket/vsock.rs b/src/device/socket/vsock.rs
@@ -242,13 +242,7 @@ impl<H: Hal, T: Transport> Drop for VirtIOSocket<H, T> {
 impl<H: Hal, T: Transport> VirtIOSocket<H, T> {
     /// Create a new VirtIO Vsock driver.
     pub fn new(mut transport: T) -> Result<Self> {
-        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::<VirtioVsockConfig>()?;
         debug!("config: {:?}", config);

diff --git 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<F: Flags<Bits = u64> + BitAnd<Output = F> + 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.