From 8a47c46e8311b809d8366649f60a80f0bf0873ce Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 01:24:45 +0200 Subject: [PATCH 1/3] pcidev: a claim takes an interrupt and gives it back; the four-slot table goes The kernel handed at most four PCI functions to processes at once (`pcidev::MAX_FUNCTIONS`), because each architecture declared one interrupt per slot at boot: vectors 0x28-0x2B and IRTEs 0-3 on x86-64, LPIs 8192-8195 and four ITTs on AArch64. `cargo run`'s Virtio profile already claimed all four (sound, net, NVMe, the clipboard's virtio-serial), so a fifth claim was refused `Exhausted`. Ownership moves from slots to functions: - pcidev keeps one record per enumerated function, published once by `publish` and reached lock-free by the handlers: its held flag (the exclusivity decision, one compare-exchange), interrupt record, watch, domain, bound claim, retired grants and residue. The slot table, `toyos_pci::slot` and its residue-exclusion rule go: a residue is a field of its own function, so no other function can be handed it, and it holds no shared resource. - x86-64: a claim takes the lowest free vector from 0x40-0xFB (188, on cpu0, where every device interrupt is delivered) and the lowest free remapping entry, and gives both back at release. Each vector's stub pushes its number and jumps to one entry; the handler finds the function through the vector's owner. The IRT's entries are now one allocator for the kernel's own sources and claims alike. - AArch64: function n's LPI is 8192 + n; every LPI is configured enabled before EnableLPIs, and a DeviceID's ITT is made on its first claim and kept. - The fault handler asks pcidev whether a claim holds the faulting requester (`note_fault`), so `note_user_owned` and the per-stream slot number go; the record says `owner=process`. The bound is now the machine's: the functions it enumerated, cpu0's 188 claim vectors and the 256-entry IRT on x86-64, the redistributor's LPIs on AArch64. Running out is refused `Exhausted` by name (`NoVector`, `TableFull`, `NoLpi`, `NoMemory`). `kernel::owners` is the lowest-free-number allocator both x86 tables use, with host tests. Closes issues/a-cargo-run-boot-claims-every-pcidev-slot.md, and issues/a-residue-slot-is-held-for-the-boot-by-a-function-never-claimed-again.md: a residue no longer holds a slot; the second gap it described (a residue range dropped by a claim that placed nothing there) was already fixed by f854ac006, whose tear_down carries `bound.residue` forward. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy --- ...cargo-run-boot-claims-every-pcidev-slot.md | 26 -- .../a-claimed-function-can-storm-its-cpu.md | 6 +- ...claims-own-refusals-are-read-by-nothing.md | 3 +- ...-boot-by-a-function-never-claimed-again.md | 40 -- issues/toyos-runs-on-arm64.md | 2 +- kernel/pci/src/lib.rs | 1 - kernel/pci/src/slot.rs | 97 ---- kernel/pure/lib.rs | 4 +- kernel/pure/owners.rs | 101 ++++ kernel/src/arch/aarch64/irqchip.rs | 4 +- kernel/src/arch/aarch64/irqchip/its.rs | 177 ++++--- kernel/src/arch/aarch64/mod.rs | 1 - kernel/src/arch/aarch64/trap.rs | 6 +- kernel/src/arch/x86_64/idt/device_irq.rs | 17 +- kernel/src/arch/x86_64/idt/mod.rs | 17 +- kernel/src/arch/x86_64/idt/unclaimed.rs | 12 +- kernel/src/arch/x86_64/idt/user_dev.rs | 136 ++++-- kernel/src/arch/x86_64/mod.rs | 1 - kernel/src/arch/x86_64/vtd/interrupt.rs | 176 +++---- kernel/src/device.rs | 7 +- kernel/src/iommu/fault.rs | 38 +- kernel/src/iommu/mod.rs | 50 +- kernel/src/pcidev/mod.rs | 438 ++++++++++-------- system/supervisor/src/main.rs | 2 +- tests/checks/serial.rs | 4 +- tests/common/claims.rs | 4 +- tests/common/iommu.rs | 2 +- tests/common/serial.rs | 4 +- tests/toyos.rs | 42 +- 29 files changed, 740 insertions(+), 678 deletions(-) delete mode 100644 issues/a-cargo-run-boot-claims-every-pcidev-slot.md delete mode 100644 issues/a-residue-slot-is-held-for-the-boot-by-a-function-never-claimed-again.md delete mode 100644 kernel/pci/src/slot.rs create mode 100644 kernel/pure/owners.rs 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..8567a6c6323 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 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-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..c7fb35470bf 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 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 f2ca5bdb127..af353de990d 100644 --- a/kernel/pure/lib.rs +++ b/kernel/pure/lib.rs @@ -5,7 +5,8 @@ //! a CPU waiting on the boot processor's write to `SMI_CMD` decides //! ([`bootwrite`]), what the clock computes from counter ticks ([`clock`]), //! which number a kernel volume's directory answers ([`dirnodes`]), which -//! number a USB pointer's records carry ([`pointers`]) and which +//! number a USB pointer's records carry ([`pointers`]), who holds each of +//! an interrupt's numbered resources ([`owners`]) and which //! state components a CPU's `XCR0` enables ([`xsave`]). //! The kernel binary links it; the host runs //! its tests, because none of it reads a register, a clock or a kernel lock. @@ -21,6 +22,7 @@ pub mod bootwrite; pub mod clock; 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..e2f44fedf77 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,13 @@ 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, "LPI {} of its function {function} is past the LPIs the redistributor has", 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 +452,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..61de8b5ed26 100644 --- a/kernel/src/arch/x86_64/idt/user_dev.rs +++ b/kernel/src/arch/x86_64/idt/user_dev.rs @@ -1,68 +1,102 @@ -//! 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 { + idt.entries[usize::from(FIRST) + number] = 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 0000fe6a09b..e30e7465be3 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/iommu/fault.rs b/kernel/src/iommu/fault.rs index f5504386374..1aed4f7a745 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), @@ -216,15 +197,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. @@ -287,8 +267,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, } @@ -297,7 +277,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 f6afe2a5ecc..b1b4be6f381 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. @@ -282,8 +282,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(()); @@ -301,18 +301,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 { @@ -321,11 +319,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 { @@ -335,19 +338,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. @@ -363,18 +367,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..5ebec01b183 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 +//! **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 slot's -//! [`RESIDUE`]. +//! 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. @@ -73,9 +73,9 @@ //! 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 +//! 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 @@ -91,34 +91,36 @@ //! **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 +//! **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 slot, by the interrupt and the requester the +//! 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 +128,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 +140,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 +165,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 +255,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 +271,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 +292,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 +329,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 +376,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 +398,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 +428,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 +453,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 +577,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 +767,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 +788,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 +800,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 +819,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 +840,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 +854,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 +870,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 +880,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 +990,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 +1216,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 +1226,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 +1486,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 +1520,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 +1544,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 +1553,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 +1567,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 +1585,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 +1621,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 +1635,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 +1672,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 +1680,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 +1704,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 +1718,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 +1741,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 +1750,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 +1786,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 +1802,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/system/supervisor/src/main.rs b/system/supervisor/src/main.rs index 408390b4947..5d1f4b49c4f 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:` or `acpi:` 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 147d89c3e86..add25e87bad 100644 --- a/tests/common/claims.rs +++ b/tests/common/claims.rs @@ -65,10 +65,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}"); records(log).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 eb9625969d9..31fee2535e7 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 f63a7321718..1ee3623049b 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -2424,13 +2424,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 @@ -2464,7 +2464,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 @@ -2472,23 +2472,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. @@ -2497,10 +2497,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:?} @@ -3689,7 +3689,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}")); } @@ -3704,7 +3704,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() { @@ -3730,7 +3730,7 @@ fn usbd_drives_the_spare(test_config: &Path) -> 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> { @@ -3740,7 +3740,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() @@ -3755,7 +3755,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"); From a97d4c09ffb9d3f717e95725911ea5e1b696eb64 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 02:31:51 +0200 Subject: [PATCH 2/3] issues: a released function never claimed again keeps its retired grants Filed as a defect the orchestrator ruled to record, not accept: a reset, released function's retired grants (up to MAX_GRANT_TOTAL) are freed only by that function's next claim. Before the slot table went, the next claim of the same slot freed them, whichever function it was for. Owned by the device lifecycle track, whose stage 2 ends a function's claim with `Gone`. The late-write issue, merged from main, named `note_user_owned`, which this branch removed: the ownership it describes is now `Function::held`, cleared at the end of `release`, still before the reset is quiet, so the defect stands. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy --- ...m-a-released-function-halts-the-machine.md | 6 ++--- ...-claimed-again-keeps-its-retired-grants.md | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 issues/a-released-function-never-claimed-again-keeps-its-retired-grants.md 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. From 2a43d55b01a31309aee2f400921b34a613b2a319 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 14 Oct 2026 02:40:29 +0200 Subject: [PATCH 3/3] pcidev: review round 1: the records say what the code does - record.rs: the one-handler invariant is restated for a claim's one vector or LPI on cpu0. `Interrupt::new` stays `const` for its true reason: `isa`'s rows are a static array of these records; pcidev's are built at runtime in `publish`. - drivers/pci.rs: the claimed MSI-X entry is aimed at `claim_msi`'s message. - user_dev::install asserts each claim vector's gate is still empty, so a future `idt_vectors!` row in 0x40..0xFC is refused rather than overwritten. - The ITS's `NoLpi` reads "function n's LPI m is past the redistributor's LPIs". - pcidev's header paragraphs this change touched are rewrapped. Issues: - Closes a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md. Its hazard was a handler that read a slot number, then wrote that slot's record after the slot was bound to a second function. Records are now per function and never change hands: the x86 handler reads the function from its vector's owner, the AArch64 one from its LPI by arithmetic, and `note_fault` resolves by requester. A late write reaches only the same function's record, which its next claim clears at hand-over. - Closes a-released-function-still-names-its-slots-domain.md. Its hazard was a released function translating through the next holder's grants on a shared slot domain. The domain is now its function's own, so the next holder attached to it is that function's own next claim. Until then it maps nothing but the released holder's retired grants. - a-claimed-function-can-storm-its-cpu.md and toyos-runs-on-arm64.md: the remaining "slot" wording becomes the claim's or the function's own. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy --- .../a-claimed-function-can-storm-its-cpu.md | 2 +- ...t-it-read-before-the-slot-changed-hands.md | 35 ----------- ...d-function-still-names-its-slots-domain.md | 19 ------ issues/toyos-runs-on-arm64.md | 6 +- kernel/src/arch/aarch64/irqchip/its.rs | 6 +- kernel/src/arch/x86_64/idt/user_dev.rs | 4 +- kernel/src/drivers/pci.rs | 5 +- kernel/src/pcidev/mod.rs | 59 +++++++++---------- kernel/src/pcidev/record.rs | 11 ++-- 9 files changed, 50 insertions(+), 97 deletions(-) delete mode 100644 issues/a-handler-tells-a-slot-it-read-before-the-slot-changed-hands.md delete mode 100644 issues/a-released-function-still-names-its-slots-domain.md diff --git a/issues/a-claimed-function-can-storm-its-cpu.md b/issues/a-claimed-function-can-storm-its-cpu.md index 8567a6c6323..ce6e329bd36 100644 --- a/issues/a-claimed-function-can-storm-its-cpu.md +++ b/issues/a-claimed-function-can-storm-its-cpu.md @@ -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-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-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/toyos-runs-on-arm64.md b/issues/toyos-runs-on-arm64.md index c7fb35470bf..351ab118fee 100644 --- a/issues/toyos-runs-on-arm64.md +++ b/issues/toyos-runs-on-arm64.md @@ -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/src/arch/aarch64/irqchip/its.rs b/kernel/src/arch/aarch64/irqchip/its.rs index e2f44fedf77..162fdba89be 100644 --- a/kernel/src/arch/aarch64/irqchip/its.rs +++ b/kernel/src/arch/aarch64/irqchip/its.rs @@ -428,7 +428,11 @@ 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, "LPI {} of its function {function} is past the LPIs the redistributor has", lpi::FIRST as usize + function), + 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"), } } diff --git a/kernel/src/arch/x86_64/idt/user_dev.rs b/kernel/src/arch/x86_64/idt/user_dev.rs index 61de8b5ed26..ceea39e8e66 100644 --- a/kernel/src/arch/x86_64/idt/user_dev.rs +++ b/kernel/src/arch/x86_64/idt/user_dev.rs @@ -59,7 +59,9 @@ pub(crate) fn give_back(vector: ClaimVector) { pub(super) fn install(idt: &mut Idt) { let first = (stubs as *const () as usize).next_multiple_of(STRIDE); for number in 0..COUNT { - idt.entries[usize::from(FIRST) + number] = IdtEntry::at_address((first + number * STRIDE) as u64); + 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); } } 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/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index 5ebec01b183..06c541f6116 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -14,15 +14,15 @@ //! claim. //! //! **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`]. +//! 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,22 +71,21 @@ //! 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 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'. +//! 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 @@ -97,12 +96,12 @@ //! //! **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. +//! 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 function's watch only while its claim holds it.** It is //! registered with the binding ([`add_poll`]), so before the release, whose 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);