feature(pci): MSI-X support

cloud-hypervisor only supports MSI-X interrupts for PCI devices, so
support for MSI-X is needed to support running on it. Additionally,
MSI-X can allow us to set separate interrupt handlers for the
configuration change interrupts and queue updates (even per-queue
handlers once we have multiple queues) in the future.

MSI-X is not working correctly or at all on all platforms, so it is used
as a fallback.

Author: cagatay-y

src/drivers/net/virtio/mmio.rs

@@ -39,7 +39,7 @@ impl VirtioNetDriver<Uninit> {
 			notif_cfg,
 			inner: Uninit,
 			num_vqs: 0,
-			irq,
+			irq: Some(irq),
 			checksums: ChecksumCapabilities::default(),
 		})
 	}

src/drivers/net/virtio/mod.rs

@@ -32,6 +32,8 @@ 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};
+#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+use crate::drivers::pci::MsixEntry;
 use crate::drivers::virtio::ControlRegisters;
 #[cfg(not(feature = "pci"))]
 use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
@@ -239,11 +241,13 @@ pub(crate) struct VirtioNetDriver<T = Init> {
 	pub(super) com_cfg: ComCfg,
 	pub(super) isr_stat: IsrStatus,
 	pub(super) notif_cfg: NotifCfg,
+	#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+	pub(super) msix_table: Option<VolatileRef<'static, [MsixEntry]>>,
 
 	pub(super) inner: T,
 
 	pub(super) num_vqs: u16,
-	pub(super) irq: InterruptLine,
+	pub(super) irq: Option<InterruptLine>,
 	pub(super) checksums: ChecksumCapabilities,
 }
 
@@ -481,7 +485,7 @@ impl smoltcp::phy::Device for VirtioNetDriver {
 
 impl Driver for VirtioNetDriver<Init> {
 	fn get_interrupt_number(&self) -> InterruptLine {
-		self.irq
+		self.irq.unwrap()
 	}
 
 	fn get_name(&self) -> &'static str {
@@ -737,11 +741,33 @@ impl VirtioNetDriver<Uninit> {
 		}
 		debug!("{:?}", self.checksums);
 
+		// If self.irq is some, it was filled with a legacy interrupt number which we prefer over MSI-X.
+		#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+		if self.irq.is_none()
+			&& let Some(msix_table) = self.msix_table.as_mut()
+		{
+			warn!(
+				"Setting up message signaled interrupts as a fallback. MSI-X is known to not work on all platforms (e.g. QEMU with TAP)."
+			);
+			// Chosen arbitatrily. Ideally does not clash with another interrupt line.
+			const MSIX_VECTOR: u8 = 112;
+			let msix_entry = unsafe {
+				msix_table
+					.as_mut_ptr()
+					.map(|table| table.get_unchecked_mut(0))
+			};
+			MsixEntry::configure(msix_entry, MSIX_VECTOR);
+			self.com_cfg.select_vq(0).unwrap().set_msix_table_index(0);
+			self.irq = Some(MSIX_VECTOR);
+		};
+
 		Ok(VirtioNetDriver {
 			dev_cfg: self.dev_cfg,
 			com_cfg: self.com_cfg,
 			isr_stat: self.isr_stat,
 			notif_cfg: self.notif_cfg,
+			#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+			msix_table: self.msix_table,
 			inner,
 			num_vqs: self.num_vqs,
 			irq: self.irq,

src/drivers/net/virtio/pci.rs

@@ -36,6 +36,8 @@ impl VirtioNetDriver<Uninit> {
 			notif_cfg,
 			isr_cfg,
 			dev_cfg_list,
+			#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+			msix_table,
 			..
 		} = caps_coll;
 
@@ -44,14 +46,21 @@ impl VirtioNetDriver<Uninit> {
 			return Err(error::VirtioNetError::NoDevCfg(device_id));
 		};
 
+		let irq = device.get_irq();
+		if irq.is_none() {
+			warn!("No interrupt lanes found for virtio-net.");
+		}
+
 		Ok(VirtioNetDriver {
 			dev_cfg,
 			com_cfg,
 			isr_stat: isr_cfg,
 			notif_cfg,
+			#[cfg(all(feature = "pci", target_arch = "x86_64"))]
+			msix_table,
 			inner: Uninit,
 			num_vqs: 0,
-			irq: device.get_irq().unwrap(),
+			irq,
 			checksums: ChecksumCapabilities::default(),
 		})
 	}

src/drivers/pci.rs

@@ -537,6 +537,30 @@ pub(crate) fn init() {
 	});
 }
 
+#[repr(C)]
+pub(crate) struct MsixEntry {
+	addr_low: u32,
+	addr_high: u32,
+	data: u32,
+	control: u32,
+}
+
+impl MsixEntry {
+	#[cfg(target_arch = "x86_64")]
+	pub fn configure(msix_entry: volatile::VolatilePtr<'_, Self>, vector: u8) {
+		use bit_field::BitField;
+		use volatile::map_field;
+
+		// Mask the entry because "[s]oftware must not modify the Address, Data, or Steering Tag fields
+		// of an entry while it is unmasked." (PCIe spec. 6.1.4.2)
+		map_field!(msix_entry.control).update(|mut control| *control.set_bit(0, true));
+
+		map_field!(msix_entry.addr_low).update(|mut addr_low| *addr_low.set_bits(20..32, 0xfee));
+		map_field!(msix_entry.data).update(|mut data| *data.set_bits(0..8, u32::from(vector) + 32));
+		map_field!(msix_entry.control).update(|mut control| *control.set_bit(0, false));
+	}
+}
+
 /// A module containing PCI specific errors
 ///
 /// Errors include...

src/drivers/virtio/transport/pci.rs

@@ -181,6 +181,12 @@ pub struct UniCapsColl {
 	pub(crate) notif_cfg: NotifCfg,
 	pub(crate) isr_cfg: IsrStatus,
 	pub(crate) dev_cfg_list: Vec<PciCap>,
+	#[cfg(all(
+		feature = "virtio-net",
+		not(feature = "rtl8139"),
+		target_arch = "x86_64"
+	))]
+	pub(crate) msix_table: Option<VolatileRef<'static, [crate::drivers::pci::MsixEntry]>>,
 }
 /// Wraps a [`CommonCfg`] in order to preserve
 /// the original structure.
@@ -256,6 +262,19 @@ impl VqCfgHandler<'_> {
 			.write(addr.as_u64().into());
 	}
 
+	#[cfg(all(
+		feature = "virtio-net",
+		not(feature = "rtl8139"),
+		target_arch = "x86_64"
+	))]
+	pub fn set_msix_table_index(&mut self, index: u16) {
+		self.select_queue();
+		self.raw
+			.as_mut_ptr()
+			.queue_msix_vector()
+			.write(index.into());
+	}
+
 	pub fn notif_off(&mut self) -> u16 {
 		self.select_queue();
 		self.raw.as_mut_ptr().queue_notify_off().read().to_ne()
@@ -509,43 +528,6 @@ impl PciBar {
 	}
 }
 
-/// Reads all PCI capabilities, starting at the capabilities list pointer from the
-/// PCI device.
-///
-/// Returns ONLY Virtio specific capabilities, which allow to locate the actual capability
-/// structures inside the memory areas, indicated by the BaseAddressRegisters (BAR's).
-fn read_caps(device: &PciDevice<PciConfigRegion>) -> Result<Vec<PciCap>, PciError> {
-	let device_id = device.device_id();
-
-	let capabilities = device
-		.capabilities()
-		.unwrap()
-		.filter_map(|capability| match capability {
-			PciCapability::Vendor(capability) => Some(capability),
-			_ => None,
-		})
-		.map(|addr| CapData::read(addr, device.access()).unwrap())
-		.filter(|cap| cap.cfg_type != CapCfgType::Pci)
-		.flat_map(|cap| {
-			let slot = cap.bar;
-			device
-				.memory_map_bar(slot, true)
-				.map(|(addr, size)| PciCap {
-					bar: VirtioPciBar::new(slot, addr.as_u64(), size.try_into().unwrap()),
-					dev_id: device_id,
-					cap,
-				})
-		})
-		.collect::<Vec<_>>();
-
-	if capabilities.is_empty() {
-		error!("No virtio capability found for device {device_id:x}");
-		return Err(PciError::NoVirtioCaps(device_id));
-	}
-
-	Ok(capabilities)
-}
-
 pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsColl, VirtioError> {
 	let device_id = device.device_id();
 
@@ -555,58 +537,106 @@ pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsCol
 		return Err(VirtioError::FromPci(PciError::NoCapPtr(device_id)));
 	}
 
-	// Get list of PciCaps pointing to capabilities
-	let cap_list = match read_caps(device) {
-		Ok(list) => list,
-		Err(pci_error) => return Err(VirtioError::FromPci(pci_error)),
-	};
-
 	let mut com_cfg = None;
 	let mut notif_cfg = None;
 	let mut isr_cfg = None;
 	let mut dev_cfg_list = Vec::new();
-	// Map Caps in virtual memory
-	for pci_cap in cap_list {
-		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!"
-						),
-					}
+	#[cfg(all(
+		feature = "virtio-net",
+		not(feature = "rtl8139"),
+		target_arch = "x86_64"
+	))]
+	let mut msix_table = None;
+
+	// Reads all PCI capabilities, starting at the capabilities list pointer from the
+	// PCI device.
+	//
+	// Maps ONLY Virtio specific capabilities and the MSI-X capability , which allow to locate the actual capability
+	// structures inside the memory areas, indicated by the BaseAddressRegisters (BAR's).
+	for capability in device.capabilities().unwrap() {
+		match capability {
+			PciCapability::Vendor(addr) => {
+				let cap = CapData::read(addr, device.access()).unwrap();
+				if cap.cfg_type == CapCfgType::Pci {
+					continue;
 				}
-			}
-			CapCfgType::Notify => {
-				if notif_cfg.is_none() {
-					match NotifCfg::new(&pci_cap) {
-						Some(notif) => notif_cfg = Some(notif),
-						None => error!(
-							"Notification config capability of device {device_id:x} could not be used!"
-						),
+				let slot = cap.bar;
+				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()),
+					dev_id: device_id,
+					cap,
+				};
+				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!"
+								),
+							}
+						}
 					}
-				}
-			}
-			CapCfgType::Isr => {
-				if isr_cfg.is_none() {
-					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!"
-						),
+					CapCfgType::Notify => {
+						if notif_cfg.is_none() {
+							match NotifCfg::new(&pci_cap) {
+								Some(notif) => notif_cfg = Some(notif),
+								None => error!(
+									"Notification config capability of device {device_id:x} could not be used!"
+								),
+							}
+						}
+					}
+					CapCfgType::Isr => {
+						if isr_cfg.is_none() {
+							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!"
+								),
+							}
+						}
 					}
+					CapCfgType::SharedMemory => {
+						let cap_id = pci_cap.cap.id;
+						error!(
+							"Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!"
+						);
+					}
+					CapCfgType::Device => dev_cfg_list.push(pci_cap),
+					_ => continue,
 				}
 			}
-			CapCfgType::SharedMemory => {
-				let cap_id = pci_cap.cap.id;
-				error!(
-					"Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!"
+			#[cfg(all(
+				feature = "virtio-net",
+				not(feature = "rtl8139"),
+				target_arch = "x86_64"
+			))]
+			PciCapability::MsiX(mut msix_capability) => {
+				// We prefer legacy interrupts
+				if device.get_irq().is_some() {
+					continue;
+				}
+				msix_capability.set_enabled(true, device.access());
+				let (base_addr, _) = device
+					.memory_map_bar(msix_capability.table_bar(), true)
+					.unwrap();
+				let table_ptr = NonNull::slice_from_raw_parts(
+					NonNull::with_exposed_provenance(
+						core::num::NonZero::new(
+							base_addr.as_usize()
+								+ usize::try_from(msix_capability.table_offset()).unwrap(),
+						)
+						.unwrap(),
+					),
+					msix_capability.table_size().into(),
 				);
+				msix_table = Some(unsafe { VolatileRef::new(table_ptr) });
 			}
-			CapCfgType::Device => dev_cfg_list.push(pci_cap),
-
-			// PCI's configuration space is allowed to hold other structures, which are not virtio specific and are therefore ignored
+			// PCI's configuration space is allowed to hold other structures, which are not useful for us and are therefore ignored
 			// in the following
 			_ => continue,
 		}
@@ -617,6 +647,12 @@ pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsCol
 		notif_cfg: notif_cfg.ok_or(VirtioError::NoNotifCfg(device_id))?,
 		isr_cfg: isr_cfg.ok_or(VirtioError::NoIsrCfg(device_id))?,
 		dev_cfg_list,
+		#[cfg(all(
+			feature = "virtio-net",
+			not(feature = "rtl8139"),
+			target_arch = "x86_64"
+		))]
+		msix_table,
 	})
 }
 
@@ -686,9 +722,10 @@ pub(crate) fn init_device(
 		))]
 		virtio::Id::Net => match VirtioNetDriver::init(device) {
 			Ok(virt_net_drv) => {
+				use crate::drivers::Driver;
 				info!("Virtio network driver initialized.");
 
-				let irq = device.get_irq().unwrap();
+				let irq = virt_net_drv.get_interrupt_number();
 				crate::arch::interrupts::add_irq_name(irq, "virtio");
 				info!("Virtio interrupt handler at line {irq}");