From c4ca2946ae68b62ee89f14fbbf40fb9df2ea8c82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sat, 4 Jul 2026 03:44:23 +0200 Subject: [PATCH 1/9] refactor(virtio): make UniCapsColl non-PCI specific This will allow sharing code between PCI and MMIO based devices. --- src/drivers/console/pci.rs | 23 +++++++++++------- src/drivers/fs/pci.rs | 10 ++++---- src/drivers/net/virtio/pci.rs | 12 ++++++---- src/drivers/virtio/transport/mod.rs | 13 ++++++++-- src/drivers/virtio/transport/pci.rs | 37 +++++++++++------------------ src/drivers/vsock/pci.rs | 12 ++++++---- 6 files changed, 58 insertions(+), 49 deletions(-) diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index b8f611648a..db57245533 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -1,3 +1,5 @@ +use alloc::vec::Vec; + use pci_types::CommandRegister; use virtio::console::Config; use volatile::VolatileRef; @@ -7,7 +9,8 @@ use crate::drivers::InterruptHandlerMap; use crate::drivers::console::{ConsoleDevCfg, RxQueue, TxQueue, VirtioConsoleDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci::{self, PciCap, UniCapsColl}; +use crate::drivers::virtio::transport::UniCapsColl; +use crate::drivers::virtio::transport::pci::{self, PciCap}; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { @@ -26,6 +29,7 @@ impl VirtioConsoleDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, + dev_cfg_list: Vec, device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -33,8 +37,7 @@ impl VirtioConsoleDriver { let UniCapsColl { com_cfg, notif_cfg, - isr_cfg, - dev_cfg_list, + int_cap: isr_cfg, .. } = caps_coll; @@ -64,13 +67,15 @@ impl VirtioConsoleDriver { device.set_command(CommandRegister::BUS_MASTER_ENABLE); let mut drv = match pci::map_caps(device) { - Ok(caps) => match VirtioConsoleDriver::new(caps, device) { - Ok(driver) => driver, - Err(console_err) => { - error!("Initializing new virtio console device driver failed. Aborting!"); - return Err(VirtioError::ConsoleDriver(console_err)); + Ok((caps, dev_cfg_list)) => { + match VirtioConsoleDriver::new(caps, dev_cfg_list, device) { + Ok(driver) => driver, + Err(console_err) => { + error!("Initializing new virtio console device driver failed. Aborting!"); + return Err(VirtioError::ConsoleDriver(console_err)); + } } - }, + } Err(err) => { error!("Mapping capabilities failed. Aborting!"); return Err(err); diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index 0605e111b9..3b240faa0c 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -7,8 +7,8 @@ use crate::drivers::InterruptHandlerMap; use crate::drivers::fs::{FsDevCfg, VirtioFsDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::{PciCap, UniCapsColl}; +use crate::drivers::virtio::transport::pci::PciCap; +use crate::drivers::virtio::transport::{UniCapsColl, pci}; impl VirtioFsDriver { fn map_cfg(cap: &PciCap) -> Option { @@ -27,6 +27,7 @@ impl VirtioFsDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, + dev_cfg_list: Vec, device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -34,8 +35,7 @@ impl VirtioFsDriver { let UniCapsColl { com_cfg, notif_cfg, - isr_cfg, - dev_cfg_list, + int_cap: isr_cfg, .. } = caps_coll; @@ -59,7 +59,7 @@ impl VirtioFsDriver { handlers: &mut InterruptHandlerMap, ) -> Result { let mut drv = match pci::map_caps(device) { - Ok(caps) => match VirtioFsDriver::new(caps, device) { + Ok((caps, dev_cfg_list)) => match VirtioFsDriver::new(caps, dev_cfg_list, device) { Ok(driver) => driver, Err(fs_err) => { error!("Initializing new network driver failed. Aborting!"); diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index f2b217438e..3ef94f4565 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -1,3 +1,5 @@ +use alloc::vec::Vec; + use pci_types::CommandRegister; use smoltcp::phy::ChecksumCapabilities; use volatile::VolatileRef; @@ -8,8 +10,8 @@ use crate::drivers::InterruptHandlerMap; use crate::drivers::net::virtio::{NetDevCfg, VirtioNetDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::{PciCap, UniCapsColl}; +use crate::drivers::virtio::transport::pci::PciCap; +use crate::drivers::virtio::transport::{UniCapsColl, pci}; // Backend-dependent interface for Virtio network driver impl VirtioNetDriver { @@ -29,14 +31,14 @@ impl VirtioNetDriver { /// configuration structures and moving them into the struct. pub(crate) fn new( caps_coll: UniCapsColl, + dev_cfg_list: Vec, device: &PciDevice, ) -> Result { let device_id = device.device_id(); let UniCapsColl { com_cfg, notif_cfg, - isr_cfg, - dev_cfg_list, + int_cap: isr_cfg, .. } = caps_coll; @@ -72,7 +74,7 @@ impl VirtioNetDriver { device.set_command(CommandRegister::BUS_MASTER_ENABLE); let drv = match pci::map_caps(device) { - Ok(caps) => match VirtioNetDriver::new(caps, device) { + Ok((caps, dev_cfg_list)) => match VirtioNetDriver::new(caps, dev_cfg_list, device) { Ok(driver) => driver, Err(vnet_err) => { error!("Initializing new network driver failed. Aborting!"); diff --git a/src/drivers/virtio/transport/mod.rs b/src/drivers/virtio/transport/mod.rs index 4c43093adc..67c3a9617e 100644 --- a/src/drivers/virtio/transport/mod.rs +++ b/src/drivers/virtio/transport/mod.rs @@ -1,7 +1,7 @@ #[cfg(not(feature = "pci"))] -use mmio::IsrStatus; +use mmio::{ComCfg, IsrStatus, NotifCfg}; #[cfg(feature = "pci")] -use pci::IsrStatus; +use pci::{ComCfg, IsrStatus, NotifCfg}; #[cfg(not(feature = "pci"))] pub mod mmio; @@ -13,3 +13,12 @@ pub(crate) enum InterruptCapability { #[cfg(all(feature = "pci", target_arch = "x86_64"))] Msix(volatile::VolatileRef<'static, [crate::drivers::pci::msix::TableEntry]>), } + +#[cfg_attr(not(feature = "pci"), expect(dead_code))] +/// Universal Caplist Collections holds all universal capability structures for +/// a given Virtio device. +pub struct UniCapsColl { + pub(crate) com_cfg: ComCfg, + pub(crate) notif_cfg: NotifCfg, + pub(crate) int_cap: InterruptCapability, +} diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 88765cd77d..47e36f3a21 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -38,8 +38,8 @@ use crate::drivers::pci::msix; #[cfg(target_arch = "x86_64")] use crate::drivers::pci::msix::MsixTableVolatileElementAccess; use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::InterruptCapability; use crate::drivers::virtio::transport::pci::PciBar as VirtioPciBar; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::{ControlRegisters, VirtioIdExt}; #[cfg(feature = "virtio-vsock")] use crate::drivers::vsock::VirtioVsockDriver; @@ -171,22 +171,6 @@ impl PciCap { } } -/// Universal Caplist Collections holds all universal capability structures for -/// a given Virtio PCI device. -/// -/// As Virtio's PCI devices are allowed to present multiple capability -/// structures of the same config type, the structure -/// provides a driver with all capabilities, sorted in descending priority, -/// allowing the driver to choose. -/// The structure contains a special dev_cfg_list field, a vector holding -/// [PciCap] objects, to allow the driver to map its -/// device specific configurations independently. -pub struct UniCapsColl { - pub(crate) com_cfg: ComCfg, - pub(crate) notif_cfg: NotifCfg, - pub(crate) isr_cfg: InterruptCapability, - pub(crate) dev_cfg_list: Vec, -} /// Wraps a [`CommonCfg`] in order to preserve /// the original structure. /// @@ -609,7 +593,12 @@ impl PciBar { } } -pub(crate) fn map_caps(device: &PciDevice) -> Result { +/// The return value contains a vector holding +/// [PciCap] objects, to allow the driver to map its +/// device specific configurations independently. +pub(crate) fn map_caps( + device: &PciDevice, +) -> Result<(UniCapsColl, Vec), VirtioError> { let device_id = device.device_id(); // In case caplist pointer is not used, abort as it is essential @@ -733,12 +722,14 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result (), } - Ok(UniCapsColl { - com_cfg: com_cfg.ok_or(VirtioError::NoComCfg(device_id))?, - notif_cfg: notif_cfg.ok_or(VirtioError::NoNotifCfg(device_id))?, - isr_cfg: isr_cfg.ok_or(VirtioError::NoIsrCfg(device_id))?, + Ok(( + UniCapsColl { + com_cfg: com_cfg.ok_or(VirtioError::NoComCfg(device_id))?, + notif_cfg: notif_cfg.ok_or(VirtioError::NoNotifCfg(device_id))?, + int_cap: isr_cfg.ok_or(VirtioError::NoIsrCfg(device_id))?, + }, dev_cfg_list, - }) + )) } /// Checks existing drivers for support of given device. Upon match, provides diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index 143075c397..fa20e4be24 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -1,11 +1,13 @@ +use alloc::vec::Vec; + use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::{PciCap, UniCapsColl}; +use crate::drivers::virtio::transport::pci::PciCap; +use crate::drivers::virtio::transport::{UniCapsColl, pci}; use crate::drivers::vsock::{EventQueue, RxQueue, TxQueue, VirtioVsockDriver, VsockDevCfg}; impl VirtioVsockDriver { @@ -25,6 +27,7 @@ impl VirtioVsockDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, + dev_cfg_list: Vec, device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -32,8 +35,7 @@ impl VirtioVsockDriver { let UniCapsColl { com_cfg, notif_cfg, - isr_cfg, - dev_cfg_list, + int_cap: isr_cfg, .. } = caps_coll; @@ -61,7 +63,7 @@ impl VirtioVsockDriver { handlers: &mut InterruptHandlerMap, ) -> Result { let mut drv = match pci::map_caps(device) { - Ok(caps) => match VirtioVsockDriver::new(caps, device) { + Ok((caps, dev_cfg_list)) => match VirtioVsockDriver::new(caps, dev_cfg_list, device) { Ok(driver) => driver, Err(vsock_err) => { error!("Initializing new virtio socket device driver failed. Aborting!"); From 036bd772f69bb326c1d6386250592c4abca223d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sat, 4 Jul 2026 04:34:38 +0200 Subject: [PATCH 2/9] refactor(virtio): compose over UniCapsColl rather than destructuring --- src/drivers/console/mmio.rs | 27 +++--------- src/drivers/console/mod.rs | 41 ++++++++---------- src/drivers/console/pci.rs | 12 +----- src/drivers/fs/mmio.rs | 27 +++--------- src/drivers/fs/mod.rs | 37 +++++++--------- src/drivers/fs/pci.rs | 11 +---- src/drivers/net/virtio/mmio.rs | 26 +++--------- src/drivers/net/virtio/mod.rs | 63 +++++++++++++--------------- src/drivers/net/virtio/pci.rs | 10 +---- src/drivers/virtio/transport/mmio.rs | 27 ++++++++++++ src/drivers/virtio/transport/mod.rs | 1 - src/drivers/vsock/mmio.rs | 27 +++--------- src/drivers/vsock/mod.rs | 45 +++++++++----------- src/drivers/vsock/pci.rs | 11 +---- 14 files changed, 137 insertions(+), 228 deletions(-) diff --git a/src/drivers/console/mmio.rs b/src/drivers/console/mmio.rs index 83cd170412..231688725e 100644 --- a/src/drivers/console/mmio.rs +++ b/src/drivers/console/mmio.rs @@ -1,42 +1,27 @@ -use virtio::console::Config; -use virtio::mmio::{DeviceRegisters, DeviceRegistersVolatileFieldAccess}; +use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; use crate::drivers::console::{ConsoleDevCfg, RxQueue, TxQueue, VirtioConsoleDriver}; use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::InterruptCapability; -use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg}; +use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { pub fn new( dev_id: u16, - mut registers: VolatileRef<'static, DeviceRegisters>, + registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { - let dev_cfg_raw: &'static Config = unsafe { - &*registers - .borrow_mut() - .as_mut_ptr() - .config() - .as_raw_ptr() - .cast::() - .as_ptr() - }; - let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); + let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = ConsoleDevCfg { raw: dev_cfg_raw, dev_id, features: virtio::console::F::empty(), }; - let isr_stat = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); - let notif_cfg = NotifCfg::new(registers.borrow_mut()); Ok(VirtioConsoleDriver { dev_cfg, - com_cfg: ComCfg::new(registers), - isr_stat, - notif_cfg, + caps_coll, recv_vq: RxQueue::new(), send_vq: TxQueue::new(), }) @@ -56,7 +41,7 @@ impl VirtioConsoleDriver { let mut drv = VirtioConsoleDriver::new(dev_id, registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::ConsoleDriver)?; - drv.com_cfg.print_information(); + drv.caps_coll.com_cfg.print_information(); Ok(drv) } } diff --git a/src/drivers/console/mod.rs b/src/drivers/console/mod.rs index f40866ce7a..8b7d6fe912 100644 --- a/src/drivers/console/mod.rs +++ b/src/drivers/console/mod.rs @@ -32,11 +32,7 @@ use crate::drivers::mmio::get_console_driver; use crate::drivers::pci::get_console_driver; use crate::drivers::virtio::ControlRegisters; use crate::drivers::virtio::error::VirtioConsoleError; -use crate::drivers::virtio::transport::InterruptCapability; -#[cfg(not(feature = "pci"))] -use crate::drivers::virtio::transport::mmio::{ComCfg, NotifCfg}; -#[cfg(feature = "pci")] -use crate::drivers::virtio::transport::pci::{ComCfg, NotifCfg}; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ AvailBufferToken, BufferElem, BufferType, UsedBufferToken, VirtQueue, Virtq, @@ -260,9 +256,7 @@ pub(crate) struct ConsoleDevCfg { pub(crate) struct VirtioConsoleDriver { pub(super) dev_cfg: ConsoleDevCfg, - pub(super) com_cfg: ComCfg, - pub(super) isr_stat: InterruptCapability, - pub(super) notif_cfg: NotifCfg, + pub(super) caps_coll: UniCapsColl, pub(super) recv_vq: RxQueue, pub(super) send_vq: TxQueue, @@ -285,7 +279,7 @@ impl VirtioConsoleDriver { not(all(feature = "pci", target_arch = "x86_64")), expect(irrefutable_let_patterns) )] - let InterruptCapability::IsrStatus(ref mut isr_stat) = self.isr_stat else { + let InterruptCapability::IsrStatus(ref mut isr_stat) = self.caps_coll.int_cap else { panic!("MSI-X vectors should be configured to the interrupt type-specific handlers.") }; @@ -304,7 +298,7 @@ impl VirtioConsoleDriver { } fn handle_device_configuration_interrupt(&self) { - if self.com_cfg.does_device_need_reset() { + if self.caps_coll.com_cfg.does_device_need_reset() { todo!("Device configuration change notification cannot be handled yet"); } } @@ -315,7 +309,7 @@ impl VirtioConsoleDriver { #[cfg(feature = "pci")] pub fn set_failed(&mut self) { - self.com_cfg.set_failed(); + self.caps_coll.com_cfg.set_failed(); } pub fn init_dev( @@ -324,16 +318,17 @@ impl VirtioConsoleDriver { irq: Option, ) -> Result<(), VirtioConsoleError> { // Reset - self.com_cfg.reset_dev(); + self.caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.com_cfg.ack_dev(); + self.caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.com_cfg.set_drv(); + self.caps_coll.com_cfg.set_drv(); let minimal_features = virtio::console::F::VERSION_1; let negotiated_features = self + .caps_coll .com_cfg .control_registers() .negotiate_features(minimal_features); @@ -345,10 +340,10 @@ impl VirtioConsoleDriver { // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.com_cfg.features_ok(); + self.caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.com_cfg.check_features() { + if self.caps_coll.com_cfg.check_features() { info!( "Features have been negotiated between virtio console device {:x} and driver.", self.dev_cfg.dev_id @@ -363,8 +358,8 @@ impl VirtioConsoleDriver { // create the queues and tell device about them self.recv_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 0, self.dev_cfg.features.into(), @@ -376,8 +371,8 @@ impl VirtioConsoleDriver { self.send_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 1, self.dev_cfg.features.into(), @@ -387,7 +382,7 @@ impl VirtioConsoleDriver { // Interrupt for communicating that a sent packet left, is not needed self.send_vq.disable_notifs(); - match &mut self.isr_stat { + match &mut self.caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -399,7 +394,7 @@ impl VirtioConsoleDriver { info!("Virtio interrupt handler at line {irq}"); } #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => self.com_cfg.register_msix_vectors( + InterruptCapability::Msix(msix_table) => self.caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -413,7 +408,7 @@ impl VirtioConsoleDriver { } // At this point the device is "live" - self.com_cfg.drv_ok(); + self.caps_coll.com_cfg.drv_ok(); Ok(()) } diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index db57245533..79c5bd8b55 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -33,14 +33,6 @@ impl VirtioConsoleDriver { device: &PciDevice, ) -> Result { let device_id = device.device_id(); - - let UniCapsColl { - com_cfg, - notif_cfg, - int_cap: isr_cfg, - .. - } = caps_coll; - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioConsoleDriver::map_cfg) else { error!("No dev config. Aborting!"); return Err(error::VirtioConsoleError::NoDevCfg(device_id)); @@ -48,9 +40,7 @@ impl VirtioConsoleDriver { Ok(VirtioConsoleDriver { dev_cfg, - com_cfg, - isr_stat: isr_cfg, - notif_cfg, + caps_coll, recv_vq: RxQueue::new(), send_vq: TxQueue::new(), }) diff --git a/src/drivers/fs/mmio.rs b/src/drivers/fs/mmio.rs index e913cca6a0..e0dfdcb8eb 100644 --- a/src/drivers/fs/mmio.rs +++ b/src/drivers/fs/mmio.rs @@ -1,44 +1,29 @@ use alloc::vec::Vec; -use virtio::fs::Config; -use virtio::mmio::{DeviceRegisters, DeviceRegistersVolatileFieldAccess}; +use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; use crate::drivers::fs::{FsDevCfg, VirtioFsDriver}; use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::InterruptCapability; -use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg}; +use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio fs driver impl VirtioFsDriver { pub fn new( dev_id: u16, - mut registers: VolatileRef<'static, DeviceRegisters>, + registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { - let dev_cfg_raw: &'static Config = unsafe { - &*registers - .borrow_mut() - .as_mut_ptr() - .config() - .as_raw_ptr() - .cast::() - .as_ptr() - }; - let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); + let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = FsDevCfg { raw: dev_cfg_raw, dev_id, features: virtio::fs::F::empty(), }; - let isr_stat = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); - let notif_cfg = NotifCfg::new(registers.borrow_mut()); Ok(VirtioFsDriver { dev_cfg, - com_cfg: ComCfg::new(registers), - isr_stat, - notif_cfg, + caps_coll, vqueues: Vec::new(), }) } @@ -57,7 +42,7 @@ impl VirtioFsDriver { let mut drv = VirtioFsDriver::new(dev_id, registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::FsDriver)?; - drv.com_cfg.print_information(); + drv.caps_coll.com_cfg.print_information(); Ok(drv) } } diff --git a/src/drivers/fs/mod.rs b/src/drivers/fs/mod.rs index d0c3f71f68..c57f26d107 100644 --- a/src/drivers/fs/mod.rs +++ b/src/drivers/fs/mod.rs @@ -36,11 +36,7 @@ use crate::drivers::mmio::get_filesystem_driver; use crate::drivers::pci::get_filesystem_driver; use crate::drivers::virtio::ControlRegisters; use crate::drivers::virtio::error::VirtioFsInitError; -use crate::drivers::virtio::transport::InterruptCapability; -#[cfg(not(feature = "pci"))] -use crate::drivers::virtio::transport::mmio::{ComCfg, NotifCfg}; -#[cfg(feature = "pci")] -use crate::drivers::virtio::transport::pci::{ComCfg, NotifCfg}; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::error::VirtqError; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ @@ -67,9 +63,7 @@ pub(crate) struct FsDevCfg { #[allow(dead_code)] pub(crate) struct VirtioFsDriver { pub(super) dev_cfg: FsDevCfg, - pub(super) com_cfg: ComCfg, - pub(super) isr_stat: InterruptCapability, - pub(super) notif_cfg: NotifCfg, + pub(super) caps_coll: UniCapsColl, pub(super) vqueues: Vec, } @@ -82,7 +76,7 @@ impl VirtioFsDriver { #[cfg(feature = "pci")] pub fn set_failed(&mut self) { - self.com_cfg.set_failed(); + self.caps_coll.com_cfg.set_failed(); } /// Initializes the device in adherence to specification. Returns Some(VirtioFsError) @@ -96,16 +90,17 @@ impl VirtioFsDriver { irq: Option, ) -> Result<(), VirtioFsInitError> { // Reset - self.com_cfg.reset_dev(); + self.caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.com_cfg.ack_dev(); + self.caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.com_cfg.set_drv(); + self.caps_coll.com_cfg.set_drv(); let minimal_features = virtio::fs::F::VERSION_1; let negotiated_features = self + .caps_coll .com_cfg .control_registers() .negotiate_features(minimal_features); @@ -117,10 +112,10 @@ impl VirtioFsDriver { // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.com_cfg.features_ok(); + self.caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.com_cfg.check_features() { + if self.caps_coll.com_cfg.check_features() { info!( "Features have been negotiated between virtio filesystem device {:x} and driver.", self.dev_cfg.dev_id @@ -149,8 +144,8 @@ impl VirtioFsDriver { for i in 0..vqnum as u16 { let vq = VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, i, self.dev_cfg.features.into(), @@ -160,7 +155,7 @@ impl VirtioFsDriver { self.vqueues.push(vq); } - match &mut self.isr_stat { + match &mut self.caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -175,7 +170,7 @@ impl VirtioFsDriver { InterruptCapability::Msix(msix_table) => { use core::iter; - self.com_cfg.register_msix_vectors( + self.caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -190,7 +185,7 @@ impl VirtioFsDriver { } // At this point the device is "live" - self.com_cfg.drv_ok(); + self.caps_coll.com_cfg.drv_ok(); Ok(()) } @@ -200,7 +195,7 @@ impl VirtioFsDriver { not(all(feature = "pci", target_arch = "x86_64")), expect(irrefutable_let_patterns) )] - let InterruptCapability::IsrStatus(ref mut isr_stat) = self.isr_stat else { + let InterruptCapability::IsrStatus(ref mut isr_stat) = self.caps_coll.int_cap else { panic!("MSI-X vectors should be configured to the interrupt type-specific handlers.") }; @@ -216,7 +211,7 @@ impl VirtioFsDriver { } fn handle_device_configuration_interrupt(&mut self) { - if self.com_cfg.does_device_need_reset() { + if self.caps_coll.com_cfg.does_device_need_reset() { todo!("Device configuration change notification cannot be handled yet"); } } diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index 3b240faa0c..8ce7996723 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -32,13 +32,6 @@ impl VirtioFsDriver { ) -> Result { let device_id = device.device_id(); - let UniCapsColl { - com_cfg, - notif_cfg, - int_cap: isr_cfg, - .. - } = caps_coll; - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioFsDriver::map_cfg) else { error!("No dev config. Aborting!"); return Err(error::VirtioFsInitError::NoDevCfg(device_id)); @@ -46,9 +39,7 @@ impl VirtioFsDriver { Ok(VirtioFsDriver { dev_cfg, - com_cfg, - isr_stat: isr_cfg, - notif_cfg, + caps_coll, vqueues: Vec::new(), }) } diff --git a/src/drivers/net/virtio/mmio.rs b/src/drivers/net/virtio/mmio.rs index 6ea44b4e6f..1fbfe35c58 100644 --- a/src/drivers/net/virtio/mmio.rs +++ b/src/drivers/net/virtio/mmio.rs @@ -1,42 +1,28 @@ use smoltcp::phy::ChecksumCapabilities; -use virtio::mmio::{DeviceRegisters, DeviceRegistersVolatileFieldAccess}; +use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; use crate::drivers::net::virtio::{Init, NetDevCfg, Uninit, VirtioNetDriver}; use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::InterruptCapability; -use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg}; +use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio network driver impl VirtioNetDriver { pub fn new( dev_id: u16, - mut registers: VolatileRef<'static, DeviceRegisters>, + registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { - let dev_cfg_raw: &'static virtio::net::Config = unsafe { - &*registers - .borrow_mut() - .as_mut_ptr() - .config() - .as_raw_ptr() - .cast::() - .as_ptr() - }; - let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); + let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = NetDevCfg { raw: dev_cfg_raw, dev_id, features: virtio::net::F::empty(), }; - let isr_stat = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); - let notif_cfg = NotifCfg::new(registers.borrow_mut()); Ok(VirtioNetDriver { dev_cfg, - com_cfg: ComCfg::new(registers), - isr_stat, - notif_cfg, + caps_coll, inner: Uninit, num_vqs: 0, checksums: ChecksumCapabilities::default(), @@ -65,7 +51,7 @@ impl VirtioNetDriver { impl VirtioNetDriver { pub fn print_information(&mut self) { - self.com_cfg.print_information(); + self.caps_coll.com_cfg.print_information(); if self.dev_status() == virtio::net::S::LINK_UP { info!("The link of the network device is up!"); } diff --git a/src/drivers/net/virtio/mod.rs b/src/drivers/net/virtio/mod.rs index 83da40f739..fbda8d8116 100644 --- a/src/drivers/net/virtio/mod.rs +++ b/src/drivers/net/virtio/mod.rs @@ -37,11 +37,7 @@ use crate::config::VIRTIO_MAX_QUEUE_SIZE; use crate::drivers::net::virtio::constants::BUFF_PER_PACKET; use crate::drivers::net::{NetworkDriver, mtu}; use crate::drivers::virtio::ControlRegisters; -use crate::drivers::virtio::transport::InterruptCapability; -#[cfg(not(feature = "pci"))] -use crate::drivers::virtio::transport::mmio::{ComCfg, NotifCfg}; -#[cfg(feature = "pci")] -use crate::drivers::virtio::transport::pci::{ComCfg, NotifCfg}; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::packed::PackedVq; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ @@ -244,10 +240,7 @@ pub(crate) struct Init { /// the device itself. pub(crate) struct VirtioNetDriver { pub(super) dev_cfg: NetDevCfg, - pub(super) com_cfg: ComCfg, - pub(super) isr_stat: InterruptCapability, - pub(super) notif_cfg: NotifCfg, - + pub(super) caps_coll: UniCapsColl, pub(super) inner: T, pub(super) num_vqs: u16, @@ -519,7 +512,8 @@ impl NetworkDriver for VirtioNetDriver { /// If VIRTIO_NET_F_MAC is not set, the function panics currently! fn get_mac_address(&self) -> [u8; 6] { if self.dev_cfg.features.contains(virtio::net::F::MAC) { - self.com_cfg + self.caps_coll + .com_cfg .device_config_space() .read_config_with(|| self.dev_cfg.raw.as_ptr().mac().read()) } else { @@ -545,7 +539,7 @@ impl NetworkDriver for VirtioNetDriver { not(all(feature = "pci", target_arch = "x86_64")), expect(irrefutable_let_patterns) )] - let InterruptCapability::IsrStatus(ref mut isr_stat) = self.isr_stat else { + let InterruptCapability::IsrStatus(ref mut isr_stat) = self.caps_coll.int_cap else { panic!("MSI-X vectors should be configured to the interrupt type-specific handlers.") }; @@ -771,7 +765,7 @@ impl VirtioNetDriver { } pub(crate) fn handle_device_configuration_interrupt(&self) { - if self.com_cfg.does_device_need_reset() { + if self.caps_coll.com_cfg.does_device_need_reset() { todo!("Device configuration change notification cannot be handled yet"); } } @@ -789,13 +783,13 @@ impl VirtioNetDriver { irq: Option, ) -> Result, VirtioNetError> { // Reset - self.com_cfg.reset_dev(); + self.caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.com_cfg.ack_dev(); + self.caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.com_cfg.set_drv(); + self.caps_coll.com_cfg.set_drv(); let minimal_features = virtio::net::F::VERSION_1 | virtio::net::F::MAC; @@ -825,6 +819,7 @@ impl VirtioNetDriver { | virtio::net::F::GUEST_TSO6; let negotiated_features = self + .caps_coll .com_cfg .control_registers() .negotiate_features(features); @@ -836,10 +831,10 @@ impl VirtioNetDriver { // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.com_cfg.features_ok(); + self.caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.com_cfg.check_features() { + if self.caps_coll.com_cfg.check_features() { info!( "Features have been negotiated between virtio network device {:x} and driver.", self.dev_cfg.dev_id @@ -867,7 +862,7 @@ impl VirtioNetDriver { self.dev_cfg.dev_id ); - match &mut self.isr_stat { + match &mut self.caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers @@ -886,7 +881,7 @@ impl VirtioNetDriver { .features .contains(virtio::net::F::CTRL_VQ) .then_some(self.num_vqs); - self.com_cfg.register_msix_vectors( + self.caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, crate::executor::network::network_device_configuration_handler, @@ -897,7 +892,7 @@ impl VirtioNetDriver { } // At this point the device is "live" - self.com_cfg.drv_ok(); + self.caps_coll.com_cfg.drv_ok(); // Not only should we offload receive checksum validation to the device for performance when possible, we MUST // offload it when GUEST_TSO{4,6} are enabled, since otherwise the coalesced packets will be rejected by smoltcp @@ -918,9 +913,7 @@ impl VirtioNetDriver { Ok(VirtioNetDriver { dev_cfg: self.dev_cfg, - com_cfg: self.com_cfg, - isr_stat: self.isr_stat, - notif_cfg: self.notif_cfg, + caps_coll: self.caps_coll, inner, num_vqs: self.num_vqs, checksums: self.checksums, @@ -937,8 +930,8 @@ impl VirtioNetDriver { let mut ctrl_vq = if self.dev_cfg.features.contains(virtio::net::F::RING_PACKED) { VirtQueue::Packed( PackedVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, self.num_vqs, self.dev_cfg.features.into(), @@ -948,8 +941,8 @@ impl VirtioNetDriver { } else { VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, self.num_vqs, self.dev_cfg.features.into(), @@ -1013,8 +1006,8 @@ impl VirtioNetDriver { for i in 0..(self.num_vqs / 2) { if self.dev_cfg.features.contains(virtio::net::F::RING_PACKED) { let mut vq = PackedVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i, self.dev_cfg.features.into(), @@ -1026,8 +1019,8 @@ impl VirtioNetDriver { inner.recv_vqs.add(VirtQueue::Packed(vq)); let mut vq = PackedVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i + 1, self.dev_cfg.features.into(), @@ -1040,8 +1033,8 @@ impl VirtioNetDriver { inner.send_vqs.add(VirtQueue::Packed(vq)); } else { let mut vq = SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i, self.dev_cfg.features.into(), @@ -1053,8 +1046,8 @@ impl VirtioNetDriver { inner.recv_vqs.add(VirtQueue::Split(vq)); let mut vq = SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i + 1, self.dev_cfg.features.into(), diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index 3ef94f4565..7b6c2c9d1d 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -35,12 +35,6 @@ impl VirtioNetDriver { device: &PciDevice, ) -> Result { let device_id = device.device_id(); - let UniCapsColl { - com_cfg, - notif_cfg, - int_cap: isr_cfg, - .. - } = caps_coll; let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioNetDriver::map_cfg) else { error!("No dev config. Aborting!"); @@ -49,9 +43,7 @@ impl VirtioNetDriver { Ok(VirtioNetDriver { dev_cfg, - com_cfg, - isr_stat: isr_cfg, - notif_cfg, + caps_coll, inner: Uninit, num_vqs: 0, checksums: ChecksumCapabilities::default(), diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index a5bdd66ff7..8ea4992899 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -24,6 +24,7 @@ use crate::drivers::fs::VirtioFsDriver; #[cfg(feature = "virtio-net")] use crate::drivers::net::virtio::VirtioNetDriver; use crate::drivers::virtio::error::VirtioError; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::{ControlRegisters, VirtioIdExt}; #[cfg(feature = "virtio-vsock")] use crate::drivers::vsock::VirtioVsockDriver; @@ -413,3 +414,29 @@ pub(crate) fn init_device( } } } + +pub fn map_caps( + mut registers: VolatileRef<'static, DeviceRegisters>, +) -> (UniCapsColl, VolatileRef<'static, Config, ReadOnly>) { + let dev_cfg_raw: &'static Config = unsafe { + &*registers + .borrow_mut() + .as_mut_ptr() + .config() + .as_raw_ptr() + .cast::() + .as_ptr() + }; + let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); + let int_cap = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); + let notif_cfg = NotifCfg::new(registers.borrow_mut()); + + ( + UniCapsColl { + com_cfg: ComCfg::new(registers), + int_cap, + notif_cfg, + }, + dev_cfg_raw, + ) +} diff --git a/src/drivers/virtio/transport/mod.rs b/src/drivers/virtio/transport/mod.rs index 67c3a9617e..466f6d3b24 100644 --- a/src/drivers/virtio/transport/mod.rs +++ b/src/drivers/virtio/transport/mod.rs @@ -14,7 +14,6 @@ pub(crate) enum InterruptCapability { Msix(volatile::VolatileRef<'static, [crate::drivers::pci::msix::TableEntry]>), } -#[cfg_attr(not(feature = "pci"), expect(dead_code))] /// Universal Caplist Collections holds all universal capability structures for /// a given Virtio device. pub struct UniCapsColl { diff --git a/src/drivers/vsock/mmio.rs b/src/drivers/vsock/mmio.rs index 208a830914..de9a77cdf5 100644 --- a/src/drivers/vsock/mmio.rs +++ b/src/drivers/vsock/mmio.rs @@ -1,10 +1,8 @@ -use virtio::mmio::{DeviceRegisters, DeviceRegistersVolatileFieldAccess}; -use virtio::vsock::Config; +use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::InterruptCapability; -use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg}; +use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::vsock::{EventQueue, RxQueue, TxQueue, VirtioVsockDriver, VsockDevCfg}; use crate::drivers::{InterruptHandlerMap, InterruptLine}; @@ -12,31 +10,18 @@ use crate::drivers::{InterruptHandlerMap, InterruptLine}; impl VirtioVsockDriver { pub fn new( dev_id: u16, - mut registers: VolatileRef<'static, DeviceRegisters>, + registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { - let dev_cfg_raw: &'static Config = unsafe { - &*registers - .borrow_mut() - .as_mut_ptr() - .config() - .as_raw_ptr() - .cast::() - .as_ptr() - }; - let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); + let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = VsockDevCfg { raw: dev_cfg_raw, dev_id, features: virtio::vsock::F::empty(), }; - let isr_stat = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); - let notif_cfg = NotifCfg::new(registers.borrow_mut()); Ok(VirtioVsockDriver { dev_cfg, - com_cfg: ComCfg::new(registers), - isr_stat, - notif_cfg, + caps_coll, event_vq: EventQueue::new(), recv_vq: RxQueue::new(), send_vq: TxQueue::new(), @@ -57,7 +42,7 @@ impl VirtioVsockDriver { let mut drv = VirtioVsockDriver::new(dev_id, registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::VsockDriver)?; - drv.com_cfg.print_information(); + drv.caps_coll.com_cfg.print_information(); Ok(drv) } } diff --git a/src/drivers/vsock/mod.rs b/src/drivers/vsock/mod.rs index 22a6904a85..c013247152 100644 --- a/src/drivers/vsock/mod.rs +++ b/src/drivers/vsock/mod.rs @@ -26,11 +26,7 @@ use crate::drivers::mmio::get_vsock_driver; use crate::drivers::pci::get_vsock_driver; use crate::drivers::virtio::ControlRegisters; use crate::drivers::virtio::error::VirtioVsockError; -use crate::drivers::virtio::transport::InterruptCapability; -#[cfg(not(feature = "pci"))] -use crate::drivers::virtio::transport::mmio::{ComCfg, NotifCfg}; -#[cfg(feature = "pci")] -use crate::drivers::virtio::transport::pci::{ComCfg, NotifCfg}; +use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ AvailBufferToken, BufferElem, BufferType, UsedBufferToken, Virtq, @@ -262,9 +258,7 @@ pub(crate) struct VsockDevCfg { pub(crate) struct VirtioVsockDriver { pub(super) dev_cfg: VsockDevCfg, - pub(super) com_cfg: ComCfg, - pub(super) isr_stat: InterruptCapability, - pub(super) notif_cfg: NotifCfg, + pub(super) caps_coll: UniCapsColl, pub(super) event_vq: EventQueue, pub(super) recv_vq: RxQueue, @@ -290,7 +284,7 @@ impl VirtioVsockDriver { #[cfg(feature = "pci")] pub fn set_failed(&mut self) { - self.com_cfg.set_failed(); + self.caps_coll.com_cfg.set_failed(); } pub fn disable_interrupts(&mut self) { @@ -310,7 +304,7 @@ impl VirtioVsockDriver { not(all(feature = "pci", target_arch = "x86_64")), expect(irrefutable_let_patterns) )] - let InterruptCapability::IsrStatus(ref mut isr_stat) = self.isr_stat else { + let InterruptCapability::IsrStatus(ref mut isr_stat) = self.caps_coll.int_cap else { panic!("MSI-X vectors should be configured to the interrupt type-specific handlers.") }; @@ -327,7 +321,7 @@ impl VirtioVsockDriver { } fn handle_device_configuration_interrupt(&mut self) { - if self.com_cfg.does_device_need_reset() { + if self.caps_coll.com_cfg.does_device_need_reset() { todo!("Device configuration change notification cannot be handled yet"); } } @@ -343,16 +337,17 @@ impl VirtioVsockDriver { irq: Option, ) -> Result<(), VirtioVsockError> { // Reset - self.com_cfg.reset_dev(); + self.caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.com_cfg.ack_dev(); + self.caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.com_cfg.set_drv(); + self.caps_coll.com_cfg.set_drv(); let minimal_features = virtio::vsock::F::VERSION_1; let negotiated_features = self + .caps_coll .com_cfg .control_registers() .negotiate_features(minimal_features); @@ -364,10 +359,10 @@ impl VirtioVsockDriver { // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.com_cfg.features_ok(); + self.caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.com_cfg.check_features() { + if self.caps_coll.com_cfg.check_features() { info!( "Features have been negotiated between virtio socket device {:x} and driver.", self.dev_cfg.dev_id @@ -382,8 +377,8 @@ impl VirtioVsockDriver { // create the queues and tell device about them self.recv_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 0, self.dev_cfg.features.into(), @@ -395,8 +390,8 @@ impl VirtioVsockDriver { self.send_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 1, self.dev_cfg.features.into(), @@ -409,8 +404,8 @@ impl VirtioVsockDriver { // create the queues and tell device about them self.event_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.com_cfg, - &self.notif_cfg, + &mut self.caps_coll.com_cfg, + &self.caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2, self.dev_cfg.features.into(), @@ -418,7 +413,7 @@ impl VirtioVsockDriver { .unwrap(), )); - match &mut self.isr_stat { + match &mut self.caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -431,7 +426,7 @@ impl VirtioVsockDriver { } #[cfg(all(feature = "pci", target_arch = "x86_64"))] InterruptCapability::Msix(msix_table) => { - self.com_cfg.register_msix_vectors( + self.caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -451,7 +446,7 @@ impl VirtioVsockDriver { self.event_vq.enable_notifs(); // At this point the device is "live" - self.com_cfg.drv_ok(); + self.caps_coll.com_cfg.drv_ok(); Ok(()) } diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index fa20e4be24..eb4083e71a 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -32,13 +32,6 @@ impl VirtioVsockDriver { ) -> Result { let device_id = device.device_id(); - let UniCapsColl { - com_cfg, - notif_cfg, - int_cap: isr_cfg, - .. - } = caps_coll; - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioVsockDriver::map_cfg) else { error!("No dev config. Aborting!"); return Err(error::VirtioVsockError::NoDevCfg(device_id)); @@ -46,9 +39,7 @@ impl VirtioVsockDriver { Ok(VirtioVsockDriver { dev_cfg, - com_cfg, - isr_stat: isr_cfg, - notif_cfg, + caps_coll, event_vq: EventQueue::new(), recv_vq: RxQueue::new(), send_vq: TxQueue::new(), From db01ac4fd71f0c6c50189104a218347314ac2f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sat, 4 Jul 2026 20:44:20 +0200 Subject: [PATCH 3/9] refactor(virtio): remove device id from device configuration structure Device ID is a constant defined in the VIRTIO specification for a given device on a transport, so storing and printing it on error does not provide additional information to the user. --- src/drivers/console/mmio.rs | 5 +---- src/drivers/console/mod.rs | 14 +++++--------- src/drivers/console/pci.rs | 6 +----- src/drivers/fs/mmio.rs | 5 +---- src/drivers/fs/mod.rs | 19 +++++-------------- src/drivers/fs/pci.rs | 6 +----- src/drivers/net/virtio/mmio.rs | 9 ++------- src/drivers/net/virtio/mod.rs | 26 ++++++-------------------- src/drivers/net/virtio/pci.rs | 6 +----- src/drivers/virtio/transport/mmio.rs | 8 ++++---- src/drivers/virtio/transport/pci.rs | 23 +++++------------------ src/drivers/vsock/mmio.rs | 5 +---- src/drivers/vsock/mod.rs | 19 +++++-------------- src/drivers/vsock/pci.rs | 1 - 14 files changed, 38 insertions(+), 114 deletions(-) diff --git a/src/drivers/console/mmio.rs b/src/drivers/console/mmio.rs index 231688725e..5bdcaa37f1 100644 --- a/src/drivers/console/mmio.rs +++ b/src/drivers/console/mmio.rs @@ -9,13 +9,11 @@ use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { pub fn new( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = ConsoleDevCfg { raw: dev_cfg_raw, - dev_id, features: virtio::console::F::empty(), }; @@ -33,12 +31,11 @@ impl VirtioConsoleDriver { /// Returns a driver instance of /// [VirtioConsoleDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub fn init( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioConsoleDriver::new(dev_id, registers)?; + let mut drv = VirtioConsoleDriver::new(registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::ConsoleDriver)?; drv.caps_coll.com_cfg.print_information(); diff --git a/src/drivers/console/mod.rs b/src/drivers/console/mod.rs index 8b7d6fe912..db35e48e6b 100644 --- a/src/drivers/console/mod.rs +++ b/src/drivers/console/mod.rs @@ -250,7 +250,6 @@ impl TxQueue { /// for the driver. pub(crate) struct ConsoleDevCfg { pub raw: VolatileRef<'static, Config, ReadOnly>, - pub dev_id: u16, pub features: virtio::console::F, } @@ -335,7 +334,7 @@ impl VirtioConsoleDriver { 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)); + return Err(VirtioConsoleError::FailFeatureNeg); } // Indicates the device, that the current feature set is final for the driver @@ -344,15 +343,12 @@ impl VirtioConsoleDriver { // Checks if the device has accepted final set. This finishes feature negotiation. if self.caps_coll.com_cfg.check_features() { - info!( - "Features have been negotiated between virtio console device {:x} and driver.", - self.dev_cfg.dev_id - ); + info!("Features have been negotiated between virtio console device and driver.",); // Set feature set in device config fur future use. self.dev_cfg.features = negotiated_features; } else { error!("The device does not support our subset of features."); - return Err(VirtioConsoleError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioConsoleError::FailFeatureNeg); } // create the queues and tell device about them @@ -456,8 +452,8 @@ pub mod error { /// The device did not acknowledge the negotiated feature set. #[error( - "Virtio console device driver failed, for device {0:x}, device did not acknowledge negotiated feature set!" + "Virtio console device driver failed, device did not acknowledge negotiated feature set!" )] - FailFeatureNeg(u16), + FailFeatureNeg, } } diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index 79c5bd8b55..6800c8dc17 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -20,7 +20,6 @@ impl VirtioConsoleDriver { Some(ConsoleDevCfg { raw: dev_cfg, - dev_id: cap.dev_id(), features: virtio::console::F::empty(), }) } @@ -74,10 +73,7 @@ impl VirtioConsoleDriver { match drv.init_dev(handlers, device.get_irq()) { Ok(()) => { - info!( - "Console device with id {:x}, has been initialized by driver!", - drv.dev_cfg.dev_id - ); + info!("Console device has been initialized by driver!",); Ok(drv) } diff --git a/src/drivers/fs/mmio.rs b/src/drivers/fs/mmio.rs index e0dfdcb8eb..e1267109a1 100644 --- a/src/drivers/fs/mmio.rs +++ b/src/drivers/fs/mmio.rs @@ -11,13 +11,11 @@ use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio fs driver impl VirtioFsDriver { pub fn new( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = FsDevCfg { raw: dev_cfg_raw, - dev_id, features: virtio::fs::F::empty(), }; @@ -34,12 +32,11 @@ impl VirtioFsDriver { /// Returns a driver instance of /// [VirtioFsDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub fn init( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioFsDriver::new(dev_id, registers)?; + let mut drv = VirtioFsDriver::new(registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::FsDriver)?; drv.caps_coll.com_cfg.print_information(); diff --git a/src/drivers/fs/mod.rs b/src/drivers/fs/mod.rs index c57f26d107..292b5adec9 100644 --- a/src/drivers/fs/mod.rs +++ b/src/drivers/fs/mod.rs @@ -52,7 +52,6 @@ use crate::mm::device_alloc::DeviceAlloc; /// for the driver. pub(crate) struct FsDevCfg { pub raw: VolatileRef<'static, virtio::fs::Config, ReadOnly>, - pub dev_id: u16, pub features: virtio::fs::F, } @@ -69,11 +68,6 @@ pub(crate) struct VirtioFsDriver { // Backend-independent interface for Virtio network driver impl VirtioFsDriver { - #[cfg(feature = "pci")] - pub fn get_dev_id(&self) -> u16 { - self.dev_cfg.dev_id - } - #[cfg(feature = "pci")] pub fn set_failed(&mut self) { self.caps_coll.com_cfg.set_failed(); @@ -107,7 +101,7 @@ impl VirtioFsDriver { if !negotiated_features.contains(minimal_features) { error!("Device features set, does not satisfy minimal features needed. Aborting!"); - return Err(VirtioFsInitError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioFsInitError::FailFeatureNeg); } // Indicates the device, that the current feature set is final for the driver @@ -116,15 +110,12 @@ impl VirtioFsDriver { // Checks if the device has accepted final set. This finishes feature negotiation. if self.caps_coll.com_cfg.check_features() { - info!( - "Features have been negotiated between virtio filesystem device {:x} and driver.", - self.dev_cfg.dev_id - ); + info!("Features have been negotiated between virtio filesystem device and driver.",); // Set feature set in device config fur future use. self.dev_cfg.features = negotiated_features; } else { error!("The device does not support our subset of features."); - return Err(VirtioFsInitError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioFsInitError::FailFeatureNeg); } // 1 highprio queue, and n normal request queues @@ -324,9 +315,9 @@ pub mod error { NoDevCfg(u16), #[error( - "Virtio filesystem driver failed, for device {0:x}, device did not acknowledge negotiated feature set!" + "Virtio filesystem driver failed, device did not acknowledge negotiated feature set!" )] - FailFeatureNeg(u16), + FailFeatureNeg, #[error("Virtio filesystem failed, driver failed due unknown reason!")] Unknown, diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index 8ce7996723..e2fdbb5681 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -18,7 +18,6 @@ impl VirtioFsDriver { Some(FsDevCfg { raw: dev_cfg, - dev_id: cap.dev_id(), features: virtio::fs::F::empty(), }) } @@ -64,10 +63,7 @@ impl VirtioFsDriver { }; match drv.init_dev(handlers, device.get_irq()) { - Ok(()) => info!( - "Filesystem device with id {:x}, has been initialized by driver!", - drv.get_dev_id() - ), + Ok(()) => info!("Filesystem device has been initialized by driver!",), Err(fs_err) => { drv.set_failed(); return Err(VirtioError::FsDriver(fs_err)); diff --git a/src/drivers/net/virtio/mmio.rs b/src/drivers/net/virtio/mmio.rs index 1fbfe35c58..91ea93a4e7 100644 --- a/src/drivers/net/virtio/mmio.rs +++ b/src/drivers/net/virtio/mmio.rs @@ -9,14 +9,10 @@ use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio network driver impl VirtioNetDriver { - pub fn new( - dev_id: u16, - registers: VolatileRef<'static, DeviceRegisters>, - ) -> Result { + pub fn new(registers: VolatileRef<'static, DeviceRegisters>) -> Result { let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = NetDevCfg { raw: dev_cfg_raw, - dev_id, features: virtio::net::F::empty(), }; @@ -35,12 +31,11 @@ impl VirtioNetDriver { /// Returns a driver instance of /// [VirtioNetDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub fn init( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result, VirtioError> { - let drv = VirtioNetDriver::new(dev_id, registers)?; + let drv = VirtioNetDriver::new(registers)?; let mut drv = drv .init_dev(handlers, Some(irq)) .map_err(VirtioError::NetDriver)?; diff --git a/src/drivers/net/virtio/mod.rs b/src/drivers/net/virtio/mod.rs index fbda8d8116..b9462f4259 100644 --- a/src/drivers/net/virtio/mod.rs +++ b/src/drivers/net/virtio/mod.rs @@ -52,7 +52,6 @@ use crate::mm::device_alloc::DeviceAlloc; /// for the driver. pub(crate) struct NetDevCfg { pub raw: VolatileRef<'static, virtio::net::Config, ReadOnly>, - pub dev_id: u16, pub features: virtio::net::F, } @@ -624,11 +623,6 @@ impl Driver for VirtioNetDriver { // Backend-independent interface for Virtio network driver impl VirtioNetDriver { - #[cfg(feature = "pci")] - pub fn get_dev_id(&self) -> u16 { - self.dev_cfg.dev_id - } - /// Returns the current status of the device, if VIRTIO_NET_F_STATUS /// has been negotiated. Otherwise assumes an active device. #[cfg(not(feature = "pci"))] @@ -826,7 +820,7 @@ impl VirtioNetDriver { if !negotiated_features.contains(minimal_features) { error!("Device features set, does not satisfy minimal features needed. Aborting!"); - return Err(VirtioNetError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioNetError::FailFeatureNeg); } // Indicates the device, that the current feature set is final for the driver @@ -835,15 +829,12 @@ impl VirtioNetDriver { // Checks if the device has accepted final set. This finishes feature negotiation. if self.caps_coll.com_cfg.check_features() { - info!( - "Features have been negotiated between virtio network device {:x} and driver.", - self.dev_cfg.dev_id - ); + info!("Features have been negotiated between virtio network device and driver.",); // Set feature set in device config fur future use. self.dev_cfg.features = negotiated_features; } else { error!("The device does not support our subset of features."); - return Err(VirtioNetError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioNetError::FailFeatureNeg); } let mut inner = Init { @@ -857,10 +848,7 @@ impl VirtioNetDriver { debug!("Using RX buffer size of {}", inner.recv_vqs.buf_size); self.dev_spec_init(&mut inner)?; - info!( - "Device specific initialization for Virtio network device {:x} finished", - self.dev_cfg.dev_id - ); + info!("Device specific initialization for Virtio network device finished",); match &mut self.caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { @@ -1084,10 +1072,8 @@ pub mod error { )] NoDevCfg(u16), - #[error( - "Virtio network driver failed, for device {0:x}, device did not acknowledge negotiated feature set!" - )] - FailFeatureNeg(u16), + #[error("Virtio network driver failed, device did not acknowledge negotiated feature set!")] + FailFeatureNeg, } } diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index 7b6c2c9d1d..4f189f1474 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -22,7 +22,6 @@ impl VirtioNetDriver { Some(NetDevCfg { raw: dev_cfg, - dev_id: cap.dev_id(), features: virtio::net::F::empty(), }) } @@ -81,10 +80,7 @@ impl VirtioNetDriver { let initialized_drv = match drv.init_dev(handlers, device.get_irq()) { Ok(initialized_drv) => { - info!( - "Network device with id {:x}, has been initialized by driver!", - initialized_drv.get_dev_id() - ); + info!("Network device has been initialized by driver!",); initialized_drv } Err(vnet_err) => { diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 8ea4992899..095c407d31 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -350,7 +350,7 @@ pub(crate) fn init_device( // Verify the device-ID to find the network card match registers.as_ptr().device_id().read() { #[cfg(feature = "virtio-console")] - virtio::Id::Console => match VirtioConsoleDriver::init(dev_id, registers, irq_no, handlers) { + virtio::Id::Console => match VirtioConsoleDriver::init(registers, irq_no, handlers) { Ok(virt_console_drv) => { info!("Virtio console driver initialized."); Ok(VirtioDriver::Console(alloc::boxed::Box::new( @@ -366,7 +366,7 @@ pub(crate) fn init_device( virtio::Id::Fs => { // TODO: check subclass // TODO: proper error handling on driver creation fail - match VirtioFsDriver::init(dev_id, registers, irq_no, handlers) { + match VirtioFsDriver::init(registers, irq_no, handlers) { Ok(virt_fs_drv) => { info!("Virtio filesystem driver initialized."); Ok(VirtioDriver::Fs(alloc::boxed::Box::new(virt_fs_drv))) @@ -378,7 +378,7 @@ pub(crate) fn init_device( } } #[cfg(feature = "virtio-net")] - virtio::Id::Net => match VirtioNetDriver::init(dev_id, registers, irq_no, handlers) { + virtio::Id::Net => match VirtioNetDriver::init(registers, irq_no, handlers) { Ok(virt_net_drv) => { info!("Virtio network driver initialized."); Ok(VirtioDriver::Net(alloc::boxed::Box::new(virt_net_drv))) @@ -389,7 +389,7 @@ pub(crate) fn init_device( } }, #[cfg(feature = "virtio-vsock")] - virtio::Id::Vsock => match VirtioVsockDriver::init(dev_id, registers, irq_no, handlers) { + virtio::Id::Vsock => match VirtioVsockDriver::init(registers, irq_no, handlers) { Ok(virt_vsock_drv) => { info!("Virtio sock driver initialized."); Ok(VirtioDriver::Vsock(alloc::boxed::Box::new(virt_vsock_drv))) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 47e36f3a21..aaeb2035fd 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -53,18 +53,14 @@ pub fn map_dev_cfg(cap: &PciCap) -> Option<&'static mut T> { }; if cap.bar_len() < cap.len() + cap.offset() { - error!( - "Device config of device {:x}, does not fit into memory specified by bar!", - cap.dev_id(), - ); + error!("Device config of device does not fit into memory specified by bar!",); return None; } // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 if cap.len() < u64::try_from(size_of::()).unwrap() { error!( - "Device specific config from device {:x}, does not represent actual structure specified by the standard!", - cap.dev_id() + "Device specific config from device does not represent actual structure specified by the standard!", ); return None; } @@ -93,7 +89,6 @@ pub fn map_dev_cfg(cap: &PciCap) -> Option<&'static mut T> { #[derive(Clone)] pub struct PciCap { bar: PciBar, - dev_id: u16, cap: CapData, } @@ -114,17 +109,12 @@ impl PciCap { self.bar.mem_addr } - pub fn dev_id(&self) -> u16 { - self.dev_id - } - /// Returns a reference to the actual structure inside the PCI devices memory space. fn map_common_cfg(&self) -> Option> { if self.bar.length < self.len() + self.offset() { - let dev_id = self.dev_id; let index = self.bar.index; error!( - "Common config of the capability of device {dev_id:x} does not fit into memory specified by bar {index:x}!" + "Common config of the capability of device does not fit into memory specified by bar {index:x}!" ); return None; } @@ -150,10 +140,9 @@ impl PciCap { fn map_isr_status(&self) -> Option> { if self.bar.length < self.len() + self.offset() { - let dev_id = self.dev_id; let index = self.bar.index; error!( - "ISR status config of device {dev_id:x}, does not fit into memory specified by bar {index:x}!" + "ISR status config of device does not fit into memory specified by bar {index:x}!" ); return None; } @@ -460,10 +449,9 @@ pub struct NotifCfg { impl NotifCfg { fn new(cap: &PciCap) -> Option { if cap.bar.length < cap.len() + cap.offset() { - let dev_id = cap.dev_id; let index = cap.bar.index; error!( - "Notification config of device {dev_id:x}, does not fit into memory specified by bar {index:x}!" + "Notification config of device does not fit into memory specified by bar {index:x}!" ); return None; } @@ -632,7 +620,6 @@ pub(crate) fn map_caps( }; let pci_cap = PciCap { bar: VirtioPciBar::new(slot, addr.as_u64(), size.try_into().unwrap()), - dev_id: device_id, cap, }; match pci_cap.cap.cfg_type { diff --git a/src/drivers/vsock/mmio.rs b/src/drivers/vsock/mmio.rs index de9a77cdf5..fe891cb72e 100644 --- a/src/drivers/vsock/mmio.rs +++ b/src/drivers/vsock/mmio.rs @@ -9,13 +9,11 @@ use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio vsock driver impl VirtioVsockDriver { pub fn new( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, ) -> Result { let (caps_coll, dev_cfg_raw) = map_caps(registers); let dev_cfg = VsockDevCfg { raw: dev_cfg_raw, - dev_id, features: virtio::vsock::F::empty(), }; @@ -34,12 +32,11 @@ impl VirtioVsockDriver { /// Returns a driver instance of /// [VirtioVsockDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub fn init( - dev_id: u16, registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioVsockDriver::new(dev_id, registers)?; + let mut drv = VirtioVsockDriver::new(registers)?; drv.init_dev(handlers, Some(irq)) .map_err(VirtioError::VsockDriver)?; drv.caps_coll.com_cfg.print_information(); diff --git a/src/drivers/vsock/mod.rs b/src/drivers/vsock/mod.rs index c013247152..7391e6f196 100644 --- a/src/drivers/vsock/mod.rs +++ b/src/drivers/vsock/mod.rs @@ -252,7 +252,6 @@ impl EventQueue { /// for the driver. pub(crate) struct VsockDevCfg { pub raw: VolatileRef<'static, virtio::vsock::Config, ReadOnly>, - pub dev_id: u16, pub features: virtio::vsock::F, } @@ -272,11 +271,6 @@ impl Driver for VirtioVsockDriver { } impl VirtioVsockDriver { - #[cfg(feature = "pci")] - pub fn get_dev_id(&self) -> u16 { - self.dev_cfg.dev_id - } - #[inline] pub fn get_cid(&self) -> u64 { self.dev_cfg.raw.as_ptr().guest_cid().read().to_ne() @@ -354,7 +348,7 @@ impl VirtioVsockDriver { if !negotiated_features.contains(minimal_features) { error!("Device features set, does not satisfy minimal features needed. Aborting!"); - return Err(VirtioVsockError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioVsockError::FailFeatureNeg); } // Indicates the device, that the current feature set is final for the driver @@ -363,15 +357,12 @@ impl VirtioVsockDriver { // Checks if the device has accepted final set. This finishes feature negotiation. if self.caps_coll.com_cfg.check_features() { - info!( - "Features have been negotiated between virtio socket device {:x} and driver.", - self.dev_cfg.dev_id - ); + info!("Features have been negotiated between virtio socket device and driver.",); // Set feature set in device config fur future use. self.dev_cfg.features = negotiated_features; } else { error!("The device does not support our subset of features."); - return Err(VirtioVsockError::FailFeatureNeg(self.dev_cfg.dev_id)); + return Err(VirtioVsockError::FailFeatureNeg); } // create the queues and tell device about them @@ -499,8 +490,8 @@ pub mod error { NoNotifCfg(u16), #[error( - "Virtio socket device driver failed, for device {0:x}, device did not acknowledge negotiated feature set!" + "Virtio socket device driver failed, device did not acknowledge negotiated feature set!" )] - FailFeatureNeg(u16), + FailFeatureNeg, } } diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index eb4083e71a..9b689c20f8 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -18,7 +18,6 @@ impl VirtioVsockDriver { Some(VsockDevCfg { raw: dev_cfg, - dev_id: cap.dev_id(), features: virtio::vsock::F::empty(), }) } From ec0de1bdb3058f5d76f067541e55191725472c76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sat, 4 Jul 2026 23:46:41 +0200 Subject: [PATCH 4/9] refactor(virtio/pci): avoid nesting in device init --- src/drivers/console/pci.rs | 22 +++++++--------------- src/drivers/fs/pci.rs | 28 +++++++++++----------------- src/drivers/net/virtio/pci.rs | 33 ++++++++++----------------------- src/drivers/vsock/pci.rs | 19 ++++++------------- 4 files changed, 34 insertions(+), 68 deletions(-) diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index 6800c8dc17..1fb5fdd00e 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -55,21 +55,13 @@ impl VirtioConsoleDriver { // enable bus master mode device.set_command(CommandRegister::BUS_MASTER_ENABLE); - let mut drv = match pci::map_caps(device) { - Ok((caps, dev_cfg_list)) => { - match VirtioConsoleDriver::new(caps, dev_cfg_list, device) { - Ok(driver) => driver, - Err(console_err) => { - error!("Initializing new virtio console device driver failed. Aborting!"); - return Err(VirtioError::ConsoleDriver(console_err)); - } - } - } - Err(err) => { - error!("Mapping capabilities failed. Aborting!"); - return Err(err); - } - }; + let (caps, dev_cfg_list) = pci::map_caps(device) + .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; + let mut drv = + VirtioConsoleDriver::new(caps, dev_cfg_list, device).map_err(|console_err| { + error!("Initializing new virtio console device driver failed. Aborting!"); + VirtioError::ConsoleDriver(console_err) + })?; match drv.init_dev(handlers, device.get_irq()) { Ok(()) => { diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index e2fdbb5681..d1f6bec5d6 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -48,28 +48,22 @@ impl VirtioFsDriver { device: &PciDevice, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = match pci::map_caps(device) { - Ok((caps, dev_cfg_list)) => match VirtioFsDriver::new(caps, dev_cfg_list, device) { - Ok(driver) => driver, - Err(fs_err) => { - error!("Initializing new network driver failed. Aborting!"); - return Err(VirtioError::FsDriver(fs_err)); - } - }, - Err(err) => { - error!("Mapping capabilities failed. Aborting!"); - return Err(err); - } - }; + let (caps, dev_cfg_list) = pci::map_caps(device) + .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; + let mut drv = VirtioFsDriver::new(caps, dev_cfg_list, device).map_err(|fs_err| { + error!("Initializing new network driver failed. Aborting!"); + VirtioError::FsDriver(fs_err) + })?; match drv.init_dev(handlers, device.get_irq()) { - Ok(()) => info!("Filesystem device has been initialized by driver!",), + Ok(()) => { + info!("Filesystem device has been initialized by driver!",); + Ok(drv) + } Err(fs_err) => { drv.set_failed(); - return Err(VirtioError::FsDriver(fs_err)); + Err(VirtioError::FsDriver(fs_err)) } } - - Ok(drv) } } diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index 4f189f1474..f8693c7d78 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -63,30 +63,17 @@ impl VirtioNetDriver { ) -> Result, VirtioError> { // enable bus master mode device.set_command(CommandRegister::BUS_MASTER_ENABLE); + let (caps, dev_cfg_list) = pci::map_caps(device) + .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; + let drv = VirtioNetDriver::new(caps, dev_cfg_list, device).map_err(|vnet_err| { + error!("Initializing new network driver failed. Aborting!"); + VirtioError::NetDriver(vnet_err) + })?; - let drv = match pci::map_caps(device) { - Ok((caps, dev_cfg_list)) => match VirtioNetDriver::new(caps, dev_cfg_list, device) { - Ok(driver) => driver, - Err(vnet_err) => { - error!("Initializing new network driver failed. Aborting!"); - return Err(VirtioError::NetDriver(vnet_err)); - } - }, - Err(err) => { - error!("Mapping capabilities failed. Aborting!"); - return Err(err); - } - }; - - let initialized_drv = match drv.init_dev(handlers, device.get_irq()) { - Ok(initialized_drv) => { - info!("Network device has been initialized by driver!",); - initialized_drv - } - Err(vnet_err) => { - return Err(VirtioError::NetDriver(vnet_err)); - } - }; + let initialized_drv = drv + .init_dev(handlers, device.get_irq()) + .map_err(VirtioError::NetDriver)?; + info!("Network device has been initialized by driver!",); if initialized_drv.is_link_up() { info!("Virtio-net link is up after initialization."); diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index 9b689c20f8..15df0733cb 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -52,19 +52,12 @@ impl VirtioVsockDriver { device: &PciDevice, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = match pci::map_caps(device) { - Ok((caps, dev_cfg_list)) => match VirtioVsockDriver::new(caps, dev_cfg_list, device) { - Ok(driver) => driver, - Err(vsock_err) => { - error!("Initializing new virtio socket device driver failed. Aborting!"); - return Err(VirtioError::VsockDriver(vsock_err)); - } - }, - Err(err) => { - error!("Mapping capabilities failed. Aborting!"); - return Err(err); - } - }; + let (caps, dev_cfg_list) = pci::map_caps(device) + .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; + let mut drv = VirtioVsockDriver::new(caps, dev_cfg_list, device).map_err(|vsock_err| { + error!("Initializing new virtio socket device driver failed. Aborting!"); + VirtioError::VsockDriver(vsock_err) + })?; match drv.init_dev(handlers, device.get_irq()) { Ok(()) => { From 4a62bd7404f302ef7b2f59ac6d5e672734713dce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sun, 5 Jul 2026 01:03:16 +0200 Subject: [PATCH 5/9] refactor(virtio): do not create half-initialized driver structures Structures initialized with placeholder values can allow incorrect code that reads them before they are actually filled in. This previously caused bugs in virtio-net. For virtio-net, our solution was to work around it by making the structure generic over its initialization stage. This approach, however, causes the way the driver is initialized to differ in structure from how the other drivers are initialized. This is an issue for us because it prevents making more parts of the driver initialization common between the different device types. --- src/drivers/console/mmio.rs | 24 +-- src/drivers/console/mod.rs | 72 +++++---- src/drivers/console/pci.rs | 58 ++----- src/drivers/fs/mmio.rs | 31 +--- src/drivers/fs/mod.rs | 68 ++++---- src/drivers/fs/pci.rs | 65 +++----- src/drivers/net/virtio/mmio.rs | 29 +--- src/drivers/net/virtio/mod.rs | 276 ++++++++++++++++----------------- src/drivers/net/virtio/pci.rs | 65 +++----- src/drivers/vsock/mmio.rs | 25 +-- src/drivers/vsock/mod.rs | 85 +++++----- src/drivers/vsock/pci.rs | 67 +++----- 12 files changed, 353 insertions(+), 512 deletions(-) diff --git a/src/drivers/console/mmio.rs b/src/drivers/console/mmio.rs index 5bdcaa37f1..48848c8765 100644 --- a/src/drivers/console/mmio.rs +++ b/src/drivers/console/mmio.rs @@ -1,30 +1,13 @@ use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; -use crate::drivers::console::{ConsoleDevCfg, RxQueue, TxQueue, VirtioConsoleDriver}; +use crate::drivers::console::VirtioConsoleDriver; use crate::drivers::virtio::error::VirtioError; use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { - pub fn new( - registers: VolatileRef<'static, DeviceRegisters>, - ) -> Result { - let (caps_coll, dev_cfg_raw) = map_caps(registers); - let dev_cfg = ConsoleDevCfg { - raw: dev_cfg_raw, - features: virtio::console::F::empty(), - }; - - Ok(VirtioConsoleDriver { - dev_cfg, - caps_coll, - recv_vq: RxQueue::new(), - send_vq: TxQueue::new(), - }) - } - /// Initializes virtio console device by mapping configuration layout to /// respective structs (configuration structs are: /// @@ -35,9 +18,8 @@ impl VirtioConsoleDriver { irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioConsoleDriver::new(registers)?; - drv.init_dev(handlers, Some(irq)) - .map_err(VirtioError::ConsoleDriver)?; + let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) + .map_err(|(err, _)| VirtioError::ConsoleDriver(err))?; drv.caps_coll.com_cfg.print_information(); Ok(drv) } diff --git a/src/drivers/console/mod.rs b/src/drivers/console/mod.rs index db35e48e6b..391a2013be 100644 --- a/src/drivers/console/mod.rs +++ b/src/drivers/console/mod.rs @@ -306,79 +306,80 @@ impl VirtioConsoleDriver { crate::console::CONSOLE_WAKER.lock().wake(); } - #[cfg(feature = "pci")] - pub fn set_failed(&mut self) { - self.caps_coll.com_cfg.set_failed(); - } - pub fn init_dev( - &mut self, + (mut caps_coll, dev_cfg_raw): (UniCapsColl, VolatileRef<'static, Config, ReadOnly>), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result<(), VirtioConsoleError> { + ) -> Result { // Reset - self.caps_coll.com_cfg.reset_dev(); + caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.caps_coll.com_cfg.ack_dev(); + caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.caps_coll.com_cfg.set_drv(); + caps_coll.com_cfg.set_drv(); let minimal_features = virtio::console::F::VERSION_1; - let negotiated_features = self - .caps_coll + let negotiated_features = caps_coll .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); + return Err((VirtioConsoleError::FailFeatureNeg, caps_coll)); } // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.caps_coll.com_cfg.features_ok(); + caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.caps_coll.com_cfg.check_features() { + let dev_cfg = if caps_coll.com_cfg.check_features() { info!("Features have been negotiated between virtio console device and driver.",); // Set feature set in device config fur future use. - self.dev_cfg.features = negotiated_features; + ConsoleDevCfg { + raw: dev_cfg_raw, + features: negotiated_features, + } } else { error!("The device does not support our subset of features."); - return Err(VirtioConsoleError::FailFeatureNeg); - } + return Err((VirtioConsoleError::FailFeatureNeg, caps_coll)); + }; + + let mut recv_vq = RxQueue::new(); // create the queues and tell device about them - self.recv_vq.add(VirtQueue::Split( + recv_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 0, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), )); // Interrupt for receiving packets is wanted - self.recv_vq.enable_notifs(); + recv_vq.enable_notifs(); + + let mut send_vq = TxQueue::new(); - self.send_vq.add(VirtQueue::Split( + send_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 1, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), )); // Interrupt for communicating that a sent packet left, is not needed - self.send_vq.disable_notifs(); + send_vq.disable_notifs(); - match &mut self.caps_coll.int_cap { + match &mut caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -390,7 +391,7 @@ impl VirtioConsoleDriver { info!("Virtio interrupt handler at line {irq}"); } #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => self.caps_coll.com_cfg.register_msix_vectors( + InterruptCapability::Msix(msix_table) => caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -404,9 +405,14 @@ impl VirtioConsoleDriver { } // At this point the device is "live" - self.caps_coll.com_cfg.drv_ok(); - - Ok(()) + caps_coll.com_cfg.drv_ok(); + + Ok(Self { + dev_cfg, + caps_coll, + recv_vq, + send_vq, + }) } } diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index 1fb5fdd00e..6dd1055e4c 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -1,51 +1,23 @@ -use alloc::vec::Vec; - use pci_types::CommandRegister; use virtio::console::Config; use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; -use crate::drivers::console::{ConsoleDevCfg, RxQueue, TxQueue, VirtioConsoleDriver}; +use crate::drivers::console::VirtioConsoleDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::UniCapsColl; use crate::drivers::virtio::transport::pci::{self, PciCap}; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { - fn map_cfg(cap: &PciCap) -> Option { - let dev_cfg = pci::map_dev_cfg::(cap)?; - let dev_cfg = VolatileRef::from_ref(dev_cfg); - - Some(ConsoleDevCfg { - raw: dev_cfg, - features: virtio::console::F::empty(), - }) + fn map_cfg(cap: &PciCap) -> Option> { + let dev_cfg = pci::map_dev_cfg(cap)?; + Some(VolatileRef::from_ref(dev_cfg)) } - /// Instantiates a new VirtioConsoleDriver struct, by checking the available - /// configuration structures and moving them into the struct. - pub fn new( - caps_coll: UniCapsColl, - dev_cfg_list: Vec, - device: &PciDevice, - ) -> Result { - let device_id = device.device_id(); - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioConsoleDriver::map_cfg) else { - error!("No dev config. Aborting!"); - return Err(error::VirtioConsoleError::NoDevCfg(device_id)); - }; - - Ok(VirtioConsoleDriver { - dev_cfg, - caps_coll, - recv_vq: RxQueue::new(), - send_vq: TxQueue::new(), - }) - } - - /// Initializes virtio console device + /// Initializes virtio console device by checking the available + /// configuration structures and calling the initializer on them. /// /// Returns a driver instance of VirtioConsoleDriver. pub(crate) fn init( @@ -57,20 +29,24 @@ impl VirtioConsoleDriver { let (caps, dev_cfg_list) = pci::map_caps(device) .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - let mut drv = - VirtioConsoleDriver::new(caps, dev_cfg_list, device).map_err(|console_err| { + + let dev_cfg = dev_cfg_list + .iter() + .find_map(VirtioConsoleDriver::map_cfg) + .ok_or_else(|| { + error!("No dev config. Aborting!"); error!("Initializing new virtio console device driver failed. Aborting!"); - VirtioError::ConsoleDriver(console_err) + VirtioError::ConsoleDriver(error::VirtioConsoleError::NoDevCfg(device.device_id())) })?; - match drv.init_dev(handlers, device.get_irq()) { - Ok(()) => { + match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { + Ok(drv) => { info!("Console device has been initialized by driver!",); Ok(drv) } - Err(console_err) => { - drv.set_failed(); + Err((console_err, mut caps_coll)) => { + caps_coll.com_cfg.set_failed(); Err(VirtioError::ConsoleDriver(console_err)) } } diff --git a/src/drivers/fs/mmio.rs b/src/drivers/fs/mmio.rs index e1267109a1..a5f1d7059c 100644 --- a/src/drivers/fs/mmio.rs +++ b/src/drivers/fs/mmio.rs @@ -1,31 +1,13 @@ -use alloc::vec::Vec; - use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; -use crate::drivers::fs::{FsDevCfg, VirtioFsDriver}; +use crate::drivers::fs::VirtioFsDriver; use crate::drivers::virtio::error::VirtioError; use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio fs driver impl VirtioFsDriver { - pub fn new( - registers: VolatileRef<'static, DeviceRegisters>, - ) -> Result { - let (caps_coll, dev_cfg_raw) = map_caps(registers); - let dev_cfg = FsDevCfg { - raw: dev_cfg_raw, - features: virtio::fs::F::empty(), - }; - - Ok(VirtioFsDriver { - dev_cfg, - caps_coll, - vqueues: Vec::new(), - }) - } - /// Initializes virtio fs device by mapping configuration layout to /// respective structs (configuration structs are: /// @@ -36,10 +18,13 @@ impl VirtioFsDriver { irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioFsDriver::new(registers)?; - drv.init_dev(handlers, Some(irq)) - .map_err(VirtioError::FsDriver)?; - drv.caps_coll.com_cfg.print_information(); + let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) + .map_err(|(err, _)| VirtioError::FsDriver(err))?; + drv.print_information(); Ok(drv) } + + fn print_information(&mut self) { + self.caps_coll.com_cfg.print_information(); + } } diff --git a/src/drivers/fs/mod.rs b/src/drivers/fs/mod.rs index 292b5adec9..9b9fe74c9a 100644 --- a/src/drivers/fs/mod.rs +++ b/src/drivers/fs/mod.rs @@ -68,85 +68,81 @@ pub(crate) struct VirtioFsDriver { // Backend-independent interface for Virtio network driver impl VirtioFsDriver { - #[cfg(feature = "pci")] - pub fn set_failed(&mut self) { - self.caps_coll.com_cfg.set_failed(); - } - /// Initializes the device in adherence to specification. Returns Some(VirtioFsError) /// upon failure and None in case everything worked as expected. /// /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.11.5 pub(crate) fn init_dev( - &mut self, + (mut caps_coll, dev_cfg_raw): ( + UniCapsColl, + VolatileRef<'static, virtio::fs::Config, ReadOnly>, + ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result<(), VirtioFsInitError> { + ) -> Result { // Reset - self.caps_coll.com_cfg.reset_dev(); + caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.caps_coll.com_cfg.ack_dev(); + caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.caps_coll.com_cfg.set_drv(); + caps_coll.com_cfg.set_drv(); let minimal_features = virtio::fs::F::VERSION_1; - let negotiated_features = self - .caps_coll + let negotiated_features = caps_coll .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(VirtioFsInitError::FailFeatureNeg); + return Err((VirtioFsInitError::FailFeatureNeg, caps_coll)); } // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.caps_coll.com_cfg.features_ok(); + caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.caps_coll.com_cfg.check_features() { + let dev_cfg = if caps_coll.com_cfg.check_features() { info!("Features have been negotiated between virtio filesystem device and driver.",); // Set feature set in device config fur future use. - self.dev_cfg.features = negotiated_features; + FsDevCfg { + raw: dev_cfg_raw, + features: negotiated_features, + } } else { error!("The device does not support our subset of features."); - return Err(VirtioFsInitError::FailFeatureNeg); - } + return Err((VirtioFsInitError::FailFeatureNeg, caps_coll)); + }; // 1 highprio queue, and n normal request queues - let vqnum = self - .dev_cfg - .raw - .as_ptr() - .num_request_queues() - .read() - .to_ne() + 1; + let vqnum = dev_cfg.raw.as_ptr().num_request_queues().read().to_ne() + 1; if vqnum == 0 { error!("0 request queues requested from device. Aborting!"); - return Err(VirtioFsInitError::Unknown); + return Err((VirtioFsInitError::Unknown, caps_coll)); } + let mut vqueues = Vec::new(); + // create the queues and tell device about them for i in 0..vqnum as u16 { let vq = VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, i, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), ); - self.vqueues.push(vq); + vqueues.push(vq); } - match &mut self.caps_coll.int_cap { + match &mut caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -161,7 +157,7 @@ impl VirtioFsDriver { InterruptCapability::Msix(msix_table) => { use core::iter; - self.caps_coll.com_cfg.register_msix_vectors( + caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -176,9 +172,13 @@ impl VirtioFsDriver { } // At this point the device is "live" - self.caps_coll.com_cfg.drv_ok(); + caps_coll.com_cfg.drv_ok(); - Ok(()) + Ok(Self { + dev_cfg, + caps_coll, + vqueues, + }) } pub fn handle_interrupt(&mut self) { diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index d1f6bec5d6..2510a3db6a 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -1,67 +1,46 @@ -use alloc::vec::Vec; - use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; -use crate::drivers::fs::{FsDevCfg, VirtioFsDriver}; +use crate::drivers::fs::VirtioFsDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; +use crate::drivers::virtio::transport::pci; use crate::drivers::virtio::transport::pci::PciCap; -use crate::drivers::virtio::transport::{UniCapsColl, pci}; impl VirtioFsDriver { - fn map_cfg(cap: &PciCap) -> Option { - let dev_cfg = pci::map_dev_cfg::(cap)?; - - let dev_cfg = VolatileRef::from_ref(dev_cfg); - - Some(FsDevCfg { - raw: dev_cfg, - features: virtio::fs::F::empty(), - }) - } - - /// Instantiates a new (VirtioFsDriver)[VirtioFsDriver] struct, by checking the available - /// configuration structures and moving them into the struct. - pub fn new( - caps_coll: UniCapsColl, - dev_cfg_list: Vec, - device: &PciDevice, - ) -> Result { - let device_id = device.device_id(); - - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioFsDriver::map_cfg) else { - error!("No dev config. Aborting!"); - return Err(error::VirtioFsInitError::NoDevCfg(device_id)); - }; - - Ok(VirtioFsDriver { - dev_cfg, - caps_coll, - vqueues: Vec::new(), - }) + fn map_cfg( + cap: &PciCap, + ) -> Option> { + let dev_cfg = pci::map_dev_cfg(cap)?; + Some(VolatileRef::from_ref(dev_cfg)) } - /// Initializes virtio filesystem device + /// Initializes virtio filesystem device by checking the available + /// configuration structures and calling the initializer on them. pub fn init( device: &PciDevice, handlers: &mut InterruptHandlerMap, ) -> Result { let (caps, dev_cfg_list) = pci::map_caps(device) .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - let mut drv = VirtioFsDriver::new(caps, dev_cfg_list, device).map_err(|fs_err| { - error!("Initializing new network driver failed. Aborting!"); - VirtioError::FsDriver(fs_err) - })?; - match drv.init_dev(handlers, device.get_irq()) { - Ok(()) => { + let dev_cfg = dev_cfg_list + .iter() + .find_map(VirtioFsDriver::map_cfg) + .ok_or_else(|| { + error!("No dev config. Aborting!"); + error!("Initializing new network driver failed. Aborting!"); + VirtioError::FsDriver(error::VirtioFsInitError::NoDevCfg(device.device_id())) + })?; + + match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { + Ok(drv) => { info!("Filesystem device has been initialized by driver!",); Ok(drv) } - Err(fs_err) => { - drv.set_failed(); + Err((fs_err, mut caps_coll)) => { + caps_coll.com_cfg.set_failed(); Err(VirtioError::FsDriver(fs_err)) } } diff --git a/src/drivers/net/virtio/mmio.rs b/src/drivers/net/virtio/mmio.rs index 91ea93a4e7..96ca4f5245 100644 --- a/src/drivers/net/virtio/mmio.rs +++ b/src/drivers/net/virtio/mmio.rs @@ -1,30 +1,13 @@ -use smoltcp::phy::ChecksumCapabilities; use virtio::mmio::DeviceRegisters; use volatile::VolatileRef; -use crate::drivers::net::virtio::{Init, NetDevCfg, Uninit, VirtioNetDriver}; +use crate::drivers::net::virtio::VirtioNetDriver; use crate::drivers::virtio::error::VirtioError; use crate::drivers::virtio::transport::mmio::map_caps; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio network driver -impl VirtioNetDriver { - pub fn new(registers: VolatileRef<'static, DeviceRegisters>) -> Result { - let (caps_coll, dev_cfg_raw) = map_caps(registers); - let dev_cfg = NetDevCfg { - raw: dev_cfg_raw, - features: virtio::net::F::empty(), - }; - - Ok(VirtioNetDriver { - dev_cfg, - caps_coll, - inner: Uninit, - num_vqs: 0, - checksums: ChecksumCapabilities::default(), - }) - } - +impl VirtioNetDriver { /// Initializes virtio network device by mapping configuration layout to /// respective structs (configuration structs are: /// @@ -34,17 +17,13 @@ impl VirtioNetDriver { registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, - ) -> Result, VirtioError> { - let drv = VirtioNetDriver::new(registers)?; - let mut drv = drv - .init_dev(handlers, Some(irq)) + ) -> Result { + let mut drv = VirtioNetDriver::init_dev(map_caps(registers), handlers, Some(irq)) .map_err(VirtioError::NetDriver)?; drv.print_information(); Ok(drv) } -} -impl VirtioNetDriver { pub fn print_information(&mut self) { self.caps_coll.com_cfg.print_information(); if self.dev_status() == virtio::net::S::LINK_UP { diff --git a/src/drivers/net/virtio/mod.rs b/src/drivers/net/virtio/mod.rs index b9462f4259..bf33e5fd92 100644 --- a/src/drivers/net/virtio/mod.rs +++ b/src/drivers/net/virtio/mod.rs @@ -223,26 +223,21 @@ impl TxQueues { } } -pub(crate) struct Uninit; -pub(crate) struct Init { - pub(super) mtu: u16, - pub(super) ctrl_vq: Option, - pub(super) recv_vqs: RxQueues, - pub(super) send_vqs: TxQueues, - /// Capacity in number of buffer descriptors, not frames. - pub(super) send_capacity: u32, -} - /// Virtio network driver struct. /// /// Struct allows to control devices virtqueues as also /// the device itself. -pub(crate) struct VirtioNetDriver { +pub(crate) struct VirtioNetDriver { pub(super) dev_cfg: NetDevCfg, pub(super) caps_coll: UniCapsColl, - pub(super) inner: T, + pub(super) mtu: u16, + #[allow(unused)] + pub(super) ctrl_vq: Option, + pub(super) recv_vqs: RxQueues, + pub(super) send_vqs: TxQueues, + /// Capacity in number of buffer descriptors, not frames. + pub(super) send_capacity: u32, - pub(super) num_vqs: u16, /// Describes for what protocols and in which directions, if any, the checksum /// should be calculated in software. It is the complement of what is offloaded /// to the hardware. @@ -506,7 +501,7 @@ impl smoltcp::phy::RxToken for RxToken<'_> { } } -impl NetworkDriver for VirtioNetDriver { +impl NetworkDriver for VirtioNetDriver { /// Returns the mac address of the device. /// If VIRTIO_NET_F_MAC is not set, the function panics currently! fn get_mac_address(&self) -> [u8; 6] { @@ -522,7 +517,7 @@ impl NetworkDriver for VirtioNetDriver { #[allow(dead_code)] fn has_packet(&self) -> bool { - self.inner.recv_vqs.has_packet() + self.recv_vqs.has_packet() } fn set_polling_mode(&mut self, value: bool) { @@ -563,9 +558,9 @@ impl smoltcp::phy::Device for VirtioNetDriver { fn capabilities(&self) -> DeviceCapabilities { let mut device_capabilities = DeviceCapabilities::default(); device_capabilities.medium = smoltcp::phy::Medium::Ethernet; - device_capabilities.max_transmission_unit = self.inner.mtu.into(); + device_capabilities.max_transmission_unit = self.mtu.into(); device_capabilities.max_burst_size = - Some(usize::try_from(self.inner.send_capacity).unwrap() / usize::from(BUFF_PER_PACKET)); + Some(usize::try_from(self.send_capacity).unwrap() / usize::from(BUFF_PER_PACKET)); device_capabilities.checksum = self.checksums.clone(); device_capabilities } @@ -574,27 +569,24 @@ impl smoltcp::phy::Device for VirtioNetDriver { &mut self, _timestamp: smoltcp::time::Instant, ) -> Option<(Self::RxToken<'_>, Self::TxToken<'_>)> { - if !self.inner.recv_vqs.has_packet() { + if !self.recv_vqs.has_packet() { return None; } self.free_up_send_capacity(); - self.inner.send_capacity = self - .inner - .send_capacity - .checked_sub(u32::from(BUFF_PER_PACKET))?; + self.send_capacity = self.send_capacity.checked_sub(u32::from(BUFF_PER_PACKET))?; Some(( RxToken { - recv_vqs: &mut self.inner.recv_vqs, + recv_vqs: &mut self.recv_vqs, is_mrg_rxbuf_enabled: self.dev_cfg.features.contains(virtio::net::F::MRG_RXBUF), checksums: self.checksums.clone(), }, TxToken { - send_vqs: &mut self.inner.send_vqs, + send_vqs: &mut self.send_vqs, checksums: self.checksums.clone(), - send_capacity: &mut self.inner.send_capacity, + send_capacity: &mut self.send_capacity, }, )) } @@ -602,27 +594,24 @@ impl smoltcp::phy::Device for VirtioNetDriver { fn transmit(&mut self, _timestamp: smoltcp::time::Instant) -> Option> { self.free_up_send_capacity(); - self.inner.send_capacity = self - .inner - .send_capacity - .checked_sub(u32::from(BUFF_PER_PACKET))?; + self.send_capacity = self.send_capacity.checked_sub(u32::from(BUFF_PER_PACKET))?; Some(TxToken { - send_vqs: &mut self.inner.send_vqs, + send_vqs: &mut self.send_vqs, checksums: self.checksums.clone(), - send_capacity: &mut self.inner.send_capacity, + send_capacity: &mut self.send_capacity, }) } } -impl Driver for VirtioNetDriver { +impl Driver for VirtioNetDriver { fn get_name(&self) -> &'static str { "virtio" } } // Backend-independent interface for Virtio network driver -impl VirtioNetDriver { +impl VirtioNetDriver { /// Returns the current status of the device, if VIRTIO_NET_F_STATUS /// has been negotiated. Otherwise assumes an active device. #[cfg(not(feature = "pci"))] @@ -686,13 +675,13 @@ impl VirtioNetDriver { pub fn disable_interrupts(&mut self) { // For send and receive queues? // Only for receive? Because send is off anyway? - self.inner.recv_vqs.disable_notifs(); + self.recv_vqs.disable_notifs(); } pub fn enable_interrupts(&mut self) { // For send and receive queues? // Only for receive? Because send is off anyway? - self.inner.recv_vqs.enable_notifs(); + self.recv_vqs.enable_notifs(); } /// If necessary, sets the TCP or UDP checksum field to the checksum of the @@ -755,7 +744,7 @@ impl VirtioNetDriver { fn free_up_send_capacity(&mut self) { // We need to poll to get the queue to remove elements from the table and open up capacity if possible. - self.inner.send_capacity += self.inner.send_vqs.poll() * u32::from(BUFF_PER_PACKET); + self.send_capacity += self.send_vqs.poll() * u32::from(BUFF_PER_PACKET); } pub(crate) fn handle_device_configuration_interrupt(&self) { @@ -765,25 +754,28 @@ impl VirtioNetDriver { } } -impl VirtioNetDriver { +impl VirtioNetDriver { /// Initializes the device in adherence to specification. Returns Some(VirtioNetError) /// upon failure and None in case everything worked as expected. /// /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.1.5 pub fn init_dev( - mut self, + (mut caps_coll, dev_cfg_raw): ( + UniCapsColl, + VolatileRef<'static, virtio::net::Config, ReadOnly>, + ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result, VirtioNetError> { + ) -> Result { // Reset - self.caps_coll.com_cfg.reset_dev(); + caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.caps_coll.com_cfg.ack_dev(); + caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.caps_coll.com_cfg.set_drv(); + caps_coll.com_cfg.set_drv(); let minimal_features = virtio::net::F::VERSION_1 | virtio::net::F::MAC; @@ -812,8 +804,7 @@ impl VirtioNetDriver { | virtio::net::F::GUEST_TSO4 | virtio::net::F::GUEST_TSO6; - let negotiated_features = self - .caps_coll + let negotiated_features = caps_coll .com_cfg .control_registers() .negotiate_features(features); @@ -825,32 +816,34 @@ impl VirtioNetDriver { // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.caps_coll.com_cfg.features_ok(); + caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.caps_coll.com_cfg.check_features() { + let dev_cfg = if caps_coll.com_cfg.check_features() { info!("Features have been negotiated between virtio network device and driver.",); // Set feature set in device config fur future use. - self.dev_cfg.features = negotiated_features; + NetDevCfg { + raw: dev_cfg_raw, + features: negotiated_features, + } } else { error!("The device does not support our subset of features."); return Err(VirtioNetError::FailFeatureNeg); - } - - let mut inner = Init { - mtu: determine_mtu(&self.dev_cfg), - ctrl_vq: None, - recv_vqs: RxQueues::new(Vec::new(), &self.dev_cfg), - send_vqs: TxQueues::new(Vec::new(), &self.dev_cfg), - send_capacity: 0, }; - debug!("Using RX buffer size of {}", inner.recv_vqs.buf_size); + let mtu = determine_mtu(&dev_cfg); + #[cfg_attr( + not(all(feature = "pci", target_arch = "x86_64")), + expect(unused_variables) + )] + let (recv_vqs, send_vqs, num_vqs, ctrl_vq, send_capacity) = + Self::dev_spec_init(&mut caps_coll, &dev_cfg)?; + + debug!("Using RX buffer size of {}", recv_vqs.buf_size); - self.dev_spec_init(&mut inner)?; info!("Device specific initialization for Virtio network device finished",); - match &mut self.caps_coll.int_cap { + match &mut caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers @@ -862,14 +855,13 @@ impl VirtioNetDriver { } #[cfg(all(feature = "pci", target_arch = "x86_64"))] InterruptCapability::Msix(msix_table) => { - let recv_vqs = (0..self.num_vqs).step_by(2); - let send_vqs = (1..self.num_vqs).step_by(2); - let ctrl_vq = self - .dev_cfg + let recv_vqs = (0..num_vqs).step_by(2); + let send_vqs = (1..num_vqs).step_by(2); + let ctrl_vq = dev_cfg .features .contains(virtio::net::F::CTRL_VQ) - .then_some(self.num_vqs); - self.caps_coll.com_cfg.register_msix_vectors( + .then_some(num_vqs); + caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, crate::executor::network::network_device_configuration_handler, @@ -880,74 +872,89 @@ impl VirtioNetDriver { } // At this point the device is "live" - self.caps_coll.com_cfg.drv_ok(); + caps_coll.com_cfg.drv_ok(); + let mut checksums = ChecksumCapabilities::default(); // Not only should we offload receive checksum validation to the device for performance when possible, we MUST // offload it when GUEST_TSO{4,6} are enabled, since otherwise the coalesced packets will be rejected by smoltcp // because of the incorrect checksum. - if self.dev_cfg.features.contains(virtio::net::F::CSUM) - && self.dev_cfg.features.contains(virtio::net::F::GUEST_CSUM) + if dev_cfg.features.contains(virtio::net::F::CSUM) + && dev_cfg.features.contains(virtio::net::F::GUEST_CSUM) { - self.checksums.udp = Checksum::None; - self.checksums.tcp = Checksum::None; - } else if self.dev_cfg.features.contains(virtio::net::F::CSUM) { - self.checksums.udp = Checksum::Rx; - self.checksums.tcp = Checksum::Rx; - } else if self.dev_cfg.features.contains(virtio::net::F::GUEST_CSUM) { - self.checksums.udp = Checksum::Tx; - self.checksums.tcp = Checksum::Tx; + checksums.udp = Checksum::None; + checksums.tcp = Checksum::None; + } else if dev_cfg.features.contains(virtio::net::F::CSUM) { + checksums.udp = Checksum::Rx; + checksums.tcp = Checksum::Rx; + } else if dev_cfg.features.contains(virtio::net::F::GUEST_CSUM) { + checksums.udp = Checksum::Tx; + checksums.tcp = Checksum::Tx; } - debug!("{:?}", self.checksums); + debug!("{checksums:?}"); Ok(VirtioNetDriver { - dev_cfg: self.dev_cfg, - caps_coll: self.caps_coll, - inner, - num_vqs: self.num_vqs, - checksums: self.checksums, + dev_cfg, + caps_coll, + mtu, + ctrl_vq, + recv_vqs, + send_vqs, + send_capacity, + checksums, }) } /// Device Specific initialization according to Virtio specifictation v1.1. - 5.1.5 - fn dev_spec_init(&mut self, inner: &mut Init) -> Result<(), VirtioNetError> { - self.virtqueue_init(inner)?; + /// + /// Returns receive and send queues, the sum of their numbers, the control queue if it exists and the total send + /// capacity. + fn dev_spec_init( + caps_coll: &mut UniCapsColl, + dev_cfg: &NetDevCfg, + ) -> Result<(RxQueues, TxQueues, u16, Option, u32), VirtioNetError> { + let (recv_vqs, send_vqs, num_vqs, send_capacity) = + Self::virtqueue_init(caps_coll, dev_cfg)?; info!("Network driver successfully initialized virtqueues."); // Add a control if feature is negotiated - if self.dev_cfg.features.contains(virtio::net::F::CTRL_VQ) { - let mut ctrl_vq = if self.dev_cfg.features.contains(virtio::net::F::RING_PACKED) { + let ctrl_vq = dev_cfg.features.contains(virtio::net::F::CTRL_VQ).then(|| { + let mut ctrl_vq = if dev_cfg.features.contains(virtio::net::F::RING_PACKED) { VirtQueue::Packed( PackedVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, - self.num_vqs, - self.dev_cfg.features.into(), + num_vqs, + dev_cfg.features.into(), ) .unwrap(), ) } else { VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, - self.num_vqs, - self.dev_cfg.features.into(), + num_vqs, + dev_cfg.features.into(), ) .unwrap(), ) }; ctrl_vq.enable_notifs(); - inner.ctrl_vq = Some(ctrl_vq); - } - - Ok(()) + ctrl_vq + }); + Ok((recv_vqs, send_vqs, num_vqs, ctrl_vq, send_capacity)) } /// Initialize virtqueues via the queue interface and populates receiving queues - fn virtqueue_init(&mut self, inner: &mut Init) -> Result<(), VirtioNetError> { + /// + /// Returns receive and send queues, the sum of their numbers and the total send capacity. + fn virtqueue_init( + caps_coll: &mut UniCapsColl, + dev_cfg: &NetDevCfg, + ) -> Result<(RxQueues, TxQueues, u16, u32), VirtioNetError> { // We are assuming here, that the device single source of truth is the // device specific configuration. Hence we do NOT check if // @@ -956,29 +963,12 @@ impl VirtioNetDriver { // - the plus 1 is due to the possibility of an existing control queue // - the num_queues is found in the ComCfg struct of the device and defines the maximal number // of supported queues. - if self.dev_cfg.features.contains(virtio::net::F::MQ) { - if self - .dev_cfg - .raw - .as_ptr() - .max_virtqueue_pairs() - .read() - .to_ne() * 2 >= MAX_NUM_VQ - { - self.num_vqs = MAX_NUM_VQ; - } else { - self.num_vqs = self - .dev_cfg - .raw - .as_ptr() - .max_virtqueue_pairs() - .read() - .to_ne() * 2; - } + let num_vqs = if dev_cfg.features.contains(virtio::net::F::MQ) { + (dev_cfg.raw.as_ptr().max_virtqueue_pairs().read().to_ne() * 2).min(MAX_NUM_VQ) } else { // Minimal number of virtqueues defined in the standard v1.1. - 5.1.5 Step 1 - self.num_vqs = 2; - } + 2 + }; // The loop is running from 0 to num_vqs and the indexes are provided in this way // in order to allow the indexes of the queues to be in a form of: @@ -989,66 +979,70 @@ impl VirtioNetDriver { // as it is wanted by the network network device. // see Virtio specification v1.1. - 5.1.2 // Assure that we have always an even number of queues (i.e. pairs of queues). - assert_eq!(self.num_vqs % 2, 0); + assert_eq!(num_vqs % 2, 0); + + let mut recv_vqs = RxQueues::new(Vec::new(), dev_cfg); + let mut send_vqs = TxQueues::new(Vec::new(), dev_cfg); + let mut send_capacity = 0; - for i in 0..(self.num_vqs / 2) { - if self.dev_cfg.features.contains(virtio::net::F::RING_PACKED) { + for i in 0..(num_vqs / 2) { + if dev_cfg.features.contains(virtio::net::F::RING_PACKED) { let mut vq = PackedVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(); // Interrupt for receiving packets is wanted vq.enable_notifs(); - inner.recv_vqs.add(VirtQueue::Packed(vq)); + recv_vqs.add(VirtQueue::Packed(vq)); let mut vq = PackedVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i + 1, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(); // Interrupt for communicating that a sent packet left, is not needed vq.disable_notifs(); - inner.send_capacity += u32::from(vq.size()); - inner.send_vqs.add(VirtQueue::Packed(vq)); + send_capacity += u32::from(vq.size()); + send_vqs.add(VirtQueue::Packed(vq)); } else { let mut vq = SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(); // Interrupt for receiving packets is wanted vq.enable_notifs(); - inner.recv_vqs.add(VirtQueue::Split(vq)); + recv_vqs.add(VirtQueue::Split(vq)); let mut vq = SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2 * i + 1, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(); // Interrupt for communicating that a sent packet left, is not needed vq.disable_notifs(); - inner.send_capacity += u32::from(vq.size()); - inner.send_vqs.add(VirtQueue::Split(vq)); + send_capacity += u32::from(vq.size()); + send_vqs.add(VirtQueue::Split(vq)); } } - Ok(()) + Ok((recv_vqs, send_vqs, num_vqs, send_capacity)) } } diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index f8693c7d78..218de0f2f6 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -1,52 +1,21 @@ -use alloc::vec::Vec; - use pci_types::CommandRegister; -use smoltcp::phy::ChecksumCapabilities; use volatile::VolatileRef; -use super::{Init, Uninit}; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; -use crate::drivers::net::virtio::{NetDevCfg, VirtioNetDriver}; +use crate::drivers::net::virtio::VirtioNetDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; +use crate::drivers::virtio::transport::pci; use crate::drivers::virtio::transport::pci::PciCap; -use crate::drivers::virtio::transport::{UniCapsColl, pci}; // Backend-dependent interface for Virtio network driver -impl VirtioNetDriver { - fn map_cfg(cap: &PciCap) -> Option { - let dev_cfg = pci::map_dev_cfg::(cap)?; - - let dev_cfg = VolatileRef::from_ref(dev_cfg); - - Some(NetDevCfg { - raw: dev_cfg, - features: virtio::net::F::empty(), - }) - } - - /// Instantiates a new (VirtioNetDriver)[VirtioNetDriver] struct, by checking the available - /// configuration structures and moving them into the struct. - pub(crate) fn new( - caps_coll: UniCapsColl, - dev_cfg_list: Vec, - device: &PciDevice, - ) -> Result { - let device_id = device.device_id(); - - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioNetDriver::map_cfg) else { - error!("No dev config. Aborting!"); - return Err(error::VirtioNetError::NoDevCfg(device_id)); - }; - - Ok(VirtioNetDriver { - dev_cfg, - caps_coll, - inner: Uninit, - num_vqs: 0, - checksums: ChecksumCapabilities::default(), - }) +impl VirtioNetDriver { + fn map_cfg( + cap: &PciCap, + ) -> Option> { + let dev_cfg = pci::map_dev_cfg(cap)?; + Some(VolatileRef::from_ref(dev_cfg)) } /// Initializes virtio network device by mapping configuration layout to @@ -60,18 +29,22 @@ impl VirtioNetDriver { pub(crate) fn init( device: &PciDevice, handlers: &mut InterruptHandlerMap, - ) -> Result, VirtioError> { + ) -> Result { // enable bus master mode device.set_command(CommandRegister::BUS_MASTER_ENABLE); let (caps, dev_cfg_list) = pci::map_caps(device) .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - let drv = VirtioNetDriver::new(caps, dev_cfg_list, device).map_err(|vnet_err| { - error!("Initializing new network driver failed. Aborting!"); - VirtioError::NetDriver(vnet_err) - })?; - let initialized_drv = drv - .init_dev(handlers, device.get_irq()) + let dev_cfg = dev_cfg_list + .iter() + .find_map(VirtioNetDriver::map_cfg) + .ok_or_else(|| { + error!("No dev config. Aborting!"); + error!("Initializing new network driver failed. Aborting!"); + VirtioError::NetDriver(error::VirtioNetError::NoDevCfg(device.device_id())) + })?; + + let initialized_drv = Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) .map_err(VirtioError::NetDriver)?; info!("Network device has been initialized by driver!",); diff --git a/src/drivers/vsock/mmio.rs b/src/drivers/vsock/mmio.rs index fe891cb72e..585a786414 100644 --- a/src/drivers/vsock/mmio.rs +++ b/src/drivers/vsock/mmio.rs @@ -3,29 +3,11 @@ use volatile::VolatileRef; use crate::drivers::virtio::error::VirtioError; use crate::drivers::virtio::transport::mmio::map_caps; -use crate::drivers::vsock::{EventQueue, RxQueue, TxQueue, VirtioVsockDriver, VsockDevCfg}; +use crate::drivers::vsock::VirtioVsockDriver; use crate::drivers::{InterruptHandlerMap, InterruptLine}; // Backend-dependent interface for Virtio vsock driver impl VirtioVsockDriver { - pub fn new( - registers: VolatileRef<'static, DeviceRegisters>, - ) -> Result { - let (caps_coll, dev_cfg_raw) = map_caps(registers); - let dev_cfg = VsockDevCfg { - raw: dev_cfg_raw, - features: virtio::vsock::F::empty(), - }; - - Ok(VirtioVsockDriver { - dev_cfg, - caps_coll, - event_vq: EventQueue::new(), - recv_vq: RxQueue::new(), - send_vq: TxQueue::new(), - }) - } - /// Initializes virtio vsock device by mapping configuration layout to /// respective structs (configuration structs are: /// @@ -36,9 +18,8 @@ impl VirtioVsockDriver { irq: InterruptLine, handlers: &mut InterruptHandlerMap, ) -> Result { - let mut drv = VirtioVsockDriver::new(registers)?; - drv.init_dev(handlers, Some(irq)) - .map_err(VirtioError::VsockDriver)?; + let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) + .map_err(|(err, _)| VirtioError::VsockDriver(err))?; drv.caps_coll.com_cfg.print_information(); Ok(drv) } diff --git a/src/drivers/vsock/mod.rs b/src/drivers/vsock/mod.rs index 7391e6f196..38f77633d4 100644 --- a/src/drivers/vsock/mod.rs +++ b/src/drivers/vsock/mod.rs @@ -276,11 +276,6 @@ impl VirtioVsockDriver { self.dev_cfg.raw.as_ptr().guest_cid().read().to_ne() } - #[cfg(feature = "pci")] - pub fn set_failed(&mut self) { - self.caps_coll.com_cfg.set_failed(); - } - pub fn disable_interrupts(&mut self) { // For send and receive queues? // Only for receive? Because send is off anyway? @@ -326,85 +321,93 @@ impl VirtioVsockDriver { /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.10.6 pub fn init_dev( - &mut self, + (mut caps_coll, dev_cfg_raw): ( + UniCapsColl, + VolatileRef<'static, virtio::vsock::Config, ReadOnly>, + ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result<(), VirtioVsockError> { + ) -> Result { // Reset - self.caps_coll.com_cfg.reset_dev(); + caps_coll.com_cfg.reset_dev(); // Indicate device, that OS noticed it - self.caps_coll.com_cfg.ack_dev(); + caps_coll.com_cfg.ack_dev(); // Indicate device, that driver is able to handle it - self.caps_coll.com_cfg.set_drv(); + caps_coll.com_cfg.set_drv(); let minimal_features = virtio::vsock::F::VERSION_1; - let negotiated_features = self - .caps_coll + let negotiated_features = caps_coll .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(VirtioVsockError::FailFeatureNeg); + return Err((VirtioVsockError::FailFeatureNeg, caps_coll)); } // Indicates the device, that the current feature set is final for the driver // and will not be changed. - self.caps_coll.com_cfg.features_ok(); + caps_coll.com_cfg.features_ok(); // Checks if the device has accepted final set. This finishes feature negotiation. - if self.caps_coll.com_cfg.check_features() { + let dev_cfg = if caps_coll.com_cfg.check_features() { info!("Features have been negotiated between virtio socket device and driver.",); // Set feature set in device config fur future use. - self.dev_cfg.features = negotiated_features; + VsockDevCfg { + raw: dev_cfg_raw, + features: negotiated_features, + } } else { error!("The device does not support our subset of features."); - return Err(VirtioVsockError::FailFeatureNeg); - } + return Err((VirtioVsockError::FailFeatureNeg, caps_coll)); + }; + let mut recv_vq = RxQueue::new(); // create the queues and tell device about them - self.recv_vq.add(VirtQueue::Split( + recv_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 0, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), )); // Interrupt for receiving packets is wanted - self.recv_vq.enable_notifs(); + recv_vq.enable_notifs(); - self.send_vq.add(VirtQueue::Split( + let mut send_vq = TxQueue::new(); + send_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 1, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), )); // Interrupt for communicating that a sent packet left, is not needed - self.send_vq.disable_notifs(); + send_vq.disable_notifs(); + let mut event_vq = EventQueue::new(); // create the queues and tell device about them - self.event_vq.add(VirtQueue::Split( + event_vq.add(VirtQueue::Split( SplitVq::new( - &mut self.caps_coll.com_cfg, - &self.caps_coll.notif_cfg, + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, VIRTIO_MAX_QUEUE_SIZE, 2, - self.dev_cfg.features.into(), + dev_cfg.features.into(), ) .unwrap(), )); - match &mut self.caps_coll.int_cap { + match &mut caps_coll.int_cap { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { @@ -417,7 +420,7 @@ impl VirtioVsockDriver { } #[cfg(all(feature = "pci", target_arch = "x86_64"))] InterruptCapability::Msix(msix_table) => { - self.caps_coll.com_cfg.register_msix_vectors( + caps_coll.com_cfg.register_msix_vectors( msix_table, handlers, || { @@ -434,12 +437,18 @@ impl VirtioVsockDriver { } // Interrupt for event packets is wanted - self.event_vq.enable_notifs(); + event_vq.enable_notifs(); // At this point the device is "live" - self.caps_coll.com_cfg.drv_ok(); - - Ok(()) + caps_coll.com_cfg.drv_ok(); + + Ok(Self { + dev_cfg, + caps_coll, + event_vq, + recv_vq, + send_vq, + }) } #[inline] diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index 15df0733cb..cc15600e32 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -1,51 +1,23 @@ -use alloc::vec::Vec; - use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; +use crate::drivers::virtio::transport::pci; use crate::drivers::virtio::transport::pci::PciCap; -use crate::drivers::virtio::transport::{UniCapsColl, pci}; -use crate::drivers::vsock::{EventQueue, RxQueue, TxQueue, VirtioVsockDriver, VsockDevCfg}; +use crate::drivers::vsock::VirtioVsockDriver; impl VirtioVsockDriver { - fn map_cfg(cap: &PciCap) -> Option { - let dev_cfg = pci::map_dev_cfg::(cap)?; - - let dev_cfg = VolatileRef::from_ref(dev_cfg); - - Some(VsockDevCfg { - raw: dev_cfg, - features: virtio::vsock::F::empty(), - }) - } - - /// Instantiates a new VirtioVsockDriver struct, by checking the available - /// configuration structures and moving them into the struct. - pub fn new( - caps_coll: UniCapsColl, - dev_cfg_list: Vec, - device: &PciDevice, - ) -> Result { - let device_id = device.device_id(); - - let Some(dev_cfg) = dev_cfg_list.iter().find_map(VirtioVsockDriver::map_cfg) else { - error!("No dev config. Aborting!"); - return Err(error::VirtioVsockError::NoDevCfg(device_id)); - }; - - Ok(VirtioVsockDriver { - dev_cfg, - caps_coll, - event_vq: EventQueue::new(), - recv_vq: RxQueue::new(), - send_vq: TxQueue::new(), - }) + fn map_cfg( + cap: &PciCap, + ) -> Option> { + let dev_cfg = pci::map_dev_cfg(cap)?; + Some(VolatileRef::from_ref(dev_cfg)) } - /// Initializes virtio socket device + /// Initializes virtio socket device by checking the available + /// configuration structures and calling the initializer on them. /// /// Returns a driver instance of VirtioVsockDriver. pub(crate) fn init( @@ -54,20 +26,25 @@ impl VirtioVsockDriver { ) -> Result { let (caps, dev_cfg_list) = pci::map_caps(device) .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - let mut drv = VirtioVsockDriver::new(caps, dev_cfg_list, device).map_err(|vsock_err| { - error!("Initializing new virtio socket device driver failed. Aborting!"); - VirtioError::VsockDriver(vsock_err) - })?; - match drv.init_dev(handlers, device.get_irq()) { - Ok(()) => { + let dev_cfg = dev_cfg_list + .iter() + .find_map(VirtioVsockDriver::map_cfg) + .ok_or_else(|| { + error!("No dev config. Aborting!"); + error!("Initializing new virtio socket device driver failed. Aborting!"); + VirtioError::VsockDriver(error::VirtioVsockError::NoDevCfg(device.device_id())) + })?; + + match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { + Ok(drv) => { let cid = drv.get_cid(); info!("Socket device with cid {cid:x}, has been initialized by driver!"); Ok(drv) } - Err(fs_err) => { - drv.set_failed(); + Err((fs_err, mut caps_coll)) => { + caps_coll.com_cfg.set_failed(); Err(VirtioError::VsockDriver(fs_err)) } } From 48a3e8e55a6fa4aad6c91261feb138e75553a1eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sun, 5 Jul 2026 03:58:35 +0200 Subject: [PATCH 6/9] refactor(virtio/pci): merge capability mapping functions --- src/drivers/console/pci.rs | 11 +- src/drivers/fs/pci.rs | 12 +- src/drivers/net/virtio/pci.rs | 11 +- src/drivers/virtio/transport/pci.rs | 178 +++++++++++++--------------- src/drivers/vsock/pci.rs | 12 +- 5 files changed, 89 insertions(+), 135 deletions(-) diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs index 6dd1055e4c..bc3a27db5b 100644 --- a/src/drivers/console/pci.rs +++ b/src/drivers/console/pci.rs @@ -1,21 +1,14 @@ use pci_types::CommandRegister; -use virtio::console::Config; -use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; use crate::drivers::console::VirtioConsoleDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci::{self, PciCap}; +use crate::drivers::virtio::transport::pci; // Backend-dependent interface for Virtio console driver impl VirtioConsoleDriver { - fn map_cfg(cap: &PciCap) -> Option> { - let dev_cfg = pci::map_dev_cfg(cap)?; - Some(VolatileRef::from_ref(dev_cfg)) - } - /// Initializes virtio console device by checking the available /// configuration structures and calling the initializer on them. /// @@ -32,7 +25,7 @@ impl VirtioConsoleDriver { let dev_cfg = dev_cfg_list .iter() - .find_map(VirtioConsoleDriver::map_cfg) + .find_map(|cap| cap.map_cap_cfg().ok()) .ok_or_else(|| { error!("No dev config. Aborting!"); error!("Initializing new virtio console device driver failed. Aborting!"); diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs index 2510a3db6a..5c7f50fd1b 100644 --- a/src/drivers/fs/pci.rs +++ b/src/drivers/fs/pci.rs @@ -1,21 +1,11 @@ -use volatile::VolatileRef; - use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; use crate::drivers::fs::VirtioFsDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::PciCap; impl VirtioFsDriver { - fn map_cfg( - cap: &PciCap, - ) -> Option> { - let dev_cfg = pci::map_dev_cfg(cap)?; - Some(VolatileRef::from_ref(dev_cfg)) - } - /// Initializes virtio filesystem device by checking the available /// configuration structures and calling the initializer on them. pub fn init( @@ -27,7 +17,7 @@ impl VirtioFsDriver { let dev_cfg = dev_cfg_list .iter() - .find_map(VirtioFsDriver::map_cfg) + .find_map(|cap| cap.map_cap_cfg().ok()) .ok_or_else(|| { error!("No dev config. Aborting!"); error!("Initializing new network driver failed. Aborting!"); diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index 218de0f2f6..a587eeda2f 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -1,5 +1,4 @@ use pci_types::CommandRegister; -use volatile::VolatileRef; use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; @@ -7,17 +6,9 @@ use crate::drivers::net::virtio::VirtioNetDriver; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::PciCap; // Backend-dependent interface for Virtio network driver impl VirtioNetDriver { - fn map_cfg( - cap: &PciCap, - ) -> Option> { - let dev_cfg = pci::map_dev_cfg(cap)?; - Some(VolatileRef::from_ref(dev_cfg)) - } - /// Initializes virtio network device by mapping configuration layout to /// respective structs (configuration structs are: /// [ComCfg](structs.comcfg.html), [NotifCfg](structs.notifcfg.html) @@ -37,7 +28,7 @@ impl VirtioNetDriver { let dev_cfg = dev_cfg_list .iter() - .find_map(VirtioNetDriver::map_cfg) + .find_map(|cap| cap.map_cap_cfg().ok()) .ok_or_else(|| { error!("No dev config. Aborting!"); error!("Initializing new network driver failed. Aborting!"); diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index aaeb2035fd..aa9977649d 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -10,6 +10,7 @@ use core::ptr::{self, NonNull}; use memory_addresses::PhysAddr; use pci_types::capability::PciCapability; +use thiserror::Error; use virtio::pci::{ CapCfgType, CapData, CommonCfg, CommonCfgVolatileFieldAccess, CommonCfgVolatileWideFieldAccess, IsrStatus as IsrStatusRaw, NotificationData, @@ -44,36 +45,6 @@ use crate::drivers::virtio::{ControlRegisters, VirtioIdExt}; #[cfg(feature = "virtio-vsock")] use crate::drivers::vsock::VirtioVsockDriver; -/// Maps a given device specific pci configuration structure and -/// returns a static reference to it. -pub fn map_dev_cfg(cap: &PciCap) -> Option<&'static mut T> { - if cap.cap.cfg_type != CapCfgType::Device { - error!("Capability of device config has wrong id. Mapping not possible..."); - return None; - }; - - if cap.bar_len() < cap.len() + cap.offset() { - error!("Device config of device does not fit into memory specified by bar!",); - return None; - } - - // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 - if cap.len() < u64::try_from(size_of::()).unwrap() { - error!( - "Device specific config from device does not represent actual structure specified by the standard!", - ); - return None; - } - - let virt_addr_raw = cap.bar_addr() + cap.offset(); - - // Create mutable reference to the PCI structure in PCI memory - let dev_cfg: &'static mut T = - unsafe { &mut *(ptr::with_exposed_provenance_mut(virt_addr_raw.try_into().unwrap())) }; - - Some(dev_cfg) -} - /// Virtio's PCI capabilities structure. /// See Virtio specification v.1.1 - 4.1.4 /// @@ -93,6 +64,10 @@ pub struct PciCap { } impl PciCap { + pub fn new(bar: PciBar, cap: CapData) -> Option { + (bar.length >= cap.length.to_ne() + cap.offset.to_ne()).then_some(Self { bar, cap }) + } + pub fn offset(&self) -> u64 { self.cap.offset.to_ne() } @@ -101,65 +76,82 @@ impl PciCap { self.cap.length.to_ne() } - pub fn bar_len(&self) -> u64 { - self.bar.length - } - pub fn bar_addr(&self) -> u64 { self.bar.mem_addr } - /// Returns a reference to the actual structure inside the PCI devices memory space. - fn map_common_cfg(&self) -> Option> { - if self.bar.length < self.len() + self.offset() { - let index = self.bar.index; - error!( - "Common config of the capability of device does not fit into memory specified by bar {index:x}!" - ); - return None; + /// Maps a given device specific pci configuration structure and + /// returns a volatile reference to the actual structure inside the PCI devices memory space. + pub fn map_cap_cfg( + &self, + ) -> Result, CapCfgError> { + if self.cap.cfg_type != T::TYPE { + return Err(CapCfgError::WrongCfgType); } - // `CommonCfg::queue_notify_data` and `CommonCfg::queue_reset` are optional. - const MIN_SIZE: usize = size_of::() - size_of::<[le16; 2]>(); - if self.len() < u64::try_from(MIN_SIZE).unwrap() { - error!("Common config does not represent actual structure specified by the standard!"); - return None; + // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 + if usize::try_from(self.len()).unwrap() < T::min_size() { + return Err(CapCfgError::StructTooLarge); } - let virt_addr_raw = self.bar.mem_addr + self.offset(); - let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( + let virt_addr_raw = self.bar_addr() + self.offset(); + let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( virt_addr_raw.try_into().unwrap(), )) .unwrap(); // Create mutable reference to the PCI structure in PCI memory - let com_cfg_raw = unsafe { VolatileRef::new(ptr) }; + let cap_cfg_raw = unsafe { VolatileRef::new(ptr) }; - Some(com_cfg_raw) + Ok(cap_cfg_raw.restrict()) } +} - fn map_isr_status(&self) -> Option> { - if self.bar.length < self.len() + self.offset() { - let index = self.bar.index; - error!( - "ISR status config of device does not fit into memory specified by bar {index:x}!" - ); - return None; - } +pub(crate) trait CapCfg: Sized { + const TYPE: CapCfgType; - let virt_addr_raw = self.bar.mem_addr + self.offset(); - let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( - virt_addr_raw.try_into().unwrap(), - )) - .unwrap(); + fn min_size() -> usize { + size_of::() + } +} - // Create mutable reference to the PCI structure in the devices memory area - let isr_stat_raw = unsafe { VolatileRef::new(ptr) }; +#[derive(Error, Debug)] +pub(crate) enum CapCfgError { + #[error("wrong capability config id, mapping not possible")] + WrongCfgType, + #[error("structure too large to fit into PCI capability")] + StructTooLarge, +} + +impl CapCfg for CommonCfg { + const TYPE: CapCfgType = CapCfgType::Common; - Some(isr_stat_raw) + fn min_size() -> usize { + // `CommonCfg::queue_notify_data` and `CommonCfg::queue_reset` are optional. + size_of::() - size_of::<[le16; 2]>() } } +impl CapCfg for IsrStatusRaw { + const TYPE: CapCfgType = CapCfgType::Isr; +} + +impl CapCfg for virtio::console::Config { + const TYPE: CapCfgType = CapCfgType::Device; +} + +impl CapCfg for virtio::fs::Config { + const TYPE: CapCfgType = CapCfgType::Device; +} + +impl CapCfg for virtio::net::Config { + const TYPE: CapCfgType = CapCfgType::Device; +} + +impl CapCfg for virtio::vsock::Config { + const TYPE: CapCfgType = CapCfgType::Device; +} + /// Wraps a [`CommonCfg`] in order to preserve /// the original structure. /// @@ -448,14 +440,6 @@ pub struct NotifCfg { impl NotifCfg { fn new(cap: &PciCap) -> Option { - if cap.bar.length < cap.len() + cap.offset() { - let index = cap.bar.index; - error!( - "Notification config of device does not fit into memory specified by bar {index:x}!" - ); - return None; - } - let notify_off_multiplier = cap.cap.notify_off_multiplier?.to_ne(); // define base memory address from which the actual Queue Notify address can be derived via @@ -566,18 +550,13 @@ impl IsrStatus { // Currently all fields are public as the struct is instantiated in the drivers::virtio::env module #[derive(Copy, Clone, Debug)] pub struct PciBar { - index: u8, mem_addr: u64, length: u64, } impl PciBar { - pub fn new(index: u8, mem_addr: u64, length: u64) -> Self { - PciBar { - index, - mem_addr, - length, - } + pub fn new(mem_addr: u64, length: u64) -> Self { + PciBar { mem_addr, length } } } @@ -618,18 +597,26 @@ pub(crate) fn map_caps( let Some((addr, size)) = device.memory_map_bar(slot, true) else { continue; }; - let pci_cap = PciCap { - bar: VirtioPciBar::new(slot, addr.as_u64(), size.try_into().unwrap()), + let Some(pci_cap) = PciCap::new( + VirtioPciBar::new(addr.as_u64(), size.try_into().unwrap()), cap, + ) else { + error!( + "The capability of device {device_id:x} does not fit into memory specified by bar {slot:x}!", + ); + continue; }; match pci_cap.cap.cfg_type { CapCfgType::Common => { if com_cfg.is_none() { - match pci_cap.map_common_cfg() { - Some(cap) => com_cfg = Some(ComCfg::new(cap)), - None => error!( - "Common config capability of device {device_id:x} could not be mapped!" - ), + match pci_cap.map_cap_cfg() { + Ok(cap) => com_cfg = Some(ComCfg::new(cap)), + Err(err) => { + error!("{err}"); + error!( + "Common config capability of device {device_id:x} could not be mapped!" + ); + } } } } @@ -649,11 +636,14 @@ pub(crate) fn map_caps( #[cfg(target_arch = "x86_64")] let cond = cond && msix_table.is_none(); if cond { - match pci_cap.map_isr_status() { - Some(isr_stat) => isr_cfg = Some(IsrStatus::new(isr_stat)), - None => error!( - "ISR status config capability of device {device_id:x} could not be used!" - ), + match pci_cap.map_cap_cfg() { + Ok(isr_stat) => isr_cfg = Some(IsrStatus::new(isr_stat)), + Err(err) => { + error!("{err}"); + error!( + "ISR status config capability of device {device_id:x} could not be used!" + ); + } } } } diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index cc15600e32..636c1eb42f 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -1,21 +1,11 @@ -use volatile::VolatileRef; - use crate::arch::kernel::pci::PciConfigRegion; use crate::drivers::InterruptHandlerMap; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; use crate::drivers::virtio::transport::pci; -use crate::drivers::virtio::transport::pci::PciCap; use crate::drivers::vsock::VirtioVsockDriver; impl VirtioVsockDriver { - fn map_cfg( - cap: &PciCap, - ) -> Option> { - let dev_cfg = pci::map_dev_cfg(cap)?; - Some(VolatileRef::from_ref(dev_cfg)) - } - /// Initializes virtio socket device by checking the available /// configuration structures and calling the initializer on them. /// @@ -29,7 +19,7 @@ impl VirtioVsockDriver { let dev_cfg = dev_cfg_list .iter() - .find_map(VirtioVsockDriver::map_cfg) + .find_map(|cap| cap.map_cap_cfg().ok()) .ok_or_else(|| { error!("No dev config. Aborting!"); error!("Initializing new virtio socket device driver failed. Aborting!"); From b0d3c80dfcf5ff06bf8d18e6b4578b1f24951b4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Sun, 5 Jul 2026 05:40:31 +0200 Subject: [PATCH 7/9] refactor(virtio): merge init functions of virtio drivers for each transport --- src/drivers/{console/mod.rs => console.rs} | 25 ++++----- src/drivers/console/mmio.rs | 26 --------- src/drivers/console/pci.rs | 47 ---------------- src/drivers/{fs/mod.rs => fs.rs} | 27 +++++----- src/drivers/fs/mmio.rs | 30 ----------- src/drivers/fs/pci.rs | 38 ------------- src/drivers/mod.rs | 2 +- src/drivers/net/gem.rs | 2 +- src/drivers/net/loopback.rs | 2 +- src/drivers/net/rtl8139.rs | 2 +- src/drivers/net/{virtio/mod.rs => virtio.rs} | 38 +++++++------ src/drivers/net/virtio/mmio.rs | 33 ------------ src/drivers/net/virtio/pci.rs | 50 ----------------- src/drivers/virtio/mod.rs | 25 +++++++-- src/drivers/virtio/transport/mmio.rs | 32 ++++++++--- src/drivers/virtio/transport/pci.rs | 57 ++++++++++++++++++-- src/drivers/{vsock/mod.rs => vsock.rs} | 27 +++++----- src/drivers/vsock/mmio.rs | 26 --------- src/drivers/vsock/pci.rs | 42 --------------- src/executor/device.rs | 10 +++- 20 files changed, 175 insertions(+), 366 deletions(-) rename src/drivers/{console/mod.rs => console.rs} (97%) delete mode 100644 src/drivers/console/mmio.rs delete mode 100644 src/drivers/console/pci.rs rename src/drivers/{fs/mod.rs => fs.rs} (97%) delete mode 100644 src/drivers/fs/mmio.rs delete mode 100644 src/drivers/fs/pci.rs rename src/drivers/net/{virtio/mod.rs => virtio.rs} (98%) delete mode 100644 src/drivers/net/virtio/mmio.rs delete mode 100644 src/drivers/net/virtio/pci.rs rename src/drivers/{vsock/mod.rs => vsock.rs} (97%) delete mode 100644 src/drivers/vsock/mmio.rs delete mode 100644 src/drivers/vsock/pci.rs diff --git a/src/drivers/console/mod.rs b/src/drivers/console.rs similarity index 97% rename from src/drivers/console/mod.rs rename to src/drivers/console.rs index 391a2013be..03c6acb323 100644 --- a/src/drivers/console/mod.rs +++ b/src/drivers/console.rs @@ -7,15 +7,6 @@ #![allow(dead_code)] -// FIXME: use cfg_select! instead once resolved: -// https://github.com/rust-lang/rust/issues/158371 -// https://github.com/rust-lang/rust/issues/158400 -#[cfg(feature = "pci")] -mod pci; - -#[cfg(not(feature = "pci"))] -mod mmio; - use alloc::vec::Vec; use embedded_io::{ErrorType, Read, ReadReady, Write}; @@ -262,8 +253,8 @@ pub(crate) struct VirtioConsoleDriver { } impl Driver for VirtioConsoleDriver { - fn get_name(&self) -> &'static str { - "virtio" + fn get_name() -> &'static str { + "virtio-console" } } @@ -305,8 +296,13 @@ impl VirtioConsoleDriver { fn handle_queue_interrupt() { crate::console::CONSOLE_WAKER.lock().wake(); } +} + +impl super::virtio::VirtioDriver for VirtioConsoleDriver { + type Config = Config; + type Error = VirtioConsoleError; - pub fn init_dev( + fn init_dev( (mut caps_coll, dev_cfg_raw): (UniCapsColl, VolatileRef<'static, Config, ReadOnly>), handlers: &mut InterruptHandlerMap, irq: Option, @@ -414,6 +410,11 @@ impl VirtioConsoleDriver { send_vq, }) } + + #[cfg(feature = "pci")] + fn no_dev_cfg_err(dev_id: u16) -> Self::Error { + VirtioConsoleError::NoDevCfg(dev_id) + } } impl ErrorType for VirtioConsoleDriver { diff --git a/src/drivers/console/mmio.rs b/src/drivers/console/mmio.rs deleted file mode 100644 index 48848c8765..0000000000 --- a/src/drivers/console/mmio.rs +++ /dev/null @@ -1,26 +0,0 @@ -use virtio::mmio::DeviceRegisters; -use volatile::VolatileRef; - -use crate::drivers::console::VirtioConsoleDriver; -use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::mmio::map_caps; -use crate::drivers::{InterruptHandlerMap, InterruptLine}; - -// Backend-dependent interface for Virtio console driver -impl VirtioConsoleDriver { - /// Initializes virtio console device by mapping configuration layout to - /// respective structs (configuration structs are: - /// - /// Returns a driver instance of - /// [VirtioConsoleDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). - pub fn init( - registers: VolatileRef<'static, DeviceRegisters>, - irq: InterruptLine, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) - .map_err(|(err, _)| VirtioError::ConsoleDriver(err))?; - drv.caps_coll.com_cfg.print_information(); - Ok(drv) - } -} diff --git a/src/drivers/console/pci.rs b/src/drivers/console/pci.rs deleted file mode 100644 index bc3a27db5b..0000000000 --- a/src/drivers/console/pci.rs +++ /dev/null @@ -1,47 +0,0 @@ -use pci_types::CommandRegister; - -use crate::arch::kernel::pci::PciConfigRegion; -use crate::drivers::InterruptHandlerMap; -use crate::drivers::console::VirtioConsoleDriver; -use crate::drivers::pci::PciDevice; -use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; - -// Backend-dependent interface for Virtio console driver -impl VirtioConsoleDriver { - /// Initializes virtio console device by checking the available - /// configuration structures and calling the initializer on them. - /// - /// Returns a driver instance of VirtioConsoleDriver. - pub(crate) fn init( - device: &PciDevice, - handlers: &mut InterruptHandlerMap, - ) -> Result { - // enable bus master mode - device.set_command(CommandRegister::BUS_MASTER_ENABLE); - - let (caps, dev_cfg_list) = pci::map_caps(device) - .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - - let dev_cfg = dev_cfg_list - .iter() - .find_map(|cap| cap.map_cap_cfg().ok()) - .ok_or_else(|| { - error!("No dev config. Aborting!"); - error!("Initializing new virtio console device driver failed. Aborting!"); - VirtioError::ConsoleDriver(error::VirtioConsoleError::NoDevCfg(device.device_id())) - })?; - - match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { - Ok(drv) => { - info!("Console device has been initialized by driver!",); - - Ok(drv) - } - Err((console_err, mut caps_coll)) => { - caps_coll.com_cfg.set_failed(); - Err(VirtioError::ConsoleDriver(console_err)) - } - } - } -} diff --git a/src/drivers/fs/mod.rs b/src/drivers/fs.rs similarity index 97% rename from src/drivers/fs/mod.rs rename to src/drivers/fs.rs index 9b9fe74c9a..5d12c157a6 100644 --- a/src/drivers/fs/mod.rs +++ b/src/drivers/fs.rs @@ -5,15 +5,6 @@ //! //! [File System Device]: https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-45800011 -// FIXME: use cfg_select! instead once resolved: -// https://github.com/rust-lang/rust/issues/158371 -// https://github.com/rust-lang/rust/issues/158400 -#[cfg(feature = "pci")] -mod pci; - -#[cfg(not(feature = "pci"))] -mod mmio; - use alloc::borrow::ToOwned; use alloc::boxed::Box; use alloc::string::String; @@ -67,13 +58,16 @@ pub(crate) struct VirtioFsDriver { } // Backend-independent interface for Virtio network driver -impl VirtioFsDriver { +impl super::virtio::VirtioDriver for VirtioFsDriver { + type Config = virtio::fs::Config; + type Error = VirtioFsInitError; + /// Initializes the device in adherence to specification. Returns Some(VirtioFsError) /// upon failure and None in case everything worked as expected. /// /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.11.5 - pub(crate) fn init_dev( + fn init_dev( (mut caps_coll, dev_cfg_raw): ( UniCapsColl, VolatileRef<'static, virtio::fs::Config, ReadOnly>, @@ -181,6 +175,13 @@ impl VirtioFsDriver { }) } + #[cfg(feature = "pci")] + fn no_dev_cfg_err(dev_id: u16) -> Self::Error { + VirtioFsInitError::NoDevCfg(dev_id) + } +} + +impl VirtioFsDriver { pub fn handle_interrupt(&mut self) { #[cfg_attr( not(all(feature = "pci", target_arch = "x86_64")), @@ -296,8 +297,8 @@ impl VirtioFsInterface for VirtioFsDriver { } impl Driver for VirtioFsDriver { - fn get_name(&self) -> &'static str { - "virtio" + fn get_name() -> &'static str { + "virtio-fs" } } diff --git a/src/drivers/fs/mmio.rs b/src/drivers/fs/mmio.rs deleted file mode 100644 index a5f1d7059c..0000000000 --- a/src/drivers/fs/mmio.rs +++ /dev/null @@ -1,30 +0,0 @@ -use virtio::mmio::DeviceRegisters; -use volatile::VolatileRef; - -use crate::drivers::fs::VirtioFsDriver; -use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::mmio::map_caps; -use crate::drivers::{InterruptHandlerMap, InterruptLine}; - -// Backend-dependent interface for Virtio fs driver -impl VirtioFsDriver { - /// Initializes virtio fs device by mapping configuration layout to - /// respective structs (configuration structs are: - /// - /// Returns a driver instance of - /// [VirtioFsDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). - pub fn init( - registers: VolatileRef<'static, DeviceRegisters>, - irq: InterruptLine, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) - .map_err(|(err, _)| VirtioError::FsDriver(err))?; - drv.print_information(); - Ok(drv) - } - - fn print_information(&mut self) { - self.caps_coll.com_cfg.print_information(); - } -} diff --git a/src/drivers/fs/pci.rs b/src/drivers/fs/pci.rs deleted file mode 100644 index 5c7f50fd1b..0000000000 --- a/src/drivers/fs/pci.rs +++ /dev/null @@ -1,38 +0,0 @@ -use crate::arch::kernel::pci::PciConfigRegion; -use crate::drivers::InterruptHandlerMap; -use crate::drivers::fs::VirtioFsDriver; -use crate::drivers::pci::PciDevice; -use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; - -impl VirtioFsDriver { - /// Initializes virtio filesystem device by checking the available - /// configuration structures and calling the initializer on them. - pub fn init( - device: &PciDevice, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let (caps, dev_cfg_list) = pci::map_caps(device) - .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - - let dev_cfg = dev_cfg_list - .iter() - .find_map(|cap| cap.map_cap_cfg().ok()) - .ok_or_else(|| { - error!("No dev config. Aborting!"); - error!("Initializing new network driver failed. Aborting!"); - VirtioError::FsDriver(error::VirtioFsInitError::NoDevCfg(device.device_id())) - })?; - - match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { - Ok(drv) => { - info!("Filesystem device has been initialized by driver!",); - Ok(drv) - } - Err((fs_err, mut caps_coll)) => { - caps_coll.com_cfg.set_failed(); - Err(VirtioError::FsDriver(fs_err)) - } - } - } -} diff --git a/src/drivers/mod.rs b/src/drivers/mod.rs index 63277722e3..a4e73db34c 100644 --- a/src/drivers/mod.rs +++ b/src/drivers/mod.rs @@ -69,7 +69,7 @@ pub mod error { #[allow(dead_code)] pub(crate) trait Driver { /// Returns the device driver name - fn get_name(&self) -> &'static str; + fn get_name() -> &'static str; } pub(crate) fn init() { diff --git a/src/drivers/net/gem.rs b/src/drivers/net/gem.rs index 3f7adfdd39..1c6de0fa89 100644 --- a/src/drivers/net/gem.rs +++ b/src/drivers/net/gem.rs @@ -314,7 +314,7 @@ impl TxFields { } impl Driver for GEMDriver { - fn get_name(&self) -> &'static str { + fn get_name() -> &'static str { "gem" } } diff --git a/src/drivers/net/loopback.rs b/src/drivers/net/loopback.rs index 3dd794add4..1233de958b 100644 --- a/src/drivers/net/loopback.rs +++ b/src/drivers/net/loopback.rs @@ -26,7 +26,7 @@ impl LoopbackDriver { } impl Driver for LoopbackDriver { - fn get_name(&self) -> &'static str { + fn get_name() -> &'static str { "loopback" } } diff --git a/src/drivers/net/rtl8139.rs b/src/drivers/net/rtl8139.rs index cb324ae111..482d295fd2 100644 --- a/src/drivers/net/rtl8139.rs +++ b/src/drivers/net/rtl8139.rs @@ -697,7 +697,7 @@ impl NetworkDriver for RTL8139Driver { } impl Driver for RTL8139Driver { - fn get_name(&self) -> &'static str { + fn get_name() -> &'static str { "rtl8139" } } diff --git a/src/drivers/net/virtio/mod.rs b/src/drivers/net/virtio.rs similarity index 98% rename from src/drivers/net/virtio/mod.rs rename to src/drivers/net/virtio.rs index bf33e5fd92..b017a72fbe 100644 --- a/src/drivers/net/virtio/mod.rs +++ b/src/drivers/net/virtio.rs @@ -5,15 +5,6 @@ //! //! [Network Device]: https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-2170001 -// FIXME: use cfg_select! instead once resolved: -// https://github.com/rust-lang/rust/issues/158371 -// https://github.com/rust-lang/rust/issues/158400 -#[cfg(feature = "pci")] -mod pci; - -#[cfg(not(feature = "pci"))] -mod mmio; - use alloc::boxed::Box; use alloc::vec::Vec; use core::mem::{ManuallyDrop, MaybeUninit}; @@ -605,8 +596,8 @@ impl smoltcp::phy::Device for VirtioNetDriver { } impl Driver for VirtioNetDriver { - fn get_name(&self) -> &'static str { - "virtio" + fn get_name() -> &'static str { + "virtio-net" } } @@ -754,20 +745,23 @@ impl VirtioNetDriver { } } -impl VirtioNetDriver { +impl crate::drivers::virtio::VirtioDriver for VirtioNetDriver { + type Config = virtio::net::Config; + type Error = VirtioNetError; + /// Initializes the device in adherence to specification. Returns Some(VirtioNetError) /// upon failure and None in case everything worked as expected. /// /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.1.5 - pub fn init_dev( + fn init_dev( (mut caps_coll, dev_cfg_raw): ( UniCapsColl, VolatileRef<'static, virtio::net::Config, ReadOnly>, ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result { + ) -> Result { // Reset caps_coll.com_cfg.reset_dev(); @@ -811,7 +805,7 @@ impl VirtioNetDriver { if !negotiated_features.contains(minimal_features) { error!("Device features set, does not satisfy minimal features needed. Aborting!"); - return Err(VirtioNetError::FailFeatureNeg); + return Err((VirtioNetError::FailFeatureNeg, caps_coll)); } // Indicates the device, that the current feature set is final for the driver @@ -828,7 +822,7 @@ impl VirtioNetDriver { } } else { error!("The device does not support our subset of features."); - return Err(VirtioNetError::FailFeatureNeg); + return Err((VirtioNetError::FailFeatureNeg, caps_coll)); }; let mtu = determine_mtu(&dev_cfg); @@ -837,7 +831,10 @@ impl VirtioNetDriver { expect(unused_variables) )] let (recv_vqs, send_vqs, num_vqs, ctrl_vq, send_capacity) = - Self::dev_spec_init(&mut caps_coll, &dev_cfg)?; + match Self::dev_spec_init(&mut caps_coll, &dev_cfg) { + Ok(it) => it, + Err(err) => return Err((err, caps_coll)), + }; debug!("Using RX buffer size of {}", recv_vqs.buf_size); @@ -904,6 +901,13 @@ impl VirtioNetDriver { }) } + #[cfg(feature = "pci")] + fn no_dev_cfg_err(dev_id: u16) -> Self::Error { + VirtioNetError::NoDevCfg(dev_id) + } +} + +impl VirtioNetDriver { /// Device Specific initialization according to Virtio specifictation v1.1. - 5.1.5 /// /// Returns receive and send queues, the sum of their numbers, the control queue if it exists and the total send diff --git a/src/drivers/net/virtio/mmio.rs b/src/drivers/net/virtio/mmio.rs deleted file mode 100644 index 96ca4f5245..0000000000 --- a/src/drivers/net/virtio/mmio.rs +++ /dev/null @@ -1,33 +0,0 @@ -use virtio::mmio::DeviceRegisters; -use volatile::VolatileRef; - -use crate::drivers::net::virtio::VirtioNetDriver; -use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::mmio::map_caps; -use crate::drivers::{InterruptHandlerMap, InterruptLine}; - -// Backend-dependent interface for Virtio network driver -impl VirtioNetDriver { - /// Initializes virtio network device by mapping configuration layout to - /// respective structs (configuration structs are: - /// - /// Returns a driver instance of - /// [VirtioNetDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). - pub fn init( - registers: VolatileRef<'static, DeviceRegisters>, - irq: InterruptLine, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let mut drv = VirtioNetDriver::init_dev(map_caps(registers), handlers, Some(irq)) - .map_err(VirtioError::NetDriver)?; - drv.print_information(); - Ok(drv) - } - - pub fn print_information(&mut self) { - self.caps_coll.com_cfg.print_information(); - if self.dev_status() == virtio::net::S::LINK_UP { - info!("The link of the network device is up!"); - } - } -} diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs deleted file mode 100644 index a587eeda2f..0000000000 --- a/src/drivers/net/virtio/pci.rs +++ /dev/null @@ -1,50 +0,0 @@ -use pci_types::CommandRegister; - -use crate::arch::kernel::pci::PciConfigRegion; -use crate::drivers::InterruptHandlerMap; -use crate::drivers::net::virtio::VirtioNetDriver; -use crate::drivers::pci::PciDevice; -use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; - -// Backend-dependent interface for Virtio network driver -impl VirtioNetDriver { - /// Initializes virtio network device by mapping configuration layout to - /// respective structs (configuration structs are: - /// [ComCfg](structs.comcfg.html), [NotifCfg](structs.notifcfg.html) - /// [IsrStatus](structs.isrstatus.html), [PciCfg](structs.pcicfg.html) - /// [ShMemCfg](structs.ShMemCfg)). - /// - /// Returns a driver instance of - /// [VirtioNetDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). - pub(crate) fn init( - device: &PciDevice, - handlers: &mut InterruptHandlerMap, - ) -> Result { - // enable bus master mode - device.set_command(CommandRegister::BUS_MASTER_ENABLE); - let (caps, dev_cfg_list) = pci::map_caps(device) - .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - - let dev_cfg = dev_cfg_list - .iter() - .find_map(|cap| cap.map_cap_cfg().ok()) - .ok_or_else(|| { - error!("No dev config. Aborting!"); - error!("Initializing new network driver failed. Aborting!"); - VirtioError::NetDriver(error::VirtioNetError::NoDevCfg(device.device_id())) - })?; - - let initialized_drv = Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) - .map_err(VirtioError::NetDriver)?; - info!("Network device has been initialized by driver!",); - - if initialized_drv.is_link_up() { - info!("Virtio-net link is up after initialization."); - } else { - info!("Virtio-net link is down after initialization!"); - } - - Ok(initialized_drv) - } -} diff --git a/src/drivers/virtio/mod.rs b/src/drivers/virtio/mod.rs index a301ea1ec5..320b037ea4 100644 --- a/src/drivers/virtio/mod.rs +++ b/src/drivers/virtio/mod.rs @@ -162,6 +162,23 @@ where } } +pub(super) trait VirtioDriver: super::Driver + Sized { + type Config: 'static; + type Error: Into; + + fn init_dev( + caps_tuple: ( + transport::UniCapsColl, + volatile::VolatileRef<'static, Self::Config, volatile::access::ReadOnly>, + ), + handlers: &mut super::InterruptHandlerMap, + irq: Option, + ) -> Result; + + #[cfg(feature = "pci")] + fn no_dev_cfg_err(dev_id: u16) -> Self::Error; +} + pub mod error { use thiserror::Error; @@ -213,18 +230,18 @@ pub mod error { feature = "virtio-net", ))] #[error(transparent)] - NetDriver(VirtioNetError), + NetDriver(#[from] VirtioNetError), #[cfg(feature = "virtio-fs")] #[error(transparent)] - FsDriver(VirtioFsInitError), + FsDriver(#[from] VirtioFsInitError), #[cfg(feature = "virtio-vsock")] #[error(transparent)] - VsockDriver(VirtioVsockError), + VsockDriver(#[from] VirtioVsockError), #[cfg(feature = "virtio-console")] #[error(transparent)] - ConsoleDriver(VirtioConsoleError), + ConsoleDriver(#[from] VirtioConsoleError), } } diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 095c407d31..951f0c423b 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -350,7 +350,7 @@ pub(crate) fn init_device( // Verify the device-ID to find the network card match registers.as_ptr().device_id().read() { #[cfg(feature = "virtio-console")] - virtio::Id::Console => match VirtioConsoleDriver::init(registers, irq_no, handlers) { + virtio::Id::Console => match init::(registers, irq_no, handlers) { Ok(virt_console_drv) => { info!("Virtio console driver initialized."); Ok(VirtioDriver::Console(alloc::boxed::Box::new( @@ -366,7 +366,7 @@ pub(crate) fn init_device( virtio::Id::Fs => { // TODO: check subclass // TODO: proper error handling on driver creation fail - match VirtioFsDriver::init(registers, irq_no, handlers) { + match init::(registers, irq_no, handlers) { Ok(virt_fs_drv) => { info!("Virtio filesystem driver initialized."); Ok(VirtioDriver::Fs(alloc::boxed::Box::new(virt_fs_drv))) @@ -378,8 +378,11 @@ pub(crate) fn init_device( } } #[cfg(feature = "virtio-net")] - virtio::Id::Net => match VirtioNetDriver::init(registers, irq_no, handlers) { + virtio::Id::Net => match init::(registers, irq_no, handlers) { Ok(virt_net_drv) => { + if virt_net_drv.dev_status() == virtio::net::S::LINK_UP { + info!("The link of the network device is up!"); + } info!("Virtio network driver initialized."); Ok(VirtioDriver::Net(alloc::boxed::Box::new(virt_net_drv))) } @@ -389,7 +392,7 @@ pub(crate) fn init_device( } }, #[cfg(feature = "virtio-vsock")] - virtio::Id::Vsock => match VirtioVsockDriver::init(registers, irq_no, handlers) { + virtio::Id::Vsock => match init::(registers, irq_no, handlers) { Ok(virt_vsock_drv) => { info!("Virtio sock driver initialized."); Ok(VirtioDriver::Vsock(alloc::boxed::Box::new(virt_vsock_drv))) @@ -430,13 +433,30 @@ pub fn map_caps( let dev_cfg_raw = VolatileRef::from_ref(dev_cfg_raw); let int_cap = InterruptCapability::IsrStatus(IsrStatus::new(registers.borrow_mut())); let notif_cfg = NotifCfg::new(registers.borrow_mut()); + let mut com_cfg = ComCfg::new(registers); + com_cfg.print_information(); ( UniCapsColl { - com_cfg: ComCfg::new(registers), - int_cap, + com_cfg, notif_cfg, + int_cap, }, dev_cfg_raw, ) } + +/// Initializes virtio vsock device by mapping configuration layout to +/// respective structs (configuration structs are: +/// +/// Returns a driver instance of +/// [VirtioVsockDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). +fn init( + registers: VolatileRef<'static, DeviceRegisters>, + irq: InterruptLine, + handlers: &mut InterruptHandlerMap, +) -> Result { + let drv = + T::init_dev(map_caps(registers), handlers, Some(irq)).map_err(|(err, _)| err.into())?; + Ok(drv) +} diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index aa9977649d..11f6120fee 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -746,7 +746,7 @@ pub(crate) fn init_device( match id { #[cfg(feature = "virtio-console")] - virtio::Id::Console => match VirtioConsoleDriver::init(device, handlers) { + virtio::Id::Console => match init::(device, handlers) { Ok(virt_console_drv) => { info!("Virtio console driver initialized."); Ok(VirtioDriver::Console(alloc::boxed::Box::new( @@ -762,7 +762,7 @@ pub(crate) fn init_device( virtio::Id::Fs => { // TODO: check subclass // TODO: proper error handling on driver creation fail - match VirtioFsDriver::init(device, handlers) { + match init::(device, handlers) { Ok(virt_fs_drv) => { info!("Virtio filesystem driver initialized."); Ok(VirtioDriver::Fs(alloc::boxed::Box::new(virt_fs_drv))) @@ -780,8 +780,14 @@ pub(crate) fn init_device( not(feature = "rtl8139"), feature = "virtio-net", ))] - virtio::Id::Net => match VirtioNetDriver::init(device, handlers) { + virtio::Id::Net => match init::(device, handlers) { Ok(virt_net_drv) => { + let link_status = if virt_net_drv.is_link_up() { + "up" + } else { + "down" + }; + info!("Virtio-net link is {link_status} after initialization!"); info!("Virtio network driver initialized."); Ok(VirtioDriver::Net(alloc::boxed::Box::new(virt_net_drv))) } @@ -793,8 +799,9 @@ pub(crate) fn init_device( } }, #[cfg(feature = "virtio-vsock")] - virtio::Id::Vsock => match VirtioVsockDriver::init(device, handlers) { + virtio::Id::Vsock => match init::(device, handlers) { Ok(virt_sock_drv) => { + info!("Socket device has cid {:x}.", virt_sock_drv.get_cid()); info!("Virtio sock driver initialized."); Ok(VirtioDriver::Vsock(alloc::boxed::Box::new(virt_sock_drv))) } @@ -833,3 +840,45 @@ pub(crate) enum VirtioDriver { #[cfg(feature = "virtio-vsock")] Vsock(alloc::boxed::Box), } + +/// Initializes virtio device by checking the available +/// configuration structures and calling the initializer on them. +/// +/// Returns a driver instance. +fn init( + device: &PciDevice, + handlers: &mut InterruptHandlerMap, +) -> Result +where + T: crate::drivers::virtio::VirtioDriver, + T::Config: CapCfg, +{ + // enable bus master mode + device.set_command(pci_types::CommandRegister::BUS_MASTER_ENABLE); + + let (caps, dev_cfg_list) = + map_caps(device).inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; + + let dev_cfg = dev_cfg_list + .iter() + .find_map(|cap| cap.map_cap_cfg().ok()) + .ok_or_else(|| { + error!("No dev config. Aborting!"); + error!( + "Initializing new {} device driver failed. Aborting!", + T::get_name() + ); + T::no_dev_cfg_err(device.device_id()).into() + })?; + + match T::init_dev((caps, dev_cfg), handlers, device.get_irq()) { + Ok(drv) => { + info!("{} device has been initialized by driver!", T::get_name()); + Ok(drv) + } + Err((err, mut caps_coll)) => { + caps_coll.com_cfg.set_failed(); + Err(err.into()) + } + } +} diff --git a/src/drivers/vsock/mod.rs b/src/drivers/vsock.rs similarity index 97% rename from src/drivers/vsock/mod.rs rename to src/drivers/vsock.rs index 38f77633d4..d13c739ce0 100644 --- a/src/drivers/vsock/mod.rs +++ b/src/drivers/vsock.rs @@ -1,14 +1,5 @@ #![allow(dead_code)] -// FIXME: use cfg_select! instead once resolved: -// https://github.com/rust-lang/rust/issues/158371 -// https://github.com/rust-lang/rust/issues/158400 -#[cfg(feature = "pci")] -mod pci; - -#[cfg(not(feature = "pci"))] -mod mmio; - use alloc::boxed::Box; use alloc::vec::Vec; @@ -265,8 +256,8 @@ pub(crate) struct VirtioVsockDriver { } impl Driver for VirtioVsockDriver { - fn get_name(&self) -> &'static str { - "virtio" + fn get_name() -> &'static str { + "virtio-vsock" } } @@ -314,13 +305,18 @@ impl VirtioVsockDriver { todo!("Device configuration change notification cannot be handled yet"); } } +} + +impl super::virtio::VirtioDriver for VirtioVsockDriver { + type Config = virtio::vsock::Config; + type Error = VirtioVsockError; /// Initializes the device in adherence to specification. Returns Some(VirtioVsockError) /// upon failure and None in case everything worked as expected. /// /// See Virtio specification v1.1. - 3.1.1. /// and v1.1. - 5.10.6 - pub fn init_dev( + fn init_dev( (mut caps_coll, dev_cfg_raw): ( UniCapsColl, VolatileRef<'static, virtio::vsock::Config, ReadOnly>, @@ -451,6 +447,13 @@ impl VirtioVsockDriver { }) } + #[cfg(feature = "pci")] + fn no_dev_cfg_err(dev_id: u16) -> Self::Error { + VirtioVsockError::NoDevCfg(dev_id) + } +} + +impl VirtioVsockDriver { #[inline] pub fn process_packet(&mut self, f: F) where diff --git a/src/drivers/vsock/mmio.rs b/src/drivers/vsock/mmio.rs deleted file mode 100644 index 585a786414..0000000000 --- a/src/drivers/vsock/mmio.rs +++ /dev/null @@ -1,26 +0,0 @@ -use virtio::mmio::DeviceRegisters; -use volatile::VolatileRef; - -use crate::drivers::virtio::error::VirtioError; -use crate::drivers::virtio::transport::mmio::map_caps; -use crate::drivers::vsock::VirtioVsockDriver; -use crate::drivers::{InterruptHandlerMap, InterruptLine}; - -// Backend-dependent interface for Virtio vsock driver -impl VirtioVsockDriver { - /// Initializes virtio vsock device by mapping configuration layout to - /// respective structs (configuration structs are: - /// - /// Returns a driver instance of - /// [VirtioVsockDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). - pub fn init( - registers: VolatileRef<'static, DeviceRegisters>, - irq: InterruptLine, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let mut drv = Self::init_dev(map_caps(registers), handlers, Some(irq)) - .map_err(|(err, _)| VirtioError::VsockDriver(err))?; - drv.caps_coll.com_cfg.print_information(); - Ok(drv) - } -} diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs deleted file mode 100644 index 636c1eb42f..0000000000 --- a/src/drivers/vsock/pci.rs +++ /dev/null @@ -1,42 +0,0 @@ -use crate::arch::kernel::pci::PciConfigRegion; -use crate::drivers::InterruptHandlerMap; -use crate::drivers::pci::PciDevice; -use crate::drivers::virtio::error::{self, VirtioError}; -use crate::drivers::virtio::transport::pci; -use crate::drivers::vsock::VirtioVsockDriver; - -impl VirtioVsockDriver { - /// Initializes virtio socket device by checking the available - /// configuration structures and calling the initializer on them. - /// - /// Returns a driver instance of VirtioVsockDriver. - pub(crate) fn init( - device: &PciDevice, - handlers: &mut InterruptHandlerMap, - ) -> Result { - let (caps, dev_cfg_list) = pci::map_caps(device) - .inspect_err(|_| error!("Mapping capabilities failed. Aborting!"))?; - - let dev_cfg = dev_cfg_list - .iter() - .find_map(|cap| cap.map_cap_cfg().ok()) - .ok_or_else(|| { - error!("No dev config. Aborting!"); - error!("Initializing new virtio socket device driver failed. Aborting!"); - VirtioError::VsockDriver(error::VirtioVsockError::NoDevCfg(device.device_id())) - })?; - - match Self::init_dev((caps, dev_cfg), handlers, device.get_irq()) { - Ok(drv) => { - let cid = drv.get_cid(); - info!("Socket device with cid {cid:x}, has been initialized by driver!"); - - Ok(drv) - } - Err((fs_err, mut caps_coll)) => { - caps_coll.com_cfg.set_failed(); - Err(VirtioError::VsockDriver(fs_err)) - } - } - } -} diff --git a/src/executor/device.rs b/src/executor/device.rs index 8d166d0627..752025684e 100644 --- a/src/executor/device.rs +++ b/src/executor/device.rs @@ -17,6 +17,13 @@ use super::network::{NetworkInterface, NetworkState}; use crate::arch::kernel::systemtime; #[cfg(feature = "write-pcap-file")] use crate::drivers::Driver; +#[cfg(any( + all(target_arch = "riscv64", feature = "gem-net", not(feature = "pci")), + feature = "rtl8139", + feature = "virtio-net", + feature = "write-pcap-file" +))] +use crate::drivers::net::NetworkDevice; use crate::drivers::net::NetworkDriver; cfg_select! { @@ -26,7 +33,6 @@ cfg_select! { feature = "virtio-net", ) => { use hermit_sync::SpinMutex; - use crate::drivers::net::NetworkDevice; pub(crate) static NETWORK_DEVICE: SpinMutex> = SpinMutex::new(None); } @@ -59,7 +65,7 @@ impl<'a> NetworkInterface<'a> { #[cfg_attr(feature = "net-trace", expect(unused_mut))] #[cfg(feature = "write-pcap-file")] let mut device = { - let default_name = device.get_name(); + let default_name = NetworkDevice::get_name(); PcapWriter::new( device, pcap_writer::FileSink::new(default_name), From cb5456cad01055d8439212032b0149474a96f578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Tue, 14 Jul 2026 03:08:12 +0200 Subject: [PATCH 8/9] refactor(virtio): move non-device specific init to a common function --- src/drivers/console.rs | 170 ++++++++------------- src/drivers/fs.rs | 156 +++++++------------ src/drivers/net/virtio.rs | 215 +++++++++++---------------- src/drivers/virtio/mod.rs | 95 +++++++++++- src/drivers/virtio/transport/mmio.rs | 9 +- src/drivers/virtio/transport/pci.rs | 3 +- src/drivers/vsock.rs | 200 ++++++++++--------------- 7 files changed, 388 insertions(+), 460 deletions(-) diff --git a/src/drivers/console.rs b/src/drivers/console.rs index 03c6acb323..7a3098881d 100644 --- a/src/drivers/console.rs +++ b/src/drivers/console.rs @@ -21,8 +21,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; +use crate::drivers::virtio::error::{VirtioConsoleError, VirtioError}; use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ @@ -236,13 +235,7 @@ impl TxQueue { } } -/// A wrapper struct for the raw configuration structure. -/// Handling the right access to fields, as some are read-only -/// for the driver. -pub(crate) struct ConsoleDevCfg { - pub raw: VolatileRef<'static, Config, ReadOnly>, - pub features: virtio::console::F, -} +type ConsoleDevCfg = super::virtio::DevCfg; pub(crate) struct VirtioConsoleDriver { pub(super) dev_cfg: ConsoleDevCfg, @@ -301,107 +294,76 @@ impl VirtioConsoleDriver { impl super::virtio::VirtioDriver for VirtioConsoleDriver { type Config = Config; type Error = VirtioConsoleError; + type DeviceFeatures = virtio::console::F; + + const MINIMAL_FEATURES: Self::DeviceFeatures = virtio::console::F::VERSION_1; + const OPTIONAL_FEATURES: Self::DeviceFeatures = virtio::console::F::empty(); fn init_dev( (mut caps_coll, dev_cfg_raw): (UniCapsColl, VolatileRef<'static, Config, ReadOnly>), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result { - // Reset - caps_coll.com_cfg.reset_dev(); - - // Indicate device, that OS noticed it - caps_coll.com_cfg.ack_dev(); - - // Indicate device, that driver is able to handle it - caps_coll.com_cfg.set_drv(); - - let minimal_features = virtio::console::F::VERSION_1; - let negotiated_features = caps_coll - .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, caps_coll)); - } - - // Indicates the device, that the current feature set is final for the driver - // and will not be changed. - caps_coll.com_cfg.features_ok(); - - // Checks if the device has accepted final set. This finishes feature negotiation. - let dev_cfg = if caps_coll.com_cfg.check_features() { - info!("Features have been negotiated between virtio console device and driver.",); - // Set feature set in device config fur future use. - ConsoleDevCfg { - raw: dev_cfg_raw, - features: negotiated_features, - } - } else { - error!("The device does not support our subset of features."); - return Err((VirtioConsoleError::FailFeatureNeg, caps_coll)); - }; - + ) -> Result { let mut recv_vq = RxQueue::new(); - - // create the queues and tell device about them - recv_vq.add(VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - 0, - dev_cfg.features.into(), - ) - .unwrap(), - )); - // Interrupt for receiving packets is wanted - recv_vq.enable_notifs(); - let mut send_vq = TxQueue::new(); - send_vq.add(VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - 1, - dev_cfg.features.into(), - ) - .unwrap(), - )); - // Interrupt for communicating that a sent packet left, is not needed - send_vq.disable_notifs(); - - match &mut caps_coll.int_cap { - InterruptCapability::IsrStatus(_) => { - let irq = irq.unwrap(); - handlers.entry(irq).or_default().push_back(|| { - if let Some(driver) = get_console_driver() { - driver.lock().handle_interrupt(); - }; - }); - crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); - info!("Virtio interrupt handler at line {irq}"); + let dev_cfg = match caps_coll.init_caps(dev_cfg_raw, |caps_coll, dev_cfg| { + // create the queues and tell device about them + recv_vq.add(VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + 0, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + )); + // Interrupt for receiving packets is wanted + recv_vq.enable_notifs(); + + send_vq.add(VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + 1, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + )); + // Interrupt for communicating that a sent packet left, is not needed + send_vq.disable_notifs(); + + match &mut caps_coll.int_cap { + InterruptCapability::IsrStatus(_) => { + let irq = irq.unwrap(); + handlers.entry(irq).or_default().push_back(|| { + if let Some(driver) = get_console_driver() { + driver.lock().handle_interrupt(); + }; + }); + crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); + info!("Virtio interrupt handler at line {irq}"); + } + #[cfg(all(feature = "pci", target_arch = "x86_64"))] + InterruptCapability::Msix(msix_table) => caps_coll.com_cfg.register_msix_vectors( + msix_table, + handlers, + || { + if let Some(driver) = get_console_driver() { + driver.lock().handle_device_configuration_interrupt(); + }; + }, + [(0..2u16, Self::handle_queue_interrupt as fn())].into_iter(), + [], + ), } - #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => caps_coll.com_cfg.register_msix_vectors( - msix_table, - handlers, - || { - if let Some(driver) = get_console_driver() { - driver.lock().handle_device_configuration_interrupt(); - }; - }, - [(0..2u16, Self::handle_queue_interrupt as fn())].into_iter(), - [], - ), - } - - // At this point the device is "live" - caps_coll.com_cfg.drv_ok(); + Ok(()) + }) { + Ok(dev_cfg) => dev_cfg, + Err(err) => return Err((err, caps_coll)), + }; Ok(Self { dev_cfg, @@ -456,11 +418,5 @@ pub mod error { "Virtio console device driver failed, for device {0:x}, due to a missing or malformed device config!" )] NoDevCfg(u16), - - /// The device did not acknowledge the negotiated feature set. - #[error( - "Virtio console device driver failed, device did not acknowledge negotiated feature set!" - )] - FailFeatureNeg, } } diff --git a/src/drivers/fs.rs b/src/drivers/fs.rs index 5d12c157a6..d2e9e436ea 100644 --- a/src/drivers/fs.rs +++ b/src/drivers/fs.rs @@ -25,8 +25,7 @@ use crate::config::VIRTIO_MAX_QUEUE_SIZE; use crate::drivers::mmio::get_filesystem_driver; #[cfg(feature = "pci")] use crate::drivers::pci::get_filesystem_driver; -use crate::drivers::virtio::ControlRegisters; -use crate::drivers::virtio::error::VirtioFsInitError; +use crate::drivers::virtio::error::{VirtioError, VirtioFsInitError}; use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::error::VirtqError; use crate::drivers::virtio::virtqueue::split::SplitVq; @@ -38,13 +37,7 @@ use crate::errno::Errno; use crate::fs::virtio_fs::{self, Rsp, RspHeader, VirtioFsError, VirtioFsInterface}; use crate::mm::device_alloc::DeviceAlloc; -/// A wrapper struct for the raw configuration structure. -/// Handling the right access to fields, as some are read-only -/// for the driver. -pub(crate) struct FsDevCfg { - pub raw: VolatileRef<'static, virtio::fs::Config, ReadOnly>, - pub features: virtio::fs::F, -} +type FsDevCfg = super::virtio::DevCfg; /// Virtio file system driver struct. /// @@ -61,6 +54,10 @@ pub(crate) struct VirtioFsDriver { impl super::virtio::VirtioDriver for VirtioFsDriver { type Config = virtio::fs::Config; type Error = VirtioFsInitError; + type DeviceFeatures = virtio::fs::F; + + const MINIMAL_FEATURES: Self::DeviceFeatures = virtio::fs::F::VERSION_1; + const OPTIONAL_FEATURES: Self::DeviceFeatures = virtio::fs::F::empty(); /// Initializes the device in adherence to specification. Returns Some(VirtioFsError) /// upon failure and None in case everything worked as expected. @@ -74,99 +71,65 @@ impl super::virtio::VirtioDriver for VirtioFsDriver { ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result { - // Reset - caps_coll.com_cfg.reset_dev(); - - // Indicate device, that OS noticed it - caps_coll.com_cfg.ack_dev(); - - // Indicate device, that driver is able to handle it - caps_coll.com_cfg.set_drv(); - - let minimal_features = virtio::fs::F::VERSION_1; - let negotiated_features = caps_coll - .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((VirtioFsInitError::FailFeatureNeg, caps_coll)); - } - - // Indicates the device, that the current feature set is final for the driver - // and will not be changed. - caps_coll.com_cfg.features_ok(); - - // Checks if the device has accepted final set. This finishes feature negotiation. - let dev_cfg = if caps_coll.com_cfg.check_features() { - info!("Features have been negotiated between virtio filesystem device and driver.",); - // Set feature set in device config fur future use. - FsDevCfg { - raw: dev_cfg_raw, - features: negotiated_features, - } - } else { - error!("The device does not support our subset of features."); - return Err((VirtioFsInitError::FailFeatureNeg, caps_coll)); - }; - - // 1 highprio queue, and n normal request queues - let vqnum = dev_cfg.raw.as_ptr().num_request_queues().read().to_ne() + 1; - if vqnum == 0 { - error!("0 request queues requested from device. Aborting!"); - return Err((VirtioFsInitError::Unknown, caps_coll)); - } - + ) -> Result { let mut vqueues = Vec::new(); - // create the queues and tell device about them - for i in 0..vqnum as u16 { - let vq = VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - i, - dev_cfg.features.into(), - ) - .unwrap(), - ); - vqueues.push(vq); - } + let dev_cfg = match caps_coll.init_caps(dev_cfg_raw, |caps_coll, dev_cfg| { + // 1 highprio queue, and n normal request queues + let vqnum = dev_cfg.raw.as_ptr().num_request_queues().read().to_ne() + 1; + if vqnum == 0 { + error!("0 request queues requested from device. Aborting!"); + return Err(VirtioFsInitError::Unknown); + } - match &mut caps_coll.int_cap { - InterruptCapability::IsrStatus(_) => { - let irq = irq.unwrap(); - handlers.entry(irq).or_default().push_back(|| { - if let Some(driver) = get_filesystem_driver() { - driver.lock().handle_interrupt(); - }; - }); - crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); - info!("Virtio interrupt handler at line {irq}"); + // create the queues and tell device about them + for i in 0..vqnum as u16 { + let vq = VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + i, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + ); + vqueues.push(vq); } - #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => { - use core::iter; - caps_coll.com_cfg.register_msix_vectors( - msix_table, - handlers, - || { + match &mut caps_coll.int_cap { + InterruptCapability::IsrStatus(_) => { + let irq = irq.unwrap(); + handlers.entry(irq).or_default().push_back(|| { if let Some(driver) = get_filesystem_driver() { - driver.lock().handle_device_configuration_interrupt(); + driver.lock().handle_interrupt(); }; - }, - iter::empty::<(iter::Empty<_>, _)>(), - 0..vqnum as u16, - ); + }); + crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); + info!("Virtio interrupt handler at line {irq}"); + } + #[cfg(all(feature = "pci", target_arch = "x86_64"))] + InterruptCapability::Msix(msix_table) => { + use core::iter; + + caps_coll.com_cfg.register_msix_vectors( + msix_table, + handlers, + || { + if let Some(driver) = get_filesystem_driver() { + driver.lock().handle_device_configuration_interrupt(); + }; + }, + iter::empty::<(iter::Empty<_>, _)>(), + 0..vqnum as u16, + ); + } } - } - - // At this point the device is "live" - caps_coll.com_cfg.drv_ok(); + Ok(()) + }) { + Ok(dev_cfg) => dev_cfg, + Err(err) => return Err((err, caps_coll)), + }; Ok(Self { dev_cfg, @@ -315,11 +278,6 @@ pub mod error { )] NoDevCfg(u16), - #[error( - "Virtio filesystem driver failed, device did not acknowledge negotiated feature set!" - )] - FailFeatureNeg, - #[error("Virtio filesystem failed, driver failed due unknown reason!")] Unknown, } diff --git a/src/drivers/net/virtio.rs b/src/drivers/net/virtio.rs index b017a72fbe..79f1db5f96 100644 --- a/src/drivers/net/virtio.rs +++ b/src/drivers/net/virtio.rs @@ -27,7 +27,7 @@ use self::error::VirtioNetError; use crate::config::VIRTIO_MAX_QUEUE_SIZE; use crate::drivers::net::virtio::constants::BUFF_PER_PACKET; use crate::drivers::net::{NetworkDriver, mtu}; -use crate::drivers::virtio::ControlRegisters; +use crate::drivers::virtio::error::VirtioError; use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::packed::PackedVq; use crate::drivers::virtio::virtqueue::split::SplitVq; @@ -38,13 +38,7 @@ use crate::drivers::{Driver, InterruptHandlerMap, InterruptLine}; use crate::executor::network::wake_network_waker; use crate::mm::device_alloc::DeviceAlloc; -/// A wrapper struct for the raw configuration structure. -/// Handling the right access to fields, as some are read-only -/// for the driver. -pub(crate) struct NetDevCfg { - pub raw: VolatileRef<'static, virtio::net::Config, ReadOnly>, - pub features: virtio::net::F, -} +type NetDevCfg = crate::drivers::virtio::DevCfg; fn determine_mtu(dev_cfg: &NetDevCfg) -> u16 { // If VIRTIO_NET_F_MTU is negotiated, "the driver uses mtu as the maximum MTU value" @@ -98,7 +92,7 @@ pub struct RxQueues { } impl RxQueues { - pub fn new(vqs: Vec, dev_cfg: &NetDevCfg) -> Self { + fn new(vqs: Vec, dev_cfg: &NetDevCfg) -> Self { Self { vqs, buf_size: determine_rx_buf_size(dev_cfg), @@ -173,7 +167,7 @@ pub struct TxQueues { } impl TxQueues { - pub fn new(vqs: Vec, dev_cfg: &NetDevCfg) -> Self { + fn new(vqs: Vec, dev_cfg: &NetDevCfg) -> Self { Self { vqs, buf_size: determine_mtu(dev_cfg).into(), @@ -748,6 +742,34 @@ impl VirtioNetDriver { impl crate::drivers::virtio::VirtioDriver for VirtioNetDriver { type Config = virtio::net::Config; type Error = VirtioNetError; + type DeviceFeatures = virtio::net::F; + + const MINIMAL_FEATURES: Self::DeviceFeatures = + virtio::net::F::VERSION_1.union(virtio::net::F::MAC); + // If wanted, push new features into feats here: + const OPTIONAL_FEATURES: Self::DeviceFeatures = + // Indirect descriptors can be used + virtio::net::F::INDIRECT_DESC + // Packed Vq can be used + .union(virtio::net::F::RING_PACKED) + .union(virtio::net::F::NOTIFICATION_DATA) + // MTU setting can be used + .union(virtio::net::F::MTU) + // Driver can merge receive buffers + .union(virtio::net::F::MRG_RXBUF) + // the link status can be announced + .union(virtio::net::F::STATUS) + // control queue support + .union(virtio::net::F::CTRL_VQ) + // Multiqueue support + .union(virtio::net::F::MQ) + // Checksum calculation can partially be offloaded to the device + .union(virtio::net::F::CSUM) + // Partially checksummed frames can be received + .union(virtio::net::F::GUEST_CSUM) + // Frames with coalesced TCP segments can be received + .union(virtio::net::F::GUEST_TSO4) + .union(virtio::net::F::GUEST_TSO6); /// Initializes the device in adherence to specification. Returns Some(VirtioNetError) /// upon failure and None in case everything worked as expected. @@ -761,116 +783,62 @@ impl crate::drivers::virtio::VirtioDriver for VirtioNetDriver { ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result { - // Reset - caps_coll.com_cfg.reset_dev(); - - // Indicate device, that OS noticed it - caps_coll.com_cfg.ack_dev(); - - // Indicate device, that driver is able to handle it - caps_coll.com_cfg.set_drv(); - - let minimal_features = virtio::net::F::VERSION_1 | virtio::net::F::MAC; - - // If wanted, push new features into feats here: - let features = minimal_features - // Indirect descriptors can be used - | virtio::net::F::INDIRECT_DESC - // Packed Vq can be used - | virtio::net::F::RING_PACKED - | virtio::net::F::NOTIFICATION_DATA - // MTU setting can be used - | virtio::net::F::MTU - // Driver can merge receive buffers - | virtio::net::F::MRG_RXBUF - // the link status can be announced - | virtio::net::F::STATUS - // control queue support - | virtio::net::F::CTRL_VQ - // Multiqueue support - | virtio::net::F::MQ - // Checksum calculation can partially be offloaded to the device - | virtio::net::F::CSUM - // Partially checksummed frames can be received - | virtio::net::F::GUEST_CSUM - // Frames with coalesced TCP segments can be received - | virtio::net::F::GUEST_TSO4 - | virtio::net::F::GUEST_TSO6; - - let negotiated_features = caps_coll - .com_cfg - .control_registers() - .negotiate_features(features); - - if !negotiated_features.contains(minimal_features) { - error!("Device features set, does not satisfy minimal features needed. Aborting!"); - return Err((VirtioNetError::FailFeatureNeg, caps_coll)); - } - - // Indicates the device, that the current feature set is final for the driver - // and will not be changed. - caps_coll.com_cfg.features_ok(); - - // Checks if the device has accepted final set. This finishes feature negotiation. - let dev_cfg = if caps_coll.com_cfg.check_features() { - info!("Features have been negotiated between virtio network device and driver.",); - // Set feature set in device config fur future use. - NetDevCfg { - raw: dev_cfg_raw, - features: negotiated_features, + ) -> Result { + let mut mtu = None; + let mut recv_vqs = None; + let mut send_vqs = None; + let mut ctrl_vq = None; + let mut send_capacity = None; + + let dev_cfg = match caps_coll.init_caps(dev_cfg_raw, |caps_coll, dev_cfg| { + mtu = Some(determine_mtu(dev_cfg)); + let dev_spec_init = Self::dev_spec_init(caps_coll, dev_cfg)?; + debug!("Using RX buffer size of {}", dev_spec_init.0.buf_size); + recv_vqs = Some(dev_spec_init.0); + send_vqs = Some(dev_spec_init.1); + #[cfg_attr( + not(all(feature = "pci", target_arch = "x86_64")), + expect(unused_variables) + )] + let num_vqs = dev_spec_init.2; + ctrl_vq = Some(dev_spec_init.3); + send_capacity = Some(dev_spec_init.4); + + info!("Device specific initialization for Virtio network device finished",); + + match &mut caps_coll.int_cap { + InterruptCapability::IsrStatus(_) => { + let irq = irq.unwrap(); + handlers + .entry(irq) + .or_default() + .push_back(crate::executor::network::network_handler); + crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); + info!("Virtio interrupt handler at line {irq}"); + } + #[cfg(all(feature = "pci", target_arch = "x86_64"))] + InterruptCapability::Msix(msix_table) => { + let recv_vqs = (0..num_vqs).step_by(2); + let send_vqs = (1..num_vqs).step_by(2); + let ctrl_vq = dev_cfg + .features + .contains(virtio::net::F::CTRL_VQ) + .then_some(num_vqs); + caps_coll.com_cfg.register_msix_vectors( + msix_table, + handlers, + crate::executor::network::network_device_configuration_handler, + [(recv_vqs, wake_network_waker as fn())].into_iter(), + send_vqs.chain(ctrl_vq), + ); + } } - } else { - error!("The device does not support our subset of features."); - return Err((VirtioNetError::FailFeatureNeg, caps_coll)); + Ok(()) + }) { + Ok(dev_cfg) => dev_cfg, + Err(err) => return Err((err, caps_coll)), }; - let mtu = determine_mtu(&dev_cfg); - #[cfg_attr( - not(all(feature = "pci", target_arch = "x86_64")), - expect(unused_variables) - )] - let (recv_vqs, send_vqs, num_vqs, ctrl_vq, send_capacity) = - match Self::dev_spec_init(&mut caps_coll, &dev_cfg) { - Ok(it) => it, - Err(err) => return Err((err, caps_coll)), - }; - - debug!("Using RX buffer size of {}", recv_vqs.buf_size); - - info!("Device specific initialization for Virtio network device finished",); - - match &mut caps_coll.int_cap { - InterruptCapability::IsrStatus(_) => { - let irq = irq.unwrap(); - handlers - .entry(irq) - .or_default() - .push_back(crate::executor::network::network_handler); - crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); - info!("Virtio interrupt handler at line {irq}"); - } - #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => { - let recv_vqs = (0..num_vqs).step_by(2); - let send_vqs = (1..num_vqs).step_by(2); - let ctrl_vq = dev_cfg - .features - .contains(virtio::net::F::CTRL_VQ) - .then_some(num_vqs); - caps_coll.com_cfg.register_msix_vectors( - msix_table, - handlers, - crate::executor::network::network_device_configuration_handler, - [(recv_vqs, wake_network_waker as fn())].into_iter(), - send_vqs.chain(ctrl_vq), - ); - } - } - - // At this point the device is "live" - caps_coll.com_cfg.drv_ok(); - let mut checksums = ChecksumCapabilities::default(); // Not only should we offload receive checksum validation to the device for performance when possible, we MUST // offload it when GUEST_TSO{4,6} are enabled, since otherwise the coalesced packets will be rejected by smoltcp @@ -892,11 +860,11 @@ impl crate::drivers::virtio::VirtioDriver for VirtioNetDriver { Ok(VirtioNetDriver { dev_cfg, caps_coll, - mtu, - ctrl_vq, - recv_vqs, - send_vqs, - send_capacity, + mtu: mtu.unwrap(), + ctrl_vq: ctrl_vq.unwrap(), + recv_vqs: recv_vqs.unwrap(), + send_vqs: send_vqs.unwrap(), + send_capacity: send_capacity.unwrap(), checksums, }) } @@ -1069,9 +1037,6 @@ pub mod error { "Virtio network driver failed, for device {0:x}, due to a missing or malformed device config!" )] NoDevCfg(u16), - - #[error("Virtio network driver failed, device did not acknowledge negotiated feature set!")] - FailFeatureNeg, } } diff --git a/src/drivers/virtio/mod.rs b/src/drivers/virtio/mod.rs index 320b037ea4..c6f0eb1913 100644 --- a/src/drivers/virtio/mod.rs +++ b/src/drivers/virtio/mod.rs @@ -17,8 +17,12 @@ pub mod virtqueue; use core::fmt; +use bitflags::Flags; use virtio::FeatureBits; +use crate::drivers::virtio::error::VirtioError; +use crate::drivers::virtio::transport::UniCapsColl; + trait VirtioIdExt { fn as_feature(&self) -> Option<&str>; } @@ -162,23 +166,103 @@ where } } -pub(super) trait VirtioDriver: super::Driver + Sized { +pub(super) trait VirtioDriver: super::Driver + Sized +where + virtio::F: + From + AsRef + AsMut, +{ type Config: 'static; - type Error: Into; + type Error: Into; + type DeviceFeatures: FeatureBits + fmt::Debug + Copy; + + const MINIMAL_FEATURES: Self::DeviceFeatures; + const OPTIONAL_FEATURES: Self::DeviceFeatures; fn init_dev( caps_tuple: ( - transport::UniCapsColl, + UniCapsColl, volatile::VolatileRef<'static, Self::Config, volatile::access::ReadOnly>, ), handlers: &mut super::InterruptHandlerMap, irq: Option, - ) -> Result; + ) -> Result; #[cfg(feature = "pci")] fn no_dev_cfg_err(dev_id: u16) -> Self::Error; } +impl UniCapsColl { + pub(super) fn init_caps( + &mut self, + dev_cfg_raw: volatile::VolatileRef<'static, T::Config, volatile::access::ReadOnly>, + mut device_specific_setup: impl FnMut(&mut Self, &mut DevCfg) -> Result<(), T::Error>, + ) -> Result, VirtioError> + where + virtio::F: From + AsRef + AsMut, + { + // Reset + self.com_cfg.reset_dev(); + + // Indicate device, that OS noticed it + self.com_cfg.ack_dev(); + + // Indicate device, that driver is able to handle it + self.com_cfg.set_drv(); + + let negotiated_features = self + .com_cfg + .control_registers() + .negotiate_features(T::MINIMAL_FEATURES.union(T::OPTIONAL_FEATURES)); + + if !negotiated_features.contains(T::MINIMAL_FEATURES) { + error!("Device features set, does not satisfy minimal features needed. Aborting!"); + return Err(VirtioError::FailFeatureNeg); + } + + // Indicates the device, that the current feature set is final for the driver + // and will not be changed. + self.com_cfg.features_ok(); + + // Checks if the device has accepted final set. This finishes feature negotiation. + let mut dev_cfg = if self.com_cfg.check_features() { + info!( + "Features have been negotiated between {} device and driver.", + T::get_name() + ); + // Set feature set in device config for future use. + DevCfg { + raw: dev_cfg_raw, + features: negotiated_features, + } + } else { + error!("The device does not support our subset of features."); + return Err(VirtioError::FailFeatureNeg); + }; + + device_specific_setup(self, &mut dev_cfg).map_err(|err| err.into())?; + + // At this point the device is "live" + self.com_cfg.drv_ok(); + + Ok(dev_cfg) + } +} + +/// A wrapper struct for the raw configuration structure. +/// Handling the right access to fields, as some are read-only +/// for the driver. +pub(super) struct DevCfg +where + virtio::F: From + AsRef + AsMut, +{ + pub(super) features: T::DeviceFeatures, + #[cfg_attr( + all(feature = "virtio-console", not(feature = "pci")), + expect(dead_code) + )] + pub(super) raw: volatile::VolatileRef<'static, T::Config, volatile::access::ReadOnly>, +} + pub mod error { use thiserror::Error; @@ -224,6 +308,9 @@ pub mod error { #[error("Device with id {0:#x} not supported.")] DevNotSupported(u16), + #[error("Virtio driver failed, device did not acknowledge negotiated feature set!")] + FailFeatureNeg, + #[cfg(all( not(all(target_arch = "riscv64", feature = "gem-net", not(feature = "pci"))), not(feature = "rtl8139"), diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 951f0c423b..c80ec10bc4 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -455,8 +455,9 @@ fn init( registers: VolatileRef<'static, DeviceRegisters>, irq: InterruptLine, handlers: &mut InterruptHandlerMap, -) -> Result { - let drv = - T::init_dev(map_caps(registers), handlers, Some(irq)).map_err(|(err, _)| err.into())?; - Ok(drv) +) -> Result +where + virtio::F: From + AsRef + AsMut, +{ + T::init_dev(map_caps(registers), handlers, Some(irq)).map_err(|(err, _)| err) } diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 11f6120fee..3bab3f09a6 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -852,6 +852,7 @@ fn init( where T: crate::drivers::virtio::VirtioDriver, T::Config: CapCfg, + virtio::F: From + AsRef + AsMut, { // enable bus master mode device.set_command(pci_types::CommandRegister::BUS_MASTER_ENABLE); @@ -878,7 +879,7 @@ where } Err((err, mut caps_coll)) => { caps_coll.com_cfg.set_failed(); - Err(err.into()) + Err(err) } } } diff --git a/src/drivers/vsock.rs b/src/drivers/vsock.rs index d13c739ce0..1c87189812 100644 --- a/src/drivers/vsock.rs +++ b/src/drivers/vsock.rs @@ -15,8 +15,7 @@ use crate::config::{VIRTIO_MAX_QUEUE_SIZE, VSOCK_PACKET_SIZE}; use crate::drivers::mmio::get_vsock_driver; #[cfg(feature = "pci")] use crate::drivers::pci::get_vsock_driver; -use crate::drivers::virtio::ControlRegisters; -use crate::drivers::virtio::error::VirtioVsockError; +use crate::drivers::virtio::error::{VirtioError, VirtioVsockError}; use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::split::SplitVq; use crate::drivers::virtio::virtqueue::{ @@ -238,13 +237,7 @@ impl EventQueue { } } -/// A wrapper struct for the raw configuration structure. -/// Handling the right access to fields, as some are read-only -/// for the driver. -pub(crate) struct VsockDevCfg { - pub raw: VolatileRef<'static, virtio::vsock::Config, ReadOnly>, - pub features: virtio::vsock::F, -} +type VsockDevCfg = super::virtio::DevCfg; pub(crate) struct VirtioVsockDriver { pub(super) dev_cfg: VsockDevCfg, @@ -310,6 +303,10 @@ impl VirtioVsockDriver { impl super::virtio::VirtioDriver for VirtioVsockDriver { type Config = virtio::vsock::Config; type Error = VirtioVsockError; + type DeviceFeatures = virtio::vsock::F; + + const MINIMAL_FEATURES: Self::DeviceFeatures = virtio::vsock::F::VERSION_1; + const OPTIONAL_FEATURES: Self::DeviceFeatures = virtio::vsock::F::empty(); /// Initializes the device in adherence to specification. Returns Some(VirtioVsockError) /// upon failure and None in case everything worked as expected. @@ -323,120 +320,87 @@ impl super::virtio::VirtioDriver for VirtioVsockDriver { ), handlers: &mut InterruptHandlerMap, irq: Option, - ) -> Result { - // Reset - caps_coll.com_cfg.reset_dev(); - - // Indicate device, that OS noticed it - caps_coll.com_cfg.ack_dev(); - - // Indicate device, that driver is able to handle it - caps_coll.com_cfg.set_drv(); - - let minimal_features = virtio::vsock::F::VERSION_1; - let negotiated_features = caps_coll - .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((VirtioVsockError::FailFeatureNeg, caps_coll)); - } - - // Indicates the device, that the current feature set is final for the driver - // and will not be changed. - caps_coll.com_cfg.features_ok(); - - // Checks if the device has accepted final set. This finishes feature negotiation. - let dev_cfg = if caps_coll.com_cfg.check_features() { - info!("Features have been negotiated between virtio socket device and driver.",); - // Set feature set in device config fur future use. - VsockDevCfg { - raw: dev_cfg_raw, - features: negotiated_features, - } - } else { - error!("The device does not support our subset of features."); - return Err((VirtioVsockError::FailFeatureNeg, caps_coll)); - }; - + ) -> Result { let mut recv_vq = RxQueue::new(); - // create the queues and tell device about them - recv_vq.add(VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - 0, - dev_cfg.features.into(), - ) - .unwrap(), - )); - // Interrupt for receiving packets is wanted - recv_vq.enable_notifs(); - let mut send_vq = TxQueue::new(); - send_vq.add(VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - 1, - dev_cfg.features.into(), - ) - .unwrap(), - )); - // Interrupt for communicating that a sent packet left, is not needed - send_vq.disable_notifs(); - let mut event_vq = EventQueue::new(); - // create the queues and tell device about them - event_vq.add(VirtQueue::Split( - SplitVq::new( - &mut caps_coll.com_cfg, - &caps_coll.notif_cfg, - VIRTIO_MAX_QUEUE_SIZE, - 2, - dev_cfg.features.into(), - ) - .unwrap(), - )); - - match &mut caps_coll.int_cap { - InterruptCapability::IsrStatus(_) => { - let irq = irq.unwrap(); - handlers.entry(irq).or_default().push_back(|| { - if let Some(driver) = get_vsock_driver() { - driver.lock().handle_interrupt(); - }; - }); - crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); - info!("Virtio interrupt handler at line {irq}"); - } - #[cfg(all(feature = "pci", target_arch = "x86_64"))] - InterruptCapability::Msix(msix_table) => { - caps_coll.com_cfg.register_msix_vectors( - msix_table, - handlers, - || { + let dev_cfg = match caps_coll.init_caps(dev_cfg_raw, |caps_coll, dev_cfg| { + // create the queues and tell device about them + recv_vq.add(VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + 0, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + )); + // Interrupt for receiving packets is wanted + recv_vq.enable_notifs(); + + send_vq.add(VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + 1, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + )); + // Interrupt for communicating that a sent packet left, is not needed + send_vq.disable_notifs(); + + // create the queues and tell device about them + event_vq.add(VirtQueue::Split( + SplitVq::new( + &mut caps_coll.com_cfg, + &caps_coll.notif_cfg, + VIRTIO_MAX_QUEUE_SIZE, + 2, + virtio::F::from(dev_cfg.features), + ) + .unwrap(), + )); + + match &mut caps_coll.int_cap { + InterruptCapability::IsrStatus(_) => { + let irq = irq.unwrap(); + handlers.entry(irq).or_default().push_back(|| { if let Some(driver) = get_vsock_driver() { - driver.lock().handle_device_configuration_interrupt(); + driver.lock().handle_interrupt(); }; - }, - // The no-op handler allows the processor to receive an interrupt and reschedule. - // FIXME: replace with a function to wake the vsock task waker once it is not woken unconditionally. - [([0], (|| {}) as fn())].into_iter(), - 1..3, - ); + }); + crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); + info!("Virtio interrupt handler at line {irq}"); + } + #[cfg(all(feature = "pci", target_arch = "x86_64"))] + InterruptCapability::Msix(msix_table) => { + caps_coll.com_cfg.register_msix_vectors( + msix_table, + handlers, + || { + if let Some(driver) = get_vsock_driver() { + driver.lock().handle_device_configuration_interrupt(); + }; + }, + // The no-op handler allows the processor to receive an interrupt and reschedule. + // FIXME: replace with a function to wake the vsock task waker once it is not woken unconditionally. + [([0], (|| {}) as fn())].into_iter(), + 1..3, + ); + } } - } - // Interrupt for event packets is wanted - event_vq.enable_notifs(); + // Interrupt for event packets is wanted + event_vq.enable_notifs(); - // At this point the device is "live" - caps_coll.com_cfg.drv_ok(); + Ok(()) + }) { + Ok(dev_cfg) => dev_cfg, + Err(err) => return Err((err, caps_coll)), + }; Ok(Self { dev_cfg, @@ -480,6 +444,7 @@ pub mod error { /// Virtio socket device error enum. #[derive(Error, Debug, Copy, Clone)] + #[allow(clippy::enum_variant_names)] pub enum VirtioVsockError { #[error( "Virtio socket device driver failed, for device {0:x}, due to a missing or malformed device config!" @@ -500,10 +465,5 @@ pub mod error { "Virtio socket device driver failed, for device {0:x}, due to a missing or malformed notification config!" )] NoNotifCfg(u16), - - #[error( - "Virtio socket device driver failed, device did not acknowledge negotiated feature set!" - )] - FailFeatureNeg, } } From d1e4d1d6ed2a176cc4e8091a157d88f403ce3559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Thu, 16 Jul 2026 18:42:42 +0200 Subject: [PATCH 9/9] refactor(virtio-console): remove VirtioUART wrapper Rather than accessing the virtio-console driver through the global driver vector, place it into the using struct in a way similar to what was done in b735657. --- src/arch/aarch64/kernel/mmio.rs | 57 +++++---------------------- src/arch/riscv64/kernel/devicetree.rs | 24 +---------- src/arch/riscv64/kernel/mmio.rs | 39 +++++------------- src/arch/x86_64/kernel/mmio.rs | 52 +++++------------------- src/console/mod.rs | 16 ++++---- src/drivers/console.rs | 57 ++++----------------------- src/drivers/mmio.rs | 2 - src/drivers/pci.rs | 18 +-------- 8 files changed, 49 insertions(+), 216 deletions(-) diff --git a/src/arch/aarch64/kernel/mmio.rs b/src/arch/aarch64/kernel/mmio.rs index 4c6730ffe8..d6e101449e 100644 --- a/src/arch/aarch64/kernel/mmio.rs +++ b/src/arch/aarch64/kernel/mmio.rs @@ -15,13 +15,7 @@ use volatile::VolatileRef; use crate::arch::aarch64::kernel::interrupts::GIC; use crate::arch::aarch64::mm::paging::{self, PageSize}; -#[cfg(feature = "virtio-console")] -use crate::console::IoDevice; use crate::drivers::InterruptHandlerMap; -#[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioConsoleDriver; -#[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioUART; #[cfg(feature = "virtio-fs")] use crate::drivers::fs::VirtioFsDriver; #[cfg(feature = "virtio-net")] @@ -43,10 +37,8 @@ use crate::mm::PhysAddr; pub(crate) static MMIO_DRIVERS: InitCell> = InitCell::new(Vec::new()); -#[allow(clippy::enum_variant_names)] +#[allow(clippy::enum_variant_names, clippy::large_enum_variant)] pub(crate) enum MmioDriver { - #[cfg(feature = "virtio-console")] - VirtioConsole(InterruptTicketMutex), #[cfg(feature = "virtio-fs")] VirtioFs(InterruptTicketMutex), #[cfg(feature = "virtio-vsock")] @@ -54,30 +46,21 @@ pub(crate) enum MmioDriver { } impl MmioDriver { - #[cfg(feature = "virtio-console")] - fn get_console_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioConsole(drv) => Some(drv), - _ => None, - } - } - #[cfg(feature = "virtio-fs")] fn get_filesystem_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioFs(drv) => Some(drv), - _ => None, + if let Self::VirtioFs(drv) = self { + Some(drv) + } else { + None } } #[cfg(feature = "virtio-vsock")] fn get_vsock_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioVsock(drv) => Some(drv), - _ => None, + if let Self::VirtioVsock(drv) = self { + Some(drv) + } else { + None } } } @@ -94,14 +77,6 @@ pub(crate) fn register_driver(drv: MmioDriver) { #[cfg(feature = "virtio-net")] pub(crate) type NetworkDevice = VirtioNetDriver; -#[cfg(feature = "virtio-console")] -pub(crate) fn get_console_driver() -> Option<&'static InterruptTicketMutex> { - MMIO_DRIVERS - .get()? - .iter() - .find_map(|drv| drv.get_console_driver()) -} - #[cfg(feature = "virtio-fs")] pub(crate) fn get_filesystem_driver() -> Option<&'static InterruptTicketMutex> { MMIO_DRIVERS @@ -231,9 +206,7 @@ pub fn init_drivers(handlers: &mut InterruptHandlerMap) { match drv { #[cfg(feature = "virtio-console")] - VirtioDriver::Console(drv) => register_driver(MmioDriver::VirtioConsole( - InterruptTicketMutex::new(*drv), - )), + VirtioDriver::Console(drv) => crate::console::switch_to_virtio(*drv), #[cfg(feature = "virtio-fs")] VirtioDriver::Fs(drv) => { register_driver(MmioDriver::VirtioFs(InterruptTicketMutex::new(*drv))); @@ -251,14 +224,4 @@ pub fn init_drivers(handlers: &mut InterruptHandlerMap) { }); MMIO_DRIVERS.finalize(); - - #[cfg(feature = "virtio-console")] - { - if get_console_driver().is_some() { - info!("Switch to virtio console"); - crate::console::CONSOLE - .lock() - .replace_device(IoDevice::Virtio(VirtioUART::new())); - } - } } diff --git a/src/arch/riscv64/kernel/devicetree.rs b/src/arch/riscv64/kernel/devicetree.rs index 3b795c996e..f6aa565613 100644 --- a/src/arch/riscv64/kernel/devicetree.rs +++ b/src/arch/riscv64/kernel/devicetree.rs @@ -20,17 +20,9 @@ use crate::arch::riscv64::kernel::interrupts::init_plic; ))] use crate::arch::riscv64::kernel::mmio::MmioDriver; use crate::arch::riscv64::mm::paging::{self, PageSize}; -#[cfg(feature = "virtio-console")] -use crate::console::IoDevice; use crate::drivers::InterruptHandlerMap; -#[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioUART; -#[cfg(all(feature = "virtio-console", not(feature = "pci")))] -use crate::drivers::mmio::get_console_driver; #[cfg(all(feature = "gem-net", not(feature = "pci")))] use crate::drivers::net::gem; -#[cfg(all(feature = "virtio-console", feature = "pci"))] -use crate::drivers::pci::get_console_driver; #[cfg(all(feature = "virtio", not(feature = "pci")))] use crate::drivers::virtio::transport::mmio as mmio_virtio; #[cfg(all( @@ -259,11 +251,7 @@ pub fn init_drivers(handlers: &mut InterruptHandlerMap) { match mmio_virtio::init_device(mmio, irq.try_into().unwrap(), handlers) { #[cfg(feature = "virtio-console")] - Ok(VirtioDriver::Console(drv)) => { - register_driver(MmioDriver::VirtioConsole( - hermit_sync::InterruptSpinMutex::new(*drv), - )); - } + Ok(VirtioDriver::Console(drv)) => crate::console::switch_to_virtio(*drv), #[cfg(feature = "virtio-fs")] Ok(VirtioDriver::Fs(drv)) => { register_driver(MmioDriver::VirtioFs( @@ -288,14 +276,4 @@ pub fn init_drivers(handlers: &mut InterruptHandlerMap) { #[cfg(all(any(feature = "virtio", feature = "gem-net"), not(feature = "pci")))] super::mmio::MMIO_DRIVERS.finalize(); - - #[cfg(feature = "virtio-console")] - { - if get_console_driver().is_some() { - info!("Switch to virtio console"); - crate::console::CONSOLE - .lock() - .replace_device(IoDevice::Virtio(VirtioUART::new())); - } - } } diff --git a/src/arch/riscv64/kernel/mmio.rs b/src/arch/riscv64/kernel/mmio.rs index 67928a388e..df3eae14f0 100644 --- a/src/arch/riscv64/kernel/mmio.rs +++ b/src/arch/riscv64/kernel/mmio.rs @@ -7,8 +7,6 @@ use alloc::vec::Vec; ))] use hermit_sync::InterruptSpinMutex; -#[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioConsoleDriver; #[cfg(feature = "virtio-fs")] use crate::drivers::fs::VirtioFsDriver; #[cfg(feature = "gem-net")] @@ -21,10 +19,8 @@ use crate::init_cell::InitCell; pub(crate) static MMIO_DRIVERS: InitCell> = InitCell::new(Vec::new()); -#[allow(clippy::enum_variant_names)] +#[allow(clippy::enum_variant_names, clippy::large_enum_variant)] pub(crate) enum MmioDriver { - #[cfg(feature = "virtio-console")] - VirtioConsole(InterruptSpinMutex), #[cfg(feature = "virtio-fs")] VirtioFs(InterruptSpinMutex), #[cfg(feature = "virtio-vsock")] @@ -32,30 +28,21 @@ pub(crate) enum MmioDriver { } impl MmioDriver { - #[cfg(feature = "virtio-console")] - fn get_console_driver(&self) -> Option<&InterruptSpinMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioConsole(drv) => Some(drv), - _ => None, - } - } - #[cfg(feature = "virtio-fs")] fn get_filesystem_driver(&self) -> Option<&InterruptSpinMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioFs(drv) => Some(drv), - _ => None, + if let Self::VirtioFs(drv) = self { + Some(drv) + } else { + None } } #[cfg(feature = "virtio-vsock")] fn get_vsock_driver(&self) -> Option<&InterruptSpinMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioVsock(drv) => Some(drv), - _ => None, + if let Self::VirtioVsock(drv) = self { + Some(drv) + } else { + None } } } @@ -75,14 +62,6 @@ pub(crate) type NetworkDevice = GEMDriver; #[cfg(all(not(feature = "gem-net"), feature = "virtio-net"))] pub(crate) type NetworkDevice = VirtioNetDriver; -#[cfg(feature = "virtio-console")] -pub(crate) fn get_console_driver() -> Option<&'static InterruptSpinMutex> { - MMIO_DRIVERS - .get()? - .iter() - .find_map(|drv| drv.get_console_driver()) -} - #[cfg(feature = "virtio-fs")] pub(crate) fn get_filesystem_driver() -> Option<&'static InterruptSpinMutex> { MMIO_DRIVERS diff --git a/src/arch/x86_64/kernel/mmio.rs b/src/arch/x86_64/kernel/mmio.rs index 50bc6871fb..f817b08080 100644 --- a/src/arch/x86_64/kernel/mmio.rs +++ b/src/arch/x86_64/kernel/mmio.rs @@ -21,8 +21,6 @@ use crate::arch::x86_64::mm::paging::{ BasePageSize, PageSize, PageTableEntryFlags, PageTableEntryFlagsExt, }; use crate::drivers::InterruptHandlerMap; -#[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioConsoleDriver; #[cfg(feature = "virtio-fs")] use crate::drivers::fs::VirtioFsDriver; #[cfg(feature = "virtio-net")] @@ -47,10 +45,8 @@ pub const MAGIC_VALUE: u32 = 0x7472_6976; static MMIO_DRIVERS: InitCell> = InitCell::new(Vec::new()); -#[allow(clippy::enum_variant_names)] +#[allow(clippy::enum_variant_names, clippy::large_enum_variant)] pub(crate) enum MmioDriver { - #[cfg(feature = "virtio-console")] - VirtioConsole(InterruptTicketMutex), #[cfg(feature = "virtio-fs")] VirtioFs(InterruptTicketMutex), #[cfg(feature = "virtio-vsock")] @@ -58,30 +54,21 @@ pub(crate) enum MmioDriver { } impl MmioDriver { - #[cfg(feature = "virtio-console")] - fn get_console_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioConsole(drv) => Some(drv), - _ => None, - } - } - #[cfg(feature = "virtio-fs")] fn get_filesystem_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioFs(drv) => Some(drv), - _ => None, + if let Self::VirtioFs(drv) = self { + Some(drv) + } else { + None } } #[cfg(feature = "virtio-vsock")] fn get_vsock_driver(&self) -> Option<&InterruptTicketMutex> { - #[allow(unreachable_patterns)] - match self { - Self::VirtioVsock(drv) => Some(drv), - _ => None, + if let Self::VirtioVsock(drv) = self { + Some(drv) + } else { + None } } } @@ -194,14 +181,6 @@ pub(crate) fn register_driver(drv: MmioDriver) { #[cfg(feature = "virtio-net")] pub(crate) type NetworkDevice = VirtioNetDriver; -#[cfg(feature = "virtio-console")] -pub(crate) fn get_console_driver() -> Option<&'static InterruptTicketMutex> { - MMIO_DRIVERS - .get()? - .iter() - .find_map(|drv| drv.get_console_driver()) -} - #[cfg(feature = "virtio-fs")] pub(crate) fn get_filesystem_driver() -> Option<&'static InterruptTicketMutex> { MMIO_DRIVERS @@ -226,7 +205,7 @@ fn register_mmio( match mmio_virtio::init_device(mmio, irq, handlers) { #[cfg(feature = "virtio-console")] Ok(VirtioDriver::Console(drv)) => { - register_driver(MmioDriver::VirtioConsole(InterruptTicketMutex::new(*drv))); + crate::console::switch_to_virtio(*drv); } #[cfg(feature = "virtio-fs")] Ok(VirtioDriver::Fs(drv)) => { @@ -263,16 +242,5 @@ pub(crate) fn init_drivers(handlers: &mut InterruptHandlerMap) { } MMIO_DRIVERS.finalize(); - - #[cfg(feature = "virtio-console")] - if get_console_driver().is_some() { - use crate::console::IoDevice; - use crate::drivers::console::VirtioUART; - - info!("Switch to virtio console"); - crate::console::CONSOLE - .lock() - .replace_device(IoDevice::Virtio(VirtioUART::new())); - } }); } diff --git a/src/console/mod.rs b/src/console/mod.rs index 4b974d8d50..159fb5bdf8 100644 --- a/src/console/mod.rs +++ b/src/console/mod.rs @@ -9,18 +9,19 @@ use hermit_sync::{InterruptTicketMutex, Lazy}; use crate::arch::kernel::serial::SerialDevice; #[cfg(feature = "virtio-console")] -use crate::drivers::console::VirtioUART; +use crate::drivers::console::VirtioConsoleDriver; use crate::errno::Errno; use crate::executor::WakerRegistration; const SERIAL_BUFFER_SIZE: usize = 256; +#[allow(clippy::large_enum_variant)] pub(crate) enum IoDevice { #[cfg(feature = "uhyve")] Uhyve(uhyve::UhyveSerial), Uart(SerialDevice), #[cfg(feature = "virtio-console")] - Virtio(VirtioUART), + Virtio(VirtioConsoleDriver), } impl ErrorType for IoDevice { @@ -77,7 +78,7 @@ impl Write for IoDevice { } pub(crate) struct Console { - device: IoDevice, + pub device: IoDevice, buffer: Vec, } @@ -88,11 +89,12 @@ impl Console { buffer: Vec::new(), } } +} - #[cfg(feature = "virtio-console")] - pub fn replace_device(&mut self, device: IoDevice) { - self.device = device; - } +#[cfg(feature = "virtio-console")] +pub(crate) fn switch_to_virtio(device: VirtioConsoleDriver) { + info!("Switch to virtio console"); + CONSOLE.lock().device = IoDevice::Virtio(device); } impl ErrorType for Console { diff --git a/src/drivers/console.rs b/src/drivers/console.rs index 7a3098881d..993304a2b9 100644 --- a/src/drivers/console.rs +++ b/src/drivers/console.rs @@ -16,11 +16,8 @@ use volatile::VolatileRef; use volatile::access::ReadOnly; use crate::config::{CONSOLE_PACKET_SIZE, VIRTIO_MAX_QUEUE_SIZE}; +use crate::console::{CONSOLE, IoDevice}; use crate::drivers::error::DriverError; -#[cfg(not(feature = "pci"))] -use crate::drivers::mmio::get_console_driver; -#[cfg(feature = "pci")] -use crate::drivers::pci::get_console_driver; use crate::drivers::virtio::error::{VirtioConsoleError, VirtioError}; use crate::drivers::virtio::transport::{InterruptCapability, UniCapsColl}; use crate::drivers::virtio::virtqueue::split::SplitVq; @@ -57,47 +54,9 @@ fn fill_queue(vq: &mut VirtQueue, num_packets: u16, packet_size: u32) { } } -pub(crate) struct VirtioUART; - -impl VirtioUART { - pub const fn new() -> Self { - Self {} - } -} - -impl ErrorType for VirtioUART { - type Error = Errno; -} - -impl Read for VirtioUART { - fn read(&mut self, buf: &mut [u8]) -> Result { - let drv = get_console_driver().ok_or(Errno::Io)?; - - drv.lock().read(buf) - } -} - -impl ReadReady for VirtioUART { +impl ReadReady for VirtioConsoleDriver { fn read_ready(&mut self) -> Result { - let Some(drv) = get_console_driver() else { - return Ok(false); - }; - - Ok(drv.lock().has_packet()) - } -} - -impl Write for VirtioUART { - fn write(&mut self, buf: &[u8]) -> Result { - if let Some(drv) = get_console_driver() { - drv.lock().write_all(buf)?; - } - - Ok(buf.len()) - } - - fn flush(&mut self) -> Result<(), Self::Error> { - Ok(()) + Ok(self.has_packet()) } } @@ -339,9 +298,9 @@ impl super::virtio::VirtioDriver for VirtioConsoleDriver { InterruptCapability::IsrStatus(_) => { let irq = irq.unwrap(); handlers.entry(irq).or_default().push_back(|| { - if let Some(driver) = get_console_driver() { - driver.lock().handle_interrupt(); - }; + if let IoDevice::Virtio(ref mut driver) = CONSOLE.lock().device { + driver.handle_interrupt(); + } }); crate::arch::kernel::interrupts::add_irq_name(irq, "virtio"); info!("Virtio interrupt handler at line {irq}"); @@ -351,8 +310,8 @@ impl super::virtio::VirtioDriver for VirtioConsoleDriver { msix_table, handlers, || { - if let Some(driver) = get_console_driver() { - driver.lock().handle_device_configuration_interrupt(); + if let IoDevice::Virtio(ref mut driver) = CONSOLE.lock().device { + driver.handle_device_configuration_interrupt(); }; }, [(0..2u16, Self::handle_queue_interrupt as fn())].into_iter(), diff --git a/src/drivers/mmio.rs b/src/drivers/mmio.rs index fca912fb82..f0e185b0d2 100644 --- a/src/drivers/mmio.rs +++ b/src/drivers/mmio.rs @@ -1,5 +1,3 @@ -#[cfg(feature = "virtio-console")] -pub(crate) use crate::arch::kernel::mmio::get_console_driver; #[cfg(feature = "virtio-fs")] pub(crate) use crate::arch::kernel::mmio::get_filesystem_driver; #[cfg(feature = "virtio-vsock")] diff --git a/src/drivers/pci.rs b/src/drivers/pci.rs index a59aaacfe7..31874b0b12 100644 --- a/src/drivers/pci.rs +++ b/src/drivers/pci.rs @@ -18,13 +18,11 @@ use pci_types::{ }; use crate::arch::kernel::pci::PciConfigRegion; -#[cfg(feature = "virtio-console")] -use crate::console::IoDevice; #[allow(unused_imports)] use crate::drivers::Driver; use crate::drivers::InterruptHandlerMap; #[cfg(feature = "virtio-console")] -use crate::drivers::console::{VirtioConsoleDriver, VirtioUART}; +use crate::drivers::console::VirtioConsoleDriver; #[cfg(feature = "virtio-fs")] use crate::drivers::fs::VirtioFsDriver; #[cfg(feature = "rtl8139")] @@ -376,14 +374,6 @@ pub(crate) type NetworkDevice = VirtioNetDriver; #[cfg(feature = "rtl8139")] pub(crate) type NetworkDevice = RTL8139Driver; -#[cfg(feature = "virtio-console")] -pub(crate) fn get_console_driver() -> Option<&'static InterruptTicketMutex> { - PCI_DRIVERS - .get()? - .iter() - .find_map(|drv| drv.get_console_driver()) -} - #[cfg(feature = "virtio-vsock")] pub(crate) fn get_vsock_driver() -> Option<&'static InterruptTicketMutex> { PCI_DRIVERS @@ -423,11 +413,7 @@ pub(crate) fn init(handlers: &mut InterruptHandlerMap) { match pci_virtio::init_device(adapter, handlers) { #[cfg(feature = "virtio-console")] Ok(VirtioDriver::Console(drv)) => { - register_driver(PciDriver::VirtioConsole(InterruptTicketMutex::new(*drv))); - info!("Switch to virtio console"); - crate::console::CONSOLE - .lock() - .replace_device(IoDevice::Virtio(VirtioUART::new())); + crate::console::switch_to_virtio(*drv); } #[cfg(feature = "virtio-fs")] Ok(VirtioDriver::Fs(drv)) => {