From e81d26db80c67c0591c9a833dcde7cec2b37cb0d Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 04:20:48 +0200 Subject: [PATCH 1/8] IOMMU stage 1's instruments: the iommu-no-remap actuator, pci_reclaim and three T14 rows Lands ahead of the stage-1 change on the same branch, so each row's negative control is that change reverted onto this commit: the rows' controls depend on an actuator the change would otherwise bring with it. - `iommu-no-remap` makes `vtd::remappable` answer None, as on a machine whose units cannot remap; admitted on the T14 since it writes no register a reset does not return to firmware. - `pci_reclaim` claims the T14's I219, gives it back and claims it again. - `claim_reuses_its_remapping_entry` and `domain_ends_below_the_host_bridges` ride the tests/testcases boot; `claim_refused_without_remapping` is its own boot under the actuator. Their judges are held against the T14's recorded irte5 -> irte6 re-claim and its 0x2000000000..0x8000000000 domain. - Files a-domains-addresses-overlap-the-host-bridges-mmio.md, and amends the exit of a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md to the metal row: once one re-claim reuses its entry the claim count moves nothing, and a metal row reaches a real function's claim before a guest test. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...addresses-overlap-the-host-bridges-mmio.md | 29 ++++ ...ends-a-remapping-entry-it-never-returns.md | 20 ++- kernel/src/actuator.rs | 4 + kernel/src/arch/x86_64/vtd/mod.rs | 8 + src/metal.rs | 4 + tests/checks.rs | 12 ++ tests/checks/claims.rs | 52 +++++++ tests/common/claims.rs | 144 ++++++++++++++++++ tests/common/mod.rs | 2 + tests/toyos-rust-tests/src/bin/pci_reclaim.rs | 27 ++++ tests/toyos.rs | 40 ++++- 11 files changed, 335 insertions(+), 7 deletions(-) create mode 100644 issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md create mode 100644 tests/checks/claims.rs create mode 100644 tests/common/claims.rs create mode 100644 tests/toyos-rust-tests/src/bin/pci_reclaim.rs diff --git a/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md b/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md new file mode 100644 index 00000000000..7c8c70928b4 --- /dev/null +++ b/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md @@ -0,0 +1,29 @@ +--- +status: assigned +kind: defect +opened: 2026-10-04 +--- + +# A domain's device addresses overlap the root bridges' memory windows + +`vtd::table::Domain` hands out device addresses from `1 << (translatable - 2)` +up to `1 << translatable`, and checks only that the floor is above RAM +(`pmm::top()`). Nothing holds the range against the windows firmware declared +the root bridges decode (`KernelArgs::root_bridge_windows`). + +On the T14 `MGAW` is 39, so every domain logs `addresses from 0x2000000000 to +0x8000000000`, and firmware declares `mem 0x4000000000..0x603dc00000` and `mem +0x603dc00000..0x8000000000`; the iGPU's `bar2` sits at `0x4000000000`. The +upper half of every domain's range is an address a bridge or switch below a +root port may route peer-to-peer before the unit sees it (PCIe Base §2.4), so +a descriptor carrying one reaches another function's registers rather than +faulting. A claim domain reaches it after about 4,096 claim/release cycles of +one slot (`issues/kernel/a-claim-spends-device-addresses-its-slot-never-gets-back.md`); +on the T14 it matters below the Thunderbolt root ports with a dock attached. + +Owner: the IOMMU track's stage 1, branch `wt/toyos-iommu1`. Exit: a domain's +range ends below the first root-bridge window or reserved region that reaches +above its floor, and a domain with no room left there is refused — held by a +`const` assertion over the T14's windows, and by the `domain_ends_below_the_host_bridges` +metal row, which reds on a domain record whose range meets a window firmware +declared. diff --git a/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md b/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md index d200855ab15..6b9e3fefcc5 100644 --- a/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md +++ b/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md @@ -1,5 +1,5 @@ --- -status: open +status: assigned kind: defect opened: 2026-09-24 --- @@ -25,8 +25,16 @@ on the machine is refused (`TableFull`, which `pcidev` reports as `NoInterrupt`), so a swap then ends with the service `gone` — and so does every later claim of any function, by any process. -Owner: the swap's author, because the swap is what turned a constant into a -leak. Exit condition: a released claim returns its entry — freed at -`pcidev::release`, or one entry kept per `pcidev` slot and rewritten on each -claim, which bounds the table's use by `MAX_FUNCTIONS` — and a guest test that -claims and releases one function more than 256 times in a boot stays armed. +The T14 recorded it on a later boot: `irte5` for `00:1f.6` at its first claim, +and `irte6` for the same function when the swap claimed it again. + +Owner: the IOMMU track's stage 1, branch `wt/toyos-iommu1`. Exit condition: a +released claim returns its entry — freed at `pcidev::release`, or one entry +kept per `pcidev` slot and rewritten on each claim, which bounds the table's +use by `MAX_FUNCTIONS` — and the `claim_reuses_its_remapping_entry` metal row +is green: the T14's I219 claimed, released and claimed again in one boot writes +one entry for both claims and leaves it not present after each release. Once +one re-claim reuses its entry, the count of claims in a boot moves nothing, so +a guest test of more than 256 is not owed; and a metal row reaches a real +function's claim before a guest test does, which is the tier root `CLAUDE.md` +puts first. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index a345b957429..10043772944 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -117,6 +117,10 @@ actuators! { /// kernel programs it. Judged by `iommu_firmware_left`. iommu_firmware_left = "iommu-firmware-left"; + /// Leave every IOMMU unit's interrupts unremapped, as on a machine whose + /// units cannot remap them. Judged by `claim_refused_without_remapping`. + iommu_no_remap = "iommu-no-remap"; + /// Leave every AP holding the CR0/CR4 that INIT left it. no_ap_control_regs = "no-ap-control-regs"; diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 2b69d91e2be..29469d70065 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -255,6 +255,14 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { /// interrupt to whatever the handle bits spell. Every condition below therefore /// refuses for the machine, not for the unit that failed it. fn remappable(ready: &[(Unit, Plan)], described: usize) -> Option { + #[cfg(feature = "boot-actuators")] + if crate::actuator::iommu_no_remap() { + log!( + "iommu: iommu-no-remap stands in for units that cannot remap, so every source stays \ + in compatibility format" + ); + return None; + } if ready.is_empty() { return None; } diff --git a/src/metal.rs b/src/metal.rs index a88a7c8214d..98e814ecbe7 100644 --- a/src/metal.rs +++ b/src/metal.rs @@ -772,6 +772,10 @@ pub const FLASHABLE: &[&str] = &[ // writes only the units' registers, which a reset returns to firmware, and // tables in memory this boot owns. "iommu-firmware-left", + // Every IOMMU unit left without interrupt remapping, which is how this + // machine ran before the kernel remapped anything. It writes no register a + // reset does not return to firmware. + "iommu-no-remap", // It seals this boot's own record under an identity one bit from this // stick's, so the pass that finds it clears it and boots a kernel. The page // is memory the loader allocated and the machine is what it was after. diff --git a/tests/checks.rs b/tests/checks.rs index db34f7fc875..0ea4a8c49fb 100644 --- a/tests/checks.rs +++ b/tests/checks.rs @@ -12,6 +12,8 @@ mod checks { #[path = "audio.rs"] mod audio_checks; + #[path = "claims.rs"] + mod claims_checks; #[path = "clock.rs"] mod clock_checks; #[path = "lan.rs"] @@ -795,6 +797,16 @@ mod checks { usb_checks::transport_break_verdict() } + #[test] + fn metal_claim_spends_one_remapping_entry() { + claims_checks::one_entry_per_slot(); + } + + #[test] + fn metal_domains_end_below_the_host_bridges() { + claims_checks::domains_end_below_the_windows(); + } + #[test] fn metal_lease_judged_is_this_boots_own() { lan_checks::the_lease_judged_is_this_boots_own(); diff --git a/tests/checks/claims.rs b/tests/checks/claims.rs new file mode 100644 index 00000000000..040be76be02 --- /dev/null +++ b/tests/checks/claims.rs @@ -0,0 +1,52 @@ +//! The claimed-function judges over the T14's own records: the recorded boot +//! whose re-claim spent a second remapping entry, and whose domains reached +//! into a root-bridge window, reds; the same boot with one entry and a capped +//! window passes. + +use super::*; + +/// The T14's records of one boot, verbatim but for the readback's `stream|` +/// prefix: the I219 enumerated, firmware's root-bridge windows, a domain, and +/// the I219 claimed, released and claimed again. +const RECORDED: &str = r"[2026-09-29 11:54:39 0.065 cpu0] PCI 00:1f.6 [0200] vendor=8086 device=15fc prog_if=00 bars=[bar0=0xbcf00000] +[2026-09-29 11:54:39 0.149 cpu0] pcidev: firmware declared root bridge memory: mem 0xa2000000..0xbd000000, mem 0x4000000000..0x603dc00000, mem 0xa0800000..0xa2000000, mem 0xbd000000..0xc0000000, mem 0xff000000..0xffb80000, mem 0xffd3a070..0x100000000, mem 0x603dc00000..0x8000000000 +[2026-09-29 11:54:39 0.250 cpu0] iommu: domain2 root=0xa31000 aw=48 mgaw=39 addresses from 0x2000000000 to 0x8000000000 +[2026-09-29 12:10:34 1.191 cpu0] iommu: irte5 source=00:1f.6 p=1 sid=0x00fe svt=1 sq=0 vector=0x28 apic=0x0 dst=0x0 trigger=edge +[2026-09-29 12:10:34 1.192 cpu0] pcidev: PCI 00:1f.6 [8086:15fc] handed over on slot 0, vector 0x28 +[2026-09-29 12:10:53 20.233 cpu1] pcidev: PCI 00:1f.6 [8086:15fc] released from slot 0; reset by nothing (Express: no capability; AF: no capability; PM: No_Soft_Reset set), so where it may still be aimed is kept for its next claim +[2026-09-29 12:10:53 20.238 cpu0] iommu: irte6 source=00:1f.6 p=1 sid=0x00fe svt=1 sq=0 vector=0x28 apic=0x0 dst=0x0 trigger=edge +[2026-09-29 12:10:53 20.239 cpu0] pcidev: PCI 00:1f.6 [8086:15fc] handed over on slot 0, vector 0x28 +"; + +/// The second release, which that boot never reached. +const RELEASED: &str = "[2026-09-29 12:10:55 22.000 cpu1] pcidev: PCI 00:1f.6 [8086:15fc] released from slot 0; reset by nothing\n"; + +fn log(text: &str) -> serial::Serial { + serial::Serial::named("T14 log", text) +} + +/// One entry for both claims, not present after each release; the recorded +/// second entry, and a release that leaves its entry present, red. +pub fn one_entry_per_slot() { + let cleared = "[2026-09-29 12:10:53 20.234 cpu1] iommu: irte0 source=00:1f.6 p=0 released\n"; + let green = format!("{RECORDED}{cleared}{RELEASED}{cleared}") + .replace("irte5 ", "irte0 ") + .replace("irte6 ", "irte0 "); + assert_eq!(claims::reuses_its_entry(&log(&green)), Ok(())); + + let recorded = format!("{RECORDED}{RELEASED}"); + let why = claims::reuses_its_entry(&log(&recorded)).expect_err("the recorded second entry"); + assert!(why.contains(r#"wrote remapping entries ["5", "6"]"#), "{why}"); + + let present = green.replace("p=0 released", "p=1 released"); + let why = claims::reuses_its_entry(&log(&present)).expect_err("an entry left present"); + assert!(why.contains(r#"left (entry, p) [("0", "1"), ("0", "1")]"#), "{why}"); +} + +/// The recorded domain reaches into `0x4000000000..`; capped there, it passes. +pub fn domains_end_below_the_windows() { + let why = claims::clear_of_host_bridges(&log(RECORDED)).expect_err("the recorded domain"); + assert!(why.contains("inside the root-bridge window 0x4000000000..0x603dc00000"), "{why}"); + let capped = RECORDED.replace("to 0x8000000000", "to 0x4000000000"); + assert_eq!(claims::clear_of_host_bridges(&log(&capped)), Ok(())); +} diff --git a/tests/common/claims.rs b/tests/common/claims.rs new file mode 100644 index 00000000000..b5eaddd7161 --- /dev/null +++ b/tests/common/claims.rs @@ -0,0 +1,144 @@ +//! A claimed function at the unit, on the T14: the remapping entry each claim +//! of it writes and each release takes back, and the window its domain hands +//! addresses out of. Every judge reads the kernel's own records. + +use toyos_build::bootlog; +use toyos_build::lan::I219; + +use super::serial::Serial; + +/// The job that claims the T14's I219, gives it back and claims it again. +pub const RECLAIM: &str = "test_rs_pci_reclaim"; + +/// The kernel's reason on a claim's refusal line where the units do not remap. +const NOT_REMAPPED: &str = "its interrupts would not be remapped on this machine"; + +/// The kernel's line under `iommu-no-remap`: the premise of the refusal. +const NO_REMAP_ARMED: &str = "iommu: iommu-no-remap stands in for units that cannot remap"; + +fn records(log: &Serial) -> impl Iterator { + log.text().lines().filter_map(bootlog::message) +} + +/// One `key=value` word of a record. +fn field<'a>(record: &'a str, key: &str) -> Option<&'a str> { + record.split_whitespace().find_map(|word| word.strip_prefix(key)?.strip_prefix('=')) +} + +/// Where the I219 is, off its enumeration record, so no judge here names the +/// T14's address for it. +fn function(log: &Serial) -> Result { + let (vendor, device) = I219.split_once(':').expect("an id is vendor:device"); + let id = format!(" vendor={vendor} device={device} "); + let mut found = records(log).filter(|m| m.contains(&id)).filter_map(|m| { + m.trim_start().strip_prefix("PCI ")?.split_whitespace().next().map(str::to_string) + }); + match (found.next(), found.next()) { + (Some(at), None) => Ok(at), + (None, _) => Err(format!("no enumeration record names {I219}:\n{}", log.text())), + (Some(_), Some(_)) => Err(format!("two enumeration records name {I219}:\n{}", log.text())), + } +} + +/// Both claims of the I219 wrote one remapping entry, and each release left +/// that entry not present. +pub fn reuses_its_entry(log: &Serial) -> Result<(), String> { + let at = function(log)?; + let mut written = Vec::new(); + let mut cleared = Vec::new(); + for record in records(log) { + let Some(rest) = record.strip_prefix("iommu: irte") else { continue }; + if field(record, "source") != Some(at.as_str()) { + continue; + } + let index = rest.split_whitespace().next().unwrap_or_default().to_string(); + let present = field(record, "p") + .ok_or_else(|| format!("a remapping record with no p=: {record:?}"))? + .to_string(); + if record.ends_with(" released") { + cleared.push((index, present)); + } else { + written.push(index); + } + } + let count = |what: &str| { + let needle = format!("[{I219}] {what} slot "); + records(log).filter(|m| m.starts_with("pcidev: PCI ") && m.contains(&needle)).count() + }; + let (handed, released) = (count("handed over on"), count("released from")); + if (handed, released) != (2, 2) { + return Err(format!( + "{I219} was handed over {handed} time(s) and released {released}, want 2 and 2:\n{}", + log.text() + )); + } + match written.as_slice() { + [first, second] if first == second => {} + _ => { + return Err(format!( + "the two claims of {at} wrote remapping entries {written:?}, want one entry twice" + )) + } + } + let want = (written[0].clone(), "0".to_string()); + if cleared != [want.clone(), want] { + return Err(format!( + "the two releases of {at} left (entry, p) {cleared:?}, want irte{} at p=0 twice", + written[0] + )); + } + eprintln!(" [claims] {at}: irte{} written by both claims, not present after each release", written[0]); + Ok(()) +} + +/// Every domain's addresses lie outside every root-bridge window firmware +/// declared: a bridge may route a request in one of those before the unit +/// sees it. +pub fn clear_of_host_bridges(log: &Serial) -> Result<(), String> { + const DECLARED: &str = "pcidev: firmware declared root bridge memory: "; + let declared = records(log) + .find_map(|m| m.strip_prefix(DECLARED)) + .ok_or_else(|| format!("no `{DECLARED}` record:\n{}", log.text()))?; + let range = |text: &str| -> Option<(u64, u64)> { + let (from, to) = text.split_once("..")?; + let hex = |t: &str| u64::from_str_radix(t.trim().strip_prefix("0x")?, 16).ok(); + Some((hex(from)?, hex(to)?)) + }; + let windows = declared + .split(", ") + .map(|w| w.strip_prefix("mem ").and_then(range).ok_or_else(|| format!("{w:?} in {declared:?}"))) + .collect::, _>>()?; + let mut domains = 0; + for record in records(log).filter(|m| m.starts_with("iommu: domain")) { + let Some((_, span)) = record.split_once(" addresses from ") else { continue }; + let (floor, ceiling) = span + .split_once(" to ") + .and_then(|(from, to)| range(&format!("{from}..{to}"))) + .ok_or_else(|| format!("an unreadable domain record: {record:?}"))?; + if let Some((base, end)) = windows.iter().find(|(base, end)| *end > floor && *base < ceiling) { + return Err(format!( + "{record:?} hands out addresses inside the root-bridge window {base:#x}..{end:#x}" + )); + } + domains += 1; + } + if domains == 0 { + return Err(format!("no domain was made on this boot:\n{}", log.text())); + } + eprintln!(" [claims] {domains} domain(s), none reaching a root-bridge window"); + Ok(()) +} + +/// On a machine whose units do not remap, the I219's claim is refused by that +/// reason before anything on the function is armed, and the boot completes. +pub fn refused_unremapped(log: &Serial) -> Result<(), String> { + log.must_say(NO_REMAP_ARMED)?; + let at = function(log)?; + log.must_say(&format!("pcidev: PCI {at} NOT HANDED OVER — {NOT_REMAPPED}"))?; + log.must_not_say(&format!("[{I219}] handed over"))?; + log.must_not_say(&format!("PCI {at}: msi address="))?; + log.must_not_say(&format!("PCI {at}: msix address="))?; + log.must_not_say(&format!(" source={at} "))?; + log.must_say(bootlog::COMPLETE)?; + Ok(()) +} diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 11a7116b285..9d19545e7ea 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1,4 +1,6 @@ pub mod audio; +/// A claimed function at the unit, on the T14. +pub mod claims; pub mod clock; pub mod lane; pub mod compile; diff --git a/tests/toyos-rust-tests/src/bin/pci_reclaim.rs b/tests/toyos-rust-tests/src/bin/pci_reclaim.rs new file mode 100644 index 00000000000..db180f18fba --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/pci_reclaim.rs @@ -0,0 +1,27 @@ +//! The T14's I219 claimed, given back and claimed again in one boot: its exit +//! is whether both claims were handed over. What each claim spent at the unit +//! is the kernel's to say, and the metal rows read it there. + +use toyos::endow::Endowments; +use toyos::syscap::SysCap; +use toyos::PciDev; +use toyos_abi::syscall::{PciId, SYSCAP_LABEL}; + +const I219: PciId = PciId { vendor: 0x8086, device: 0x15fc }; + +fn main() { + let cap: SysCap = Endowments::get() + .take(SYSCAP_LABEL) + .expect("the test estate is endowed a device-minting capability"); + for claim in 1..=2 { + match cap.claim_pci::(I219) { + // Dropped at once: the release is what the second claim follows. + Ok(function) => drop(function), + Err(why) => { + println!("pci_reclaim: claim {claim} of 8086:15fc was refused: {why:?}"); + std::process::exit(1); + } + } + } + println!("pci_reclaim: 8086:15fc was handed over and given back twice"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 9e55a58846d..bb04b2e405b 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -14,7 +14,7 @@ use common::qemu::{ self, await_guest, await_marker, BootOptions, QemuInstance, STALLED, TIMED_OUT, }; -use common::{audio, compile, devices, faults, isa, lan, metal, power, screen, serial, usb}; +use common::{audio, claims, compile, devices, faults, isa, lan, metal, power, screen, serial, usb}; use toyos_build::bootlog::{self}; use toyos_build::testargs::{self, SUITE}; @@ -93,6 +93,10 @@ const RUST_SKIP: &[&str] = &[ // metal rows run them. "isa_grant", "isa_lines", + // It claims the T14's I219, which no guest has: the + // `claim_reuses_its_remapping_entry` and `claim_refused_without_remapping` + // metal rows run it. + "pci_reclaim", // It takes the machine down; `virt_fatal_halts_the_others_first` runs it. "panic_halts_first", // Needs a launcher and a declared `cat` and shell, which `tests/testcases` @@ -656,6 +660,39 @@ const METAL: &[(&str, metal::Metal)] = &[ "iommu_firmware_left", metal::Metal { arms: SELFTESTS, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, ), + // ---- a claimed function at the unit, on tests/testcases ---- + ( + // Claimed, given back and claimed again: both claims name one + // remapping entry, and each release leaves it not present. + "claim_reuses_its_remapping_entry", + metal::Metal { + arms: TESTCASES, + judge: |b| { + b[0].job_passed(claims::RECLAIM)?; + claims::reuses_its_entry(&b[0].kernel()) + }, + }, + ), + ( + // Every domain's addresses end below the first root-bridge window + // above where they start. + "domain_ends_below_the_host_bridges", + metal::Metal { arms: TESTCASES, judge: |b| claims::clear_of_host_bridges(&b[0].kernel()) }, + ), + ( + // A machine whose units do not remap: the claim is refused before + // anything on the function changes, and the boot goes on. + "claim_refused_without_remapping", + metal::Metal { + arms: &[metal::once( + "iommu-no-remap", + "tests/testcases", + &["iommu-no-remap"], + &[claims::RECLAIM], + )], + judge: |b| claims::refused_unremapped(&b[0].kernel()), + }, + ), // ---- the `isa` claim: one image whose i8042 the kernel leaves alone ---- ( // The I/O permission bitmap on the machine's own processor: the ports @@ -717,6 +754,7 @@ const TESTCASES: &[metal::Arm] = &[metal::once( "test_rs_hda_client_stall", "test_rs_syscall_cost", "test_rs_null_sink_client_exits", + claims::RECLAIM, ], )]; From 16aeb9174107c55510fc967fdfbea3bad5ffac9f Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 04:33:10 +0200 Subject: [PATCH 2/8] IOMMU stage 1: a claim speaks only through its own remapping entry, in a window clear of the host bridges - A claimed function is armed only with a `Remapped`: claim slot `slot`'s own remapping entry, written for that function by `iommu::claim_msi`, which takes a `Remapping` witness only `iommu::remapping()` mints. The arming that can produce a compatibility-format message (`enable_msix`, `enable_msi`) is `pub(in crate::drivers)`, so `pcidev` cannot reach it; its arming takes the message and nothing else (`arm_claimed_msix`, `arm_claimed_msi`). - `Refusal::NotRemapped` is taken first in `bring_up`, before the retired holder, the reset wait or the slot's domain is touched. Dropping a `Remapped` makes its entry not present, low half first, and invalidates the interrupt entry cache: every refusal after the entry is written, and every release, leaves the function no entry. - The interrupt table's first `pcidev::MAX_FUNCTIONS` entries are the claim slots', passed in at `interrupt::arm`; kernel sources allocate after them. A re-claim writes the slot's entry again, so the table no longer leaks one per claim (closes a-reclaimed-function-spends-a-remapping-entry-it-never- returns.md; recorded on the T14 as irte5 then irte6 for 00:1f.6). The invalidation is the existing global IEC descriptor: claims are rare. - `remappable` refuses on a DMAR whose INTR_REMAP flag is clear, logged by the flags' value. X2APIC_OPT_OUT is about turning x2APIC on, which happens before the DMAR is read and outside this change: filed as x2apic-is-enabled-without-reading-firmwares-opt-out.md. - A domain's addresses end below the first root-bridge window or RMRR that reaches above its floor (one `min`, `table::ceiling`), and a domain with less room than its creator asks is refused with no id spent (`IommuError::NoRoom`); `DeviceSpace::own(room)` hands the lend window out with the domain. On the T14 every domain ends at 0x4000000000, not 0x8000000000 (closes a-domains-addresses-overlap-the-host-bridges-mmio.md). `DeviceSpace::reserve` and its backends lose their only caller and go. - The fault handler's function table is a leaked slice of exactly the enumerated functions, published once before any unit is armed: the 64-function ceiling and its log go, and a claimed function is always one the handler can stop. Structural; no machine in reach has more than 64. - Files a-claimed-function-can-storm-its-cpu.md. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- .../a-claimed-function-can-storm-its-cpu.md | 27 ++++ ...addresses-overlap-the-host-bridges-mmio.md | 29 ---- ...ends-a-remapping-entry-it-never-returns.md | 40 ----- ...abled-without-reading-firmwares-opt-out.md | 24 +++ kernel/src/arch/aarch64/iommu_unit.rs | 21 ++- kernel/src/arch/x86_64/vtd/domain.rs | 33 ++-- kernel/src/arch/x86_64/vtd/fault.rs | 83 +++++----- kernel/src/arch/x86_64/vtd/interrupt.rs | 153 +++++++++++++----- kernel/src/arch/x86_64/vtd/mod.rs | 25 ++- kernel/src/arch/x86_64/vtd/table.rs | 95 ++++++++--- kernel/src/drivers/hda.rs | 2 +- kernel/src/drivers/pci.rs | 92 ++++++++--- kernel/src/drivers/virtio.rs | 13 +- kernel/src/drivers/virtio_sound.rs | 2 +- kernel/src/drivers/xhci/wait/boot.rs | 2 +- kernel/src/iommu/mod.rs | 130 ++++++++++++--- kernel/src/main.rs | 2 +- kernel/src/pcidev/mod.rs | 85 ++++++---- 18 files changed, 560 insertions(+), 298 deletions(-) create mode 100644 issues/kernel/a-claimed-function-can-storm-its-cpu.md delete mode 100644 issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md delete mode 100644 issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md create mode 100644 issues/kernel/x2apic-is-enabled-without-reading-firmwares-opt-out.md diff --git a/issues/kernel/a-claimed-function-can-storm-its-cpu.md b/issues/kernel/a-claimed-function-can-storm-its-cpu.md new file mode 100644 index 00000000000..213d20791c9 --- /dev/null +++ b/issues/kernel/a-claimed-function-can-storm-its-cpu.md @@ -0,0 +1,27 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A claimed function can storm its CPU + +A function a process drives raises `pcidev::VECTORS[slot]` on cpu0 for every +message it sends (`kernel/src/pcidev/mod.rs`, `isr`), and nothing bounds how +often. Its holder programs the device through the BARs it maps, so a hostile +or broken holder can make the device send messages as fast as the bus carries +them, and each one is an interrupt cpu0 takes before anything else it runs. +`issues/kernel/an-xhci-storm-starves-the-cpu-that-takes-it.md` measured what +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 remapping entry: making it not present stops the +function's messages at the unit, where masking through the device would need +the holder's cooperation. What rate is a storm is not decided, and a number +chosen without a measurement behind it is the silent decision this file +exists to refuse. + +Owner: the IOMMU track. Exit: a claimed function's messages past a bound the +kernel derives and states are stopped at its entry and recorded against its +claim, and a T14 row with a holder that provokes them shows cpu0's timer and +other sources still served. diff --git a/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md b/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md deleted file mode 100644 index 7c8c70928b4..00000000000 --- a/issues/kernel/a-domains-addresses-overlap-the-host-bridges-mmio.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -status: assigned -kind: defect -opened: 2026-10-04 ---- - -# A domain's device addresses overlap the root bridges' memory windows - -`vtd::table::Domain` hands out device addresses from `1 << (translatable - 2)` -up to `1 << translatable`, and checks only that the floor is above RAM -(`pmm::top()`). Nothing holds the range against the windows firmware declared -the root bridges decode (`KernelArgs::root_bridge_windows`). - -On the T14 `MGAW` is 39, so every domain logs `addresses from 0x2000000000 to -0x8000000000`, and firmware declares `mem 0x4000000000..0x603dc00000` and `mem -0x603dc00000..0x8000000000`; the iGPU's `bar2` sits at `0x4000000000`. The -upper half of every domain's range is an address a bridge or switch below a -root port may route peer-to-peer before the unit sees it (PCIe Base §2.4), so -a descriptor carrying one reaches another function's registers rather than -faulting. A claim domain reaches it after about 4,096 claim/release cycles of -one slot (`issues/kernel/a-claim-spends-device-addresses-its-slot-never-gets-back.md`); -on the T14 it matters below the Thunderbolt root ports with a dock attached. - -Owner: the IOMMU track's stage 1, branch `wt/toyos-iommu1`. Exit: a domain's -range ends below the first root-bridge window or reserved region that reaches -above its floor, and a domain with no room left there is refused — held by a -`const` assertion over the T14's windows, and by the `domain_ends_below_the_host_bridges` -metal row, which reds on a domain record whose range meets a window firmware -declared. diff --git a/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md b/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md deleted file mode 100644 index 6b9e3fefcc5..00000000000 --- a/issues/kernel/a-reclaimed-function-spends-a-remapping-entry-it-never-returns.md +++ /dev/null @@ -1,40 +0,0 @@ ---- -status: assigned -kind: defect -opened: 2026-09-24 ---- - -# A re-claimed PCI function spends an interrupt remapping entry it never returns - -`iommu::vtd::interrupt::allocate` (`kernel/src/arch/x86_64/vtd/interrupt.rs`) hands -out the next entry of a 256-entry table (`ENTRIES`) by bumping `used`, and no -path gives one back. Every `pcidev` claim arms MSI or MSI-X through -`interrupt::msi`, so every claim of a function takes a new entry, and its -release keeps it. - -A boot claims each function once, so this was a constant cost until the -service swap (`toyos-swap`) made re-claiming a function routine: each swap -releases a service's claims and mints them again for the replacement. The -review of the swap's pull request (#484) read, on the T14's run 123 log, the -highest entry at `irte5` after boot and `irte6` after one swap; by its reading -of the code, a swap that goes into service costs one entry per claimed -function, and a failed one that restarts the old binary costs two. - -**What it costs when it runs out:** once `used` reaches 256, every MSI claim -on the machine is refused (`TableFull`, which `pcidev` reports as -`NoInterrupt`), so a swap then ends with the service `gone` — and so does -every later claim of any function, by any process. - -The T14 recorded it on a later boot: `irte5` for `00:1f.6` at its first claim, -and `irte6` for the same function when the swap claimed it again. - -Owner: the IOMMU track's stage 1, branch `wt/toyos-iommu1`. Exit condition: a -released claim returns its entry — freed at `pcidev::release`, or one entry -kept per `pcidev` slot and rewritten on each claim, which bounds the table's -use by `MAX_FUNCTIONS` — and the `claim_reuses_its_remapping_entry` metal row -is green: the T14's I219 claimed, released and claimed again in one boot writes -one entry for both claims and leaves it not present after each release. Once -one re-claim reuses its entry, the count of claims in a boot moves nothing, so -a guest test of more than 256 is not owed; and a metal row reaches a real -function's claim before a guest test does, which is the tier root `CLAUDE.md` -puts first. diff --git a/issues/kernel/x2apic-is-enabled-without-reading-firmwares-opt-out.md b/issues/kernel/x2apic-is-enabled-without-reading-firmwares-opt-out.md new file mode 100644 index 00000000000..d1e0397c302 --- /dev/null +++ b/issues/kernel/x2apic-is-enabled-without-reading-firmwares-opt-out.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# x2APIC is enabled without reading firmware's opt-out + +VT-d Rev. 4.1 §8.1, DMAR Flags bit 1 `X2APIC_OPT_OUT`: firmware asks system +software "to opt out of enabling Extended xAPIC (X2APIC) mode", and software +checks it "as part of detecting X2APIC mode support". This kernel enables +x2APIC unconditionally in `arch::apic::init` (`enable_x2apic`), reached from +`arch::boot::interrupts`, before the DMAR is first read in `iommu::init` +(`kernel/src/main.rs`). The flag is logged (`x2apic_opt_out=` on the `iommu: +DMAR` line) and decides nothing. + +The owner's ruling on extended interrupt mode +(`issues/kernel/the-iommu-refuses-nothing-yet.md`) is about the width of a +remapping entry's destination, not whether x2APIC is on, so it does not cover +this. The T14 clears the flag (`flags=0x05`), so no machine in reach sets it. + +Owner: the IOMMU track. Exit: the flag is read before the local APIC is put in +x2APIC mode, and a machine that sets it either runs xAPIC or is refused by +name; an actuator standing in for the flag reaches both arms on the T14. diff --git a/kernel/src/arch/aarch64/iommu_unit.rs b/kernel/src/arch/aarch64/iommu_unit.rs index 3593845081f..13c0b28e305 100644 --- a/kernel/src/arch/aarch64/iommu_unit.rs +++ b/kernel/src/arch/aarch64/iommu_unit.rs @@ -4,7 +4,12 @@ use crate::drivers::pci::PciDevice; use crate::log; -pub fn init(_rsdp_addr: u64, _devices: &[PciDevice]) { +pub fn init( + _rsdp_addr: u64, + _devices: &[PciDevice], + _windows: &[toyos_abi::boot::RootBridgeWindow], + _claims: usize, +) { log!("IOMMU: the SMMUv3 is the port's stage 6; no device is translated this boot"); } @@ -12,7 +17,7 @@ pub mod domain { use crate::iommu::{DomainId, IommuError, Iova, StreamId}; /// No unit is driven, so there is no domain to give. - pub fn create() -> Result { + pub fn create(_room: u64) -> Result<(DomainId, Iova), IommuError> { Err(IommuError::NoUnit) } @@ -24,10 +29,6 @@ pub mod domain { unreachable!("no domain exists: `create` refuses every one") } - pub fn reserve(_id: DomainId, _bytes: u64) -> Result { - unreachable!("no domain exists: `create` refuses every one") - } - pub fn place(_id: DomainId, _at: Iova, _phys: u64, _bytes: u64) -> Result { unreachable!("no domain exists: `create` refuses every one") } @@ -63,6 +64,14 @@ pub mod interrupt { unreachable!("no interrupt remapping without an IOMMU unit, and `is_armed` said so") } + pub fn claim(_slot: usize, _source: StreamId, _vector: u8) -> Msi { + unreachable!("no interrupt remapping without an IOMMU unit, and `is_armed` said so") + } + + pub fn release(_slot: usize, _source: StreamId) { + unreachable!("no interrupt remapping without an IOMMU unit, and `is_armed` said so") + } + pub fn pin(_apic_id: u8, _vector: u8, _dest: u32, _level: bool) -> Result { unreachable!("no interrupt remapping without an IOMMU unit, and `is_armed` said so") } diff --git a/kernel/src/arch/x86_64/vtd/domain.rs b/kernel/src/arch/x86_64/vtd/domain.rs index e40b7d1734d..b0027c93edd 100644 --- a/kernel/src/arch/x86_64/vtd/domain.rs +++ b/kernel/src/arch/x86_64/vtd/domain.rs @@ -41,10 +41,18 @@ struct Domains { /// By id minus [`FIRST`]; never shrinks, since a released id would name a /// domain some unit may still have cached. live: Vec, + /// What no domain's addresses may reach: the root bridges' windows and + /// the regions firmware reserved, as `(start, end)`. + reserved: Vec<(u64, u64)>, } static DOMAINS: Lock = - Lock::new(Domains { agreement: Agreement::None, live: Vec::new() }); + Lock::new(Domains { agreement: Agreement::None, live: Vec::new(), reserved: Vec::new() }); + +/// Before any domain is made: every one is built clear of these. +pub fn avoid(reserved: Vec<(u64, u64)>) { + DOMAINS.lock().reserved = reserved; +} pub fn unit_agrees(width: AddressWidth, ceiling: u32, mgaw: u8) { let mut domains = DOMAINS.lock(); @@ -57,7 +65,9 @@ pub fn unit_agrees(width: AddressWidth, ceiling: u32, mgaw: u8) { }; } -pub fn create() -> Result { +/// A new domain, with `room` bytes of it handed out first: refused, with no id +/// spent, where the domain would have less than that. +pub fn create(room: u64) -> Result<(DomainId, Iova), IommuError> { let mut domains = DOMAINS.lock(); let (width, ceiling, mgaw) = match domains.agreement { Agreement::None => return Err(IommuError::NoUnit), @@ -68,7 +78,8 @@ pub fn create() -> Result { if u32::from(id) >= ceiling { return Err(IommuError::DomainsExhausted(ceiling)); } - let domain = Domain::new(&mut TABLES.lock(), id, width, mgaw)?; + let mut domain = Domain::new(&mut TABLES.lock(), id, width, mgaw, &domains.reserved, room)?; + let first = domain.reserve(room).expect("`Domain::new` refuses a domain without the room"); log!( "iommu: domain{id} root={:#x} aw={} mgaw={} addresses from {:#x} to {:#x}", domain.root().phys(), @@ -78,7 +89,7 @@ pub fn create() -> Result { domain.ceiling() ); domains.live.push(domain); - Ok(DomainId::new(id)) + Ok((DomainId::new(id), first)) } pub fn map(id: DomainId, phys: u64, bytes: u64) -> Result { @@ -87,11 +98,7 @@ pub fn map(id: DomainId, phys: u64, bytes: u64) -> Result { } let mut domains = DOMAINS.lock(); let domain = domains.at(id); - let at = domain - .reserve(bytes) - // Named by what actually ran out: the unit's translatable width, which - // on a machine whose `MGAW` is under its `SAGAW` is not the table depth. - .ok_or(IommuError::AddressesExhausted(domain.translatable()))?; + let at = domain.reserve(bytes).ok_or(IommuError::AddressesExhausted(domain.ceiling()))?; let (did, domain) = (domain.id(), *domain); let mut units = UNITS.lock(); table::map(&mut TABLES.lock(), &domain, at, phys, bytes); @@ -107,14 +114,6 @@ pub fn map(id: DomainId, phys: u64, bytes: u64) -> Result { Ok(at) } -/// Hand out room for `bytes` and map nothing there: a caller that places its -/// own mappings in it later, with [`place`]. -pub fn reserve(id: DomainId, bytes: u64) -> Result { - let mut domains = DOMAINS.lock(); - let domain = domains.at(id); - domain.reserve(bytes).ok_or(IommuError::AddressesExhausted(domain.translatable())) -} - /// Put `bytes` at `phys` at `at`, room this domain handed out before and whose /// mapping was taken back: a device still aimed there reaches these pages. /// Room it never handed out is a kernel bug, since [`map`] may yet hand it out. diff --git a/kernel/src/arch/x86_64/vtd/fault.rs b/kernel/src/arch/x86_64/vtd/fault.rs index 3de98c0f382..c2533716fcf 100644 --- a/kernel/src/arch/x86_64/vtd/fault.rs +++ b/kernel/src/arch/x86_64/vtd/fault.rs @@ -1,8 +1,8 @@ //! Vt-d fault interrupt handling: MSI-delivered, never polled. //! -//! The handler is bounded, allocates nothing; unit and -//! function state lives in fixed arrays of atomics, written once before the -//! mask comes off. Whatever the stream, the same things happen first: Bus +//! The handler is bounded, allocates nothing and takes no lock; unit state +//! lives in a fixed array of atomics and function state in a slice of exactly +//! the enumerated functions, each published once before the mask comes off. Whatever the stream, the same things happen first: Bus //! Master Enable cleared on the function that faulted, the first record latched //! whole, and a count kept per unit and per function. Clearing `BME` is also //! the ceiling on a storm, since a function that cannot master the bus cannot @@ -16,7 +16,9 @@ //! thing moving a driver out of the kernel was for. use crate::log; -use core::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use alloc::boxed::Box; +use alloc::vec::Vec; +use core::sync::atomic::{AtomicPtr, AtomicU32, AtomicU64, Ordering}; use crate::drivers::pci::{self, PciDevice}; use crate::iommu::StreamId; @@ -62,18 +64,14 @@ impl FaultUnit { static UNITS: [FaultUnit; MAX_UNITS] = [const { FaultUnit::EMPTY }; MAX_UNITS]; -/// Functions the handler can act on; a machine with more is told, and the ones -/// past it keep bus mastering through a fault. -const MAX_FUNCTIONS: usize = 64; - /// 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 { - // `pci::NO_FUNCTION` while the slot is free; a requester id once taken. - who: AtomicU32, - config: AtomicU64, + who: u16, + /// Physical base of its config window, through which a fault clears `BME`. + config: u64, domain: AtomicU32, // Faults the unit has reported against it; non-zero is the per-domain flag. faults: AtomicU32, @@ -82,18 +80,19 @@ struct Function { user_slot: AtomicU32, } -impl Function { - #[allow(clippy::declare_interior_mutable_const)] - const EMPTY: Self = Self { - who: AtomicU32::new(pci::NO_FUNCTION), - config: AtomicU64::new(0), - domain: AtomicU32::new(0), - faults: AtomicU32::new(0), - user_slot: AtomicU32::new(NO_SLOT), - }; -} +/// Exactly the functions this machine enumerated, leaked once by [`describe`]: +/// null until then, and never written again. +static FUNCTIONS: AtomicPtr<&'static [Function]> = AtomicPtr::new(core::ptr::null_mut()); -static FUNCTIONS: [Function; MAX_FUNCTIONS] = [const { Function::EMPTY }; MAX_FUNCTIONS]; +fn functions() -> &'static [Function] { + let published = FUNCTIONS.load(Ordering::Acquire); + if published.is_null() { + return &[]; + } + // SAFETY: non-null only as `describe` stored it: a leaked box, never freed + // or written again, published with Release once it was whole. + unsafe { *published } +} /// The first fault this machine took, whole: what a later one says is decided /// by what the first one already broke. @@ -114,23 +113,24 @@ static FIRST: FirstFault = FirstFault { /// Every function this machine enumerated, before any unit is armed: the /// handler reaches a faulting function's config space through this, with no lock. pub fn describe(devices: &[PciDevice]) { - for (slot, device) in FUNCTIONS.iter().zip(devices) { - let stream = StreamId::pci(device.bus, device.dev, device.func); - slot.config.store( - DirectMap::phys_of(device.config_window().addr() as *const u8), - Ordering::Relaxed, - ); - // Last, with Release: `who` is what the handler tests, so `config` must - // already be visible to whoever sees it. - slot.who.store(u32::from(stream.requester()), Ordering::Release); - } - if devices.len() > MAX_FUNCTIONS { - log!( - "iommu: {} functions enumerated, past the {MAX_FUNCTIONS} the fault handler can \ - stop — the rest keep bus mastering through a fault", - devices.len() - ); - } + let functions: Vec = devices + .iter() + .map(|device| Function { + who: StreamId::pci(device.bus, device.dev, device.func).requester(), + 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()); + let first = FUNCTIONS.compare_exchange( + core::ptr::null_mut(), + Box::leak(Box::new(functions)), + Ordering::Release, + Ordering::Relaxed, + ); + assert!(first.is_ok(), "iommu: the fault handler's functions were described twice"); } /// Record which domain a function moved to, for the flag the handler sets. @@ -155,7 +155,7 @@ pub fn user_owned(stream: StreamId, slot: Option) { } fn find(requester: u32) -> Option<&'static Function> { - FUNCTIONS.iter().find(|f| f.who.load(Ordering::Acquire) == requester) + functions().iter().find(|f| u32::from(f.who) == requester) } /// A unit's fault-record location and count: `CAP.FRO` and `CAP.NFR`. @@ -352,8 +352,7 @@ fn stop(stream: StreamId) -> bool { let Some(slot) = find(u32::from(stream.requester())) else { return false; }; - let config = slot.config.load(Ordering::Relaxed); - pci::stop_bus_mastering(window_of(config, CONFIG_WINDOW)); + pci::stop_bus_mastering(window_of(slot.config, CONFIG_WINDOW)); true } diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 62cd1ae9c53..861acd96b9c 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -25,12 +25,17 @@ //! 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. +//! //! This module's lock is taken before `UNITS` and before `TABLES`, never after //! either. use crate::log; use alloc::vec::Vec; +use crate::drivers::pci::MSG_DEST; use crate::iommu::{Refused, StreamId}; use crate::sync::Lock; @@ -76,6 +81,9 @@ pub struct Pin { struct Remap { /// `None` until a unit is armed, which is what "no source may use the remappable format" means. table: Option, + /// Entries `0..claims` are the claim slots'; the kernel's own sources take + /// them from here up. + claims: u16, used: u16, /// `ECAP.EIM` on every unit. Clear bounds a destination to the eight bits `DST` then holds. extended: bool, @@ -84,7 +92,11 @@ struct Remap { } static REMAP: Lock = - Lock::new(Remap { table: None, used: 0, extended: false, apics: Vec::new() }); + Lock::new(Remap { table: None, claims: 0, used: 0, extended: false, apics: Vec::new() }); + +/// A claimed function's message reaches [`MSG_DEST`], which an entry holds with +/// or without `EIME`, so writing a claim slot's entry is never refused. +const _: () = assert!(MSG_DEST < NARROW_DESTINATIONS); pub fn describe_apic(apic_id: u8, source: StreamId) { REMAP.lock().apics.push((apic_id, source)); @@ -95,13 +107,20 @@ pub fn apics_are_named(apics: &[u8]) -> bool { apics.iter().all(|id| remap.apics.iter().any(|(named, _)| named == id)) } -/// Allocate the shared table on first ask and return the value `IRTA_REG` takes for it. -pub fn arm(extended: bool) -> u64 { +/// Allocate the shared table on first ask, its first `claims` entries kept for +/// the claim slots, and return the value `IRTA_REG` takes for it. +pub fn arm(extended: bool, claims: usize) -> u64 { + let claims = u16::try_from(claims).ok().filter(|c| *c < ENTRIES).unwrap_or_else(|| { + panic!("iommu: {claims} claim slots leave none of the table's {ENTRIES} entries over") + }); let mut remap = REMAP.lock(); remap.extended = extended; let table = match remap.table { Some(table) => table, - None => *remap.table.insert(super::TABLES.lock().alloc()), + None => { + (remap.claims, remap.used) = (claims, claims); + *remap.table.insert(super::TABLES.lock().alloc()) + } }; table.phys() | if extended { EXTENDED_INTERRUPT_MODE } else { 0 } | SIZE_FIELD } @@ -114,11 +133,42 @@ pub fn is_armed() -> bool { REMAP.lock().table.is_some() } +/// The entry that delivers `vector` to `dest` for `source` alone: delivery mode +/// 000b, destination mode physical and no redirection hint, which is what the +/// compatibility message it replaces said. +fn entry(extended: bool, source: StreamId, vector: u8, dest: u32, level: bool) -> (u64, u64) { + let destination = if extended { dest as u64 } else { (dest as u64) << NARROW_DESTINATION_SHIFT }; + ( + PRESENT + | if level { TRIGGER_LEVEL } else { 0 } + | ((vector as u64) << VECTOR_SHIFT) + | (destination << DESTINATION_SHIFT), + VERIFY_SOURCE_ID | source.requester() as u64, + ) +} + +/// What an entry holds, read back out of the table rather than restated from +/// 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)) { + log!( + // `apic=` is the id this was asked for, `dst=` the field it encoded + // into; printing only the second leaves the encoding compared against + // itself. + "iommu: irte{index} source={source} p={} sid={:#06x} svt={} sq={} vector={:#04x} \ + apic={dest:#x} dst={:#x} trigger={}", + lo & PRESENT, + hi & 0xFFFF, + (hi >> 18) & 0x3, + (hi >> 16) & 0x3, + (lo >> VECTOR_SHIFT) & 0xFF, + lo >> DESTINATION_SHIFT, + if lo & TRIGGER_LEVEL != 0 { "level" } else { "edge" } + ); +} + /// Fills the next free entry for `source` and returns its index. -/// -/// What it reports is the entry read back out of the table, never the words it -/// meant to write: a line restating the intent agrees with itself however the -/// entry was composed, and these fields keep one device off another's entry. fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result { let written = { let mut remap = REMAP.lock(); @@ -133,44 +183,19 @@ fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result { - log!( - // `apic=` is the id this was asked for, `dst=` the field it - // encoded into; printing only the second leaves the encoding - // compared against itself. - "iommu: irte{index} source={source} p={} sid={:#06x} svt={} sq={} \ - vector={:#04x} apic={:#x} dst={:#x} trigger={}", - lo & PRESENT, - hi & 0xFFFF, - (hi >> 18) & 0x3, - (hi >> 16) & 0x3, - (lo >> VECTOR_SHIFT) & 0xFF, - dest, - lo >> DESTINATION_SHIFT, - if lo & TRIGGER_LEVEL != 0 { "level" } else { "edge" } - ); + Ok((index, read)) => { + report(index, source, dest, read); Ok(index) } Err(why) => { @@ -180,16 +205,60 @@ fn allocate(source: StreamId, vector: u8, dest: u32, level: bool) -> Result Result { - let index = allocate(source, vector, dest, false)? as u32; - Ok(Msi { +/// The message that indexes entry `index` by its handle alone (Section 5.1.3). +fn message(index: u16) -> Msi { + let index = index as u32; + Msi { address: crate::arch::MSI_DOORBELL | ((index & 0x7FFF) << 5) | MESSAGE_REMAPPABLE | MESSAGE_SUBHANDLE_VALID | ((index >> 15) << 2), data: 0, - }) + } +} + +pub fn msi(source: StreamId, vector: u8, dest: u32) -> Result { + Ok(message(allocate(source, vector, dest, false)?)) +} + +/// Claim slot `slot`'s entry, written for `source` at `vector` and published +/// to every unit before this returns, and the message that reaches it. +pub fn claim(slot: usize, source: StreamId, vector: u8) -> Msi { + let (index, read) = { + let remap = REMAP.lock(); + let (Some(table), Ok(index)) = (remap.table, u16::try_from(slot)) else { + panic!("iommu: claim slot {slot} written with no table armed"); + }; + assert!(index < remap.claims, "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 (lo, hi) = entry(remap.extended, source, vector, MSG_DEST, false); + table.write_pair(slot, lo, hi); + super::invalidate_interrupt_entries(); + (index, table.read_pair(slot)) + }; + report(index, source, MSG_DEST, read); + message(index) +} + +/// 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) { + let lo = { + let remap = REMAP.lock(); + let Some(table) = remap.table else { + panic!("iommu: claim slot {slot} released with no table armed"); + }; + table.clear_pair(slot); + super::invalidate_interrupt_entries(); + table.read_pair(slot).0 + }; + log!("iommu: irte{slot} source={source} p={} released", lo & PRESENT); } pub fn pin(apic_id: u8, vector: u8, dest: u32, level: bool) -> Result { diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 29469d70065..923755cd563 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -20,6 +20,7 @@ use alloc::vec::Vec; use crate::drivers::acpi::TableError; use crate::drivers::pci::PciDevice; +use toyos_abi::boot::RootBridgeWindow; use crate::iommu::{AddressWidth, StreamId}; use crate::mm::policy::MmioPolicy; use crate::mm::Mmio; @@ -123,7 +124,9 @@ pub(super) fn invalidate_interrupt_entries() { } } -pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { +/// `windows` and every region firmware reserved are what no domain's addresses +/// reach; the interrupt table's first `claims` entries are the claim slots'. +pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: &[RootBridgeWindow], claims: usize) { let dmar = match Dmar::open(rsdp_addr) { Ok(dmar) => dmar, // ACPI cannot distinguish "no VT-d silicon" from "VT-d disabled in @@ -164,6 +167,8 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { // Described and planned before any unit is armed: whether sources may move // to the remappable format is one decision, taken before the first `IRE`. let mut ready: Vec<(Unit, Plan)> = Vec::new(); + // What no domain's addresses may reach, RMRRs added below as the walk meets them. + let mut reserved: Vec<(u64, u64)> = windows.iter().map(|w| (w.base, w.end())).collect(); for structure in dmar.structures() { match structure { Ok(Structure::Drhd(drhd)) => { @@ -210,6 +215,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { identity domain 0x0..{identity_top:#x}" ); } + reserved.push((base, limit.saturating_add(1))); describe_scopes("rmrr", regions, rmrr.scopes()); regions += 1; held_regions += usize::from(held); @@ -238,12 +244,13 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { them inside the identity domain" ); - let remap = remappable(&ready, units); + domain::avoid(reserved); + let remap = remappable(&ready, units, dmar.flags); // One identity-domain table set per address width: units may disagree on // `CAP.SAGAW`, and a shared set would be programmed at the wrong depth for one. let mut domains: [Option
; 2] = [None, None]; for (unit, plan) in ready { - enable(unit, plan, devices, &mut domains, remap); + enable(unit, plan, devices, &mut domains, remap, claims); } } @@ -254,7 +261,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice]) { /// it would read that address as a compatibility message and deliver the /// interrupt to whatever the handle bits spell. Every condition below therefore /// refuses for the machine, not for the unit that failed it. -fn remappable(ready: &[(Unit, Plan)], described: usize) -> Option { +fn remappable(ready: &[(Unit, Plan)], described: usize, flags: u8) -> Option { #[cfg(feature = "boot-actuators")] if crate::actuator::iommu_no_remap() { log!( @@ -266,6 +273,13 @@ fn remappable(ready: &[(Unit, Plan)], described: usize) -> Option { if ready.is_empty() { return None; } + if flags & dmar::FLAG_INTR_REMAP == 0 { + log!( + "iommu: DMAR flags={flags:#04x} leave INTR_REMAP clear, so firmware says this \ + platform does not remap — every source stays in compatibility format" + ); + return None; + } if ready.len() != described { log!( "iommu: {} of {described} units are programmed, so no source may use the remappable \ @@ -512,6 +526,7 @@ fn enable( devices: &[PciDevice], domains: &mut [Option
; 2], remap: Option, + claims: usize, ) { let index = unit.index; let Plan { width, records } = plan; @@ -539,7 +554,7 @@ fn enable( let mut queue = Queue::new(&mut TABLES.lock(), unit.regs); // Outside the `TABLES` lock: `interrupt::arm` takes its own lock and // then that one, and the order this subsystem holds is the reverse. - let irta = remap.map(interrupt::arm); + let irta = remap.map(|extended| interrupt::arm(extended, claims)); // Before `TE`: the first blocked transaction must be reportable, not merely counted. fault::arm(index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index 84a2dbd0e33..a1b895387f5 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -104,6 +104,14 @@ impl Table { self.write(index * 2, lo); } + /// Zeroes a 16-byte entry low half first: the low half holds `P`, and an + /// entry whose high half went first would for a moment be present over a + /// field it no longer means. + pub fn clear_pair(self, index: usize) { + self.write(index * 2, 0); + self.write(index * 2 + 1, 0); + } + /// The 16-byte entry at `index`, back out of memory: `write` flushed the line, so this refetches. pub fn read_pair(self, index: usize) -> (u64, u64) { (self.read(index * 2), self.read(index * 2 + 1)) @@ -214,12 +222,33 @@ pub struct Domain { root: Table, id: u16, width: AddressWidth, - /// Bits of device address this domain may hand out, which is not - /// [`AddressWidth::bits`] — see [`Domain::translatable`]. - translatable: u8, + /// Where its addresses start: [`Domain::first_address`] of what its unit + /// translates, which is not [`AddressWidth::bits`] — see + /// [`Domain::translatable_bits`]. + floor: u64, + /// Where they end: [`ceiling`]. + ceiling: u64, next: u64, } +/// Where a domain's addresses end: under what its unit translates, and under +/// the first of `reserved` that reaches above `floor` — a root bridge's window, +/// which a bridge may route peer-to-peer before the unit sees the request +/// (PCIe Base §2.4), or a region firmware reserved (VT-d §3.16). At or below +/// `floor` where one of them covers it. +const fn ceiling(translatable: u8, floor: u64, reserved: &[(u64, u64)]) -> u64 { + let mut ceiling = 1u64 << translatable; + let mut i = 0; + while i < reserved.len() { + let (start, end) = reserved[i]; + if end > floor && start < ceiling { + ceiling = start; + } + i += 1; + } + ceiling +} + impl Domain { /// The bits of device address a unit will translate: the lesser of the page /// tables' depth and what the hardware accepts at all. @@ -235,29 +264,34 @@ impl Domain { if mgaw < width.bits() { mgaw } else { width.bits() } } - /// A quarter of the way up what this domain can translate — above any - /// physical address these machines have, so a descriptor still carrying one - /// names nothing this domain maps and faults rather than landing. + /// A quarter of the way up what this domain can translate. const fn first_address(translatable: u8) -> u64 { 1 << (translatable - 2) } + /// A domain with `room` bytes of addresses between its floor and its + /// [`ceiling`], or the reason it has not. pub fn new( tables: &mut Tables, id: u16, width: AddressWidth, mgaw: u8, + reserved: &[(u64, u64)], + room: u64, ) -> Result { let translatable = Self::translatable_bits(width, mgaw); let floor = Self::first_address(translatable); - // The property `first_address` is chosen for, asserted rather than - // assumed: a machine with enough memory to reach the window would have - // stale descriptors landing on real pages instead of faulting. + // Above memory, so a descriptor still carrying one of these names + // nothing this domain maps and faults rather than landing on a page. let top = crate::mm::pmm::top(); if floor <= top { return Err(IommuError::WindowBelowMemory { translatable, floor, top }); } - Ok(Self { root: tables.alloc(), id, width, translatable, next: floor }) + let ceiling = ceiling(translatable, floor, reserved); + if ceiling < floor || ceiling - floor < room.next_multiple_of(PAGE_2M) { + return Err(IommuError::NoRoom { floor, ceiling, room }); + } + Ok(Self { root: tables.alloc(), id, width, floor, ceiling, next: floor }) } pub fn root(&self) -> Table { @@ -269,20 +303,11 @@ impl Domain { } pub const fn floor(&self) -> u64 { - Self::first_address(self.translatable) - } - - /// Where this domain's addresses end: what this unit will translate, not - /// what the tables can express — past `MGAW` the hardware faults before the - /// walk it has entries for. - pub fn ceiling(&self) -> u64 { - 1u64 << self.translatable + self.floor } - /// The bits of device address this domain hands out, for a refusal that - /// names what ran out rather than the depth of the tables. - pub fn translatable(&self) -> u8 { - self.translatable + pub const fn ceiling(&self) -> u64 { + self.ceiling } /// Reserve room for `bytes`, rounded up to whole leaves. An address is @@ -333,7 +358,8 @@ const _: () = { root: ROOT, id: KERNEL_DOMAIN + 1, width: AddressWidth::Bits48, - translatable: 48, + floor: FLOOR, + ceiling: 1 << 48, next: FLOOR + PAGE_2M, }; const TWO_LEAVES: Domain = Domain { next: FLOOR + 2 * PAGE_2M, ..ONE_LEAF }; @@ -353,6 +379,26 @@ const _: () = { assert!(!TWO_LEAVES.handed_out(Iova::translated(FLOOR + 1), PAGE_2M)); }; +/// [`ceiling`] over the windows the T14's firmware declares, unsorted as it +/// declares them: a 39-bit unit's domain ends where the first window above its +/// floor begins, and a window reaching over the floor leaves it nothing. +const _: () = { + const FLOOR: u64 = Domain::first_address(39); + const T14: [(u64, u64); 7] = [ + (0xA200_0000, 0xBD00_0000), + (0x40_0000_0000, 0x60_3DC0_0000), + (0xA080_0000, 0xA200_0000), + (0xBD00_0000, 0xC000_0000), + (0xFF00_0000, 0xFFB8_0000), + (0xFFD3_A070, 0x1_0000_0000), + (0x60_3DC0_0000, 0x80_0000_0000), + ]; + assert!(FLOOR == 0x20_0000_0000); + assert!(ceiling(39, FLOOR, &T14) == 0x40_0000_0000); + assert!(ceiling(39, FLOOR, &[]) == 1 << 39); + assert!(ceiling(39, FLOOR, &[(0x10_0000_0000, FLOOR + 1)]) < FLOOR); +}; + pub fn map(tables: &mut Tables, domain: &Domain, at: Iova, phys: u64, bytes: u64) { let levels = levels(domain.width); let mut offset = 0u64; @@ -513,7 +559,8 @@ const _: () = { root: Table { phys: 0x5000 }, id: u16::MAX, width: AddressWidth::Bits39, - translatable: 39, + floor: 0, + ceiling: 0, next: 0, }; let identity = context_entry(Table { phys: 0x3000 }, KERNEL_DOMAIN, AddressWidth::Bits48); diff --git a/kernel/src/drivers/hda.rs b/kernel/src/drivers/hda.rs index ff65cef757b..a7977f22067 100644 --- a/kernel/src/drivers/hda.rs +++ b/kernel/src/drivers/hda.rs @@ -578,7 +578,7 @@ fn reset_stream(stream: Mmio) -> bool { /// panic, over a peripheral. fn arm_interrupt(pci: &PciDevice) -> bool { let vector = crate::arch::trap::HDA_VECTOR; - if pci.enable_msix(vector).is_ok() || pci.enable_msi(vector) { + if pci.enable_msix(vector).is_some() || pci.enable_msi(vector) { return true; } log!( diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index 1e033aedbe8..e021e1a449a 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -41,15 +41,20 @@ pub enum NoCapability { Truncated, } -/// Why [`PciDevice::enable_msix`] armed nothing. -pub enum Unarmed { +/// Why a function has no MSI-X entry this kernel can write. +pub enum NoEntry { /// No MSI-X capability came off the walk, which is a table this function /// does not have only where the walk reached the list's terminator. NoTable(NoCapability), /// It publishes one whose table this kernel could not reach. Unusable, - /// The unit refuses this function's message, and MSI would carry the same one. - Blocked, +} + +/// Where a function's [`MSIX_ENTRY`] is, found before any message is made for it. +pub(in crate::drivers) struct MsixEntry<'a> { + cap: Capability<'a>, + control: u16, + address: u64, } pub struct Capability<'a> { @@ -268,20 +273,32 @@ impl PciDevice { stop_bus_mastering(self.mmio); } - /// Point this function's [`MSIX_ENTRY`] at `vector` and enable it. + /// Point this function's [`MSIX_ENTRY`] at `vector` and enable it: a + /// kernel driver's arming, whose message is compatibility format on a + /// machine that remaps nothing. A claimed function is armed by + /// [`arm_claimed_msix`] instead. /// /// Answers the entry's own window, which stays this kernel's: masking is a /// write to it, and a claimant that could reach it could aim the device's /// message at any address the LAPIC decodes. - pub fn enable_msix(&self, vector: u8) -> Result { - let cap = self.capability(msix::CAP_ID).map_err(Unarmed::NoTable)?; + /// + /// `None` where it has no MSI-X entry this kernel can write, or the unit + /// refuses its message. + pub(in crate::drivers) fn enable_msix(&self, vector: u8) -> Option { + let entry = self.msix_entry().ok()?; + let (address, data) = self.message(vector)?; + Some(self.write_msix(entry, address, data)) + } + + pub(in crate::drivers) fn msix_entry(&self) -> Result, NoEntry> { + let cap = self.capability(msix::CAP_ID).map_err(NoEntry::NoTable)?; let control = cap.read_u16(msix::MESSAGE_CONTROL); let table = match msix::Msix::decode(control, cap.read_u32(msix::TABLE)) { Ok(table) => table, Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, {}", self.bus, self.dev, self.func, why); - return Err(Unarmed::Unusable); + return Err(NoEntry::Unusable); } }; // Decoded, not assumed memory: a device may name a BAR that is an I/O BAR. @@ -290,35 +307,35 @@ impl PciDevice { Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, its table names BAR {} and {}", self.bus, self.dev, self.func, table.bir(), why); - return Err(Unarmed::Unusable); + return Err(NoEntry::Unusable); } }; - let address = match table.table_address(base) { - Ok(address) => address, + match table.table_address(base) { + Ok(address) => Ok(MsixEntry { cap, control, address }), Err(why) => { log!("PCI {:02x}:{:02x}.{}: MSI-X not armed, {}", self.bus, self.dev, self.func, why); - return Err(Unarmed::Unusable); + Err(NoEntry::Unusable) } - }; - - let (message, data) = self.message(vector).ok_or(Unarmed::Blocked)?; + } + } - let entry = address + MSIX_ENTRY as u64 * msix::ENTRY_BYTES; - let table = crate::mm::paging::map_mmio(entry, 0x1000, MmioPolicy::Uncacheable); + fn write_msix(&self, entry: MsixEntry<'_>, address: u32, data: u32) -> Mmio { + let at = entry.address + MSIX_ENTRY as u64 * msix::ENTRY_BYTES; + let table = crate::mm::paging::map_mmio(at, 0x1000, MmioPolicy::Uncacheable); - table.write_u32(msix::ENTRY_ADDRESS_LO, message); + table.write_u32(msix::ENTRY_ADDRESS_LO, address); table.write_u32(msix::ENTRY_ADDRESS_HI, 0); table.write_u32(msix::ENTRY_DATA, data); table.write_u32(msix::ENTRY_VECTOR_CONTROL, msix::ENTRY_UNMASKED); - cap.write_u16(msix::MESSAGE_CONTROL, msix::Msix::enabled(control)); + entry.cap.write_u16(msix::MESSAGE_CONTROL, msix::Msix::enabled(entry.control)); self.report_message( "msix", table.read_u32(msix::ENTRY_ADDRESS_LO), table.read_u32(msix::ENTRY_DATA), ); - Ok(table) + table } /// Put MSI-X back off, for a hand-over that armed a vector and was then @@ -360,25 +377,29 @@ impl PciDevice { } } - /// Point this function's single MSI message at `vector` and enable it. + /// Point this function's single MSI message at `vector` and enable it: a + /// kernel driver's arming, as [`Self::enable_msix`] is. /// /// **A driver in this kernel may arm this however the MSI-X walk failed**, a /// list that ended early included: it hands no BAR of its function to a /// holder, so an MSI-X table past that link is one nobody but this kernel /// could reach. A hand-over is the caller that has to tell the two apart, /// and `crate::pcidev`'s header says why. - pub fn enable_msi(&self, vector: u8) -> bool { + pub(in crate::drivers) fn enable_msi(&self, vector: u8) -> bool { let Ok(cap) = self.capability(msi::CAP_ID) else { return false; }; - - let Some((message, data)) = self.message(vector) else { + let Some((address, data)) = self.message(vector) else { return false; }; + self.write_msi(cap, address, data); + true + } + fn write_msi(&self, cap: Capability<'_>, address: u32, data: u32) { let control = cap.read_u16(msi::MESSAGE_CONTROL); let msi = msi::Msi::decode(control); - cap.write_u32(msi.address_lo(), message); + cap.write_u32(msi.address_lo(), address); if let Some(address_hi) = msi.address_hi() { cap.write_u32(address_hi, 0); } @@ -392,7 +413,6 @@ impl PciDevice { cap.read_u32(msi.address_lo()), cap.read_u16(msi.data()) as u32, ); - true } /// Put MSI back off. @@ -470,6 +490,26 @@ impl PciDevice { } } +/// Point a claimed function's [`MSIX_ENTRY`] at its slot's own remapping +/// entry 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 { + let pci = message.function(); + let entry = pci.msix_entry()?; + Ok(pci.write_msix(entry, message.address(), message.data())) +} + +/// [`arm_claimed_msix`] for a function with MSI and no MSI-X; `false` where it +/// has no MSI either. +pub fn arm_claimed_msi(message: &crate::iommu::Remapped) -> bool { + let pci = message.function(); + let Ok(cap) = pci.capability(msi::CAP_ID) else { + return false; + }; + pci.write_msi(cap, message.address(), message.data()); + true +} + pub struct CapabilityIter<'a> { device: &'a PciDevice, walk: caps::CapWalk, diff --git a/kernel/src/drivers/virtio.rs b/kernel/src/drivers/virtio.rs index 381d8983253..9f40bfb0055 100644 --- a/kernel/src/drivers/virtio.rs +++ b/kernel/src/drivers/virtio.rs @@ -662,7 +662,7 @@ fn wait_until(at: u64, mut now: impl FnMut() -> u64, mut look: impl FnMut() - /// or forbidden link, a BAR/offset/length past the window, a chain missing a required capability. #[cfg(feature = "boot-actuators")] pub fn cap_selftest() { - use super::pci::{NoCapability, PciDevice, Unarmed, CAPABILITIES_PTR}; + use super::pci::{NoCapability, NoEntry, PciDevice, CAPABILITIES_PTR}; use super::DmaPool; use crate::mm::{DirectMap, Mmio}; use alloc::vec::Vec; @@ -732,12 +732,11 @@ pub fn cap_selftest() { } // No layout here publishes an MSI-X capability the walk reaches, so // this returns at the capability lookup and touches no MMIO. - let armed = match device.enable_msix(0) { - Ok(_) => "armed", - Err(Unarmed::NoTable(NoCapability::Absent)) => "absent", - Err(Unarmed::NoTable(NoCapability::Truncated)) => "truncated", - Err(Unarmed::Unusable) => "unusable", - Err(Unarmed::Blocked) => "blocked", + let armed = match device.msix_entry() { + Ok(_) => "found", + Err(NoEntry::NoTable(NoCapability::Absent)) => "absent", + Err(NoEntry::NoTable(NoCapability::Truncated)) => "truncated", + Err(NoEntry::Unusable) => "unusable", }; if armed == want_split { split_passed += 1; diff --git a/kernel/src/drivers/virtio_sound.rs b/kernel/src/drivers/virtio_sound.rs index 449c82d5660..e39ff6c5b76 100644 --- a/kernel/src/drivers/virtio_sound.rs +++ b/kernel/src/drivers/virtio_sound.rs @@ -429,7 +429,7 @@ fn build_chains( /// every period in flight forever. fn arm_interrupt(pci: &PciDevice, device: &VirtioDevice) -> bool { let vector = crate::arch::trap::VIRTIO_SOUND_VECTOR; - if pci.enable_msix(vector).is_err() { + if pci.enable_msix(vector).is_none() { log!( "virtio-sound: NOT INITIALISED at PCI {:02x}:{:02x}.{} — its MSI-X could not be \ armed and this driver has no other way to be told a period completed", diff --git a/kernel/src/drivers/xhci/wait/boot.rs b/kernel/src/drivers/xhci/wait/boot.rs index 66d252b0a1e..b887a741103 100644 --- a/kernel/src/drivers/xhci/wait/boot.rs +++ b/kernel/src/drivers/xhci/wait/boot.rs @@ -95,7 +95,7 @@ fn await_connect_settle(controllers: &[XhciController]) { // `None` must stay a refusal, never a degradation: there is no polled mode, and // every event-ring read depends on `irq_ring`, which only the ISR sets. fn arm_interrupt(pci_dev: &PciDevice) -> Option<&'static str> { - if pci_dev.enable_msix(XHCI_VECTOR).is_ok() { + if pci_dev.enable_msix(XHCI_VECTOR).is_some() { return Some("MSI-X"); } pci_dev.enable_msi(XHCI_VECTOR).then_some("MSI") diff --git a/kernel/src/iommu/mod.rs b/kernel/src/iommu/mod.rs index cbcb8c22011..9e4527ed705 100644 --- a/kernel/src/iommu/mod.rs +++ b/kernel/src/iommu/mod.rs @@ -2,7 +2,7 @@ //! //! Inventories the machine's IOMMU units, gives every enumerated PCI function an identity-mapped context entry, turns translation on, remaps every interrupt source through a source-id-verified table entry, 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 `vtd/` stay backend-neutral so a second backend drops in without moving the seam. //! -//! 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 ([`DeviceSpace::own`]), because a descriptor it writes a physical address into is an arbitrary read and write over all of memory. +//! 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 ([`DeviceSpace::own`], [`remapping`]), because a descriptor it writes a physical address into is an arbitrary read and write over all of memory, and a message it sends unremapped is any vector at any CPU; its message is its claim slot's own entry ([`Remapped`]). //! //! `trait Iommu` is deliberately not added: with one backend it would have a single implementor. @@ -109,10 +109,14 @@ pub enum IommuError { /// The units disagree on the depth a domain's tables would be built at. WidthsDisagree, DomainsExhausted(u32), - AddressesExhausted(u8), + /// Every address up to this ceiling is handed out. + AddressesExhausted(u64), /// What this machine's units translate does not reach above its memory, so /// a device window has nowhere to sit that a stale descriptor would miss. WindowBelowMemory { translatable: u8, floor: u64, top: u64 }, + /// From the floor to the first root-bridge window or reserved region above + /// it is less than a new domain was asked to hand out. + NoRoom { floor: u64, ceiling: u64, room: u64 }, /// Not a whole number of the 2 MiB leaves this kernel writes. Unaligned(u64), NotMapped(Iova), @@ -128,14 +132,19 @@ impl core::fmt::Display for IommuError { Self::DomainsExhausted(ceiling) => { write!(f, "every one of this machine's {ceiling} domains is taken") } - Self::AddressesExhausted(bits) => { - write!(f, "a domain's {bits} bits of device address are all handed out") + Self::AddressesExhausted(ceiling) => { + write!(f, "a domain's device addresses up to {ceiling:#x} are all handed out") } Self::WindowBelowMemory { translatable, floor, top } => write!( f, "this machine's units translate {translatable} bits, whose device window would \ start at {floor:#x}, at or below the {top:#x} its memory reaches" ), + Self::NoRoom { floor, ceiling, room } => write!( + f, + "a domain's addresses from {floor:#x} end at {ceiling:#x}, where a root bridge's \ + window or a reserved region begins, short of the {room:#x} bytes asked of it" + ), Self::Unaligned(at) => write!(f, "{at:#x} is not a 2 MiB boundary"), Self::NotMapped(at) => write!(f, "{:#x} is not mapped in this domain", at.raw()), } @@ -153,21 +162,22 @@ pub enum DeviceSpace { } impl DeviceSpace { - /// One of a device's own, or the reason there is none. + /// One of a device's own with `room` bytes of it handed out at the address + /// answered, or the reason there is none. /// /// The refusing form, for `pcidev`: a function a *process* drives must /// never be handed a physical address, so a machine with no unit and a /// machine out of domains are both answers its caller refuses the claim /// with rather than degrading past. - pub fn own() -> Result { - unit::domain::create().map(Self::Own) + pub fn own(room: u64) -> Result<(Self, u64), IommuError> { + unit::domain::create(room).map(|(id, at)| (Self::Own(id), at.raw())) } /// One of a device's own, or the machine's own with the reason. For a /// driver **in this kernel**, whose addresses are the kernel's either way. pub fn create() -> Self { - match unit::domain::create() { - Ok(id) => Self::Own(id), + match unit::domain::create(0) { + Ok((id, _)) => Self::Own(id), Err(why) => { log!("iommu: no domain of its own for a device: {why}"); Self::Untranslated @@ -199,16 +209,7 @@ impl DeviceSpace { } } - /// Hand out room for `bytes` and map nothing there: where [`Self::place`] - /// puts mappings later. Only a space of its own has room to hand out. - pub fn reserve(self, bytes: u64) -> Result { - match self { - Self::Untranslated => panic!("iommu: an untranslated space was asked for room"), - Self::Own(id) => unit::domain::reserve(id, bytes).map(Iova::raw), - } - } - - /// Put `bytes` at `phys` at `at`, inside room [`Self::reserve`] handed out, + /// Put `bytes` at `phys` at `at`, inside room [`Self::own`] handed out, /// and write no record of it: for a mapping its holder makes and takes back /// as often as it likes. pub fn place(self, at: u64, phys: u64, bytes: u64) -> Result<(), IommuError> { @@ -248,14 +249,20 @@ impl core::fmt::Display for StreamId { /// /// The device list must be the complete enumeration: enabling translation with an unenumerated device left off it can brick the machine's own boot disk. /// +/// No domain's addresses reach into one of `windows`, the memory firmware says the root bridges decode. +/// /// Calls `unit::init` directly rather than through a dispatch, because x86-64 has one backend and the dispatch is not yet a real seam. -pub fn init(rsdp_addr: u64, devices: &[crate::drivers::pci::PciDevice]) { - unit::init(rsdp_addr, devices); +pub fn init( + rsdp_addr: u64, + devices: &[crate::drivers::pci::PciDevice], + windows: &[toyos_abi::boot::RootBridgeWindow], +) { + unit::init(rsdp_addr, devices, windows, crate::pcidev::MAX_FUNCTIONS); } -/// How a source must address its interrupt. Not a yes/no: a caller that folded -/// the third answer into [`Delivery::Direct`] would write a message the unit -/// blocks and lose the device in silence. +/// How a kernel driver's source must address its interrupt. Not a yes/no: a +/// caller that folded the third answer into [`Delivery::Direct`] would write a +/// message the unit blocks and lose the device in silence. pub enum Delivery { /// No unit remaps interrupts on this machine; write what has always been written. Direct, @@ -301,8 +308,79 @@ pub struct PinRedirect { pub high: u32, } -/// Where `bus:device.function`'s message-signalled interrupt must point. Takes -/// the triple, not a [`StreamId`]: what a requester id is stays in this module. +/// Every unit on this machine remaps interrupts, so a claimed function can be +/// given a message only its own entry delivers. Only [`remapping`] makes one. +#[derive(Clone, Copy)] +pub struct Remapping(()); + +/// No unit remaps this machine's interrupts, so a function a process drives +/// would carry a compatibility-format message, which names any vector at any +/// CPU. +pub struct NotRemapped; + +/// Whether a claimed function's message can be remapped: a fact of the +/// machine, fixed before the first driver arms anything. +pub fn remapping() -> Result { + if unit::interrupt::is_armed() { + Ok(Remapping(())) + } else { + Err(NotRemapped) + } +} + +/// A claimed function's message: claim slot `slot`'s own remapping entry, +/// written 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 puts the +/// entry back to not present, so no refusal after it is written leaves the +/// function an entry it can reach. +pub struct Remapped { + slot: usize, + function: crate::drivers::pci::PciDevice, + address: u32, + data: u32, +} + +impl Remapped { + pub fn function(&self) -> &crate::drivers::pci::PciDevice { + &self.function + } + + pub fn address(&self) -> u32 { + self.address + } + + pub fn data(&self) -> u32 { + self.data + } + + fn stream(&self) -> StreamId { + StreamId::pci(self.function.bus, self.function.dev, self.function.func) + } +} + +impl Drop for Remapped { + fn drop(&mut self) { + unit::interrupt::release(self.slot, self.stream()); + } +} + +/// Write claim slot `slot`'s entry for `function` at `vector`. +pub fn claim_msi( + _: Remapping, + slot: usize, + function: &crate::drivers::pci::PciDevice, + vector: u8, +) -> Remapped { + let stream = StreamId::pci(function.bus, function.dev, function.func); + let msi = unit::interrupt::claim(slot, stream, vector); + Remapped { slot, function: *function, address: msi.address, data: msi.data } +} + +/// Where a kernel driver's `bus:device.function`'s message-signalled interrupt +/// must point. Takes the triple, not a [`StreamId`]: what a requester id is +/// stays in this module. pub fn remap_msi( bus: u8, device: u8, diff --git a/kernel/src/main.rs b/kernel/src/main.rs index 5b38375ff06..093650462c8 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -426,7 +426,7 @@ pub(crate) unsafe extern "C" fn kernel_main(kernel_args: &KernelArgs) -> ! { } // After ACPI is readable and PCI is enumerable, before any driver `init`: each enumerated device needs a context entry before it can DMA. // Refuses nothing — a machine with no usable IOMMU boots exactly as one without it. - iommu::init(kernel_args.rsdp_addr, &pci_devices); + iommu::init(kernel_args.rsdp_addr, &pci_devices, kernel_args.root_bridge_windows()); // Before storage and everything under it: what it covers is the rest of this // boot, and a wedge down there is the reason to have one. arch::watchdog::init(&pci_devices); diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index e87fb652658..c1fad1c82c5 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -4,7 +4,8 @@ //! 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 the interrupt vector into whichever of the function's two -//! message mechanisms it has, and hands out every device address a descriptor +//! message mechanisms it has, through its slot's own remapping entry and no +//! other message, 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 @@ -57,7 +58,9 @@ //! //! **A function with no address space of its own is not handed over**, because //! every grant would answer with a physical address and a descriptor holding -//! one is an arbitrary read and write over all of memory. +//! one is an arbitrary read and write over all of memory. **Nor is one whose +//! interrupts this machine does not remap**, because its message would be +//! compatibility format, which raises any vector on any CPU. //! //! **A function masters the bus only once it has memory it may reach.** What //! comes back from a process still holds the device addresses of a domain that @@ -111,8 +114,8 @@ use toyos_pci::slot::{self, Slot}; use toyos_pci::{af, aperture, bar, express, msix, placement, pm, probe}; use crate::device::{Claim, ClaimError}; -use crate::drivers::pci::{NoCapability, PciDevice, Unarmed}; -use crate::iommu::{DeviceSpace, IommuError}; +use crate::drivers::pci::{arm_claimed_msi, arm_claimed_msix, NoCapability, NoEntry, PciDevice}; +use crate::iommu::{DeviceSpace, IommuError, Remapped}; use crate::mm::policy::{CachePolicy, MmioPolicy}; use crate::mm::{align_2m, DirectMap, Mmio, PAGE_2M}; use crate::object::shm::{Region, SharedMemObject}; @@ -214,12 +217,13 @@ struct Aimed { static RESIDUE: [Lock>; MAX_FUNCTIONS] = [const { Lock::new(Vec::new()) }; MAX_FUNCTIONS]; -/// How a claimed function was made to speak. Both deliver [`VECTORS`]`[slot]` -/// into the same [`Interrupt`] and the claim answers the same handle either way. +/// How a claimed function was made to speak, and the slot's remapping entry it +/// speaks through. Both deliver [`VECTORS`]`[slot]` 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. - Msix(Mmio), - Msi, + Msix(Mmio, Remapped), + Msi(Remapped), } /// What a live slot drives. The ISR never reads this. @@ -555,6 +559,7 @@ fn account_for(firmware: &[RootBridgeWindow], decoded: &[(u16, u64, u64)]) { /// place. #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Refusal { + NotRemapped, NoInterrupt, MsixUnusable, CapsTruncated, @@ -587,6 +592,11 @@ enum Refusal { impl core::fmt::Display for Refusal { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { match self { + Self::NotRemapped => write!( + f, + "its interrupts would not be remapped on this machine, and a message that is not \ + remapped can raise any vector on any CPU" + ), Self::NoInterrupt => write!( f, "neither its MSI-X nor its MSI could be armed, and a claim with no interrupt \ @@ -746,6 +756,10 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, u8, Claim), ClaimError> { /// 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 { + // A fact of the whole machine, fixed at boot: refused before the slot or + // the function is spent or touched. + 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 // function before a register of it is read (PCIe §6.6.2). @@ -769,15 +783,17 @@ 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. - let armed = match pci.enable_msix(VECTORS[slot]) { - Ok(entry) => Armed::Msix(entry), - Err(Unarmed::Unusable) => return Err(Refusal::MsixUnusable), - Err(Unarmed::Blocked) => return Err(Refusal::NoInterrupt), - Err(Unarmed::NoTable(NoCapability::Truncated)) => return Err(Refusal::CapsTruncated), - Err(Unarmed::NoTable(NoCapability::Absent)) => { - pci.enable_msi(VECTORS[slot]).then_some(Armed::Msi).ok_or(Refusal::NoInterrupt)? + // 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, VECTORS[slot]); + let armed = match arm_claimed_msix(&message) { + Ok(entry) => Armed::Msix(entry, message), + Err(NoEntry::Unusable) => return Err(Refusal::MsixUnusable), + Err(NoEntry::NoTable(NoCapability::Truncated)) => return Err(Refusal::CapsTruncated), + Err(NoEntry::NoTable(NoCapability::Absent)) if arm_claimed_msi(&message) => { + Armed::Msi(message) } + Err(NoEntry::NoTable(NoCapability::Absent)) => return Err(Refusal::NoInterrupt), }; // From here a refusal has to undo: a vector is armed, and the arms below @@ -800,9 +816,16 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { }) } Err(why) => { + // The entry goes not present once the function stops speaking. match armed { - Armed::Msix(_) => pci.disable_msix(), - Armed::Msi => pci.disable_msi(), + Armed::Msix(_, message) => { + pci.disable_msix(); + drop(message); + } + Armed::Msi(message) => { + pci.disable_msi(); + drop(message); + } } Err(why) } @@ -875,12 +898,7 @@ fn slot_space(slot: usize) -> Result { match *held { Some(space) => Ok(space), None => { - let space = DeviceSpace::own()?; - // A fresh domain's room starts a quarter of the way up what its - // unit translates, so a refusal here is a kernel bug. - let lend = space - .reserve(MAX_GRANT_TOTAL) - .unwrap_or_else(|why| panic!("pcidev: slot {slot}'s new domain has no room to lend in: {why}")); + let (space, lend) = DeviceSpace::own(MAX_GRANT_TOTAL)?; *held = Some(Space { space, lend }); Ok(Space { space, lend }) } @@ -1395,10 +1413,19 @@ pub fn release(slot: usize) { fn tear_down(slot: usize, mut bound: Bound) { bound.pci.disable_bus_master(); - match &bound.armed { - Armed::Msix(entry) => entry.write_u32(msix::ENTRY_VECTOR_CONTROL, msix::ENTRY_MASKED), - Armed::Msi => bound.pci.disable_msi(), - } + let message = match bound.armed { + Armed::Msix(entry, message) => { + entry.write_u32(msix::ENTRY_VECTOR_CONTROL, msix::ENTRY_MASKED); + message + } + Armed::Msi(message) => { + bound.pci.disable_msi(); + message + } + }; + // Once the function no longer speaks, and before the slot can be reserved + // again: its remapping entry goes not present. + 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 @@ -1454,8 +1481,6 @@ fn tear_down(slot: usize, mut bound: Bound) { bound.id.vendor, bound.id.device, ); - // Last: the rest of the claim goes with this drop. - drop(bound); } /// What every call a claim answers checks first. From 6d16e8b13d3edfc7baa1926f70411819da09a971 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 04:51:19 +0200 Subject: [PATCH 3/8] IOMMU stage 1: a claim slot past the table's claims names itself, and a domain holds at least one leaf `interrupt::claim` refused a slot outside the claims' entries with a message about an unarmed table; it now says the slot has no entry of its own. A domain asked for no room was accepted with none between its floor and its ceiling; it now needs one 2 MiB leaf. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/arch/x86_64/vtd/interrupt.rs | 7 +++++-- kernel/src/arch/x86_64/vtd/table.rs | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 861acd96b9c..42fe3233cc7 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -227,10 +227,13 @@ pub fn msi(source: StreamId, vector: u8, dest: u32) -> Result { pub fn claim(slot: usize, source: StreamId, vector: u8) -> Msi { let (index, read) = { let remap = REMAP.lock(); - let (Some(table), Ok(index)) = (remap.table, u16::try_from(slot)) else { + let Some(table) = remap.table else { panic!("iommu: claim slot {slot} written with no table armed"); }; - assert!(index < remap.claims, "iommu: claim slot {slot} has no entry of its own"); + let index = u16::try_from(slot) + .ok() + .filter(|index| *index < remap.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!( diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index a1b895387f5..a8e923db8d5 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -288,7 +288,8 @@ impl Domain { return Err(IommuError::WindowBelowMemory { translatable, floor, top }); } let ceiling = ceiling(translatable, floor, reserved); - if ceiling < floor || ceiling - floor < room.next_multiple_of(PAGE_2M) { + // At least one leaf, whatever was asked: a domain with none is no domain. + if ceiling.saturating_sub(floor) < room.next_multiple_of(PAGE_2M).max(PAGE_2M) { return Err(IommuError::NoRoom { floor, ceiling, room }); } Ok(Self { root: tables.alloc(), id, width, floor, ceiling, next: floor }) From 43cbe73d439e526863e390fe8537edb2b56df382 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 05:03:16 +0200 Subject: [PATCH 4/8] iommu_virtio_platform: a machine with no unit refuses a claim first for its unremapped interrupts Stage 1 takes `Refusal::NotRemapped` before the slot's domain is asked for, so on QEMU's machine without a unit the NIC's and the NVMe controller's claims are refused by that reason rather than by the missing address space. The arm's other assertions are unchanged: nothing is handed over, nothing armed, no BAR moved, and the boot completes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- tests/common/iommu.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 6f6a4ff3c9d..1afa5559ce6 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -153,7 +153,7 @@ pub fn iommu_virtio_platform(test_config: &Path) -> Result<(), String> { " [iommu] {name}: {} virtio function(s) behind a unit = {behind_unit}, the audio \ function {sound} among them{}", negotiated.len(), - if behind_unit { "" } else { "; the NIC's claim refused for want of a domain" } + if behind_unit { "" } else { "; the NIC's claim refused for want of a unit" } ); } declining_is_not_free(test_config) @@ -179,18 +179,20 @@ const NVME_AT: &str = "00:02.0"; /// /// The ordering ruling this whole stage stands on /// (`issues/kernel/every-driver-is-still-in-the-kernel.md`) is that moving a -/// driver out without translation costs security: a descriptor holding a -/// physical address is an arbitrary read and write over all of memory. So the -/// kernel refuses the claim by name, the supervisor says which device it could not mint, -/// and netstack exits rather than driving anything — and the machine finishes -/// booting, which is the half a refusal that panicked would fail. The NVMe -/// controller is refused the same, and DATA with it by name: a disk that is -/// there and cannot be used is never answered with memory. +/// driver out without the unit costs security: a message nothing remaps raises +/// any vector on any CPU, and a descriptor holding a physical address is an +/// arbitrary read and write over all of memory. The first is the refusal a +/// claim takes first, before anything of the slot is spent. So the kernel +/// refuses the claim by name, the supervisor says which device it could not +/// mint, and netstack exits rather than driving anything — and the machine +/// finishes booting, which is the half a refusal that panicked would fail. The +/// NVMe controller is refused the same, and DATA with it by name: a disk that +/// is there and cannot be used is never answered with memory. fn no_unit_is_no_claim(log: &Serial) -> Result<(), String> { - const NO_DOMAIN: &str = "it would have no address space of its own"; + const NOT_REMAPPED: &str = "its interrupts would not be remapped on this machine"; // netstack's own exit is the third saying, and is not read here. - refused_claim(log, NETSTACK_CLAIMS, NO_DOMAIN, &[NVME_AT])?; - log.must_say(&format!("pcidev: PCI {NVME_AT} NOT HANDED OVER — {NO_DOMAIN}"))?; + refused_claim(log, NETSTACK_CLAIMS, NOT_REMAPPED, &[NVME_AT])?; + log.must_say(&format!("pcidev: PCI {NVME_AT} NOT HANDED OVER — {NOT_REMAPPED}"))?; log.must_say("supervisor: diskserver: pci:1b36:0010 is on this machine and could not be handed over")?; log.must_say(DISKSERVER_REFUSED)?; log.must_say(FILESERVER_WITHOUT_DATA)?; From 487c7746ced6a9c18b427852268ebf35d4e89d5c Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 06:03:51 +0200 Subject: [PATCH 5/8] Answer review round 1: a claim's space has no untranslated form, and the actuator stands in for firmware's flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - iommu: `OwnSpace` is a device's own address space with no `Untranslated` arm, made by `OwnSpace::create(room)`; `pcidev`'s `Space`, `Bound` and `Retired` hold it. `DeviceSpace::own`, `::map_at` and `::place` go, with the two panicking `Untranslated` arms. The only test of `Refusal::Untranslated` went when `NotRemapped` moved first; the type now refuses what it guarded: no claim path can hand a process a physical address. - vtd: `iommu-no-remap` clears `dmar::FLAG_INTR_REMAP` from the flags `remappable` reads instead of returning early with its own line, so the flag's arm is what refuses under it and `claim_refused_without_remapping` reads that arm's `INTR_REMAP clear` line. - vtd: the claim slots' entry count is `pcidev::MAX_FUNCTIONS`, read in `interrupt.rs` as a const with a const assertion against the table's size; the `claims` parameter goes from four signatures and the aarch64 stub. - pcidev: `tear_down` and `bring_up`'s refusal read `COMMAND` back (`PciDevice::drain_messages`) before the slot's entry goes not present: a completion passes none of the function's earlier posted writes (PCIe Base §2.4.1), so no message sent before mastering stopped can reach the not-present entry and halt the machine in the fault handler. - claims judge: the refusal row also reads that no BAR of the I219 moved. - A false comment that the remapping refusal precedes the slot's spending goes from `bring_up` and `iommu_virtio_platform`'s doc: `claim` reserves the slot first. - Issues: the no-IOMMU claim file says `NotRemapped` comes first and the space has no untranslated form; the MSI-arm file is narrowed to the arms the T14 still does not reach (its I219 claim reaches `arm_claimed_msi` and `tear_down`'s MSI arm) and renamed to match; the 4.3 ms paint stall at `subsystems ready` on the head's lantalkcase boot is filed for the orchestrator. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...ubsystems-ready-stalled-4-ms-on-the-t14.md | 37 +++++++++ ...ne-without-an-iommu-refuses-every-claim.md | 14 ++-- ...aches-the-msi-arm-of-a-claimed-function.md | 25 ------ ...-msi-refusal-arms-of-a-claimed-function.md | 23 ++++++ kernel/src/actuator.rs | 4 +- kernel/src/arch/aarch64/iommu_unit.rs | 1 - kernel/src/arch/x86_64/vtd/interrupt.rs | 29 +++---- kernel/src/arch/x86_64/vtd/mod.rs | 19 ++--- kernel/src/drivers/pci.rs | 8 ++ kernel/src/iommu/mod.rs | 80 +++++++++++-------- kernel/src/pcidev/mod.rs | 23 +++--- src/metal.rs | 7 +- tests/common/claims.rs | 8 +- tests/common/iommu.rs | 3 +- 14 files changed, 165 insertions(+), 116 deletions(-) create mode 100644 issues/kernel/a-boot-paint-at-subsystems-ready-stalled-4-ms-on-the-t14.md delete mode 100644 issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md create mode 100644 issues/kernel/nothing-reaches-the-msi-refusal-arms-of-a-claimed-function.md diff --git a/issues/kernel/a-boot-paint-at-subsystems-ready-stalled-4-ms-on-the-t14.md b/issues/kernel/a-boot-paint-at-subsystems-ready-stalled-4-ms-on-the-t14.md new file mode 100644 index 00000000000..15b0cdfe59f --- /dev/null +++ b/issues/kernel/a-boot-paint-at-subsystems-ready-stalled-4-ms-on-the-t14.md @@ -0,0 +1,37 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A boot paint at `subsystems ready` stalled 4.3 ms on the T14 + +One T14 boot of `lantalkcase`, at `43cbe73d4` (pull request #710's head, run +on 2026-10-04), wrote `panel: paints=10 px=8513536 us=25527 max_us=8194`. The +machine's record for that boot is `boot.lantalkcase.panel_max_us = 3919` +(`tests/metal/lenovo-20w0003amz.toml`), and the judge's ceiling 7838, so the +judge exited 1. The same kernel's four other boots of that run read `max_us` +3921, 3802, 3878 and 3870, and the negative control's four 3837, 3883, 3927 +and 3853. + +The extra time is one stall, not slower painting: the pixel count matches the +other boots (8513536 against 8496384..8518016), and the total paint time is +about 4 ms over theirs (25527 µs against 20828..21906), all of it in the one +maximum. It sits at the `Boot: subsystems ready` checkpoint's paint: on that +boot the next record after `subsystems ready` (0.244 s) is at 0.253 s and the +xHCI's first remapping entry at 0.254 s, where every other boot of both +kernels reads 0.248..0.249 s for the one and 0.249 s for the other; `storage +ready` is late by as much (0.740 s against 0.734..0.736). + +The review of #710 read it as not that change's +(https://github.com/ToyOSOrg/ToyOS/pull/710#issuecomment-5976306774): every +panel paint is at a boot checkpoint, the last at `Boot: complete`, before the +I219's claim; and nothing the change adds runs between `peripherals ready` and +the claim. What stalled a paint for 4.3 ms is not known — a single boot, with +no instrument inside the paint. + +Owner: the orchestrator, who holds the T14 record. Exit: the stall's cause is +named from a measurement — the review's alternating `lantalkcase` boots of +`e81d26db8` and `43cbe73d4`, three each, comparing `max_us` and the gap from +`subsystems ready` to the next record, is the first — and removed, and three +`lantalkcase` boots then read `panel_max_us` inside the record's ceiling. diff --git a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md index 5b385c85bbe..22a1fbf8f68 100644 --- a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md +++ b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md @@ -20,11 +20,15 @@ where it is checked) and so what the "identity check" compares. No signature or image-identity mechanism exists in the kernel today. The exit below cannot be written as a test until it is. -Today `pcidev`'s `bring_up` refuses every claim on such a machine with -`Refusal::Untranslated`, whoever asks. A claim that goes on without a domain -cannot pass through `slot_space` as it stands: it calls `reserve`, grants -call `place`, and both panic on `DeviceSpace::Untranslated` — a kernel panic -a userland claim would reach. +Today `pcidev`'s `bring_up` refuses every claim on such a machine first with +`Refusal::NotRemapped`, whoever asks: no unit remaps its interrupts, and a +claimed function is armed only through `iommu::claim_msi`, which takes a +`Remapping` such a machine never mints. Behind that, `Refusal::Untranslated`: +a slot's space is an `iommu::OwnSpace`, which has no untranslated form, so +there is no physical address for a grant to answer with. The ruled path has +to pass both — a message for the claimed function that is not compatibility +format's any-vector-at-any-CPU, and a space of its own kind for physical +addresses. Exit, in both arms of `iommu_virtio_platform` (`tests/common/iommu.rs`): diff --git a/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md b/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md deleted file mode 100644 index 97806cade81..00000000000 --- a/issues/kernel/nothing-reaches-the-msi-arm-of-a-claimed-function.md +++ /dev/null @@ -1,25 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-09-08 ---- - -# Nothing reaches the MSI arm of a claimed function - -`pcidev::bring_up` arms a claimed function on MSI where it publishes no MSI-X, -and no test arms one, so nothing reaches: - -- `PciDevice::disable_msi` from either hand-back site (`bring_up`'s `place_bars` - failure and `tear_down`), or `Armed::Msi`'s teardown, which turns the - capability off where there is no table entry to mask; -- `Refusal::MsixUnusable` and `Unarmed::Blocked`, owed only by a function that - publishes MSI-X this kernel cannot arm and by a unit that refuses the message. - -The two pre-existing MSI armings in this kernel — xHCI's and HDA's -`arm_interrupt` — never disarm, so MSI teardown is exercised nowhere in the tree -at all. - -Owned by the network track's stage-2 I219 worker. Exit condition: the first -`userdev` interrupt counted against a claim on `00:1f.6` on the bench, which -needs the 32-bit BAR window before it, plus netd exiting from that claim, which -runs `tear_down`'s MSI arm. diff --git a/issues/kernel/nothing-reaches-the-msi-refusal-arms-of-a-claimed-function.md b/issues/kernel/nothing-reaches-the-msi-refusal-arms-of-a-claimed-function.md new file mode 100644 index 00000000000..aa9144d2175 --- /dev/null +++ b/issues/kernel/nothing-reaches-the-msi-refusal-arms-of-a-claimed-function.md @@ -0,0 +1,23 @@ +--- +status: open +kind: tooling +opened: 2026-09-08 +--- + +# Nothing reaches the MSI refusal arms of a claimed function + +`pcidev::bring_up` arms a claimed function on MSI where it publishes no MSI-X. +The T14's I219 is one: `claim_reuses_its_remapping_entry` arms it through +`arm_claimed_msi` and releases it through `tear_down`'s MSI arm, and +`lan_message_delivery` counts its messages. Nothing reaches: + +- `bring_up`'s refusal after the MSI is armed — `place_bars` refusing a + function armed on MSI — where `PciDevice::disable_msi` runs and the slot's + remapping entry is dropped; +- `Refusal::MsixUnusable`, owed only by a function that publishes MSI-X this + kernel cannot arm. + +Owned by the network track's stage-2 I219 worker. Exit condition: a bench run +logs a claim refused after its `msi address=` line, with that slot's +`irteN … p=0 released` after it; and a function whose MSI-X cannot be armed is +refused by `MsixUnusable`'s reason. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 10043772944..24d668cdf17 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -117,8 +117,8 @@ actuators! { /// kernel programs it. Judged by `iommu_firmware_left`. iommu_firmware_left = "iommu-firmware-left"; - /// Leave every IOMMU unit's interrupts unremapped, as on a machine whose - /// units cannot remap them. Judged by `claim_refused_without_remapping`. + /// Read the DMAR's flags with `INTR_REMAP` clear, as on a platform whose + /// firmware says it does not remap. Judged by `claim_refused_without_remapping`. iommu_no_remap = "iommu-no-remap"; /// Leave every AP holding the CR0/CR4 that INIT left it. diff --git a/kernel/src/arch/aarch64/iommu_unit.rs b/kernel/src/arch/aarch64/iommu_unit.rs index 13c0b28e305..bb07a406314 100644 --- a/kernel/src/arch/aarch64/iommu_unit.rs +++ b/kernel/src/arch/aarch64/iommu_unit.rs @@ -8,7 +8,6 @@ pub fn init( _rsdp_addr: u64, _devices: &[PciDevice], _windows: &[toyos_abi::boot::RootBridgeWindow], - _claims: usize, ) { log!("IOMMU: the SMMUv3 is the port's stage 6; no device is translated this boot"); } diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 42fe3233cc7..a4d8b254607 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -81,9 +81,6 @@ pub struct Pin { struct Remap { /// `None` until a unit is armed, which is what "no source may use the remappable format" means. table: Option
, - /// Entries `0..claims` are the claim slots'; the kernel's own sources take - /// them from here up. - claims: u16, used: u16, /// `ECAP.EIM` on every unit. Clear bounds a destination to the eight bits `DST` then holds. extended: bool, @@ -92,7 +89,12 @@ struct Remap { } static REMAP: Lock = - Lock::new(Remap { table: None, claims: 0, used: 0, extended: false, apics: Vec::new() }); + 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); /// A claimed function's message reaches [`MSG_DEST`], which an entry holds with /// or without `EIME`, so writing a claim slot's entry is never refused. @@ -107,21 +109,12 @@ pub fn apics_are_named(apics: &[u8]) -> bool { apics.iter().all(|id| remap.apics.iter().any(|(named, _)| named == id)) } -/// Allocate the shared table on first ask, its first `claims` entries kept for -/// the claim slots, and return the value `IRTA_REG` takes for it. -pub fn arm(extended: bool, claims: usize) -> u64 { - let claims = u16::try_from(claims).ok().filter(|c| *c < ENTRIES).unwrap_or_else(|| { - panic!("iommu: {claims} claim slots leave none of the table's {ENTRIES} entries over") - }); +/// Allocate the shared table on first ask and return the value `IRTA_REG` +/// takes for it. +pub fn arm(extended: bool) -> u64 { let mut remap = REMAP.lock(); remap.extended = extended; - let table = match remap.table { - Some(table) => table, - None => { - (remap.claims, remap.used) = (claims, claims); - *remap.table.insert(super::TABLES.lock().alloc()) - } - }; + let table = *remap.table.get_or_insert_with(|| super::TABLES.lock().alloc()); table.phys() | if extended { EXTENDED_INTERRUPT_MODE } else { 0 } | SIZE_FIELD } @@ -232,7 +225,7 @@ pub fn claim(slot: usize, source: StreamId, vector: u8) -> Msi { }; let index = u16::try_from(slot) .ok() - .filter(|index| *index < remap.claims) + .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. diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 923755cd563..996b3aab9f1 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -125,8 +125,8 @@ pub(super) fn invalidate_interrupt_entries() { } /// `windows` and every region firmware reserved are what no domain's addresses -/// reach; the interrupt table's first `claims` entries are the claim slots'. -pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: &[RootBridgeWindow], claims: usize) { +/// reach. +pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: &[RootBridgeWindow]) { let dmar = match Dmar::open(rsdp_addr) { Ok(dmar) => dmar, // ACPI cannot distinguish "no VT-d silicon" from "VT-d disabled in @@ -250,7 +250,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: &[RootBridgeWindow], // `CAP.SAGAW`, and a shared set would be programmed at the wrong depth for one. let mut domains: [Option
; 2] = [None, None]; for (unit, plan) in ready { - enable(unit, plan, devices, &mut domains, remap, claims); + enable(unit, plan, devices, &mut domains, remap); } } @@ -262,14 +262,10 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: &[RootBridgeWindow], /// interrupt to whatever the handle bits spell. Every condition below therefore /// refuses for the machine, not for the unit that failed it. fn remappable(ready: &[(Unit, Plan)], described: usize, flags: u8) -> Option { + // Stands in for firmware, so the flag's own arm is what refuses. #[cfg(feature = "boot-actuators")] - if crate::actuator::iommu_no_remap() { - log!( - "iommu: iommu-no-remap stands in for units that cannot remap, so every source stays \ - in compatibility format" - ); - return None; - } + let flags = + if crate::actuator::iommu_no_remap() { flags & !dmar::FLAG_INTR_REMAP } else { flags }; if ready.is_empty() { return None; } @@ -526,7 +522,6 @@ fn enable( devices: &[PciDevice], domains: &mut [Option
; 2], remap: Option, - claims: usize, ) { let index = unit.index; let Plan { width, records } = plan; @@ -554,7 +549,7 @@ fn enable( let mut queue = Queue::new(&mut TABLES.lock(), unit.regs); // Outside the `TABLES` lock: `interrupt::arm` takes its own lock and // then that one, and the order this subsystem holds is the reverse. - let irta = remap.map(|extended| interrupt::arm(extended, claims)); + let irta = remap.map(|extended| interrupt::arm(extended)); // Before `TE`: the first blocked transaction must be reportable, not merely counted. fault::arm(index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); diff --git a/kernel/src/drivers/pci.rs b/kernel/src/drivers/pci.rs index e021e1a449a..caddc3743d2 100644 --- a/kernel/src/drivers/pci.rs +++ b/kernel/src/drivers/pci.rs @@ -273,6 +273,14 @@ impl PciDevice { stop_bus_mastering(self.mmio); } + /// Answers once every message this function sent before the writes that + /// silenced it has reached the root complex: a read's completion passes + /// none of the function's earlier posted writes, and the read itself none + /// of this kernel's (PCIe Base §2.4.1). + pub fn drain_messages(&self) { + self.mmio.read_u16(COMMAND); + } + /// Point this function's [`MSIX_ENTRY`] at `vector` and enable it: a /// kernel driver's arming, whose message is compatibility format on a /// machine that remaps nothing. A claimed function is armed by diff --git a/kernel/src/iommu/mod.rs b/kernel/src/iommu/mod.rs index 9e4527ed705..3bcf02d17d1 100644 --- a/kernel/src/iommu/mod.rs +++ b/kernel/src/iommu/mod.rs @@ -2,7 +2,7 @@ //! //! Inventories the machine's IOMMU units, gives every enumerated PCI function an identity-mapped context entry, turns translation on, remaps every interrupt source through a source-id-verified table entry, 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 `vtd/` stay backend-neutral so a second backend drops in without moving the seam. //! -//! 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 ([`DeviceSpace::own`], [`remapping`]), because a descriptor it writes a physical address into is an arbitrary read and write over all of memory, and a message it sends unremapped is any vector at any CPU; its message is its claim slot's own entry ([`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 it sends unremapped is any vector at any CPU; its message is its claim slot's own entry ([`Remapped`]). //! //! `trait Iommu` is deliberately not added: with one backend it would have a single implementor. @@ -162,17 +162,6 @@ pub enum DeviceSpace { } impl DeviceSpace { - /// One of a device's own with `room` bytes of it handed out at the address - /// answered, or the reason there is none. - /// - /// The refusing form, for `pcidev`: a function a *process* drives must - /// never be handed a physical address, so a machine with no unit and a - /// machine out of domains are both answers its caller refuses the claim - /// with rather than degrading past. - pub fn own(room: u64) -> Result<(Self, u64), IommuError> { - unit::domain::create(room).map(|(id, at)| (Self::Own(id), at.raw())) - } - /// One of a device's own, or the machine's own with the reason. For a /// driver **in this kernel**, whose addresses are the kernel's either way. pub fn create() -> Self { @@ -199,26 +188,6 @@ impl DeviceSpace { } } - /// Put `bytes` at `phys` at `at` again, where a device may still be aimed - /// from a mapping this space took back. Only a space of its own has such - /// an address; a physical one has no address to choose. - pub fn map_at(self, at: u64, phys: u64, bytes: u64) -> Result<(), IommuError> { - match self { - Self::Untranslated => panic!("iommu: an untranslated space was asked to place {phys:#x} at {at:#x}"), - Self::Own(id) => unit::domain::map_at(id, Iova::translated(at), phys, bytes), - } - } - - /// Put `bytes` at `phys` at `at`, inside room [`Self::own`] handed out, - /// and write no record of it: for a mapping its holder makes and takes back - /// as often as it likes. - pub fn place(self, at: u64, phys: u64, bytes: u64) -> Result<(), IommuError> { - match self { - Self::Untranslated => panic!("iommu: an untranslated space was asked to place {phys:#x} at {at:#x}"), - Self::Own(id) => unit::domain::place(id, Iova::translated(at), phys, bytes).map(|_| ()), - } - } - /// Take `bytes` at `at` back, so the pages behind them can be reused. pub fn unmap(self, at: u64, bytes: u64) -> Result<(), IommuError> { match self { @@ -231,11 +200,54 @@ impl DeviceSpace { /// in place: the device is translating the moment this returns. pub fn attach(self, bus: u8, device: u8, function: u8) { if let Self::Own(id) = self { - unit::domain::attach(StreamId::pci(bus, device, function), id); + OwnSpace(id).attach(bus, device, function); } } } +/// A device's own address space, for a function a *process* drives: it has no +/// untranslated form, so nothing holding one can hand that process a physical +/// address to write into a descriptor. +#[derive(Clone, Copy)] +pub struct OwnSpace(DomainId); + +impl OwnSpace { + /// One with `room` bytes of it handed out at the address answered, or the + /// reason there is none: a machine with no unit and a machine out of + /// domains are both refusals. + pub fn create(room: u64) -> Result<(Self, u64), IommuError> { + unit::domain::create(room).map(|(id, at)| (Self(id), at.raw())) + } + + /// [`DeviceSpace::map`]. + pub fn map(self, phys: u64, bytes: u64) -> Result { + unit::domain::map(self.0, phys, bytes).map(Iova::raw) + } + + /// Put `bytes` at `phys` at `at` again, where a device may still be aimed + /// from a mapping this space took back. + pub fn map_at(self, at: u64, phys: u64, bytes: u64) -> Result<(), IommuError> { + unit::domain::map_at(self.0, Iova::translated(at), phys, bytes) + } + + /// Put `bytes` at `phys` at `at`, inside room [`Self::create`] handed out, + /// and write no record of it: for a mapping its holder makes and takes back + /// as often as it likes. + pub fn place(self, at: u64, phys: u64, bytes: u64) -> Result<(), IommuError> { + unit::domain::place(self.0, Iova::translated(at), phys, bytes).map(|_| ()) + } + + /// [`DeviceSpace::unmap`]. + pub fn unmap(self, at: u64, bytes: u64) -> Result<(), IommuError> { + unit::domain::unmap(self.0, Iova::translated(at), bytes) + } + + /// [`DeviceSpace::attach`]. + pub fn attach(self, bus: u8, device: u8, function: u8) { + unit::domain::attach(StreamId::pci(bus, device, function), self.0); + } +} + /// Formats as `bb:dd.f`, the same form `pci::enumerate` prints, so a stream id can be matched against it. impl core::fmt::Display for StreamId { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { @@ -257,7 +269,7 @@ pub fn init( devices: &[crate::drivers::pci::PciDevice], windows: &[toyos_abi::boot::RootBridgeWindow], ) { - unit::init(rsdp_addr, devices, windows, crate::pcidev::MAX_FUNCTIONS); + unit::init(rsdp_addr, devices, windows); } /// How a kernel driver's source must address its interrupt. Not a yes/no: a diff --git a/kernel/src/pcidev/mod.rs b/kernel/src/pcidev/mod.rs index c1fad1c82c5..1590f7a8533 100644 --- a/kernel/src/pcidev/mod.rs +++ b/kernel/src/pcidev/mod.rs @@ -115,7 +115,7 @@ use toyos_pci::{af, aperture, bar, express, msix, placement, pm, probe}; use crate::device::{Claim, ClaimError}; use crate::drivers::pci::{arm_claimed_msi, arm_claimed_msix, NoCapability, NoEntry, PciDevice}; -use crate::iommu::{DeviceSpace, IommuError, Remapped}; +use crate::iommu::{IommuError, OwnSpace, Remapped}; use crate::mm::policy::{CachePolicy, MmioPolicy}; use crate::mm::{align_2m, DirectMap, Mmio, PAGE_2M}; use crate::object::shm::{Region, SharedMemObject}; @@ -172,7 +172,7 @@ static SPACE: [Lock>; MAX_FUNCTIONS] = /// A slot's address space, and the room in it every holder's lent regions go. #[derive(Clone, Copy)] struct Space { - space: DeviceSpace, + space: OwnSpace, /// [`MAX_GRANT_TOTAL`] of addresses handed out with the domain and never /// mapped by anything but [`dma_map`]. lend: u64, @@ -229,7 +229,7 @@ enum Armed { /// What a live slot drives. The ISR never reads this. struct Bound { pci: PciDevice, - space: DeviceSpace, + space: OwnSpace, /// [`Space::lend`]. lend: u64, armed: Armed, @@ -756,8 +756,6 @@ pub fn claim(id: PciId) -> Result<(PciFunctionInfo, u8, Claim), ClaimError> { /// 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 { - // A fact of the whole machine, fixed at boot: refused before the slot or - // the function is spent or touched. let remapping = crate::iommu::remapping().map_err(|_| Refusal::NotRemapped)?; // What the slot's previous holder left mapped goes before anything attaches @@ -817,16 +815,18 @@ fn bring_up(pci: PciDevice, id: PciId, slot: usize) -> Result { } Err(why) => { // The entry goes not present once the function stops speaking. - match armed { + let message = match armed { Armed::Msix(_, message) => { pci.disable_msix(); - drop(message); + message } Armed::Msi(message) => { pci.disable_msi(); - drop(message); + message } - } + }; + pci.drain_messages(); + drop(message); Err(why) } } @@ -898,7 +898,7 @@ fn slot_space(slot: usize) -> Result { match *held { Some(space) => Ok(space), None => { - let (space, lend) = DeviceSpace::own(MAX_GRANT_TOTAL)?; + let (space, lend) = OwnSpace::create(MAX_GRANT_TOTAL)?; *held = Some(Space { space, lend }); Ok(Space { space, lend }) } @@ -1123,7 +1123,7 @@ fn wait_until(at: u64) { /// released holder's own pages rather than faulting, or landing in pages the /// allocator has handed somebody else. struct Retired { - space: DeviceSpace, + space: OwnSpace, grants: Vec, quiet_at: u64, } @@ -1425,6 +1425,7 @@ fn tear_down(slot: usize, mut bound: Bound) { }; // Once the function no longer speaks, and before the slot can be reserved // again: its remapping entry goes not present. + 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 diff --git a/src/metal.rs b/src/metal.rs index 98e814ecbe7..c73ea919702 100644 --- a/src/metal.rs +++ b/src/metal.rs @@ -772,9 +772,10 @@ pub const FLASHABLE: &[&str] = &[ // writes only the units' registers, which a reset returns to firmware, and // tables in memory this boot owns. "iommu-firmware-left", - // Every IOMMU unit left without interrupt remapping, which is how this - // machine ran before the kernel remapped anything. It writes no register a - // reset does not return to firmware. + // The DMAR's flags read with INTR_REMAP clear, so every IOMMU unit is left + // without interrupt remapping, which is how this machine ran before the + // kernel remapped anything. It writes no register a reset does not return + // to firmware. "iommu-no-remap", // It seals this boot's own record under an identity one bit from this // stick's, so the pass that finds it clears it and boots a kernel. The page diff --git a/tests/common/claims.rs b/tests/common/claims.rs index b5eaddd7161..c40c30b6fd7 100644 --- a/tests/common/claims.rs +++ b/tests/common/claims.rs @@ -13,8 +13,9 @@ pub const RECLAIM: &str = "test_rs_pci_reclaim"; /// The kernel's reason on a claim's refusal line where the units do not remap. const NOT_REMAPPED: &str = "its interrupts would not be remapped on this machine"; -/// The kernel's line under `iommu-no-remap`: the premise of the refusal. -const NO_REMAP_ARMED: &str = "iommu: iommu-no-remap stands in for units that cannot remap"; +/// The kernel's line where firmware's DMAR flags leave interrupt remapping off, +/// which `iommu-no-remap` stands in for: the premise of the refusal. +const INTR_REMAP_CLEAR: &str = "leave INTR_REMAP clear, so firmware says this platform does not remap"; fn records(log: &Serial) -> impl Iterator { log.text().lines().filter_map(bootlog::message) @@ -132,13 +133,14 @@ pub fn clear_of_host_bridges(log: &Serial) -> Result<(), String> { /// On a machine whose units do not remap, the I219's claim is refused by that /// reason before anything on the function is armed, and the boot completes. pub fn refused_unremapped(log: &Serial) -> Result<(), String> { - log.must_say(NO_REMAP_ARMED)?; + log.must_say(INTR_REMAP_CLEAR)?; let at = function(log)?; log.must_say(&format!("pcidev: PCI {at} NOT HANDED OVER — {NOT_REMAPPED}"))?; log.must_not_say(&format!("[{I219}] handed over"))?; log.must_not_say(&format!("PCI {at}: msi address="))?; log.must_not_say(&format!("PCI {at}: msix address="))?; log.must_not_say(&format!(" source={at} "))?; + log.must_not_say(&format!("pcidev: PCI {at} BAR"))?; log.must_say(bootlog::COMPLETE)?; Ok(()) } diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 1afa5559ce6..a4f7a7d7388 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -181,8 +181,7 @@ const NVME_AT: &str = "00:02.0"; /// (`issues/kernel/every-driver-is-still-in-the-kernel.md`) is that moving a /// driver out without the unit costs security: a message nothing remaps raises /// any vector on any CPU, and a descriptor holding a physical address is an -/// arbitrary read and write over all of memory. The first is the refusal a -/// claim takes first, before anything of the slot is spent. So the kernel +/// arbitrary read and write over all of memory. So the kernel /// refuses the claim by name, the supervisor says which device it could not /// mint, and netstack exits rather than driving anything — and the machine /// finishes booting, which is the half a refusal that panicked would fail. The From 8abd897f091a82e5fc35ec561131707b153cffda Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 06:21:17 +0200 Subject: [PATCH 6/8] vtd: interrupt::arm is passed as the function, not a closure around it Clippy's redundant_closure, once the claims argument went. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/arch/x86_64/vtd/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 996b3aab9f1..7fc808bd536 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -549,7 +549,7 @@ fn enable( let mut queue = Queue::new(&mut TABLES.lock(), unit.regs); // Outside the `TABLES` lock: `interrupt::arm` takes its own lock and // then that one, and the order this subsystem holds is the reverse. - let irta = remap.map(|extended| interrupt::arm(extended)); + let irta = remap.map(interrupt::arm); // Before `TE`: the first blocked transaction must be reportable, not merely counted. fault::arm(index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); From fba596ca9f4a81657db7ce3fbe98a802b6eb37cb Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 07:50:44 +0200 Subject: [PATCH 7/8] Answer review round 2: the no-unit issue's ruled path is where both refusals give way, and the iommu-no-remap boot has its record - issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md: the "ruled path has to pass both" sentence narrowed the owner's 2026-09-29 ruling (with no unit nothing remaps, so no non-compatibility message exists); it now says the signed driver's claim is where NotRemapped and Untranslated both give way under the ruling. - tests/metal/lenovo-20w0003amz.toml: the iommu-no-remap boot's recorded complete_ms/panel_max_us/panel_us (1152/3912/21226), from the judge of metal-head-claims at e0a49d06c, as 314aa6129 did for isa-withheld. - src/metal.rs: the actuator's comment loses its chronology clause. - Two owner rulings of 2026-10-04 for the track's later stages, recorded where those stages land, not built: the TTM/ESRTPS forced gap is accepted and filed (a-unit-left-translating-...), and a T14 row in which a test program claims the GPU and darkens the panel for that boot is allowed (a-device-without-a-domain-of-its-own-...). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...vice-without-a-domain-of-its-own-reaches-all-memory.md | 5 +++++ .../a-machine-without-an-iommu-refuses-every-claim.md | 7 +++---- ...ranslating-passes-dma-untranslated-while-programmed.md | 8 ++++++++ src/metal.rs | 5 ++--- tests/metal/lenovo-20w0003amz.toml | 3 +++ 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/issues/kernel/a-device-without-a-domain-of-its-own-reaches-all-memory.md b/issues/kernel/a-device-without-a-domain-of-its-own-reaches-all-memory.md index 2faa9d1d82d..6d9a129f78c 100644 --- a/issues/kernel/a-device-without-a-domain-of-its-own-reaches-all-memory.md +++ b/issues/kernel/a-device-without-a-domain-of-its-own-reaches-all-memory.md @@ -17,6 +17,11 @@ under the T14's `CONFIG_INTEL_IOMMU_DEFAULT_ON=y` and default domain (`drivers/iommu/iommu.c:197-210`), which maps only what its driver maps. +**Owner ruling, 2026-10-04: a T14 row in which a test program claims the GPU +(`00:02.0`, alone in `rmrr0`) is allowed, and the kernel panel goes dark for +that boot.** It is the one row that can reach a claim of a function with a +reserved region of its own. + **Exit**: a function's DMA reaches only what its driver mapped and the reserved ranges firmware names for it; a `boot-actuators` arm that points an unclaimed function's DMA at a kernel page records a translation fault, and binding it to diff --git a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md index 22a1fbf8f68..01b54321b91 100644 --- a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md +++ b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md @@ -25,10 +25,9 @@ Today `pcidev`'s `bring_up` refuses every claim on such a machine first with claimed function is armed only through `iommu::claim_msi`, which takes a `Remapping` such a machine never mints. Behind that, `Refusal::Untranslated`: a slot's space is an `iommu::OwnSpace`, which has no untranslated form, so -there is no physical address for a grant to answer with. The ruled path has -to pass both — a message for the claimed function that is not compatibility -format's any-vector-at-any-CPU, and a space of its own kind for physical -addresses. +there is no physical address for a grant to answer with. The signed driver's +claim is the path where `NotRemapped` and `Untranslated` both give way under +the ruling. Exit, in both arms of `iommu_virtio_platform` (`tests/common/iommu.rs`): diff --git a/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md b/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md index 6638fddcb42..2b9a981bf27 100644 --- a/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md +++ b/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md @@ -27,6 +27,14 @@ with translation on` at 0.083 s, just before translation goes off, and `translating` at 0.090 s, just after it is back; two of the hand-over's own log lines fall between. +One case keeps the window whatever this kernel does: a unit handed over with +`TTM` not `00` and `CAP.ESRTPS` clear, where §6.6 forbids switching tables +under translation, so `TE` has to go off. **Owner ruling, 2026-10-04: accept +it and file it.** The fix that closes this issue logs that case and files it +as its own defect, whose exit covers the window with protected memory regions +(`PMEN` over every pmm frame) or has the kernel speak scalable mode. +No machine here reaches it. + **Exit**: under `iommu-firmware-left`, on QEMU and on the T14, no unit logs `was handed over with translation on; it goes off first`, and every unit's `translating` line reads `tes=y`. diff --git a/src/metal.rs b/src/metal.rs index c73ea919702..e36a0b46af9 100644 --- a/src/metal.rs +++ b/src/metal.rs @@ -773,9 +773,8 @@ pub const FLASHABLE: &[&str] = &[ // tables in memory this boot owns. "iommu-firmware-left", // The DMAR's flags read with INTR_REMAP clear, so every IOMMU unit is left - // without interrupt remapping, which is how this machine ran before the - // kernel remapped anything. It writes no register a reset does not return - // to firmware. + // without interrupt remapping. It writes no register a reset does not + // return to firmware. "iommu-no-remap", // It seals this boot's own record under an identity one bit from this // stick's, so the pass that finds it clears it and boots a kernel. The page diff --git a/tests/metal/lenovo-20w0003amz.toml b/tests/metal/lenovo-20w0003amz.toml index 21efc0c463b..be66f3ca55f 100644 --- a/tests/metal/lenovo-20w0003amz.toml +++ b/tests/metal/lenovo-20w0003amz.toml @@ -13,6 +13,9 @@ bios = "N34ET71W (1.71 )" "boot.hardlockup.complete_ms" = 1164 "boot.hardlockup.panel_max_us" = 3949 "boot.hardlockup.panel_us" = 21772 +"boot.iommu-no-remap.complete_ms" = 1152 +"boot.iommu-no-remap.panel_max_us" = 3912 +"boot.iommu-no-remap.panel_us" = 21226 "boot.isa-withheld.complete_ms" = 780 "boot.isa-withheld.panel_max_us" = 3888 "boot.isa-withheld.panel_us" = 19251 From cfe2cf728285f163a9250863a4876c0243f7049d Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 08:15:34 +0200 Subject: [PATCH 8/8] Answer review round 3: the no-unit issue cites the claim's space, OwnSpace The parenthetical named DeviceSpace, which serves kernel drivers only; a claim's space is OwnSpace, which has no untranslated form until the ruled path adds one. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- .../kernel/a-machine-without-an-iommu-refuses-every-claim.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md index 01b54321b91..5f67cab685a 100644 --- a/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md +++ b/issues/kernel/a-machine-without-an-iommu-refuses-every-claim.md @@ -10,8 +10,9 @@ Owner ruling, 2026-09-29: on a machine with no IOMMU unit a driver signed and shipped in the ToyOS image may claim a device, and no other may. It is the same driver code as on a machine with a unit, never a second driver: the kernel's DMA layer hands it a physical address where there is no -unit and a domain address where there is (`DeviceSpace`, -`kernel/src/iommu/mod.rs`). Such a machine states plainly that it has no +unit and a domain address where there is. A claim's space is an +`iommu::OwnSpace` (`kernel/src/iommu/mod.rs`), which has no untranslated form +until the ruled path adds one. Such a machine states plainly that it has no isolation — the full isolation guarantee needs an IOMMU, and there a bad signed driver can still crash or corrupt the system.