Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/drivers/net/virtio/mmio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl VirtioNetDriver<Uninit> {
notif_cfg,
inner: Uninit,
num_vqs: 0,
irq,
irq: Some(irq),
checksums: ChecksumCapabilities::default(),
})
}
Expand Down
30 changes: 28 additions & 2 deletions src/drivers/net/virtio/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion src/drivers/net/virtio/pci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this not be an error instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the section 7.5.1.1.13 of the PCIe specification version 6.0, "A [Interrupt Pin Register] value of 00h indicates that the Function uses no legacy interrupt Message(s)," so a device may be well-behaving and still not have an interrupt pin and line, in which case we may still be able to use MSI-X. There are reserved and unknown values and values that indicate no connection to the interrupt controller but for get_irq we do not differentiate between these and return None just like we do for the no legacy interrupt message(s) case.

}

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(),
})
}
Expand Down
32 changes: 32 additions & 0 deletions src/drivers/pci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,38 @@ pub(crate) fn init() {
});
}

/// MSI-X Table entry.
#[repr(C)]
pub(crate) struct MsixTableEntry {
/// Message Address
message_address: u32,

/// Message Upper Address
message_upper_address: u32,

/// Message Data
message_data: u32,

/// Vector Control
vector_control: u32,
}
Comment on lines +541 to +554
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to upstream this to pci_types?

That crate currently does not use any volatile operations at all, though. Is volatile necessary here? Should pci_types use volatile for some current operations? Or does this live in the PCI config region and should be accessed via pci_types::ConfigRegionAccess instead?


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...
Expand Down
195 changes: 116 additions & 79 deletions src/drivers/virtio/transport/pci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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();

Expand All @@ -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,
}
Expand All @@ -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,
})
}

Expand Down Expand Up @@ -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}");

Expand Down
Loading