From 88dab83e0af1eeb2686913e1dd710dce8a2c446d Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 18:44:27 +0200 Subject: [PATCH 1/5] pcidev: a BAR behind a bridge is placed only where every bridge on its path forwards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A claim moves each BAR it maps onto a 2 MiB window of its own, because a 2 MiB page is the only one this kernel maps into a process: at firmware's address the page holds other functions' registers too. The candidates came from the root's free runs alone, and those exclude every range a bridge forwards. So a function behind a bridge was only ever offered addresses its bridge does not forward. On the AMD bench, 03:00.4 (the xHCI behind the port that forwards 0xd0000000..0xd0700000) was moved onto eight addresses from 0x450200000 up and answered 0xffffffff at each. A q35 virtio NIC behind a pcie-root-port reproduces it on main: eight candidates above its port's 0xc000000000..0xc020000000, none answering, "NOT HANDED OVER". A function on a bus a bridge leads to now takes its candidates from the windows of the bridge whose secondary bus it is, of the BAR's kind (a non-prefetchable BAR only in the memory window, PCI-to-PCI Bridge Architecture §3.2.5.6 to §3.2.5.8), below 4 GiB for a 32-bit BAR, inside the root's declaration, and free of every BAR, every window cut or being tried, and the windows of every bridge not on the path. A candidate some bridge on the path does not forward is passed over unread. Where nothing is free the BAR stays where firmware put it and is handed to nobody, named NoRoutedRun: there it shares its page, so the reason for moving it still holds. `bar_map_behind_a_bridge` claims the NIC behind a root port on a new q35 profile and checks the placed address is inside what the port forwards; it fails on main's kernel. Not caused by #916, which changed no candidate: the root runs never held a bridge's windows. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016UzbDnM9m6nQWhbjGi6Lv1 --- kernel/pci/src/bridge.rs | 33 ++++++++ kernel/pci/src/placement.rs | 141 ++++++++++++++++++++++++++++++++- kernel/src/drivers/pci.rs | 27 ++++--- kernel/src/pcidev/mod.rs | 152 ++++++++++++++++++++++++++++++------ tests/common/qemu.rs | 14 ++++ tests/toyos.rs | 53 +++++++++++++ 6 files changed, 388 insertions(+), 32 deletions(-) diff --git a/kernel/pci/src/bridge.rs b/kernel/pci/src/bridge.rs index e22c8040d91..42b517837c7 100644 --- a/kernel/pci/src/bridge.rs +++ b/kernel/pci/src/bridge.rs @@ -22,6 +22,39 @@ pub const PREFETCH_BASE: u64 = 0x24; /// removed. pub const HEADER_TYPE_BRIDGE: u8 = 1; +/// Byte offsets of a Type 1 header's Secondary and Subordinate Bus Number +/// registers (§3.2.5.3, §3.2.5.4): the bus directly below the bridge, and +/// the highest bus number anywhere below it. +pub const SECONDARY_BUS: u64 = 0x19; +pub const SUBORDINATE_BUS: u64 = 0x1A; + +/// One bridge, as the path to a function behind it reads it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Bridge { + pub secondary: u8, + pub subordinate: u8, + pub memory: Option, + pub prefetchable: Option, +} + +impl Bridge { + /// Whether `bus` is below this bridge, so every request for a function + /// on it passes through this bridge's windows. + pub fn leads_to(&self, bus: u8) -> bool { + self.secondary <= bus && bus <= self.subordinate + } + + /// Whether this bridge forwards all of `at..end` to its secondary side + /// through a window a BAR of this kind may sit in (§3.2.5.6 to + /// §3.2.5.8). The prefetchable window may be read ahead and its writes + /// merged, so only a prefetchable BAR belongs there; the memory window + /// takes either kind. + pub fn forwards(&self, at: u64, end: u64, prefetchable: bool) -> bool { + let holds = |window: Option| window.is_some_and(|w| w.start <= at && end <= w.end); + holds(self.memory) || (prefetchable && holds(self.prefetchable)) + } +} + /// The granularity both windows are expressed in: bits 31:20. const GRANULE: u64 = 1 << 20; diff --git a/kernel/pci/src/placement.rs b/kernel/pci/src/placement.rs index 07d2c35f8ba..621f601b95b 100644 --- a/kernel/pci/src/placement.rs +++ b/kernel/pci/src/placement.rs @@ -10,7 +10,7 @@ use toyos_abi::boot::RootBridgeWindow; -use crate::bridge::Window; +use crate::bridge::{Bridge, Window}; /// Where the fixed platform devices start on a PC: the I/O APIC, the HPET and /// the LAPIC window are at and above this, so a 32-bit run may not reach it. @@ -32,6 +32,11 @@ pub const WIDE_FLOOR: u64 = 1 << 32; pub fn free_runs(taken: &mut [Window], wide: bool) -> impl Iterator + '_ { let (floor, ceiling) = if wide { (WIDE_FLOOR, u64::MAX) } else { (0, PLATFORM_MMIO) }; taken.sort_unstable_by_key(|extent| extent.start); + gaps(taken, floor, ceiling) +} + +/// What sorted `taken` leaves free of `floor..ceiling`, lowest first. +fn gaps(taken: &[Window], floor: u64, ceiling: u64) -> impl Iterator + '_ { let mut at = floor; let mut rest = taken.iter(); core::iter::from_fn(move || { @@ -104,6 +109,49 @@ pub fn reserve( None } +/// The lowest `span`-aligned address for a BAR on `bus` behind `bridges`, +/// inside one of `windows` and free of `taken`, and that window's base; +/// `None` where there is none. +/// +/// **Only what every bridge on the path forwards is offered** (PCI-to-PCI +/// Bridge Architecture §3.2.5.6 to §3.2.5.8): an address outside a window of +/// one of them is never forwarded to the function, and a read of it does not +/// come back on real hardware. Candidates come from the windows of the bridge +/// whose secondary bus is `bus`, of the BAR's kind, below 4 GiB for a 32-bit +/// BAR. A run whose first such address an ancestor does not also forward is +/// passed over unread: firmware nests a bridge's windows inside its parent's, +/// and a machine that did not is offered nothing there. +/// `taken` is every extent something else decodes in there: every BAR, every +/// window cut or being tried, and the windows of every bridge not on the path. +pub fn behind_bridges( + bridges: &[Bridge], + bus: u8, + prefetchable: bool, + wide: bool, + taken: &mut [Window], + windows: &[RootBridgeWindow], + span: u64, +) -> Option<(u64, u64)> { + let parent = bridges.iter().find(|bridge| bridge.secondary == bus)?; + let reach = if wide { u64::MAX } else { WIDE_FLOOR }; + taken.sort_unstable_by_key(|extent| extent.start); + let kinds = [parent.memory, if prefetchable { parent.prefetchable } else { None }]; + let mut first: Option<(u64, u64)> = None; + for window in kinds.into_iter().flatten() { + for run in gaps(taken, window.start, window.end.min(reach)) { + let Some((at, base)) = offered(&run, windows, span) else { continue }; + let routed = bridges + .iter() + .filter(|bridge| bridge.leads_to(bus)) + .all(|bridge| bridge.forwards(at, at + span, prefetchable)); + if routed && first.is_none_or(|(held, _)| at < held) { + first = Some((at, base)); + } + } + } + first +} + /// Put a reservation back. /// /// A run hands its addresses out upward and holds one extent, so the only one @@ -389,4 +437,95 @@ mod tests { release(&mut runs, second, 2 * MIB); assert_eq!(reserve(&mut runs, &T14_WINDOWS, 2 * MIB), Some(second)); } + + /// The AMD bench's bus 3 behind its root port, as its boot enumerated it: + /// the port forwards `0xd0000000..0xd0700000` and, prefetchable, + /// `0xfc90000000..0xfca0200000`, and firmware packed every bus-3 BAR into + /// them. The loader's declaration for that root is the bench's own. + mod behind { + use super::*; + + const MIB: u64 = 1024 * 1024; + const PORT: Bridge = Bridge { + secondary: 3, + subordinate: 3, + memory: Some(Window { start: 0xd000_0000, end: 0xd070_0000 }), + prefetchable: Some(Window { start: 0xfc_9000_0000, end: 0xfc_a020_0000 }), + }; + const DECLARED: [RootBridgeWindow; 3] = [ + RootBridgeWindow { base: 0xd000_0000, length: 0x70_0000 }, + RootBridgeWindow { base: 0xfc_9000_0000, length: 0x1020_0000 }, + RootBridgeWindow { base: 0x4_5020_0000, length: 0xf8_3fe0_0000 }, + ]; + + fn bus_3() -> std::vec::Vec { + let bar = |start, bytes| Window { start, end: start + bytes }; + std::vec![ + bar(0xd000_0000, MIB), + bar(0xd010_0000, MIB), + bar(0xd030_0000, MIB), + bar(0xd040_0000, 512 * 1024), + bar(0xd048_0000, 256 * 1024), + bar(0xd04c_0000, 32 * 1024), + bar(0xd04c_8000, 16 * 1024), + bar(0xd04c_e000, 8 * 1024), + bar(0xfc_9000_0000, 256 * MIB), + bar(0xfc_a000_0000, 2 * MIB), + ] + } + + /// The bench's xHCI at `03:00.4`, a 64-bit non-prefetchable BAR: no + /// 2 MiB of the port's memory window is free, so nothing is offered, + /// and in particular not `0x450200000`, which the declaration names + /// and the port does not forward. + #[test] + fn a_packed_bridge_window_offers_nothing_and_never_the_roots_space() { + assert_eq!(behind_bridges(&[PORT], 3, false, true, &mut bus_3(), &DECLARED, 2 * MIB), None); + assert_eq!(behind_bridges(&[PORT], 3, true, true, &mut bus_3(), &DECLARED, 2 * MIB), None); + } + + /// The same port with 2 MiB of its memory window free: that address, + /// and nothing outside the window. + #[test] + fn a_free_run_inside_the_path_is_offered() { + let port = Bridge { memory: Some(Window { start: 0xd000_0000, end: 0xd080_0000 }), ..PORT }; + let declared = [RootBridgeWindow { base: 0xd000_0000, length: 0x80_0000 }]; + assert_eq!( + behind_bridges(&[port], 3, false, true, &mut bus_3(), &declared, 2 * MIB), + Some((0xd060_0000, 0xd000_0000)) + ); + } + + /// A prefetchable BAR may sit in the prefetchable window; a + /// non-prefetchable one never does, and a 32-bit one never above 4 GiB. + #[test] + fn the_bars_kind_picks_the_window() { + let port = Bridge { memory: None, ..PORT }; + let mut taken = [Window { start: 0xfc_9000_0000, end: 0xfc_a000_0000 }]; + assert_eq!( + behind_bridges(&[port], 3, true, true, &mut taken, &DECLARED, 2 * MIB), + Some((0xfc_a000_0000, 0xfc_9000_0000)) + ); + assert_eq!(behind_bridges(&[port], 3, false, true, &mut taken, &DECLARED, 2 * MIB), None); + assert_eq!(behind_bridges(&[port], 3, true, false, &mut taken, &DECLARED, 2 * MIB), None); + } + + /// Every bridge on the path has to forward the address: a port whose + /// window lies outside its parent's is offered nothing, and so is a + /// bus no bridge leads to directly or one whose bus numbers are absurd. + #[test] + fn an_address_one_bridge_on_the_path_does_not_forward_is_never_offered() { + let free = Window { start: 0xd000_0000, end: 0xd040_0000 }; + let declared = [RootBridgeWindow { base: 0xd000_0000, length: 0x40_0000 }]; + let parent = Bridge { secondary: 2, subordinate: 3, memory: Some(Window { start: 0xe000_0000, end: 0xe040_0000 }), prefetchable: None }; + let child = Bridge { secondary: 3, subordinate: 3, memory: Some(free), prefetchable: None }; + assert_eq!(behind_bridges(&[child], 3, false, false, &mut [], &declared, 2 * MIB), Some((0xd000_0000, 0xd000_0000))); + assert_eq!(behind_bridges(&[parent, child], 3, false, false, &mut [], &declared, 2 * MIB), None); + assert_eq!(behind_bridges(&[child], 4, false, false, &mut [], &declared, 2 * MIB), None); + let backwards = Bridge { secondary: 3, subordinate: 2, ..child }; + assert!(!backwards.leads_to(3)); + assert_eq!(behind_bridges(&[parent, backwards], 3, false, false, &mut [], &declared, 2 * MIB), None); + } + } + } diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index 750fd54ff6f..488e15904d9 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -442,17 +442,26 @@ impl PciDevice { /// may hand out, and reading them costs the machine nothing — unlike /// `bar_size`, which takes memory decode off for the length of its probe. pub fn forwarded(&self) -> Vec { + self.bridge().map_or_else(Vec::new, |b| b.memory.into_iter().chain(b.prefetchable).collect()) + } + + /// This function as a bridge: the buses below it and the windows it + /// forwards to them, read and never probed; `None` for a function that is + /// not a bridge. + pub fn bridge(&self) -> Option { if self.read_config_u8(HEADER_TYPE) & !MULTI_FUNCTION != bridge::HEADER_TYPE_BRIDGE { - return Vec::new(); + return None; } - let mut out = Vec::new(); - out.extend(bridge::window(self.read_config_u32(bridge::MEMORY_BASE))); - out.extend(bridge::prefetch( - self.read_config_u32(bridge::PREFETCH_BASE), - self.read_config_u32(bridge::PREFETCH_BASE_UPPER), - self.read_config_u32(bridge::PREFETCH_LIMIT_UPPER), - )); - out + Some(bridge::Bridge { + secondary: self.read_config_u8(bridge::SECONDARY_BUS), + subordinate: self.read_config_u8(bridge::SUBORDINATE_BUS), + memory: bridge::window(self.read_config_u32(bridge::MEMORY_BASE)), + prefetchable: bridge::prefetch( + self.read_config_u32(bridge::PREFETCH_BASE), + self.read_config_u32(bridge::PREFETCH_BASE_UPPER), + self.read_config_u32(bridge::PREFETCH_LIMIT_UPPER), + ), + }) } pub fn capabilities(&self) -> CapabilityIter<'_> { diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 23fe9220c7a..6d28ccf3c4e 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -33,7 +33,10 @@ //! window of the declaration [`Apertures`] holds: the address firmware put in //! the BAR is held against those windows here, no other address is ever //! offered ([`placement::reserve`]), and with no declaration held no function -//! is handed over. +//! is handed over. Behind a bridge an address is offered only where every +//! bridge on the path forwards it ([`placement::behind_bridges`]); where none +//! of that is free the BAR stays where firmware put it and is mapped to +//! nobody, since its page there is not its own. //! //! **Inside one, the function itself is the proof.** [`place_bar`] reads one //! dword through the BAR where firmware put it, moves the BAR onto the @@ -127,7 +130,7 @@ use toyos_abi::boot::{MemoryMapEntry, RootBridgeWindow}; use toyos_abi::pci::{DeviceIrqRecord, PciFunctionInfo, BARS}; use toyos_abi::syscall::{PciId, RegWidth, SyscallError}; use toyos_dma::Register; -use toyos_pci::bridge::Window; +use toyos_pci::bridge::{Bridge, Window}; use toyos_pci::{af, aperture, bar, express, msix, placement, pm, probe}; use crate::device::{Claim, ClaimError}; @@ -363,6 +366,13 @@ struct Machine { /// which is not the same claim as reaching the bus. low: Vec, high: Vec, + /// Every bridge on the bus, read in [`publish`]: what a function behind + /// one may be placed inside ([`placement::behind_bridges`]). + bridges: Vec, + /// Addresses behind a bridge a claim has tried and not cut: no other claim + /// is offered one while it is probed, and one the function did not answer + /// at stays spent, as a refused run address does. + trying: Vec, /// What this kernel holds of the memory the root bridges decode: the /// necessary condition for any read this module issues. apertures: Apertures, @@ -403,6 +413,8 @@ static MACHINE: Lock = Lock::new(Machine { kernel_driven: Vec::new(), low: Vec::new(), high: Vec::new(), + bridges: Vec::new(), + trying: Vec::new(), apertures: Apertures::Unknown, windows: Vec::new(), resetting: Vec::new(), @@ -532,6 +544,7 @@ pub fn note_kernel_driver(pci: &PciDevice) { /// driver mid-transfer must not meet that. pub fn publish(devices: &[PciDevice], segment: u16, maps: &[MemoryMapEntry], firmware: &[RootBridgeWindow]) { let mut decoded = Vec::new(); + let bridges: Vec = devices.iter().filter_map(PciDevice::bridge).collect(); let mut taken: Vec = maps.iter().map(|entry| Window { start: entry.start, end: entry.end }).collect(); for device in devices { @@ -624,6 +637,7 @@ pub fn publish(devices: &[PciDevice], segment: u16, maps: &[MemoryMapEntry], fir machine.segment = segment; machine.decoded = decoded; machine.apertures = apertures; + machine.bridges = bridges; machine.low = low; machine.high = high; } @@ -680,6 +694,9 @@ enum Refusal { /// This machine offered the BAR no address of that width at all: no free /// run holds a span-aligned one inside a window firmware declared. NoRun { wide: bool }, + /// The function is behind a bridge, and no 2 MiB the bridges on its path + /// forward to it is free, so the BAR stays where firmware put it. + NoRoutedRun(u8), /// Every address this machine offered was tried and the function answered /// at none of them. NoPlacement { wide: bool, asked: usize }, @@ -752,6 +769,12 @@ impl core::fmt::Display for Refusal { this bus's assigned BARs and its bridges' forwarded ranges and inside a window \ firmware declared" ), + Self::NoRoutedRun(i) => write!( + f, + "no 2 MiB the bridges above it forward is free for BAR {i}, so it stays where \ + firmware put it, and there it shares a 2 MiB page a process would be given \ + with whatever else decodes there" + ), Self::NoPlacement { wide, asked } => write!( f, "its BAR was moved onto {asked} {}-bit address(es) inside the windows firmware \ @@ -1014,7 +1037,8 @@ fn place_bars( // is that window's loss and not the function's. Err(why @ (Refusal::BarUnassigned(_) | Refusal::BarUnrouted(_) - | Refusal::BarReferenceEmpty(_))) => { + | Refusal::BarReferenceEmpty(_) + | Refusal::NoRoutedRun(_))) => { log!( "pcidev: PCI {:02x}:{:02x}.{} keeps BAR {index} where firmware put it and \ hands it to nobody — {why}", @@ -1334,8 +1358,9 @@ fn probe_dword(at: u64, span: u64, offset: u64) -> u32 { crate::mm::paging::map_mmio(at, span, MmioPolicy::Uncacheable).read_u32(offset) } -/// Move BAR `index` onto a 2 MiB boundary inside a window firmware declared, -/// and answer where. +/// Move BAR `index` onto a 2 MiB boundary inside a window firmware declared +/// and, behind a bridge, inside what every bridge on its path forwards, and +/// answer where. /// /// **Only an address inside a window firmware declared is ever read**, because /// a load no bridge forwards does not come back on real hardware — the address @@ -1377,7 +1402,8 @@ fn place_bar(pci: &PciDevice, id: PciId, index: u8, size: u64, windows: &[RootBr // A BAR firmware assigned no address has no such reference, and reading one // at zero would take low RAM for the function's answer — so it is refused // by name rather than settled against a number that is nothing's. - let was = pci.memory_bar(index).map_err(|_| Refusal::BarUnplaceable(index))?.address(); + let firmware = pci.memory_bar(index).map_err(|_| Refusal::BarUnplaceable(index))?; + let was = firmware.address(); if was == 0 { return Err(Refusal::BarUnassigned(index)); } @@ -1392,17 +1418,17 @@ fn place_bar(pci: &PciDevice, id: PciId, index: u8, size: u64, windows: &[RootBr } let who = alloc::format!("PCI {:02x}:{:02x}.{}", pci.bus, pci.dev, pci.func); + let source = Source::of(pci.bus, wide, firmware.prefetchable()); let mut refused = [None; MAX_CANDIDATES]; let mut asked = 0usize; while asked < MAX_CANDIDATES { - let Some(candidate) = with_runs(wide, |runs| placement::reserve(runs, windows, span)) - else { + let Some(candidate) = source.take(windows, span) else { break; }; - let at = candidate.at; + let at = candidate.at(); let Ok(placed) = bar::placement(index, low, at, size) else { refused[asked] = Some(candidate); - give_back(wide, &refused, span); + give_back(&refused, span); return Err(Refusal::BarUnplaceable(index)); }; pci.set_memory_decode(false); @@ -1416,7 +1442,7 @@ fn place_bar(pci: &PciDevice, id: PciId, index: u8, size: u64, windows: &[RootBr if !matches!(pci.memory_bar(index), Ok(memory) if memory.address() == at) { restore(); refused[asked] = Some(candidate); - give_back(wide, &refused, span); + give_back(&refused, span); return Err(Refusal::BarUnplaceable(index)); } let after = probe_dword(at, span, reference); @@ -1427,7 +1453,7 @@ fn place_bar(pci: &PciDevice, id: PciId, index: u8, size: u64, windows: &[RootBr "pcidev: {who} BAR {index} ({size:#x} bytes) placed at {at:#x} — inside \ firmware's mem {:#x}; its +{reference:#x} dword answers {after:#010x} there, \ and answered {signature:#010x} from {was:#x}, where firmware put it", - candidate.window + candidate.window() ); return Ok(at); } @@ -1441,8 +1467,84 @@ fn place_bar(pci: &PciDevice, id: PciId, index: u8, size: u64, windows: &[RootBr refused[asked] = Some(candidate); asked += 1; } - give_back(wide, &refused, span); - Err(if asked == 0 { Refusal::NoRun { wide } } else { Refusal::NoPlacement { wide, asked } }) + give_back(&refused, span); + Err(match (asked, source) { + (0, Source::Behind { .. }) => Refusal::NoRoutedRun(index), + (0, Source::Runs { .. }) => Refusal::NoRun { wide }, + _ => Refusal::NoPlacement { wide, asked }, + }) +} + +/// Where a BAR's candidate addresses come from: this machine's free runs for a +/// function on a root bus, and the windows of the bridges on its path for one +/// behind a bridge, which no root-level run lies in. +#[derive(Clone, Copy)] +enum Source { + Runs { wide: bool }, + Behind { bus: u8, wide: bool, prefetchable: bool }, +} + +/// One address [`Source::take`] handed out, and where it goes back to. +#[derive(Clone, Copy)] +enum Candidate { + Run { wide: bool, reservation: placement::Reservation }, + Behind { at: u64, window: u64 }, +} + +impl Candidate { + fn at(self) -> u64 { + match self { + Self::Run { reservation, .. } => reservation.at, + Self::Behind { at, .. } => at, + } + } + + /// The base of the declared root window the address is inside. + fn window(self) -> u64 { + match self { + Self::Run { reservation, .. } => reservation.window, + Self::Behind { window, .. } => window, + } + } +} + +impl Source { + fn of(bus: u8, wide: bool, prefetchable: bool) -> Self { + if MACHINE.lock().bridges.iter().any(|bridge| bridge.leads_to(bus)) { + Self::Behind { bus, wide, prefetchable } + } else { + Self::Runs { wide } + } + } + + /// The next address to try, already out of reach of every other claim. + fn take(self, windows: &[RootBridgeWindow], span: u64) -> Option { + match self { + Self::Runs { wide } => with_runs(wide, |runs| placement::reserve(runs, windows, span)) + .map(|reservation| Candidate::Run { wide, reservation }), + Self::Behind { bus, wide, prefetchable } => { + let mut machine = MACHINE.lock(); + let mut taken: Vec = machine + .decoded + .iter() + .map(|&(_, start, end)| Window { start, end }) + .chain(machine.windows.iter().map(|&(_, _, at, span)| Window { start: at, end: at + span })) + .chain(machine.trying.iter().copied()) + .chain( + machine + .bridges + .iter() + .filter(|bridge| !bridge.leads_to(bus)) + .flat_map(|bridge| bridge.memory.into_iter().chain(bridge.prefetchable)), + ) + .collect(); + let (at, window) = + placement::behind_bridges(&machine.bridges, bus, prefetchable, wide, &mut taken, windows, span)?; + machine.trying.push(Window { start: at, end: at + span }); + Some(Candidate::Behind { at, window }) + } + } + } } /// This machine's free runs of one width, under the lock that hands them out. @@ -1457,14 +1559,18 @@ fn with_runs(wide: bool, f: impl FnOnce(&mut Vec) -> T) -> T { f(if wide { &mut machine.high } else { &mut machine.low }) } -/// Put back every address a refused walk took, newest first: a run gives back -/// only the address it handed out last. -fn give_back(wide: bool, refused: &[Option], span: u64) { - with_runs(wide, |runs| { - for candidate in refused.iter().rev().flatten() { - placement::release(runs, *candidate, span); +/// Put back every address a walk took and did not keep, newest first: a run +/// gives back only the address it handed out last. +fn give_back(refused: &[Option], span: u64) { + let mut machine = MACHINE.lock(); + for candidate in refused.iter().rev().flatten() { + match *candidate { + Candidate::Run { wide, reservation } => { + placement::release(if wide { &mut machine.high } else { &mut machine.low }, reservation, span) + } + Candidate::Behind { at, .. } => machine.trying.retain(|tried| tried.start != at), } - }); + } } /// Where this BAR was already put, if a claim before this one put it there. @@ -1500,7 +1606,9 @@ fn was_cut(pci: &PciDevice, index: u8) -> bool { /// under [`with_runs`], which is what keeps a second claim from being offered /// it. fn cut(pci: &PciDevice, index: u8, at: u64, span: u64) { - MACHINE.lock().windows.push((requester(pci), index, at, span)); + let mut machine = MACHINE.lock(); + machine.trying.retain(|tried| tried.start != at); + machine.windows.push((requester(pci), index, at, span)); } /// Nothing else on this machine decodes inside the page that is about to be diff --git a/tests/common/qemu.rs b/tests/common/qemu.rs index 4c020f44dd6..d87033a407b 100644 --- a/tests/common/qemu.rs +++ b/tests/common/qemu.rs @@ -691,6 +691,10 @@ pub enum Profile { /// the T14's two are, because a claim never maps the BAR that holds an /// MSI-X table and QEMU puts this one's in its registers' BAR. HeadlessUsbSpare, + /// [`Profile::Headless`] with its virtio NIC behind a `pcie-root-port`: + /// a function a claim may take whose BAR has to land inside the port's + /// windows, as every function behind a bridge on a laptop does. + HeadlessNicBehindPort, /// M1 metal-sim: GOP, NVMe, xHCI with the boot stick on it, i8042 from /// q35, and nothing else -- no virtio device and no USB HID. This is the /// machine shape that gets flashed, so it is the one the input tests run @@ -770,6 +774,7 @@ impl Profile { | Self::HeadlessNoUsb | Self::HeadlessVirtioGpu | Self::HeadlessUsbSpare + | Self::HeadlessNicBehindPort | Self::Metal | Self::MetalNoX2apic | Self::MetalAmdVi @@ -883,6 +888,8 @@ impl Virtio { enum Nic { Absent, Virtio, + /// The virtio NIC on the secondary bus of a root port of its own. + BehindPort, } /// Everything a profile decides about the machine, in one table. A new @@ -1049,6 +1056,7 @@ impl Profile { Self::HeadlessNoIommu => Shape { iommu: None, ..Self::Headless.shape() }, Self::HeadlessNoUsb => Shape { xhci: &[], usb: &[], ..Self::Headless.shape() }, Self::HeadlessVirtioGpu => Shape { virtio_gpu: true, ..Self::Headless.shape() }, + Self::HeadlessNicBehindPort => Shape { nic: Nic::BehindPort, ..Self::Headless.shape() }, Self::HeadlessUsbSpare => Shape { xhci: &[XHCI_DEFAULT, XHCI_SPARE], usb: &[ @@ -2322,6 +2330,12 @@ fn qemu_command( "virtio-net-pci-non-transitional,netdev=net0{platform}" )); } + Nic::BehindPort => { + qemu.arg("-device").arg("pcie-root-port,id=nicport,chassis=9"); + qemu.arg("-netdev").arg("user,id=net0").arg("-device").arg(format!( + "virtio-net-pci-non-transitional,netdev=net0,bus=nicport{platform}" + )); + } } if shape.virtio.present() { if shape.virtio.sound() { diff --git a/tests/toyos.rs b/tests/toyos.rs index 86f930e33cd..beaa9c0a569 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -403,6 +403,12 @@ const MACHINE_TESTS: &[&str] = &[ // function is its I219, and a kernel that dies on this takes the bench's // machine down with it. "bar_map_again", + // The same claim with the NIC behind a root port: the BAR has to land + // inside the windows the port forwards, which a host test of the rule + // cannot say the kernel reads off a real bridge. The T14's claimed + // functions are all on bus 0; the AMD bench's xHCI that is not has its + // MSI-X table in its one BAR. + "bar_map_behind_a_bridge", // A region that is not memory the kernel allocated, lent as a window: the // kernel describes the BAR of a function no program of the boot holds, // which on the T14 is its I219, as `bar_map_again` says. @@ -3719,6 +3725,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { "machine_shutdown_wire_kept" => power::machine_shutdown_wire_held(test_config, true), "machine_shutdown_wire_at_the_seal" => power::machine_shutdown_wire_at_the_seal(test_config), "bar_map_again" => bar_map_again(test_config), + "bar_map_behind_a_bridge" => bar_map_behind_a_bridge(test_config), "lent_device_memory_refused" => lent_device_memory_refused(test_config), "console_image_boots" => console_image_boots(), "desktop_without_x2apic" => desktop_without_x2apic(), @@ -4023,6 +4030,52 @@ fn bar_map_again(test_config: &Path) -> Result<(), String> { Ok(()) } +/// The virtio NIC behind a root port is claimed and its BAR mapped, and the +/// kernel put the BAR inside a window the port forwards. +fn bar_map_behind_a_bridge(test_config: &Path) -> Result<(), String> { + const JOB: &str = "bar_map_again"; + const FORWARDS: &str = "pcidev: PCI 00:"; + const PLACED: &str = "pcidev: PCI 01:00.0 BAR "; + let bin = qemu::build_toyos_bin(qemu::SUITE_ARCH, &compile::repo_root().join("tests/guest/rust-tests"), JOB); + let options = BootOptions { profile: qemu::Profile::HeadlessNicBehindPort, ..Default::default() }; + let mut qemu = QemuInstance::boot_with_options(test_config, &[], &[(JOB.to_string(), bin)], options); + let boot = serial::Serial::boot(&qemu); + boot.must_be_clean()?; + let range = |line: &str| -> Option<(u64, u64)> { + let (from, to) = line.split_once("..")?; + let hex = |t: &str| u64::from_str_radix(t.trim().strip_prefix("0x")?, 16).ok(); + Some((hex(from.rsplit(' ').next()?)?, hex(to.split(' ').next()?)?)) + }; + let forwarded: Vec<(u64, u64)> = boot + .text() + .lines() + .filter(|l| l.contains(FORWARDS) && l.contains(" forwards ")) + .filter_map(|l| range(l.split_once(" forwards ")?.1)) + .collect(); + if forwarded.is_empty() { + return Err(format!("no root port said what it forwards:\n{}", boot.text())); + } + let result = qemu.run_test("test_rs_bar_map_again", Duration::from_secs(60)); + if let Some(why) = &result.error { + return Err(format!("{why}\nthe job said:\n{}", result.stdout)); + } + if result.exit_code != Some(0) { + return Err(format!("the job ended {:?}:\n{}", result.exit_code, result.stdout)); + } + let Some(placed) = result.serial.lines().find(|l| l.contains(PLACED) && l.contains(" placed at ")) else { + return Err(format!("the NIC's BAR was never placed:\n{}", result.serial)); + }; + let at = placed + .split_once(" placed at ") + .and_then(|(_, rest)| u64::from_str_radix(rest.split_whitespace().next()?.strip_prefix("0x")?, 16).ok()) + .ok_or_else(|| format!("an unreadable placement: {placed:?}"))?; + if !forwarded.iter().any(|(start, end)| *start <= at && at < *end) { + return Err(format!("{placed:?} is inside none of what the port forwards, {forwarded:x?}")); + } + eprintln!(" [claims] {}", placed.trim()); + Ok(()) +} + /// The kernel answers a lent window that is the virtio NIC's BAR as no memory /// it allocated, by `fstat`, so no reader maps it as a region. fn lent_device_memory_refused(test_config: &Path) -> Result<(), String> { From 24ed5a756bc0b14959389e00cb003110542326fc Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 19:00:21 +0200 Subject: [PATCH 2/5] review: the bench's port forwards 0xd0000000..0xd0500000, as its boot read it The host test's AMD bus 3 had the root's window for its port, and lacked 03:00.7's BARs; both are now the bench's own reading. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016UzbDnM9m6nQWhbjGi6Lv1 --- kernel/pci/src/placement.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/kernel/pci/src/placement.rs b/kernel/pci/src/placement.rs index 621f601b95b..d57062aba0f 100644 --- a/kernel/pci/src/placement.rs +++ b/kernel/pci/src/placement.rs @@ -438,10 +438,10 @@ mod tests { assert_eq!(reserve(&mut runs, &T14_WINDOWS, 2 * MIB), Some(second)); } - /// The AMD bench's bus 3 behind its root port, as its boot enumerated it: - /// the port forwards `0xd0000000..0xd0700000` and, prefetchable, + /// The AMD bench's bus 3 behind its port `00:08.1`, as its boot read it: + /// the port forwards `0xd0000000..0xd0500000` and, prefetchable, /// `0xfc90000000..0xfca0200000`, and firmware packed every bus-3 BAR into - /// them. The loader's declaration for that root is the bench's own. + /// them. The declaration is the bench's own loader list. mod behind { use super::*; @@ -449,7 +449,7 @@ mod tests { const PORT: Bridge = Bridge { secondary: 3, subordinate: 3, - memory: Some(Window { start: 0xd000_0000, end: 0xd070_0000 }), + memory: Some(Window { start: 0xd000_0000, end: 0xd050_0000 }), prefetchable: Some(Window { start: 0xfc_9000_0000, end: 0xfc_a020_0000 }), }; const DECLARED: [RootBridgeWindow; 3] = [ @@ -463,11 +463,13 @@ mod tests { std::vec![ bar(0xd000_0000, MIB), bar(0xd010_0000, MIB), + bar(0xd020_0000, MIB), bar(0xd030_0000, MIB), bar(0xd040_0000, 512 * 1024), bar(0xd048_0000, 256 * 1024), bar(0xd04c_0000, 32 * 1024), bar(0xd04c_8000, 16 * 1024), + bar(0xd04c_c000, 8 * 1024), bar(0xd04c_e000, 8 * 1024), bar(0xfc_9000_0000, 256 * MIB), bar(0xfc_a000_0000, 2 * MIB), @@ -484,8 +486,8 @@ mod tests { assert_eq!(behind_bridges(&[PORT], 3, true, true, &mut bus_3(), &DECLARED, 2 * MIB), None); } - /// The same port with 2 MiB of its memory window free: that address, - /// and nothing outside the window. + /// The same port forwarding 2 MiB more: that address, and nothing + /// outside the window. #[test] fn a_free_run_inside_the_path_is_offered() { let port = Bridge { memory: Some(Window { start: 0xd000_0000, end: 0xd080_0000 }), ..PORT }; From 5321afa0cfcc0adf288d234f33fef499ef9ba59e Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 19:02:22 +0200 Subject: [PATCH 3/5] issues: the bridge-window defect is taken up in part by #928 The interim places a BAR behind a bridge inside what its path forwards and names the case with no room; the move itself and the exit stay with P1. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016UzbDnM9m6nQWhbjGi6Lv1 --- ...ridge-out-of-the-range-the-bridge-forwards.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/issues/pcidev-moves-a-bar-behind-a-bridge-out-of-the-range-the-bridge-forwards.md b/issues/pcidev-moves-a-bar-behind-a-bridge-out-of-the-range-the-bridge-forwards.md index 79f0da9cbc3..f3fd25ae74c 100644 --- a/issues/pcidev-moves-a-bar-behind-a-bridge-out-of-the-range-the-bridge-forwards.md +++ b/issues/pcidev-moves-a-bar-behind-a-bridge-out-of-the-range-the-bridge-forwards.md @@ -1,5 +1,5 @@ --- -status: open +status: assigned kind: defect opened: 2026-10-12 --- @@ -48,6 +48,18 @@ this one: Under this issue no candidate lies inside a port's window, so none ever answers there. +**Interim, #928.** A function on a bus a bridge leads to now takes its +candidates from the windows of the bridge whose secondary bus it is, of a +kind the BAR may use (a non-prefetchable BAR only in the memory window; a +64-bit BAR also below 4 GiB, where the AMD bench's bus-3 window is), inside +the declaration, and only where every bridge on its path forwards +(`placement::behind_bridges`, host-tested; `bar_map_behind_a_bridge` claims a +q35 NIC behind a `pcie-root-port`). Where none of that is free, as on the AMD +bench's packed bus 3, the BAR stays where firmware put it and is handed to +nobody, by name (`NoRoutedRun`). The move itself stays until P1: a 2 MiB page +is still the only one a process is given, so a BAR that cannot have one of +its own is not handed over, and the two radios' claims rest on P1 still. + Owner: P1 of `issues/there-is-no-wifi.md`. **Exit**: @@ -55,7 +67,7 @@ Owner: P1 of `issues/there-is-no-wifi.md`. 4 KiB pages; - a BAR that shares a 4 KiB page with another function's is moved within those windows, never out of them, or is refused by name; -- a host test places a BAR behind a bridge; +- a host test places a BAR behind a bridge (#928's `behind` tests, met); - the T14's AX210 is claimed with its BAR in place; - each of the two placement issues above closed, or its exit rewritten to the placement P1 lands. From 503d1959945d867ea6b4dad6f6c5fefb01503012 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 19:38:28 +0200 Subject: [PATCH 4/5] issues: the defect left is nobody's held work until P1 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016UzbDnM9m6nQWhbjGi6Lv1 --- ...idge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md b/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md index 9ee3a55a229..28bf9d20ec0 100644 --- a/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md +++ b/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md @@ -1,5 +1,5 @@ --- -status: assigned +status: open kind: defect opened: 2026-10-12 --- From 317b1bb5298d04bf18776d3587af3c77c70ff0da Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 20:27:31 +0200 Subject: [PATCH 5/5] review: the issue says which function meets which refusal on this tree The AMD bench's 03:00.4 and the T14's AX210 hold their MSI-X table in their one BAR, which a claim withholds, so each is refused NoMappableBar before placement; NoRoutedRun is what a claim handing that BAR over would meet. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016UzbDnM9m6nQWhbjGi6Lv1 --- ...-free-2-mib-of-its-windows-is-not-handed-over.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md b/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md index 28bf9d20ec0..7908b672932 100644 --- a/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md +++ b/issues/a-bar-behind-a-bridge-with-no-free-2-mib-of-its-windows-is-not-handed-over.md @@ -13,17 +13,22 @@ where every bridge on the function's path forwards (`placement::behind_bridges`, `kernel/pci/src/placement.rs`); where none of that is free, the BAR stays where firmware put it and is handed to nobody, named `NoRoutedRun`, and a function with no other mappable BAR is refused. -A bridge firmware sized to its BARs leaves no such 2 MiB, so both benches' -radios and the AMD bench's spare xHCI are refused this way: +A bridge firmware sized to its BARs leaves no such 2 MiB. Where the BAR is +also the one holding the function's MSI-X table, a claim withholds it and +the function is refused `NoMappableBar` before placement; a claim that +handed the table's BAR over, as #921 does, would meet `NoRoutedRun`: - **The AMD bench**: the AX200 at 01:00.0 has BAR 0 at 0xd0600000, behind bridge 00:02.2, which forwards 0xd0600000..0xd0700000 (#884's E13 §1, https://github.com/ToyOSOrg/evidence/blob/2eb315326d1b4d0363b4c918da8dd2bfaabd1234/e13-amd-ax200-bar.log); the xHCI at 03:00.4 is behind 00:08.1, whose 0xd0000000..0xd0500000 - bus 3 fills (#921's bench log). + bus 3 fills. Its one BAR holds its MSI-X table, which a claim withholds, + so it is refused `NoMappableBar` before placement; a claim that handed + the table's BAR over, as #921 does, would meet `NoRoutedRun` there. - **The T14**, as S1 read it (lines 111 to 116, https://github.com/ToyOSOrg/evidence/blob/cd368b25b2259a76b80dc31f4219dac1c91d84dc/s1/wifi-s1-a-kernel.log): - the AX210 at 09:00.0 has BAR 0 at 0xbc300000, 16 KiB, behind root port + the AX210 at 09:00.0 has BAR 0 at 0xbc300000, 16 KiB, its MSI-X table + in it (`issues/there-is-no-wifi.md`, S1's read-out), behind root port 00:1c.4, which forwards 0xbc300000..0xbc400000. Two placement issues rest on placement as it is, and P1 takes them up with