From a36332a264dc95fa096a37202bca62da36905776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 22 Apr 2024 14:19:42 +0200 Subject: [PATCH 1/9] refactor(virtio/pci): move `ComCfgRaw::map` to `PciCap::map_common_cfg` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/drivers/virtio/transport/pci.rs | 66 ++++++++++++++--------------- 1 file changed, 31 insertions(+), 35 deletions(-) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index bdd3268f4e..48c5a0466b 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -219,6 +219,36 @@ impl PciCap { pub fn dev_id(&self) -> u16 { self.origin.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 < u64::from(self.length + self.offset) { + error!("Common config of with id {} of device {:x}, does not fit into memory specified by bar {:x}!", + self.id, + self.origin.dev_id, + self.bar.index + ); + return None; + } + + // Using "as u32" is safe here as ComCfgRaw has a defined size smaller 2^31-1 + // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 + if self.length < MemLen::from(mem::size_of::() * 8) { + error!("Common config of with id {}, does not represent actual structure specified by the standard!", self.id); + return None; + } + + let virt_addr_raw = self.bar.mem_addr + self.offset; + let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( + virt_addr_raw.into(), + )) + .unwrap(); + + // Create mutable reference to the PCI structure in PCI memory + let com_cfg_raw = unsafe { VolatileRef::new(ptr) }; + + Some(com_cfg_raw) + } } /// Virtio's PCI capabilities structure. @@ -630,40 +660,6 @@ struct ComCfgRaw { queue_device: u64, // read-write } -// Common configuration raw does NOT provide a PUBLIC -// interface. -impl ComCfgRaw { - /// Returns a reference to the actual structure inside the PCI devices memory space. - fn map(cap: &PciCap) -> Option> { - if cap.bar.length < u64::from(cap.length + cap.offset) { - error!("Common config of with id {} of device {:x}, does not fit into memory specified by bar {:x}!", - cap.id, - cap.origin.dev_id, - cap.bar.index - ); - return None; - } - - // Using "as u32" is safe here as ComCfgRaw has a defined size smaller 2^31-1 - // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 - if cap.length < MemLen::from(mem::size_of::() * 8) { - error!("Common config of with id {}, does not represent actual structure specified by the standard!", cap.id); - return None; - } - - let virt_addr_raw = cap.bar.mem_addr + cap.offset; - let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( - virt_addr_raw.into(), - )) - .unwrap(); - - // Create mutable reference to the PCI structure in PCI memory - let com_cfg_raw = unsafe { VolatileRef::new(ptr) }; - - Some(com_cfg_raw) - } -} - /// Notification Structure to handle virtqueue notification settings. /// See Virtio specification v1.1 - 4.1.4.4 pub struct NotifCfg { @@ -1222,7 +1218,7 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result match ComCfgRaw::map(&pci_cap) { + CfgType::VIRTIO_PCI_CAP_COMMON_CFG => match pci_cap.map_common_cfg() { Some(cap) => caps.add_cfg_common(ComCfg::new(cap, pci_cap.id)), None => error!( "Common config capability with id {}, of device {:x}, could not be mapped!", From 00b617cdb4eea8e2f529552d92c465e527f201bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 24 Apr 2024 12:18:36 +0200 Subject: [PATCH 2/9] refactor(virtio/mmio): make `device_status` an u8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/drivers/virtio/transport/mmio.rs | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 45b4fea523..bb46d0910e 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -141,13 +141,13 @@ impl ComCfg { /// Returns the device status field. pub fn dev_status(&self) -> u8 { - unsafe { read_volatile(&self.com_cfg.status).try_into().unwrap() } + unsafe { read_volatile(&self.com_cfg.status) } } /// Resets the device status field to zero. pub fn reset_dev(&mut self) { unsafe { - write_volatile(&mut self.com_cfg.status, 0u32); + write_volatile(&mut self.com_cfg.status, 0u8); } } @@ -156,7 +156,7 @@ impl ComCfg { /// A driver MAY use the device again after a proper reset of the device. pub fn set_failed(&mut self) { unsafe { - write_volatile(&mut self.com_cfg.status, u32::from(device::Status::FAILED)); + write_volatile(&mut self.com_cfg.status, u8::from(device::Status::FAILED)); } } @@ -167,7 +167,7 @@ impl ComCfg { let status = read_volatile(&self.com_cfg.status); write_volatile( &mut self.com_cfg.status, - status | u32::from(device::Status::ACKNOWLEDGE), + status | u8::from(device::Status::ACKNOWLEDGE), ); } } @@ -179,7 +179,7 @@ impl ComCfg { let status = read_volatile(&self.com_cfg.status); write_volatile( &mut self.com_cfg.status, - status | u32::from(device::Status::DRIVER), + status | u8::from(device::Status::DRIVER), ); } } @@ -192,7 +192,7 @@ impl ComCfg { let status = read_volatile(&self.com_cfg.status); write_volatile( &mut self.com_cfg.status, - status | u32::from(device::Status::FEATURES_OK), + status | u8::from(device::Status::FEATURES_OK), ); } } @@ -206,8 +206,7 @@ impl ComCfg { pub fn check_features(&self) -> bool { unsafe { let status = read_volatile(&self.com_cfg.status); - status & u32::from(device::Status::FEATURES_OK) - == u32::from(device::Status::FEATURES_OK) + status & u8::from(device::Status::FEATURES_OK) == u8::from(device::Status::FEATURES_OK) } } @@ -219,7 +218,7 @@ impl ComCfg { let status = read_volatile(&self.com_cfg.status); write_volatile( &mut self.com_cfg.status, - status | u32::from(device::Status::DRIVER_OK), + status | u8::from(device::Status::DRIVER_OK), ); } } @@ -440,8 +439,8 @@ pub struct MmioRegisterLayout { interrupt_ack: u32, _reserved4: [u32; 2], - status: u32, - _reserved5: [u32; 3], + status: u8, + _reserved5: [u8; 15], queue_desc_low: u32, // non-legacy only queue_desc_high: u32, // non-legacy only From 393b9c53640a75926009df5aa54f317183335b3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Sun, 21 Apr 2024 19:23:24 +0200 Subject: [PATCH 3/9] feat: add `virtio-def` crate for virtio definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- Cargo.lock | 22 ++++++++ Cargo.toml | 1 + virtio-def/Cargo.toml | 13 +++++ virtio-def/rustfmt.toml | 2 + virtio-def/src/lib.rs | 62 +++++++++++++++++++++++ virtio-def/src/num.rs | 47 +++++++++++++++++ virtio-def/src/pci.rs | 109 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 256 insertions(+) create mode 100644 virtio-def/Cargo.toml create mode 100644 virtio-def/rustfmt.toml create mode 100644 virtio-def/src/lib.rs create mode 100644 virtio-def/src/num.rs create mode 100644 virtio-def/src/pci.rs diff --git a/Cargo.lock b/Cargo.lock index ebd7980622..de9f7cb558 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1435,6 +1435,14 @@ version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +[[package]] +name = "virtio-def" +version = "0.0.0" +dependencies = [ + "bitflags 2.5.0", + "volatile 0.5.4", +] + [[package]] name = "volatile" version = "0.4.6" @@ -1446,6 +1454,20 @@ name = "volatile" version = "0.5.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f05a5337d258c4ef40d920ffcc5a729278951d57e6596f8b62ca7d4827613e77" +dependencies = [ + "volatile-macro", +] + +[[package]] +name = "volatile-macro" +version = "0.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c0503c38e8d76a80ae2ab71fae739510bb959b683b3a82e742e257c5f1c81845" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.60", +] [[package]] name = "wait-timeout" diff --git a/Cargo.toml b/Cargo.toml index 1f94cc3ce1..ea0068781e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -154,6 +154,7 @@ llvm-tools = "0.1" [workspace] members = [ "hermit-macro", + "virtio-def", "xtask", ] exclude = [ diff --git a/virtio-def/Cargo.toml b/virtio-def/Cargo.toml new file mode 100644 index 0000000000..2413fa748d --- /dev/null +++ b/virtio-def/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "virtio-def" +authors = ["Martin Kröning "] +edition = "2021" +description = "Definitions from the Virtual I/O Device (VIRTIO) specification." +repository = "https://github.com/hermit-os/kernel" +license = "MIT OR Apache-2.0" +keywords = ["virtio", "driver", "volatile"] +categories = ["no-std", "no-std::no-alloc"] + +[dependencies] +bitflags = "2" +volatile = { version = "0.5.3", features = ["derive"] } diff --git a/virtio-def/rustfmt.toml b/virtio-def/rustfmt.toml new file mode 100644 index 0000000000..64d94def26 --- /dev/null +++ b/virtio-def/rustfmt.toml @@ -0,0 +1,2 @@ +group_imports = "StdExternalCrate" +imports_granularity = "Module" diff --git a/virtio-def/src/lib.rs b/virtio-def/src/lib.rs new file mode 100644 index 0000000000..03cd3a0ab9 --- /dev/null +++ b/virtio-def/src/lib.rs @@ -0,0 +1,62 @@ +//! This crate provides common definitions from the Virtio 1.2 specification. +//! This crate does not provide any additional driver-related functionality. +//! +//! For the actual specification see [Virtual I/O Device (VIRTIO) Version 1.2—Committee Specification 01](https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html). + +#![no_std] + +pub mod num; +pub mod pci; + +use bitflags::bitflags; + +bitflags! { + /// Device Status Field + /// + /// During device initialization by a driver, + /// the driver follows the sequence of steps specified in + /// _General Initialization And Device Operation / Device + /// Initialization_. + /// + /// The `device status` field provides a simple low-level + /// indication of the completed steps of this sequence. + /// It's most useful to imagine it hooked up to traffic + /// lights on the console indicating the status of each device. The + /// following bits are defined (listed below in the order in which + /// they would be typically set): + #[derive(Clone, Copy, Debug)] + pub struct DeviceStatus: u8 { + /// Indicates that the guest OS has found the + /// device and recognized it as a valid virtio device. + const ACKNOWLEDGE = 1; + + /// Indicates that the guest OS knows how to drive the + /// device. + /// + ///
+ /// + /// There could be a significant (or infinite) delay before setting + /// this bit. For example, under Linux, drivers can be loadable modules. + /// + ///
+ const DRIVER = 2; + + /// Indicates that something went wrong in the guest, + /// and it has given up on the device. This could be an internal + /// error, or the driver didn't like the device for some reason, or + /// even a fatal error during device operation. + const FAILED = 128; + + /// Indicates that the driver has acknowledged all the + /// features it understands, and feature negotiation is complete. + const FEATURES_OK = 8; + + /// Indicates that the driver is set up and ready to + /// drive the device. + const DRIVER_OK = 4; + + /// Indicates that the device has experienced + /// an error from which it can't recover. + const DEVICE_NEEDS_RESET = 64; + } +} diff --git a/virtio-def/src/num.rs b/virtio-def/src/num.rs new file mode 100644 index 0000000000..bf31c95cba --- /dev/null +++ b/virtio-def/src/num.rs @@ -0,0 +1,47 @@ +//! Byte order-aware numeric primitives. + +macro_rules! le_impl { + ($SelfT:ident, $ActualT:ty, $to:ident, $from:ident, $bits:expr, $order:expr) => { + #[doc = concat!("A ", stringify!($bits), "-bit unsigned integer stored in ", $order, " byte order.")] + #[allow(non_camel_case_types)] + #[must_use] + #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] + #[repr(transparent)] + pub struct $SelfT($ActualT); + + impl $SelfT { + #[doc = concat!("Creates a new ", $order, "integer from native-endian byte order.")] + #[inline] + pub const fn new(n: $ActualT) -> Self { + Self(n.$to()) + } + + /// Returns the integer in native-endian byte order. + #[inline] + pub const fn get(self) -> $ActualT { + <$ActualT>::$from(self.0) + } + } + + impl From<$ActualT> for $SelfT { + #[inline] + fn from(value: $ActualT) -> Self { + Self::new(value) + } + } + + impl From<$SelfT> for $ActualT { + #[inline] + fn from(value: $SelfT) -> Self { + value.get() + } + } + }; +} + +le_impl!(be16, u16, to_be, from_be, 16, "big-endian"); +le_impl!(be32, u32, to_be, from_be, 32, "big-endian"); +le_impl!(be64, u64, to_be, from_be, 64, "big-endian"); +le_impl!(le16, u16, to_le, from_le, 16, "little-endian"); +le_impl!(le32, u32, to_le, from_le, 32, "little-endian"); +le_impl!(le64, u64, to_le, from_le, 64, "little-endian"); diff --git a/virtio-def/src/pci.rs b/virtio-def/src/pci.rs new file mode 100644 index 0000000000..f348377778 --- /dev/null +++ b/virtio-def/src/pci.rs @@ -0,0 +1,109 @@ +//! Definitions for Virtio over PCI bus. + +use volatile::access::ReadOnly; +use volatile::VolatileFieldAccess; + +use crate::num::*; +use crate::DeviceStatus; + +/// Common configuration structure +/// +/// The common configuration structure is found at the bar and offset within the [`VIRTIO_PCI_CAP_COMMON_CFG`] capability. +#[doc(alias = "virtio_pci_common_cfg")] +#[derive(VolatileFieldAccess)] +#[allow(non_camel_case_types)] +#[repr(C)] +pub struct CommonCfg { + /// The driver uses this to select which feature bits `device_feature` shows. + /// Value 0x0 selects Feature Bits 0 to 31, 0x1 selects Feature Bits 32 to 63, etc. + device_feature_select: le32, + + /// The device uses this to report which feature bits it is + /// offering to the driver: the driver writes to + /// `device_feature_select` to select which feature bits are presented. + #[access(ReadOnly)] + device_feature: le32, + + /// The driver uses this to select which feature bits `driver_feature` shows. + /// Value 0x0 selects Feature Bits 0 to 31, 0x1 selects Feature Bits 32 to 63, etc. + driver_feature_select: le32, + + /// The driver writes this to accept feature bits offered by the device. + /// Driver Feature Bits selected by `driver_feature_select`. + driver_feature: le32, + + /// The driver sets the Configuration Vector for MSI-X. + config_msix_vector: le16, + + /// The device specifies the maximum number of virtqueues supported here. + #[access(ReadOnly)] + num_queues: le16, + + /// The driver writes the device status here (see [`DeviceStatus`]). Writing 0 into this + /// field resets the device. + device_status: DeviceStatus, + + /// Configuration atomicity value. The device changes this every time the + /// configuration noticeably changes. + #[access(ReadOnly)] + config_generation: u8, + + /// Queue Select. The driver selects which virtqueue the following + /// fields refer to. + queue_select: le16, + + /// Queue Size. On reset, specifies the maximum queue size supported by + /// the device. This can be modified by the driver to reduce memory requirements. + /// A 0 means the queue is unavailable. + queue_size: le16, + + /// The driver uses this to specify the queue vector for MSI-X. + queue_msix_vector: le16, + + /// The driver uses this to selectively prevent the device from executing requests from this virtqueue. + /// 1 - enabled; 0 - disabled. + queue_enable: le16, + + /// The driver reads this to calculate the offset from start of Notification structure at + /// which this virtqueue is located. + /// + ///
+ /// + /// This is _not_ an offset in bytes. + /// See _Virtio Transport Options / Virtio Over PCI Bus / PCI Device Layout / Notification capability_ below. + /// + ///
+ #[access(ReadOnly)] + queue_notify_off: le16, + + /// The driver writes the physical address of Descriptor Area here. See section _Basic Facilities of a Virtio Device / Virtqueues_. + queue_desc: le64, + + /// The driver writes the physical address of Driver Area here. See section _Basic Facilities of a Virtio Device / Virtqueues_. + queue_driver: le64, + + /// The driver writes the physical address of Device Area here. See section _Basic Facilities of a Virtio Device / Virtqueues_. + queue_device: le64, + + /// This field exists only if [`VIRTIO_F_NOTIF_CONFIG_DATA`] has been negotiated. + /// The driver will use this value to put it in the 'virtqueue number' field + /// in the available buffer notification structure. + /// See section _Virtio Transport Options / Virtio Over PCI Bus / PCI-specific Initialization And Device Operation / Available Buffer Notifications_. + /// + ///
+ /// + /// This field provides the device with flexibility to determine how virtqueues + /// will be referred to in available buffer notifications. + /// In a trivial case the device can set `queue_notify_data`=vqn. Some devices + /// may benefit from providing another value, for example an internal virtqueue + /// identifier, or an internal offset related to the virtqueue number. + /// + ///
+ #[access(ReadOnly)] + queue_notify_data: le16, + + /// The driver uses this to selectively reset the queue. + /// This field exists only if [`VIRTIO_F_RING_RESET`] has been + /// negotiated. (see _Basic Facilities of a Virtio Device / Virtqueues / Virtqueue Reset_). + queue_reset: le16, +} From 544ff3dcb2764f529f5013b56418af4ad251502b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 24 Apr 2024 12:29:07 +0200 Subject: [PATCH 4/9] refactor(virtio): migrate `DeviceStatus` to `virtio-def` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- Cargo.lock | 1 + Cargo.toml | 1 + src/drivers/virtio/mod.rs | 42 ---------------------------- src/drivers/virtio/transport/mmio.rs | 33 ++++++++-------------- src/drivers/virtio/transport/pci.rs | 37 ++++++++++++------------ 5 files changed, 32 insertions(+), 82 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index de9f7cb558..30a9d82cfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -580,6 +580,7 @@ dependencies = [ "time", "trapframe", "uart_16550", + "virtio-def", "volatile 0.5.4", "x86", "x86_64 0.15.1", diff --git a/Cargo.toml b/Cargo.toml index ea0068781e..ca75b96743 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -71,6 +71,7 @@ shell = ["simple-shell"] [dependencies] hermit-macro = { path = "hermit-macro" } +virtio-def = { path = "virtio-def" } ahash = { version = "0.8", default-features = false } align-address = "0.1" bit_field = "0.10" diff --git a/src/drivers/virtio/mod.rs b/src/drivers/virtio/mod.rs index cfb3f9a256..628004f40e 100644 --- a/src/drivers/virtio/mod.rs +++ b/src/drivers/virtio/mod.rs @@ -165,45 +165,3 @@ pub mod features { } } } - -/// A module containing virtios device specfific information. -pub mod device { - /// An enum of the device's status field interpretations. - #[allow(dead_code, non_camel_case_types, clippy::upper_case_acronyms)] - #[derive(Clone, Copy, Debug)] - #[repr(u8)] - pub enum Status { - ACKNOWLEDGE = 1, - DRIVER = 2, - DRIVER_OK = 4, - FEATURES_OK = 8, - DEVICE_NEEDS_RESET = 64, - FAILED = 128, - } - - impl From for u8 { - fn from(stat: Status) -> Self { - match stat { - Status::ACKNOWLEDGE => 1, - Status::DRIVER => 2, - Status::DRIVER_OK => 4, - Status::FEATURES_OK => 8, - Status::DEVICE_NEEDS_RESET => 64, - Status::FAILED => 128, - } - } - } - - impl From for u32 { - fn from(stat: Status) -> Self { - match stat { - Status::ACKNOWLEDGE => 1, - Status::DRIVER => 2, - Status::DRIVER_OK => 4, - Status::FEATURES_OK => 8, - Status::DEVICE_NEEDS_RESET => 64, - Status::FAILED => 128, - } - } - } -} diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index bb46d0910e..200d630259 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -7,6 +7,8 @@ use core::ptr; use core::ptr::{read_volatile, write_volatile}; use core::sync::atomic::{fence, Ordering}; +use virtio_def::DeviceStatus; + #[cfg(any(feature = "tcp", feature = "udp"))] use crate::arch::kernel::interrupts::*; use crate::arch::mm::PhysAddr; @@ -15,7 +17,6 @@ use crate::drivers::error::DriverError; use crate::drivers::net::network_irqhandler; #[cfg(any(feature = "tcp", feature = "udp"))] use crate::drivers::net::virtio_net::VirtioNetDriver; -use crate::drivers::virtio::device; use crate::drivers::virtio::error::VirtioError; /// Virtio device ID's @@ -141,13 +142,13 @@ impl ComCfg { /// Returns the device status field. pub fn dev_status(&self) -> u8 { - unsafe { read_volatile(&self.com_cfg.status) } + unsafe { read_volatile(&self.com_cfg.status).bits() } } /// Resets the device status field to zero. pub fn reset_dev(&mut self) { unsafe { - write_volatile(&mut self.com_cfg.status, 0u8); + write_volatile(&mut self.com_cfg.status, DeviceStatus::empty()); } } @@ -156,7 +157,7 @@ impl ComCfg { /// A driver MAY use the device again after a proper reset of the device. pub fn set_failed(&mut self) { unsafe { - write_volatile(&mut self.com_cfg.status, u8::from(device::Status::FAILED)); + write_volatile(&mut self.com_cfg.status, DeviceStatus::FAILED); } } @@ -165,10 +166,7 @@ impl ComCfg { pub fn ack_dev(&mut self) { unsafe { let status = read_volatile(&self.com_cfg.status); - write_volatile( - &mut self.com_cfg.status, - status | u8::from(device::Status::ACKNOWLEDGE), - ); + write_volatile(&mut self.com_cfg.status, status | DeviceStatus::ACKNOWLEDGE); } } @@ -177,10 +175,7 @@ impl ComCfg { pub fn set_drv(&mut self) { unsafe { let status = read_volatile(&self.com_cfg.status); - write_volatile( - &mut self.com_cfg.status, - status | u8::from(device::Status::DRIVER), - ); + write_volatile(&mut self.com_cfg.status, status | DeviceStatus::DRIVER); } } @@ -190,10 +185,7 @@ impl ComCfg { pub fn features_ok(&mut self) { unsafe { let status = read_volatile(&self.com_cfg.status); - write_volatile( - &mut self.com_cfg.status, - status | u8::from(device::Status::FEATURES_OK), - ); + write_volatile(&mut self.com_cfg.status, status | DeviceStatus::FEATURES_OK); } } @@ -206,7 +198,7 @@ impl ComCfg { pub fn check_features(&self) -> bool { unsafe { let status = read_volatile(&self.com_cfg.status); - status & u8::from(device::Status::FEATURES_OK) == u8::from(device::Status::FEATURES_OK) + status.contains(DeviceStatus::FEATURES_OK) } } @@ -216,10 +208,7 @@ impl ComCfg { pub fn drv_ok(&mut self) { unsafe { let status = read_volatile(&self.com_cfg.status); - write_volatile( - &mut self.com_cfg.status, - status | u8::from(device::Status::DRIVER_OK), - ); + write_volatile(&mut self.com_cfg.status, status | DeviceStatus::DRIVER_OK); } } @@ -439,7 +428,7 @@ pub struct MmioRegisterLayout { interrupt_ack: u32, _reserved4: [u32; 2], - status: u8, + status: DeviceStatus, _reserved5: [u8; 15], queue_desc_low: u32, // non-legacy only diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 48c5a0466b..237d7c5be1 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -8,6 +8,7 @@ use core::ptr::NonNull; use core::sync::atomic::{fence, Ordering}; use core::{mem, ptr}; +use virtio_def::DeviceStatus; use volatile::{map_field, VolatileRef}; #[cfg(all(not(feature = "rtl8139"), any(feature = "tcp", feature = "udp")))] @@ -24,7 +25,6 @@ use crate::drivers::net::network_irqhandler; use crate::drivers::net::virtio_net::VirtioNetDriver; use crate::drivers::pci::error::PciError; use crate::drivers::pci::{DeviceHeader, Masks, PciDevice}; -use crate::drivers::virtio::device; use crate::drivers::virtio::env::memory::{MemLen, MemOff, VirtMemAddr}; use crate::drivers::virtio::error::VirtioError; @@ -513,14 +513,14 @@ impl ComCfg { /// Returns the device status field. pub fn dev_status(&self) -> u8 { let com_cfg = self.com_cfg.as_ptr(); - map_field!(com_cfg.device_status).read() + map_field!(com_cfg.device_status).read().bits() } /// Resets the device status field to zero. pub fn reset_dev(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).write(0); + map_field!(com_cfg.device_status).write(DeviceStatus::empty()); } /// Sets the device status field to FAILED. @@ -529,7 +529,7 @@ impl ComCfg { pub fn set_failed(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).write(u8::from(device::Status::FAILED)); + map_field!(com_cfg.device_status).write(DeviceStatus::FAILED); } /// Sets the ACKNOWLEDGE bit in the device status field. This indicates, the @@ -537,7 +537,7 @@ impl ComCfg { pub fn ack_dev(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | u8::from(device::Status::ACKNOWLEDGE)); + map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::ACKNOWLEDGE); } /// Sets the DRIVER bit in the device status field. This indicates, the OS @@ -545,7 +545,7 @@ impl ComCfg { pub fn set_drv(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | u8::from(device::Status::DRIVER)); + map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::DRIVER); } /// Sets the FEATURES_OK bit in the device status field. @@ -554,7 +554,7 @@ impl ComCfg { pub fn features_ok(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | u8::from(device::Status::FEATURES_OK)); + map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::FEATURES_OK); } /// In order to correctly check feature negotiaten, this function @@ -566,8 +566,9 @@ impl ComCfg { pub fn check_features(&self) -> bool { memory_barrier(); let com_cfg = self.com_cfg.as_ptr(); - map_field!(com_cfg.device_status).read() & u8::from(device::Status::FEATURES_OK) - == u8::from(device::Status::FEATURES_OK) + map_field!(com_cfg.device_status) + .read() + .contains(DeviceStatus::FEATURES_OK) } /// Sets the DRIVER_OK bit in the device status field. @@ -576,7 +577,7 @@ impl ComCfg { pub fn drv_ok(&mut self) { memory_barrier(); let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | u8::from(device::Status::DRIVER_OK)); + map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::DRIVER_OK); } /// Returns the features offered by the device. Coded in a 64bit value. @@ -640,14 +641,14 @@ impl ComCfg { #[repr(C)] struct ComCfgRaw { // About whole device - device_feature_select: u32, // read-write - device_feature: u32, // read-only for driver - driver_feature_select: u32, // read-write - driver_feature: u32, // read-write - config_msix_vector: u16, // read-write - num_queues: u16, // read-only for driver - device_status: u8, // read-write - config_generation: u8, // read-only for driver + device_feature_select: u32, // read-write + device_feature: u32, // read-only for driver + driver_feature_select: u32, // read-write + driver_feature: u32, // read-write + config_msix_vector: u16, // read-write + num_queues: u16, // read-only for driver + device_status: DeviceStatus, // read-write + config_generation: u8, // read-only for driver // About a specific virtqueue queue_select: u16, // read-write From 923844d5666c0deb0d17dc59073adf119660c573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 29 Apr 2024 10:45:31 +0200 Subject: [PATCH 5/9] build(deps): use unreleased `volatile` crate for read-access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- Cargo.lock | 6 ++---- Cargo.toml | 4 ++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 30a9d82cfa..85c0b1c78a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1453,8 +1453,7 @@ checksum = "442887c63f2c839b346c192d047a7c87e73d0689c9157b00b53dcc27dd5ea793" [[package]] name = "volatile" version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f05a5337d258c4ef40d920ffcc5a729278951d57e6596f8b62ca7d4827613e77" +source = "git+https://github.com/rust-osdev/volatile.git#ac3a01640f0d93283a9e8def75038bbea85164b6" dependencies = [ "volatile-macro", ] @@ -1462,8 +1461,7 @@ dependencies = [ [[package]] name = "volatile-macro" version = "0.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c0503c38e8d76a80ae2ab71fae739510bb959b683b3a82e742e257c5f1c81845" +source = "git+https://github.com/rust-osdev/volatile.git#ac3a01640f0d93283a9e8def75038bbea85164b6" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index ca75b96743..e86764940e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -161,3 +161,7 @@ members = [ exclude = [ "hermit-builtins", ] + +[patch.crates-io] +# We need https://github.com/rust-osdev/volatile/pull/61 +volatile = { git = "https://github.com/rust-osdev/volatile.git" } From 84daffc6f1c6f62b8fb78a1c671ea3bb39d82eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 24 Apr 2024 16:50:30 +0200 Subject: [PATCH 6/9] refactor(virtio/pci): migrate `CommonCfg` to `virtio-def` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/drivers/virtio/transport/pci.rs | 132 ++++++++++++++++------------ 1 file changed, 74 insertions(+), 58 deletions(-) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 237d7c5be1..24717872b2 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -8,8 +8,9 @@ use core::ptr::NonNull; use core::sync::atomic::{fence, Ordering}; use core::{mem, ptr}; +use virtio_def::pci::{CommonCfg, CommonCfgVolatileFieldAccess}; use virtio_def::DeviceStatus; -use volatile::{map_field, VolatileRef}; +use volatile::VolatileRef; #[cfg(all(not(feature = "rtl8139"), any(feature = "tcp", feature = "udp")))] use crate::arch::kernel::interrupts::*; @@ -221,7 +222,7 @@ impl PciCap { } /// Returns a reference to the actual structure inside the PCI devices memory space. - fn map_common_cfg(&self) -> Option> { + fn map_common_cfg(&self) -> Option> { if self.bar.length < u64::from(self.length + self.offset) { error!("Common config of with id {} of device {:x}, does not fit into memory specified by bar {:x}!", self.id, @@ -231,15 +232,15 @@ impl PciCap { return None; } - // Using "as u32" is safe here as ComCfgRaw has a defined size smaller 2^31-1 + // Using "as u32" is safe here as CommonCfg has a defined size smaller 2^31-1 // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 - if self.length < MemLen::from(mem::size_of::() * 8) { + if self.length < MemLen::from(mem::size_of::() * 8) { error!("Common config of with id {}, does not represent actual structure specified by the standard!", self.id); return None; } let virt_addr_raw = self.bar.mem_addr + self.offset; - let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( + let ptr = NonNull::new(ptr::with_exposed_provenance_mut::( virt_addr_raw.into(), )) .unwrap(); @@ -410,7 +411,7 @@ impl UniCapsColl { } } -/// Wraps a [ComCfgRaw] in order to preserve +/// Wraps a [`CommonCfg`] in order to preserve /// the original structure. /// /// Provides a safe API for Raw structure and allows interaction with the device via @@ -418,28 +419,30 @@ impl UniCapsColl { pub struct ComCfg { /// References the raw structure in PCI memory space. Is static as /// long as the device is present, which is mandatory in order to let this code work. - com_cfg: VolatileRef<'static, ComCfgRaw>, + com_cfg: VolatileRef<'static, CommonCfg>, /// Preferences of the device for this config. From 1 (highest) to 2^7-1 (lowest) rank: u8, } // Private interface of ComCfg impl ComCfg { - fn new(raw: VolatileRef<'static, ComCfgRaw>, rank: u8) -> Self { + fn new(raw: VolatileRef<'static, CommonCfg>, rank: u8) -> Self { ComCfg { com_cfg: raw, rank } } } pub struct VqCfgHandler<'a> { vq_index: u16, - raw: VolatileRef<'a, ComCfgRaw>, + raw: VolatileRef<'a, CommonCfg>, } impl<'a> VqCfgHandler<'a> { // TODO: Create type for queue selected invariant to get rid of `self.select_queue()` everywhere. fn select_queue(&mut self) { - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_select).write(self.vq_index); + self.raw + .as_mut_ptr() + .queue_select() + .write(self.vq_index.into()); } /// Sets the size of a given virtqueue. In case the provided size exceeds the maximum allowed @@ -448,44 +451,47 @@ impl<'a> VqCfgHandler<'a> { /// Returns the set size in form of a `u16`. pub fn set_vq_size(&mut self, size: u16) -> u16 { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - let queue_size = map_field!(raw.queue_size); + let queue_size = self.raw.as_mut_ptr().queue_size(); - if queue_size.read() >= size { - queue_size.write(size); + if queue_size.read().get() >= size { + queue_size.write(size.into()); } - queue_size.read() + queue_size.read().get() } pub fn set_ring_addr(&mut self, addr: PhysAddr) { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_desc).write(addr.as_u64()); + self.raw + .as_mut_ptr() + .queue_desc() + .write(addr.as_u64().into()); } pub fn set_drv_ctrl_addr(&mut self, addr: PhysAddr) { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_driver).write(addr.as_u64()); + self.raw + .as_mut_ptr() + .queue_driver() + .write(addr.as_u64().into()); } pub fn set_dev_ctrl_addr(&mut self, addr: PhysAddr) { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_device).write(addr.as_u64()); + self.raw + .as_mut_ptr() + .queue_device() + .write(addr.as_u64().into()); } pub fn notif_off(&mut self) -> u16 { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_notify_off).read() + self.raw.as_mut_ptr().queue_notify_off().read().get() } pub fn enable_queue(&mut self) { self.select_queue(); - let raw = self.raw.as_mut_ptr(); - map_field!(raw.queue_enable).write(1); + self.raw.as_mut_ptr().queue_enable().write(1.into()); } } @@ -496,11 +502,9 @@ impl ComCfg { /// /// INFO: The queue size is automatically bounded by constant `src::config:VIRTIO_MAX_QUEUE_SIZE`. pub fn select_vq(&mut self, index: u16) -> Option> { - let com_cfg = self.com_cfg.as_mut_ptr(); - - map_field!(com_cfg.queue_select).write(index); + self.com_cfg.as_mut_ptr().queue_select().write(index.into()); - if map_field!(com_cfg.queue_size).read() == 0 { + if self.com_cfg.as_mut_ptr().queue_size().read().get() == 0 { None } else { Some(VqCfgHandler { @@ -512,15 +516,16 @@ impl ComCfg { /// Returns the device status field. pub fn dev_status(&self) -> u8 { - let com_cfg = self.com_cfg.as_ptr(); - map_field!(com_cfg.device_status).read().bits() + self.com_cfg.as_ptr().device_status().read().bits() } /// Resets the device status field to zero. pub fn reset_dev(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).write(DeviceStatus::empty()); + self.com_cfg + .as_mut_ptr() + .device_status() + .write(DeviceStatus::empty()); } /// Sets the device status field to FAILED. @@ -528,24 +533,30 @@ impl ComCfg { /// A driver MAY use the device again after a proper reset of the device. pub fn set_failed(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).write(DeviceStatus::FAILED); + self.com_cfg + .as_mut_ptr() + .device_status() + .write(DeviceStatus::FAILED); } /// Sets the ACKNOWLEDGE bit in the device status field. This indicates, the /// OS has notived the device pub fn ack_dev(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::ACKNOWLEDGE); + self.com_cfg + .as_mut_ptr() + .device_status() + .update(|s| s | DeviceStatus::ACKNOWLEDGE); } /// Sets the DRIVER bit in the device status field. This indicates, the OS /// know how to run this device. pub fn set_drv(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::DRIVER); + self.com_cfg + .as_mut_ptr() + .device_status() + .update(|s| s | DeviceStatus::DRIVER); } /// Sets the FEATURES_OK bit in the device status field. @@ -553,8 +564,10 @@ impl ComCfg { /// Drivers MUST NOT accept new features after this step. pub fn features_ok(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::FEATURES_OK); + self.com_cfg + .as_mut_ptr() + .device_status() + .update(|s| s | DeviceStatus::FEATURES_OK); } /// In order to correctly check feature negotiaten, this function @@ -565,8 +578,9 @@ impl ComCfg { /// otherwise, the device does not support our subset of features and the device is unusable. pub fn check_features(&self) -> bool { memory_barrier(); - let com_cfg = self.com_cfg.as_ptr(); - map_field!(com_cfg.device_status) + self.com_cfg + .as_ptr() + .device_status() .read() .contains(DeviceStatus::FEATURES_OK) } @@ -576,32 +590,34 @@ impl ComCfg { /// After this call, the device is "live"! pub fn drv_ok(&mut self) { memory_barrier(); - let com_cfg = self.com_cfg.as_mut_ptr(); - map_field!(com_cfg.device_status).update(|s| s | DeviceStatus::DRIVER_OK); + self.com_cfg + .as_mut_ptr() + .device_status() + .update(|s| s | DeviceStatus::DRIVER_OK); } /// Returns the features offered by the device. Coded in a 64bit value. pub fn dev_features(&mut self) -> u64 { let com_cfg = self.com_cfg.as_mut_ptr(); - let device_feature_select = map_field!(com_cfg.device_feature_select); - let device_feature = map_field!(com_cfg.device_feature); + let device_feature_select = com_cfg.device_feature_select(); + let device_feature = com_cfg.device_feature(); // Indicate device to show high 32 bits in device_feature field. // See Virtio specification v1.1. - 4.1.4.3 memory_barrier(); - device_feature_select.write(1); + device_feature_select.write(1.into()); memory_barrier(); // read high 32 bits of device features - let mut dev_feat = u64::from(device_feature.read()) << 32; + let mut dev_feat = u64::from(device_feature.read().get()) << 32; // Indicate device to show low 32 bits in device_feature field. // See Virtio specification v1.1. - 4.1.4.3 - device_feature_select.write(0); + device_feature_select.write(0.into()); memory_barrier(); // read low 32 bits of device features - dev_feat |= u64::from(device_feature.read()); + dev_feat |= u64::from(device_feature.read().get()); dev_feat } @@ -609,8 +625,8 @@ impl ComCfg { /// Write selected features into driver_select field. pub fn set_drv_features(&mut self, feats: u64) { let com_cfg = self.com_cfg.as_mut_ptr(); - let driver_feature_select = map_field!(com_cfg.driver_feature_select); - let driver_feature = map_field!(com_cfg.driver_feature); + let driver_feature_select = com_cfg.driver_feature_select(); + let driver_feature = com_cfg.driver_feature(); let high: u32 = (feats >> 32) as u32; let low: u32 = feats as u32; @@ -618,19 +634,19 @@ impl ComCfg { // Indicate to device that driver_features field shows low 32 bits. // See Virtio specification v1.1. - 4.1.4.3 memory_barrier(); - driver_feature_select.write(0); + driver_feature_select.write(0.into()); memory_barrier(); // write low 32 bits of device features - driver_feature.write(low); + driver_feature.write(low.into()); // Indicate to device that driver_features field shows high 32 bits. // See Virtio specification v1.1. - 4.1.4.3 - driver_feature_select.write(1); + driver_feature_select.write(1.into()); memory_barrier(); // write high 32 bits of device features - driver_feature.write(high); + driver_feature.write(high.into()); } } From d1dad7f3f2ab7aaec279e9c4dcceebc803f070f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 29 Apr 2024 17:56:00 +0200 Subject: [PATCH 7/9] feat(virtio-def): add `zerocopy` support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- Cargo.lock | 2 ++ virtio-def/Cargo.toml | 5 +++++ virtio-def/src/lib.rs | 41 ++++++++++++++++++++++++++--------------- virtio-def/src/num.rs | 1 + virtio-def/src/pci.rs | 4 ++++ 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85c0b1c78a..c172335335 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1442,6 +1442,8 @@ version = "0.0.0" dependencies = [ "bitflags 2.5.0", "volatile 0.5.4", + "zerocopy", + "zerocopy-derive", ] [[package]] diff --git a/virtio-def/Cargo.toml b/virtio-def/Cargo.toml index 2413fa748d..399ce5426a 100644 --- a/virtio-def/Cargo.toml +++ b/virtio-def/Cargo.toml @@ -11,3 +11,8 @@ categories = ["no-std", "no-std::no-alloc"] [dependencies] bitflags = "2" volatile = { version = "0.5.3", features = ["derive"] } +zerocopy = { version = "0.7", optional = true, default-features = false } +zerocopy-derive = { version = "0.7", optional = true } + +[features] +zerocopy = ["dep:zerocopy", "dep:zerocopy-derive"] diff --git a/virtio-def/src/lib.rs b/virtio-def/src/lib.rs index 03cd3a0ab9..944690c789 100644 --- a/virtio-def/src/lib.rs +++ b/virtio-def/src/lib.rs @@ -10,22 +10,33 @@ pub mod pci; use bitflags::bitflags; +/// Device Status Field +/// +/// During device initialization by a driver, +/// the driver follows the sequence of steps specified in +/// _General Initialization And Device Operation / Device +/// Initialization_. +/// +/// The `device status` field provides a simple low-level +/// indication of the completed steps of this sequence. +/// It's most useful to imagine it hooked up to traffic +/// lights on the console indicating the status of each device. The +/// following bits are defined (listed below in the order in which +/// they would be typically set): +#[cfg_attr( + feature = "zerocopy", + derive( + zerocopy_derive::FromZeroes, + zerocopy_derive::FromBytes, + zerocopy_derive::AsBytes + ) +)] +#[derive(Clone, Copy, Debug)] +#[repr(transparent)] +pub struct DeviceStatus(u8); + bitflags! { - /// Device Status Field - /// - /// During device initialization by a driver, - /// the driver follows the sequence of steps specified in - /// _General Initialization And Device Operation / Device - /// Initialization_. - /// - /// The `device status` field provides a simple low-level - /// indication of the completed steps of this sequence. - /// It's most useful to imagine it hooked up to traffic - /// lights on the console indicating the status of each device. The - /// following bits are defined (listed below in the order in which - /// they would be typically set): - #[derive(Clone, Copy, Debug)] - pub struct DeviceStatus: u8 { + impl DeviceStatus: u8 { /// Indicates that the guest OS has found the /// device and recognized it as a valid virtio device. const ACKNOWLEDGE = 1; diff --git a/virtio-def/src/num.rs b/virtio-def/src/num.rs index bf31c95cba..f2c0dd0475 100644 --- a/virtio-def/src/num.rs +++ b/virtio-def/src/num.rs @@ -5,6 +5,7 @@ macro_rules! le_impl { #[doc = concat!("A ", stringify!($bits), "-bit unsigned integer stored in ", $order, " byte order.")] #[allow(non_camel_case_types)] #[must_use] + #[cfg_attr(feature = "zerocopy", derive(zerocopy_derive::FromZeroes, zerocopy_derive::FromBytes, zerocopy_derive::AsBytes))] #[derive(Hash, PartialEq, Eq, Clone, Copy, Debug)] #[repr(transparent)] pub struct $SelfT($ActualT); diff --git a/virtio-def/src/pci.rs b/virtio-def/src/pci.rs index f348377778..e426892513 100644 --- a/virtio-def/src/pci.rs +++ b/virtio-def/src/pci.rs @@ -10,6 +10,10 @@ use crate::DeviceStatus; /// /// The common configuration structure is found at the bar and offset within the [`VIRTIO_PCI_CAP_COMMON_CFG`] capability. #[doc(alias = "virtio_pci_common_cfg")] +#[cfg_attr( + feature = "zerocopy", + derive(zerocopy_derive::FromZeroes, zerocopy_derive::FromBytes) +)] #[derive(VolatileFieldAccess)] #[allow(non_camel_case_types)] #[repr(C)] From 0b64a1bbb8c8734842776163c6246b14c243c9c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 29 Apr 2024 17:57:39 +0200 Subject: [PATCH 8/9] fix(virtio/pci): error message typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Kröning --- src/drivers/virtio/transport/pci.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 24717872b2..6d3461ad4a 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -224,7 +224,7 @@ impl PciCap { /// Returns a reference to the actual structure inside the PCI devices memory space. fn map_common_cfg(&self) -> Option> { if self.bar.length < u64::from(self.length + self.offset) { - error!("Common config of with id {} of device {:x}, does not fit into memory specified by bar {:x}!", + error!("Common config of the capability with id {} of device {:x} does not fit into memory specified by bar {:x}!", self.id, self.origin.dev_id, self.bar.index From 27f04670708dc903161837d1902abe6d1e70d45f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 29 Apr 2024 18:02:13 +0200 Subject: [PATCH 9/9] fix(virtio/pci): remove obsolete comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This comment was made obsolete by https://github.com/hermit-os/kernel/commit/68effc939613c4818f94b377070f1b463565e0fd#diff-aec828209d10fe04d7a8a6f063279edaa909f90ecabc93f062445611e4f63972L380 Signed-off-by: Martin Kröning --- src/drivers/virtio/transport/pci.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 6d3461ad4a..4658bde4a1 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -232,7 +232,6 @@ impl PciCap { return None; } - // Using "as u32" is safe here as CommonCfg has a defined size smaller 2^31-1 // Drivers MAY do this check. See Virtio specification v1.1. - 4.1.4.1 if self.length < MemLen::from(mem::size_of::() * 8) { error!("Common config of with id {}, does not represent actual structure specified by the standard!", self.id);