From 65fb5826adfddb255b24555826dc95d877337827 Mon Sep 17 00:00:00 2001 From: Louis Vialar Date: Fri, 25 Apr 2025 12:02:19 +0200 Subject: [PATCH 1/4] x86_64/pci: enable support for PCIe enhanced configuration --- src/arch/x86_64/kernel/acpi.rs | 16 +++ src/arch/x86_64/kernel/mod.rs | 8 +- src/arch/x86_64/kernel/pci.rs | 148 +++++++++++++++++++++++++++- src/drivers/fs/virtio_pci.rs | 6 +- src/drivers/net/rtl8139.rs | 4 +- src/drivers/net/virtio/pci.rs | 6 +- src/drivers/pci.rs | 4 +- src/drivers/virtio/transport/pci.rs | 8 +- src/drivers/vsock/pci.rs | 6 +- 9 files changed, 180 insertions(+), 26 deletions(-) diff --git a/src/arch/x86_64/kernel/acpi.rs b/src/arch/x86_64/kernel/acpi.rs index 264815846d..3c27ea4083 100644 --- a/src/arch/x86_64/kernel/acpi.rs +++ b/src/arch/x86_64/kernel/acpi.rs @@ -45,6 +45,11 @@ const SLP_EN: u16 = 1 << 13; /// The "Multiple APIC Description Table" (MADT) preserved for get_apic_table(). static MADT: OnceCell> = OnceCell::new(); + +/// The MCFG table, to address PCI-E configuration space +#[cfg(feature = "pci")] +static MCFG: OnceCell> = OnceCell::new(); + /// The PM1A Control I/O Port for powering off the computer through ACPI. static PM1A_CNT_BLK: OnceCell> = OnceCell::new(); /// The Sleeping State Type code for powering off the computer through ACPI. @@ -487,6 +492,11 @@ pub fn get_madt() -> Option<&'static AcpiTable<'static>> { MADT.get() } +#[cfg(feature = "pci")] +pub fn get_mcfg_table() -> Option<&'static AcpiTable<'static>> { + MCFG.get() +} + pub fn poweroff() { if let (Some(mut pm1a_cnt_blk), Some(&slp_typa)) = (PM1A_CNT_BLK.get().cloned(), SLP_TYPA.get()) { @@ -562,6 +572,12 @@ pub fn init() { "SSDT at {table_physical_address:p} has invalid checksum" ); parse_ssdt(table); + } else if cfg!(feature = "pci") && table.header.signature() == "MCFG" { + assert!( + verify_checksum(table.header_start_address(), table.header.length as usize).is_ok(), + "MCFG at {table_physical_address:p} has invalid checksum" + ); + MCFG.set(table).unwrap(); } } } diff --git a/src/arch/x86_64/kernel/mod.rs b/src/arch/x86_64/kernel/mod.rs index 6d602830f8..117a5216d6 100644 --- a/src/arch/x86_64/kernel/mod.rs +++ b/src/arch/x86_64/kernel/mod.rs @@ -116,14 +116,14 @@ pub fn boot_processor_init() { interrupts::install(); systemtime::init(); - if is_uhyve_with_pci() || !is_uhyve() { - #[cfg(feature = "pci")] - pci::init(); - } if !env::is_uhyve() { #[cfg(feature = "acpi")] acpi::init(); } + if is_uhyve_with_pci() || !is_uhyve() { + #[cfg(feature = "pci")] + pci::init(); + } apic::init(); scheduler::install_timer_handler(); diff --git a/src/arch/x86_64/kernel/pci.rs b/src/arch/x86_64/kernel/pci.rs index 9bc8d57c0a..98f93a0ac9 100644 --- a/src/arch/x86_64/kernel/pci.rs +++ b/src/arch/x86_64/kernel/pci.rs @@ -12,7 +12,7 @@ const CONFIG_ADDRESS: Port = Port::new(0xcf8); const CONFIG_DATA: Port = Port::new(0xcfc); #[derive(Debug, Copy, Clone)] -pub(crate) struct PciConfigRegion; +struct PciConfigRegion; impl PciConfigRegion { pub const fn new() -> Self { @@ -20,6 +20,29 @@ impl PciConfigRegion { } } +#[derive(Debug, Copy, Clone)] +pub enum PciConfigAccess { + PciConfigRegion(PciConfigRegion), + #[cfg(feature = "acpi")] + PcieConfigRegion(pcie::McfgTableEntry), +} + +impl ConfigRegionAccess for PciConfigAccess { + unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { + match self { + PciConfigAccess::PciConfigRegion(entry) => entry.read(address, offset), + PciConfigAccess::PcieConfigRegion(entry) => entry.read(address, offset), + } + } + + unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { + match self { + PciConfigAccess::PciConfigRegion(entry) => entry.write(address, offset, value), + PciConfigAccess::PcieConfigRegion(entry) => entry.write(address, offset, value), + } + } +} + impl ConfigRegionAccess for PciConfigRegion { #[inline] unsafe fn read(&self, pci_addr: PciAddress, register: u16) -> u32 { @@ -59,20 +82,135 @@ impl ConfigRegionAccess for PciConfigRegion { pub(crate) fn init() { debug!("Scanning PCI Busses 0 to {}", PCI_MAX_BUS_NUMBER - 1); + #[cfg(feature = "acpi")] + if pcie::init_pcie() { return; } + + enumerate_devices(0, PCI_MAX_BUS_NUMBER, PciConfigAccess::PciConfigRegion(PciConfigRegion::new())) +} + +fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigAccess) { // Hermit only uses PCI for network devices. // Therefore, multifunction devices as well as additional bridges are not scanned. // We also limit scanning to the first 32 buses. - let pci_config = PciConfigRegion::new(); - for bus in 0..PCI_MAX_BUS_NUMBER { + for bus in bus_start..bus_end { for device in 0..PCI_MAX_DEVICE_NUMBER { let pci_address = PciAddress::new(0, bus, device, 0); let header = PciHeader::new(pci_address); - let (device_id, vendor_id) = header.id(pci_config); + let (device_id, vendor_id) = header.id(access); if device_id != u16::MAX && vendor_id != u16::MAX { - let device = PciDevice::new(pci_address, pci_config); + let device = PciDevice::new(pci_address, access); PCI_DEVICES.with(|pci_devices| pci_devices.unwrap().push(device)); } } } } + +#[cfg(feature = "acpi")] +mod pcie { + use core::ptr; + use pci_types::{ConfigRegionAccess, PciAddress}; + use memory_addresses::{PhysAddr, VirtAddr}; + use super::{PciConfigAccess, PCI_MAX_BUS_NUMBER}; + use crate::env; + use crate::env::kernel::acpi; + + pub fn init_pcie() -> bool { + let Some(table) = acpi::get_mcfg_table() else { return false; }; + + let mut start_addr: *const McfgTableEntry = core::ptr::with_exposed_provenance(table.table_start_address() + 8); + let end_addr: *const McfgTableEntry = core::ptr::with_exposed_provenance(table.table_end_address() + 8); + + if start_addr == end_addr { + return false; + } + + while start_addr < end_addr { + unsafe { + let read = ptr::read_unaligned(start_addr); + init_pcie_bus(read); + start_addr = start_addr.add(1); + } + } + + true + } + + #[derive(Debug)] + #[repr(C)] + struct PcieDeviceConfig { + vendor_id: u16, + device_id: u16, + _reserved: [u8; 4096 - 8] + } + + #[derive(Debug, Copy, Clone)] + #[repr(C)] + pub(crate) struct McfgTableEntry { + pub base_address: u64, + pub pci_segment_number: u16, + pub start_pci_bus: u8, + pub end_pci_bus: u8, + _reserved: u32 + } + + impl McfgTableEntry { + pub fn pci_config_space_address(&self, bus_number: u8, device: u8, function: u8) -> PhysAddr { + PhysAddr::new( + self.base_address + + ((bus_number as u64) << 20) | + (((device as u64) & 0x1f) << 15) | + (((function as u64) & 0x7) << 12) + ) + } + } + + #[derive(Debug)] + #[cfg(feature = "pci")] + struct McfgTable(alloc::vec::Vec); + + impl PcieDeviceConfig { + fn get<'a>(physical_address: PhysAddr) -> &'a Self { + assert!(env::is_uefi()); + + // For UEFI Systems, the tables are already mapped so we only need to return a proper reference to the table + let allocated_virtual_address = VirtAddr::new(physical_address.as_u64()); + let ptr: *const PcieDeviceConfig = allocated_virtual_address.as_ptr(); + + unsafe { ptr.as_ref().unwrap() } + } + } + + impl ConfigRegionAccess for McfgTableEntry { + unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { + assert_eq!(address.segment(), self.pci_segment_number); + assert!(address.bus() >= self.start_pci_bus); + assert!(address.bus() <= self.end_pci_bus); + + let ptr = self.pci_config_space_address(address.bus(), address.device(), address.function()) + offset as u64; + let ptr = ptr.as_usize() as *const u32; + + unsafe { *ptr } + } + + unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { + assert_eq!(address.segment(), self.pci_segment_number); + assert!(address.bus() >= self.start_pci_bus); + assert!(address.bus() <= self.end_pci_bus); + + let ptr = self.pci_config_space_address(address.bus(), address.device(), address.function()) + offset as u64; + let ptr = ptr.as_usize() as *mut u32; + + unsafe { *ptr = value; } + } + } + + fn init_pcie_bus(bus_entry: McfgTableEntry) { + if bus_entry.start_pci_bus > PCI_MAX_BUS_NUMBER { + return; + } + + let end = if bus_entry.end_pci_bus > PCI_MAX_BUS_NUMBER { PCI_MAX_BUS_NUMBER } else { bus_entry.end_pci_bus }; + super::enumerate_devices(bus_entry.start_pci_bus, end, PciConfigAccess::PcieConfigRegion(bus_entry)); + } +} \ No newline at end of file diff --git a/src/drivers/fs/virtio_pci.rs b/src/drivers/fs/virtio_pci.rs index 86dc7c5689..9f07b06ffe 100644 --- a/src/drivers/fs/virtio_pci.rs +++ b/src/drivers/fs/virtio_pci.rs @@ -2,7 +2,7 @@ use alloc::vec::Vec; use volatile::VolatileRef; -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; use crate::drivers::fs::virtio_fs::{FsDevCfg, VirtioFsDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; @@ -26,7 +26,7 @@ impl VirtioFsDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -54,7 +54,7 @@ impl VirtioFsDriver { } /// Initializes virtio filesystem device - pub fn init(device: &PciDevice) -> Result { + pub fn init(device: &PciDevice) -> Result { let mut drv = match pci::map_caps(device) { Ok(caps) => match VirtioFsDriver::new(caps, device) { Ok(driver) => driver, diff --git a/src/drivers/net/rtl8139.rs b/src/drivers/net/rtl8139.rs index 1c2fc33bd2..e9ddeee5b4 100644 --- a/src/drivers/net/rtl8139.rs +++ b/src/drivers/net/rtl8139.rs @@ -10,7 +10,7 @@ use pci_types::{Bar, CommandRegister, InterruptLine, MAX_BARS}; use x86_64::instructions::port::Port; use crate::arch::kernel::interrupts::*; -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; use crate::drivers::Driver; use crate::drivers::error::DriverError; use crate::drivers::net::{NetworkDriver, mtu}; @@ -423,7 +423,7 @@ impl Drop for RTL8139Driver { } pub(crate) fn init_device( - device: &PciDevice, + device: &PciDevice, ) -> Result { let irq = device.get_irq().unwrap(); let mut iobase: Option = None; diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index d3b7282fb8..c5b284a5e6 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -7,7 +7,7 @@ use smoltcp::phy::ChecksumCapabilities; use volatile::VolatileRef; use super::{Init, Uninit}; -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; use crate::drivers::net::virtio::{NetDevCfg, VirtioNetDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; @@ -32,7 +32,7 @@ impl VirtioNetDriver { /// configuration structures and moving them into the struct. pub(crate) fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); let UniCapsColl { @@ -69,7 +69,7 @@ impl VirtioNetDriver { /// Returns a driver instance of /// [VirtioNetDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub(crate) fn init( - device: &PciDevice, + device: &PciDevice, ) -> Result, VirtioError> { // enable bus master mode device.set_command(CommandRegister::BUS_MASTER_ENABLE); diff --git a/src/drivers/pci.rs b/src/drivers/pci.rs index 3174564c5e..4e635df05e 100644 --- a/src/drivers/pci.rs +++ b/src/drivers/pci.rs @@ -22,7 +22,7 @@ use pci_types::{ InterruptPin, MAX_BARS, PciAddress, PciHeader, StatusRegister, VendorId, }; -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; #[cfg(feature = "console")] use crate::console::IoDevice; #[cfg(feature = "console")] @@ -64,7 +64,7 @@ use crate::drivers::vsock::VirtioVsockDriver; use crate::drivers::{Driver, InterruptHandlerQueue}; use crate::init_cell::InitCell; -pub(crate) static PCI_DEVICES: InitCell>> = +pub(crate) static PCI_DEVICES: InitCell>> = InitCell::new(Vec::new()); static PCI_DRIVERS: InitCell> = InitCell::new(Vec::new()); diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index b4fdc5cec7..1817c031c3 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -20,7 +20,7 @@ use volatile::access::ReadOnly; use volatile::{VolatilePtr, VolatileRef}; use crate::arch::memory_barrier; -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; #[cfg(feature = "console")] use crate::drivers::console::VirtioConsoleDriver; use crate::drivers::error::DriverError; @@ -677,7 +677,7 @@ impl PciBar { /// /// 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) -> Result, PciError> { +fn read_caps(device: &PciDevice) -> Result, PciError> { let device_id = device.device_id(); let capabilities = device @@ -708,7 +708,7 @@ fn read_caps(device: &PciDevice) -> Result, PciErro } } -pub(crate) fn map_caps(device: &PciDevice) -> Result { +pub(crate) fn map_caps(device: &PciDevice) -> Result { let device_id = device.device_id(); // In case caplist pointer is not used, abort as it is essential @@ -792,7 +792,7 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result`] reference, allowing access to the capabilities /// list of the given device through [map_caps]. pub(crate) fn init_device( - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index 7b2db01f2b..804d2ede12 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -1,4 +1,4 @@ -use crate::arch::pci::PciConfigRegion; +use crate::arch::pci::PciConfigAccess; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; use crate::drivers::virtio::transport::pci; @@ -31,7 +31,7 @@ impl VirtioVsockDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -64,7 +64,7 @@ impl VirtioVsockDriver { /// /// Returns a driver instance of VirtioVsockDriver. pub(crate) fn init( - device: &PciDevice, + device: &PciDevice, ) -> Result { let mut drv = match pci::map_caps(device) { Ok(caps) => match VirtioVsockDriver::new(caps, device) { From a0e91993c562ec0ad6900cc9168cd0e123aee491 Mon Sep 17 00:00:00 2001 From: Louis Vialar Date: Mon, 7 Jul 2025 16:03:31 +0200 Subject: [PATCH 2/4] x86_64/pci: reformat and fix clippy warnings --- src/arch/aarch64/kernel/pci.rs | 3 + src/arch/riscv64/kernel/pci.rs | 3 + src/arch/x86_64/kernel/acpi.rs | 1 + src/arch/x86_64/kernel/pci.rs | 112 ++++++++++++++++++--------------- 4 files changed, 68 insertions(+), 51 deletions(-) diff --git a/src/arch/aarch64/kernel/pci.rs b/src/arch/aarch64/kernel/pci.rs index cada7a447f..ba99e6dbab 100644 --- a/src/arch/aarch64/kernel/pci.rs +++ b/src/arch/aarch64/kernel/pci.rs @@ -23,6 +23,9 @@ const PCI_MAX_FUNCTION_NUMBER: u8 = 8; #[derive(Debug, Copy, Clone)] pub(crate) struct PciConfigRegion(VirtAddr); +// Compatibility with x86_64 +pub(crate) use PciConfigRegion as PciConfigAccess; + impl PciConfigRegion { pub const fn new(addr: VirtAddr) -> Self { assert!( diff --git a/src/arch/riscv64/kernel/pci.rs b/src/arch/riscv64/kernel/pci.rs index 8dd15a3711..5d1ec2a4fb 100644 --- a/src/arch/riscv64/kernel/pci.rs +++ b/src/arch/riscv64/kernel/pci.rs @@ -3,6 +3,9 @@ use pci_types::{ConfigRegionAccess, PciAddress}; #[derive(Debug, Copy, Clone)] pub struct PciConfigRegion; +// Compatibility with x86_64 +pub use PciConfigRegion as PciConfigAccess; + impl ConfigRegionAccess for PciConfigRegion { unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 { warn!("pci_config_region.read({addr}, {offset}) called but not implemented"); diff --git a/src/arch/x86_64/kernel/acpi.rs b/src/arch/x86_64/kernel/acpi.rs index 3c27ea4083..ec4523abbb 100644 --- a/src/arch/x86_64/kernel/acpi.rs +++ b/src/arch/x86_64/kernel/acpi.rs @@ -577,6 +577,7 @@ pub fn init() { verify_checksum(table.header_start_address(), table.header.length as usize).is_ok(), "MCFG at {table_physical_address:p} has invalid checksum" ); + #[cfg(feature = "pci")] MCFG.set(table).unwrap(); } } diff --git a/src/arch/x86_64/kernel/pci.rs b/src/arch/x86_64/kernel/pci.rs index 98f93a0ac9..b9061b39f7 100644 --- a/src/arch/x86_64/kernel/pci.rs +++ b/src/arch/x86_64/kernel/pci.rs @@ -12,7 +12,7 @@ const CONFIG_ADDRESS: Port = Port::new(0xcf8); const CONFIG_DATA: Port = Port::new(0xcfc); #[derive(Debug, Copy, Clone)] -struct PciConfigRegion; +pub struct PciConfigRegion; impl PciConfigRegion { pub const fn new() -> Self { @@ -30,15 +30,21 @@ pub enum PciConfigAccess { impl ConfigRegionAccess for PciConfigAccess { unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { match self { - PciConfigAccess::PciConfigRegion(entry) => entry.read(address, offset), - PciConfigAccess::PcieConfigRegion(entry) => entry.read(address, offset), + PciConfigAccess::PciConfigRegion(entry) => unsafe { entry.read(address, offset) }, + #[cfg(feature = "acpi")] + PciConfigAccess::PcieConfigRegion(entry) => unsafe { entry.read(address, offset) }, } } unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { match self { - PciConfigAccess::PciConfigRegion(entry) => entry.write(address, offset, value), - PciConfigAccess::PcieConfigRegion(entry) => entry.write(address, offset, value), + PciConfigAccess::PciConfigRegion(entry) => unsafe { + entry.write(address, offset, value); + }, + #[cfg(feature = "acpi")] + PciConfigAccess::PcieConfigRegion(entry) => unsafe { + entry.write(address, offset, value); + }, } } } @@ -83,9 +89,15 @@ pub(crate) fn init() { debug!("Scanning PCI Busses 0 to {}", PCI_MAX_BUS_NUMBER - 1); #[cfg(feature = "acpi")] - if pcie::init_pcie() { return; } + if pcie::init_pcie() { + return; + } - enumerate_devices(0, PCI_MAX_BUS_NUMBER, PciConfigAccess::PciConfigRegion(PciConfigRegion::new())) + enumerate_devices( + 0, + PCI_MAX_BUS_NUMBER, + PciConfigAccess::PciConfigRegion(PciConfigRegion::new()), + ); } fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigAccess) { @@ -108,18 +120,21 @@ fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigAccess) { #[cfg(feature = "acpi")] mod pcie { - use core::ptr; + use memory_addresses::PhysAddr; use pci_types::{ConfigRegionAccess, PciAddress}; - use memory_addresses::{PhysAddr, VirtAddr}; - use super::{PciConfigAccess, PCI_MAX_BUS_NUMBER}; - use crate::env; + + use super::{PCI_MAX_BUS_NUMBER, PciConfigAccess}; use crate::env::kernel::acpi; pub fn init_pcie() -> bool { - let Some(table) = acpi::get_mcfg_table() else { return false; }; + let Some(table) = acpi::get_mcfg_table() else { + return false; + }; - let mut start_addr: *const McfgTableEntry = core::ptr::with_exposed_provenance(table.table_start_address() + 8); - let end_addr: *const McfgTableEntry = core::ptr::with_exposed_provenance(table.table_end_address() + 8); + let mut start_addr: *const McfgTableEntry = + core::ptr::with_exposed_provenance(table.table_start_address() + 8); + let end_addr: *const McfgTableEntry = + core::ptr::with_exposed_provenance(table.table_end_address() + 8); if start_addr == end_addr { return false; @@ -127,7 +142,7 @@ mod pcie { while start_addr < end_addr { unsafe { - let read = ptr::read_unaligned(start_addr); + let read = core::ptr::read_unaligned(start_addr); init_pcie_bus(read); start_addr = start_addr.add(1); } @@ -136,14 +151,6 @@ mod pcie { true } - #[derive(Debug)] - #[repr(C)] - struct PcieDeviceConfig { - vendor_id: u16, - device_id: u16, - _reserved: [u8; 4096 - 8] - } - #[derive(Debug, Copy, Clone)] #[repr(C)] pub(crate) struct McfgTableEntry { @@ -151,43 +158,34 @@ mod pcie { pub pci_segment_number: u16, pub start_pci_bus: u8, pub end_pci_bus: u8, - _reserved: u32 + _reserved: u32, } impl McfgTableEntry { - pub fn pci_config_space_address(&self, bus_number: u8, device: u8, function: u8) -> PhysAddr { + pub fn pci_config_space_address( + &self, + bus_number: u8, + device: u8, + function: u8, + ) -> PhysAddr { PhysAddr::new( - self.base_address + - ((bus_number as u64) << 20) | - (((device as u64) & 0x1f) << 15) | - (((function as u64) & 0x7) << 12) + self.base_address + + ((u64::from(bus_number) << 20) + | ((u64::from(device) & 0x1f) << 15) + | ((u64::from(function) & 0x7) << 12)), ) } } - #[derive(Debug)] - #[cfg(feature = "pci")] - struct McfgTable(alloc::vec::Vec); - - impl PcieDeviceConfig { - fn get<'a>(physical_address: PhysAddr) -> &'a Self { - assert!(env::is_uefi()); - - // For UEFI Systems, the tables are already mapped so we only need to return a proper reference to the table - let allocated_virtual_address = VirtAddr::new(physical_address.as_u64()); - let ptr: *const PcieDeviceConfig = allocated_virtual_address.as_ptr(); - - unsafe { ptr.as_ref().unwrap() } - } - } - impl ConfigRegionAccess for McfgTableEntry { unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { assert_eq!(address.segment(), self.pci_segment_number); assert!(address.bus() >= self.start_pci_bus); assert!(address.bus() <= self.end_pci_bus); - let ptr = self.pci_config_space_address(address.bus(), address.device(), address.function()) + offset as u64; + let ptr = + self.pci_config_space_address(address.bus(), address.device(), address.function()) + + u64::from(offset); let ptr = ptr.as_usize() as *const u32; unsafe { *ptr } @@ -198,10 +196,14 @@ mod pcie { assert!(address.bus() >= self.start_pci_bus); assert!(address.bus() <= self.end_pci_bus); - let ptr = self.pci_config_space_address(address.bus(), address.device(), address.function()) + offset as u64; + let ptr = + self.pci_config_space_address(address.bus(), address.device(), address.function()) + + u64::from(offset); let ptr = ptr.as_usize() as *mut u32; - unsafe { *ptr = value; } + unsafe { + *ptr = value; + } } } @@ -210,7 +212,15 @@ mod pcie { return; } - let end = if bus_entry.end_pci_bus > PCI_MAX_BUS_NUMBER { PCI_MAX_BUS_NUMBER } else { bus_entry.end_pci_bus }; - super::enumerate_devices(bus_entry.start_pci_bus, end, PciConfigAccess::PcieConfigRegion(bus_entry)); + let end = if bus_entry.end_pci_bus > PCI_MAX_BUS_NUMBER { + PCI_MAX_BUS_NUMBER + } else { + bus_entry.end_pci_bus + }; + super::enumerate_devices( + bus_entry.start_pci_bus, + end, + PciConfigAccess::PcieConfigRegion(bus_entry), + ); } -} \ No newline at end of file +} From 298c7e866825d5e18b0c1228fcbdb1998b99a5ca Mon Sep 17 00:00:00 2001 From: Louis Vialar Date: Tue, 8 Jul 2025 17:14:49 +0200 Subject: [PATCH 3/4] pcie: refrain from renaming PciConfigRegion --- src/arch/aarch64/kernel/pci.rs | 3 -- src/arch/riscv64/kernel/pci.rs | 3 -- src/arch/x86_64/kernel/pci.rs | 45 +++++++++++++++-------------- src/drivers/fs/virtio_pci.rs | 6 ++-- src/drivers/net/rtl8139.rs | 4 +-- src/drivers/net/virtio/pci.rs | 6 ++-- src/drivers/pci.rs | 4 +-- src/drivers/virtio/transport/pci.rs | 8 ++--- src/drivers/vsock/pci.rs | 6 ++-- 9 files changed, 40 insertions(+), 45 deletions(-) diff --git a/src/arch/aarch64/kernel/pci.rs b/src/arch/aarch64/kernel/pci.rs index ba99e6dbab..cada7a447f 100644 --- a/src/arch/aarch64/kernel/pci.rs +++ b/src/arch/aarch64/kernel/pci.rs @@ -23,9 +23,6 @@ const PCI_MAX_FUNCTION_NUMBER: u8 = 8; #[derive(Debug, Copy, Clone)] pub(crate) struct PciConfigRegion(VirtAddr); -// Compatibility with x86_64 -pub(crate) use PciConfigRegion as PciConfigAccess; - impl PciConfigRegion { pub const fn new(addr: VirtAddr) -> Self { assert!( diff --git a/src/arch/riscv64/kernel/pci.rs b/src/arch/riscv64/kernel/pci.rs index 5d1ec2a4fb..8dd15a3711 100644 --- a/src/arch/riscv64/kernel/pci.rs +++ b/src/arch/riscv64/kernel/pci.rs @@ -3,9 +3,6 @@ use pci_types::{ConfigRegionAccess, PciAddress}; #[derive(Debug, Copy, Clone)] pub struct PciConfigRegion; -// Compatibility with x86_64 -pub use PciConfigRegion as PciConfigAccess; - impl ConfigRegionAccess for PciConfigRegion { unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 { warn!("pci_config_region.read({addr}, {offset}) called but not implemented"); diff --git a/src/arch/x86_64/kernel/pci.rs b/src/arch/x86_64/kernel/pci.rs index b9061b39f7..9e4b3e0791 100644 --- a/src/arch/x86_64/kernel/pci.rs +++ b/src/arch/x86_64/kernel/pci.rs @@ -11,45 +11,46 @@ const PCI_CONFIG_ADDRESS_ENABLE: u32 = 1 << 31; const CONFIG_ADDRESS: Port = Port::new(0xcf8); const CONFIG_DATA: Port = Port::new(0xcfc); +#[allow(clippy::upper_case_acronyms)] #[derive(Debug, Copy, Clone)] -pub struct PciConfigRegion; - -impl PciConfigRegion { - pub const fn new() -> Self { - Self {} - } -} - -#[derive(Debug, Copy, Clone)] -pub enum PciConfigAccess { - PciConfigRegion(PciConfigRegion), +pub enum PciConfigRegion { + PCI(LegacyPciConfigRegion), #[cfg(feature = "acpi")] - PcieConfigRegion(pcie::McfgTableEntry), + PCIe(pcie::McfgTableEntry), } -impl ConfigRegionAccess for PciConfigAccess { +impl ConfigRegionAccess for PciConfigRegion { unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { match self { - PciConfigAccess::PciConfigRegion(entry) => unsafe { entry.read(address, offset) }, + PciConfigRegion::PCI(entry) => unsafe { entry.read(address, offset) }, #[cfg(feature = "acpi")] - PciConfigAccess::PcieConfigRegion(entry) => unsafe { entry.read(address, offset) }, + PciConfigRegion::PCIe(entry) => unsafe { entry.read(address, offset) }, } } unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) { match self { - PciConfigAccess::PciConfigRegion(entry) => unsafe { + PciConfigRegion::PCI(entry) => unsafe { entry.write(address, offset, value); }, #[cfg(feature = "acpi")] - PciConfigAccess::PcieConfigRegion(entry) => unsafe { + PciConfigRegion::PCIe(entry) => unsafe { entry.write(address, offset, value); }, } } } -impl ConfigRegionAccess for PciConfigRegion { +#[derive(Debug, Copy, Clone)] +pub(crate) struct LegacyPciConfigRegion; + +impl LegacyPciConfigRegion { + pub const fn new() -> Self { + Self {} + } +} + +impl ConfigRegionAccess for LegacyPciConfigRegion { #[inline] unsafe fn read(&self, pci_addr: PciAddress, register: u16) -> u32 { let mut config_address = CONFIG_ADDRESS; @@ -96,11 +97,11 @@ pub(crate) fn init() { enumerate_devices( 0, PCI_MAX_BUS_NUMBER, - PciConfigAccess::PciConfigRegion(PciConfigRegion::new()), + PciConfigRegion::PCI(LegacyPciConfigRegion::new()), ); } -fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigAccess) { +fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigRegion) { // Hermit only uses PCI for network devices. // Therefore, multifunction devices as well as additional bridges are not scanned. // We also limit scanning to the first 32 buses. @@ -123,7 +124,7 @@ mod pcie { use memory_addresses::PhysAddr; use pci_types::{ConfigRegionAccess, PciAddress}; - use super::{PCI_MAX_BUS_NUMBER, PciConfigAccess}; + use super::{PCI_MAX_BUS_NUMBER, PciConfigRegion}; use crate::env::kernel::acpi; pub fn init_pcie() -> bool { @@ -220,7 +221,7 @@ mod pcie { super::enumerate_devices( bus_entry.start_pci_bus, end, - PciConfigAccess::PcieConfigRegion(bus_entry), + PciConfigRegion::PCIe(bus_entry), ); } } diff --git a/src/drivers/fs/virtio_pci.rs b/src/drivers/fs/virtio_pci.rs index 9f07b06ffe..86dc7c5689 100644 --- a/src/drivers/fs/virtio_pci.rs +++ b/src/drivers/fs/virtio_pci.rs @@ -2,7 +2,7 @@ use alloc::vec::Vec; use volatile::VolatileRef; -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; use crate::drivers::fs::virtio_fs::{FsDevCfg, VirtioFsDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; @@ -26,7 +26,7 @@ impl VirtioFsDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -54,7 +54,7 @@ impl VirtioFsDriver { } /// Initializes virtio filesystem device - pub fn init(device: &PciDevice) -> Result { + pub fn init(device: &PciDevice) -> Result { let mut drv = match pci::map_caps(device) { Ok(caps) => match VirtioFsDriver::new(caps, device) { Ok(driver) => driver, diff --git a/src/drivers/net/rtl8139.rs b/src/drivers/net/rtl8139.rs index e9ddeee5b4..1c2fc33bd2 100644 --- a/src/drivers/net/rtl8139.rs +++ b/src/drivers/net/rtl8139.rs @@ -10,7 +10,7 @@ use pci_types::{Bar, CommandRegister, InterruptLine, MAX_BARS}; use x86_64::instructions::port::Port; use crate::arch::kernel::interrupts::*; -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; use crate::drivers::Driver; use crate::drivers::error::DriverError; use crate::drivers::net::{NetworkDriver, mtu}; @@ -423,7 +423,7 @@ impl Drop for RTL8139Driver { } pub(crate) fn init_device( - device: &PciDevice, + device: &PciDevice, ) -> Result { let irq = device.get_irq().unwrap(); let mut iobase: Option = None; diff --git a/src/drivers/net/virtio/pci.rs b/src/drivers/net/virtio/pci.rs index c5b284a5e6..d3b7282fb8 100644 --- a/src/drivers/net/virtio/pci.rs +++ b/src/drivers/net/virtio/pci.rs @@ -7,7 +7,7 @@ use smoltcp::phy::ChecksumCapabilities; use volatile::VolatileRef; use super::{Init, Uninit}; -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; use crate::drivers::net::virtio::{NetDevCfg, VirtioNetDriver}; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; @@ -32,7 +32,7 @@ impl VirtioNetDriver { /// configuration structures and moving them into the struct. pub(crate) fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); let UniCapsColl { @@ -69,7 +69,7 @@ impl VirtioNetDriver { /// Returns a driver instance of /// [VirtioNetDriver](structs.virtionetdriver.html) or an [VirtioError](enums.virtioerror.html). pub(crate) fn init( - device: &PciDevice, + device: &PciDevice, ) -> Result, VirtioError> { // enable bus master mode device.set_command(CommandRegister::BUS_MASTER_ENABLE); diff --git a/src/drivers/pci.rs b/src/drivers/pci.rs index 4e635df05e..3174564c5e 100644 --- a/src/drivers/pci.rs +++ b/src/drivers/pci.rs @@ -22,7 +22,7 @@ use pci_types::{ InterruptPin, MAX_BARS, PciAddress, PciHeader, StatusRegister, VendorId, }; -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; #[cfg(feature = "console")] use crate::console::IoDevice; #[cfg(feature = "console")] @@ -64,7 +64,7 @@ use crate::drivers::vsock::VirtioVsockDriver; use crate::drivers::{Driver, InterruptHandlerQueue}; use crate::init_cell::InitCell; -pub(crate) static PCI_DEVICES: InitCell>> = +pub(crate) static PCI_DEVICES: InitCell>> = InitCell::new(Vec::new()); static PCI_DRIVERS: InitCell> = InitCell::new(Vec::new()); diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 1817c031c3..b4fdc5cec7 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -20,7 +20,7 @@ use volatile::access::ReadOnly; use volatile::{VolatilePtr, VolatileRef}; use crate::arch::memory_barrier; -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; #[cfg(feature = "console")] use crate::drivers::console::VirtioConsoleDriver; use crate::drivers::error::DriverError; @@ -677,7 +677,7 @@ impl PciBar { /// /// 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) -> Result, PciError> { +fn read_caps(device: &PciDevice) -> Result, PciError> { let device_id = device.device_id(); let capabilities = device @@ -708,7 +708,7 @@ fn read_caps(device: &PciDevice) -> Result, PciErro } } -pub(crate) fn map_caps(device: &PciDevice) -> Result { +pub(crate) fn map_caps(device: &PciDevice) -> Result { let device_id = device.device_id(); // In case caplist pointer is not used, abort as it is essential @@ -792,7 +792,7 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result`] reference, allowing access to the capabilities /// list of the given device through [map_caps]. pub(crate) fn init_device( - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); diff --git a/src/drivers/vsock/pci.rs b/src/drivers/vsock/pci.rs index 804d2ede12..7b2db01f2b 100644 --- a/src/drivers/vsock/pci.rs +++ b/src/drivers/vsock/pci.rs @@ -1,4 +1,4 @@ -use crate::arch::pci::PciConfigAccess; +use crate::arch::pci::PciConfigRegion; use crate::drivers::pci::PciDevice; use crate::drivers::virtio::error::{self, VirtioError}; use crate::drivers::virtio::transport::pci; @@ -31,7 +31,7 @@ impl VirtioVsockDriver { /// configuration structures and moving them into the struct. pub fn new( caps_coll: UniCapsColl, - device: &PciDevice, + device: &PciDevice, ) -> Result { let device_id = device.device_id(); @@ -64,7 +64,7 @@ impl VirtioVsockDriver { /// /// Returns a driver instance of VirtioVsockDriver. pub(crate) fn init( - device: &PciDevice, + device: &PciDevice, ) -> Result { let mut drv = match pci::map_caps(device) { Ok(caps) => match VirtioVsockDriver::new(caps, device) { From 398eb1b2edd484d7b00401922d2e90e3a7885e56 Mon Sep 17 00:00:00 2001 From: Louis Vialar Date: Mon, 28 Jul 2025 12:03:09 +0200 Subject: [PATCH 4/4] feat(PCIe): add some debugging messages --- src/arch/x86_64/kernel/pci.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/arch/x86_64/kernel/pci.rs b/src/arch/x86_64/kernel/pci.rs index 9e4b3e0791..e79b602df5 100644 --- a/src/arch/x86_64/kernel/pci.rs +++ b/src/arch/x86_64/kernel/pci.rs @@ -102,6 +102,10 @@ pub(crate) fn init() { } fn enumerate_devices(bus_start: u8, bus_end: u8, access: PciConfigRegion) { + debug!( + "Enumerating PCI devices on buses {bus_start}:{bus_end} using configuration accessor {access:?}" + ); + // Hermit only uses PCI for network devices. // Therefore, multifunction devices as well as additional bridges are not scanned. // We also limit scanning to the first 32 buses. @@ -141,6 +145,13 @@ mod pcie { return false; } + debug!( + "Found MCFG ACPI table at {:p}:{:p} (should contain {} entries)", + start_addr, + end_addr, + unsafe { end_addr.offset_from(start_addr) } + ); + while start_addr < end_addr { unsafe { let read = core::ptr::read_unaligned(start_addr); @@ -210,6 +221,10 @@ mod pcie { fn init_pcie_bus(bus_entry: McfgTableEntry) { if bus_entry.start_pci_bus > PCI_MAX_BUS_NUMBER { + debug!( + "Skipping PCI bus {}: is higher than maximum number allowed ({PCI_MAX_BUS_NUMBER})", + bus_entry.start_pci_bus + ); return; } @@ -218,6 +233,7 @@ mod pcie { } else { bus_entry.end_pci_bus }; + super::enumerate_devices( bus_entry.start_pci_bus, end,