Merge pull request #1245 from hermit-os/virtio-id

refactor(virtio): migrate `DevId` to `virtio-spec`

src/arch/riscv64/kernel/devicetree.rs

@@ -16,8 +16,6 @@ use crate::arch::riscv64::kernel::mmio::MmioDriver;
 use crate::arch::riscv64::mm::{paging, PhysAddr};
 #[cfg(feature = "gem-net")]
 use crate::drivers::net::gem;
-#[cfg(all(feature = "tcp", not(feature = "pci")))]
-use crate::drivers::virtio::transport::mmio::DevId;
 #[cfg(all(feature = "tcp", not(feature = "pci"), not(feature = "gem-net")))]
 use crate::drivers::virtio::transport::mmio::{self as mmio_virtio, VirtioDriver};
 #[cfg(all(feature = "tcp", not(feature = "pci")))]
@@ -209,9 +207,9 @@ pub fn init_drivers() {
 					trace!("Found a MMIO-device at {mmio:p}");
 
 					// Verify the device-ID to find the network card
-					let id = DevId::from(mmio.as_ptr().device_id().read().to_ne());
+					let id = mmio.as_ptr().device_id().read();
 
-					if id != DevId::VIRTIO_DEV_ID_NET {
+					if id != virtio_spec::Id::Net {
 						debug!("It's not a network card at {mmio:p}");
 					} else {
 						info!("Found network card at {mmio:p}");

src/arch/x86_64/kernel/mmio.rs

@@ -14,7 +14,7 @@ use crate::arch::x86_64::mm::paging::{
 use crate::arch::x86_64::mm::{paging, PhysAddr};
 use crate::drivers::net::virtio_net::VirtioNetDriver;
 use crate::drivers::virtio::transport::mmio as mmio_virtio;
-use crate::drivers::virtio::transport::mmio::{DevId, VirtioDriver};
+use crate::drivers::virtio::transport::mmio::VirtioDriver;
 use crate::env;
 
 pub const MAGIC_VALUE: u32 = 0x74726976;
@@ -60,9 +60,9 @@ unsafe fn check_ptr(ptr: *mut u8) -> Option<VolatileRef<'static, DeviceRegisters
 	trace!("Found a MMIO-device at {mmio:p}");
 
 	// Verify the device-ID to find the network card
-	let id = DevId::from(mmio.as_ptr().device_id().read().to_ne());
+	let id = mmio.as_ptr().device_id().read();
 
-	if id != DevId::VIRTIO_DEV_ID_NET {
+	if id != virtio_spec::Id::Net {
 		trace!("It's not a network card at {mmio:p}");
 		return None;
 	}

src/drivers/virtio/transport/mmio.rs

@@ -23,45 +23,6 @@ use crate::drivers::net::network_irqhandler;
 use crate::drivers::net::virtio_net::VirtioNetDriver;
 use crate::drivers::virtio::error::VirtioError;
 
-/// Virtio device ID's
-/// See Virtio specification v1.1. - 5
-///
-// WARN: Upon changes in the set of the enum variants
-// one MUST adjust the associated From<u32>
-// implementation, in order catch all cases correctly,
-// as this function uses the catch-all "_" case!
-#[allow(non_camel_case_types, clippy::upper_case_acronyms)]
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Copy)]
-#[repr(u32)]
-pub enum DevId {
-	INVALID = 0x0,
-	VIRTIO_DEV_ID_NET = 1,
-	VIRTIO_DEV_ID_BLK = 2,
-	VIRTIO_DEV_ID_CONSOLE = 3,
-}
-
-impl From<DevId> for u32 {
-	fn from(val: DevId) -> u32 {
-		match val {
-			DevId::VIRTIO_DEV_ID_NET => 1,
-			DevId::VIRTIO_DEV_ID_BLK => 2,
-			DevId::VIRTIO_DEV_ID_CONSOLE => 3,
-			DevId::INVALID => 0x0,
-		}
-	}
-}
-
-impl From<u32> for DevId {
-	fn from(val: u32) -> Self {
-		match val {
-			1 => DevId::VIRTIO_DEV_ID_NET,
-			2 => DevId::VIRTIO_DEV_ID_BLK,
-			3 => DevId::VIRTIO_DEV_ID_CONSOLE,
-			_ => DevId::INVALID,
-		}
-	}
-}
-
 pub struct VqCfgHandler<'a> {
 	vq_index: u16,
 	raw: VolatileRef<'a, DeviceRegisters>,
@@ -442,9 +403,9 @@ pub(crate) fn init_device(
 	}
 
 	// Verify the device-ID to find the network card
-	match registers.as_ptr().device_id().read().to_ne().into() {
+	match registers.as_ptr().device_id().read() {
 		#[cfg(any(feature = "tcp", feature = "udp"))]
-		DevId::VIRTIO_DEV_ID_NET => {
+		virtio_spec::Id::Net => {
 			match VirtioNetDriver::init(dev_id, registers, irq_no) {
 				Ok(virt_net_drv) => {
 					info!("Virtio network driver initialized.");

src/drivers/virtio/transport/pci.rs

@@ -32,63 +32,6 @@ use crate::drivers::pci::{DeviceHeader, Masks, PciDevice};
 use crate::drivers::virtio::env::memory::{MemLen, MemOff, VirtMemAddr};
 use crate::drivers::virtio::error::VirtioError;
 
-/// Virtio device ID's
-/// See Virtio specification v1.1. - 5
-///                      and v1.1. - 4.1.2.1
-///
-// WARN: Upon changes in the set of the enum variants
-// one MUST adjust the associated From<u16>
-// implementation, in order catch all cases correctly,
-// as this function uses the catch-all "_" case!
-#[allow(dead_code, non_camel_case_types, clippy::upper_case_acronyms)]
-#[repr(u16)]
-pub enum DevId {
-	INVALID = 0x0,
-	VIRTIO_TRANS_DEV_ID_NET = 0x1000,
-	VIRTIO_TRANS_DEV_ID_BLK = 0x1001,
-	VIRTIO_TRANS_DEV_ID_MEM_BALL = 0x1002,
-	VIRTIO_TRANS_DEV_ID_CONS = 0x1003,
-	VIRTIO_TRANS_DEV_ID_SCSI = 0x1004,
-	VIRTIO_TRANS_DEV_ID_ENTROPY = 0x1005,
-	VIRTIO_TRANS_DEV_ID_9P = 0x1009,
-	VIRTIO_DEV_ID_NET = 0x1041,
-	VIRTIO_DEV_ID_FS = 0x105A,
-}
-
-impl From<DevId> for u16 {
-	fn from(val: DevId) -> u16 {
-		match val {
-			DevId::VIRTIO_TRANS_DEV_ID_NET => 0x1000,
-			DevId::VIRTIO_TRANS_DEV_ID_BLK => 0x1001,
-			DevId::VIRTIO_TRANS_DEV_ID_MEM_BALL => 0x1002,
-			DevId::VIRTIO_TRANS_DEV_ID_CONS => 0x1003,
-			DevId::VIRTIO_TRANS_DEV_ID_SCSI => 0x1004,
-			DevId::VIRTIO_TRANS_DEV_ID_ENTROPY => 0x1005,
-			DevId::VIRTIO_TRANS_DEV_ID_9P => 0x1009,
-			DevId::VIRTIO_DEV_ID_NET => 0x1041,
-			DevId::VIRTIO_DEV_ID_FS => 0x105A,
-			DevId::INVALID => 0x0,
-		}
-	}
-}
-
-impl From<u16> for DevId {
-	fn from(val: u16) -> Self {
-		match val {
-			0x1000 => DevId::VIRTIO_TRANS_DEV_ID_NET,
-			0x1001 => DevId::VIRTIO_TRANS_DEV_ID_BLK,
-			0x1002 => DevId::VIRTIO_TRANS_DEV_ID_MEM_BALL,
-			0x1003 => DevId::VIRTIO_TRANS_DEV_ID_CONS,
-			0x1004 => DevId::VIRTIO_TRANS_DEV_ID_SCSI,
-			0x1005 => DevId::VIRTIO_TRANS_DEV_ID_ENTROPY,
-			0x1009 => DevId::VIRTIO_TRANS_DEV_ID_9P,
-			0x1041 => DevId::VIRTIO_DEV_ID_NET,
-			0x105A => DevId::VIRTIO_DEV_ID_FS,
-			_ => DevId::INVALID,
-		}
-	}
-}
-
 /// Virtio's cfg_type constants; indicating type of structure in capabilities list
 /// See Virtio specification v1.1 - 4.1.4
 //
@@ -1260,26 +1203,23 @@ pub(crate) fn init_device(
 ) -> Result<VirtioDriver, DriverError> {
 	let device_id = device.device_id();
 
-	let virt_drv = match DevId::from(device_id) {
-		DevId::VIRTIO_TRANS_DEV_ID_NET
-		| DevId::VIRTIO_TRANS_DEV_ID_BLK
-		| DevId::VIRTIO_TRANS_DEV_ID_MEM_BALL
-		| DevId::VIRTIO_TRANS_DEV_ID_CONS
-		| DevId::VIRTIO_TRANS_DEV_ID_SCSI
-		| DevId::VIRTIO_TRANS_DEV_ID_ENTROPY
-		| DevId::VIRTIO_TRANS_DEV_ID_9P => {
-			warn!(
-				"Legacy/transitional Virtio device, with id: {:#x} is NOT supported, skipping!",
-				device_id
-			);
+	if device_id < 0x1040 {
+		warn!(
+			"Legacy/transitional Virtio device, with id: {:#x} is NOT supported, skipping!",
+			device_id
+		);
 
-			// Return Driver error inidacting device is not supported
-			Err(DriverError::InitVirtioDevFail(
-				VirtioError::DevNotSupported(device_id),
-			))
-		}
+		// Return Driver error inidacting device is not supported
+		return Err(DriverError::InitVirtioDevFail(
+			VirtioError::DevNotSupported(device_id),
+		));
+	}
+
+	let id = virtio_spec::Id::from(u8::try_from(device_id - 0x1040).unwrap());
+
+	let virt_drv = match id {
 		#[cfg(all(not(feature = "rtl8139"), any(feature = "tcp", feature = "udp")))]
-		DevId::VIRTIO_DEV_ID_NET => match VirtioNetDriver::init(device) {
+		virtio_spec::Id::Net => match VirtioNetDriver::init(device) {
 			Ok(virt_net_drv) => {
 				info!("Virtio network driver initialized.");
 				Ok(VirtioDriver::Network(virt_net_drv))
@@ -1293,7 +1233,7 @@ pub(crate) fn init_device(
 			}
 		},
 		#[cfg(feature = "fuse")]
-		DevId::VIRTIO_DEV_ID_FS => {
+		virtio_spec::Id::Fs => {
 			// TODO: check subclass
 			// TODO: proper error handling on driver creation fail
 			match VirtioFsDriver::init(device) {
@@ -1310,11 +1250,8 @@ pub(crate) fn init_device(
 				}
 			}
 		}
-		_ => {
-			warn!(
-				"Virtio device with id: {:#x} is NOT supported, skipping!",
-				device_id
-			);
+		id => {
+			warn!("Virtio device {id:?} is not supported, skipping!");
 
 			// Return Driver error inidacting device is not supported
 			Err(DriverError::InitVirtioDevFail(

virtio-spec/Cargo.toml

@@ -11,6 +11,7 @@ categories = ["no-std", "no-std::no-alloc"]
 [dependencies]
 bitflags = "2"
 endian-num = { version = "0.1", features = ["bitflags", "linux-types"] }
+num_enum = { version = "0.7", default-features = false }
 volatile = { version = "0.5.3", features = ["derive"] }
 zerocopy = { version = "0.7", optional = true, default-features = false }
 zerocopy-derive = { version = "0.7", optional = true }