From f4bf447f4cf44e3a3159346b482063821b1fc2bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=87a=C4=9Fatay=20Yi=C4=9Fit=20=C5=9Eahin?= Date: Wed, 22 Jul 2026 19:44:38 +0200 Subject: [PATCH] fix(virtio/pci): map BARs only once It is possible for multiple capabilities to be accessed through the same base address, so create the mappings for the BAR slots at the beginning and use saved virtual addresses. --- src/drivers/net/rtl8139.rs | 24 ++++---- src/drivers/pci.rs | 88 ++++++++++++++++------------- src/drivers/virtio/transport/pci.rs | 13 +++-- 3 files changed, 69 insertions(+), 56 deletions(-) diff --git a/src/drivers/net/rtl8139.rs b/src/drivers/net/rtl8139.rs index 482d295fd2..b8c4325064 100644 --- a/src/drivers/net/rtl8139.rs +++ b/src/drivers/net/rtl8139.rs @@ -9,7 +9,7 @@ use core::mem::ManuallyDrop; use core::ptr::NonNull; use endian_num::{le16, le32, le64}; -use pci_types::{Bar, CommandRegister, MAX_BARS}; +use pci_types::CommandRegister; use smoltcp::phy::DeviceCapabilities; use thiserror::Error; use volatile::access::{NoAccess, ReadOnly, ReadWrite}; @@ -746,19 +746,15 @@ pub(crate) fn init_device( handlers: &mut InterruptHandlerMap, ) -> Result { let irq = device.get_irq().unwrap(); - let mut regs = None; - - for i in 0..MAX_BARS { - let Some(Bar::Memory32 { .. }) = device.get_bar(i.try_into().unwrap()) else { - continue; - }; - - let (addr, _size) = device.memory_map_bar(i.try_into().unwrap(), true).unwrap(); - - regs = Some(unsafe { VolatileRef::new(NonNull::new(addr.as_mut_ptr()).unwrap()) }); - } - - let mut regs = regs.ok_or(DriverError::InitRTL8139DevFail(RTL8139Error::Unknown))?; + let mut regs = device + .memory_map_bars(true) + .into_iter() + .find_map(|bar| { + bar.map(|(addr, _size)| unsafe { + VolatileRef::new(NonNull::new(addr.as_mut_ptr()).unwrap()) + }) + }) + .ok_or(DriverError::InitRTL8139DevFail(RTL8139Error::Unknown))?; debug!("Found RTL8139 at IO {regs:?} (irq {irq})"); diff --git a/src/drivers/pci.rs b/src/drivers/pci.rs index a59aaacfe7..8e042a2c0f 100644 --- a/src/drivers/pci.rs +++ b/src/drivers/pci.rs @@ -111,53 +111,65 @@ impl PciDevice { } } - /// Memory maps pci bar with specified index to identical location in virtual memory. + /// Memory maps pci BARs to identical location in virtual memory. /// no_cache determines if we set the `Cache Disable` flag in the page-table-entry. - /// Returns (virtual-pointer, size) if successful, else None (if bar non-existent or IOSpace) - pub fn memory_map_bar(&self, index: u8, no_cache: bool) -> Option<(VirtAddr, usize)> { - let (address, size, prefetchable, _width) = match self.get_bar(index) { - Some(Bar::Io { .. }) => { - warn!("Cannot map IOBar!"); + /// Element at index is [Some] if the mapping of the BAR at the same index is successful, else [None] (if bar non-existent or IOSpace) + pub fn memory_map_bars(&self, no_cache: bool) -> [Option<(VirtAddr, usize)>; MAX_BARS] { + let mut should_skip = false; + core::array::from_fn(|index| { + if should_skip { + should_skip = false; return None; } - Some(Bar::Memory32 { - address, - size, - prefetchable, - }) => ( - u64::from(address), - usize::try_from(size).unwrap(), - prefetchable, - 32, - ), - Some(Bar::Memory64 { - address, - size, - prefetchable, - }) => (address, usize::try_from(size).unwrap(), prefetchable, 64), - _ => { + + let index = u8::try_from(index).unwrap(); + let (address, size, prefetchable, _width) = match self.get_bar(index) { + Some(Bar::Io { .. }) => { + warn!("Cannot map IOBar!"); + return None; + } + Some(Bar::Memory32 { + address, + size, + prefetchable, + }) => ( + u64::from(address), + usize::try_from(size).unwrap(), + prefetchable, + 32, + ), + Some(Bar::Memory64 { + address, + size, + prefetchable, + }) => { + should_skip = true; + (address, usize::try_from(size).unwrap(), prefetchable, 64) + } + _ => { + return None; + } + }; + + if address == 0 { return None; } - }; - if address == 0 { - return None; - } - - debug!("Mapping bar {index} at {address:#x} with length {size:#x}"); + debug!("Mapping bar {index} at {address:#x} with length {size:#x}"); - if !prefetchable { - warn!("Currently only mapping of prefetchable bars is supported!"); - } + if !prefetchable { + warn!("Currently only mapping of prefetchable bars is supported!"); + } - // Since the bios/bootloader manages the physical address space, the address got from the bar is unique and not overlapping. - // We therefore do not need to reserve any additional memory in our kernel. - // Map bar into RW^X virtual memory - let physical_address = address; - let virtual_address = - crate::mm::map(PhysAddr::new(physical_address), size, true, true, no_cache); + // Since the bios/bootloader manages the physical address space, the address got from the bar is unique and not overlapping. + // We therefore do not need to reserve any additional memory in our kernel. + // Map bar into RW^X virtual memory + let physical_address = address; + let virtual_address = + crate::mm::map(PhysAddr::new(physical_address), size, true, true, no_cache); - Some((virtual_address, size)) + Some((virtual_address, size)) + }) } pub fn get_irq(&self) -> Option { diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 3bab3f09a6..9898500305 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -581,6 +581,8 @@ pub(crate) fn map_caps( #[cfg(target_arch = "x86_64")] let mut msix_table = None; + let bar_mappings = device.memory_map_bars(true); + // Reads all PCI capabilities, starting at the capabilities list pointer from the // PCI device. // @@ -594,7 +596,7 @@ pub(crate) fn map_caps( continue; } let slot = cap.bar; - let Some((addr, size)) = device.memory_map_bar(slot, true) else { + let Some((addr, size)) = bar_mappings[usize::from(slot)] else { continue; }; let Some(pci_cap) = PciCap::new( @@ -661,9 +663,12 @@ pub(crate) fn map_caps( #[cfg(target_arch = "x86_64")] PciCapability::MsiX(mut msix_capability) => { msix_capability.set_enabled(true, device.access()); - let (base_addr, _) = device - .memory_map_bar(msix_capability.table_bar(), true) - .unwrap(); + + // the capability should provide a valid BAR ID and "[t]he BAR [...] must map + // Memory Space" (PCIe spec. 6.0 sec. 7.7.2) for the MSI-X capability + let (base_addr, _) = + bar_mappings[usize::from(msix_capability.table_bar())].unwrap(); + let table_ptr = NonNull::slice_from_raw_parts( NonNull::with_exposed_provenance( core::num::NonZero::new(