virtq: use enum_dispatch

Closes hermit-os#989

Co-authored-by: Jens Reidel <[email protected]>

Author: cagatay-y

Cargo.lock

@@ -110,6 +110,7 @@ bitflags = "2"
 build-time = "0.1.3"
 cfg-if = "1"
 crossbeam-utils = { version = "0.8", default-features = false }
+enum_dispatch = "0.3"
 fdt = { version = "0.1", features = ["pretty-printing"] }
 free-list = "0.3"
 fuse-abi = { version = "0.2", features = ["linux", "zerocopy"], optional = true }

Cargo.toml

@@ -19,7 +19,7 @@ use crate::drivers::virtio::transport::pci::{ComCfg, IsrStatus, NotifCfg};
 use crate::drivers::virtio::virtqueue::error::VirtqError;
 use crate::drivers::virtio::virtqueue::split::SplitVq;
 use crate::drivers::virtio::virtqueue::{
-	AvailBufferToken, BufferElem, BufferType, Virtq, VqIndex, VqSize,
+	AvailBufferToken, BufferElem, BufferType, VirtQueue, Virtq, VqIndex, VqSize,
 };
 use crate::fs::fuse::{self, FuseInterface, Rsp, RspHeader};
 use crate::mm::device_alloc::DeviceAlloc;
@@ -43,7 +43,7 @@ pub(crate) struct VirtioFsDriver {
 	pub(super) com_cfg: ComCfg,
 	pub(super) isr_stat: IsrStatus,
 	pub(super) notif_cfg: NotifCfg,
-	pub(super) vqueues: Vec<Box<dyn Virtq>>,
+	pub(super) vqueues: Vec<VirtQueue>,
 	pub(super) irq: InterruptLine,
 }
 
@@ -133,15 +133,17 @@ impl VirtioFsDriver {
 
 		// create the queues and tell device about them
 		for i in 0..vqnum as u16 {
-			let vq = SplitVq::new(
-				&mut self.com_cfg,
-				&self.notif_cfg,
-				VqSize::from(VIRTIO_MAX_QUEUE_SIZE),
-				VqIndex::from(i),
-				self.dev_cfg.features.into(),
-			)
-			.unwrap();
-			self.vqueues.push(Box::new(vq));
+			let vq = VirtQueue::Split(
+				SplitVq::new(
+					&mut self.com_cfg,
+					&self.notif_cfg,
+					VqSize::from(VIRTIO_MAX_QUEUE_SIZE),
+					VqIndex::from(i),
+					self.dev_cfg.features.into(),
+				)
+				.unwrap(),
+			);
+			self.vqueues.push(vq);
 		}
 
 		// At this point the device is "live"

src/drivers/fs/virtio_fs.rs

@@ -2,7 +2,6 @@
 //!
 //! The module contains ...
 
-use alloc::boxed::Box;
 use alloc::vec::Vec;
 use core::str::FromStr;
 
@@ -14,7 +13,7 @@ use crate::drivers::InterruptLine;
 use crate::drivers::net::virtio::{CtrlQueue, NetDevCfg, RxQueues, TxQueues, VirtioNetDriver};
 use crate::drivers::virtio::error::{VirtioError, VirtioNetError};
 use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
-use crate::drivers::virtio::virtqueue::Virtq;
+use crate::drivers::virtio::virtqueue::VirtQueue;
 
 // Backend-dependent interface for Virtio network driver
 impl VirtioNetDriver {
@@ -48,8 +47,8 @@ impl VirtioNetDriver {
 			1514
 		};
 
-		let send_vqs = TxQueues::new(Vec::<Box<dyn Virtq>>::new(), &dev_cfg);
-		let recv_vqs = RxQueues::new(Vec::<Box<dyn Virtq>>::new(), &dev_cfg);
+		let send_vqs = TxQueues::new(Vec::<VirtQueue>::new(), &dev_cfg);
+		let recv_vqs = RxQueues::new(Vec::<VirtQueue>::new(), &dev_cfg);
 		Ok(VirtioNetDriver {
 			dev_cfg,
 			com_cfg: ComCfg::new(registers, 1),

src/drivers/net/virtio/mmio.rs

@@ -31,7 +31,7 @@ use crate::drivers::virtio::transport::pci::{ComCfg, IsrStatus, NotifCfg};
 use crate::drivers::virtio::virtqueue::packed::PackedVq;
 use crate::drivers::virtio::virtqueue::split::SplitVq;
 use crate::drivers::virtio::virtqueue::{
-	AvailBufferToken, BufferElem, BufferType, UsedBufferToken, Virtq, VqIndex, VqSize,
+	AvailBufferToken, BufferElem, BufferType, UsedBufferToken, VirtQueue, Virtq, VqIndex, VqSize,
 };
 use crate::drivers::{Driver, InterruptLine};
 use crate::executor::device::{RxToken, TxToken};
@@ -46,21 +46,21 @@ pub(crate) struct NetDevCfg {
 	pub features: virtio::net::F,
 }
 
-pub struct CtrlQueue(Option<Box<dyn Virtq>>);
+pub struct CtrlQueue(Option<VirtQueue>);
 
 impl CtrlQueue {
-	pub fn new(vq: Option<Box<dyn Virtq>>) -> Self {
+	pub fn new(vq: Option<VirtQueue>) -> Self {
 		CtrlQueue(vq)
 	}
 }
 
 pub struct RxQueues {
-	vqs: Vec<Box<dyn Virtq>>,
+	vqs: Vec<VirtQueue>,
 	packet_size: u32,
 }
 
 impl RxQueues {
-	pub fn new(vqs: Vec<Box<dyn Virtq>>, dev_cfg: &NetDevCfg) -> Self {
+	pub fn new(vqs: Vec<VirtQueue>, dev_cfg: &NetDevCfg) -> Self {
 		// See Virtio specification v1.1 - 5.1.6.3.1
 		//
 		let packet_size = if dev_cfg.features.contains(virtio::net::F::MRG_RXBUF) {
@@ -83,10 +83,10 @@ impl RxQueues {
 	/// Adds a given queue to the underlying vector and populates the queue with RecvBuffers.
 	///
 	/// Queues are all populated according to Virtio specification v1.1. - 5.1.6.3.1
-	fn add(&mut self, mut vq: Box<dyn Virtq>) {
+	fn add(&mut self, mut vq: VirtQueue) {
 		const BUFF_PER_PACKET: u16 = 2;
 		let num_packets: u16 = u16::from(vq.size()) / BUFF_PER_PACKET;
-		fill_queue(vq.as_mut(), num_packets, self.packet_size);
+		fill_queue(&mut vq, num_packets, self.packet_size);
 		self.vqs.push(vq);
 	}
 
@@ -111,7 +111,7 @@ impl RxQueues {
 	}
 }
 
-fn fill_queue(vq: &mut dyn Virtq, num_packets: u16, packet_size: u32) {
+fn fill_queue(vq: &mut VirtQueue, num_packets: u16, packet_size: u32) {
 	for _ in 0..num_packets {
 		let buff_tkn = match AvailBufferToken::new(
 			vec![],
@@ -143,14 +143,14 @@ fn fill_queue(vq: &mut dyn Virtq, num_packets: u16, packet_size: u32) {
 /// Structure which handles transmission of packets and delegation
 /// to the respective queue structures.
 pub struct TxQueues {
-	vqs: Vec<Box<dyn Virtq>>,
+	vqs: Vec<VirtQueue>,
 	/// Indicates, whether the Driver/Device are using multiple
 	/// queues for communication.
 	packet_length: u32,
 }
 
 impl TxQueues {
-	pub fn new(vqs: Vec<Box<dyn Virtq>>, dev_cfg: &NetDevCfg) -> Self {
+	pub fn new(vqs: Vec<VirtQueue>, dev_cfg: &NetDevCfg) -> Self {
 		let packet_length = if dev_cfg.features.contains(virtio::net::F::GUEST_TSO4)
 			| dev_cfg.features.contains(virtio::net::F::GUEST_TSO6)
 			| dev_cfg.features.contains(virtio::net::F::GUEST_UFO)
@@ -184,7 +184,7 @@ impl TxQueues {
 		}
 	}
 
-	fn add(&mut self, vq: Box<dyn Virtq>) {
+	fn add(&mut self, vq: VirtQueue) {
 		// Currently we are doing nothing with the additional queues. They are inactive and might be used in the
 		// future
 		self.vqs.push(vq);
@@ -333,7 +333,7 @@ impl NetworkDriver for VirtioNetDriver {
 		}
 
 		fill_queue(
-			self.recv_vqs.vqs[0].as_mut(),
+			&mut self.recv_vqs.vqs[0],
 			num_buffers,
 			self.recv_vqs.packet_size,
 		);
@@ -657,7 +657,7 @@ impl VirtioNetDriver {
 		// Add a control if feature is negotiated
 		if self.dev_cfg.features.contains(virtio::net::F::CTRL_VQ) {
 			if self.dev_cfg.features.contains(virtio::net::F::RING_PACKED) {
-				self.ctrl_vq = CtrlQueue(Some(Box::new(
+				self.ctrl_vq = CtrlQueue(Some(VirtQueue::Packed(
 					PackedVq::new(
 						&mut self.com_cfg,
 						&self.notif_cfg,
@@ -668,7 +668,7 @@ impl VirtioNetDriver {
 					.unwrap(),
 				)));
 			} else {
-				self.ctrl_vq = CtrlQueue(Some(Box::new(
+				self.ctrl_vq = CtrlQueue(Some(VirtQueue::Split(
 					SplitVq::new(
 						&mut self.com_cfg,
 						&self.notif_cfg,
@@ -744,7 +744,7 @@ impl VirtioNetDriver {
 				// Interrupt for receiving packets is wanted
 				vq.enable_notifs();
 
-				self.recv_vqs.add(Box::from(vq));
+				self.recv_vqs.add(VirtQueue::Packed(vq));
 
 				let mut vq = PackedVq::new(
 					&mut self.com_cfg,
@@ -757,7 +757,7 @@ impl VirtioNetDriver {
 				// Interrupt for communicating that a sended packet left, is not needed
 				vq.disable_notifs();
 
-				self.send_vqs.add(Box::from(vq));
+				self.send_vqs.add(VirtQueue::Packed(vq));
 			} else {
 				let mut vq = SplitVq::new(
 					&mut self.com_cfg,
@@ -770,7 +770,7 @@ impl VirtioNetDriver {
 				// Interrupt for receiving packets is wanted
 				vq.enable_notifs();
 
-				self.recv_vqs.add(Box::from(vq));
+				self.recv_vqs.add(VirtQueue::Split(vq));
 
 				let mut vq = SplitVq::new(
 					&mut self.com_cfg,
@@ -783,7 +783,7 @@ impl VirtioNetDriver {
 				// Interrupt for communicating that a sended packet left, is not needed
 				vq.disable_notifs();
 
-				self.send_vqs.add(Box::from(vq));
+				self.send_vqs.add(VirtQueue::Split(vq));
 			}
 		}
 

src/drivers/net/virtio/mod.rs

@@ -542,7 +542,7 @@ pub(crate) fn init() {
 				}
 				#[cfg(feature = "vsock")]
 				Ok(VirtioDriver::Vsock(drv)) => {
-					register_driver(PciDriver::VirtioVsock(InterruptTicketMutex::new(drv)));
+					register_driver(PciDriver::VirtioVsock(InterruptTicketMutex::new(*drv)));
 				}
 				#[cfg(feature = "fuse")]
 				Ok(VirtioDriver::FileSystem(drv)) => {

src/drivers/pci.rs

@@ -3,6 +3,7 @@
 //! The module contains ...
 #![allow(dead_code)]
 
+use alloc::boxed::Box;
 use alloc::vec::Vec;
 use core::ptr::NonNull;
 use core::{mem, ptr};
@@ -836,7 +837,7 @@ pub(crate) fn init_device(
 				crate::arch::interrupts::add_irq_name(irq, "virtio");
 				info!("Virtio interrupt handler at line {irq}");
 
-				Ok(VirtioDriver::Vsock(virt_sock_drv))
+				Ok(VirtioDriver::Vsock(Box::new(virt_sock_drv)))
 			}
 			Err(virtio_error) => {
 				error!("Virtio sock driver could not be initialized with device: {device_id:x}");
@@ -878,7 +879,7 @@ pub(crate) enum VirtioDriver {
 	))]
 	Network(VirtioNetDriver),
 	#[cfg(feature = "vsock")]
-	Vsock(VirtioVsockDriver),
+	Vsock(Box<VirtioVsockDriver>),
 	#[cfg(feature = "fuse")]
 	FileSystem(VirtioFsDriver),
 }

src/drivers/virtio/transport/pci.rs

@@ -19,15 +19,14 @@ use core::any::Any;
 use core::mem::MaybeUninit;
 use core::{mem, ptr};
 
+use enum_dispatch::enum_dispatch;
 use memory_addresses::VirtAddr;
 use virtio::{le32, le64, pvirtq, virtq};
 
 use self::error::VirtqError;
-#[cfg(not(feature = "pci"))]
-use super::transport::mmio::{ComCfg, NotifCfg};
-#[cfg(feature = "pci")]
-use super::transport::pci::{ComCfg, NotifCfg};
 use crate::arch::mm::paging;
+use crate::drivers::virtio::virtqueue::packed::PackedVq;
+use crate::drivers::virtio::virtqueue::split::SplitVq;
 use crate::mm::device_alloc::DeviceAlloc;
 
 /// A u16 newtype. If instantiated via ``VqIndex::from(T)``, the newtype is ensured to be
@@ -97,6 +96,7 @@ impl From<VqSize> for u16 {
 /// might not provide the complete feature set of each queue. Drivers who
 /// do need these features should refrain from providing support for both
 /// Virtqueue types and use the structs directly instead.
+#[enum_dispatch]
 pub trait Virtq: Send {
 	/// The `notif` parameter indicates if the driver wants to have a notification for this specific
 	/// transfer. This is only for performance optimization. As it is NOT ensured, that the device sees the
@@ -185,21 +185,6 @@ pub trait Virtq: Send {
 		notif: bool,
 	) -> Result<(), VirtqError>;
 
-	/// Creates a new Virtq of the specified [VqSize] and the [VqIndex].
-	/// The index represents the "ID" of the virtqueue.
-	/// Upon creation the virtqueue is "registered" at the device via the `ComCfg` struct.
-	///
-	/// Be aware, that devices define a maximum number of queues and a maximal size they can handle.
-	fn new(
-		com_cfg: &mut ComCfg,
-		notif_cfg: &NotifCfg,
-		size: VqSize,
-		index: VqIndex,
-		features: virtio::F,
-	) -> Result<Self, VirtqError>
-	where
-		Self: Sized;
-
 	/// Returns the size of a Virtqueue. This represents the overall size and not the capacity the
 	/// queue currently has for new descriptors.
 	fn size(&self) -> VqSize;
@@ -282,6 +267,12 @@ trait VirtqPrivate {
 	}
 }
 
+#[enum_dispatch(Virtq)]
+pub(crate) enum VirtQueue {
+	Split(SplitVq),
+	Packed(PackedVq),
+}
+
 trait VirtqDescriptor {
 	fn flags_mut(&mut self) -> &mut virtq::DescF;