Skip to content

Commit 2f3ab43

Browse files
committed
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.
1 parent 944570d commit 2f3ab43

5 files changed

Lines changed: 151 additions & 83 deletions

File tree

src/drivers/net/virtio/mmio.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl VirtioNetDriver<Uninit> {
3939
notif_cfg,
4040
inner: Uninit,
4141
num_vqs: 0,
42-
irq,
42+
irq: Some(irq),
4343
checksums: ChecksumCapabilities::default(),
4444
})
4545
}

src/drivers/net/virtio/mod.rs

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ use self::error::VirtioNetError;
3131
use crate::config::VIRTIO_MAX_QUEUE_SIZE;
3232
use crate::drivers::net::virtio::constants::BUFF_PER_PACKET;
3333
use crate::drivers::net::{NetworkDriver, mtu};
34+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
35+
use crate::drivers::pci::MsixEntry;
3436
use crate::drivers::virtio::ControlRegisters;
3537
#[cfg(not(feature = "pci"))]
3638
use crate::drivers::virtio::transport::mmio::{ComCfg, IsrStatus, NotifCfg};
@@ -238,11 +240,13 @@ pub(crate) struct VirtioNetDriver<T = Init> {
238240
pub(super) com_cfg: ComCfg,
239241
pub(super) isr_stat: IsrStatus,
240242
pub(super) notif_cfg: NotifCfg,
243+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
244+
pub(super) msix_table: Option<VolatileRef<'static, [MsixEntry]>>,
241245

242246
pub(super) inner: T,
243247

244248
pub(super) num_vqs: u16,
245-
pub(super) irq: InterruptLine,
249+
pub(super) irq: Option<InterruptLine>,
246250
pub(super) checksums: ChecksumCapabilities,
247251
}
248252

@@ -476,7 +480,7 @@ impl smoltcp::phy::Device for VirtioNetDriver {
476480

477481
impl Driver for VirtioNetDriver<Init> {
478482
fn get_interrupt_number(&self) -> InterruptLine {
479-
self.irq
483+
self.irq.unwrap()
480484
}
481485

482486
fn get_name(&self) -> &'static str {
@@ -731,11 +735,30 @@ impl VirtioNetDriver<Uninit> {
731735
}
732736
debug!("{:?}", self.checksums);
733737

738+
// If self.irq is some, it was filled with a legacy interrupt number which we prefer over MSI-X.
739+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
740+
if self.irq.is_none()
741+
&& let Some(msix_table) = self.msix_table.as_mut()
742+
{
743+
// Chosen arbitatrily. Ideally does not clash with another interrupt line.
744+
const MSIX_VECTOR: u8 = 112;
745+
let msix_entry = unsafe {
746+
msix_table
747+
.as_mut_ptr()
748+
.map(|table| table.get_unchecked_mut(0))
749+
};
750+
MsixEntry::configure(msix_entry, MSIX_VECTOR);
751+
self.com_cfg.select_vq(0).unwrap().set_msix_table_index(0);
752+
self.irq = Some(MSIX_VECTOR);
753+
};
754+
734755
Ok(VirtioNetDriver {
735756
dev_cfg: self.dev_cfg,
736757
com_cfg: self.com_cfg,
737758
isr_stat: self.isr_stat,
738759
notif_cfg: self.notif_cfg,
760+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
761+
msix_table: self.msix_table,
739762
inner,
740763
num_vqs: self.num_vqs,
741764
irq: self.irq,

src/drivers/net/virtio/pci.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ impl VirtioNetDriver<Uninit> {
3636
notif_cfg,
3737
isr_cfg,
3838
dev_cfg_list,
39+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
40+
msix_table,
3941
..
4042
} = caps_coll;
4143

@@ -49,9 +51,11 @@ impl VirtioNetDriver<Uninit> {
4951
com_cfg,
5052
isr_stat: isr_cfg,
5153
notif_cfg,
54+
#[cfg(all(feature = "pci", target_arch = "x86_64"))]
55+
msix_table,
5256
inner: Uninit,
5357
num_vqs: 0,
54-
irq: device.get_irq().unwrap(),
58+
irq: device.get_irq(),
5559
checksums: ChecksumCapabilities::default(),
5660
})
5761
}

src/drivers/pci.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,30 @@ pub(crate) fn init() {
539539
});
540540
}
541541

542+
#[repr(C)]
543+
pub(crate) struct MsixEntry {
544+
addr_low: u32,
545+
addr_high: u32,
546+
data: u32,
547+
control: u32,
548+
}
549+
550+
impl MsixEntry {
551+
#[cfg(target_arch = "x86_64")]
552+
pub fn configure(msix_entry: volatile::VolatilePtr<'_, Self>, vector: u8) {
553+
use bit_field::BitField;
554+
use volatile::map_field;
555+
556+
// Mask the entry because "[s]oftware must not modify the Address, Data, or Steering Tag fields
557+
// of an entry while it is unmasked." (PCIe spec. 6.1.4.2)
558+
map_field!(msix_entry.control).update(|mut control| *control.set_bit(0, true));
559+
560+
map_field!(msix_entry.addr_low).update(|mut addr_low| *addr_low.set_bits(20..32, 0xfee));
561+
map_field!(msix_entry.data).update(|mut data| *data.set_bits(0..8, u32::from(vector) + 32));
562+
map_field!(msix_entry.control).update(|mut control| *control.set_bit(0, false));
563+
}
564+
}
565+
542566
/// A module containing PCI specific errors
543567
///
544568
/// Errors include...

src/drivers/virtio/transport/pci.rs

Lines changed: 96 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ pub struct UniCapsColl {
181181
pub(crate) notif_cfg: NotifCfg,
182182
pub(crate) isr_cfg: IsrStatus,
183183
pub(crate) dev_cfg_list: Vec<PciCap>,
184+
#[cfg(all(feature = "virtio-net", target_arch = "x86_64"))]
185+
pub(crate) msix_table: Option<VolatileRef<'static, [crate::drivers::pci::MsixEntry]>>,
184186
}
185187
/// Wraps a [`CommonCfg`] in order to preserve
186188
/// the original structure.
@@ -256,6 +258,15 @@ impl VqCfgHandler<'_> {
256258
.write(addr.as_u64().into());
257259
}
258260

261+
#[cfg(all(feature = "virtio-net", target_arch = "x86_64"))]
262+
pub fn set_msix_table_index(&mut self, index: u16) {
263+
self.select_queue();
264+
self.raw
265+
.as_mut_ptr()
266+
.queue_msix_vector()
267+
.write(index.into());
268+
}
269+
259270
pub fn notif_off(&mut self) -> u16 {
260271
self.select_queue();
261272
self.raw.as_mut_ptr().queue_notify_off().read().to_ne()
@@ -515,43 +526,6 @@ impl PciBar {
515526
}
516527
}
517528

518-
/// Reads all PCI capabilities, starting at the capabilities list pointer from the
519-
/// PCI device.
520-
///
521-
/// Returns ONLY Virtio specific capabilities, which allow to locate the actual capability
522-
/// structures inside the memory areas, indicated by the BaseAddressRegisters (BAR's).
523-
fn read_caps(device: &PciDevice<PciConfigRegion>) -> Result<Vec<PciCap>, PciError> {
524-
let device_id = device.device_id();
525-
526-
let capabilities = device
527-
.capabilities()
528-
.unwrap()
529-
.filter_map(|capability| match capability {
530-
PciCapability::Vendor(capability) => Some(capability),
531-
_ => None,
532-
})
533-
.map(|addr| CapData::read(addr, device.access()).unwrap())
534-
.filter(|cap| cap.cfg_type != CapCfgType::Pci)
535-
.flat_map(|cap| {
536-
let slot = cap.bar;
537-
device
538-
.memory_map_bar(slot, true)
539-
.map(|(addr, size)| PciCap {
540-
bar: VirtioPciBar::new(slot, addr.as_u64(), size.try_into().unwrap()),
541-
dev_id: device_id,
542-
cap,
543-
})
544-
})
545-
.collect::<Vec<_>>();
546-
547-
if capabilities.is_empty() {
548-
error!("No virtio capability found for device {device_id:x}");
549-
Err(PciError::NoVirtioCaps(device_id))
550-
} else {
551-
Ok(capabilities)
552-
}
553-
}
554-
555529
pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsColl, VirtioError> {
556530
let device_id = device.device_id();
557531

@@ -561,58 +535,98 @@ pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsCol
561535
return Err(VirtioError::FromPci(PciError::NoCapPtr(device_id)));
562536
}
563537

564-
// Get list of PciCaps pointing to capabilities
565-
let cap_list = match read_caps(device) {
566-
Ok(list) => list,
567-
Err(pci_error) => return Err(VirtioError::FromPci(pci_error)),
568-
};
569-
570538
let mut com_cfg = None;
571539
let mut notif_cfg = None;
572540
let mut isr_cfg = None;
573541
let mut dev_cfg_list = Vec::new();
574-
// Map Caps in virtual memory
575-
for pci_cap in cap_list {
576-
match pci_cap.cap.cfg_type {
577-
CapCfgType::Common => {
578-
if com_cfg.is_none() {
579-
match pci_cap.map_common_cfg() {
580-
Some(cap) => com_cfg = Some(ComCfg::new(cap)),
581-
None => error!(
582-
"Common config capability of device {device_id:x} could not be mapped!"
583-
),
584-
}
542+
#[cfg(all(feature = "virtio-net", target_arch = "x86_64"))]
543+
let mut msix_table = None;
544+
545+
// Reads all PCI capabilities, starting at the capabilities list pointer from the
546+
// PCI device.
547+
//
548+
// Maps ONLY Virtio specific capabilities and the MSI-X capability , which allow to locate the actual capability
549+
// structures inside the memory areas, indicated by the BaseAddressRegisters (BAR's).
550+
for capability in device.capabilities().unwrap() {
551+
match capability {
552+
PciCapability::Vendor(addr) => {
553+
let cap = CapData::read(addr, device.access()).unwrap();
554+
if cap.cfg_type == CapCfgType::Pci {
555+
continue;
585556
}
586-
}
587-
CapCfgType::Notify => {
588-
if notif_cfg.is_none() {
589-
match NotifCfg::new(&pci_cap) {
590-
Some(notif) => notif_cfg = Some(notif),
591-
None => error!(
592-
"Notification config capability of device {device_id:x} could not be used!"
593-
),
557+
let slot = cap.bar;
558+
let Some((addr, size)) = device.memory_map_bar(slot, true) else {
559+
continue;
560+
};
561+
let pci_cap = PciCap {
562+
bar: VirtioPciBar::new(slot, addr.as_u64(), size.try_into().unwrap()),
563+
dev_id: device_id,
564+
cap,
565+
};
566+
match pci_cap.cap.cfg_type {
567+
CapCfgType::Common => {
568+
if com_cfg.is_none() {
569+
match pci_cap.map_common_cfg() {
570+
Some(cap) => com_cfg = Some(ComCfg::new(cap)),
571+
None => error!(
572+
"Common config capability of device {device_id:x} could not be mapped!"
573+
),
574+
}
575+
}
594576
}
595-
}
596-
}
597-
CapCfgType::Isr => {
598-
if isr_cfg.is_none() {
599-
match pci_cap.map_isr_status() {
600-
Some(isr_stat) => isr_cfg = Some(IsrStatus::new(isr_stat)),
601-
None => error!(
602-
"ISR status config capability of device {device_id:x} could not be used!"
603-
),
577+
CapCfgType::Notify => {
578+
if notif_cfg.is_none() {
579+
match NotifCfg::new(&pci_cap) {
580+
Some(notif) => notif_cfg = Some(notif),
581+
None => error!(
582+
"Notification config capability of device {device_id:x} could not be used!"
583+
),
584+
}
585+
}
586+
}
587+
CapCfgType::Isr => {
588+
if isr_cfg.is_none() {
589+
match pci_cap.map_isr_status() {
590+
Some(isr_stat) => isr_cfg = Some(IsrStatus::new(isr_stat)),
591+
None => error!(
592+
"ISR status config capability of device {device_id:x} could not be used!"
593+
),
594+
}
595+
}
596+
}
597+
CapCfgType::SharedMemory => {
598+
let cap_id = pci_cap.cap.id;
599+
error!(
600+
"Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!"
601+
);
604602
}
603+
CapCfgType::Device => dev_cfg_list.push(pci_cap),
604+
_ => continue,
605605
}
606606
}
607-
CapCfgType::SharedMemory => {
608-
let cap_id = pci_cap.cap.id;
609-
error!(
610-
"Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!"
607+
#[cfg(all(feature = "virtio-net", target_arch = "x86_64"))]
608+
PciCapability::MsiX(mut msix_capability) => {
609+
// We prefer legacy interrupts
610+
if device.get_irq().is_some() {
611+
continue;
612+
}
613+
msix_capability.set_enabled(true, device.access());
614+
let (base_addr, _) = device
615+
.memory_map_bar(msix_capability.table_bar(), true)
616+
.unwrap();
617+
let table_ptr = NonNull::slice_from_raw_parts(
618+
NonNull::with_exposed_provenance(
619+
core::num::NonZero::new(
620+
base_addr.as_usize()
621+
+ usize::try_from(msix_capability.table_offset()).unwrap(),
622+
)
623+
.unwrap(),
624+
),
625+
msix_capability.table_size().into(),
611626
);
627+
msix_table = Some(unsafe { VolatileRef::new(table_ptr) });
612628
}
613-
CapCfgType::Device => dev_cfg_list.push(pci_cap),
614-
615-
// PCI's configuration space is allowed to hold other structures, which are not virtio specific and are therefore ignored
629+
// PCI's configuration space is allowed to hold other structures, which are not useful for us and are therefore ignored
616630
// in the following
617631
_ => continue,
618632
}
@@ -623,6 +637,8 @@ pub(crate) fn map_caps(device: &PciDevice<PciConfigRegion>) -> Result<UniCapsCol
623637
notif_cfg: notif_cfg.ok_or(VirtioError::NoNotifCfg(device_id))?,
624638
isr_cfg: isr_cfg.ok_or(VirtioError::NoIsrCfg(device_id))?,
625639
dev_cfg_list,
640+
#[cfg(all(feature = "virtio-net", target_arch = "x86_64"))]
641+
msix_table,
626642
})
627643
}
628644

@@ -690,9 +706,10 @@ pub(crate) fn init_device(
690706
))]
691707
virtio::Id::Net => match VirtioNetDriver::init(device) {
692708
Ok(virt_net_drv) => {
709+
use crate::drivers::Driver;
693710
info!("Virtio network driver initialized.");
694711

695-
let irq = device.get_irq().unwrap();
712+
let irq = virt_net_drv.get_interrupt_number();
696713
crate::arch::interrupts::add_irq_name(irq, "virtio");
697714
info!("Virtio interrupt handler at line {irq}");
698715

0 commit comments

Comments
 (0)