diff --git a/issues/a-cargo-run-boot-claims-every-pcidev-slot.md b/issues/a-cargo-run-boot-claims-every-pcidev-slot.md deleted file mode 100644 index 9781738e36a..00000000000 --- a/issues/a-cargo-run-boot-claims-every-pcidev-slot.md +++ /dev/null @@ -1,26 +0,0 @@ ---- -status: assigned -kind: defect -opened: 2026-10-13 ---- - -# A `cargo run` boot claims every one of pcidev's slots - -Held by the orchestrator. - -`images/system.toml`'s rows claim four PCI functions on `cargo run`'s q35: -virtio-sound (`soundserver`), virtio-net (`netstack`), the NVMe controller -(`diskserver`) and the clipboard's virtio-serial function (`vdagent`). That is -every one of `pcidev::MAX_FUNCTIONS` (`kernel/src/pcidev/mod.rs`), so a fifth -claim on that machine is refused `Exhausted`. - -None of them goes back: a virtio function on q35's root bus publishes no power -management capability and no PCI Express capability, so it has no -function-level reset, and an ended holder leaves its slot as that function's -residue (`kernel/pci/src/slot.rs`). - -More slots reach both architectures' interrupt tables, which declare one -interrupt per slot (`kernel/src/arch/x86_64/idt/user_dev.rs`, -`kernel/src/arch/aarch64/irqchip/its.rs`). - -**Exit condition.** A `cargo run` boot leaves a claim slot free. diff --git a/issues/a-claimed-function-can-storm-its-cpu.md b/issues/a-claimed-function-can-storm-its-cpu.md index b00177f7f24..ce6e329bd36 100644 --- a/issues/a-claimed-function-can-storm-its-cpu.md +++ b/issues/a-claimed-function-can-storm-its-cpu.md @@ -6,9 +6,9 @@ opened: 2026-10-04 # A claimed function can storm its CPU -A function a process drives raises its claim slot's interrupt on cpu0 for -every message it sends — on x86-64 the slot's vector -(`kernel/src/arch/x86_64/idt/user_dev.rs`), on AArch64 the slot's LPI through +A function a process drives raises its claim's interrupt on cpu0 for +every message it sends — on x86-64 the claim's vector +(`kernel/src/arch/x86_64/idt/user_dev.rs`), on AArch64 the function's LPI through the ITS (`kernel/src/arch/aarch64/irqchip/its.rs`) — and `pcidev::isr` (`kernel/src/pcidev/mod.rs`) takes each one; nothing bounds how often. Its holder programs the device through the BARs it maps, so a hostile or broken @@ -18,7 +18,7 @@ them, and each one is an interrupt cpu0 takes before anything else it runs. that does to a CPU for a kernel driver's source; a claim is the same source with its holder outside the kernel. -The lever is the slot's own translation: on x86-64 its remapping entry, which +The lever is the claim's own translation: on x86-64 its remapping entry, which made not present stops the function's messages at the unit; on AArch64 its LPI's configuration byte, which cleared and followed by an `INV` disables the LPI at the redistributor. Masking through the device would need the diff --git a/issues/a-claims-own-refusals-are-read-by-nothing.md b/issues/a-claims-own-refusals-are-read-by-nothing.md index 171054a1f12..2b5f100c486 100644 --- a/issues/a-claims-own-refusals-are-read-by-nothing.md +++ b/issues/a-claims-own-refusals-are-read-by-nothing.md @@ -14,7 +14,8 @@ Refusals of `kernel/src/pcidev/mod.rs` that no test reaches: - `ClaimError::Ambiguous`, a config naming a device this machine has two of; - `ClaimError::KernelDriven`, a claim on a function one of this kernel's own drivers bound; -- `ClaimError::Exhausted`, a claim past `MAX_FUNCTIONS` slots; +- `ClaimError::Exhausted`, a claim past the vectors, remapping entries or + LPIs a machine has; - every window refusal — `Refusal::NoWindow`, `BarUnsizable`, `BarUnplaceable`, `BarResized` and `Dead`; - every bound `SYS_DEVICE_BAR_MAP` and `SYS_DEVICE_DMA_ALLOC` check, which is diff --git a/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md b/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md deleted file mode 100644 index 75b4c9a9e90..00000000000 --- a/issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md +++ /dev/null @@ -1,35 +0,0 @@ ---- -status: open -kind: finding -opened: 2026-10-08 ---- - -# A handler tells a slot it read before the slot changed hands - -Two interrupt handlers name a `pcidev` slot by a number and write that slot's -record a moment later, with nothing excluding a release in between: - -- the remapping fault handler reads the faulting stream's slot - (`owner_of`, `kernel/src/arch/x86_64/vtd/fault.rs`) and then calls - `pcidev::note_fault(slot)`; -- a claimed function's vector handler is its slot (`idt/user_dev.rs`) and - calls `pcidev::isr(slot)`. - -`pcidev::tear_down` clears the stream's slot and takes the remapping entry -away before the slot is free, so a handler that *starts* after a release names -nothing. One already past its read on another CPU still holds the number: if -the slot is released and bound to a second function before its write, that -function's claim is told a fault — every call on it refuses `Io` from then -on — or counts one interrupt its device did not raise. - -The interval is two adjacent statements of a handler running with interrupts -off, and a second function's bring-up in between is config-space writes and, -after a reset, a wait of at least the reset's settle time; no authority moves -either way. Nothing in the tree can hold a handler there, so this is read and -not run. - -**Exit condition**: the slot's record says which binding it is for and a -handler's write is refused by a record that has moved on, or this is folded -into `kernel/src/pcidev/mod.rs`'s header as the bound it is. - -**Owner**: none. No track covers a slot's record. diff --git a/issues/a-late-write-from-a-released-function-halts-the-machine.md b/issues/a-late-write-from-a-released-function-halts-the-machine.md index 80b27463406..9f80d465382 100644 --- a/issues/a-late-write-from-a-released-function-halts-the-machine.md +++ b/issues/a-late-write-from-a-released-function-halts-the-machine.md @@ -6,9 +6,9 @@ opened: 2026-10-10 # A late write from a released function halts the machine -`kernel/src/pcidev/mod.rs`'s `tear_down` clears the function's ownership -(`crate::iommu::note_user_owned(…, None)`) before its reset has quiesced what -it already issued: Bus Master Enable is cleared first, so it starts nothing +`kernel/src/pcidev/mod.rs`'s `release` clears the function's ownership +(`Function::held`, which `pcidev::note_fault` reads) at its end, before its +reset has quiesced what it already issued: Bus Master Enable is cleared first, so it starts nothing new, and its grants stay mapped until it is quiet, so nothing it issued lands in a page handed on. A transaction already in flight that faults after the ownership is cleared — at an address a grant never covered, or one an diff --git a/issues/a-released-function-never-claimed-again-keeps-its-retired-grants.md b/issues/a-released-function-never-claimed-again-keeps-its-retired-grants.md new file mode 100644 index 00000000000..a23fbdea6f6 --- /dev/null +++ b/issues/a-released-function-never-claimed-again-keeps-its-retired-grants.md @@ -0,0 +1,22 @@ +--- +status: open +kind: defect +opened: 2026-10-14 +--- + +# A released function never claimed again keeps its retired grants + +`kernel/src/pcidev/mod.rs`'s `release` resets a function whose holder ended +and parks that holder's grants in `Function::retired`, mapped in its domain +until the reset is quiet, so nothing the function had issued lands in a page +handed on. Only that function's next claim frees them (`finish_retired`). A +function reset, released and never claimed again keeps up to +`MAX_GRANT_TOTAL` (32 MiB) of grant pages for the rest of the boot. Across +the machine that is bounded only by the functions released and not reclaimed. + +Owner: `issues/a-device-function-is-driven-from-its-arrival-to-its-end.md`, +whose stage 2 ends a function's claim with `Gone`. + +Exit condition: a released function's retired grants are freed once its +reset is quiet, whether or not it is claimed again, and a test that releases +a reset-capable function and never claims it again sees the pages go back. diff --git a/issues/a-released-function-still-names-its-slots-domain.md b/issues/a-released-function-still-names-its-slots-domain.md deleted file mode 100644 index 7e10da87b20..00000000000 --- a/issues/a-released-function-still-names-its-slots-domain.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-03 ---- - -# A released function still names its slot's domain - -`pcidev::release` (`kernel/src/pcidev/mod.rs`, `tear_down`) clears Bus Master -Enable and resets the function, and leaves its context entry naming the slot's -domain (`kernel/src/arch/x86_64/vtd/table.rs`, `bind`, is reached from no -release). The slot's next holder is attached to that same domain -(`slot_space`), so the released function's requests translate through the -next holder's grants for as long as it is that holder's. Nothing but the -released function's own obedience to `BME` stands between them: a function -that masters the bus with `BME` clear reaches another process's memory. - -**Exit**: a released function's context entry names no domain once it is -quiet, read back off the unit's table on the T14 after its slot's next claim. diff --git a/issues/a-residue-slot-is-held-for-the-boot-by-a-function-never-claimed-again.md b/issues/a-residue-slot-is-held-for-the-boot-by-a-function-never-claimed-again.md deleted file mode 100644 index 5951f7a012d..00000000000 --- a/issues/a-residue-slot-is-held-for-the-boot-by-a-function-never-claimed-again.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-25 ---- - -# A residue slot is held for the boot by a function never claimed again - -`pcidev::release` keeps a function it could not reset in its slot as -`Slot::Residue` (`kernel/pci/src/slot.rs`), with the device addresses its last -holder's grants were at, so only that function's next claim is given the slot -and its grants are placed at those addresses. Nothing ends that hold except -that function's next claim. A function released without a reset and never -claimed again keeps one of `MAX_FUNCTIONS` slots for the rest of the boot: -no memory (the pages go back at the release), but a slot and a domain. - -A boot that swaps out a reset-less function's service and never swaps it back, -or a manifest that stops naming the function, gets there: with one such -residue and three held slots, a claim of a fourth function is refused -`Exhausted` while a slot drives nothing. - -Owner: the author of the residue (PR #492). Exit condition: a residue is given -up after a named, logged condition — a claim finding no free slot takes the -oldest residue slot and logs whose addresses it dropped, with the dropped -function detached from that domain first — and a guest test that leaves a -residue and then claims `MAX_FUNCTIONS` other functions is handed all of them. - -A second, narrower gap in the same neighbourhood (`kernel/src/pcidev/mod.rs`, -`tear_down`'s `None` arm): a successor takes `RESIDUE[slot]` into -`Bound::residue` (`take_residue`) and places a grant at a range it chooses to -use, but is never required to place one at every range. A range it never -places is still mapped — `map_at` is what unmaps a residue range's old leaf, -by placing a fresh one over it — and when *that* successor is torn down in -turn, only `bound.grants` is unmapped and returned to `RESIDUE[slot]`; -`bound.residue`'s untouched leftover is simply dropped with the rest of -`Bound`. The range stays mapped, in nobody's `RESIDUE` and nobody's `grants`, -until a third holder's mastering reaches whatever the first holder's -descriptors named there and takes the fault the design promises the first -holder's own dead reach, one generation late — the same shape `T14` run 132 -first showed. diff --git a/issues/toyos-runs-on-arm64.md b/issues/toyos-runs-on-arm64.md index 86c55424387..351ab118fee 100644 --- a/issues/toyos-runs-on-arm64.md +++ b/issues/toyos-runs-on-arm64.md @@ -367,7 +367,7 @@ Each stage names its exit; "measured" means a number from a run. before its body) get a test here that reds with `put` written back as one `self.buf.write(off, trb)`; x86's TSO hides all three from every guest test until then. - A claimed function's message is its claim slot's LPI through the GICv3 + A claimed function's message is its function's LPI through the GICv3 ITS (`kernel/src/arch/aarch64/irqchip/its.rs`), which `Arch::irqchip` gives `cargo run`'s machine and `Profile::VirtIts`, on QEMU's own GIC under HVF, whose in-kernel vGIC refuses an ITS. QEMU's SMMUv3 @@ -378,16 +378,16 @@ Each stage names its exit; "measured" means a number from a run. the claim the IORT routes past the SMMUv3 and a fault in a claimed BAR. `irq()` holds the preempt count across every device arm, as x86-64's `device_irq_entry` does, and `pcidev::isr` asserts it. - A released function's stream stays on its slot's domain, which maps + A released function's stream stays on its own domain, which maps the doorbell; its bus mastering off and its message masked are what stop its writes, and a write it makes regardless reaches the ITS, where the release's `DISCARD` and `MAPD` with `V` clear leave its DeviceID translating nothing. That last is read, not tested: no function QEMU offers writes once mastering is off but `iommu-testdev`, which no claim can tell to write. **Owed**: a `smmu-selftest` arm that puts an - `iommu-testdev` on a domain, maps its DeviceID to a slot and unmaps it as + `iommu-testdev` on a domain, maps its DeviceID to an LPI and unmaps it as a release does, has it write event 0 to `GITS_TRANSLATER`, and is red - when the slot's LPI is raised. + when that LPI is raised. 7. **Userland boots.** `init`, `logd`, the compositor, netd, soundd and sshd, built for `aarch64-unknown-toyos`. Every program of `system.toml` builds diff --git a/kernel/pci/src/lib.rs b/kernel/pci/src/lib.rs index 265422929da..8d28427a5c3 100644 --- a/kernel/pci/src/lib.rs +++ b/kernel/pci/src/lib.rs @@ -39,6 +39,5 @@ pub mod msix; pub mod placement; pub mod pm; pub mod probe; -pub mod slot; pub use choose::{claimed, console, Unclaimable}; diff --git a/kernel/pci/src/slot.rs b/kernel/pci/src/slot.rs deleted file mode 100644 index 68b1393a13e..00000000000 --- a/kernel/pci/src/slot.rs +++ /dev/null @@ -1,97 +0,0 @@ -//! Which claim each of `pcidev`'s slots is for: the decision alone, over the -//! array the kernel's slot lock guards. -//! -//! **A slot a function left a residue in is that function's alone.** Its -//! domain keeps the device addresses the function was left aimed at, and the -//! function stays attached to it; a second function given that slot would be -//! handed those addresses and share the domain with a part that may still -//! reach it. So [`reserve`] answers a requester its own residue slot first, a -//! free slot next, and never another requester's residue. - -/// Who a slot is for. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum Slot { - Free, - /// A claim on this requester holds it. - Held(u16), - /// Nobody holds it, and its domain keeps the addresses this requester's - /// function was left aimed at. - Residue(u16), -} - -/// Why a requester is given no slot. -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum Refused { - /// A claim on this requester holds a slot already. - Owned, - /// Every slot is held, or is another function's residue. - Exhausted, -} - -/// Take a slot for `who`: the one its residue is in, else a free one. -pub fn reserve(slots: &mut [Slot], who: u16) -> Result { - if slots.contains(&Slot::Held(who)) { - return Err(Refused::Owned); - } - let slot = slots - .iter() - .position(|slot| *slot == Slot::Residue(who)) - .or_else(|| slots.iter().position(|slot| *slot == Slot::Free)) - .ok_or(Refused::Exhausted)?; - slots[slot] = Slot::Held(who); - Ok(slot) -} - -/// Give a slot up: its function's alone while it left a residue, free -/// otherwise. A slot that was not held — a hand-over refused before it bound -/// anything — is free. -pub fn release(slots: &mut [Slot], slot: usize, residue: bool) { - slots[slot] = match slots[slot] { - Slot::Held(who) if residue => Slot::Residue(who), - _ => Slot::Free, - }; -} - -#[cfg(test)] -mod tests { - use super::*; - - const NIC: u16 = 0x0010; - const OTHER: u16 = 0x0018; - - #[test] - fn a_second_function_takes_a_free_slot_and_never_anothers_residue() { - let mut slots = [Slot::Residue(NIC), Slot::Free, Slot::Free, Slot::Free]; - assert_eq!(reserve(&mut slots, OTHER), Ok(1)); - assert_eq!(slots[0], Slot::Residue(NIC), "another function's claim took the residue's slot"); - } - - #[test] - fn a_function_goes_back_to_its_own_residue_ahead_of_a_free_slot() { - let mut slots = [Slot::Free, Slot::Residue(OTHER), Slot::Residue(NIC), Slot::Free]; - assert_eq!(reserve(&mut slots, NIC), Ok(2)); - assert_eq!(slots[2], Slot::Held(NIC)); - } - - #[test] - fn other_functions_residues_are_no_free_slot() { - let mut slots = [Slot::Residue(NIC), Slot::Held(3), Slot::Residue(4), Slot::Held(5)]; - assert_eq!(reserve(&mut slots, OTHER), Err(Refused::Exhausted)); - assert_eq!(slots, [Slot::Residue(NIC), Slot::Held(3), Slot::Residue(4), Slot::Held(5)]); - } - - #[test] - fn a_held_function_is_owned() { - let mut slots = [Slot::Free, Slot::Held(NIC), Slot::Free, Slot::Free]; - assert_eq!(reserve(&mut slots, NIC), Err(Refused::Owned)); - } - - #[test] - fn a_release_keeps_the_slot_only_for_a_residue() { - let mut slots = [Slot::Held(NIC), Slot::Held(OTHER), Slot::Free, Slot::Free]; - release(&mut slots, 0, true); - release(&mut slots, 1, false); - release(&mut slots, 2, true); - assert_eq!(slots, [Slot::Residue(NIC), Slot::Free, Slot::Free, Slot::Free]); - } -} diff --git a/kernel/pure/lib.rs b/kernel/pure/lib.rs index f076c3c74df..8baacf429cb 100644 --- a/kernel/pure/lib.rs +++ b/kernel/pure/lib.rs @@ -14,6 +14,7 @@ pub mod clock; pub mod cpu; pub mod dirnodes; pub mod mtrr; +pub mod owners; pub mod pcid; pub mod pipe; pub mod pointers; diff --git a/kernel/pure/owners.rs b/kernel/pure/owners.rs new file mode 100644 index 00000000000..57852e21511 --- /dev/null +++ b/kernel/pure/owners.rs @@ -0,0 +1,101 @@ +//! Who holds each of `N` numbered things an interrupt is delivered through — a +//! CPU's vectors, a remapping table's entries: the lowest free number is taken, +//! given back by number, and read by number from an interrupt handler with no +//! lock. Running out is the machine's limit, answered as `None`. + +use core::sync::atomic::{AtomicU32, Ordering}; + +/// The word a free number holds; a held one holds its owner plus one. +const FREE: u32 = 0; + +pub struct Owners([AtomicU32; N]); + +impl Owners { + pub const fn new() -> Self { + Self([const { AtomicU32::new(FREE) }; N]) + } + + /// The lowest free number, now `owner`'s; `None` while all `N` are held. + pub fn take(&self, owner: u32) -> Option { + let held = owner.checked_add(1).expect("an owner is below u32::MAX"); + self.0 + .iter() + .position(|word| word.compare_exchange(FREE, held, Ordering::AcqRel, Ordering::Relaxed).is_ok()) + } + + /// `number`, which [`Self::take`] answered, is free again. + pub fn give_back(&self, number: usize) { + let was = self.0[number].swap(FREE, Ordering::AcqRel); + assert!(was != FREE, "number {number} given back while free"); + } + + /// Who holds `number`: `None` while it is free, and for one past `N`. + pub fn owner(&self, number: usize) -> Option { + match self.0.get(number)?.load(Ordering::Acquire) { + FREE => None, + held => Some(held - 1), + } + } +} + +impl Default for Owners { + fn default() -> Self { + Self::new() + } +} + +#[cfg(test)] +mod tests { + use super::Owners; + use std::sync::Arc; + use std::vec::Vec; + + #[test] + fn the_lowest_free_number_is_taken_and_names_its_owner() { + let owners = Owners::<4>::new(); + assert_eq!((owners.take(7), owners.take(0), owners.take(7)), (Some(0), Some(1), Some(2))); + assert_eq!((0..5).map(|n| owners.owner(n)).collect::>(), [Some(7), Some(0), Some(7), None, None]); + } + + #[test] + fn a_number_given_back_is_the_next_one_taken() { + let owners = Owners::<4>::new(); + (0..3).for_each(|owner| assert!(owners.take(owner).is_some())); + owners.give_back(1); + assert_eq!(owners.owner(1), None); + assert_eq!(owners.take(9), Some(1)); + assert_eq!(owners.take(9), Some(3)); + } + + #[test] + fn every_number_held_refuses_and_changes_nothing() { + let owners = Owners::<2>::new(); + assert_eq!((owners.take(1), owners.take(2)), (Some(0), Some(1))); + assert_eq!(owners.take(3), None); + assert_eq!((owners.owner(0), owners.owner(1)), (Some(1), Some(2))); + } + + #[test] + fn racing_takers_are_given_distinct_numbers() { + const N: usize = 64; + let owners = Arc::new(Owners::::new()); + let takers: Vec<_> = (0..8u32) + .map(|owner| { + let owners = Arc::clone(&owners); + std::thread::spawn(move || (0..N).filter_map(|_| owners.take(owner)).collect::>()) + }) + .collect(); + let mut taken: Vec = takers.into_iter().flat_map(|t| t.join().expect("a taker")).collect(); + taken.sort_unstable(); + assert_eq!(taken, (0..N).collect::>()); + } + + #[test] + #[should_panic(expected = "number 1 given back while free")] + fn a_number_given_back_twice_panics() { + let owners = Owners::<2>::new(); + (0..2).for_each(|owner| assert!(owners.take(owner).is_some())); + owners.give_back(1); + owners.give_back(1); + } +} diff --git a/kernel/src/arch/aarch64/irqchip.rs b/kernel/src/arch/aarch64/irqchip.rs index 3f2e3709138..304bade314c 100644 --- a/kernel/src/arch/aarch64/irqchip.rs +++ b/kernel/src/arch/aarch64/irqchip.rs @@ -5,8 +5,8 @@ //! GTDT names. //! //! **What this kernel takes is SGIs, the timer's PPI, the IOMMU's event SPI -//! ([`route_iommu_events`]) and one LPI per claim slot ([`its`]), and nothing -//! else.** No other SPI is routed: a device's interrupt is a message the ITS +//! ([`route_iommu_events`]) and one LPI per claimed function ([`its`]), and +//! nothing else.** No other SPI is routed: a device's interrupt is a message the ITS //! translates, and only a claimed function's is. //! //! **The virtual timer, not the physical**: it is the one EL1 owns outright — diff --git a/kernel/src/arch/aarch64/irqchip/its.rs b/kernel/src/arch/aarch64/irqchip/its.rs index 0de1b110714..162fdba89be 100644 --- a/kernel/src/arch/aarch64/irqchip/its.rs +++ b/kernel/src/arch/aarch64/irqchip/its.rs @@ -1,13 +1,17 @@ //! The GICv3 Interrupt Translation Service (IHI 0069H.b chapter 5), driven //! with `toyos_its`'s encodings: what turns a claimed function's message into -//! that claim's own LPI, and into no other. +//! that function's own LPI, and into no other. //! //! **A message is translated by the DeviceID of the function that wrote it**, //! which the bus says and the function cannot (§5.1, §5.2.1): a claim's //! function is mapped to a translation table of its own (`MAPD`) whose one -//! event is its slot's LPI (`MAPTI`), so whatever a function writes to the -//! doorbell, it raises its own slot's LPI or nothing. A DeviceID no claim -//! holds is mapped to no table, and its writes are dropped (§5.3.10). +//! event is its function's LPI (`MAPTI`), so whatever a function writes to the +//! doorbell, it raises its own LPI or nothing. A DeviceID no claim holds is +//! mapped to no table, and its writes are dropped (§5.3.10). +//! +//! **`pcidev` function `n`'s LPI is INTID 8192 + `n`**, so the LPIs bound the +//! functions a claim can be given one to, and the handler finds the function +//! by arithmetic. //! //! **Every LPI is collection 0's, on the boot CPU's redistributor**, as every //! claimed function's interrupt on x86-64 is cpu0's. @@ -15,8 +19,8 @@ //! **What the ITS and the redistributor read is handed over before it is //! named:** each table is written whole through [`Mmio`], whose write orders //! it before the register write or command that names it, and the -//! configuration of every claim slot's LPI is written, enabled, before the -//! redistributor's `EnableLPIs`, so no `INV` is owed (§5.1.1). +//! configuration of every LPI is written, enabled, before the redistributor's +//! `EnableLPIs`, so no `INV` is owed (§5.1.1). //! //! An ITS this kernel cannot drive is left disabled, and every claim is //! refused on its account ([`is_armed`]); a refusal found once the @@ -40,7 +44,6 @@ use crate::log; use crate::mm::pmm::{self, PhysPage}; use crate::mm::policy::MmioPolicy; use crate::mm::{DirectMap, Mmio, PAGE_2M}; -use crate::pcidev::MAX_FUNCTIONS; use crate::sync::Lock; /// A command is consumed within a second, and a register settles in one: @@ -59,7 +62,7 @@ const GICR_CTLR: u64 = 0x0000; const GICR_TYPER: u64 = 0x0008; /// INTID bits the LPI tables are laid out for: the least that names an LPI -/// (§5.1.1), 8192 of them, which no claim slot count comes near. +/// (§5.1.1), 8192 of them. const ID_BITS: u8 = 14; /// The widest DeviceID a claim is given: a PCI requester ID's, which is what @@ -72,8 +75,8 @@ const EVENT_BITS: u8 = 1; /// A claim's one event: what its message carries as data. const EVENT: u32 = 0; -/// Every LPI a claim slot raises runs at the one priority every SGI and PPI -/// does (`super::PRIORITY`). +/// Every LPI a claim raises runs at the one priority every SGI and PPI does +/// (`super::PRIORITY`). const PRIORITY: u8 = super::PRIORITY; /// `GITS_BASER`'s `Type` [58:56] and `Entry_Size` [52:48], which the ITS @@ -101,18 +104,30 @@ struct Live { target: Target, collection: Collection, events: EventBits, - /// Each claim slot's translation table, and the DeviceID mapped to it - /// while the slot is claimed. - itts: [Phys<8>; MAX_FUNCTIONS], - held: [Option; MAX_FUNCTIONS], - /// Each slot's LPI. - lpis: [Lpi; MAX_FUNCTIONS], - /// The pages every table is in: never given back, since the ITS and the - /// redistributor may read any of them for the machine's life. - _pages: Vec, + space: lpi::Space, + /// Each DeviceID's translation table, made on its first claim and kept. + itts: Vec<(u32, Phys<8>)>, + itt_bytes: u64, + /// The DeviceIDs a claim has mapped. + held: Vec, + /// What every table is in: never given back, since the ITS and the + /// redistributor may read any of it for the machine's life. + memory: Memory, } impl Live { + /// `device`'s translation table, made on its first claim: `None` where + /// there is no memory for one. + fn table_of(&mut self, device: u32) -> Option> { + if let Some((_, table)) = self.itts.iter().find(|(held, _)| *held == device) { + return Some(*table); + } + let at = self.memory.alloc(self.itt_bytes, ITT_ALIGN)?; + let table = Phys::<8>::new(at).expect("256-byte aligned below 2^48"); + self.itts.push((device, table)); + Some(table) + } + /// `commands`, then a `SYNC` to the boot CPU's redistributor, consumed: /// every one has taken effect when this returns (§5.3.15). fn issue(&mut self, commands: &[Command]) { @@ -149,16 +164,17 @@ struct Memory { } impl Memory { - fn alloc(&mut self, bytes: u64, align: u64) -> u64 { + /// `None` where the allocator has no page left. + fn alloc(&mut self, bytes: u64, align: u64) -> Option { assert!(align.is_power_of_two() && bytes <= PAGE_2M, "ITS: {bytes:#x} bytes aligned to {align:#x}"); let mut at = self.used.next_multiple_of(align); if self.pages.is_empty() || at + bytes > PAGE_2M { // Zeroed by the allocator. - self.pages.push(pmm::alloc_page().expect("ITS: no memory for its tables")); + self.pages.push(pmm::alloc_page()?); at = 0; } self.used = at + bytes; - self.pages.last().expect("a page was just pushed").direct_map().phys() + at + Some(self.pages.last().expect("a page was just pushed").direct_map().phys() + at) } } @@ -169,17 +185,11 @@ fn window(phys: u64, bytes: u64) -> Mmio { unsafe { Mmio::over_phys(DirectMap::from_phys(phys), bytes) } } -/// What claim slot `slot`'s message raises, for the record. -pub(crate) fn slot_interrupt(slot: usize) -> impl core::fmt::Display { - let intid = lpi::FIRST + slot as u32; - alloc::format!("LPI {intid}") -} - -/// The claim slot LPI `intid` is raised for, for the interrupt's handler: -/// `None` for any INTID no slot owns. -pub(in crate::arch::aarch64) fn slot_of(intid: u32) -> Option { - let slot = usize::try_from(intid.checked_sub(lpi::FIRST)?).ok()?; - (slot < MAX_FUNCTIONS).then_some(slot) +/// The `pcidev` function LPI `intid` is raised for, for the interrupt's +/// handler: `None` for an INTID that is no LPI. +pub(in crate::arch::aarch64) fn function_of(intid: u32) -> Option { + let function = intid.checked_sub(lpi::FIRST)?; + (function < (1 << ID_BITS) - lpi::FIRST).then_some(function as usize) } /// Bring the one ITS the MADT names up, on the boot CPU's redistributor at @@ -201,9 +211,8 @@ pub(super) fn init(named: &[(u32, u64)], frame: u64) { Ok(live) => { log!( "ITS: {id} at {base:#x} armed: doorbell {:#x}, collection 0 on the boot CPU's redistributor at {frame:#x}, \ - LPIs {:?} for the claim slots", + every LPI enabled", live.doorbell, - live.lpis.map(|lpi| lpi.intid()), ); *ITS.lock() = Some(live); } @@ -248,9 +257,10 @@ fn bring_up(id: u32, base: u64, frame: u64) -> Result` that asks for a table this kernel needs is given @@ -272,7 +282,7 @@ fn bring_up(id: u32, base: u64, frame: u64) -> Result PAGE_2M { return Err(format!("GITS_BASER{n}'s table of {entries} entries is {bytes:#x} bytes, past a page")); } - let table = Phys::<16>::new(memory.alloc(bytes, 1 << 16)).expect("a 64 KiB-aligned page below 2^48"); + let table = Phys::<16>::new(alloc(bytes, 1 << 16)?).expect("a 64 KiB-aligned page below 2^48"); let value = backing.baser(table, pages).expect("pages inside the field"); regs.write_u64(at, value); let read = regs.read_u64(at); @@ -307,12 +317,10 @@ fn bring_up(id: u32, base: u64, frame: u64) -> Result Result::new(memory.alloc(itt_bytes, ITT_ALIGN)).expect("256-byte aligned below 2^48")); let mut live = Live { regs, id, @@ -350,10 +356,11 @@ fn bring_up(id: u32, base: u64, frame: u64) -> Result bool { + matches!(self, Self::NoLpi(_) | Self::NoMemory) + } } impl core::fmt::Display for Refused { @@ -409,11 +428,17 @@ impl core::fmt::Display for Refused { "the IORT gives it DeviceID {id:#x}, which another claimed function's messages \ are already translated by" ), + Self::NoLpi(function) => write!( + f, + "function {function}'s LPI {} is past the redistributor's LPIs", + lpi::FIRST as usize + function + ), + Self::NoMemory => write!(f, "there is no memory for its translation table"), } } } -/// A driver in this kernel is given no LPI: only a claim slot's function is +/// A driver in this kernel is given no LPI: only a claimed function is /// mapped, and a driver in this kernel on this machine has no address space /// to do DMA in (`super::super::iommu_unit`). pub fn msi(_source: StreamId, _irq: crate::arch::DriverIrq) -> Result { @@ -431,36 +456,58 @@ fn device_of(live: &Live, routed: Option) -> Result Result { +/// A claimed function's DeviceID and the LPI its message raises, held until +/// [`release`]. +pub struct Claimed { + pub msi: Msi, + device: u32, + lpi: Lpi, +} + +impl Claimed { + /// What the message raises, for the record. + pub fn interrupt(&self) -> impl core::fmt::Display { + alloc::format!("LPI {}", self.lpi.intid()) + } +} + +/// Map `source`'s DeviceID to its translation table, its one event to `pcidev` +/// function `function`'s LPI, and answer the message that raises it. +pub fn claim(function: usize, source: StreamId) -> Result { // Before this lock, so it never nests with the unit's. let routed = super::super::smmu::its_device(source); let mut held = ITS.lock(); - let live = held.as_mut().unwrap_or_else(|| panic!("ITS: claim slot {slot} mapped with no ITS armed")); - assert!(live.held[slot].is_none(), "ITS: claim slot {slot} is still mapped"); + let live = held.as_mut().unwrap_or_else(|| panic!("ITS: {source} claimed with no ITS armed")); let device = device_of(live, routed)?; - if live.held.contains(&Some(device)) { + if live.held.contains(&device) { return Err(Refused::DeviceIdHeld(device)); } + let lpi = u32::try_from(function) + .ok() + .and_then(|function| lpi::FIRST.checked_add(function)) + .and_then(|intid| live.space.lpi(intid)) + .ok_or(Refused::NoLpi(function))?; + let table = live.table_of(device).ok_or(Refused::NoMemory)?; let event = live.events.event(EVENT).expect("event 0 is in every table"); - let (table, lpi, collection, events) = (live.itts[slot], live.lpis[slot], live.collection, live.events); + let (collection, events) = (live.collection, live.events); live.issue(&[ Command::MapDevice { device, table, events }, Command::MapEvent { device, event, lpi, collection }, ]); - live.held[slot] = Some(device); - log!("ITS: {source}, DeviceID {device:#x}, event {EVENT} is slot {slot}'s LPI {}", lpi.intid()); - Ok(Msi { address: live.doorbell, data: EVENT }) + live.held.push(device); + log!("ITS: {source}, DeviceID {device:#x}, event {EVENT} is LPI {}", lpi.intid()); + Ok(Claimed { msi: Msi { address: live.doorbell, data: EVENT }, device, lpi }) } -/// Claim slot `slot`'s DeviceID unmapped, and anything it left pending -/// dropped, before this returns. -pub fn release(slot: usize, source: StreamId) { +/// A claim's DeviceID unmapped, and anything it left pending dropped, before +/// this returns. +pub fn release(claimed: &Claimed, source: StreamId) { + let device = claimed.device; let mut held = ITS.lock(); - let live = held.as_mut().unwrap_or_else(|| panic!("ITS: claim slot {slot} released with no ITS armed")); - let device = live.held[slot].take().unwrap_or_else(|| panic!("ITS: claim slot {slot} released unmapped")); + let live = held.as_mut().unwrap_or_else(|| panic!("ITS: {source} released with no ITS armed")); + let at = live.held.iter().position(|held| *held == device); + live.held.swap_remove(at.unwrap_or_else(|| panic!("ITS: DeviceID {device:#x} released unmapped"))); let event = live.events.event(EVENT).expect("event 0 is in every table"); live.issue(&[Command::Discard { device, event }, Command::UnmapDevice { device }]); - log!("ITS: {source}, DeviceID {device:#x}, unmapped from slot {slot}"); + log!("ITS: {source}, DeviceID {device:#x}, unmapped from LPI {}", claimed.lpi.intid()); } diff --git a/kernel/src/arch/aarch64/mod.rs b/kernel/src/arch/aarch64/mod.rs index fe6c745360e..c66b6296c77 100644 --- a/kernel/src/arch/aarch64/mod.rs +++ b/kernel/src/arch/aarch64/mod.rs @@ -39,7 +39,6 @@ pub mod trap; pub mod watchdog; pub use irqchip::its as message_unit; -pub(crate) use irqchip::its::slot_interrupt; pub use smmu as iommu_unit; pub use trap::DriverIrq; diff --git a/kernel/src/arch/aarch64/trap.rs b/kernel/src/arch/aarch64/trap.rs index 0cef71f72f6..ad28aa45a7b 100644 --- a/kernel/src/arch/aarch64/trap.rs +++ b/kernel/src/arch/aarch64/trap.rs @@ -227,10 +227,10 @@ fn irq(frame: &Frame, from_el0: bool) -> bool { crate::iommu::fault_interrupt(); false } - _ => match irqchip::its::slot_of(intid) { - Some(slot) => { + _ => match irqchip::its::function_of(intid) { + Some(function) => { percpu::irq_took(Source::UserDev); - crate::pcidev::isr(slot); + crate::pcidev::isr(function); true } None => { diff --git a/kernel/src/arch/x86_64/idt/device_irq.rs b/kernel/src/arch/x86_64/idt/device_irq.rs index 6ab3bd56712..9f2824fb654 100644 --- a/kernel/src/arch/x86_64/idt/device_irq.rs +++ b/kernel/src/arch/x86_64/idt/device_irq.rs @@ -6,8 +6,18 @@ //! `kernel_exit_to_user_check`'s IF=0-on-entry contract without an explicit cli. /// Defines a naked device-interrupt entry point running `$handler` then the deferred-preempt epilogue. +/// +/// The `numbered` form is jumped to by a stub that pushed one word above the +/// CPU's frame, which `$handler` takes as its argument and the exit drops. macro_rules! device_irq_entry { ($(#[$meta:meta])* $vis:vis fn $name:ident => $handler:path) => { + device_irq_entry!(@entry $(#[$meta])* $vis fn $name => $handler; 88; []; []); + }; + ($(#[$meta:meta])* $vis:vis fn $name:ident => $handler:path, numbered) => { + device_irq_entry!(@entry $(#[$meta])* $vis fn $name => $handler; 96; ["mov rdi, [rbp + 80]"]; ["add rsp, 8"]); + }; + // `$cs` is the CS word's offset from `rbp`: 10 GPRs and RIP, and the stub's word if there is one. + (@entry $(#[$meta:meta])* $vis:vis fn $name:ident => $handler:path; $cs:literal; [$($argument:literal)?]; [$($drop:literal)?]) => { $(#[$meta])* #[unsafe(naked)] $vis extern "sysv64" fn $name() { @@ -29,13 +39,14 @@ macro_rules! device_irq_entry { "and rsp, -16", #[cfg(feature = "mask-windows")] "call {windows_entered}", + $($argument,)? "call {handler}", #[cfg(feature = "mask-windows")] - "mov rdi, [rbp + 88]", + "mov rdi, [rbp + {cs}]", #[cfg(feature = "mask-windows")] "call {windows_leaving}", "sub dword ptr gs:[{preempt_count}], 1", - "test dword ptr [rbp + 88], 3", // CS = 10 GPRs + RIP above + "test dword ptr [rbp + {cs}], 3", "jz 1f", "call {exit_to_user}", "1:", @@ -50,8 +61,10 @@ macro_rules! device_irq_entry { "pop rdx", "pop rcx", "pop rax", + $($drop,)? "iretq", handler = sym $handler, + cs = const $cs, exit_to_user = sym crate::arch::idt::kernel_exit_to_user_check, preempt_count = const $crate::arch::percpu::OFF_PREEMPT_COUNT, #[cfg(feature = "mask-windows")] diff --git a/kernel/src/arch/x86_64/idt/mod.rs b/kernel/src/arch/x86_64/idt/mod.rs index 684e4af299f..d7af9067757 100644 --- a/kernel/src/arch/x86_64/idt/mod.rs +++ b/kernel/src/arch/x86_64/idt/mod.rs @@ -9,10 +9,10 @@ pub(crate) mod spurious; mod timer; mod tlb; pub(crate) mod unclaimed; -mod user_dev; +pub(crate) mod user_dev; mod xhci; -pub(crate) use user_dev::{claim_vector, slot_interrupt}; +pub(crate) use user_dev::ClaimVector; use core::arch::naked_asm; @@ -92,7 +92,11 @@ impl IdtEntry { /// An interrupt gate onto `entry`. fn at(entry: unsafe extern "sysv64" fn()) -> Self { - let handler = entry as *const () as u64; + Self::at_address(entry as *const () as u64) + } + + /// An interrupt gate onto the code at `handler`. + fn at_address(handler: u64) -> Self { Self { offset_low: handler as u16, selector: 0x08, // kernel CS @@ -253,12 +257,6 @@ idt_vectors! { Xhci = 0x21, xhci::xhci_entry; DmaFault = 0x25, dma_fault::dma_fault_entry; Hda = 0x26, hda::hda_entry; - // One per `pcidev` claim slot: the vector is how the kernel knows - // which claim a message belongs to. - UserDev0 = 0x28, user_dev::user_dev0_entry; - UserDev1 = 0x29, user_dev::user_dev1_entry; - UserDev2 = 0x2A, user_dev::user_dev2_entry; - UserDev3 = 0x2B, user_dev::user_dev3_entry; Isa0 = 0x2C, isa::isa0_entry; Isa1 = 0x2D, isa::isa1_entry; Kick = 0xFC, kick::kick_entry; @@ -450,6 +448,7 @@ pub fn init() { disable_pic(); install_gates(&mut IDT.lock()); + user_dev::install(&mut IDT.lock()); // Every slot no row filled: delivery through a P = 0 gate is a // contributory fault, and the machine would halt as #DF with no name. let mut unclaimed = 0u32; diff --git a/kernel/src/arch/x86_64/idt/unclaimed.rs b/kernel/src/arch/x86_64/idt/unclaimed.rs index c371a8f3404..cba44587205 100644 --- a/kernel/src/arch/x86_64/idt/unclaimed.rs +++ b/kernel/src/arch/x86_64/idt/unclaimed.rs @@ -1,5 +1,6 @@ //! Every vector no `idt_vectors!` row claims — one gate that counts and names //! it, since a `P = 0` slot escalates to `#DF` and halts the machine namelessly. +//! A claim vector no claim holds is counted here too ([`count`]). //! //! The EOI is conditional, as the spurious handler's is (SDM Vol. 3A §11.9): a //! genuinely spurious delivery sets no ISR bit, so the handler asks before it @@ -61,13 +62,13 @@ extern "sysv64" fn took() { crate::windows::irqs_masked(); crate::windows::handler_entered(); } - crate::arch::percpu::irq_took!(Unclaimed); match apic::in_service_highest() { Some(vector) => { - TAKEN[(vector >> 6) as usize].fetch_or(1 << (vector & 63), Ordering::Relaxed); + count(vector); apic::eoi(); } None => { + crate::arch::percpu::irq_took!(Unclaimed); NO_ISR.fetch_add(1, Ordering::Relaxed); } } @@ -78,6 +79,13 @@ extern "sysv64" fn took() { } } +/// A delivery on `vector` that nothing here handles, counted and remembered; +/// the caller acknowledges it. +pub(super) fn count(vector: u8) { + crate::arch::percpu::irq_took!(Unclaimed); + TAKEN[(vector >> 6) as usize].fetch_or(1 << (vector & 63), Ordering::Relaxed); +} + /// Whether `vector` has been taken through this gate. #[cfg(feature = "boot-actuators")] pub fn was_taken(vector: u8) -> bool { diff --git a/kernel/src/arch/x86_64/idt/user_dev.rs b/kernel/src/arch/x86_64/idt/user_dev.rs index f26206dfcb3..ceea39e8e66 100644 --- a/kernel/src/arch/x86_64/idt/user_dev.rs +++ b/kernel/src/arch/x86_64/idt/user_dev.rs @@ -1,68 +1,104 @@ -//! The vectors a PCI function a *process* drives delivers on. +//! The vectors a PCI function a *process* drives delivers on: one taken by +//! each claim and given back at its release, from cpu0's vectors this kernel +//! declares nothing on. //! -//! One per claim slot, because the slot is what the kernel needs to know: the -//! count goes to that slot's own record, and the wake it earns goes to that -//! slot's watchers and nobody else's. A single shared vector would -//! wake every user driver in the machine on any of their interrupts, which is -//! one process learning when another's device is busy. +//! One per claim, because the vector is what the kernel knows a message by: +//! the count goes to that function's own record, and the wake it earns goes to +//! its watchers and nobody else's. A single shared vector would wake every +//! user driver in the machine on any of their interrupts, which is one process +//! learning when another's device is busy. //! -//! Heap-free like every other device entry here: the record is atomics, and -//! the claim's watch is posted from the handler. +//! Heap-free like every other device entry here: each vector's stub pushes its +//! number, the record is atomics, and the claim's watch is posted from the +//! handler. + +use kernel::owners::Owners; use super::device_irq::device_irq_entry; -use super::Vector; +use super::{Idt, IdtEntry}; + +/// The claims' vectors, from here up to the IPIs at 0xFC: `idt_vectors!` +/// declares its device vectors below. +const FIRST: u8 = 0x40; +const COUNT: usize = 0xFC - FIRST as usize; + +/// Each stub's bytes: a push and a jump fit in 10. +const STRIDE: usize = 16; -/// The vector each claim slot's message carries, by slot: the vector is how -/// the kernel knows which claim a message belongs to. -const VECTORS: [Vector; crate::pcidev::MAX_FUNCTIONS] = - [Vector::UserDev0, Vector::UserDev1, Vector::UserDev2, Vector::UserDev3]; +/// The `pcidev` function each vector is held for. +static HELD: Owners = Owners::new(); -/// The vector claim slot `slot`'s remapping entry delivers. -pub(crate) fn claim_vector(slot: usize) -> u8 { - VECTORS[slot] as u8 +/// A claim's vector, held until [`give_back`]. +#[derive(Clone, Copy)] +pub(crate) struct ClaimVector(u8); + +impl ClaimVector { + pub(crate) fn vector(self) -> u8 { + self.0 + } } -/// What claim slot `slot`'s message raises, for the record. -pub(crate) fn slot_interrupt(slot: usize) -> impl core::fmt::Display { - super::DriverIrq(VECTORS[slot]) +impl core::fmt::Display for ClaimVector { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "vector {:#04x}", self.0) + } } -fn took(slot: usize) { - crate::arch::percpu::irq_took!(UserDev); - crate::pcidev::isr(slot); - crate::preempt::set_need_resched(); - crate::arch::apic::eoi(); +/// A vector for `pcidev` function `function`, or `None` once every one is held. +pub(crate) fn take(function: usize) -> Option { + let owner = u32::try_from(function).expect("a function number fits a u32"); + HELD.take(owner).map(|number| ClaimVector(FIRST + number as u8)) } -/// One handler and one entry per slot. A macro because the slot has to be an -/// immediate in the handler rather than a value read from somewhere: an ISR -/// that had to look up which device it was would be taking a lock. -macro_rules! user_dev_vectors { - ($($handler:ident, $entry:ident => $slot:literal;)+) => { - $( - extern "sysv64" fn $handler() { - took($slot); - } +/// Free `vector` again, once nothing can raise it for the claim it was taken for. +pub(crate) fn give_back(vector: ClaimVector) { + HELD.give_back(usize::from(vector.0 - FIRST)); +} - device_irq_entry! { - /// A claimed PCI function's MSI-X entry (see `device_irq_entry` - /// for the asm contract). - pub(super) fn $entry => $handler - } - )+ +/// Every claim vector's gate onto its stub. +pub(super) fn install(idt: &mut Idt) { + let first = (stubs as *const () as usize).next_multiple_of(STRIDE); + for number in 0..COUNT { + let entry = &mut idt.entries[usize::from(FIRST) + number]; + assert!(entry.type_attr == 0, "idt: vector {:#04x} is declared and a claim vector", usize::from(FIRST) + number); + *entry = IdtEntry::at_address((first + number * STRIDE) as u64); + } +} - /// As many entries as `pcidev` has slots; a mismatch would be a claim - /// whose vector nothing answers. - const _: () = assert!( - [$($slot),+].len() == crate::pcidev::MAX_FUNCTIONS, - "every claim slot needs a vector of its own", - ); - }; +/// `COUNT` stubs, `STRIDE` apart from the first `STRIDE` boundary: each pushes +/// its number and jumps to [`claim_entry`]. Never called. +#[unsafe(naked)] +extern "sysv64" fn stubs() { + core::arch::naked_asm!( + ".set toyos_claim_vector, 0", + ".rept {count}", + ".balign {stride}", + "pushq $toyos_claim_vector", + "jmp {entry}", + ".set toyos_claim_vector, toyos_claim_vector + 1", + ".endr", + count = const COUNT, + stride = const STRIDE, + entry = sym claim_entry, + options(att_syntax), + ); +} + +extern "sysv64" fn took(number: u64) { + match HELD.owner(number as usize) { + Some(function) => { + crate::arch::percpu::irq_took!(UserDev); + crate::pcidev::isr(function as usize); + crate::preempt::set_need_resched(); + } + // A message its claim's release had not yet stopped before the vector + // was given back. + None => super::unclaimed::count(FIRST + number as u8), + } + crate::arch::apic::eoi(); } -user_dev_vectors! { - user_dev0_handler, user_dev0_entry => 0; - user_dev1_handler, user_dev1_entry => 1; - user_dev2_handler, user_dev2_entry => 2; - user_dev3_handler, user_dev3_entry => 3; +device_irq_entry! { + /// Every claim vector's stub ends here (see `device_irq_entry` for the asm contract). + fn claim_entry => took, numbered } diff --git a/kernel/src/arch/x86_64/mod.rs b/kernel/src/arch/x86_64/mod.rs index bdc25b5b815..1fe1b12ce8c 100644 --- a/kernel/src/arch/x86_64/mod.rs +++ b/kernel/src/arch/x86_64/mod.rs @@ -50,7 +50,6 @@ pub use iommu_unit::interrupt as message_unit; pub use apic::MSI_DOORBELL; pub use idt::DriverIrq; -pub(crate) use idt::slot_interrupt; /// The machine every program image this kernel loads must be built for. pub const ELF_MACHINE: toyos_elf::Machine = toyos_elf::Machine::X86_64; diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 675a666b57b..784f429f61a 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -25,9 +25,9 @@ //! One table serves every unit, which Section 5.1.3 permits explicitly, so an //! index names the same interrupt whichever unit walks it. //! -//! **Its first entries are the claim slots', one each**, not present until a -//! claim writes its slot's and not present again once the claim is released; -//! the kernel's own sources take the entries after them, one per arming. +//! **An entry is the lowest one free**, held by the kernel's own sources for +//! the boot, one per arming, and by a claim until its release, when it goes +//! not present and is free again. //! //! This module's lock is taken before `UNITS` and before `TABLES`, never after //! either. @@ -35,8 +35,10 @@ use crate::log; use alloc::vec::Vec; -use super::super::apic::{narrow_destination, MSG_DEST, MSG_DEST_NARROW}; -use super::super::idt::DriverIrq; +use kernel::owners::Owners; + +use super::super::apic::{narrow_destination, MSG_DEST}; +use super::super::idt::{self, ClaimVector, DriverIrq}; use crate::iommu::StreamId; use crate::sync::Lock; @@ -51,7 +53,7 @@ pub const COMPATIBILITY_FORMAT: u32 = 1 << 23; /// `2^(7+1)` entries of 128 bits is exactly the 4 KiB [`Tables::alloc`] hands out. const SIZE_FIELD: u64 = 7; -const ENTRIES: u16 = 256; +const ENTRIES: usize = 256; const EXTENDED_INTERRUPT_MODE: u64 = 1 << 11; @@ -81,10 +83,20 @@ pub enum Refused { DestinationTooWide(u32), /// Every entry in the table is already spoken for. TableFull, + /// Every vector a claim's message may carry is held by another claim. + NoVector, /// Firmware's device scopes named no requester id for this interrupt controller. ControllerUnnamed(u8), } +impl Refused { + /// Whether this machine has run out of something a claim spends, rather + /// than this source being one no entry can be written for. + pub fn exhausted(self) -> bool { + matches!(self, Self::TableFull | Self::NoVector) + } +} + impl core::fmt::Display for Refused { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { @@ -92,6 +104,7 @@ impl core::fmt::Display for Refused { write!(f, "apic id {id:#x} does not fit a remapping entry's destination") } Self::TableFull => write!(f, "the interrupt remapping table is full"), + Self::NoVector => write!(f, "every vector a claimed function's message may carry is held"), Self::ControllerUnnamed(id) => { write!(f, "firmware named no requester id for interrupt controller {id}") } @@ -107,7 +120,8 @@ pub struct Pin { struct Remap { /// `None` until a unit is armed, which is what "no source may use the remappable format" means. table: Option, - used: u16, + /// The requester each entry is held for. + held: Owners, /// `ECAP.EIM` on every unit. Clear bounds a destination to the eight bits `DST` then holds. extended: bool, /// Requester ids firmware gave this machine's I/O APICs, which Section 8.3.1.1 requires it to name. @@ -115,12 +129,7 @@ struct Remap { } static REMAP: Lock = - Lock::new(Remap { table: None, used: CLAIMS, extended: false, apics: Vec::new() }); - -/// Entries `0..CLAIMS` are the claim slots'; the kernel's own sources take them -/// from here up. -const CLAIMS: u16 = crate::pcidev::MAX_FUNCTIONS as u16; -const _: () = assert!(crate::pcidev::MAX_FUNCTIONS < ENTRIES as usize); + Lock::new(Remap { table: None, held: Owners::new(), extended: false, apics: Vec::new() }); pub fn describe_apic(apic_id: u8, source: StreamId) { REMAP.lock().apics.push((apic_id, source)); @@ -176,7 +185,7 @@ fn entry(source: StreamId, vector: u8, dest: Destination, level: bool) -> (u64, /// the words meant for it: a line restating the intent agrees with itself /// however the entry was composed, and these fields keep one device off /// another's entry. -fn report(index: u16, source: StreamId, dest: u32, (lo, hi): (u64, u64)) { +fn report(index: usize, source: StreamId, dest: u32, (lo, hi): (u64, u64)) { log!( // `apic=` is the id this was asked for, `dst=` the field it encoded // into; printing only the second leaves the encoding compared against @@ -193,38 +202,9 @@ fn report(index: u16, source: StreamId, dest: u32, (lo, hi): (u64, u64)) { ); } -/// Fills the next free entry for `source` and returns its index. -fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result { - let written = { - let mut remap = REMAP.lock(); - // Unreachable behind `is_armed`, which every caller passes first. - let Some(table) = remap.table else { - return Err(Refused::TableFull); - }; - let destination = if remap.extended { - Ok(Destination::Extended(dest)) - } else { - narrow_destination(dest) - .map(Destination::Narrow) - .map_err(|_| Refused::DestinationTooWide(dest)) - }; - match destination { - Err(why) => Err(why), - Ok(_) if remap.used == ENTRIES => Err(Refused::TableFull), - Ok(destination) => { - let index = remap.used; - remap.used += 1; - let (lo, hi) = entry(source, vector, destination, level); - table.write_pair(index as usize, lo, hi); - // Under the same lock as the write, so no other entry can be - // written between an entry and the invalidation that publishes it. - // Section 6.4: a unit reporting `CAP.CM`, as these do, may cache the - // entry a fault was taken on. - super::invalidate_interrupt_entries(); - Ok((index, table.read_pair(index as usize))) - } - } - }; +/// Fills the lowest free entry for `source` and returns its index. +fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result { + let written = REMAP.lock().write(source, vector, dest, level); match written { Ok((index, read)) => { report(index, source, dest, read); @@ -237,8 +217,36 @@ fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result Result<(usize, (u64, u64)), Refused> { + // Unreachable behind `is_armed`, which every caller passes first. + let Some(table) = self.table else { + return Err(Refused::TableFull); + }; + let destination = if self.extended { + Destination::Extended(dest) + } else { + Destination::Narrow(narrow_destination(dest).map_err(|_| Refused::DestinationTooWide(dest))?) + }; + let index = self.held.take(source.requester().into()).ok_or(Refused::TableFull)?; + // Not present since its last holder's release, so the high half + // written first reaches nothing the unit can walk. + assert!(table.read_pair(index).0 & PRESENT == 0, "iommu: irte{index} is free and still present"); + let (lo, hi) = entry(source, vector, destination, level); + table.write_pair(index, lo, hi); + // Under the same lock as the write, so no other entry can be + // written between an entry and the invalidation that publishes it. + // Section 6.4: a unit reporting `CAP.CM`, as these do, may cache the + // entry a fault was taken on. + super::invalidate_interrupt_entries(); + Ok((index, table.read_pair(index))) + } +} + /// The message that indexes entry `index` by its handle alone (Section 5.1.3). -fn message(index: u16) -> Msi { +fn message(index: usize) -> Msi { let index = index as u32; Msi { address: crate::arch::MSI_DOORBELL @@ -261,53 +269,49 @@ pub fn msi(source: StreamId, irq: DriverIrq) -> Result { Ok(message(allocate(source, irq.vector(), MSG_DEST, false)?)) } -/// Claim slot `slot`'s entry, written for `source` at the slot's vector and -/// published to every unit before this returns, and the message that reaches -/// it. -pub fn claim(slot: usize, source: StreamId) -> Result { - let vector = super::super::idt::claim_vector(slot); - let (index, read) = { - let remap = REMAP.lock(); - let Some(table) = remap.table else { - panic!("iommu: claim slot {slot} written with no table armed"); - }; - let index = u16::try_from(slot) - .ok() - .filter(|index| *index < CLAIMS) - .unwrap_or_else(|| panic!("iommu: claim slot {slot} has no entry of its own")); - // Not present since the last release, so the high half written first - // reaches nothing the unit can walk. - assert!( - table.read_pair(slot).0 & PRESENT == 0, - "iommu: claim slot {slot}'s entry is still present" - ); - let destination = if remap.extended { - Destination::Extended(MSG_DEST) - } else { - Destination::Narrow(MSG_DEST_NARROW) - }; - let (lo, hi) = entry(source, vector, destination, false); - table.write_pair(slot, lo, hi); - super::invalidate_interrupt_entries(); - (index, table.read_pair(slot)) - }; - report(index, source, MSG_DEST, read); - Ok(message(index)) +/// A claimed function's entry and the vector it delivers, held until +/// [`release`]. +pub struct Claimed { + pub msi: Msi, + entry: usize, + vector: ClaimVector, +} + +impl Claimed { + /// What the message raises, for the record. + pub fn interrupt(&self) -> impl core::fmt::Display { + self.vector + } +} + +/// A vector for `pcidev` function `function` and an entry delivering it, written +/// for `source` and published to every unit before this returns. +pub fn claim(function: usize, source: StreamId) -> Result { + let vector = idt::user_dev::take(function).ok_or(Refused::NoVector)?; + match allocate(source, vector.vector(), MSG_DEST, false) { + Ok(entry) => Ok(Claimed { msi: message(entry), entry, vector }), + Err(why) => { + idt::user_dev::give_back(vector); + Err(why) + } + } } -/// Claim slot `slot`'s entry not present again, and gone from every unit's -/// cache before this returns. -pub fn release(slot: usize, source: StreamId) { +/// A claim's entry not present again and gone from every unit's cache, and +/// then its entry and vector free. +pub fn release(claimed: &Claimed, source: StreamId) { let lo = { let remap = REMAP.lock(); let Some(table) = remap.table else { - panic!("iommu: claim slot {slot} released with no table armed"); + panic!("iommu: irte{} released with no table armed", claimed.entry); }; - table.clear_pair(slot); + table.clear_pair(claimed.entry); super::invalidate_interrupt_entries(); - table.read_pair(slot).0 + remap.held.give_back(claimed.entry); + table.read_pair(claimed.entry).0 }; - log!("iommu: irte{slot} source={source} p={} released", lo & PRESENT); + idt::user_dev::give_back(claimed.vector); + log!("iommu: irte{} source={source} p={} released", claimed.entry, lo & PRESENT); } pub fn pin(apic_id: u8, vector: u8, dest: u32, level: bool) -> Result { diff --git a/kernel/src/device.rs b/kernel/src/device.rs index e3fbd00a38b..66c71e75596 100644 --- a/kernel/src/device.rs +++ b/kernel/src/device.rs @@ -159,7 +159,8 @@ pub enum ClaimError { /// A driver in this kernel already binds every function the name matches; /// two drivers on one device is not something a claim may create. KernelDriven, - /// Every slot that can carry a driven function is taken. + /// This machine has no interrupt left to give the function: every vector, + /// remapping entry or LPI a claim spends is held. Exhausted, /// The function is there and could not be handed over — no MSI-X, no /// window to move a BAR into, an address it did not take. The kernel names @@ -198,7 +199,7 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { let id = toyos_abi::syscall::PciId::from_wire(selector[0]) .ok_or(ClaimError::Absent)?; - // The slot's own guard, taken inside: a PCI claim's exclusivity is + // The function's own guard, taken inside: a PCI claim's exclusivity is // per function rather than per class, so there is no flag here to // acquire first. let (info, claim) = crate::pcidev::claim(id)?; @@ -216,7 +217,7 @@ pub fn try_claim(class: DeviceType, selector: [u64; 2]) -> Result { let set = toyos_abi::syscall::IsaId::from_wire(selector).ok_or(ClaimError::Absent)?; - // The row's own guard, taken inside as a PCI slot's is. + // The row's own guard, taken inside as a PCI function's is. let row = crate::isa::claim(set)?; let claim = Claim { what: Claimed::Isa(row) }; Ok(DeviceClaim::new(class, DeviceInfo::Isa(set), claim)) diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index eb9ac0881cc..750fd54ff6f 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -336,7 +336,7 @@ impl PciDevice { /// Put MSI-X back off, for a hand-over that armed a vector and was then /// refused: a function left enabled at a vector nobody holds delivers into - /// a slot with no reader. + /// a claim with no reader. pub fn disable_msix(&self) { let Ok(cap) = self.capability(msix::CAP_ID) else { return }; let control = cap.read_u16(msix::MESSAGE_CONTROL); @@ -488,7 +488,8 @@ impl PciDevice { } } -/// Point a claimed function's [`MSIX_ENTRY`] at its slot's own message and +/// Point a claimed function's [`MSIX_ENTRY`] at the message `claim_msi` made +/// for it and /// enable it. Takes the message and nothing else: the function is /// the one the entry was written for, and no other message reaches here. pub fn arm_claimed_msix(message: &crate::iommu::Remapped) -> Result { diff --git a/kernel/src/iommu/fault.rs b/kernel/src/iommu/fault.rs index 516b46c678b..210cb5d6b96 100644 --- a/kernel/src/iommu/fault.rs +++ b/kernel/src/iommu/fault.rs @@ -30,9 +30,6 @@ use crate::drivers::pci::{self, PciDevice}; use crate::iommu::StreamId; use crate::mm::{DirectMap, Mmio, PAGE_SIZE}; -/// A `pcidev` slot number no claim has: this function is not driven by one. -const NO_SLOT: u32 = u32::MAX; - /// One enumerated function, published before any unit is armed. struct Function { who: StreamId, @@ -41,9 +38,6 @@ struct Function { domain: AtomicU32, // Faults a unit has reported against it; non-zero is the per-domain flag. faults: AtomicU32, - /// The `pcidev` slot a process drives this function on, or [`NO_SLOT`]. - /// What decides whether a fault on it is terminal for the machine. - user_slot: AtomicU32, } /// Exactly the functions this machine enumerated, leaked once by [`describe`]: @@ -97,7 +91,6 @@ pub fn describe(devices: &[PciDevice]) { config: DirectMap::phys_of(device.config_window().addr() as *const u8), domain: AtomicU32::new(0), faults: AtomicU32::new(0), - user_slot: AtomicU32::new(NO_SLOT), }) .collect(); let functions: &'static [Function] = Box::leak(functions.into_boxed_slice()); @@ -117,18 +110,6 @@ pub fn attached(stream: StreamId, domain: u16) { } } -/// Record that a process drives this function on `slot`, or no longer does. -/// -/// The handler reads it to decide what a fault on this stream is *terminal -/// for*: a kernel-driven function has nothing to hand a fault to and the -/// response is the halt in [`conclude`]; one a process drives has an owner, so -/// the record goes to that owner's claim and the machine stays up. -pub fn user_owned(stream: StreamId, slot: Option) { - if let Some(function) = find(Who::Function(stream)) { - function.user_slot.store(slot.map_or(NO_SLOT, |slot| slot as u32), Ordering::Release); - } -} - fn find(who: Who) -> Option<&'static Function> { match who { Who::Function(stream) => functions().iter().find(|f| f.who == stream), @@ -214,15 +195,14 @@ pub fn report(unit: usize, count: &AtomicU32, fault: Fault) -> bool { None => stray_fault(key), }; let _ = FIRST.compare_exchange(u64::MAX, key, Ordering::AcqRel, Ordering::Relaxed); - // Before the line, so `owner=` in it is what was actually told. - let owner = match function.map(|f| f.user_slot.load(Ordering::Acquire)) { + // Before the line, so `owner=` in it is what was actually told: a + // function a claim holds has its holder to refuse, and one only this + // kernel drives has nobody. + let owner = match function { None => Owner::Nobody, - Some(NO_SLOT) => Owner::Kernel, - Some(slot) => Owner::Slot(slot as usize), + Some(function) if crate::pcidev::note_fault(function.who.requester()) => Owner::Process, + Some(_) => Owner::Kernel, }; - if let Owner::Slot(slot) = owner { - crate::pcidev::note_fault(slot); - } let count = count.fetch_add(1, Ordering::Relaxed) + 1; // A stray's line is bounded by its count, never by the device: the next // line it writes says how many were read in between. @@ -285,8 +265,8 @@ fn config_window(phys: u64) -> Mmio { enum Owner { /// An enumerated function only this kernel drives: the halt. Kernel, - /// An enumerated function the process on this `pcidev` slot drives. - Slot(usize), + /// An enumerated function a process holds a claim on. + Process, /// No enumerated function: a requester this kernel never took on. Nobody, } @@ -295,7 +275,7 @@ impl core::fmt::Display for Owner { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { Self::Kernel => write!(f, "kernel"), - Self::Slot(slot) => write!(f, "slot{slot}"), + Self::Process => write!(f, "process"), Self::Nobody => write!(f, "none"), } } diff --git a/kernel/src/iommu/mod.rs b/kernel/src/iommu/mod.rs index 1844305a9b3..0ac953fe923 100644 --- a/kernel/src/iommu/mod.rs +++ b/kernel/src/iommu/mod.rs @@ -2,7 +2,7 @@ //! //! Inventories the machine's IOMMU units, turns translation on, and hands a driver an address space of its own to put its DMA in; an unusable unit is logged and left off rather than halting boot. Names above the backend stay backend-neutral, and what every backend decides alike is declared here once: a domain's addresses ([`window`]) and what a fault record ends ([`fault`]). //! -//! The refusal is deliberately not yet built for a driver in this kernel: landing it before any userspace driver exists would cost every machine and protect nothing. A function a *process* drives is the other case and is refused ([`OwnSpace`], [`remapping`]), because a descriptor it writes a physical address into is an arbitrary read and write over all of memory, and a message nothing translates for it alone raises any claim's interrupt; its message is its claim slot's own, translated for it alone ([`Remapped`]). +//! The refusal is deliberately not yet built for a driver in this kernel: landing it before any userspace driver exists would cost every machine and protect nothing. A function a *process* drives is the other case and is refused ([`OwnSpace`], [`remapping`]), because a descriptor it writes a physical address into is an arbitrary read and write over all of memory, and a message nothing translates for it alone raises any claim's interrupt; its message raises its own interrupt, translated for it alone ([`Remapped`]). //! //! `trait Iommu` is deliberately not added: with one backend it would have a single implementor. @@ -294,8 +294,8 @@ pub struct MsiMessage { pub data: u32, } -/// Every claimed function's message is translated to its claim slot's own -/// interrupt, and no other's. Only [`remapping`] makes one. +/// Every claimed function's message is translated to its own interrupt, and +/// no other's. Only [`remapping`] makes one. #[derive(Clone, Copy)] pub struct Remapping(()); @@ -313,18 +313,16 @@ pub fn remapping() -> Result { } } -/// A claimed function's message: claim slot `slot`'s own, translated for -/// [`Self::function`] alone. +/// A claimed function's message, raising an interrupt of its own and +/// translated for [`Self::function`] alone. /// /// The only message a function a process drives is armed with, since /// [`claim_msi`] is the only thing that makes one; and dropping it takes the -/// translation back, so no refusal after it is written leaves the function a -/// message that reaches anything. +/// translation and the interrupt back, so no refusal after it is written +/// leaves the function a message that reaches anything. pub struct Remapped { - slot: usize, + claimed: messages::Claimed, function: crate::drivers::pci::PciDevice, - address: u32, - data: u32, } impl Remapped { @@ -333,11 +331,16 @@ impl Remapped { } pub fn address(&self) -> u32 { - self.address + self.claimed.msi.address } pub fn data(&self) -> u32 { - self.data + self.claimed.msi.data + } + + /// What the message raises, for the record. + pub fn interrupt(&self) -> impl core::fmt::Display + '_ { + self.claimed.interrupt() } fn stream(&self) -> StreamId { @@ -347,19 +350,20 @@ impl Remapped { impl Drop for Remapped { fn drop(&mut self) { - messages::release(self.slot, self.stream()); + messages::release(&self.claimed, self.stream()); } } -/// Claim slot `slot`'s message, translated for `function` alone. +/// A message for `pcidev` function `number`, raising an interrupt taken for +/// it and translated for `function` alone. pub fn claim_msi( _: Remapping, - slot: usize, + number: usize, function: &crate::drivers::pci::PciDevice, ) -> Result { let stream = StreamId::pci(function.bus, function.dev, function.func); - let msi = messages::claim(slot, stream)?; - Ok(Remapped { slot, function: *function, address: msi.address, data: msi.data }) + let claimed = messages::claim(number, stream)?; + Ok(Remapped { claimed, function: *function }) } /// The message a kernel driver's `bus:device.function` raises `irq` with. @@ -375,18 +379,6 @@ pub fn driver_msi( .map(|msi| MsiMessage { address: msi.address, data: msi.data }) } -/// Record that `bus:device.function` is driven by a process on `slot`, or is no -/// longer driven by one. -/// -/// What the fault handler needs it for is its *terminal* action. A stream every -/// driver of which is in this kernel has nothing to hand a fault to, so the -/// response is a halt; a stream a process drives has an owner to refuse, and -/// the machine keeps running. Takes the triple rather than a [`StreamId`], like -/// [`driver_msi`]: what a requester id is stays in this module. -pub fn note_user_owned(bus: u8, device: u8, function: u8, slot: Option) { - fault::user_owned(StreamId::pci(bus, device, function), slot); -} - /// Reached from the IDT gate the unit's own `FEDATA` names. /// /// Fires when a device has been told no, so what it reports is a bug in whoever owns that device, not in the IOMMU. diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index b3f286ce246..06c541f6116 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -3,9 +3,9 @@ //! The line through the device is **who can name an address**. This module //! keeps config space — there is no write path to it from userland — puts the //! function in an address space of its own at the unit *before* it enables bus -//! mastering, programs its slot's own message into whichever of the function's -//! two message mechanisms it has, translated for that function alone and no -//! other, and hands out every device address a descriptor +//! mastering, programs a message raising an interrupt of its own into whichever +//! of the function's two message mechanisms it has, translated for that +//! function alone and no other, and hands out every device address a descriptor //! may carry. Nothing the holder writes into a descriptor can make the device //! touch memory the kernel did not grant it: the domain maps the grants — the //! claim's own, and the regions of ordinary memory its holder lends it @@ -13,16 +13,16 @@ //! an address outside them is refused at the unit and recorded against that //! claim. //! -//! **What a holder lends is placed in a window of its slot's domain reserved -//! for lending, [`MAX_GRANT_TOTAL`] long, and nowhere else**, so lending and -//! taking back as often as a holder likes spends no address and no remapping -//! table past that window's, and a window with no room left is a refusal -//! ([`SyscallError::ResourceExhausted`]). An address in it is lent again once -//! it is taken back. Within a claim, what the function still had in flight -//! there lands in memory the same holder lent, which is authority it already -//! had over both regions; across claims, only once the function was reset and -//! is quiet ([`Retired`]), and never while the address is the slot's -//! [`RESIDUE`]. +//! **What a holder lends is placed in a window of its function's domain +//! reserved for lending, [`MAX_GRANT_TOTAL`] long, and nowhere else**, so +//! lending and taking back as often as a holder likes spends no address and no +//! remapping table past that window's, and a window with no room left is a +//! refusal ([`SyscallError::ResourceExhausted`]). An address in it is lent +//! again once it is taken back. Within a claim, what the function still had in +//! flight there lands in memory the same holder lent, which is authority it +//! already had over both regions; across claims, only once the function was +//! reset and is quiet ([`Retired`]), and never while the address is the +//! function's [`Function::residue`]. //! //! **A window is 2 MiB because that is the only page this kernel maps**, so a //! BAR a process may see is re-assigned onto a 2 MiB boundary of its own. @@ -71,54 +71,55 @@ //! granted.** Nothing but that function's own driver can stop its queues, and //! no register write retracts a transfer it had taken in before its holder //! died: the first grant that starts it mastering again lets that transfer out. -//! So [`release`] takes those grants back like any other — mastering is off, -//! so the function reaches nothing until its next claim's first grant — and -//! keeps only their device addresses, as the slot's [`RESIDUE`], and the slot -//! is that function's alone (`toyos_pci::slot`), for the rest of the boot if it -//! is never claimed again. A residue address is never lent -//! ([`Bound::lend_room`]) and never a fresh grant's, so the next claim's grant -//! of a range's size is fresh pages placed at that range; a range it places -//! nothing at stays unmapped, so a transfer aimed there faults, and stays -//! residue across its release until a release resets the function. No page is -//! ever two holders'. +//! So [`release`] takes those grants back like any other — mastering is off, so +//! the function reaches nothing until its next claim's first grant — and keeps +//! only their device addresses, as the function's [`Function::residue`], for +//! the rest of the boot if it is never claimed again. A residue address is +//! never lent ([`Bound::lend_room`]) and never a fresh grant's, so the next +//! claim's grant of a range's size is fresh pages placed at that range; a range +//! it places nothing at stays unmapped, so a transfer aimed there faults, and +//! stays residue across its release until a release resets the function. No +//! page is ever two holders'. //! -//! **That rests on the next holder laying its grants out as the last one -//! did.** The stale transfer — data and descriptor write-back — lands in -//! whatever the new holder keeps at that address, after it has set up its own -//! rings or not: harmless for a netstack succeeding a netstack, a silent write into its -//! own memory for a holder with another layout. +//! **That rests on the next holder laying its grants out as the last one did.** +//! The stale transfer — data and descriptor write-back — lands in whatever the +//! new holder keeps at that address, after it has set up its own rings or not: +//! harmless for a netstack succeeding a netstack, a silent write into its own +//! memory for a holder with another layout. //! //! **A reset returns a function's configuration to its defaults, BARs //! included, and the next claim puts back what it needs.** [`release`] resets //! the function by the first mechanism it advertises ([`reset`]) while the old -//! grants are still mapped; the next claim of the slot waits the reset out and +//! grants are still mapped; the function's next claim waits the reset out and //! writes back what the reset cleared and [`bring_up`] does not write itself //! ([`Kept`]) before it reads a register of the function. //! -//! **A call names its function by its claim's [`Binding`] and never by a slot -//! number.** A slot is the next claim's once its holder's release is over, so -//! a number carried from a claim to a call would be another holder's function -//! by the time it was used. The binding is lent by the claim under the lock -//! its release takes it with (`object::Held`): a call runs wholly before the -//! release or finds no binding. Outside a claim's own mint and release, only -//! the two handlers name a slot, by the interrupt and the requester the -//! hardware gave them. +//! **A call names its function by its claim's [`Binding`] and never by the +//! function's number.** A function is the next claim's once its holder's +//! release is over, so a number carried from a claim to a call would be another +//! holder's claim by the time it was used. The binding is lent by the claim +//! under the lock its release takes it with (`object::Held`): a call runs +//! wholly before the release or finds no binding. Outside a claim's own mint +//! and release, only the two handlers name a function, by the interrupt and the +//! requester the hardware gave them. //! -//! **A poll is on a slot's watch only while its claim holds the slot.** It is +//! **A poll is on a function's watch only while its claim holds it.** It is //! registered with the binding ([`add_poll`]), so before the release, whose -//! own answer to every poll on the watch comes before the slot can be -//! reserved again; a registration that finds the claim released is ended by -//! its registrant (`DeviceClaim::add_poll`). So a slot's next holder's +//! own answer to every poll on the watch comes before the function can be +//! claimed again; a registration that finds the claim released is ended by +//! its registrant (`DeviceClaim::add_poll`). So a function's next holder's //! interrupt fires no poll of the last one's, in whatever ring. /// No `crate::` reference, so `kernel-loom` compiles it and models the /// interleaving no guest test lands on. pub(crate) mod record; +use alloc::boxed::Box; use alloc::string::String; use alloc::sync::Arc; use alloc::vec::Vec; use core::fmt::Write; +use core::sync::atomic::{AtomicBool, AtomicPtr, Ordering}; use record::Interrupt; use toyos_abi::boot::{MemoryMapEntry, RootBridgeWindow}; @@ -126,7 +127,6 @@ 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::slot::{self, Slot}; use toyos_pci::{af, aperture, bar, express, msix, placement, pm, probe}; use crate::device::{Claim, ClaimError}; @@ -139,12 +139,6 @@ use crate::sync::Lock; use crate::inbox::PollEntry; use crate::watch::IrqWatch; -/// How many functions this machine can hand out at once. -/// -/// A small fixed number because each one costs an interrupt of its own, which -/// the architecture declares per slot rather than allocates. -pub const MAX_FUNCTIONS: usize = 4; - /// The most one grant may be. A driver's rings and buffers are kilobytes to a /// megabyte; this is room for a queue depth nothing in reach uses, and a bound /// so a refused-but-plausible request is a refusal rather than the machine's @@ -170,18 +164,61 @@ const CONFIG_BYTES: u64 = 4096; /// function that answers nowhere costs, however many times it is claimed. const MAX_CANDIDATES: usize = 8; -static IRQ: [Interrupt; MAX_FUNCTIONS] = [const { Interrupt::new() }; MAX_FUNCTIONS]; +/// One enumerated function's claims, for the life of the boot: what its holder +/// drives, and what outlives a holder. +struct Function { + who: u16, + /// Whether a claim holds it: taken by [`claim`] before the function is + /// touched, and given back last by [`release`]. + /// + /// **This is where a function's exclusivity is decided.** + /// `Claim::acquire`'s per-class flag does not answer for a class that + /// names several devices, `build/build.rs`'s `one_claimant_per_device` + /// compares `system.toml` strings, and a host-side gate is not the + /// capability boundary. + held: AtomicBool, + irq: Interrupt, + /// What its holder's poll waits on: two processes each driving a function + /// must not learn when the other's device is busy. + watch: IrqWatch, + /// Made on its first claim and kept, because a domain id is never given + /// back (`arch/x86_64/vtd/domain.rs`). [`release`] empties it, so the next + /// holder attaches to one that maps nothing. + space: Lock>, + bound: Lock>, + retired: Lock>, + /// Where [`release`] could not reset it was left aimed: its last holder's + /// grants, by address alone, until it is claimed again. + residue: Lock>, +} + +impl core::fmt::Display for Function { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "PCI {:02x}:{:02x}.{}", self.who >> 8, (self.who >> 3) & 0x1f, self.who & 7) + } +} -/// One address space per slot, made on that slot's first claim and kept. -/// -/// Kept because a domain id is never given back (`arch/x86_64/vtd/domain.rs`), so a -/// domain per claim would let a process spawn and die its way through every id -/// the units report. [`release`] empties it, so the next holder attaches to one -/// that maps nothing. -static SPACE: [Lock>; MAX_FUNCTIONS] = - [const { Lock::new(None) }; MAX_FUNCTIONS]; - -/// A slot's address space, and the room in it every holder's lent regions go. +/// Exactly the functions [`publish`] took, in [`Machine::functions`]' order, +/// leaked once: null until then and never written again, so a handler reaches +/// a function's record with no lock. +static FUNCTIONS: AtomicPtr<&'static [Function]> = AtomicPtr::new(core::ptr::null_mut()); + +fn functions() -> &'static [Function] { + let published = FUNCTIONS.load(Ordering::Acquire); + if published.is_null() { + return &[]; + } + // SAFETY: non-null only as `publish` stored it: a leaked box, never freed + // or written again, published with Release once it was whole. + unsafe { *published } +} + +/// Function `number`, which a claim or an interrupt named. +fn function(number: usize) -> &'static Function { + functions().get(number).unwrap_or_else(|| panic!("pcidev: function {number} was never enumerated")) +} + +/// A function's address space, and the room in it every holder's lent regions go. #[derive(Clone, Copy)] struct Space { space: OwnSpace, @@ -217,20 +254,15 @@ enum Origin { Mapped, } -/// Device addresses a slot's domain handed out and no longer maps. +/// Device addresses a function's domain handed out and no longer maps. #[derive(Clone, Copy)] struct Aimed { at: u64, bytes: u64, } -/// Where a function [`release`] could not reset was left aimed: its last -/// holder's grants, by address alone, until that function is claimed again. -static RESIDUE: [Lock>; MAX_FUNCTIONS] = - [const { Lock::new(Vec::new()) }; MAX_FUNCTIONS]; - -/// How a claimed function was made to speak, and the slot's message it speaks -/// with. Both deliver the slot's own interrupt into the same [`Interrupt`] and +/// How a claimed function was made to speak, and the message it speaks with. +/// Both deliver the function's own interrupt into the same [`Interrupt`] and /// the claim answers the same handle either way. enum Armed { /// This function's one MSI-X table entry, mapped for the kernel alone. @@ -238,7 +270,15 @@ enum Armed { Msi(Remapped), } -/// What a live slot drives. The ISR never reads this. +impl Armed { + fn message(&self) -> &Remapped { + match self { + Self::Msix(_, message) | Self::Msi(message) => message, + } + } +} + +/// What a live claim drives. The ISR never reads this. struct Bound { pci: PciDevice, space: OwnSpace, @@ -251,7 +291,7 @@ struct Bound { bar_at: [u64; BARS], bar_bytes: [u64; BARS], grants: Vec, - /// The [`RESIDUE`] this claim took over and has placed no grant at: mapped + /// The [`Function::residue`] this claim took over and has placed no grant at: mapped /// to nothing and counted against nothing, and where a grant of a range's /// size is placed. residue: Vec, @@ -288,13 +328,6 @@ impl Bound { } } -static BOUND: [Lock>; MAX_FUNCTIONS] = - [const { Lock::new(None) }; MAX_FUNCTIONS]; - -/// What a claimed function's poll waits on, one per slot: two processes each driving a -/// function must not learn when the other's device is busy. -static WATCHES: [IrqWatch; MAX_FUNCTIONS] = [const { IrqWatch::new() }; MAX_FUNCTIONS]; - /// Every function this machine enumerated, and the two windows a BAR may be /// moved into. struct Machine { @@ -342,7 +375,7 @@ struct Machine { /// returned to a free list would let a later claim put a second function on /// top of a live one; and one cut per claim would let a process spawn and /// die its way through the whole span. Taken once and reused, which is - /// [`SPACE`]'s treatment for the same reason. + /// [`Function::space`]'s treatment for the same reason. windows: Vec<(u16, u8, u64, u64)>, /// Functions this module has reset, when each may be touched again (PCIe /// §6.6.2), and what the reset cleared that the next claim puts back. One @@ -364,45 +397,26 @@ static MACHINE: Lock = Lock::new(Machine { resetting: Vec::new(), }); -/// Which requester holds each slot. -/// -/// **One lock over the whole array, because this is where a function's -/// exclusivity is decided.** `Claim::acquire`'s per-class flag does not answer -/// for a class that names several devices, and `build/build.rs`'s -/// `one_claimant_per_device` compares `system.toml` strings, and a host-side -/// gate is not the capability boundary. -/// -/// **Held across nothing**, and taken with nothing of this module's held — -/// `reserve` runs after `claim` has dropped `MACHINE`, and `release` takes it -/// once the teardown is over — so it may be taken under any lock outside it. -static SLOTS: Lock<[Slot; MAX_FUNCTIONS]> = Lock::new([Slot::Free; MAX_FUNCTIONS]); - -/// One claim's hold on a slot, from its reservation to the release its drop -/// is: what every call on the claim is made with. +/// One claim's hold on a function, from its reservation to the release its +/// drop is: what every call on the claim is made with. /// /// Neither `Copy` nor `Clone` and made only by [`claim`], so there is one per -/// reservation, and a reference to it is a slot no other claim holds. +/// reservation, and a reference to it is a function no other claim holds. pub struct Binding { - slot: usize, - /// The requester the slot is held for. - who: u16, + /// Its index in [`functions`]. + number: usize, } -impl Drop for Binding { - fn drop(&mut self) { - release(self.slot); +impl Binding { + fn function(&self) -> &'static Function { + function(self.number) } } -/// Take a slot for `who`, or say why not. -/// -/// The scan and the take are one critical section: two claims arriving together -/// must not both find the function unheld. -fn reserve(who: u16) -> Result { - slot::reserve(&mut *SLOTS.lock(), who).map_err(|refused| match refused { - slot::Refused::Owned => ClaimError::Owned, - slot::Refused::Exhausted => ClaimError::Exhausted, - }) +impl Drop for Binding { + fn drop(&mut self) { + release(self.number); + } } /// Every function this machine enumerated, and who drives it now: a kernel @@ -413,13 +427,11 @@ pub fn inventory() -> Vec { let machine = MACHINE.lock(); (machine.identities.clone(), machine.kernel_driven.clone()) }; - let slots = *SLOTS.lock(); let mut out = Vec::with_capacity(identities.len()); - for mut pci in identities { - let who = requester_of(pci.at); - pci.driven = if kernel_driven.contains(&who) { + for (mut pci, function) in identities.into_iter().zip(functions()) { + pci.driven = if kernel_driven.contains(&function.who) { Driven::Kernel - } else if slots.contains(&Slot::Held(who)) { + } else if function.held.load(Ordering::Acquire) { Driven::Claimed } else { Driven::Free @@ -440,7 +452,7 @@ pub fn enumerated(bus: u8, dev: u8, func: u8) -> Option u16 { /// The function a claim holds. pub fn held_at(binding: &Binding) -> toyos_abi::inventory::PciAddr { - addr_of(MACHINE.lock().segment, binding.who) + addr_of(MACHINE.lock().segment, binding.function().who) } pub(crate) fn requester(pci: &PciDevice) -> u16 { @@ -564,6 +576,27 @@ pub fn publish(devices: &[PciDevice], segment: u16, maps: &[MemoryMapEntry], fir for run in low.iter().chain(high.iter()) { log!("pcidev: {:#x}..{:#x} ({} MiB)", run.start, run.end, (run.end - run.start) / MIB); } + let records: Vec = devices + .iter() + .map(|device| Function { + who: requester(device), + held: AtomicBool::new(false), + irq: Interrupt::new(), + watch: IrqWatch::new(), + space: Lock::new(None), + bound: Lock::new(None), + retired: Lock::new(None), + residue: Lock::new(Vec::new()), + }) + .collect(); + let records: &'static [Function] = Box::leak(records.into_boxed_slice()); + let first = FUNCTIONS.compare_exchange( + core::ptr::null_mut(), + Box::leak(Box::new(records)), + Ordering::Release, + Ordering::Relaxed, + ); + assert!(first.is_ok(), "pcidev: the functions were published twice"); let mut machine = MACHINE.lock(); machine.functions = devices.to_vec(); machine.identities = @@ -733,13 +766,13 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { // the console's virtio-serial and a process's carry one ID. let (matches, resolved) = { let machine = MACHINE.lock(); - let matches: Vec = - machine.functions.iter().filter(|d| d.is_id(id.vendor, id.device)).copied().collect(); + let matches: Vec<(usize, PciDevice)> = + machine.functions.iter().copied().enumerate().filter(|(_, d)| d.is_id(id.vendor, id.device)).collect(); let resolved = - toyos_pci::claimed(matches.iter().map(|d| machine.kernel_driven.contains(&requester(d)))); + toyos_pci::claimed(matches.iter().map(|(_, d)| machine.kernel_driven.contains(&requester(d)))); (matches, resolved) }; - let pci = match resolved { + let (number, pci) = match resolved { Ok(at) => matches[at], Err(toyos_pci::Unclaimable::Absent) => return Err(ClaimError::Absent), Err(toyos_pci::Unclaimable::Ambiguous) => { @@ -754,7 +787,7 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { return Err(ClaimError::Ambiguous); } Err(toyos_pci::Unclaimable::KernelDriven) => { - for pci in &matches { + for (_, pci) in &matches { log!( "pcidev: PCI {:02x}:{:02x}.{} is driven by this kernel and cannot be claimed", pci.bus, @@ -766,16 +799,18 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { } }; - // `Owned` before `Exhausted`: a second claim on a function somebody holds - // is a different fact from a machine with no slot left. - let who = requester(&pci); - let slot = reserve(who)?; - // Dropped by every refusal below, which is what gives the slot back. - let binding = Binding { slot, who }; + // The test and the take are one exchange: two claims arriving together + // must not both find the function unheld. + let held = &function(number).held; + if held.compare_exchange(false, true, Ordering::Acquire, Ordering::Relaxed).is_err() { + return Err(ClaimError::Owned); + } + // Dropped by every refusal below, which is what gives the function back. + let binding = Binding { number }; - match bring_up(pci, id, slot) { + match bring_up(pci, id, number) { Ok(mut bound) => { - bound.residue = take_residue(slot); + bound.residue = take_residue(number); let info = PciFunctionInfo { bar_bytes: bound.bar_bytes, bus: pci.bus, @@ -783,18 +818,18 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { func: pci.func, _pad: [0; 5], }; - *BOUND[slot].lock() = Some(bound); - IRQ[slot].clear(); - crate::iommu::note_user_owned(pci.bus, pci.dev, pci.func, Some(slot)); log!( - "pcidev: PCI {:02x}:{:02x}.{} [{:04x}:{:04x}] handed over on slot {slot}, {}", + "pcidev: PCI {:02x}:{:02x}.{} [{:04x}:{:04x}] handed over, {}", pci.bus, pci.dev, pci.func, id.vendor, id.device, - crate::arch::slot_interrupt(slot), + bound.armed.message().interrupt(), ); + let function = binding.function(); + *function.bound.lock() = Some(bound); + function.irq.clear(); Ok((info, Claim::pci(binding))) } Err(why) => { @@ -804,7 +839,10 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { pci.dev, pci.func ); - Err(ClaimError::Unusable) + match why { + Refusal::NoMessage(refused) if refused.exhausted() => Err(ClaimError::Exhausted), + _ => Err(ClaimError::Unusable), + } } } } @@ -815,13 +853,13 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, Claim), ClaimError> { /// The order is the whole security argument: a function that could master the /// bus before its domain existed would be reaching physical memory with /// whatever addresses its registers still held. -fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { +fn bring_up(pci: PciDevice, id: PciId, number: usize) -> Result { let remapping = crate::iommu::remapping().map_err(|_| Refusal::NotRemapped)?; - // What the slot's previous holder left mapped goes before anything attaches - // to its domain, and whatever is left of a reset [`release`] started on this + // What the function's previous holder left mapped goes before anything + // attaches to its domain, and whatever is left of a reset [`release`] started on this // function before a register of it is read (PCIe §6.6.2). - finish_retired(slot); + finish_retired(number); settle_after_reset(&pci); // **Every refusal that can be taken without touching the function is taken @@ -831,7 +869,7 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { // descriptors. It is asked for here rather than after the BARs because a // refusal that had already armed a vector and moved a function's BARs would // leave the machine changed by a hand-over that did not happen. - let Space { space, lend } = slot_space(slot).map_err(Refusal::Untranslated)?; + let Space { space, lend } = function_space(number).map_err(Refusal::Untranslated)?; // The table's own BAR, so it can be left where it is and kept out of what // the holder maps. @@ -841,9 +879,9 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { // Then the interrupt, still before a window is cut: a function neither // mechanism can be armed on is one no holder could ever be told anything - // about. Every refusal from here drops `message`, which puts the slot's - // entry back to not present. - let message = crate::iommu::claim_msi(remapping, slot, &pci).map_err(Refusal::NoMessage)?; + // about. Every refusal from here drops `message`, which gives its + // translation and its interrupt back. + let message = crate::iommu::claim_msi(remapping, number, &pci).map_err(Refusal::NoMessage)?; let armed = match arm_claimed_msix(&message) { Ok(entry) => Armed::Msix(entry, message), Err(NoEntry::Unusable) => return Err(Refusal::MsixUnusable), @@ -951,9 +989,9 @@ fn place_bars( Ok((bar_at, bar_bytes)) } -/// This slot's address space, made on its first claim. -fn slot_space(slot: usize) -> Result { - let mut held = SPACE[slot].lock(); +/// Function `number`'s address space, made on its first claim. +fn function_space(number: usize) -> Result { + let mut held = function(number).space.lock(); match *held { Some(space) => Ok(space), None => { @@ -1177,7 +1215,7 @@ fn wait_until(at: u64) { } } -/// A released function's grants, still mapped in its slot's domain until its +/// A released function's grants, still mapped in its domain until its /// reset has finished: a write the function had already issued lands in the /// released holder's own pages rather than faulting, or landing in pages the /// allocator has handed somebody else. @@ -1187,32 +1225,32 @@ struct Retired { quiet_at: u64, } -static RETIRED: [Lock>; MAX_FUNCTIONS] = - [const { Lock::new(None) }; MAX_FUNCTIONS]; - -/// Unmap and free what a slot's previous holder left mapped, once its function -/// can no longer write into it. Before anything else attaches to the domain. -fn finish_retired(slot: usize) { - let retired = RETIRED[slot].lock().take(); +/// Unmap and free what a function's previous holder left mapped, once the +/// function can no longer write into it. Before anything else attaches to the +/// domain. +fn finish_retired(number: usize) { + let function = function(number); + let retired = function.retired.lock().take(); let Some(retired) = retired else { return }; wait_until(retired.quiet_at); for grant in retired.grants.iter() { if let Err(why) = retired.space.unmap(grant.at, grant.bytes) { - panic!("pcidev: slot {slot} could not take {:#x} back: {why}", grant.at); + panic!("pcidev: {function} could not take {:#x} back: {why}", grant.at); } } // The pages go back with this drop, after the unmap above. drop(retired); } -/// The slot's [`RESIDUE`], for the claim that now holds its function. -fn take_residue(slot: usize) -> Vec { - let residue = core::mem::take(&mut *RESIDUE[slot].lock()); +/// The function's [`Function::residue`], for the claim that now holds it. +fn take_residue(number: usize) -> Vec { + let function = function(number); + let residue = core::mem::take(&mut *function.residue.lock()); if !residue.is_empty() { let bytes: u64 = residue.iter().map(|aimed| aimed.bytes).sum(); log!( - "pcidev: slot {slot} holds {} range(s), {bytes} bytes, its function was left aimed \ - at: a grant of a range's size is placed there, on fresh pages", + "pcidev: {function} was left aimed at {} range(s), {bytes} bytes: a grant of a \ + range's size is placed there, on fresh pages", residue.len() ); } @@ -1447,30 +1485,29 @@ fn alone_in_its_page(claimed: &PciDevice, index: u8, at: u64, span: u64) { } } -/// Give up a slot: the device stops mastering the bus, then loses its -/// addresses, and only then do the pages behind them go back — and where -/// nothing reset it, the addresses are kept as the slot's [`RESIDUE`]. +/// Give up a function: it stops mastering the bus, then loses its addresses, +/// and only then do the pages behind them go back — and where nothing reset +/// it, the addresses are kept as its [`Function::residue`]. /// /// The order is what makes a dying driver safe. A page freed while the function /// could still reach it is a device writing into memory the allocator has /// already handed to somebody else. -fn release(slot: usize) { +fn release(number: usize) { + let function = function(number); // Two statements, because edition 2021 keeps an `if let`'s scrutinee - // temporaries alive to the end of its block: `BOUND[slot]`'s guard would be + // temporaries alive to the end of its block: the bound guard would be // held across the unmaps, the reset and the watch's answer. - let bound = BOUND[slot].lock().take(); + let bound = function.bound.lock().take(); if let Some(bound) = bound { - tear_down(slot, bound); + tear_down(function, bound); } // Unconditional and last: a hand-over refused inside [`bring_up`] bound - // nothing and still holds its reservation, and the slot comes back only - // once the function that was in it can no longer reach memory — or, with - // a residue, stays that function's. - let residue = !RESIDUE[slot].lock().is_empty(); - slot::release(&mut *SLOTS.lock(), slot, residue); + // nothing and still holds its reservation, and the function comes back + // only once it can no longer reach memory. + function.held.store(false, Ordering::Release); } -fn tear_down(slot: usize, mut bound: Bound) { +fn tear_down(function: &Function, mut bound: Bound) { bound.pci.disable_bus_master(); let message = match bound.armed { Armed::Msix(entry, message) => { @@ -1482,11 +1519,10 @@ fn tear_down(slot: usize, mut bound: Bound) { message } }; - // Once the function no longer speaks, and before the slot can be reserved - // again: its remapping entry goes not present. + // Once the function no longer speaks, and before it can be claimed again: + // its translation goes, and its interrupt is free. bound.pci.drain_messages(); drop(message); - crate::iommu::note_user_owned(bound.pci.bus, bound.pci.dev, bound.pci.func, None); // **The reset before the domain gives anything back.** With mastering off // the function starts nothing new; its grants stay mapped until it is // quiet, so nothing it had already issued lands in a page the allocator @@ -1507,8 +1543,8 @@ fn tear_down(slot: usize, mut bound: Bound) { } } let parked = Retired { space: bound.space, grants, quiet_at: resetting.quiet_at() }; - let previous = RETIRED[slot].lock().replace(parked); - assert!(previous.is_none(), "pcidev: slot {slot} was claimed with a retired holder left"); + let previous = function.retired.lock().replace(parked); + assert!(previous.is_none(), "pcidev: {function} was claimed with a retired holder left"); } // Nothing reset it, so whatever it had taken in is still aimed at these // addresses; mastering is off, so it reaches none of them until the @@ -1516,11 +1552,11 @@ fn tear_down(slot: usize, mut bound: Bound) { None => { for grant in grants.iter() { if let Err(why) = bound.space.unmap(grant.at, grant.bytes) { - panic!("pcidev: slot {slot} could not take {:#x} back: {why}", grant.at); + panic!("pcidev: {function} could not take {:#x} back: {why}", grant.at); } } - let mut residue = RESIDUE[slot].lock(); - assert!(residue.is_empty(), "pcidev: slot {slot} was claimed with a residue left untaken"); + let mut residue = function.residue.lock(); + assert!(residue.is_empty(), "pcidev: {function} was claimed with a residue left untaken"); // With what this claim took over and placed nothing at: the function may // still be aimed there too, until something resets it. *residue = grants @@ -1530,11 +1566,11 @@ fn tear_down(slot: usize, mut bound: Bound) { .collect(); } } - IRQ[slot].clear(); - // The function is gone from this slot, so a poll on it is answered rather than left for the next holder's interrupts. - WATCHES[slot].cancel_polls(); + function.irq.clear(); + // The holder is gone, so a poll on it is answered rather than left for the next holder's interrupts. + function.watch.cancel_polls(); log!( - "pcidev: PCI {:02x}:{:02x}.{} [{:04x}:{:04x}] released from slot {slot}; reset by {how}", + "pcidev: PCI {:02x}:{:02x}.{} [{:04x}:{:04x}] released; reset by {how}", bound.pci.bus, bound.pci.dev, bound.pci.func, @@ -1548,10 +1584,11 @@ fn with_bound( binding: &Binding, f: impl FnOnce(&mut Bound) -> Result, ) -> Result { - if IRQ[binding.slot].faulted() { + let function = binding.function(); + if function.irq.faulted() { return Err(SyscallError::Io); } - let mut guard = BOUND[binding.slot].lock(); + let mut guard = function.bound.lock(); // Bound before `claim` hands the binding out, and unbound only by its drop. f(guard.as_mut().expect("pcidev: a claim's binding has its function bound")) } @@ -1583,7 +1620,7 @@ pub fn dma_alloc( binding: &Binding, bytes: u64, ) -> Result<(Arc, u64, u64), SyscallError> { - let slot = binding.slot; + let function = binding.function(); with_bound(binding, |bound| { if bytes == 0 || bytes > MAX_GRANT_BYTES { return Err(SyscallError::InvalidArgument); @@ -1597,7 +1634,7 @@ pub fn dma_alloc( let phys = memory.phys().phys(); // The pages are the allocator's own and 2 MiB, so a domain that cannot // take them is a kernel bug rather than the device's answer. - let refused = |why| -> ! { panic!("pcidev: slot {slot} could not map a grant: {why}") }; + let refused = |why| -> ! { panic!("pcidev: {function} could not map a grant: {why}") }; let (at, residual) = match bound.residue.iter().position(|aimed| aimed.bytes == span) { // The function may still be aimed here, so this holder is the one // whatever it had in flight lands on. @@ -1634,7 +1671,7 @@ pub fn dma_alloc( /// the same holder can push between the grant and this, and the address the /// caller was *told* is not always the address the grant is at. pub fn dma_undo(binding: &Binding, memory: &Arc) { - let slot = binding.slot; + let function = binding.function(); let _ = with_bound(binding, |bound| { let Some(index) = bound.grants.iter().position(|grant| Arc::ptr_eq(&grant.memory, memory)) else { @@ -1642,7 +1679,7 @@ pub fn dma_undo(binding: &Binding, memory: &Arc) { }; let grant = bound.grants.remove(index); if let Err(why) = bound.space.unmap(grant.at, grant.bytes) { - panic!("pcidev: slot {slot} could not take {:#x} back: {why}", grant.at); + panic!("pcidev: {function} could not take {:#x} back: {why}", grant.at); } if grant.origin == (Origin::Allocated { residual: true }) { bound.residue.push(Aimed { at: grant.at, bytes: grant.bytes }); @@ -1666,7 +1703,7 @@ pub fn dma_undo(binding: &Binding, memory: &Arc) { /// mapping every region its clients send is bounded by the same number; and a /// window with no run of the region's length left free is the same refusal. pub fn dma_map(binding: &Binding, region: &Arc) -> Result<(u64, u64), SyscallError> { - let slot = binding.slot; + let function = binding.function(); with_bound(binding, |bound| { let (phys, span) = region.ram().ok_or(SyscallError::InvalidArgument)?; // One region, one address: a second mapping of it would be two grants @@ -1680,10 +1717,10 @@ pub fn dma_map(binding: &Binding, region: &Arc) -> Result<(u64, return Err(SyscallError::ResourceExhausted); } let at = bound.lend_room(span).ok_or(SyscallError::ResourceExhausted)?; - // Inside room the domain handed out with the slot, and the pages are + // Inside room the domain handed out with the function, and the pages are // the allocator's own whole 2 MiB ones: a refusal here is a kernel bug. if let Err(why) = bound.space.place(at, phys, span) { - panic!("pcidev: slot {slot} could not lend a region at {at:#x}: {why}"); + panic!("pcidev: {function} could not lend a region at {at:#x}: {why}"); } bound.grants.push(Grant { memory: Arc::clone(region), at, bytes: span, origin: Origin::Mapped }); bound.start_mastering(); @@ -1703,7 +1740,7 @@ pub fn dma_map(binding: &Binding, region: &Arc) -> Result<(u64, /// last `Arc` of one can run its zero-handle teardown, which takes other /// objects' locks, and the caller holds its claim's. pub fn dma_unmap(binding: &Binding, at: u64) -> Result, SyscallError> { - let slot = binding.slot; + let function = binding.function(); with_bound(binding, |bound| { let index = bound .grants @@ -1712,7 +1749,7 @@ pub fn dma_unmap(binding: &Binding, at: u64) -> Result, Sys .ok_or(SyscallError::NotFound)?; let grant = bound.grants.remove(index); if let Err(why) = bound.space.unmap(grant.at, grant.bytes) { - panic!("pcidev: slot {slot} could not take {:#x} back: {why}", grant.at); + panic!("pcidev: {function} could not take {:#x} back: {why}", grant.at); } Ok(grant.memory) }) @@ -1748,13 +1785,15 @@ pub fn config_read(binding: &Binding, at: Register, width: RegWidth) -> Result Result, SyscallError> { - let slot = binding.slot; - if IRQ[slot].faulted() { + let function = binding.function(); + if function.irq.faulted() { return Err(SyscallError::Io); } - let taken = IRQ[slot].take(); - if taken.is_some() && IRQ[slot].take_unannounced() { - log!("pcidev: slot {slot} took its first message on {}", crate::arch::slot_interrupt(slot)); + let taken = function.irq.take(); + if taken.is_some() && function.irq.take_unannounced() { + let bound = function.bound.lock(); + let bound = bound.as_ref().expect("pcidev: a claim's binding has its function bound"); + log!("pcidev: {function} took its first message on {}", bound.armed.message().interrupt()); } Ok(taken) } @@ -1762,34 +1801,44 @@ pub fn take_record(binding: &Binding) -> Result, Syscall /// Whether a read of the claim answers at once: a message is waiting, or the /// refusal is. pub fn has_irq(binding: &Binding) -> bool { - IRQ[binding.slot].armed() || IRQ[binding.slot].faulted() + let function = binding.function(); + function.irq.armed() || function.irq.faulted() } /// Records one message and posts the claim's watch. Called from the -/// interrupt's handler, so it allocates nothing; `record.rs` owns the -/// counting, and `kernel-loom` models it against a concurrent reader. +/// interrupt's handler for function `number`, so it allocates nothing; +/// `record.rs` owns the counting, and `kernel-loom` models it against a +/// concurrent reader. /// /// **The handler holds the preempt count raised across it**: the post lets go /// of a count of its own, and at depth zero that would run a scheduler pass /// inside the handler, before its end of interrupt. -pub fn isr(slot: usize) { - assert!(crate::preempt::count() > 0, "pcidev: slot {slot}'s interrupt arrived with the preempt count at zero"); - IRQ[slot].took(crate::clock::nanos_since_boot()); - WATCHES[slot].post_in_place(); +pub fn isr(number: usize) { + let function = function(number); + assert!(crate::preempt::count() > 0, "pcidev: {function}'s interrupt arrived with the preempt count at zero"); + function.irq.took(crate::clock::nanos_since_boot()); + function.watch.post_in_place(); } -/// The unit refused this function an access. +/// The unit refused requester `who` an access: `true` where a claim holds it. /// -/// Called from the fault handler: every call the claim answers refuses from -/// here on, its interrupt read included, and the post wakes whoever waits on -/// the claim to read that refusal, as a message's does. -pub fn note_fault(slot: usize) { - IRQ[slot].fault(); - WATCHES[slot].post_in_place(); +/// Called from the fault handler, so it takes no lock: every call the claim +/// answers refuses from here on, its interrupt read included, and the post +/// wakes whoever waits on the claim to read that refusal, as a message's does. +pub fn note_fault(who: u16) -> bool { + let Some(function) = functions().iter().find(|function| function.who == who) else { + return false; + }; + if !function.held.load(Ordering::Acquire) { + return false; + } + function.irq.fault(); + function.watch.post_in_place(); crate::preempt::set_need_resched(); + true } /// Register a poll on the watch of the function a claim holds. pub(crate) fn add_poll(binding: &Binding, entry: PollEntry) { - WATCHES[binding.slot].add_poll(entry); + binding.function().watch.add_poll(entry); } diff --git a/kernel/src/pcidev/record.rs b/kernel/src/pcidev/record.rs index 8ba27c7b569..47694d97228 100644 --- a/kernel/src/pcidev/record.rs +++ b/kernel/src/pcidev/record.rs @@ -7,7 +7,8 @@ //! //! **Two parties race**: the ISR, on whichever CPU the unit routed the message //! to, and the holder reading its record through a syscall on any CPU. One -//! handler at a time: a slot has one vector, delivered to one CPU. +//! handler at a time: a function's claim has one vector or LPI, delivered to +//! cpu0. //! //! **The invariant is that every message is counted exactly once.** The count //! is a read-modify-write on both sides and never a load followed by a store: @@ -94,7 +95,7 @@ pub struct Interrupt { /// refuses from here on: its bus mastering is gone, so a driver that kept /// going would be driving nothing. faulted: AtomicBool, - /// True until this slot's first message has been announced. + /// True until this claim's first message has been announced. /// /// **A count of zero at the end of a boot is two facts**: a device that was /// never made to speak, and a message that never reached a CPU. Only the @@ -104,7 +105,7 @@ pub struct Interrupt { } impl Interrupt { - /// Must stay `const`: the slots are a `static` array. + /// Must stay `const`: `isa`'s rows are a `static` array. #[cfg(not(feature = "loom"))] pub const fn new() -> Self { Self { @@ -180,7 +181,7 @@ impl Interrupt { self.word.load(ORDER) & COUNT != 0 } - /// Whether this is the first message this slot has taken. Answers `true` + /// Whether this is the first message this claim has taken. Answers `true` /// once per claim and `false` ever after, so a caller may log on it. /// /// `swap` for [`Self::take`]'s reason: two reads that both loaded `true` @@ -199,7 +200,7 @@ impl Interrupt { self.faulted.load(ORDER) } - /// Back to the state a fresh slot is in, for a claim being minted or given + /// Back to the state a fresh record is in, for a claim being minted or given /// up. The holder is not running at either point. pub fn clear(&self) { self.word.store(0, ORDER); diff --git a/system/supervisor/src/main.rs b/system/supervisor/src/main.rs index fc78dee7b9f..4884c10c797 100644 --- a/system/supervisor/src/main.rs +++ b/system/supervisor/src/main.rs @@ -2396,7 +2396,7 @@ fn refused(name: &str, why: SyscallError) -> String { SyscallError::PermissionDenied => { format!("{name} is driven by the kernel and cannot be claimed") } - SyscallError::ResourceExhausted => format!("no claim slot is free for {name}"), + SyscallError::ResourceExhausted => format!("this machine has no interrupt left for {name}"), SyscallError::NotSupported => { format!("{name} is on this machine and could not be handed over; the kernel's \ `pcidev:`, `partclaim:`, `isa:`, `acpi:` or `hda:` line says why") diff --git a/tests/checks/serial.rs b/tests/checks/serial.rs index d5d9091cacd..7d80c3dc103 100644 --- a/tests/checks/serial.rs +++ b/tests/checks/serial.rs @@ -67,7 +67,7 @@ pub fn self_check() -> Result<(), String> { Serial::named( "test capture", "[ 0.001 cpu0 kernel] NVMe: found\n[ 4.100 cpu0 kernel] iommu: DMA FAULT \ - owner=slot0 unit0 stream=00:03.0 addr=0x1000 access=read reason=0x06 \ + owner=process unit0 stream=00:03.0 addr=0x1000 access=read reason=0x06 \ read-permission\n", ) .must_be_clean() @@ -173,7 +173,7 @@ pub fn self_check() -> Result<(), String> { // two lines differ in one field, which is what makes the case worth // stating rather than assuming. ( - "[ 4.100 cpu0 kernel] iommu: DMA FAULT owner=slot0 unit0 stream=00:03.0 \ + "[ 4.100 cpu0 kernel] iommu: DMA FAULT owner=process unit0 stream=00:03.0 \ addr=0x1000 access=read reason=0x06 read-permission", None, ), diff --git a/tests/common/claims.rs b/tests/common/claims.rs index 6419def050c..028bf4d0ce8 100644 --- a/tests/common/claims.rs +++ b/tests/common/claims.rs @@ -93,10 +93,10 @@ pub fn reuses_its_entry(log: &Serial) -> Result<(), String> { } } let count = |what: &str| { - let needle = format!("[{I219}] {what} slot "); + let needle = format!("[{I219}] {what}"); window.iter().filter(|m| m.starts_with("pcidev: PCI ") && m.contains(&needle)).count() }; - let (handed, released) = (count("handed over on"), count("released from")); + let (handed, released) = (count("handed over"), count("released")); if (handed, released) != (2, 2) { return Err(format!( "{I219} was handed over {handed} time(s) and released {released}, want 2 and 2:\n{}", diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 64062b25c46..57a2dd6c83c 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -189,7 +189,7 @@ fn no_unit_is_no_claim(log: &Serial) -> Result<(), String> { // And this machine handed *nothing* over, which is more than the claim's // own refusal says: with no unit there is no function any process could be // given an address space for. - log.must_not_say("handed over on slot")?; + log.must_not_say("] handed over, ")?; Ok(()) } diff --git a/tests/common/serial.rs b/tests/common/serial.rs index 8142b8be1b8..bd2e8b4ccdc 100644 --- a/tests/common/serial.rs +++ b/tests/common/serial.rs @@ -98,7 +98,7 @@ pub(crate) const DEATHS: &[(&str, Died, Died)] = &[ ("MACHINE CHECK", Died::Kernel, Died::Kernel), // kernel/src/iommu/fault.rs — a fault on a function this kernel drives has // nobody to hand it to, so the handler halts. One a *process* drives says - // `owner=slot`, and one naming no enumerated function `owner=none`, and + // `owner=process`, and one naming no enumerated function `owner=none`, and // the machine goes on, which is why the needle is the owner rather than // the fault. ("iommu: DMA FAULT owner=kernel", Died::Kernel, Died::Kernel), @@ -354,7 +354,7 @@ const NEVER_CLEAN: &[&str] = &[ // refuses every later call, so this is not a death; it is a driver whose // descriptors are wrong, and a netstack that did it on every boot would // otherwise pass everywhere. - "iommu: DMA FAULT owner=slot", + "iommu: DMA FAULT owner=process", // The same file — a requester no enumerated function is: refused by the // unit, and on a boot that staged none, a device nothing here accounts for. "iommu: DMA FAULT owner=none", diff --git a/tests/toyos.rs b/tests/toyos.rs index 3aa92da16e3..6b463ee879e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -2538,13 +2538,13 @@ const VIRT_CLAIM_BAR_FAULT: &str = "claim_bar_fault"; /// the console only if the kernel read through the BAR for a crash report. const BAR_MARKS: [u64; 2] = [0xbad0_c0de_0000_5eed, 0xbad1_c0de_0000_5eed]; -/// A claimed function's message, translated by the ITS into its claim slot's +/// A claimed function's message, translated by the ITS into its function's /// LPI, and the claims the kernel refuses beside it, on `tests/images/virtclaimcase.toml`. /// Its first job claims QEMU's `edu` and reads every message it asks it for as /// one interrupt of its claim (`tests/guest/rust-tests/src/bin/claim_lpi.rs`), /// judged beside it on the kernel's lines: the ITS armed from the MADT; the -/// function handed over on a slot, its DeviceID the requester ID QEMU's IORT -/// maps it to, its slot's LPI raised; and at the claim's end the DeviceID +/// function handed over, its DeviceID the requester ID QEMU's IORT maps it +/// to, its LPI raised; and at the claim's end the DeviceID /// unmapped again. The second is refused the `e1000e` the IORT routes past /// the SMMUv3, by the kernel's name for it, and claims edu after it. The /// third faults twice in edu's BAR, and names it as a syscall's buffer; no @@ -2578,7 +2578,7 @@ fn virt_claim_lpi(profile: qemu::Profile) -> Result<(), String> { let mut serial = virt_console(&qemu); judge_virt_job(&mut qemu, &mut serial, JOB, SAID)?; // The release's own line, its last: the DeviceID's unmapping comes before it. - await_marker(&mut qemu, &mut serial, " released from slot ", "the claim's release")?; + await_marker(&mut qemu, &mut serial, &format!("{EDU} released; "), "the claim's release")?; let line = |want: &str| serial.lines().find(|l| l.contains(want)).map(str::to_string); let Some(armed) = line("ITS: 0 at 0x8080000 armed") else { return Err(format!("the MADT's ITS was never armed @@ -2586,23 +2586,23 @@ serial: {serial}")); }; eprintln!(" [virt] {armed}"); - let Some(handed) = line(&format!("{EDU} handed over on slot ")) else { + let Some(handed) = line(&format!("{EDU} handed over, ")) else { return Err(format!("edu was never handed over serial: {serial}")); }; eprintln!(" [virt] {handed}"); - // `pcidev: PCI bb:dd.f [1234:11e8] handed over on slot N, LPI M`. + // `pcidev: PCI bb:dd.f [1234:11e8] handed over, LPI M`. let parsed = handed.split_once("pcidev: PCI ").and_then(|(_, rest)| { let (at, rest) = rest.split_once(' ')?; - let (slot, lpi) = rest.split_once("on slot ")?.1.split_once(", LPI ")?; - Some((at.to_string(), slot.parse::().ok()?, lpi.trim().parse::().ok()?)) + let lpi = rest.split_once("handed over, LPI ")?.1; + Some((at.to_string(), lpi.trim().parse::().ok()?)) }); - let Some((at, slot, lpi)) = parsed else { - return Err(format!("{handed:?} names no function, slot and LPI")); + let Some((at, lpi)) = parsed else { + return Err(format!("{handed:?} names no function and LPI")); }; - if lpi != 8192 + slot { - return Err(format!("slot {slot}'s LPI is {lpi}, not the {} its slot names", 8192 + slot)); + if lpi < 8192 { + return Err(format!("{at}'s interrupt {lpi} is no LPI")); } // QEMU's IORT maps a function's requester ID to the SMMUv3's StreamID and // that to the ITS's DeviceID unchanged. @@ -2611,10 +2611,10 @@ serial: let hex = |text: &str| u32::from_str_radix(text, 16).map_err(|e| format!("{text}: {e}")); let rid = hex(bus)? << 8 | hex(dev)? << 3 | hex(func)?; for want in [ - format!("ITS: {at}, DeviceID {rid:#x}, event 0 is slot {slot}'s LPI {lpi}"), - format!("pcidev: slot {slot} took its first message on LPI {lpi}"), - format!("pcidev: PCI {at} {EDU} released from slot {slot}"), - format!("ITS: {at}, DeviceID {rid:#x}, unmapped from slot {slot}"), + format!("ITS: {at}, DeviceID {rid:#x}, event 0 is LPI {lpi}"), + format!("pcidev: PCI {at} took its first message on LPI {lpi}"), + format!("pcidev: PCI {at} {EDU} released; "), + format!("ITS: {at}, DeviceID {rid:#x}, unmapped from LPI {lpi}"), ] { let Some(found) = line(&want) else { return Err(format!("the kernel never said {want:?} @@ -3827,7 +3827,7 @@ fn usbd_drives_the_spare(test_config: &Path) -> Result<(), String> { run.must_be_clean()?; run.must_say(USBD_NOOP)?; run.must_say("usbd_spare: settled: usb.events.unannounced = 0")?; - let handed = format!("pcidev: PCI {at} [{SPARE}] handed over on slot "); + let handed = format!("pcidev: PCI {at} [{SPARE}] handed over, vector "); if said.matches(&handed).count() != 1 { return Err(format!("{at} was not handed over once, to the one usbd:\n{said}")); } @@ -3842,7 +3842,7 @@ fn usbd_drives_the_spare(test_config: &Path) -> Result<(), String> { .ok_or_else(|| format!("the job never said where it aimed:\n{faulted}"))?; let record = faulted .lines() - .find(|l| l.contains("iommu: DMA FAULT owner=slot") && l.contains(&format!("stream={at} "))) + .find(|l| l.contains("iommu: DMA FAULT owner=process") && l.contains(&format!("stream={at} "))) .ok_or_else(|| format!("no DMA FAULT record names {at}:\n{faulted}"))?; let addr = record.split_once(" addr=").and_then(|(_, a)| a.split_whitespace().next()).unwrap_or_default(); if u64::from_str_radix(addr.trim_start_matches("0x"), 16).ok() != u64::from_str_radix(aimed.trim_start_matches("0x"), 16).ok() { @@ -3901,7 +3901,7 @@ fn amdvi_translates(kernel: &serial::Serial) -> Result<(), String> { /// The T14's Type-C controller left to a claim and driven by usbd: the /// kernel left it, the claim was handed over with its MSI in remappable -/// format and its remapping entry written, the message reached the slot, and +/// format and its remapping entry written, the message reached its claim, and /// the No-Op completed. What reset the function advertises is read off the /// release and printed, not judged. fn usbd_on_metal(kernel: &serial::Serial, log: &serial::Serial) -> Result<(), String> { @@ -3911,7 +3911,7 @@ fn usbd_on_metal(kernel: &serial::Serial, log: &serial::Serial) -> Result<(), St .lines() .find_map(|l| l.split_once("xHCI: leaving PCI ")?.1.strip_suffix(&format!(" {TYPE_C} to a claim (xhci-leave)")).map(str::to_string)) .ok_or_else(|| format!("the kernel never said it left {TYPE_C} alone:\n{}", kernel.text()))?; - kernel.must_say(&format!("pcidev: PCI {at} [{TYPE_C}] handed over on slot "))?; + kernel.must_say(&format!("pcidev: PCI {at} [{TYPE_C}] handed over, vector "))?; let message = kernel .text() .lines() @@ -3926,7 +3926,7 @@ fn usbd_on_metal(kernel: &serial::Serial, log: &serial::Serial) -> Result<(), St kernel.must_say(&format!(" source={at} p=1 "))?; kernel.must_say("took its first message on vector")?; log.must_say(USBD_NOOP)?; - for released in kernel.text().lines().filter(|l| l.contains(&format!("pcidev: PCI {at} [{TYPE_C}] released from slot"))) { + for released in kernel.text().lines().filter(|l| l.contains(&format!("pcidev: PCI {at} [{TYPE_C}] released; "))) { eprintln!(" [usbd] {}", released.trim()); } eprintln!(" [usbd] {at}: handed over, MSI {address:#010x} through its remapping entry, the No-Op answered");