Skip to content

refactor(virtio): unify feature negotiation #2147

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.

Sign up for GitHub

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
mkroening merged 5 commits into from
Jan 5, 2026
Merged

refactor(virtio): unify feature negotiation #2147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42887dd
refactor(virtio-net-mmio): simplify VirtioNetDriver::init
mkroening Jan 4, 2026
19e49fb
fix(virtio): dead_code on VirtioError
mkroening Jan 4, 2026
d5b4660
fix(virtio-fs-mmio): print information
mkroening Jan 4, 2026
3c608e4
refactor(virtio): make allows more fine-grained
mkroening Jan 4, 2026
05b17b4
refactor(virtio): unify feature negotiation
mkroening Jan 4, 2026
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
56 changes: 12 additions & 44 deletions src/drivers/console/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use alloc::vec::Vec;

use embedded_io::{ErrorType, Read, ReadReady, Write};
use smallvec::SmallVec;
use virtio::FeatureBits;
use virtio::console::Config;
use volatile::VolatileRef;
use volatile::access::ReadOnly;
Expand All @@ -30,6 +29,7 @@ use crate::drivers::error::DriverError;
use crate::drivers::mmio::get_console_driver;
#[cfg(feature = "pci")]
use crate::drivers::pci::get_console_driver;
use crate::drivers::virtio::ControlRegisters;
use crate::drivers::virtio::error::VirtioConsoleError;
#[cfg(not(feature = "pci"))]
use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
Expand Down Expand Up @@ -293,34 +293,6 @@ impl VirtioConsoleDriver {
self.com_cfg.set_failed();
}

/// Negotiates a subset of features, understood and wanted by both the OS
/// and the device.
fn negotiate_features(
&mut self,
driver_features: virtio::console::F,
) -> Result<(), VirtioConsoleError> {
let device_features = virtio::console::F::from(self.com_cfg.dev_features());

if device_features.requirements_satisfied() {
info!("Feature set wanted by console driver are in conformance with specification.");
} else {
return Err(VirtioConsoleError::FeatureRequirementsNotMet(
device_features,
));
}

if device_features.contains(driver_features) {
// If device supports subset of features write feature set to common config
self.com_cfg.set_drv_features(driver_features.into());
Ok(())
} else {
Err(VirtioConsoleError::IncompatibleFeatureSets(
driver_features,
device_features,
))
}
}

pub fn init_dev(&mut self) -> Result<(), VirtioConsoleError> {
// Reset
self.com_cfg.reset_dev();
Expand All @@ -331,8 +303,16 @@ impl VirtioConsoleDriver {
// Indicate device, that driver is able to handle it
self.com_cfg.set_drv();

let features = virtio::console::F::VERSION_1;
self.negotiate_features(features)?;
let minimal_features = virtio::console::F::VERSION_1;
let negotiated_features = self
.com_cfg
.control_registers()
.negotiate_features(minimal_features);

if !negotiated_features.contains(minimal_features) {
error!("Device features set, does not satisfy minimal features needed. Aborting!");
return Err(VirtioConsoleError::FailFeatureNeg(self.dev_cfg.dev_id));
}

// Indicates the device, that the current feature set is final for the driver
// and will not be changed.
Expand All @@ -345,7 +325,7 @@ impl VirtioConsoleDriver {
self.dev_cfg.dev_id
);
// Set feature set in device config fur future use.
self.dev_cfg.features = features;
self.dev_cfg.features = negotiated_features;
} else {
return Err(VirtioConsoleError::FailFeatureNeg(self.dev_cfg.dev_id));
}
Expand Down Expand Up @@ -429,17 +409,5 @@ pub mod error {
"Virtio console device driver failed, for device {0:x}, device did not acknowledge negotiated feature set!"
)]
FailFeatureNeg(u16),

/// Set of features does not adhere to the requirements of features
/// indicated by the specification
#[error(
"Virtio console driver tried to set feature bit without setting dependency feature. Feat set: {0:?}"
)]
FeatureRequirementsNotMet(virtio::console::F),

/// The first u64 contains the feature bits wanted by the driver.
/// but which are incompatible with the device feature set, second u64.
#[error("Feature set: {0:?} , is incompatible with the device features: {1:?}")]
IncompatibleFeatureSets(virtio::console::F, virtio::console::F),
}
}
1 change: 1 addition & 0 deletions src/drivers/fs/mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl VirtioFsDriver {
) -> Result<VirtioFsDriver, VirtioError> {
let mut drv = VirtioFsDriver::new(dev_id, registers, irq)?;
drv.init_dev().map_err(VirtioError::FsDriver)?;
drv.com_cfg.print_information();
Ok(drv)
}
}
53 changes: 12 additions & 41 deletions src/drivers/fs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ use fuse_abi::linux::fuse_out_header;
use num_enum::TryFromPrimitive;
use pci_types::InterruptLine;
use smallvec::SmallVec;
use virtio::FeatureBits;
use virtio::fs::ConfigVolatileFieldAccess;
use volatile::VolatileRef;
use volatile::access::ReadOnly;

use crate::config::VIRTIO_MAX_QUEUE_SIZE;
use crate::drivers::Driver;
use crate::drivers::virtio::ControlRegisters;
use crate::drivers::virtio::error::VirtioFsError;
#[cfg(not(feature = "pci"))]
use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
Expand Down Expand Up @@ -80,31 +80,6 @@ impl VirtioFsDriver {
self.com_cfg.set_failed();
}

/// Negotiates a subset of features, understood and wanted by both the OS
/// and the device.
fn negotiate_features(&mut self, driver_features: virtio::fs::F) -> Result<(), VirtioFsError> {
let device_features = virtio::fs::F::from(self.com_cfg.dev_features());

if device_features.requirements_satisfied() {
debug!(
"Feature set wanted by filesystem driver are in conformance with specification."
);
} else {
return Err(VirtioFsError::FeatureRequirementsNotMet(device_features));
}

if device_features.contains(driver_features) {
// If device supports subset of features write feature set to common config
self.com_cfg.set_drv_features(driver_features.into());
Ok(())
} else {
Err(VirtioFsError::IncompatibleFeatureSets(
driver_features,
device_features,
))
}
}

/// Initializes the device in adherence to specification. Returns Some(VirtioFsError)
/// upon failure and None in case everything worked as expected.
///
Expand All @@ -120,8 +95,16 @@ impl VirtioFsDriver {
// Indicate device, that driver is able to handle it
self.com_cfg.set_drv();

let features = virtio::fs::F::VERSION_1;
self.negotiate_features(features)?;
let minimal_features = virtio::fs::F::VERSION_1;
let negotiated_features = self
.com_cfg
.control_registers()
.negotiate_features(minimal_features);

if !negotiated_features.contains(minimal_features) {
error!("Device features set, does not satisfy minimal features needed. Aborting!");
return Err(VirtioFsError::FailFeatureNeg(self.dev_cfg.dev_id));
}

// Indicates the device, that the current feature set is final for the driver
// and will not be changed.
Expand All @@ -134,7 +117,7 @@ impl VirtioFsDriver {
self.dev_cfg.dev_id
);
// Set feature set in device config fur future use.
self.dev_cfg.features = features;
self.dev_cfg.features = negotiated_features;
} else {
return Err(VirtioFsError::FailFeatureNeg(self.dev_cfg.dev_id));
}
Expand Down Expand Up @@ -290,18 +273,6 @@ pub mod error {
)]
FailFeatureNeg(u16),

/// The first field contains the feature bits wanted by the driver.
/// but which are incompatible with the device feature set, second field.
#[error("Feature set: {0:?} , is incompatible with the device features: {1:?}")]
IncompatibleFeatureSets(virtio::fs::F, virtio::fs::F),

/// Set of features does not adhere to the requirements of features
/// indicated by the specification
#[error(
"Virtio filesystem driver tried to set feature bit without setting dependency feature. Feat set: {0:?}"
)]
FeatureRequirementsNotMet(virtio::fs::F),

#[error("Virtio filesystem failed, driver failed due unknown reason!")]
Unknown,
}
Expand Down
20 changes: 6 additions & 14 deletions src/drivers/net/virtio/mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use volatile::VolatileRef;

use crate::drivers::InterruptLine;
use crate::drivers::net::virtio::{Init, NetDevCfg, Uninit, VirtioNetDriver};
use crate::drivers::virtio::error::{VirtioError, VirtioNetError};
use crate::drivers::virtio::error::VirtioError;
use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};

// Backend-dependent interface for Virtio network driver
Expand All @@ -13,7 +13,7 @@ impl VirtioNetDriver<Uninit> {
dev_id: u16,
mut registers: VolatileRef<'static, DeviceRegisters>,
irq: InterruptLine,
) -> Result<Self, VirtioNetError> {
) -> Result<Self, VirtioError> {
let dev_cfg_raw: &'static virtio::net::Config = unsafe {
&*registers
.borrow_mut()
Expand Down Expand Up @@ -54,18 +54,10 @@ impl VirtioNetDriver<Uninit> {
registers: VolatileRef<'static, DeviceRegisters>,
irq: InterruptLine,
) -> Result<VirtioNetDriver<Init>, VirtioError> {
if let Ok(drv) = VirtioNetDriver::new(dev_id, registers, irq) {
match drv.init_dev() {
Err(error_code) => Err(VirtioError::NetDriver(error_code)),
Ok(mut initialized_drv) => {
initialized_drv.print_information();
Ok(initialized_drv)
}
}
} else {
error!("Unable to create Driver. Aborting!");
Err(VirtioError::Unknown)
}
let drv = VirtioNetDriver::new(dev_id, registers, irq)?;
let mut drv = drv.init_dev().map_err(VirtioError::NetDriver)?;
drv.print_information();
Ok(drv)
}
}

Expand Down
Loading
Loading