--- a/drivers/pcid/src/cfg_access/mod.rs +++ b/drivers/pcid/src/cfg_access/mod.rs @@ -152,20 +152,22 @@ fn with( f: impl FnOnce(PcieAllocs<'_>, Vec, [u32; 4]) -> io::Result, ) -> io::Result { - let table_dir = fs::read_dir("/scheme/acpi/tables")?; + let table_dir = match fs::read_dir("/scheme/acpi/tables") { + Ok(dir) => dir, + Err(e) => { + log::debug!("pcid: cannot read /scheme/acpi/tables: {e} (acpid running?)"); + return Err(e); + } + }; - // TODO: validate/print MCFG? + let mut found_tables: Vec = Vec::new(); for table_direntry in table_dir { let table_path = table_direntry?.path(); - // Every directory entry has to have a filename unless - // the filesystem (or in this case acpid) misbehaves. - // If it misbehaves we have worse problems than pcid - // crashing. `as_encoded_bytes()` returns some superset - // of ASCII, so directly comparing it with an ASCII name - // is fine. let table_filename = table_path.file_name().unwrap().as_encoded_bytes(); + found_tables.push(String::from_utf8_lossy(table_filename).into_owned()); + if table_filename.get(0..4) == Some(&MCFG_NAME) { let bytes = fs::read(table_path)?.into_boxed_slice(); match Mcfg::parse(&*bytes) { @@ -174,6 +176,7 @@ return f(allocs, Vec::new(), [u32::MAX, u32::MAX, u32::MAX, u32::MAX]); } None => { + log::warn!("pcid: MCFG table found but failed to parse ({} bytes)", bytes.len()); return Err(io::Error::new( io::ErrorKind::InvalidData, "couldn't find mcfg table", @@ -183,6 +186,12 @@ } } + log::debug!( + "pcid: MCFG not found among {} ACPI table(s): {:?}", + found_tables.len(), + found_tables + ); + Err(io::Error::new( io::ErrorKind::NotFound, "couldn't find mcfg table", @@ -219,6 +228,24 @@ pub interrupt_map_mask: [u32; 4], fallback: Pci, } + +/// Validate an MCFG allocation entry address (cross-referenced with Linux +/// `acpi_mcfg_valid_entry` in arch/x86/pci/mmconfig-shared.c). +/// +/// - Addresses below 4 GiB are valid for legacy PCIe ECAM. +/// - Addresses at or above 4 GiB require the MCFG table revision >= 1, matching +/// the ACPI 4.0+ rule that 64-bit ECAM addresses are only valid when the +/// firmware signals support via the revision field. +fn validate_mcfg_addr(addr: u64) -> Result<(), String> { + const FOUR_GIB: u64 = 0x1_0000_0000; + + if addr < FOUR_GIB { + return Ok(()); + } + + Err(format!("address {addr:#018x} >= 4 GiB requires MCFG revision >= 1 (ACPI 4.0+); entry skipped")) +} + struct Alloc { seg: u16, start_bus: u8, @@ -239,9 +266,11 @@ Ok(pcie) => pcie, Err(fdt_error) => { log::warn!( - "Couldn't retrieve PCIe info, perhaps the kernel is not compiled with \ - acpi or device tree support? Using the PCI 3.0 configuration space \ - instead. ACPI error: {:?} FDT error: {:?}", + "PCIe (ECAM/MCFG) not available: {}. \ + Device tree ECAM also not found: {}. \ + Falling back to PCI 3.0 config space (I/O ports 0xCF8/0xCFC). \ + For PCI Express support, use QEMU Q35 machine type (-machine q35) \ + or ensure your platform firmware provides an MCFG ACPI table.", acpi_error, fdt_error ); @@ -266,24 +295,44 @@ .0 .iter() .filter_map(|desc| { + if desc.seg_group_num != 0 { + let seg = desc.seg_group_num; + let start = desc.start_bus; + let end = desc.end_bus; + log::warn!( + "pcid: skipping MCFG entry at seg={seg} bus {start}..={end}: multi-segment not yet implemented", + ); + return None; + } + + if let Err(reason) = validate_mcfg_addr(desc.base_addr) { + let addr = desc.base_addr; + let seg = desc.seg_group_num; + let start = desc.start_bus; + let end = desc.end_bus; + log::warn!( + "pcid: skipping MCFG entry at {addr:#018x} (seg={seg} bus {start}..={end}): {reason}", + ); + return None; + } + + let seg = desc.seg_group_num; + let start = desc.start_bus; + let end = desc.end_bus; Some(Alloc { - seg: desc.seg_group_num, - start_bus: desc.start_bus, - end_bus: desc.end_bus, + seg, + start_bus: start, + end_bus: end, mem: PhysBorrowed::map( desc.base_addr.try_into().ok()?, BYTES_PER_BUS - * (usize::from(desc.end_bus) - usize::from(desc.start_bus) + 1), + * (usize::from(end) - usize::from(start) + 1), Prot::RW, MemoryType::Uncacheable, ) .inspect_err(|err| { log::error!( - "failed to map seg {} bus {}..={}: {}", - { desc.seg_group_num }, - { desc.start_bus }, - { desc.end_bus }, - err + "failed to map seg {seg} bus {start}..={end}: {err}", ) }) .ok()?,