From e8d181879432c67597efac3e5aa0950e3754fa08 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 28 Oct 2026 10:14:42 +0100 Subject: [PATCH 1/4] vtd: a unit left translating stays on where firmware declares its DMA, and a reproducer for the T14's iGPU fault MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One T14 shared-debug boot under iommu-firmware-left halted on a read by the iGPU (00:02.0) at 0x9cea7000, inside the RMRR firmware reserves for it, with reason 6h: a second-stage entry with R clear (VT-d 5.0 Table 30, LGN.3). The identity domain maps that address read-write, so unit0 did not translate by the tables it had been given. Under the actuator, and only there, unit0 goes on, off and on while the firmware's scanout reads through it. The orchestrator's T14 logs hold 787 boots with a single on and no iGPU fault, and 46 cycled boots, one of which faulted. That points at translation going off under live DMA, but does not establish it. The hand-over now keeps translation on where §6.6 allows it. With DMA_CTRL_PLATFORM_OPT_IN (§8.1), firmware's DMA reaches only its RMRRs, and §3.16 has those mapped identity and read-write. Where the identity domain holds every RMRR and every device they name is enumerated, this kernel's root translates every request in flight as firmware's did. The root switch then happens under translation (RTADDR.RTA may change while TES is set, §11.4.5), with the global invalidation immediately after SRTP. The T14 declares the flag. QEMU does not, so its unit still goes off first and says why; the new vtd_firmware_left machine test holds that branch. iommu-handover-cycles moves every unit between its root and a copy of it 200 times by each route: off-on and the switch under translation. Fault events are held throughout, every record is counted against the cycle it followed, and the first sixteen are printed with the walk the tables give from memory. Its row, iommu_handover_cycles, runs only by name, on a boot of its own. The iommu_firmware_left judge now requires, on a declaring machine, that every unit kept translation on. Co-Authored-By: Claude Opus 5.5 --- ...asses-dma-untranslated-while-programmed.md | 30 +-- ...r-after-a-handover-took-translation-off.md | 60 ++++++ kernel/src/actuator.rs | 8 + kernel/src/arch/x86_64/vtd/cycles.rs | 184 ++++++++++++++++++ kernel/src/arch/x86_64/vtd/fault.rs | 105 ++++++++-- kernel/src/arch/x86_64/vtd/mod.rs | 138 ++++++++++--- kernel/src/arch/x86_64/vtd/table.rs | 82 ++++++++ tests/common/iommu.rs | 31 +++ tests/metal/src/lib.rs | 6 + tests/toyos.rs | 57 +++++- 10 files changed, 639 insertions(+), 62 deletions(-) create mode 100644 issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md create mode 100644 kernel/src/arch/x86_64/vtd/cycles.rs diff --git a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md b/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md index bff8201b065..ebce09af18b 100644 --- a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md +++ b/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md @@ -8,20 +8,26 @@ opened: 2026-10-04 `vtd::enable` (`kernel/src/arch/x86_64/vtd/mod.rs`) switches off the translation firmware left on (`Unit::hand_over`) once the unit's tables are -built, and turns it back on through this kernel's root table only after the -queue, fault reporting, both table pointers and a global invalidation are -programmed. In between, every function behind that unit reaches all of memory. +built, unless firmware declares what it has in flight (below), and turns it +back on through this kernel's root table only after the queue, fault +reporting, both table pointers and a global invalidation are programmed. In +between, every function behind that unit reaches all of memory. -Keeping `TE` on instead is what VT-d Rev. 4.1 §6.6 allows only onto tables that -remap exactly as the ones being walked, which takes firmware's tables read and -copied first, as Linux's kdump path does (`copy_translation_tables`, -`drivers/iommu/intel/iommu.c`); Linux outside kdump switches `TE` off as this -kernel does. Any copy has to read them before the pmm hands their memory out: -it takes `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, -`is_usable_type`). +Keeping `TE` on is what VT-d Rev. 4.1 §6.6 allows only onto tables that remap +in-flight requests exactly as the ones being walked. Where firmware sets +`DMA_CTRL_PLATFORM_OPT_IN` (§8.1), its DMA reaches only its RMRRs, which +§3.16 has mapped identity and read-write. If the identity domain holds every +RMRR and every device they name is enumerated, this kernel's root translates +them alike, and the hand-over keeps `TE` on. The T14 is such a machine. +Elsewhere, firmware's tables have to be read and copied first, as Linux's +kdump path does (`copy_translation_tables`, `drivers/iommu/intel/iommu.c`); +Linux outside kdump switches `TE` off as this kernel does. Any copy has to +read them before the pmm hands their memory out: it takes +`EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, `is_usable_type`). Evidence: no machine here is handed a unit translating; the T14's four units -read `gsts=0x40000000` (pull request #700's selftests boot). Staged by the +read `gsts=0x40000000` (pull request #700's selftests boot). QEMU's DMAR leaves +`DMA_CTRL_PLATFORM_OPT_IN` clear (`dma_ctrl_opt_in=n`). Staged by the `iommu-firmware-left` actuator on QEMU 11.1.1, unit0 logs `was handed over 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 @@ -35,6 +41,6 @@ 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 +**Exit**: under `iommu-firmware-left`, on QEMU as 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/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md b/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md new file mode 100644 index 00000000000..3caee299824 --- /dev/null +++ b/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md @@ -0,0 +1,60 @@ +--- +status: open +kind: defect +opened: 2026-10-28 +--- + +# The T14's iGPU faulted inside its RMRR after a hand-over took translation off + +One T14 boot under `iommu-firmware-left` (shared-debug, at 7ef0a48c8) halted +1 ms after `Boot: peripherals ready`: + +``` +iommu: unit0 fault recording overflowed — earlier faults are lost +iommu: DMA FAULT owner=kernel unit0 stream=00:02.0 addr=0x000000009cea7000 access=read reason=0x06 domain=unrecorded bme=cleared unitfaults=1 streamfaults=1 first=y read-permission +``` + +What the boot establishes: + +- `00:02.0` is the iGPU, alone in unit0's scope. No driver runs it, so the + read is the scanout firmware left running. `rmrr0` reserves + `0x9c000000..0xa07fffff` for it, and the root bridges' memory starts at + `0xa0800000`, so the region ends at `TOLUD`. That is where the 11th + Generation Intel Core datasheet (631122 §2.5) puts GTT and data stolen + memory. The address lies inside the region. +- Reason 6h is a read that met a second-stage entry with `R` clear (VT-d 5.0 + Table 30, `LGN.3`). The identity domain maps all of `0x0..0x45f800000` read + and write, so unit0 did not translate by the tables it had been given. +- Fault reporting is armed after `hand_over` and clears every older record, + and `IF` is set throughout `vtd::init`. So the fault was taken after unit0 + was back on through this kernel's root, after units 1 to 3 were programmed. +- unit0 has one recording register (`nfr=1`), and the overflow shows at least + one more fault after that. + +On the T14 under `iommu-firmware-left`, unit0's translation went on, off and +on again while the scanout read through it. Every other T14 boot only turns +it on. The orchestrator's logs to 2026-10-28 hold 787 boots with one +transition and no iGPU fault, and 46 cycled boots with this one. That +supports translation going off under live DMA as the trigger, at p ≈ 0.055, +but does not establish it. + +**The change made**: where firmware sets `DMA_CTRL_PLATFORM_OPT_IN` and this +kernel's root translates every RMRR as firmware must, the hand-over leaves +translation on. The T14's root is then switched under translation (§6.6) +rather than through off and on. Under `iommu-firmware-left` the T14 now makes +the same single transition as every other boot. + +**What is not yet known**: whether the off-and-on cycle is what faults. The +`iommu-handover-cycles` actuator repeats it and the switch under translation +hundreds of times each, with fault events held, and prints each fault with +the cycle that preceded it and the walk the tables give from memory. Its +row, `iommu_handover_cycles`, runs only by name. + +Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). + +**Exit**: `iommu_handover_cycles` on the T14 places the fault. If the +`switch` cycles fault, this change does not cure it, and the defect is +re-scoped to what does. If only the `off-on` cycles fault, the cause is +recorded at `Unit::hand_over`, where translation still goes off for a unit +whose in-flight DMA is undeclared. Close once `iommu_firmware_left` passes on +the T14 with unit0 keeping translation on. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index c4c79a0b14c..819044cbafa 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -109,6 +109,14 @@ actuators! { /// `iommu_firmware_left` and `amdvi_firmware_left`. iommu_firmware_left = "iommu-firmware-left"; + /// Once every VT-d unit is programmed, move each between its root and a + /// copy of it hundreds of times, alternately with translation going off + /// and staying on, and count the fault records each way. Judged by + /// `iommu_handover_cycles`. + // A VT-d unit's alone, so AArch64 builds an accessor it never reads. + #[allow(dead_code)] + iommu_handover_cycles = "iommu-handover-cycles"; + /// QEMU's two `iommu-testdev` asked three things of the AMD-Vi unit: /// one on a domain of its own writes where it maps, the other on the /// identity domain writes to memory, then where that domain maps diff --git a/kernel/src/arch/x86_64/vtd/cycles.rs b/kernel/src/arch/x86_64/vtd/cycles.rs new file mode 100644 index 00000000000..e8da899157b --- /dev/null +++ b/kernel/src/arch/x86_64/vtd/cycles.rs @@ -0,0 +1,184 @@ +//! [`crate::actuator::iommu_handover_cycles`]: every translating unit moved +//! between its root and a copy of it, alternately by the two ways a hand-over +//! can take, while whatever DMA firmware left running goes on through it. +//! +//! `off-on` is what a hand-over did when translation had to go off: `TE` off, +//! the other root pointed, the global invalidation, `TE` on. `switch` is §6.6's +//! root switch under translation: the other root pointed and the global +//! invalidation, `TE` left on. The two roots translate alike, so a fault +//! either way is the unit's disagreement with tables it was given, and each is +//! printed with the walk those tables give from memory. +//! +//! Fault events are held for the whole run and the records read here, so a +//! fault is counted against the cycle it followed and the machine goes on. + +use alloc::vec::Vec; + +use crate::drivers::pci::PciDevice; +use crate::iommu::fault::Who; +use crate::log; + +use super::fault::{self, Held}; +use super::table::{self, Table}; +use super::{command, RTADDR_REG, SET_ROOT_TABLE_POINTER, TABLES, TRANSLATION_ENABLE, UNITS}; + +/// Cycles of each kind. They alternate, `off-on` onto the copy and `switch` +/// back, so the last lands every unit on its own root again. +const CYCLES: usize = 200; + +/// How long each cycle's result is watched: at least one refresh of a panel +/// scanning out at 50 Hz or faster, so every address a scanout reads is read +/// once after each transition. +const WATCH_NS: u64 = 20_000_000; + +/// Fault lines printed in full; every later one is only counted. +const LINES: usize = 16; + +#[derive(Clone, Copy, PartialEq)] +enum Kind { + OffOn, + Switch, +} + +impl Kind { + const fn name(self) -> &'static str { + match self { + Self::OffOn => "off-on", + Self::Switch => "switch", + } + } +} + +#[derive(Default)] +struct Tally { + /// Cycles after which at least one record was read. + faulted: usize, + records: usize, + overflowed: usize, + /// The fewest nanoseconds from a transition's end to a record being read. + soonest: Option, +} + +/// One live unit: the index its fault records are armed under, its own root +/// and the copy. +struct Roots { + index: usize, + own: Table, + copy: Table, +} + +pub(super) fn run(devices: &[PciDevice]) { + host_bridge(devices); + let roots: Vec = { + let units = UNITS.lock(); + let mut tables = TABLES.lock(); + units + .iter() + .map(|unit| Roots { + index: fault::index_of(unit.regs).expect("iommu-handover-cycles: a live unit with no fault records"), + own: unit.root, + copy: table::copy_root(&mut tables, unit.root), + }) + .collect() + }; + log!("iommu-handover-cycles: {} unit(s), {CYCLES} cycles of each kind, each watched {WATCH_NS} ns", roots.len()); + + fault::hold(true); + let mut tallies = [Tally::default(), Tally::default()]; + let mut printed = 0usize; + for cycle in 0..2 * CYCLES { + let kind = if cycle % 2 == 0 { Kind::OffOn } else { Kind::Switch }; + let onto = |roots: &Roots| if kind == Kind::OffOn { roots.copy } else { roots.own }; + { + let mut units = UNITS.lock(); + for (unit, roots) in units.iter_mut().zip(&roots) { + if kind == Kind::OffOn { + command(roots.index, unit.regs, TRANSLATION_ENABLE, false, "translation"); + } + unit.regs.write_u64(RTADDR_REG, onto(roots).phys()); + command(roots.index, unit.regs, SET_ROOT_TABLE_POINTER, true, "the root table pointer"); + unit.queue.invalidate_all(unit.regs); + if kind == Kind::OffOn { + command(roots.index, unit.regs, TRANSLATION_ENABLE, true, "translation"); + } + } + } + let tally = &mut tallies[kind as usize]; + let start = crate::clock::nanos_since_boot(); + let mut seen = false; + while crate::clock::nanos_since_boot() - start < WATCH_NS { + fault::take_held(|Held { unit, fault, overflowed }| { + let after = crate::clock::nanos_since_boot() - start; + seen = true; + tally.records += 1; + tally.overflowed += usize::from(overflowed); + tally.soonest = Some(tally.soonest.map_or(after, |s| s.min(after))); + let Who::Function(stream) = fault.who else { return }; + if printed == LINES { + return; + } + printed += 1; + let walk = roots.iter().find(|r| r.index == unit).map(|r| table::walk(onto(r), stream, fault.address)); + let walk: &dyn core::fmt::Display = match &walk { + Some(walk) => walk, + None => &"none", + }; + log!( + "iommu-handover-cycles: cycle {cycle} {} unit{unit} stream={stream} addr={:#018x} access={} \ + reason={:#04x} {} overflowed={} after={after}ns walk={walk}", + kind.name(), + fault.address, + fault.access, + fault.reason, + fault.name, + if overflowed { 'y' } else { 'n' }, + ); + }); + core::hint::spin_loop(); + } + tally.faulted += usize::from(seen); + } + fault::hold(false); + + for kind in [Kind::OffOn, Kind::Switch] { + let tally = &tallies[kind as usize]; + log!( + "iommu-handover-cycles: {} {CYCLES} cycles, {} with a fault, {} records, {} overflowed, soonest {}", + kind.name(), + tally.faulted, + tally.records, + tally.overflowed, + Soonest(tally.soonest), + ); + } +} + +struct Soonest(Option); + +impl core::fmt::Display for Soonest { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self.0 { + Some(ns) => write!(f, "{ns}ns"), + None => f.write_str("none"), + } + } +} + +/// Where an Intel host bridge put graphics stolen memory, so a record's address +/// can be placed in it: `BDSM` (data) at B0h and `BGSM` (GTT) at B4h, bits +/// 31:20 each, and `TOLUD` at BCh (11th Generation Intel Core Datasheet Vol. +/// 2a, 631122, §3.1.31-§3.1.32). The values as read; what they mean is +/// Intel's host bridge alone. +fn host_bridge(devices: &[PciDevice]) { + let Some(bridge) = devices.iter().find(|d| (d.bus, d.dev, d.func) == (0, 0, 0) && d.vendor_id() == 0x8086) else { + return; + }; + log!( + "iommu-handover-cycles: host bridge {:04x}:{:04x} BDSM={:#010x} BGSM={:#010x} TOLUD={:#010x}", + bridge.vendor_id(), + bridge.device_id(), + bridge.read_config_u32(0xB0), + bridge.read_config_u32(0xB4), + bridge.read_config_u32(0xBC), + ); +} diff --git a/kernel/src/arch/x86_64/vtd/fault.rs b/kernel/src/arch/x86_64/vtd/fault.rs index e5ea287014d..acf9fe873fb 100644 --- a/kernel/src/arch/x86_64/vtd/fault.rs +++ b/kernel/src/arch/x86_64/vtd/fault.rs @@ -134,23 +134,7 @@ fn drain(index: usize, regs: Mmio, records: u64, count: u32) -> usize { let mut seen = 0usize; for i in 0..count { let record = records + i as u64 * 16; - let high = regs.read_u64(record + 8); - if high & (1u64 << 63) == 0 { - continue; - } - let stream = StreamId::pci( - (high >> 8) as u8, - ((high >> 3) & 0x1F) as u8, - (high & 0x7) as u8, - ); - let reason = ((high >> 32) & 0xFF) as u8; - let fault = Fault { - who: Who::Function(stream), - address: regs.read_u64(record) & !0xFFF, - access: if high & (1u64 << 62) != 0 { Access::Read } else { Access::Write }, - reason, - name: reason_name(reason), - }; + let Some(fault) = read_record(regs, record) else { continue }; let halts = policy::report(index, &UNITS[index].faults, fault); clear_record(regs, record); if halts { @@ -161,10 +145,97 @@ fn drain(index: usize, regs: Mmio, records: u64, count: u32) -> usize { seen } +/// The fault recording register at `record`, if its `F` bit is set. +fn read_record(regs: Mmio, record: u64) -> Option { + let high = regs.read_u64(record + 8); + if high & (1u64 << 63) == 0 { + return None; + } + let stream = StreamId::pci((high >> 8) as u8, ((high >> 3) & 0x1F) as u8, (high & 0x7) as u8); + let reason = ((high >> 32) & 0xFF) as u8; + Some(Fault { + who: Who::Function(stream), + address: regs.read_u64(record) & !0xFFF, + access: if high & (1u64 << 62) != 0 { Access::Read } else { Access::Write }, + reason, + name: reason_name(reason), + }) +} + fn clear_record(regs: Mmio, record: u64) { regs.write_u32(record + 12, RECORD_FAULT); } +/// `FECTL.IM`: the unit records faults but raises no event for them. +#[cfg(feature = "boot-actuators")] +const FECTL_MASK: u32 = 1 << 31; + +/// Every armed unit's fault event masked, or unmasked once every record and +/// status bit is cleared: [`crate::actuator::iommu_handover_cycles`] reads the +/// records itself while it holds them, rather than [`service`] ending the +/// machine on the first. +#[cfg(feature = "boot-actuators")] +pub(super) fn hold(held: bool) { + for unit in &UNITS { + let phys = unit.regs.load(Ordering::Acquire); + if phys == 0 { + continue; + } + let regs = window(phys); + if held { + regs.write_u32(FECTL_REG, FECTL_MASK); + } else { + let (records, count) = (unit.records.load(Ordering::Relaxed), unit.count.load(Ordering::Relaxed)); + for i in 0..count { + clear_record(regs, records + i as u64 * 16); + } + regs.write_u32(FSTS_REG, regs.read_u32(FSTS_REG) & FSTS_WRITE_ONE_TO_CLEAR); + regs.write_u32(FECTL_REG, 0); + } + } +} + +/// The index the unit whose registers are `regs` was armed under. +#[cfg(feature = "boot-actuators")] +pub(super) fn index_of(regs: Mmio) -> Option { + let phys = DirectMap::phys_of(regs.addr() as *const u8); + UNITS.iter().position(|unit| unit.regs.load(Ordering::Acquire) == phys) +} + +/// One record a held unit took. +#[cfg(feature = "boot-actuators")] +pub(super) struct Held { + pub unit: usize, + pub fault: Fault, + /// `FSTS.PFO`: faults arrived with every record full, and are lost. + pub overflowed: bool, +} + +/// Every record the held units took since the last call, each cleared. +#[cfg(feature = "boot-actuators")] +pub(super) fn take_held(mut each: impl FnMut(Held)) { + for (index, unit) in UNITS.iter().enumerate() { + let phys = unit.regs.load(Ordering::Acquire); + if phys == 0 { + continue; + } + let regs = window(phys); + let status = regs.read_u32(FSTS_REG); + if status & (FSTS_PENDING | FSTS_OVERFLOW) == 0 { + continue; + } + let (records, count) = (unit.records.load(Ordering::Relaxed), unit.count.load(Ordering::Relaxed)); + for i in 0..count { + let record = records + i as u64 * 16; + if let Some(fault) = read_record(regs, record) { + clear_record(regs, record); + each(Held { unit: index, fault, overflowed: status & FSTS_OVERFLOW != 0 }); + } + } + regs.write_u32(FSTS_REG, status & FSTS_WRITE_ONE_TO_CLEAR); + } +} + // Fault reasons by the condition VT-d 5.20's Table 30 (legacy mode) and // Table 15 (interrupt remapping) give each. // `issues/a-vt-d-fault-is-named-unlike-the-specification-or-not-at-all.md`: diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 8acbe253d5d..f62169572e1 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -8,6 +8,8 @@ //! cannot use leaves the unit switched off rather than halting, logged by //! register value rather than a bare "unsupported". +#[cfg(feature = "boot-actuators")] +mod cycles; pub mod dmar; pub mod domain; pub mod fault; @@ -49,6 +51,9 @@ const IQH_REG: u64 = 0x80; const IQT_REG: u64 = 0x88; const IQA_REG: u64 = 0x90; +/// `RTADDR_REG.TTM`, bits 11:10: `00` is legacy mode, the only one this kernel writes. +const TRANSLATION_TABLE_MODE: u64 = 0b11 << 10; + /// `GCMD` bits; `GSTS` confirms each at the same bit position. const TRANSLATION_ENABLE: u32 = 1 << 31; const SET_ROOT_TABLE_POINTER: u32 = 1 << 30; @@ -155,6 +160,10 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW // meets them; nothing where no declaration is held. let mut reserved: Option> = windows .map(|w| w.iter().map(|w| (w.base, w.end())).chain([super::iommu_unit::INTERRUPT_RANGE]).collect()); + // Cleared by the first region this kernel's root does not translate as + // firmware must have: one outside the identity domain, or one naming a + // device no context entry is written for. + let mut regions_identity = true; for structure in dmar.structures() { match structure { Ok(Structure::Drhd(drhd)) => { @@ -205,6 +214,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW reserved.push((base, limit.saturating_add(1))); } describe_scopes("rmrr", regions, rmrr.scopes()); + regions_identity &= held && rmrr.scopes().all(|scope| enumerated(devices, scope)); regions += 1; held_regions += usize::from(held); } @@ -234,15 +244,50 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW domain::avoid(reserved); let remap = remappable(&ready, units, dmar.flags); + let in_flight = in_flight(dmar.flags, regions_identity); // 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, in_flight); + } + #[cfg(feature = "boot-actuators")] + if crate::actuator::iommu_handover_cycles() { + cycles::run(devices); } true } +/// Whether an RMRR scope names a function this kernel writes a context entry for. +fn enumerated(devices: &[PciDevice], scope: Result) -> bool { + let Ok(Some(stream)) = scope.map(|s| s.stream_id()) else { return false }; + devices.iter().any(|d| StreamId::pci(d.bus, d.dev, d.func) == stream) +} + +/// What a unit firmware left translating may have in flight when this kernel's +/// root replaces firmware's. +#[derive(Clone, Copy)] +enum InFlight { + /// Only what this kernel's root translates as firmware's must: firmware + /// declares `DMA_CTRL_PLATFORM_OPT_IN` (§8.1), so the DMA it leaves running + /// reaches only its RMRRs, which §3.16 has it map identity and read-write, + /// and the identity domain does too for every device they name. §6.6 then + /// allows the switch with translation on. + Identical, + /// Anything, for the reason given. + Unknown(&'static str), +} + +fn in_flight(flags: u8, regions_identity: bool) -> InFlight { + if flags & dmar::FLAG_DMA_CTRL_OPT_IN == 0 { + InFlight::Unknown("firmware does not declare its DMA confined to its reserved regions") + } else if !regions_identity { + InFlight::Unknown("a reserved region is one this kernel's root does not translate as firmware must") + } else { + InFlight::Identical + } +} + /// Whether every interrupt source may be moved to the remappable format, and /// with `EIME` if so; `None` leaves the machine exactly as it boots with no unit. /// @@ -303,27 +348,30 @@ struct Unit { caps: Capabilities, } +/// Turn one `GCMD` field on or off by §11.4.4.1's recipe — every other field +/// as `GSTS` reports it — and wait for `GSTS` to agree. +fn command(index: usize, regs: Mmio, bit: u32, on: bool, what: &str) { + let status = regs.read_u32(GSTS_REG) & PERSISTENT; + regs.write_u32(GCMD_REG, if on { status | bit } else { status & !bit }); + let state = if on { "" } else { " off" }; + wait(index, regs, what, state, || (regs.read_u32(GSTS_REG) & bit != 0) == on); +} + +fn wait(index: usize, regs: Mmio, what: &str, state: &str, done: impl Fn() -> bool) { + let deadline = crate::clock::nanos_since_boot() + COMMAND_TIMEOUT.nanos(); + while !done() { + assert!( + crate::clock::nanos_since_boot() < deadline, + "iommu: unit{index} never reported {what}{state}, GSTS={:#010x}", + regs.read_u32(GSTS_REG) + ); + core::hint::spin_loop(); + } +} + impl Unit { - /// Turn one `GCMD` field on or off by §11.4.4.1's recipe — every other - /// field as `GSTS` reports it — and wait for `GSTS` to agree. fn command(&self, bit: u32, on: bool, what: &str) { - let status = self.regs.read_u32(GSTS_REG) & PERSISTENT; - self.regs.write_u32(GCMD_REG, if on { status | bit } else { status & !bit }); - let state = if on { "" } else { " off" }; - self.wait(what, state, || (self.regs.read_u32(GSTS_REG) & bit != 0) == on); - } - - fn wait(&self, what: &str, state: &str, done: impl Fn() -> bool) { - let deadline = crate::clock::nanos_since_boot() + COMMAND_TIMEOUT.nanos(); - while !done() { - assert!( - crate::clock::nanos_since_boot() < deadline, - "iommu: unit{} never reported {what}{state}, GSTS={:#010x}", - self.index, - self.regs.read_u32(GSTS_REG) - ); - core::hint::spin_loop(); - } + command(self.index, self.regs, bit, on, what); } /// [`crate::actuator::iommu_firmware_left`]: the state [`Self::hand_over`] @@ -357,17 +405,44 @@ impl Unit { } } - /// Every field of [`FIRMWARE_LEFT`] that is on, off. - fn hand_over(&self) { + /// Every field of [`FIRMWARE_LEFT`] that is on, off, except translation + /// through a legacy-mode root while `in_flight` is [`InFlight::Identical`]: + /// [`enable`] then moves the unit onto this kernel's root under translation, + /// as §6.6 allows (`RTADDR_REG.RTA` may change while `TES` is set, §11.4.5). + fn hand_over(&self, in_flight: InFlight) { for (bit, what) in FIRMWARE_LEFT { if self.regs.read_u32(GSTS_REG) & bit == 0 { continue; } + if bit == TRANSLATION_ENABLE { + let legacy = self.regs.read_u64(RTADDR_REG) & TRANSLATION_TABLE_MODE == 0; + match in_flight { + InFlight::Identical if legacy => { + log!( + "iommu: unit{} was handed over with translation on; it stays on while \ + this kernel's root replaces firmware's", + self.index + ); + continue; + } + InFlight::Identical => log!( + "iommu: unit{} was handed over with translation on; it goes off first: its \ + root table is not in legacy mode", + self.index + ), + InFlight::Unknown(why) => log!( + "iommu: unit{} was handed over with translation on; it goes off first: {why}", + self.index + ), + } + self.command(bit, false, what); + continue; + } log!("iommu: unit{} was handed over with {what} on; it goes off first", self.index); if bit == QUEUED_INVALIDATION_ENABLE { // §6.5.2: a queue is switched off only once it is empty and its // last descriptor was a wait, which only firmware could have sent. - self.wait("its invalidation queue", " empty", || { + wait(self.index, self.regs, "its invalidation queue", " empty", || { self.regs.read_u64(IQH_REG) == self.regs.read_u64(IQT_REG) }); } @@ -488,15 +563,16 @@ fn plan(unit: &Unit) -> Option { Some(Plan { width, records }) } -/// Programs the unit in its own order — queue, root pointer, interrupt remap -/// pointer, global invalidation, then `TE` — each confirmed in `GSTS` before -/// the next. +/// Programs the unit in its own order — queue, root pointer and the global +/// invalidation §6.6 owes it, interrupt remap pointer, then `TE` — each +/// confirmed in `GSTS` before the next. fn enable( unit: Unit, plan: Plan, devices: &[PciDevice], domains: &mut [Option
; 2], remap: Option, + in_flight: InFlight, ) { let index = unit.index; let Plan { width, records } = plan; @@ -516,11 +592,11 @@ fn enable( } let root = identity_root(&mut TABLES.lock(), devices, domain, width); - // After the tables are built, so a unit firmware left translating passes DMA - // untranslated only while it is programmed + // After the tables are built, so a unit whose translation goes off passes + // DMA untranslated only while it is programmed // (`issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md`), // and before the queue is pointed, which §6.5.2 does only with it off. - unit.hand_over(); + unit.hand_over(in_flight); 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. @@ -532,12 +608,14 @@ fn enable( unit.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); unit.regs.write_u64(RTADDR_REG, root.phys()); unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); + // At once on a unit still translating: until it completes, its walks may + // mix firmware's cached entries with this root's (§6.6). + queue.invalidate_all(unit.regs); if let Some(irta) = irta { unit.regs.write_u64(interrupt::IRTA_REG, irta); unit.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); queue.invalidate_interrupts(unit.regs); } - queue.invalidate_all(unit.regs); unit.command(TRANSLATION_ENABLE, true, "translation"); if remap.is_some() { diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index 115414e579f..79830c70a34 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -368,3 +368,85 @@ const _: () = { assert!(displaced.domain == KERNEL_DOMAIN && displaced.requester == 0x00F8); assert!(new.0 == 0x5000 | PRESENT && new.1 == (u16::MAX as u64) << 8 | 1); }; + +/// A second root over the same context entries as `root`: every root and +/// context entry copied, every domain shared, so the two translate alike. +#[cfg(feature = "boot-actuators")] +pub fn copy_root(tables: &mut Tables, root: Table) -> Table { + let copy = tables.alloc(); + for bus in 0..TABLE_BYTES / 16 { + let (lo, hi) = root.read_pair(bus); + if lo & PRESENT == 0 { + continue; + } + let context = Table { phys: lo & ADDR_MASK }; + let copied = tables.alloc(); + for devfn in 0..TABLE_BYTES / 16 { + let (lo, hi) = context.read_pair(devfn); + if lo & PRESENT != 0 { + copied.write_pair(devfn, lo, hi); + } + } + copy.write_pair(bus, (lo & !ADDR_MASK) | copied.phys, hi); + } + copy +} + +/// What a legacy-mode walk from `root` reads for `stream` at `at`, every +/// entry fetched from memory rather than this CPU's cache: the root entry, +/// both halves of the context entry, then each second-level entry down to +/// the first that is a leaf or has neither `R` nor `W`. +#[cfg(feature = "boot-actuators")] +pub struct Walk { + entries: [u64; 8], + read: usize, +} + +#[cfg(feature = "boot-actuators")] +pub fn walk(root: Table, stream: StreamId, at: u64) -> Walk { + let mut walk = Walk { entries: [0; 8], read: 0 }; + let mut push = |entry: u64| { + walk.entries[walk.read] = entry; + walk.read += 1; + entry + }; + let fetch = |table: Table, index: usize| { + flush(table.slot(index).addr() as usize); + table.read(index) + }; + let root_entry = push(fetch(root, stream.bus() as usize * 2)); + if root_entry & PRESENT == 0 { + return walk; + } + let context = Table { phys: root_entry & ADDR_MASK }; + let index = stream.devfn() as usize * 2; + let (lo, hi) = (push(fetch(context, index)), push(fetch(context, index + 1))); + if lo & PRESENT == 0 { + return walk; + } + let mut table = Table { phys: lo & ADDR_MASK }; + let mut level = (hi & 0x7) + 2; + while level >= 1 { + let entry = push(fetch(table, ((at >> (12 + 9 * (level - 1))) & 0x1FF) as usize)); + if entry & (SL_READ | SL_WRITE) == 0 || level == 1 || entry & SL_LARGE != 0 { + break; + } + table = Table { phys: entry & ADDR_MASK }; + level -= 1; + } + walk +} + +#[cfg(feature = "boot-actuators")] +impl core::fmt::Display for Walk { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("[")?; + for (i, entry) in self.entries[..self.read].iter().enumerate() { + if i > 0 { + f.write_str(" ")?; + } + write!(f, "{entry:#x}")?; + } + f.write_str("]") + } +} diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 3d05f046a5f..d0789965c55 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -4,6 +4,8 @@ //! over behind the unit and refused where no unit is. //! `amdvi_firmware_left`: whether an AMD-Vi unit firmware left blocking is //! switched off before the boot stick is driven. +//! `vtd_firmware_left`: whether a VT-d unit firmware left translating under +//! a DMAR that declares nothing in flight goes off before it is programmed. use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; @@ -345,6 +347,35 @@ pub(crate) fn amdvi_left_on(unit: &str) -> [String; 3] { .map(|field| format!("iommu: amdvi {unit} was handed over with {field} on; it goes off")) } +/// **A VT-d unit left translating where firmware declares nothing about its +/// DMA goes off first, and is programmed.** QEMU's DMAR leaves +/// `DMA_CTRL_PLATFORM_OPT_IN` clear, so this is the hand-over the T14 no +/// longer takes: its firmware declares the flag and keeps translation on +/// (`iommu_firmware_left`'s row). Under `iommu-firmware-left` the unit is +/// left queueing, translating and remapping; each goes off by its own write, +/// translation with the reason, and the unit ends translating through this +/// kernel's root with the boot stick's DMA behind it. +pub fn vtd_firmware_left(test_config: &Path) -> Result<(), String> { + let options = BootOptions { profile: Profile::Metal, kernel_params: &["iommu-firmware-left"], ..Default::default() }; + let qemu = QemuInstance::boot_with_options(test_config, &[], &[], options); + let log = Serial::boot(&qemu); + log.must_be_clean()?; + log.must_say("dma_ctrl_opt_in=n")?; + let off = log.must_say( + "iommu: unit0 was handed over with translation on; it goes off first: firmware does not declare \ + its DMA confined to its reserved regions", + )?; + log.must_say("iommu: unit0 was handed over with queued invalidation on; it goes off first")?; + log.must_not_say("it stays on while")?; + let translating = log.must_say(" translating gsts=")?; + if !translating.contains(" tes=y ") { + return Err(format!("unit0 was programmed and is not translating: {translating:?}")); + } + log.must_say("Boot: complete")?; + eprintln!(" [vtd-firmware-left] {}; {}", off.trim(), translating.trim()); + Ok(()) +} + /// **A unit an AMD machine's firmware left on is switched off and /// programmed.** QEMU's `amd-iommu` is the only AMD-Vi unit the nightly guest /// suite runs: the T14 is an Intel machine, and the hand-over's register diff --git a/tests/metal/src/lib.rs b/tests/metal/src/lib.rs index be53c64a004..d371ebe7535 100644 --- a/tests/metal/src/lib.rs +++ b/tests/metal/src/lib.rs @@ -910,6 +910,12 @@ 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 VT-d unit moved between its root and a copy of it hundreds of + // times, with translation off between some, while its fault events are + // held and their records counted. It writes only the units' registers, + // which a reset returns to firmware, and tables in memory this boot owns; + // what a fault refuses is a read or write a device made, never a register. + "iommu-handover-cycles", // Every CPU is handed the loader's Intel update with its header's revision // raised by one and its checksum sealed again. Its payload is Intel's own, // the revision the CPU already runs, so the CPU refuses it or runs what it diff --git a/tests/toyos.rs b/tests/toyos.rs index 56c71fdc049..1715b2b3883 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -482,6 +482,9 @@ const MACHINE_TESTS: &[&str] = &[ // DMA it lets through are no host test's, and the AMD bench is staged by // hand rather than by CI. "amdvi_firmware_left", + // A VT-d unit left translating where the DMAR declares nothing in flight: + // the T14's declares it, so only QEMU's unit takes this hand-over. + "vtd_firmware_left", // A translating AMD-Vi unit's refusal, recorded and read: a kernel-owned // record halts the machine, so no metal row reaches it, and only QEMU's // `iommu-testdev` aims a DMA where no domain maps. @@ -1302,6 +1305,14 @@ const METAL: &[(&str, metal::Metal)] = &[ "iommu_firmware_left", metal::Metal { arms: SELFTESTS, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, ), + ( + // Each VT-d unit taken between two roots that translate alike, by + // translation going off and by a switch under it, with whatever DMA + // firmware left running going on through it: the count of faults each + // way. A measurement, so a named pass alone runs it. + "iommu_handover_cycles", + metal::Metal { arms: HANDOVER_CYCLES, judge: |b| iommu_handover_cycles(b[0].kernel().text()) }, + ), // ---- a claimed function at the unit, on tests/images/testcases.toml ---- ( // Claimed, given back and claimed again: both claims name one @@ -1806,6 +1817,19 @@ const SELFTEST_PARAMS: &[&str] = &[ "iommu-firmware-left", ]; +/// Its own boot: it holds every unit's fault events for its eight seconds, +/// so a fault the shared boot's tests stage would be counted here instead. +const HANDOVER_CYCLES: &[metal::Arm] = &[metal::Arm { + features: ACTUATOR_KERNEL, + named_only: true, + ..metal::once( + "iommu-cycles", + "tests/images/testcases.toml", + &["iommu-firmware-left", "iommu-handover-cycles"], + &[], + ) +}]; + const SELFTESTS: &[metal::Arm] = &[metal::Arm { features: ACTUATOR_KERNEL, ..metal::once("shared-debug", "tests/images/testcases.toml", SELFTEST_PARAMS, &[]) @@ -4425,6 +4449,7 @@ fn run_machine_test(name: &str, test_config: &Path) -> Result<(), String> { common::iommu::the_kernel_tells_a_bypassing_console_physical_addresses() } "amdvi_firmware_left" => common::iommu::amdvi_firmware_left(test_config), + "vtd_firmware_left" => common::iommu::vtd_firmware_left(test_config), "amdvi_selftest" => common::iommu::amdvi_selftest(test_config), "nested_nmi_is_loud" => faults::nested_nmi_is_loud(test_config), "kernel_wx_leaf_is_refused" => faults::kernel_wx_leaf_is_refused(test_config, qemu::Profile::Metal), @@ -5977,11 +6002,15 @@ fn sysret_ss(log: &str) -> Result<(), String> { } /// Every unit this kernel went on to program, and every AMD-Vi unit it -/// switched off, said it was handed over with what the actuator left on. +/// switched off, said it was handed over with what the actuator left on; and +/// where firmware declares its DMA confined to reserved regions this kernel's +/// root translates alike, as the T14's does, every VT-d unit kept translating. /// /// An AMD-Vi unit's lines are on the boot stick only because the switch-off /// let the stick's DMA through the unit the actuator left blocking it. fn iommu_firmware_left(log: &str) -> Result<(), String> { + let declared = log.contains(" dma_ctrl_opt_in=y") + && !log.lines().any(|l| l.contains("iommu: rmrr") && (l.contains(" OUTSIDE the identity domain") || l.contains("bridge walk"))); let mut units = 0; for line in log.lines().filter(|l| l.contains(" translating gsts=")) { let unit = line @@ -6006,6 +6035,12 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { )); } } + if declared && !log.contains(&format!("iommu: {unit} was handed over with translation on; it stays on")) { + return Err(format!( + "firmware declares its DMA confined to reserved regions this kernel's root \ + translates alike, and {unit} still took translation off:\n{log}" + )); + } units += 1; } let amdvi = log @@ -6031,13 +6066,29 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { return Err(format!("no unit was programmed or switched off, so nothing was handed over:\n{log}")); } let passed = log.matches("handed over with compatibility-format pass-through on").count(); + let kept = log.matches("handed over with translation on; it stays on").count(); eprintln!( - " [iommu-firmware-left] {units} unit(s) switched off what they were handed over with; \ - {passed} of them reported compatibility format passed" + " [iommu-firmware-left] {units} unit(s) switched off what they were handed over with, \ + {kept} of them keeping translation on; {passed} of them reported compatibility format passed" ); Ok(()) } +/// Both kinds of cycle ran to their count; the fault lines and the counts are +/// the measurement, printed rather than judged. +fn iommu_handover_cycles(log: &str) -> Result<(), String> { + let lines: Vec<&str> = log.lines().filter(|l| l.contains("iommu-handover-cycles: ")).collect(); + for kind in ["off-on", "switch"] { + if !lines.iter().any(|l| l.contains(&format!("iommu-handover-cycles: {kind} ")) && l.contains(" with a fault")) { + return Err(format!("the {kind} cycles never reported their count:\n{log}")); + } + } + for line in lines { + eprintln!(" [iommu-handover-cycles] {}", line.split("iommu-handover-cycles: ").nth(1).unwrap_or(line)); + } + Ok(()) +} + /// The machine's kernel thread is hosted. fn klogd_hosted(boot: &serial::Serial) -> Result<(), String> { boot.must_be_clean()?; From 6dbf3fb92130360a1f0ad9428f477d8b9f2571f0 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 28 Oct 2026 10:54:30 +0100 Subject: [PATCH 2/4] vtd: review r1, the hand-over's premise fails closed and says what it infers; the reproducer leaves the PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The declared branch now holds only for a DMAR read to its end, and only where every RMRR is whole, has base <= limit, is on segment 0 and names endpoints alone. A walk stopped by a malformed structure or the unit ceiling leaves regions unread. A short RMRR reads its fields as zero. A bridge scope's requesters are the devices below it. Each of these now clears it. The comments and records stop presenting the premise as specification. §8.1 reports platform DMA confined to RMRRs. §3.16 expects identity mappings of system software's tables, not firmware's, so "firmware's tables translate alike" is labelled an inference. The root-pointer permission is §6.6's, and TTM's guard is §6.6 and §11.4.5. The DMA_CTRL_PLATFORM_OPT_IN constant said something false of the flag. The iommu-handover-cycles measurement moves to a branch of its own and does not land. Its next run takes every switch first and every off-on after, so off-on cannot leave the unit faulting into the switch phase. The issue's exit covers all three outcomes, calls zero faults both ways inconclusive, and stays open while a path known to fault takes translation off. The narrowed hand-over issue drops its Linux citation and records the pmm exposure both routes share. The IOMMU track records the containment proposal. Co-Authored-By: Claude Opus 5.5 --- ...asses-dma-untranslated-while-programmed.md | 29 +-- issues/the-iommu-refuses-nothing-yet.md | 15 ++ ...r-after-a-handover-took-translation-off.md | 67 ++++--- kernel/src/actuator.rs | 8 - kernel/src/arch/x86_64/vtd/cycles.rs | 184 ------------------ kernel/src/arch/x86_64/vtd/dmar.rs | 9 +- kernel/src/arch/x86_64/vtd/fault.rs | 70 ------- kernel/src/arch/x86_64/vtd/mod.rs | 100 +++++----- kernel/src/arch/x86_64/vtd/table.rs | 82 -------- tests/metal/src/lib.rs | 6 - tests/toyos.rs | 36 ---- 11 files changed, 134 insertions(+), 472 deletions(-) delete mode 100644 kernel/src/arch/x86_64/vtd/cycles.rs diff --git a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md b/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md index ebce09af18b..9e17ce4106d 100644 --- a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md +++ b/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md @@ -13,17 +13,24 @@ back on through this kernel's root table only after the queue, fault reporting, both table pointers and a global invalidation are programmed. In between, every function behind that unit reaches all of memory. -Keeping `TE` on is what VT-d Rev. 4.1 §6.6 allows only onto tables that remap -in-flight requests exactly as the ones being walked. Where firmware sets -`DMA_CTRL_PLATFORM_OPT_IN` (§8.1), its DMA reaches only its RMRRs, which -§3.16 has mapped identity and read-write. If the identity domain holds every -RMRR and every device they name is enumerated, this kernel's root translates -them alike, and the hand-over keeps `TE` on. The T14 is such a machine. -Elsewhere, firmware's tables have to be read and copied first, as Linux's -kdump path does (`copy_translation_tables`, `drivers/iommu/intel/iommu.c`); -Linux outside kdump switches `TE` off as this kernel does. Any copy has to -read them before the pmm hands their memory out: it takes -`EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, `is_usable_type`). +Keeping `TE` on is what VT-d Rev. 4.1 §6.6 allows only onto structures that +give identical remapping results. Where firmware reports +`DMA_CTRL_PLATFORM_OPT_IN` (§8.1), the DMA it leaves running reaches only its +RMRRs. This kernel infers that firmware's tables map those identity and +read-write, the mapping §3.16 expects of system software's tables but does +not guarantee of firmware's. On that inference, where the identity domain +holds every RMRR and every device they name is enumerated, the hand-over +keeps `TE` on. The T14 reports the flag, but its firmware leaves translation +off, so only `iommu-firmware-left` takes that path there. Elsewhere, +firmware's tables have to be read and copied before the switch. + +Both routes share one exposure. `mm::init` runs before `iommu::init` and +treats `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, +`is_usable_type`). From then until the switch, or until translation goes off, +a unit left translating walks firmware's tables in memory the pmm may already +have handed out. The same exposure weakens the inference above and any copy +of firmware's tables, which would have to be read before the pmm hands that +memory out. Evidence: no machine here is handed a unit translating; the T14's four units read `gsts=0x40000000` (pull request #700's selftests boot). QEMU's DMAR leaves diff --git a/issues/the-iommu-refuses-nothing-yet.md b/issues/the-iommu-refuses-nothing-yet.md index 131c9a09b23..e7eaa0fb61b 100644 --- a/issues/the-iommu-refuses-nothing-yet.md +++ b/issues/the-iommu-refuses-nothing-yet.md @@ -67,6 +67,21 @@ a unit firmware left translating: boot, he answered: "You can do with the t14 what you want." The grant goes beyond the question asked. +**Proposed, for the owner** (pull request #1086's review): a fault from an +enumerated function that no driver runs halts the machine only when the +address is one its domain maps. A mapped address means the unit contradicted +tables this kernel wrote, which the shared identity domain does not confine +to one unit. An unmapped address is firmware's DMA straying outside its +grant: the function is stopped, the fault logged, and the machine goes on, as +"Apply it at hand-over" already rules for reserved regions. An unclaimed +function sits in the identity domain, which maps all of `[0, top)` with +nothing carved out, so "unmapped" is `address >= top` and the handler stays +bounded and lock-free. The change is to the representation: `Owner::Kernel` +today merges functions this kernel drives with functions nobody drives, and +the split gives the second its own arm. One caveat: the iGPU is not wholly +undriven. Its scanout reads the firmware framebuffer the kernel's panic +console draws into, so stopping it blanks that console. + Owner: the orchestrator. **Exit**: a test or a T14 row reads each ruled refusal, each with its diff --git a/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md b/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md index 3caee299824..767fd3669aa 100644 --- a/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md +++ b/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md @@ -17,44 +17,53 @@ iommu: DMA FAULT owner=kernel unit0 stream=00:02.0 addr=0x000000009cea7000 acces What the boot establishes: - `00:02.0` is the iGPU, alone in unit0's scope. No driver runs it, so the - read is the scanout firmware left running. `rmrr0` reserves - `0x9c000000..0xa07fffff` for it, and the root bridges' memory starts at - `0xa0800000`, so the region ends at `TOLUD`. That is where the 11th - Generation Intel Core datasheet (631122 §2.5) puts GTT and data stolen - memory. The address lies inside the region. + read is the scanout firmware left running. +- `rmrr0` reserves `0x9c000000..0xa07fffff` for it. The root bridges' memory + begins at `0xa0800000`, where the 11th Generation Intel Core datasheet + (631122 §2.5) puts the top of graphics stolen memory. The address lies + inside the region. - Reason 6h is a read that met a second-stage entry with `R` clear (VT-d 5.0 Table 30, `LGN.3`). The identity domain maps all of `0x0..0x45f800000` read and write, so unit0 did not translate by the tables it had been given. - Fault reporting is armed after `hand_over` and clears every older record, and `IF` is set throughout `vtd::init`. So the fault was taken after unit0 - was back on through this kernel's root, after units 1 to 3 were programmed. + was back on through this kernel's root. - unit0 has one recording register (`nfr=1`), and the overflow shows at least - one more fault after that. + one more fault after it. -On the T14 under `iommu-firmware-left`, unit0's translation went on, off and +Under `iommu-firmware-left` on the T14, unit0's translation went on, off and on again while the scanout read through it. Every other T14 boot only turns it on. The orchestrator's logs to 2026-10-28 hold 787 boots with one -transition and no iGPU fault, and 46 cycled boots with this one. That -supports translation going off under live DMA as the trigger, at p ≈ 0.055, -but does not establish it. - -**The change made**: where firmware sets `DMA_CTRL_PLATFORM_OPT_IN` and this -kernel's root translates every RMRR as firmware must, the hand-over leaves -translation on. The T14's root is then switched under translation (§6.6) -rather than through off and on. Under `iommu-firmware-left` the T14 now makes -the same single transition as every other boot. - -**What is not yet known**: whether the off-and-on cycle is what faults. The -`iommu-handover-cycles` actuator repeats it and the switch under translation -hundreds of times each, with fault events held, and prints each fault with -the cycle that preceded it and the walk the tables give from memory. Its -row, `iommu_handover_cycles`, runs only by name. +transition and no iGPU fault, and 46 cycled boots with this one (p ≈ 0.055). +That suggests translation going off under live DMA. It does not establish it. + +Since pull request #1086, a unit whose firmware reports +`DMA_CTRL_PLATFORM_OPT_IN`, whose RMRRs the identity domain holds, and whose +root is in legacy mode keeps translation on. The hand-over switches its root +under translation (§6.6). Under the actuator the T14 now makes a single +transition. On a machine whose firmware reports nothing, translation still +goes off first. + +**Measurement**: the `iommu-handover-cycles` actuator, on a measurement +branch and never landed, moves every unit between its root and an identical +copy. It runs 200 switches under translation first, so no cycle in that phase +follows translation going off, then 200 cycles of translation off and on. +Fault events are held throughout, and each record is counted against the +cycle it followed. Its `off-on` omits the queue and interrupt-remapping +toggles of the original hand-over, so an `off-on` phase without faults does +not clear that sequence. Results go to the pull request's evidence. Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). -**Exit**: `iommu_handover_cycles` on the T14 places the fault. If the -`switch` cycles fault, this change does not cure it, and the defect is -re-scoped to what does. If only the `off-on` cycles fault, the cause is -recorded at `Unit::hand_over`, where translation still goes off for a unit -whose in-flight DMA is undeclared. Close once `iommu_firmware_left` passes on -the T14 with unit0 keeping translation on. +**Exit**, after the measurement boots: + +- If the `switch` phase faults, keeping translation on does not cure this. + The defect is re-scoped to what does. +- If only the `off-on` phase faults, turning translation off under live DMA + faults on this unit. This file stays open, for the path that still does + that on machines whose firmware reports nothing, until that path no longer + takes translation off. +- If neither phase faults, the measurement is inconclusive. The cause stays + unknown and this file stays open. + +It closes when no remaining hand-over path is known to fault. diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 819044cbafa..c4c79a0b14c 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -109,14 +109,6 @@ actuators! { /// `iommu_firmware_left` and `amdvi_firmware_left`. iommu_firmware_left = "iommu-firmware-left"; - /// Once every VT-d unit is programmed, move each between its root and a - /// copy of it hundreds of times, alternately with translation going off - /// and staying on, and count the fault records each way. Judged by - /// `iommu_handover_cycles`. - // A VT-d unit's alone, so AArch64 builds an accessor it never reads. - #[allow(dead_code)] - iommu_handover_cycles = "iommu-handover-cycles"; - /// QEMU's two `iommu-testdev` asked three things of the AMD-Vi unit: /// one on a domain of its own writes where it maps, the other on the /// identity domain writes to memory, then where that domain maps diff --git a/kernel/src/arch/x86_64/vtd/cycles.rs b/kernel/src/arch/x86_64/vtd/cycles.rs deleted file mode 100644 index e8da899157b..00000000000 --- a/kernel/src/arch/x86_64/vtd/cycles.rs +++ /dev/null @@ -1,184 +0,0 @@ -//! [`crate::actuator::iommu_handover_cycles`]: every translating unit moved -//! between its root and a copy of it, alternately by the two ways a hand-over -//! can take, while whatever DMA firmware left running goes on through it. -//! -//! `off-on` is what a hand-over did when translation had to go off: `TE` off, -//! the other root pointed, the global invalidation, `TE` on. `switch` is §6.6's -//! root switch under translation: the other root pointed and the global -//! invalidation, `TE` left on. The two roots translate alike, so a fault -//! either way is the unit's disagreement with tables it was given, and each is -//! printed with the walk those tables give from memory. -//! -//! Fault events are held for the whole run and the records read here, so a -//! fault is counted against the cycle it followed and the machine goes on. - -use alloc::vec::Vec; - -use crate::drivers::pci::PciDevice; -use crate::iommu::fault::Who; -use crate::log; - -use super::fault::{self, Held}; -use super::table::{self, Table}; -use super::{command, RTADDR_REG, SET_ROOT_TABLE_POINTER, TABLES, TRANSLATION_ENABLE, UNITS}; - -/// Cycles of each kind. They alternate, `off-on` onto the copy and `switch` -/// back, so the last lands every unit on its own root again. -const CYCLES: usize = 200; - -/// How long each cycle's result is watched: at least one refresh of a panel -/// scanning out at 50 Hz or faster, so every address a scanout reads is read -/// once after each transition. -const WATCH_NS: u64 = 20_000_000; - -/// Fault lines printed in full; every later one is only counted. -const LINES: usize = 16; - -#[derive(Clone, Copy, PartialEq)] -enum Kind { - OffOn, - Switch, -} - -impl Kind { - const fn name(self) -> &'static str { - match self { - Self::OffOn => "off-on", - Self::Switch => "switch", - } - } -} - -#[derive(Default)] -struct Tally { - /// Cycles after which at least one record was read. - faulted: usize, - records: usize, - overflowed: usize, - /// The fewest nanoseconds from a transition's end to a record being read. - soonest: Option, -} - -/// One live unit: the index its fault records are armed under, its own root -/// and the copy. -struct Roots { - index: usize, - own: Table, - copy: Table, -} - -pub(super) fn run(devices: &[PciDevice]) { - host_bridge(devices); - let roots: Vec = { - let units = UNITS.lock(); - let mut tables = TABLES.lock(); - units - .iter() - .map(|unit| Roots { - index: fault::index_of(unit.regs).expect("iommu-handover-cycles: a live unit with no fault records"), - own: unit.root, - copy: table::copy_root(&mut tables, unit.root), - }) - .collect() - }; - log!("iommu-handover-cycles: {} unit(s), {CYCLES} cycles of each kind, each watched {WATCH_NS} ns", roots.len()); - - fault::hold(true); - let mut tallies = [Tally::default(), Tally::default()]; - let mut printed = 0usize; - for cycle in 0..2 * CYCLES { - let kind = if cycle % 2 == 0 { Kind::OffOn } else { Kind::Switch }; - let onto = |roots: &Roots| if kind == Kind::OffOn { roots.copy } else { roots.own }; - { - let mut units = UNITS.lock(); - for (unit, roots) in units.iter_mut().zip(&roots) { - if kind == Kind::OffOn { - command(roots.index, unit.regs, TRANSLATION_ENABLE, false, "translation"); - } - unit.regs.write_u64(RTADDR_REG, onto(roots).phys()); - command(roots.index, unit.regs, SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - unit.queue.invalidate_all(unit.regs); - if kind == Kind::OffOn { - command(roots.index, unit.regs, TRANSLATION_ENABLE, true, "translation"); - } - } - } - let tally = &mut tallies[kind as usize]; - let start = crate::clock::nanos_since_boot(); - let mut seen = false; - while crate::clock::nanos_since_boot() - start < WATCH_NS { - fault::take_held(|Held { unit, fault, overflowed }| { - let after = crate::clock::nanos_since_boot() - start; - seen = true; - tally.records += 1; - tally.overflowed += usize::from(overflowed); - tally.soonest = Some(tally.soonest.map_or(after, |s| s.min(after))); - let Who::Function(stream) = fault.who else { return }; - if printed == LINES { - return; - } - printed += 1; - let walk = roots.iter().find(|r| r.index == unit).map(|r| table::walk(onto(r), stream, fault.address)); - let walk: &dyn core::fmt::Display = match &walk { - Some(walk) => walk, - None => &"none", - }; - log!( - "iommu-handover-cycles: cycle {cycle} {} unit{unit} stream={stream} addr={:#018x} access={} \ - reason={:#04x} {} overflowed={} after={after}ns walk={walk}", - kind.name(), - fault.address, - fault.access, - fault.reason, - fault.name, - if overflowed { 'y' } else { 'n' }, - ); - }); - core::hint::spin_loop(); - } - tally.faulted += usize::from(seen); - } - fault::hold(false); - - for kind in [Kind::OffOn, Kind::Switch] { - let tally = &tallies[kind as usize]; - log!( - "iommu-handover-cycles: {} {CYCLES} cycles, {} with a fault, {} records, {} overflowed, soonest {}", - kind.name(), - tally.faulted, - tally.records, - tally.overflowed, - Soonest(tally.soonest), - ); - } -} - -struct Soonest(Option); - -impl core::fmt::Display for Soonest { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self.0 { - Some(ns) => write!(f, "{ns}ns"), - None => f.write_str("none"), - } - } -} - -/// Where an Intel host bridge put graphics stolen memory, so a record's address -/// can be placed in it: `BDSM` (data) at B0h and `BGSM` (GTT) at B4h, bits -/// 31:20 each, and `TOLUD` at BCh (11th Generation Intel Core Datasheet Vol. -/// 2a, 631122, §3.1.31-§3.1.32). The values as read; what they mean is -/// Intel's host bridge alone. -fn host_bridge(devices: &[PciDevice]) { - let Some(bridge) = devices.iter().find(|d| (d.bus, d.dev, d.func) == (0, 0, 0) && d.vendor_id() == 0x8086) else { - return; - }; - log!( - "iommu-handover-cycles: host bridge {:04x}:{:04x} BDSM={:#010x} BGSM={:#010x} TOLUD={:#010x}", - bridge.vendor_id(), - bridge.device_id(), - bridge.read_config_u32(0xB0), - bridge.read_config_u32(0xB4), - bridge.read_config_u32(0xBC), - ); -} diff --git a/kernel/src/arch/x86_64/vtd/dmar.rs b/kernel/src/arch/x86_64/vtd/dmar.rs index 38ef40e19fd..cf5a202005d 100644 --- a/kernel/src/arch/x86_64/vtd/dmar.rs +++ b/kernel/src/arch/x86_64/vtd/dmar.rs @@ -22,7 +22,8 @@ const FLAGS: usize = 37; pub const FLAG_INTR_REMAP: u8 = 1 << 0; /// Flags bit 1: firmware asks the OS not to enable x2APIC mode. pub const FLAG_X2APIC_OPT_OUT: u8 = 1 << 1; -/// Flags bit 2: DMA blocked until the OS programs the unit; honoured for free by this kernel's ordering. +/// Flags bit 2, `DMA_CTRL_PLATFORM_OPT_IN_FLAG` (§8.1): firmware reports that +/// any DMA the platform initiates at hand-over is restricted to its RMRRs. pub const FLAG_DMA_CTRL_OPT_IN: u8 = 1 << 2; /// A list element whose declared size the list cannot hold; the last item a walk yields. @@ -185,6 +186,12 @@ impl Rmrr { Scopes { table: self.table, offset: self.at + RMRR_SCOPES, end: self.at + self.len } } + /// Whether the declared length covers every fixed field, so none of them + /// read as the zero `bounded` gives a field past it. + pub fn whole(&self) -> bool { + self.len >= RMRR_SCOPES + } + fn field(&self, offset: usize) -> T { bounded(self.table, self.at, self.len, offset) } diff --git a/kernel/src/arch/x86_64/vtd/fault.rs b/kernel/src/arch/x86_64/vtd/fault.rs index acf9fe873fb..00584db8c3e 100644 --- a/kernel/src/arch/x86_64/vtd/fault.rs +++ b/kernel/src/arch/x86_64/vtd/fault.rs @@ -166,76 +166,6 @@ fn clear_record(regs: Mmio, record: u64) { regs.write_u32(record + 12, RECORD_FAULT); } -/// `FECTL.IM`: the unit records faults but raises no event for them. -#[cfg(feature = "boot-actuators")] -const FECTL_MASK: u32 = 1 << 31; - -/// Every armed unit's fault event masked, or unmasked once every record and -/// status bit is cleared: [`crate::actuator::iommu_handover_cycles`] reads the -/// records itself while it holds them, rather than [`service`] ending the -/// machine on the first. -#[cfg(feature = "boot-actuators")] -pub(super) fn hold(held: bool) { - for unit in &UNITS { - let phys = unit.regs.load(Ordering::Acquire); - if phys == 0 { - continue; - } - let regs = window(phys); - if held { - regs.write_u32(FECTL_REG, FECTL_MASK); - } else { - let (records, count) = (unit.records.load(Ordering::Relaxed), unit.count.load(Ordering::Relaxed)); - for i in 0..count { - clear_record(regs, records + i as u64 * 16); - } - regs.write_u32(FSTS_REG, regs.read_u32(FSTS_REG) & FSTS_WRITE_ONE_TO_CLEAR); - regs.write_u32(FECTL_REG, 0); - } - } -} - -/// The index the unit whose registers are `regs` was armed under. -#[cfg(feature = "boot-actuators")] -pub(super) fn index_of(regs: Mmio) -> Option { - let phys = DirectMap::phys_of(regs.addr() as *const u8); - UNITS.iter().position(|unit| unit.regs.load(Ordering::Acquire) == phys) -} - -/// One record a held unit took. -#[cfg(feature = "boot-actuators")] -pub(super) struct Held { - pub unit: usize, - pub fault: Fault, - /// `FSTS.PFO`: faults arrived with every record full, and are lost. - pub overflowed: bool, -} - -/// Every record the held units took since the last call, each cleared. -#[cfg(feature = "boot-actuators")] -pub(super) fn take_held(mut each: impl FnMut(Held)) { - for (index, unit) in UNITS.iter().enumerate() { - let phys = unit.regs.load(Ordering::Acquire); - if phys == 0 { - continue; - } - let regs = window(phys); - let status = regs.read_u32(FSTS_REG); - if status & (FSTS_PENDING | FSTS_OVERFLOW) == 0 { - continue; - } - let (records, count) = (unit.records.load(Ordering::Relaxed), unit.count.load(Ordering::Relaxed)); - for i in 0..count { - let record = records + i as u64 * 16; - if let Some(fault) = read_record(regs, record) { - clear_record(regs, record); - each(Held { unit: index, fault, overflowed: status & FSTS_OVERFLOW != 0 }); - } - } - regs.write_u32(FSTS_REG, status & FSTS_WRITE_ONE_TO_CLEAR); - } -} - // Fault reasons by the condition VT-d 5.20's Table 30 (legacy mode) and // Table 15 (interrupt remapping) give each. // `issues/a-vt-d-fault-is-named-unlike-the-specification-or-not-at-all.md`: diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index f62169572e1..f9e02be81ea 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -8,8 +8,6 @@ //! cannot use leaves the unit switched off rather than halting, logged by //! register value rather than a bare "unsupported". -#[cfg(feature = "boot-actuators")] -mod cycles; pub mod dmar; pub mod domain; pub mod fault; @@ -51,7 +49,11 @@ const IQH_REG: u64 = 0x80; const IQT_REG: u64 = 0x88; const IQA_REG: u64 = 0x90; -/// `RTADDR_REG.TTM`, bits 11:10: `00` is legacy mode, the only one this kernel writes. +/// `RTADDR_REG.TTM`, bits 11:10, `00` for legacy mode: this kernel's root is +/// legacy, and with `CAP.ESRTPS` clear `TTM` may not change while `TES` is set +/// (§6.6, §11.4.5). +// No machine here reports `ESRTPS`, so a unit in another mode goes off +// whatever it reports (`issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md`). const TRANSLATION_TABLE_MODE: u64 = 0b11 << 10; /// `GCMD` bits; `GSTS` confirms each at the same bit position. @@ -160,9 +162,9 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW // meets them; nothing where no declaration is held. let mut reserved: Option> = windows .map(|w| w.iter().map(|w| (w.base, w.end())).chain([super::iommu_unit::INTERRUPT_RANGE]).collect()); - // Cleared by the first region this kernel's root does not translate as - // firmware must have: one outside the identity domain, or one naming a - // device no context entry is written for. + // Whether every region was read whole and is one this kernel's root maps + // for every device it names; cleared too by a walk that stops before the + // table's end, since the regions past it are unread. let mut regions_identity = true; for structure in dmar.structures() { match structure { @@ -172,6 +174,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW units += 1; if units > MAX_UNITS { log!("iommu: more than {MAX_UNITS} units described; the rest are not inventoried"); + regions_identity = false; break; } if let Some(unit) = describe_unit(units - 1, &drhd) { @@ -214,7 +217,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW reserved.push((base, limit.saturating_add(1))); } describe_scopes("rmrr", regions, rmrr.scopes()); - regions_identity &= held && rmrr.scopes().all(|scope| enumerated(devices, scope)); + regions_identity &= held && identity_mapped(devices, &rmrr); regions += 1; held_regions += usize::from(held); } @@ -230,6 +233,7 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW "iommu: DMAR structure at +{at} declares {declared} bytes the table cannot \ hold — stopping" ); + regions_identity = false; } } } @@ -251,28 +255,34 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW for (unit, plan) in ready { enable(unit, plan, devices, &mut domains, remap, in_flight); } - #[cfg(feature = "boot-actuators")] - if crate::actuator::iommu_handover_cycles() { - cycles::run(devices); - } true } -/// Whether an RMRR scope names a function this kernel writes a context entry for. -fn enumerated(devices: &[PciDevice], scope: Result) -> bool { - let Ok(Some(stream)) = scope.map(|s| s.stream_id()) else { return false }; - devices.iter().any(|d| StreamId::pci(d.bus, d.dev, d.func) == stream) +/// Whether `rmrr` is whole, `base <= limit`, on the one segment this kernel +/// enumerates, and names only endpoints it writes a context entry for. A +/// bridge scope's requesters are the devices below it, which it does not name. +fn identity_mapped(devices: &[PciDevice], rmrr: &dmar::Rmrr) -> bool { + rmrr.whole() + && rmrr.base() <= rmrr.limit() + && rmrr.segment() == toyos_acpi::SEGMENT_GROUP + && rmrr.scopes().all(|scope| { + let Ok(scope) = scope else { return false }; + let Some(stream) = scope.stream_id().filter(|_| scope.kind() == SCOPE_ENDPOINT) else { return false }; + devices.iter().any(|d| StreamId::pci(d.bus, d.dev, d.func) == stream) + }) } /// What a unit firmware left translating may have in flight when this kernel's /// root replaces firmware's. #[derive(Clone, Copy)] enum InFlight { - /// Only what this kernel's root translates as firmware's must: firmware - /// declares `DMA_CTRL_PLATFORM_OPT_IN` (§8.1), so the DMA it leaves running - /// reaches only its RMRRs, which §3.16 has it map identity and read-write, - /// and the identity domain does too for every device they name. §6.6 then - /// allows the switch with translation on. + /// Only what this kernel's root is taken to translate as firmware's does: + /// firmware reports `DMA_CTRL_PLATFORM_OPT_IN` (§8.1), so the DMA it leaves + /// running reaches only its RMRRs, and the identity domain maps each of + /// them, identity and read-write, for every device it names. That + /// firmware's own tables map them the same way is an inference, not a + /// guarantee: §3.16 sets that expectation for system software's tables. + /// §6.6 allows the switch under translation only onto identical results. Identical, /// Anything, for the reason given. Unknown(&'static str), @@ -348,30 +358,27 @@ struct Unit { caps: Capabilities, } -/// Turn one `GCMD` field on or off by §11.4.4.1's recipe — every other field -/// as `GSTS` reports it — and wait for `GSTS` to agree. -fn command(index: usize, regs: Mmio, bit: u32, on: bool, what: &str) { - let status = regs.read_u32(GSTS_REG) & PERSISTENT; - regs.write_u32(GCMD_REG, if on { status | bit } else { status & !bit }); - let state = if on { "" } else { " off" }; - wait(index, regs, what, state, || (regs.read_u32(GSTS_REG) & bit != 0) == on); -} - -fn wait(index: usize, regs: Mmio, what: &str, state: &str, done: impl Fn() -> bool) { - let deadline = crate::clock::nanos_since_boot() + COMMAND_TIMEOUT.nanos(); - while !done() { - assert!( - crate::clock::nanos_since_boot() < deadline, - "iommu: unit{index} never reported {what}{state}, GSTS={:#010x}", - regs.read_u32(GSTS_REG) - ); - core::hint::spin_loop(); - } -} - impl Unit { + /// Turn one `GCMD` field on or off by §11.4.4.1's recipe — every other + /// field as `GSTS` reports it — and wait for `GSTS` to agree. fn command(&self, bit: u32, on: bool, what: &str) { - command(self.index, self.regs, bit, on, what); + let status = self.regs.read_u32(GSTS_REG) & PERSISTENT; + self.regs.write_u32(GCMD_REG, if on { status | bit } else { status & !bit }); + let state = if on { "" } else { " off" }; + self.wait(what, state, || (self.regs.read_u32(GSTS_REG) & bit != 0) == on); + } + + fn wait(&self, what: &str, state: &str, done: impl Fn() -> bool) { + let deadline = crate::clock::nanos_since_boot() + COMMAND_TIMEOUT.nanos(); + while !done() { + assert!( + crate::clock::nanos_since_boot() < deadline, + "iommu: unit{} never reported {what}{state}, GSTS={:#010x}", + self.index, + self.regs.read_u32(GSTS_REG) + ); + core::hint::spin_loop(); + } } /// [`crate::actuator::iommu_firmware_left`]: the state [`Self::hand_over`] @@ -407,8 +414,8 @@ impl Unit { /// Every field of [`FIRMWARE_LEFT`] that is on, off, except translation /// through a legacy-mode root while `in_flight` is [`InFlight::Identical`]: - /// [`enable`] then moves the unit onto this kernel's root under translation, - /// as §6.6 allows (`RTADDR_REG.RTA` may change while `TES` is set, §11.4.5). + /// [`enable`] then sets this kernel's root pointer with translation on, as + /// §6.6 allows onto structures with identical remapping results. fn hand_over(&self, in_flight: InFlight) { for (bit, what) in FIRMWARE_LEFT { if self.regs.read_u32(GSTS_REG) & bit == 0 { @@ -442,7 +449,7 @@ impl Unit { if bit == QUEUED_INVALIDATION_ENABLE { // §6.5.2: a queue is switched off only once it is empty and its // last descriptor was a wait, which only firmware could have sent. - wait(self.index, self.regs, "its invalidation queue", " empty", || { + self.wait("its invalidation queue", " empty", || { self.regs.read_u64(IQH_REG) == self.regs.read_u64(IQT_REG) }); } @@ -675,6 +682,9 @@ fn describe_scopes(owner: &str, index: usize, scopes: Scopes) { } } +/// Device scope type 1, a PCI endpoint: the requester is the function named. +const SCOPE_ENDPOINT: u8 = 1; + /// Device scope type 3, whose enumeration id is an I/O APIC's MADT id. const SCOPE_IOAPIC: u8 = 3; diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index 79830c70a34..115414e579f 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -368,85 +368,3 @@ const _: () = { assert!(displaced.domain == KERNEL_DOMAIN && displaced.requester == 0x00F8); assert!(new.0 == 0x5000 | PRESENT && new.1 == (u16::MAX as u64) << 8 | 1); }; - -/// A second root over the same context entries as `root`: every root and -/// context entry copied, every domain shared, so the two translate alike. -#[cfg(feature = "boot-actuators")] -pub fn copy_root(tables: &mut Tables, root: Table) -> Table { - let copy = tables.alloc(); - for bus in 0..TABLE_BYTES / 16 { - let (lo, hi) = root.read_pair(bus); - if lo & PRESENT == 0 { - continue; - } - let context = Table { phys: lo & ADDR_MASK }; - let copied = tables.alloc(); - for devfn in 0..TABLE_BYTES / 16 { - let (lo, hi) = context.read_pair(devfn); - if lo & PRESENT != 0 { - copied.write_pair(devfn, lo, hi); - } - } - copy.write_pair(bus, (lo & !ADDR_MASK) | copied.phys, hi); - } - copy -} - -/// What a legacy-mode walk from `root` reads for `stream` at `at`, every -/// entry fetched from memory rather than this CPU's cache: the root entry, -/// both halves of the context entry, then each second-level entry down to -/// the first that is a leaf or has neither `R` nor `W`. -#[cfg(feature = "boot-actuators")] -pub struct Walk { - entries: [u64; 8], - read: usize, -} - -#[cfg(feature = "boot-actuators")] -pub fn walk(root: Table, stream: StreamId, at: u64) -> Walk { - let mut walk = Walk { entries: [0; 8], read: 0 }; - let mut push = |entry: u64| { - walk.entries[walk.read] = entry; - walk.read += 1; - entry - }; - let fetch = |table: Table, index: usize| { - flush(table.slot(index).addr() as usize); - table.read(index) - }; - let root_entry = push(fetch(root, stream.bus() as usize * 2)); - if root_entry & PRESENT == 0 { - return walk; - } - let context = Table { phys: root_entry & ADDR_MASK }; - let index = stream.devfn() as usize * 2; - let (lo, hi) = (push(fetch(context, index)), push(fetch(context, index + 1))); - if lo & PRESENT == 0 { - return walk; - } - let mut table = Table { phys: lo & ADDR_MASK }; - let mut level = (hi & 0x7) + 2; - while level >= 1 { - let entry = push(fetch(table, ((at >> (12 + 9 * (level - 1))) & 0x1FF) as usize)); - if entry & (SL_READ | SL_WRITE) == 0 || level == 1 || entry & SL_LARGE != 0 { - break; - } - table = Table { phys: entry & ADDR_MASK }; - level -= 1; - } - walk -} - -#[cfg(feature = "boot-actuators")] -impl core::fmt::Display for Walk { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.write_str("[")?; - for (i, entry) in self.entries[..self.read].iter().enumerate() { - if i > 0 { - f.write_str(" ")?; - } - write!(f, "{entry:#x}")?; - } - f.write_str("]") - } -} diff --git a/tests/metal/src/lib.rs b/tests/metal/src/lib.rs index d371ebe7535..be53c64a004 100644 --- a/tests/metal/src/lib.rs +++ b/tests/metal/src/lib.rs @@ -910,12 +910,6 @@ 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 VT-d unit moved between its root and a copy of it hundreds of - // times, with translation off between some, while its fault events are - // held and their records counted. It writes only the units' registers, - // which a reset returns to firmware, and tables in memory this boot owns; - // what a fault refuses is a read or write a device made, never a register. - "iommu-handover-cycles", // Every CPU is handed the loader's Intel update with its header's revision // raised by one and its checksum sealed again. Its payload is Intel's own, // the revision the CPU already runs, so the CPU refuses it or runs what it diff --git a/tests/toyos.rs b/tests/toyos.rs index 1715b2b3883..8827db569d7 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1305,14 +1305,6 @@ const METAL: &[(&str, metal::Metal)] = &[ "iommu_firmware_left", metal::Metal { arms: SELFTESTS, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, ), - ( - // Each VT-d unit taken between two roots that translate alike, by - // translation going off and by a switch under it, with whatever DMA - // firmware left running going on through it: the count of faults each - // way. A measurement, so a named pass alone runs it. - "iommu_handover_cycles", - metal::Metal { arms: HANDOVER_CYCLES, judge: |b| iommu_handover_cycles(b[0].kernel().text()) }, - ), // ---- a claimed function at the unit, on tests/images/testcases.toml ---- ( // Claimed, given back and claimed again: both claims name one @@ -1817,19 +1809,6 @@ const SELFTEST_PARAMS: &[&str] = &[ "iommu-firmware-left", ]; -/// Its own boot: it holds every unit's fault events for its eight seconds, -/// so a fault the shared boot's tests stage would be counted here instead. -const HANDOVER_CYCLES: &[metal::Arm] = &[metal::Arm { - features: ACTUATOR_KERNEL, - named_only: true, - ..metal::once( - "iommu-cycles", - "tests/images/testcases.toml", - &["iommu-firmware-left", "iommu-handover-cycles"], - &[], - ) -}]; - const SELFTESTS: &[metal::Arm] = &[metal::Arm { features: ACTUATOR_KERNEL, ..metal::once("shared-debug", "tests/images/testcases.toml", SELFTEST_PARAMS, &[]) @@ -6074,21 +6053,6 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { Ok(()) } -/// Both kinds of cycle ran to their count; the fault lines and the counts are -/// the measurement, printed rather than judged. -fn iommu_handover_cycles(log: &str) -> Result<(), String> { - let lines: Vec<&str> = log.lines().filter(|l| l.contains("iommu-handover-cycles: ")).collect(); - for kind in ["off-on", "switch"] { - if !lines.iter().any(|l| l.contains(&format!("iommu-handover-cycles: {kind} ")) && l.contains(" with a fault")) { - return Err(format!("the {kind} cycles never reported their count:\n{log}")); - } - } - for line in lines { - eprintln!(" [iommu-handover-cycles] {}", line.split("iommu-handover-cycles: ").nth(1).unwrap_or(line)); - } - Ok(()) -} - /// The machine's kernel thread is hosted. fn klogd_hosted(boot: &serial::Serial) -> Result<(), String> { boot.must_be_clean()?; From 902bffb409c436eea9d14b425545b475076772b7 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 28 Oct 2026 13:42:44 +0100 Subject: [PATCH 3/4] vtd: every unit is handed over with translation off, inside protected memory regions; the keep-on branch goes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit VT-d gives system software no condition under which it may switch firmware's tables for its own under translation. §6.6 asks for identical remapping results, and nothing documented tells this kernel firmware's mappings. DMA_CTRL_PLATFORM_OPT_IN (§8.1) says only where platform DMA goes. So the keep-on branch and everything it alone used are deleted: InFlight, regions_identity, identity_mapped, SCOPE_ENDPOINT, Rmrr::whole, TRANSLATION_TABLE_MODE, and the judge's declared and kept. Every hand-over now takes translation off. While it is off, a unit's protected memory regions block DMA to the memory they cover (§11.4.8.1). Where every unit reports PLMR and PHMR, vtd/pmr.rs programs one pair on all of them, as §11.4.8.1 requires. They cover the largest stretch of firmware's RAM from 16 MiB to 4 GiB and the largest above it, each clear of every RMRR (kernel::dmaguard). The same alignment, probed and logged, applies to every unit. The regions go on before the first unit goes off and are released once the last translates. A unit without them logs that its hand-over is unguarded; QEMU's is one. Inside the regions, the hand-over invalidates through the unit's registers (§6.5.1). The invalidation queue and its status write are listed apart from the remapping structures the regions exempt (Appendix A, ECAP.C), so nothing depends on the queue's memory being reachable. The queue and interrupt remapping are programmed after the release. That makes Queue::invalidate_all dead: the actuator's leave_on invalidates through the registers too. A unit of version 6 or later refuses register-based invalidation (§11.4.6.1), so plan leaves it unprogrammed, as it does a unit whose IOTLB registers lie past its window. The steps run in kernel::dmaguard::order, whose host test reads arm, then every translate, then release, then every finish. It goes red with the arm step removed. Records: - The two hand-over issues become one: a unit without protected memory regions passes DMA untranslated while it is handed over. - The T14 iGPU fault is the existing 2026-10-04 issue, renamed because its claim is refused: no period of refused reads is needed. It carries #1086's measurement: on the T14, a switch under translation faulted in 0 of 600 cycles, and translation off and back on in 22 of 600, the unit refusing reads its tables grant. Intel's 631123-034 lists no erratum for it. Its exit is this kernel's sequence fixed, or a documented erratum's workaround applied. This change does not cure it: under iommu-firmware-left the T14 still takes translation off. Co-Authored-By: Claude Opus 5.5 --- ...lts-read-permission-after-the-hand-over.md | 69 ----- ...s-translation-for-its-root-table-switch.md | 45 --- ...asses-dma-untranslated-while-programmed.md | 53 ---- ...ma-untranslated-while-it-is-handed-over.md | 60 ++++ issues/the-iommu-refuses-nothing-yet.md | 5 +- ...r-after-a-handover-took-translation-off.md | 69 ----- ...ation-goes-off-and-on-under-its-scanout.md | 88 ++++++ kernel/pure/dmaguard.rs | 170 +++++++++++ kernel/pure/lib.rs | 1 + kernel/src/arch/x86_64/vtd/dmar.rs | 6 - kernel/src/arch/x86_64/vtd/mod.rs | 281 +++++++++--------- kernel/src/arch/x86_64/vtd/pmr.rs | 126 ++++++++ kernel/src/arch/x86_64/vtd/queue.rs | 22 +- tests/common/iommu.rs | 31 +- tests/toyos.rs | 30 +- 15 files changed, 629 insertions(+), 427 deletions(-) delete mode 100644 issues/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md delete mode 100644 issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md delete mode 100644 issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md create mode 100644 issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md delete mode 100644 issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md create mode 100644 issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md create mode 100644 kernel/pure/dmaguard.rs create mode 100644 kernel/src/arch/x86_64/vtd/pmr.rs diff --git a/issues/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md b/issues/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md deleted file mode 100644 index 3069f536bae..00000000000 --- a/issues/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md +++ /dev/null @@ -1,69 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-04 ---- - -# A function refused under translation faults read permission after the hand-over - -On the T14, when the integrated GPU (`00:02.0`, unit0) has its scanout reads -refused for about 2 ms (0.153 s to 0.155 s) by a unit left translating, and that unit is then -handed over and programmed, the GPU's next reads fault `0x06` (read -permission) through this kernel's identity domain, which grants them, and the -kernel halts on `DMA FAULT owner=kernel`. `vtd::enable` and `Unit::hand_over` -(`kernel/src/arch/x86_64/vtd/mod.rs`) do what VT-d Rev. 4.1 asks before `TE` -goes back on: `SRTP` to a fresh root, then a global context-cache and a global -IOTLB invalidation with both drains, acknowledged (§6.6). A unit reporting -`CAP.CM=0`, as unit0 does, caches no faulted translation (§6.2.4.1), so a -refusal before the hand-over leaves nothing in the unit the spec lets fault -after it. What carries it across, the GPU or the unit, is unknown. - -Firmware does not hand the T14 over this way: its four units read -`gsts=0x40000000`. The state is the one the `iommu-firmware-left` actuator left -at `9be0a8ac8` of pull request #700, which pointed every unit at an empty root -while unit0's identity domain was built. Today's actuator translates through -the identity domain and refuses nothing. - -Evidence, pull request #700, T14 boots: - -- `9be0a8ac8` plus the `FSTS` probe: unit0's one record, before `fault::arm` - clears it, is `00:02.0` read `0x9cb80000` reason `0x01` with `FSTS=0x3` - (overflowed), from the empty root. Once `TE` is confirmed, `FSTS=0` and the - record is clear. Next, `unit0 fault recording overflowed`, then - `00:02.0` read `0x9cc39000` reason `0x06`. Every address is inside - `rmrr0` (`0x9c000000..0xa07fffff`, scope `00:02.0`). -- `55a5ccb42`, the same empty-root actuator but handed over before the build, - so the refusal and the hand-over are both logged at 0.153 s and `TE` stayed - off until 0.155 s: no fault. -- `25ab31bd5` and `5e8cbb06e`, no refusal, and the hand-over and `translating` - logged in the same millisecond: no fault. - -- `9be0a8ac8` plus a walk probe: unit0's record before `fault::arm` is again - `00:02.0` read `0x9ccc0000` reason `0x01` with `FSTS=0x3`. At the fault, - `00:02.0` read `0x9cdb7000` reason `0x06`, raw high word - `0xc000000600000010`: `AT=0`, `PP=0`, `EXE=0`, `PRIV=0`, `T2=0`, an - untranslated read with no PASID. The unit's walk, from `RTADDR` `0xa18000`, - each entry read from DRAM after `clflush`: root `0xa19001`, context - `0xa04001:0x102` (`P=1`, `TT=00`, `AW=2`, `DID=1`), then `0xa05003`, - `0xa08003` and the 2 MiB leaf `0x9cc00083`, present with read and write over - `0x9cdb7000`. The GPU's PCI `STATUS` reads `0x0010`, no master or target - abort, before the actuator, before the hand-over, at `TE` and at the fault. - The unit refused a read the tables in memory grant. -- the walk probe plus 3 ms with `TE` off after each hand-over: the same - empty-root refusals (`FSTS=0x3`, record `0x01`) before `fault::arm`, then no - `DMA FAULT` and `Boot: complete`. - -So the tables are not what refuses, and neither is any cached state VT-d Rev. -4.1 lets the unit keep. The `0x06` needs both a period of refused reads and -`TE` back on within microseconds of the hand-over: three boots of that -actuator with no hold faulted (`9be0a8ac8` alone, with the `FSTS` probe and -with the walk probe), one with a 3 ms hold did not, and `55a5ccb42`, -`TE` off about 2 ms after its refusals, did not. - -**Exit**: on the T14, the reproduction (the selftests boot at `9be0a8ac8`) -reaches `Boot: complete` with no `DMA FAULT` line, through a hand-over that -waits on an event the unit or the function reports. A timed hold with `TE` -off does not close this: it is a flat wait no hardware document mandates, it -widens the untranslated window -`issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md` -records, and it rests on one green boot against three reds. diff --git a/issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md b/issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md deleted file mode 100644 index b5cf73c5fb2..00000000000 --- a/issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md +++ /dev/null @@ -1,45 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-04 ---- - -# A unit in scalable or abort mode loses translation for its root-table switch - -A unit firmware leaves translating with a root table in scalable or abort-DMA -mode (`RTADDR_REG.TTM` not `00`) and `CAP.ESRTPS` clear may not have its root -table pointer set while translation is on (VT-d Rev. 4.1 §6.6). Handing such a -unit over to this kernel's root table therefore turns translation off for that -one switch, and for that moment every function behind the unit reaches all of -memory. `issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md` -removes the gap where the switch may stay translating; on these units it may -not. - -Evidence: the specification alone. No machine here has such a unit: every unit of the -T14 reports `CAP` bit 63 (`ESRTPS`), `ECAP` bit 43 (`SMTS`) and bit 52 -(`ADMS`) clear. - -**Ruled** (owner, 2026-10-04, "Accept and file"): "Allow the one-moment gap on -those machines, log it, and file it as a known weakness with a fix (older -protection registers) as its exit." - -Owner: the orchestrator, under `issues/the-iommu-refuses-nothing-yet.md`. - -**Exit**: on such a unit that reports `CAP.PLMR` (bit 5) and `CAP.PHMR` -(bit 6), `PMEN`'s protected memory regions (§11.4.8.1) cover all of memory -before translation goes off for the switch and are released only once it is -back on; a host test reads that order, and is red with the `PMEN` step -removed. It is a host test because no machine reaches the path: QEMU's unit -always reports `ESRTPS` and implements `PMEN_REG` as read-only zero, and the -T14's units report `ESRTPS` clear but support neither mode. - -A unit that reports either bit clear has no fix under this exit: with either -clear, that region's base and limit are read-only (§11.4.8.1), so `PMEN` -cannot cover all of memory; the same section plans the protected memory -registers for deprecation, pointing new software at abort-DMA mode. Such a -unit keeps the gap, logged, and this file stays open for it once the `PMEN` -exit is met. - -**Exit for a unit with `PLMR` or `PHMR` clear**: on such a unit no function -behind it reaches memory while translation is off for the switch; a host test -reads that, and is red with the fix removed. diff --git a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md b/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md deleted file mode 100644 index 9e17ce4106d..00000000000 --- a/issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md +++ /dev/null @@ -1,53 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-04 ---- - -# A unit left translating passes DMA untranslated while it is programmed - -`vtd::enable` (`kernel/src/arch/x86_64/vtd/mod.rs`) switches off the -translation firmware left on (`Unit::hand_over`) once the unit's tables are -built, unless firmware declares what it has in flight (below), and turns it -back on through this kernel's root table only after the queue, fault -reporting, both table pointers and a global invalidation are programmed. In -between, every function behind that unit reaches all of memory. - -Keeping `TE` on is what VT-d Rev. 4.1 §6.6 allows only onto structures that -give identical remapping results. Where firmware reports -`DMA_CTRL_PLATFORM_OPT_IN` (§8.1), the DMA it leaves running reaches only its -RMRRs. This kernel infers that firmware's tables map those identity and -read-write, the mapping §3.16 expects of system software's tables but does -not guarantee of firmware's. On that inference, where the identity domain -holds every RMRR and every device they name is enumerated, the hand-over -keeps `TE` on. The T14 reports the flag, but its firmware leaves translation -off, so only `iommu-firmware-left` takes that path there. Elsewhere, -firmware's tables have to be read and copied before the switch. - -Both routes share one exposure. `mm::init` runs before `iommu::init` and -treats `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, -`is_usable_type`). From then until the switch, or until translation goes off, -a unit left translating walks firmware's tables in memory the pmm may already -have handed out. The same exposure weakens the inference above and any copy -of firmware's tables, which would have to be read before the pmm hands that -memory out. - -Evidence: no machine here is handed a unit translating; the T14's four units -read `gsts=0x40000000` (pull request #700's selftests boot). QEMU's DMAR leaves -`DMA_CTRL_PLATFORM_OPT_IN` clear (`dma_ctrl_opt_in=n`). Staged by the -`iommu-firmware-left` actuator on QEMU 11.1.1, unit0 logs `was handed over -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 as 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/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md b/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md new file mode 100644 index 00000000000..ac5de0d678d --- /dev/null +++ b/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md @@ -0,0 +1,60 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A unit without protected memory regions passes DMA untranslated while it is handed over + +`vtd::init` (`kernel/src/arch/x86_64/vtd/mod.rs`) hands every unit over with +translation off. That covers a unit firmware left translating, whatever its +table mode. VT-d Rev. 4.1 gives no condition under which system software may +switch firmware's tables for its own under translation. §6.6 requires +identical remapping results, and nothing documented lets this kernel know +firmware's mappings. `DMA_CTRL_PLATFORM_OPT_IN` (§8.1) restricts only where +platform DMA goes. + +While translation is off, a unit's protected memory regions (§11.4.8) block +DMA to the memory they cover. Where every unit reports `CAP.PLMR` and +`CAP.PHMR`, as the T14's four do, `vtd/pmr.rs` programs one pair of regions +on all of them before the first goes off, and releases them once the last is +translating. The pair is the largest stretch of firmware's RAM below 4 GiB, +from 16 MiB, and the largest at or above it, each clear of every RMRR +(`kernel::dmaguard`). Register-based invalidation completes the hand-over +inside them, since the invalidation queue's memory is no remapping structure +(Appendix A). + +What still passes untranslated: + +- **On a unit without both regions, all of memory.** QEMU's unit is one. The + boot logs `has no protected memory regions`. +- **On the T14, outside the pair:** + - RAM below 16 MiB; + - RAM in a stretch the map breaks with a reserved or runtime entry; + - every RMRR, the iGPU's stolen memory among them; + - peer MMIO. +- **Before the hand-over.** `mm::init` runs before `iommu::init` and takes + `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, `is_usable_type`). + A unit left translating may walk firmware's tables in memory the pmm has + already handed out. + +A unit of major version 6 or later refuses register-based invalidation +(§11.4.6.1), so `plan` leaves it unprogrammed. No machine here has one. + +Evidence: +- No machine here is handed a unit translating; the T14's four read + `gsts=0x40000000`. +- `iommu-firmware-left` stages one. QEMU 11.1.1 runs the unguarded hand-over + (`vtd_firmware_left`). The T14 runs the guarded one (`iommu_firmware_left`'s + row, which reads the arm before translation goes off and the release after). + +Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). + +**Exit**: no unit passes DMA to memory outside its RMRRs while it is handed +over. + +- §11.4.8.1 marks the protected regions "planned for deprecation". A unit + without them needs another documented mechanism. Abort-DMA mode (§3.4.4) + is one: it needs `ECAP.ADMS`, which the T14 lacks, and §3.4.4 rules it out + while a device an RMRR names is mastering the bus. +- A host test or a machine reads each case. diff --git a/issues/the-iommu-refuses-nothing-yet.md b/issues/the-iommu-refuses-nothing-yet.md index e7eaa0fb61b..73c27ccb78c 100644 --- a/issues/the-iommu-refuses-nothing-yet.md +++ b/issues/the-iommu-refuses-nothing-yet.md @@ -52,8 +52,9 @@ a unit firmware left translating: scalable or abort-DMA mode, with `CAP.ESRTPS` clear, may not have its root table switched under translation (VT-d Rev. 4.1 §6.6), so translation goes off for that one switch. The older protection registers are `PMEN`'s - protected memory regions (§11.4.8.1). The weakness is - `issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md`. + protected memory regions (§11.4.8.1). Every hand-over now takes + translation off inside them where every unit has them; what remains is + `issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md`. - **"Apply it at hand-over"**, on reserved memory: "From the hand-over on, only display and USB controllers keep access to their reserved region; any other device's access is refused. A device that firmware was still using diff --git a/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md b/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md deleted file mode 100644 index 767fd3669aa..00000000000 --- a/issues/the-t14s-igpu-faulted-inside-its-rmrr-after-a-handover-took-translation-off.md +++ /dev/null @@ -1,69 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-28 ---- - -# The T14's iGPU faulted inside its RMRR after a hand-over took translation off - -One T14 boot under `iommu-firmware-left` (shared-debug, at 7ef0a48c8) halted -1 ms after `Boot: peripherals ready`: - -``` -iommu: unit0 fault recording overflowed — earlier faults are lost -iommu: DMA FAULT owner=kernel unit0 stream=00:02.0 addr=0x000000009cea7000 access=read reason=0x06 domain=unrecorded bme=cleared unitfaults=1 streamfaults=1 first=y read-permission -``` - -What the boot establishes: - -- `00:02.0` is the iGPU, alone in unit0's scope. No driver runs it, so the - read is the scanout firmware left running. -- `rmrr0` reserves `0x9c000000..0xa07fffff` for it. The root bridges' memory - begins at `0xa0800000`, where the 11th Generation Intel Core datasheet - (631122 §2.5) puts the top of graphics stolen memory. The address lies - inside the region. -- Reason 6h is a read that met a second-stage entry with `R` clear (VT-d 5.0 - Table 30, `LGN.3`). The identity domain maps all of `0x0..0x45f800000` read - and write, so unit0 did not translate by the tables it had been given. -- Fault reporting is armed after `hand_over` and clears every older record, - and `IF` is set throughout `vtd::init`. So the fault was taken after unit0 - was back on through this kernel's root. -- unit0 has one recording register (`nfr=1`), and the overflow shows at least - one more fault after it. - -Under `iommu-firmware-left` on the T14, unit0's translation went on, off and -on again while the scanout read through it. Every other T14 boot only turns -it on. The orchestrator's logs to 2026-10-28 hold 787 boots with one -transition and no iGPU fault, and 46 cycled boots with this one (p ≈ 0.055). -That suggests translation going off under live DMA. It does not establish it. - -Since pull request #1086, a unit whose firmware reports -`DMA_CTRL_PLATFORM_OPT_IN`, whose RMRRs the identity domain holds, and whose -root is in legacy mode keeps translation on. The hand-over switches its root -under translation (§6.6). Under the actuator the T14 now makes a single -transition. On a machine whose firmware reports nothing, translation still -goes off first. - -**Measurement**: the `iommu-handover-cycles` actuator, on a measurement -branch and never landed, moves every unit between its root and an identical -copy. It runs 200 switches under translation first, so no cycle in that phase -follows translation going off, then 200 cycles of translation off and on. -Fault events are held throughout, and each record is counted against the -cycle it followed. Its `off-on` omits the queue and interrupt-remapping -toggles of the original hand-over, so an `off-on` phase without faults does -not clear that sequence. Results go to the pull request's evidence. - -Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). - -**Exit**, after the measurement boots: - -- If the `switch` phase faults, keeping translation on does not cure this. - The defect is re-scoped to what does. -- If only the `off-on` phase faults, turning translation off under live DMA - faults on this unit. This file stays open, for the path that still does - that on machines whose firmware reports nothing, until that path no longer - takes translation off. -- If neither phase faults, the measurement is inconclusive. The cause stays - unknown and this file stays open. - -It closes when no remaining hand-over path is known to fault. diff --git a/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md b/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md new file mode 100644 index 00000000000..074ddd3e13e --- /dev/null +++ b/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md @@ -0,0 +1,88 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# The T14's iGPU faults read permission after translation goes off and on under its scanout + +On the T14, when unit0, the integrated GPU's (`00:02.0`), has translation +turned off for microseconds and back on while the GPU's scanout reads through +it, the GPU's +reads sometimes fault `0x06` (read permission) through this kernel's identity +domain, which grants them. The kernel then halts on `DMA FAULT owner=kernel`. +Before `TE` goes back on, the hand-over does what VT-d Rev. 4.1 asks: `SRTP` +to a fresh root, then a global context-cache and a global IOTLB invalidation +with both drains, acknowledged (§6.6). A unit reporting `CAP.CM=0`, as unit0 +does, caches no faulted translation (§6.2.4.1). What carries the fault across, +the GPU or the unit, is unknown. + +Firmware does not hand the T14 over this way: its four units read +`gsts=0x40000000`. The state below is the one the `iommu-firmware-left` +actuator left at `9be0a8ac8` of pull request #700, which pointed every unit at +an empty root while unit0's identity domain was built. + +Evidence, pull request #700, T14 boots: + +- `9be0a8ac8` plus the `FSTS` probe: unit0's one record, before `fault::arm` + clears it, is `00:02.0` read `0x9cb80000` reason `0x01` with `FSTS=0x3` + (overflowed), from the empty root. Once `TE` is confirmed, `FSTS=0` and the + record is clear. Next, `unit0 fault recording overflowed`, then + `00:02.0` read `0x9cc39000` reason `0x06`. Every address is inside + `rmrr0` (`0x9c000000..0xa07fffff`, scope `00:02.0`). +- `55a5ccb42`, the same empty-root actuator but handed over before the build, + so the refusal and the hand-over are both logged at 0.153 s and `TE` stayed + off until 0.155 s: no fault. +- `25ab31bd5` and `5e8cbb06e`, no refusal, and the hand-over and `translating` + logged in the same millisecond: no fault. + +- `9be0a8ac8` plus a walk probe: unit0's record before `fault::arm` is again + `00:02.0` read `0x9ccc0000` reason `0x01` with `FSTS=0x3`. At the fault, + `00:02.0` read `0x9cdb7000` reason `0x06`, raw high word + `0xc000000600000010`: `AT=0`, `PP=0`, `EXE=0`, `PRIV=0`, `T2=0`, an + untranslated read with no PASID. The unit's walk, from `RTADDR` `0xa18000`, + each entry read from DRAM after `clflush`: root `0xa19001`, context + `0xa04001:0x102` (`P=1`, `TT=00`, `AW=2`, `DID=1`), then `0xa05003`, + `0xa08003` and the 2 MiB leaf `0x9cc00083`, present with read and write over + `0x9cdb7000`. The GPU's PCI `STATUS` reads `0x0010`, no master or target + abort, before the actuator, before the hand-over, at `TE` and at the fault. + The unit refused a read the tables in memory grant. +- the walk probe plus 3 ms with `TE` off after each hand-over: the same + empty-root refusals (`FSTS=0x3`, record `0x01`) before `fault::arm`, then no + `DMA FAULT` and `Boot: complete`. + +Pull request #1086, T14: + +- One shared-debug boot (7ef0a48c8, today's actuator, which refuses nothing) + halted on `00:02.0` read `0x9cea7000` reason `0x06`. It was the one fault in + 46 such boots, against 0 in 787 boots whose unit0 only turns translation on. +- `iommu-handover-cycles` (measurement branch `igpu-fault-measure`, e3b1562e7, + never landed) moved every unit between two identical identity roots, three + boots, with fault events held. 200 root switches under translation ran + first, then 200 cycles of translation off, root, queued invalidation and + translation on. Totals: + - **switch: 0 of 600 cycles faulted;** + - **off-on: 22 of 600 cycles faulted**, with 94 records. +- Every off-on record is `00:02.0`, a read, `0x06`, at an address inside the + firmware framebuffer in data stolen memory (`BDSM=0x9c800000`, + `BGSM=0x9c000000`, `TOLUD=0xa0800000`). The first comes 546 to 598 µs after + translation is back on. +- Each printed walk from memory reads a present root and context entry and a + read-write 2 MiB leaf. + +So no period of refused reads is needed. Translation going off for +microseconds and back on under live scanout is enough, in about one cycle in +27, and the unit then refuses reads its tables grant. Turning translation on +for the first time since firmware is not: the 787 boots did only that. The 3 ms hold's one green boot is no evidence at that rate. +Neither the tables nor any cached state VT-d Rev. 4.1 lets the unit keep is +what refuses. Intel's 11th Generation Core specification update (631123-034, +errata TGL001 to TGL075) documents no erratum for VT-d or the iGPU's DMA. + +Every hand-over still takes translation off. Under `iommu-firmware-left` the +T14 therefore still reaches this, in about one boot in 27. Real T14 firmware +leaves translation off, so no other boot does. + +**Exit**: this kernel's hand-over sequence is fixed so that the measurement's +off-on cycles fault no more than its switch cycles, or a documented erratum +for it is cited and its workaround applied. Either is judged on the T14 by +the measurement and by `iommu_firmware_left`'s row. diff --git a/kernel/pure/dmaguard.rs b/kernel/pure/dmaguard.rs new file mode 100644 index 00000000000..b4bd1d44ef0 --- /dev/null +++ b/kernel/pure/dmaguard.rs @@ -0,0 +1,170 @@ +//! What a remapping unit's protected memory regions cover while every unit is +//! handed over, and the order the hand-over takes. +//! +//! A unit has one low region, below 4 GiB, and one high region. Each is the +//! largest stretch of memory the firmware map calls RAM, with no reserved +//! region in it and no gap, since a gap may decode as MMIO, above a floor the +//! caller's platform sets. Each is shrunk to +//! the units' alignment, so what is protected is never more than that. + +#![forbid(unsafe_code)] + +use alloc::vec::Vec; + +pub const FOUR_GIB: u64 = 1 << 32; + +/// `[base, end)`, never empty. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Span { + pub base: u64, + pub end: u64, +} + +/// The largest part of `ram` inside `[lo, hi)` that touches no `avoid` range +/// and no gap between `ram`'s ranges, with its base rounded up and its end +/// rounded down to `align` (a power of two). `None` if nothing is left. +pub fn largest(ram: &[(u64, u64)], lo: u64, hi: u64, avoid: &[(u64, u64)], align: u64) -> Option { + assert!(align.is_power_of_two(), "a protected region's alignment {align:#x} is not a power of two"); + let mut ram: Vec<(u64, u64)> = ram.iter().copied().filter(|&(s, e)| s < e).collect(); + ram.sort_unstable(); + // Adjacent ranges are one stretch; a gap ends it. + let mut runs: Vec<(u64, u64)> = Vec::new(); + for (start, end) in ram { + match runs.last_mut() { + Some(last) if start <= last.1 => last.1 = last.1.max(end), + _ => runs.push((start, end)), + } + } + let mut avoid: Vec<(u64, u64)> = avoid.iter().copied().filter(|&(s, e)| s < e).collect(); + avoid.sort_unstable(); + let mut best: Option = None; + for (start, end) in runs { + let mut piece = start.max(lo); + let stop = end.min(hi); + for &(a, b) in avoid.iter().chain(core::iter::once(&(stop, u64::MAX))) { + if b <= piece { + continue; + } + let cut = a.min(stop); + if cut > piece { + let base = piece.div_ceil(align).saturating_mul(align); + let end = cut / align * align; + if base < end && best.is_none_or(|b| end - base > b.end - b.base) { + best = Some(Span { base, end }); + } + } + if b >= stop { + break; + } + piece = piece.max(b); + } + } + best +} + +/// One step of handing every unit over, units named by index. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Step { + /// Every unit's protected regions on. + Arm, + /// Translation off, this kernel's root pointed and invalidated through the + /// unit's registers, translation on. + Translate(usize), + /// Every unit's protected regions off. + Release, + /// The invalidation queue and interrupt remapping, which need memory the + /// regions may cover. + Finish(usize), +} + +/// The hand-over of `units` units, between `Arm` and `Release` where `guarded`. +pub fn order(units: usize, guarded: bool) -> Vec { + let mut steps = Vec::with_capacity(2 * units + 2); + if guarded { + steps.push(Step::Arm); + } + steps.extend((0..units).map(Step::Translate)); + if guarded { + steps.push(Step::Release); + } + steps.extend((0..units).map(Step::Finish)); + steps +} + +#[cfg(test)] +mod tests { + use super::*; + use std::vec; + + const MIB: u64 = 1 << 20; + + #[test] + fn every_translation_happens_inside_the_guard_and_nothing_needs_memory_until_it_is_gone() { + for units in 0..5 { + let steps = order(units, true); + let at = |s: Step| steps.iter().position(|&t| t == s).unwrap(); + let arm = at(Step::Arm); + let release = at(Step::Release); + for unit in 0..units { + assert!(arm < at(Step::Translate(unit)), "{steps:?}"); + assert!(at(Step::Translate(unit)) < release, "{steps:?}"); + assert!(release < at(Step::Finish(unit)), "{steps:?}"); + } + assert_eq!(steps.len(), 2 * units + 2); + } + } + + #[test] + fn an_unguarded_hand_over_still_translates_every_unit_before_finishing_any() { + let steps = order(3, false); + assert_eq!(steps.iter().filter(|s| matches!(s, Step::Arm | Step::Release)).count(), 0); + let last_translate = steps.iter().rposition(|s| matches!(s, Step::Translate(_))).unwrap(); + let first_finish = steps.iter().position(|s| matches!(s, Step::Finish(_))).unwrap(); + assert!(last_translate < first_finish); + } + + /// The T14's shape: RAM up to its graphics region, which an RMRR names, + /// and RAM again above 4 GiB. + #[test] + fn a_region_stops_at_the_reserved_region_and_at_4_gib() { + let ram = [(0x10_0000, 0x9c00_0000), (0x1_0000_0000, 0x4_5f80_0000)]; + let avoid = [(0x9c00_0000, 0xa080_0000)]; + assert_eq!( + largest(&ram, 16 * MIB, FOUR_GIB, &avoid, MIB), + Some(Span { base: 16 * MIB, end: 0x9c00_0000 }) + ); + assert_eq!( + largest(&ram, FOUR_GIB, u64::MAX, &avoid, MIB), + Some(Span { base: FOUR_GIB, end: 0x4_5f80_0000 }) + ); + } + + #[test] + fn a_gap_in_the_map_ends_a_stretch_and_adjacent_ranges_join() { + let ram = [(0x0100_0000, 0x2000_0000), (0x2000_0000, 0x3000_0000), (0x3010_0000, 0x3800_0000)]; + assert_eq!(largest(&ram, 0, FOUR_GIB, &[], MIB), Some(Span { base: 0x0100_0000, end: 0x3000_0000 })); + } + + #[test] + fn a_reserved_region_inside_a_stretch_leaves_its_larger_side() { + let ram = [(0x0100_0000, 0x8000_0000)]; + let avoid = [(0x2000_0000, 0x2010_0000)]; + assert_eq!(largest(&ram, 0, FOUR_GIB, &avoid, MIB), Some(Span { base: 0x2010_0000, end: 0x8000_0000 })); + } + + #[test] + fn alignment_only_ever_shrinks_a_region() { + let ram = [(0x0123_4000, 0x0987_6000)]; + let span = largest(&ram, 0, FOUR_GIB, &[], 16 * MIB).unwrap(); + assert_eq!(span, Span { base: 0x0200_0000, end: 0x0900_0000 }); + assert!(largest(&ram, 0, FOUR_GIB, &[], 1 << 30).is_none()); + } + + #[test] + fn nothing_is_protected_where_nothing_is_ram() { + assert!(largest(&[], 0, FOUR_GIB, &[], MIB).is_none()); + let ram = vec![(0x1000_0000, 0x2000_0000)]; + assert!(largest(&ram, 0, FOUR_GIB, &[(0, u64::MAX)], MIB).is_none()); + assert!(largest(&ram, FOUR_GIB, u64::MAX, &[], MIB).is_none()); + } +} diff --git a/kernel/pure/lib.rs b/kernel/pure/lib.rs index a78b30e8250..6bc8a883667 100644 --- a/kernel/pure/lib.rs +++ b/kernel/pure/lib.rs @@ -13,6 +13,7 @@ pub mod bootwrite; pub mod clock; pub mod cpu; pub mod dirnodes; +pub mod dmaguard; pub mod frames; pub mod iova; pub mod message; diff --git a/kernel/src/arch/x86_64/vtd/dmar.rs b/kernel/src/arch/x86_64/vtd/dmar.rs index cf5a202005d..e803ab85c74 100644 --- a/kernel/src/arch/x86_64/vtd/dmar.rs +++ b/kernel/src/arch/x86_64/vtd/dmar.rs @@ -186,12 +186,6 @@ impl Rmrr { Scopes { table: self.table, offset: self.at + RMRR_SCOPES, end: self.at + self.len } } - /// Whether the declared length covers every fixed field, so none of them - /// read as the zero `bounded` gives a field past it. - pub fn whole(&self) -> bool { - self.len >= RMRR_SCOPES - } - fn field(&self, offset: usize) -> T { bounded(self.table, self.at, self.len, offset) } diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index f9e02be81ea..984702b71e1 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -7,11 +7,15 @@ //! points every unit at one interrupt remapping table. A capability the kernel //! cannot use leaves the unit switched off rather than halting, logged by //! register value rather than a bare "unsupported". +//! +//! Every unit is handed over with translation off, and where every unit has +//! protected memory regions, inside them (`pmr`). pub mod dmar; pub mod domain; pub mod fault; pub mod interrupt; +mod pmr; mod queue; mod table; @@ -27,6 +31,7 @@ use crate::sync::Lock; use crate::time::{Duration, Tripwire}; use super::iommu_unit::{yn, MAX_UNITS}; +use kernel::dmaguard::Step; use dmar::{Dmar, Malformed, Scope, Scopes, Structure}; use queue::Queue; use table::{Table, Tables}; @@ -48,13 +53,9 @@ const FEUADDR_REG: u64 = 0x44; const IQH_REG: u64 = 0x80; const IQT_REG: u64 = 0x88; const IQA_REG: u64 = 0x90; - -/// `RTADDR_REG.TTM`, bits 11:10, `00` for legacy mode: this kernel's root is -/// legacy, and with `CAP.ESRTPS` clear `TTM` may not change while `TES` is set -/// (§6.6, §11.4.5). -// No machine here reports `ESRTPS`, so a unit in another mode goes off -// whatever it reports (`issues/a-unit-in-scalable-or-abort-mode-loses-translation-for-its-root-table-switch.md`). -const TRANSLATION_TABLE_MODE: u64 = 0b11 << 10; +/// Register-based invalidation (§6.5.1); `IOTLB_REG` sits `ECAP.IRO` past the base. +const CCMD_REG: u64 = 0x28; +const IOTLB_REG_AFTER_IRO: u64 = 0x08; /// `GCMD` bits; `GSTS` confirms each at the same bit position. const TRANSLATION_ENABLE: u32 = 1 << 31; @@ -158,14 +159,12 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW // 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(); + // Every RMRR, which no protected region may cover during the hand-over. + let mut rmrrs: Vec<(u64, u64)> = Vec::new(); // What no domain's addresses may reach, RMRRs added below as the walk // meets them; nothing where no declaration is held. let mut reserved: Option> = windows .map(|w| w.iter().map(|w| (w.base, w.end())).chain([super::iommu_unit::INTERRUPT_RANGE]).collect()); - // Whether every region was read whole and is one this kernel's root maps - // for every device it names; cleared too by a walk that stops before the - // table's end, since the regions past it are unread. - let mut regions_identity = true; for structure in dmar.structures() { match structure { Ok(Structure::Drhd(drhd)) => { @@ -174,7 +173,6 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW units += 1; if units > MAX_UNITS { log!("iommu: more than {MAX_UNITS} units described; the rest are not inventoried"); - regions_identity = false; break; } if let Some(unit) = describe_unit(units - 1, &drhd) { @@ -216,8 +214,8 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW if let Some(reserved) = reserved.as_mut() { reserved.push((base, limit.saturating_add(1))); } + rmrrs.push((base, limit.saturating_add(1))); describe_scopes("rmrr", regions, rmrr.scopes()); - regions_identity &= held && identity_mapped(devices, &rmrr); regions += 1; held_regions += usize::from(held); } @@ -233,7 +231,6 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW "iommu: DMAR structure at +{at} declares {declared} bytes the table cannot \ hold — stopping" ); - regions_identity = false; } } } @@ -248,54 +245,59 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW domain::avoid(reserved); let remap = remappable(&ready, units, dmar.flags); - let in_flight = in_flight(dmar.flags, regions_identity); // 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, in_flight); + let mut steps: Vec> = ready.into_iter().map(Some).collect(); + // Before any table is built: what a domain of a driver's own can be is the + // narrowest thing every translating unit on this machine agrees to. + for (unit, plan) in steps.iter().flatten() { + domain::unit_agrees(plan.width, unit.caps.domains(), unit.caps.mgaw()); + } + #[cfg(feature = "boot-actuators")] + if crate::actuator::iommu_firmware_left() { + for (unit, plan) in steps.iter().flatten() { + unit.leave_on(devices, identity_domain(&mut domains, plan.width), plan.width); + } + } + // A unit whose translation goes off passes DMA untranslated until it is + // back on, so the protected regions cover every unit's window at once. + let mut guard = { + let units: Vec<(usize, Mmio, bool)> = steps + .iter() + .flatten() + .map(|(unit, _)| (unit.index, unit.regs, unit.caps.protected_memory())) + .collect(); + let ram: Vec<(u64, u64)> = crate::mm::firmware_map() + .iter() + .filter(|e| toyos_bootmap::is_read_as_memory(e.uefi_type)) + .map(|e| (e.start, e.end)) + .collect(); + if units.is_empty() { None } else { pmr::program(&units, &ram, &rmrrs, crate::mm::pmm::top()) } + }; + let mut translating: Vec> = Vec::new(); + for step in kernel::dmaguard::order(steps.len(), guard.is_some()) { + match step { + Step::Arm => guard.as_ref().expect("an armed step without a guard").arm(), + Step::Translate(at) => { + let (unit, plan) = steps[at].take().expect("a unit translated twice"); + translating.push(Some(translate(unit, plan, devices, &mut domains))); + } + Step::Release => guard.take().expect("a release without a guard").release(), + Step::Finish(at) => finish(translating[at].take().expect("a unit finished twice"), devices, remap), + } } true } -/// Whether `rmrr` is whole, `base <= limit`, on the one segment this kernel -/// enumerates, and names only endpoints it writes a context entry for. A -/// bridge scope's requesters are the devices below it, which it does not name. -fn identity_mapped(devices: &[PciDevice], rmrr: &dmar::Rmrr) -> bool { - rmrr.whole() - && rmrr.base() <= rmrr.limit() - && rmrr.segment() == toyos_acpi::SEGMENT_GROUP - && rmrr.scopes().all(|scope| { - let Ok(scope) = scope else { return false }; - let Some(stream) = scope.stream_id().filter(|_| scope.kind() == SCOPE_ENDPOINT) else { return false }; - devices.iter().any(|d| StreamId::pci(d.bus, d.dev, d.func) == stream) - }) -} - -/// What a unit firmware left translating may have in flight when this kernel's -/// root replaces firmware's. -#[derive(Clone, Copy)] -enum InFlight { - /// Only what this kernel's root is taken to translate as firmware's does: - /// firmware reports `DMA_CTRL_PLATFORM_OPT_IN` (§8.1), so the DMA it leaves - /// running reaches only its RMRRs, and the identity domain maps each of - /// them, identity and read-write, for every device it names. That - /// firmware's own tables map them the same way is an inference, not a - /// guarantee: §3.16 sets that expectation for system software's tables. - /// §6.6 allows the switch under translation only onto identical results. - Identical, - /// Anything, for the reason given. - Unknown(&'static str), -} - -fn in_flight(flags: u8, regions_identity: bool) -> InFlight { - if flags & dmar::FLAG_DMA_CTRL_OPT_IN == 0 { - InFlight::Unknown("firmware does not declare its DMA confined to its reserved regions") - } else if !regions_identity { - InFlight::Unknown("a reserved region is one this kernel's root does not translate as firmware must") - } else { - InFlight::Identical - } +/// The identity domain at `width`, built the first time it is asked for. +fn identity_domain(domains: &mut [Option
; 2], width: AddressWidth) -> Table { + *domains[domain_slot(width)].get_or_insert_with(|| { + let top = crate::mm::pmm::top(); + let (domain, frames) = table::identity_domain(&mut TABLES.lock(), width, top); + log!("iommu: identity domain aw={} covers 0x0..{top:#x} in {frames} 2 MiB leaves", width.bits()); + domain + }) } /// Whether every interrupt source may be moved to the remappable format, and @@ -356,6 +358,8 @@ struct Unit { base: u64, regs: Mmio, caps: Capabilities, + /// `VER_REG`'s major version. + major: u32, } impl Unit { @@ -397,11 +401,11 @@ impl Unit { let root = identity_root(&mut tables, devices, domain, width); (queue, root, tables.alloc()) }; - self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); self.regs.write_u64(RTADDR_REG, root.phys()); self.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - queue.invalidate_all(self.regs); + self.invalidate_through_registers(); self.command(TRANSLATION_ENABLE, true, "translation"); + self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); if self.caps.interrupt_remapping() { self.regs.write_u64(interrupt::IRTA_REG, remap.phys()); self.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); @@ -412,39 +416,35 @@ impl Unit { } } - /// Every field of [`FIRMWARE_LEFT`] that is on, off, except translation - /// through a legacy-mode root while `in_flight` is [`InFlight::Identical`]: - /// [`enable`] then sets this kernel's root pointer with translation on, as - /// §6.6 allows onto structures with identical remapping results. - fn hand_over(&self, in_flight: InFlight) { + /// A global context-cache invalidation, then a global IOTLB one draining + /// reads and writes, through the unit's registers (§6.5.1); valid only with + /// queued invalidation off, whatever `TES` says. A unit that reports either + /// done at no granularity refused it, as every unit from version 6 does. + fn invalidate_through_registers(&self) { + const START: u64 = 1 << 63; + const GLOBAL_CONTEXT: u64 = 0b01 << 61; + const CONTEXT_DONE: u64 = 0b11 << 59; + const GLOBAL_IOTLB: u64 = 0b01 << 60; + const DRAIN: u64 = (1 << 49) | (1 << 48); + const IOTLB_DONE: u64 = 0b11 << 57; + let iotlb = self.caps.iotlb_registers() + IOTLB_REG_AFTER_IRO; + for (at, request, done, what) in [ + (CCMD_REG, START | GLOBAL_CONTEXT, CONTEXT_DONE, "the context-cache invalidation"), + (iotlb, START | GLOBAL_IOTLB | DRAIN, IOTLB_DONE, "the IOTLB invalidation"), + ] { + self.regs.write_u64(at, request); + self.wait(what, " done", || self.regs.read_u64(at) & START == 0); + let read = self.regs.read_u64(at); + assert!(read & done != 0, "iommu: unit{} refused {what}: {read:#018x}", self.index); + } + } + + /// Every field of [`FIRMWARE_LEFT`] that is on, off. + fn hand_over(&self) { for (bit, what) in FIRMWARE_LEFT { if self.regs.read_u32(GSTS_REG) & bit == 0 { continue; } - if bit == TRANSLATION_ENABLE { - let legacy = self.regs.read_u64(RTADDR_REG) & TRANSLATION_TABLE_MODE == 0; - match in_flight { - InFlight::Identical if legacy => { - log!( - "iommu: unit{} was handed over with translation on; it stays on while \ - this kernel's root replaces firmware's", - self.index - ); - continue; - } - InFlight::Identical => log!( - "iommu: unit{} was handed over with translation on; it goes off first: its \ - root table is not in legacy mode", - self.index - ), - InFlight::Unknown(why) => log!( - "iommu: unit{} was handed over with translation on; it goes off first: {why}", - self.index - ), - } - self.command(bit, false, what); - continue; - } log!("iommu: unit{} was handed over with {what} on; it goes off first", self.index); if bit == QUEUED_INVALIDATION_ENABLE { // §6.5.2: a queue is switched off only once it is empty and its @@ -482,7 +482,7 @@ fn describe_unit(index: usize, drhd: &dmar::Drhd) -> Option { log!( "iommu: unit{index} @{base:#x} seg={} pci_all={} ver={}.{} cap={:#018x} ecap={:#018x} \ gsts={:#010x} aw={} sagaw={:#04x} mgaw={} nd={} sps2m={} cm={} psi={} nfr={} fro={:#x} \ - qi={} ir={} eim={} pt={} coherent={} sc={}", + qi={} ir={} eim={} pt={} coherent={} sc={} pmr={}", drhd.segment(), yn(drhd.include_pci_all()), (version >> 4) & 0xF, @@ -509,10 +509,11 @@ fn describe_unit(index: usize, drhd: &dmar::Drhd) -> Option { yn(caps.passthrough()), yn(caps.coherent()), yn(caps.snoop_control()), + yn(caps.protected_memory()), ); describe_scopes("unit", index, drhd.scopes()); - Some(Unit { index, base, regs, caps }) + Some(Unit { index, base, regs, caps, major: (version >> 4) & 0xF }) } /// What a unit's capabilities let this kernel program, or `None` with the @@ -567,65 +568,70 @@ fn plan(unit: &Unit) -> Option { ); return None; } + // The hand-over invalidates through these registers (`Unit::invalidate_through_registers`). + if unit.major >= 6 { + log!( + "iommu: unit{index} is version {}, which refuses register-based invalidation (§11.4.6.1) \ + and so the hand-over's — not programmed", + unit.major + ); + return None; + } + if unit.caps.iotlb_registers() + 16 > REGISTER_WINDOW { + log!( + "iommu: unit{index} puts its IOTLB registers at {:#x}, past its own {REGISTER_WINDOW}-byte \ + window (ECAP={:#018x}) — not programmed", + unit.caps.iotlb_registers(), + unit.caps.ecap + ); + return None; + } Some(Plan { width, records }) } -/// Programs the unit in its own order — queue, root pointer and the global -/// invalidation §6.6 owes it, interrupt remap pointer, then `TE` — each -/// confirmed in `GSTS` before the next. -fn enable( +/// A unit translating through this kernel's root, its queue and interrupt +/// remapping not yet programmed. +struct Translating { unit: Unit, - plan: Plan, - devices: &[PciDevice], - domains: &mut [Option
; 2], - remap: Option, - in_flight: InFlight, -) { - let index = unit.index; - let Plan { width, records } = plan; - // Before any table is built: what a domain of a driver's own can be is the - // narrowest thing every translating unit on this machine agrees to. - domain::unit_agrees(width, unit.caps.domains(), unit.caps.mgaw()); + width: AddressWidth, + root: Table, +} - let domain = *domains[domain_slot(width)].get_or_insert_with(|| { - let top = crate::mm::pmm::top(); - let (domain, frames) = table::identity_domain(&mut TABLES.lock(), width, top); - log!("iommu: identity domain aw={} covers 0x0..{top:#x} in {frames} 2 MiB leaves", width.bits()); - domain - }); - #[cfg(feature = "boot-actuators")] - if crate::actuator::iommu_firmware_left() { - unit.leave_on(devices, domain, width); - } +/// Whatever firmware left on, off; then this kernel's root pointed, the global +/// invalidation §6.6 owes it, and translation on. The invalidation goes through +/// the unit's registers, since the queue's memory is no remapping structure +/// (Appendix A) and may lie under a protected region. +fn translate(unit: Unit, plan: Plan, devices: &[PciDevice], domains: &mut [Option
; 2]) -> Translating { + let Plan { width, records } = plan; + // Built before `TABLES` is held here: building it takes that lock too. + let domain = identity_domain(domains, width); let root = identity_root(&mut TABLES.lock(), devices, domain, width); + // After the tables are built, so a unit firmware left translating passes + // DMA untranslated only while it is programmed. + unit.hand_over(); + // Before `TE`: the first blocked transaction must be reportable, not merely counted. + fault::arm(unit.index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); + unit.regs.write_u64(RTADDR_REG, root.phys()); + unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); + unit.invalidate_through_registers(); + unit.command(TRANSLATION_ENABLE, true, "translation"); + Translating { unit, width, root } +} - // After the tables are built, so a unit whose translation goes off passes - // DMA untranslated only while it is programmed - // (`issues/a-unit-left-translating-passes-dma-untranslated-while-programmed.md`), - // and before the queue is pointed, which §6.5.2 does only with it off. - unit.hand_over(in_flight); +/// The invalidation queue and interrupt remapping, each confirmed in `GSTS` +/// before the next. +fn finish(translating: Translating, devices: &[PciDevice], remap: Option) { + let Translating { unit, width, root } = translating; + let index = unit.index; 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); - - // Before `TE`: the first blocked transaction must be reportable, not merely counted. - fault::arm(index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); - unit.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); - unit.regs.write_u64(RTADDR_REG, root.phys()); - unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - // At once on a unit still translating: until it completes, its walks may - // mix firmware's cached entries with this root's (§6.6). - queue.invalidate_all(unit.regs); if let Some(irta) = irta { unit.regs.write_u64(interrupt::IRTA_REG, irta); unit.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); queue.invalidate_interrupts(unit.regs); - } - - unit.command(TRANSLATION_ENABLE, true, "translation"); - if remap.is_some() { unit.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); } // After `IRE`, so nothing is invalidated on a unit that is not yet reading @@ -682,9 +688,6 @@ fn describe_scopes(owner: &str, index: usize, scopes: Scopes) { } } -/// Device scope type 1, a PCI endpoint: the requester is the function named. -const SCOPE_ENDPOINT: u8 = 1; - /// Device scope type 3, whose enumeration id is an I/O APIC's MADT id. const SCOPE_IOAPIC: u8 = 3; @@ -767,6 +770,16 @@ impl Capabilities { self.cap & (1 << 39) != 0 } + /// `CAP.PLMR` and `CAP.PHMR` (bits 5 and 6): both protected memory regions. + fn protected_memory(&self) -> bool { + self.cap & (0b11 << 5) == 0b11 << 5 + } + + /// `ECAP.IRO`, in 16-byte units: where the IOTLB registers start. + fn iotlb_registers(&self) -> u64 { + ((self.ecap >> 8) & 0x3FF) * 16 + } + /// `CAP.NFR`, encoded one less than it is. fn fault_recording_registers(&self) -> u32 { (((self.cap >> 40) & 0xFF) + 1) as u32 diff --git a/kernel/src/arch/x86_64/vtd/pmr.rs b/kernel/src/arch/x86_64/vtd/pmr.rs new file mode 100644 index 00000000000..7e0d6a5f8b0 --- /dev/null +++ b/kernel/src/arch/x86_64/vtd/pmr.rs @@ -0,0 +1,126 @@ +//! Protected memory regions (VT-d §11.4.8): while a unit's translation is off, +//! it blocks DMA to them, and a unit's own reads of its remapping structures +//! are exempt (§11.4.8.1, `EPM`). They hold while every unit is handed over, +//! so a unit with translation off passes no DMA to the memory they cover. +//! +//! The same regions go on every unit, as §11.4.8.1 requires for them to +//! protect anything. They avoid every RMRR, so firmware's own DMA to its +//! reserved memory runs on through the hand-over. + +use alloc::vec::Vec; + +use crate::log; +use crate::mm::Mmio; +use crate::time::{Duration, Tripwire}; +use kernel::dmaguard::{self, Span}; + +/// Below 16 MiB an Intel host bridge may decode legacy ranges and the optional +/// ISA hole as MMIO (11th Generation Intel Core datasheet, 631122 §2.5.1), and +/// no protected region may cover MMIO (§11.4.8.1). +const LOW_FLOOR: u64 = 16 << 20; + +const PMEN_REG: u64 = 0x64; +const PLMBASE_REG: u64 = 0x68; +const PLMLIMIT_REG: u64 = 0x6C; +const PHMBASE_REG: u64 = 0x70; +const PHMLIMIT_REG: u64 = 0x78; + +/// `PMEN.EPM`, and `PMEN.PRS` reporting it in force. +const ENABLE: u32 = 1 << 31; +const STATUS: u32 = 1 << 0; + +const STATUS_TIMEOUT: Tripwire = Tripwire::absurd( + Duration::from_secs(1), + "a unit whose protected regions never report their state is one whose reach nothing can state", +); + +/// Every unit's protected regions, programmed and not yet on. +pub struct Guard { + units: Vec<(usize, Mmio)>, +} + +/// Programs the same regions on every unit in `units`, each `(index, registers, +/// supported)`, or says why it cannot. `ram` is firmware's RAM, `avoid` +/// every RMRR, `top` the highest memory the identity domain maps. +pub fn program(units: &[(usize, Mmio, bool)], ram: &[(u64, u64)], avoid: &[(u64, u64)], top: u64) -> Option { + if let Some(&(index, _, _)) = units.iter().find(|(_, _, supported)| !supported) { + log!( + "iommu: unit{index} has no protected memory regions (CAP.PLMR or CAP.PHMR clear), so while \ + the units are handed over every function behind one reaches all of memory" + ); + return None; + } + // The coarsest alignment any unit decodes, so one pair of regions suits them all. + let mut low_align = 1u64; + let mut high_align = 1u64; + for &(index, regs, _) in units { + regs.write_u32(PLMBASE_REG, u32::MAX); + regs.write_u64(PHMBASE_REG, u64::MAX); + let (low, high) = (regs.read_u32(PLMBASE_REG), regs.read_u64(PHMBASE_REG)); + if low == 0 || high == 0 { + log!("iommu: unit{index} decodes no protected region base (PLMBASE={low:#x} PHMBASE={high:#x})"); + return None; + } + low_align = low_align.max(1 << low.trailing_zeros()); + high_align = high_align.max(1 << high.trailing_zeros()); + } + let low = dmaguard::largest(ram, LOW_FLOOR, dmaguard::FOUR_GIB, avoid, low_align); + let high = dmaguard::largest(ram, dmaguard::FOUR_GIB, top, avoid, high_align); + log!( + "iommu: protected memory for the hand-over on {} unit(s): low {} high {}, aligned to {low_align:#x} and {high_align:#x}", + units.len(), + Shown(low), + Shown(high), + ); + for &(_, regs, _) in units { + // A limit below its base is a region switched off (§11.4.8.3, §11.4.8.5). + let (base, limit) = low.map_or((low_align, 0), |s| (s.base, s.end - 1)); + regs.write_u32(PLMBASE_REG, base as u32); + regs.write_u32(PLMLIMIT_REG, limit as u32); + let (base, limit) = high.map_or((high_align, 0), |s| (s.base, s.end - 1)); + regs.write_u64(PHMBASE_REG, base); + regs.write_u64(PHMLIMIT_REG, limit); + } + Some(Guard { units: units.iter().map(|&(index, regs, _)| (index, regs)).collect() }) +} + +impl Guard { + pub fn arm(&self) { + for &(index, regs) in &self.units { + set(index, regs, true); + } + } + + pub fn release(self) { + for &(index, regs) in &self.units { + set(index, regs, false); + } + log!("iommu: protected memory released on {} unit(s)", self.units.len()); + } +} + +/// §11.4.8.1: `EPM` written, then `PRS` waited for before it changes again. +fn set(index: usize, regs: Mmio, on: bool) { + regs.write_u32(PMEN_REG, if on { ENABLE } else { 0 }); + let deadline = crate::clock::nanos_since_boot() + STATUS_TIMEOUT.nanos(); + while (regs.read_u32(PMEN_REG) & STATUS != 0) != on { + assert!( + crate::clock::nanos_since_boot() < deadline, + "iommu: unit{index}'s protected regions never reported {}, PMEN={:#010x}", + if on { "on" } else { "off" }, + regs.read_u32(PMEN_REG) + ); + core::hint::spin_loop(); + } +} + +struct Shown(Option); + +impl core::fmt::Display for Shown { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self.0 { + Some(s) => write!(f, "{:#x}..{:#x}", s.base, s.end), + None => f.write_str("none"), + } + } +} diff --git a/kernel/src/arch/x86_64/vtd/queue.rs b/kernel/src/arch/x86_64/vtd/queue.rs index 1c4c106ade4..9737347e5c5 100644 --- a/kernel/src/arch/x86_64/vtd/queue.rs +++ b/kernel/src/arch/x86_64/vtd/queue.rs @@ -1,9 +1,9 @@ //! Queued invalidation: submits a descriptor and an Invalidation Wait //! descriptor, then polls the wait's status write to completion. //! Acknowledgement timeout is a panic — the kernel cannot say what a device -//! can reach past it. Only the queued-invalidation path is implemented: every -//! unit reachable here has it, so a `CCMD_REG`/`IOTLB_REG` fallback would be -//! dead code; a unit without `ECAP.QI` is left unprogrammed. +//! can reach past it. Every invalidation after the hand-over goes through +//! here; the hand-over's own goes through the unit's registers, before the +//! queue exists. A unit without `ECAP.QI` is left unprogrammed. use crate::mm::Mmio; use crate::time::{Duration, Tripwire}; @@ -20,9 +20,7 @@ const IOTLB: u64 = 0x2; const INTERRUPT_ENTRY_CACHE: u64 = 0x4; const WAIT: u64 = 0x5; -/// Granularity field, bits 5:4: `01` is global — every domain, every entry. -const GLOBAL: u64 = 1 << 4; -/// `10`: everything cached for one domain id, whatever address or requester it came from. +/// Granularity field, bits 5:4, `10`: everything cached for one domain id, whatever address or requester it came from. const DOMAIN: u64 = 2 << 4; /// `11`, context cache only: one requester id, Section 6.5.2.1. const DEVICE: u64 = 3 << 4; @@ -64,18 +62,6 @@ impl Queue { queue } - /// Every cached translation and context entry, gone, acknowledged by the - /// unit before this returns. - pub fn invalidate_all(&mut self, regs: Mmio) { - self.submit( - regs, - &[ - (CONTEXT_CACHE | GLOBAL, 0), - (IOTLB | GLOBAL | DRAIN_WRITES | DRAIN_READS, 0), - ], - ); - } - /// Every translation cached for one domain, gone. Owed after any change to /// its second-level tables, a mapping becoming present included, because /// `CAP.CM` says the unit may have cached the absence. diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index d0789965c55..9dca9463103 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -4,8 +4,8 @@ //! over behind the unit and refused where no unit is. //! `amdvi_firmware_left`: whether an AMD-Vi unit firmware left blocking is //! switched off before the boot stick is driven. -//! `vtd_firmware_left`: whether a VT-d unit firmware left translating under -//! a DMAR that declares nothing in flight goes off before it is programmed. +//! `vtd_firmware_left`: whether a VT-d unit firmware left translating, with no +//! protected memory regions, goes off before it is programmed. use std::collections::{BTreeMap, BTreeSet}; use std::path::Path; @@ -347,32 +347,29 @@ pub(crate) fn amdvi_left_on(unit: &str) -> [String; 3] { .map(|field| format!("iommu: amdvi {unit} was handed over with {field} on; it goes off")) } -/// **A VT-d unit left translating where firmware declares nothing about its -/// DMA goes off first, and is programmed.** QEMU's DMAR leaves -/// `DMA_CTRL_PLATFORM_OPT_IN` clear, so this is the hand-over the T14 no -/// longer takes: its firmware declares the flag and keeps translation on -/// (`iommu_firmware_left`'s row). Under `iommu-firmware-left` the unit is -/// left queueing, translating and remapping; each goes off by its own write, -/// translation with the reason, and the unit ends translating through this -/// kernel's root with the boot stick's DMA behind it. +/// **A VT-d unit left translating, on a unit with no protected memory +/// regions, goes off first and is programmed.** QEMU's unit reports +/// `CAP.PLMR` and `CAP.PHMR` clear, so this is the hand-over without the guard +/// the T14 takes (`iommu_firmware_left`'s row), logged as such. Under +/// `iommu-firmware-left` the unit is left queueing, translating and remapping; +/// each goes off by its own write, the root is set and invalidated through +/// the unit's registers, and the unit ends translating with the boot stick's +/// DMA behind it. pub fn vtd_firmware_left(test_config: &Path) -> Result<(), String> { let options = BootOptions { profile: Profile::Metal, kernel_params: &["iommu-firmware-left"], ..Default::default() }; let qemu = QemuInstance::boot_with_options(test_config, &[], &[], options); let log = Serial::boot(&qemu); log.must_be_clean()?; - log.must_say("dma_ctrl_opt_in=n")?; - let off = log.must_say( - "iommu: unit0 was handed over with translation on; it goes off first: firmware does not declare \ - its DMA confined to its reserved regions", - )?; + let unguarded = log.must_say("iommu: unit0 has no protected memory regions")?; + let off = log.must_say("iommu: unit0 was handed over with translation on; it goes off first")?; log.must_say("iommu: unit0 was handed over with queued invalidation on; it goes off first")?; - log.must_not_say("it stays on while")?; + log.must_not_say("iommu: protected memory for the hand-over")?; let translating = log.must_say(" translating gsts=")?; if !translating.contains(" tes=y ") { return Err(format!("unit0 was programmed and is not translating: {translating:?}")); } log.must_say("Boot: complete")?; - eprintln!(" [vtd-firmware-left] {}; {}", off.trim(), translating.trim()); + eprintln!(" [vtd-firmware-left] {}; {}; {}", unguarded.trim(), off.trim(), translating.trim()); Ok(()) } diff --git a/tests/toyos.rs b/tests/toyos.rs index 8827db569d7..6a11590d55e 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -482,8 +482,8 @@ const MACHINE_TESTS: &[&str] = &[ // DMA it lets through are no host test's, and the AMD bench is staged by // hand rather than by CI. "amdvi_firmware_left", - // A VT-d unit left translating where the DMAR declares nothing in flight: - // the T14's declares it, so only QEMU's unit takes this hand-over. + // A VT-d unit left translating, with no protected memory regions to guard + // its hand-over: the T14's units have them, so only QEMU's takes this one. "vtd_firmware_left", // A translating AMD-Vi unit's refusal, recorded and read: a kernel-owned // record halts the machine, so no metal row reaches it, and only QEMU's @@ -5982,14 +5982,13 @@ fn sysret_ss(log: &str) -> Result<(), String> { /// Every unit this kernel went on to program, and every AMD-Vi unit it /// switched off, said it was handed over with what the actuator left on; and -/// where firmware declares its DMA confined to reserved regions this kernel's -/// root translates alike, as the T14's does, every VT-d unit kept translating. +/// where every VT-d unit has protected memory regions, as the T14's do, they +/// went on before the hand-over and off after it. /// /// An AMD-Vi unit's lines are on the boot stick only because the switch-off /// let the stick's DMA through the unit the actuator left blocking it. fn iommu_firmware_left(log: &str) -> Result<(), String> { - let declared = log.contains(" dma_ctrl_opt_in=y") - && !log.lines().any(|l| l.contains("iommu: rmrr") && (l.contains(" OUTSIDE the identity domain") || l.contains("bridge walk"))); + let guarded = log.lines().filter(|l| l.contains(" cap=") && l.contains("iommu: unit")).all(|l| l.contains(" pmr=y")); let mut units = 0; for line in log.lines().filter(|l| l.contains(" translating gsts=")) { let unit = line @@ -6014,12 +6013,6 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { )); } } - if declared && !log.contains(&format!("iommu: {unit} was handed over with translation on; it stays on")) { - return Err(format!( - "firmware declares its DMA confined to reserved regions this kernel's root \ - translates alike, and {unit} still took translation off:\n{log}" - )); - } units += 1; } let amdvi = log @@ -6044,11 +6037,20 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { if units == 0 { return Err(format!("no unit was programmed or switched off, so nothing was handed over:\n{log}")); } + let armed = log.lines().position(|l| l.contains("iommu: protected memory for the hand-over on")); + let released = log.lines().position(|l| l.contains("iommu: protected memory released on")); + let first_off = log.lines().position(|l| l.contains("was handed over with translation on; it goes off first")); + if guarded && !matches!((armed, first_off, released), (Some(a), Some(o), Some(r)) if a < o && o < r) { + return Err(format!( + "every unit has protected memory regions, and the hand-over did not take translation \ + off between their going on and their release:\n{log}" + )); + } let passed = log.matches("handed over with compatibility-format pass-through on").count(); - let kept = log.matches("handed over with translation on; it stays on").count(); eprintln!( " [iommu-firmware-left] {units} unit(s) switched off what they were handed over with, \ - {kept} of them keeping translation on; {passed} of them reported compatibility format passed" + {} protected memory; {passed} of them reported compatibility format passed", + if guarded { "inside" } else { "without" } ); Ok(()) } From c6a9c9970ff40701fc6cf2f58c5f11c7bd79664f Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 28 Oct 2026 14:35:26 +0100 Subject: [PATCH 4/4] vtd: review r3, the regions hold the hand-over's tables and the kernel, and a unit the window cannot serve is unguarded rather than refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit B1: the identity domains and every root are built before the regions are chosen. Each region is the largest stretch of the OS's RAM, clear of the RMRRs, that holds every 2 MiB page those tables were carved from and this kernel's image, which iommu::init now takes. The boot logs what the regions cover. Where they cannot hold all of it, it logs the hand-over unguarded and does not arm them. The RAM is usable or ACPI-reclaimable, never NVS, and the 16 MiB floor goes: the map marks the legacy ranges as non-RAM. B2: plan refuses no unit for the hand-over's sake. A unit that cannot take register-based invalidation (major version 6 or later, or IOTLB registers outside the window) makes the machine's hand-over unguarded, and an unguarded hand-over invalidates through the queue, enabled before translation comes on, as main does. leave_on is main's again. B3: protected memory firmware left on goes off before anything is probed or programmed (§11.4.8.2 to §11.4.8.5). B4: the step interpreter and its tests go; init runs arm, every translate, release and every finish in sequence, the units moving through by value. largest keeps its tests, now with the ranges a region must hold. B5: Guard::arm logs, and the judge requires that line before translation first goes off and the release before the first unit is programmed. It treats a machine as guarded only when it has VT-d unit lines, so an AMD-Vi machine is not. Judged on an r3 T14 boot, which armed without that line, it is red. B6 and B7: comments and records. The merged issue is renamed to what it holds, a unit passing DMA untranslated outside its regions, and its exit names its four cases. It records that a machine whose firmware leaves translation off is open to every function from ExitBootServices until the regions go on. The iGPU issue labels its rate an estimate. Suggestions taken: the limits are probed with the bases (S1); a hand-over with nothing covered or fewer units programmed than described is unguarded (S2, S3); identity_domain takes the held tables, so the self-deadlock cannot be written (S4); the RAM set as above (S5); the functions mastering the bus are logged as the regions go on (S7). The orchestrator's ruling: the iGPU fault stays an open defect only the actuator reaches. Its row rides a boot of its own, iommu-firmware-left, so a halt costs only that boot. Co-Authored-By: Claude Opus 5.5 --- ...-memory-regions-while-it-is-handed-over.md | 74 +++++++ ...not-program-keeps-what-firmware-left-on.md | 7 +- ...ma-untranslated-while-it-is-handed-over.md | 60 ------ issues/the-iommu-refuses-nothing-yet.md | 2 +- ...ation-goes-off-and-on-under-its-scanout.md | 10 +- kernel/pure/dmaguard.rs | 136 ++++++------- kernel/src/arch/aarch64/smmu/mod.rs | 8 +- kernel/src/arch/x86_64/iommu_unit.rs | 4 +- kernel/src/arch/x86_64/vtd/mod.rs | 182 +++++++++++------- kernel/src/arch/x86_64/vtd/pmr.rs | 122 ++++++++---- kernel/src/arch/x86_64/vtd/queue.rs | 22 ++- kernel/src/arch/x86_64/vtd/table.rs | 8 + kernel/src/iommu/mod.rs | 5 +- kernel/src/main.rs | 7 +- lib/bootmap/src/lib.rs | 2 +- tests/common/iommu.rs | 7 +- tests/toyos.rs | 33 +++- 17 files changed, 418 insertions(+), 271 deletions(-) create mode 100644 issues/a-unit-passes-dma-untranslated-outside-its-protected-memory-regions-while-it-is-handed-over.md delete mode 100644 issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md diff --git a/issues/a-unit-passes-dma-untranslated-outside-its-protected-memory-regions-while-it-is-handed-over.md b/issues/a-unit-passes-dma-untranslated-outside-its-protected-memory-regions-while-it-is-handed-over.md new file mode 100644 index 00000000000..bc198747115 --- /dev/null +++ b/issues/a-unit-passes-dma-untranslated-outside-its-protected-memory-regions-while-it-is-handed-over.md @@ -0,0 +1,74 @@ +--- +status: open +kind: defect +opened: 2026-10-04 +--- + +# A unit passes DMA untranslated outside its protected memory regions while it is handed over + +`vtd::init` (`kernel/src/arch/x86_64/vtd/mod.rs`) hands every unit over with +translation off. That covers a unit firmware left translating, whatever its +table mode. VT-d Rev. 4.1 gives no condition under which system software may +switch firmware's tables for its own under translation. §6.6 requires +identical remapping results, and nothing documented lets this kernel know +firmware's mappings. `DMA_CTRL_PLATFORM_OPT_IN` (§8.1) says only where +platform DMA goes. + +The hand-over is guarded where every described unit is programmed, has both +protected memory regions (`CAP.PLMR`, `CAP.PHMR`) and takes register-based +invalidation. On such units the regions block DMA to what they cover whether +or not the unit translates (§11.4.8.1). They are programmed on every unit +after any firmware left on is switched off, and released once the last unit +translates (`vtd/pmr.rs`). The low region and the high region are each the +largest stretch of the OS's RAM (usable or ACPI-reclaimable, never NVS) that +avoids every RMRR and holds every 2 MiB page the hand-over's tables were +carved from, and this kernel's image. The boot logs the regions, what they +cover and the functions mastering the bus as they go on. The T14's four units +are guarded. The queue and interrupt remapping are programmed after the +release. + +What passes untranslated during the hand-over: + +- **A machine whose hand-over is unguarded:** all of memory. Its log says why: + - a unit without both regions (QEMU's); + - a unit that refuses register-based invalidation (major version 6 or later, + §11.4.6.1), or whose IOTLB registers lie outside the window this kernel + maps; + - fewer units programmed than described; + - tables or kernel outside every region. + + Such a machine invalidates through the queue, enabled before translation + comes on. +- **A guarded machine:** + - RAM outside the two chosen stretches; + - every RMRR; + - peer MMIO. +- **Before the hand-over, on any machine:** + - Where firmware leaves translation off, as the T14's does, every function + reaches all of memory from ExitBootServices until the regions go on. + - Where firmware leaves it on, `mm::init` has already taken + `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, + `is_usable_type`). A unit may walk firmware's tables in memory the pmm has + handed out. + +Evidence: +- No machine here is handed a unit translating. +- `iommu-firmware-left` stages one: + - QEMU 11.1.1 runs the unguarded hand-over (`vtd_firmware_left`); + - the T14 runs the guarded one (`iommu_firmware_left`'s row, which reads + the regions go on before translation first goes off and come off before + the first unit is programmed). + +Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). + +**Exit**: during the hand-over no function reaches memory outside its RMRRs. +Each of these cases is closed by a documented mechanism and read on a +machine or a host test: + +1. a unit without protected memory regions. §11.4.8.1 marks the regions + "planned for deprecation", so new units will be this case. Abort-DMA mode + (§3.4.4) is one mechanism; it needs `ECAP.ADMS` and is ruled out while a + device an RMRR names masters the bus; +2. a unit that refuses register-based invalidation; +3. RAM a guarded hand-over leaves outside its regions; +4. peer MMIO. diff --git a/issues/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md b/issues/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md index f95ecaf94d1..c4bc8d37375 100644 --- a/issues/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md +++ b/issues/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md @@ -7,11 +7,12 @@ opened: 2026-10-04 # A unit this kernel does not program keeps what firmware left on `vtd::init` (`kernel/src/arch/x86_64/vtd/mod.rs`) switches off what firmware -left on (`Unit::hand_over`) only inside `enable`, which a unit reaches only if -`plan` accepts it and it is among the first `MAX_UNITS` described. A unit +left on (`Unit::hand_over`) only in a unit's hand-over (`translate`), which a +unit reaches only if `plan` accepts it and it is among the first `MAX_UNITS` described. A unit `plan` refuses, and every unit past `MAX_UNITS`, keeps whatever firmware left on while the kernel treats it as absent: translation through firmware's tables, -and interrupt remapping, which blocks every compatibility-format message this +protected memory regions (`vtd/pmr.rs` switches them off on programmed units +only), and interrupt remapping, which blocks every compatibility-format message this kernel then sends through the unit unless firmware also left `CFI` on with `EIME` off (VT-d Rev. 4.1 §5.1.4). The module header's "leaves the unit switched off" is false of such a unit. diff --git a/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md b/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md deleted file mode 100644 index ac5de0d678d..00000000000 --- a/issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md +++ /dev/null @@ -1,60 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-04 ---- - -# A unit without protected memory regions passes DMA untranslated while it is handed over - -`vtd::init` (`kernel/src/arch/x86_64/vtd/mod.rs`) hands every unit over with -translation off. That covers a unit firmware left translating, whatever its -table mode. VT-d Rev. 4.1 gives no condition under which system software may -switch firmware's tables for its own under translation. §6.6 requires -identical remapping results, and nothing documented lets this kernel know -firmware's mappings. `DMA_CTRL_PLATFORM_OPT_IN` (§8.1) restricts only where -platform DMA goes. - -While translation is off, a unit's protected memory regions (§11.4.8) block -DMA to the memory they cover. Where every unit reports `CAP.PLMR` and -`CAP.PHMR`, as the T14's four do, `vtd/pmr.rs` programs one pair of regions -on all of them before the first goes off, and releases them once the last is -translating. The pair is the largest stretch of firmware's RAM below 4 GiB, -from 16 MiB, and the largest at or above it, each clear of every RMRR -(`kernel::dmaguard`). Register-based invalidation completes the hand-over -inside them, since the invalidation queue's memory is no remapping structure -(Appendix A). - -What still passes untranslated: - -- **On a unit without both regions, all of memory.** QEMU's unit is one. The - boot logs `has no protected memory regions`. -- **On the T14, outside the pair:** - - RAM below 16 MiB; - - RAM in a stretch the map breaks with a reserved or runtime entry; - - every RMRR, the iGPU's stolen memory among them; - - peer MMIO. -- **Before the hand-over.** `mm::init` runs before `iommu::init` and takes - `EfiBootServicesData` as free (`lib/bootmap/src/lib.rs`, `is_usable_type`). - A unit left translating may walk firmware's tables in memory the pmm has - already handed out. - -A unit of major version 6 or later refuses register-based invalidation -(§11.4.6.1), so `plan` leaves it unprogrammed. No machine here has one. - -Evidence: -- No machine here is handed a unit translating; the T14's four read - `gsts=0x40000000`. -- `iommu-firmware-left` stages one. QEMU 11.1.1 runs the unguarded hand-over - (`vtd_firmware_left`). The T14 runs the guarded one (`iommu_firmware_left`'s - row, which reads the arm before translation goes off and the release after). - -Owner: the IOMMU track (`issues/the-iommu-refuses-nothing-yet.md`). - -**Exit**: no unit passes DMA to memory outside its RMRRs while it is handed -over. - -- §11.4.8.1 marks the protected regions "planned for deprecation". A unit - without them needs another documented mechanism. Abort-DMA mode (§3.4.4) - is one: it needs `ECAP.ADMS`, which the T14 lacks, and §3.4.4 rules it out - while a device an RMRR names is mastering the bus. -- A host test or a machine reads each case. diff --git a/issues/the-iommu-refuses-nothing-yet.md b/issues/the-iommu-refuses-nothing-yet.md index 73c27ccb78c..f4d2558dbc8 100644 --- a/issues/the-iommu-refuses-nothing-yet.md +++ b/issues/the-iommu-refuses-nothing-yet.md @@ -54,7 +54,7 @@ a unit firmware left translating: off for that one switch. The older protection registers are `PMEN`'s protected memory regions (§11.4.8.1). Every hand-over now takes translation off inside them where every unit has them; what remains is - `issues/a-unit-without-protected-memory-regions-passes-dma-untranslated-while-it-is-handed-over.md`. + `issues/a-unit-passes-dma-untranslated-outside-its-protected-memory-regions-while-it-is-handed-over.md`. - **"Apply it at hand-over"**, on reserved memory: "From the hand-over on, only display and USB controllers keep access to their reserved region; any other device's access is refused. A device that firmware was still using diff --git a/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md b/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md index 074ddd3e13e..13c34473d0f 100644 --- a/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md +++ b/issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md @@ -78,9 +78,13 @@ Neither the tables nor any cached state VT-d Rev. 4.1 lets the unit keep is what refuses. Intel's 11th Generation Core specification update (631123-034, errata TGL001 to TGL075) documents no erratum for VT-d or the iGPU's DMA. -Every hand-over still takes translation off. Under `iommu-firmware-left` the -T14 therefore still reaches this, in about one boot in 27. Real T14 firmware -leaves translation off, so no other boot does. +Every hand-over still takes translation off, now inside protected memory +regions, so `iommu-firmware-left` still reaches this on the T14. Real T14 +firmware leaves translation off, so no other boot does. How often a boot of +today's sequence halts is not measured. The estimate is between the cycles' +one in 27, a different sequence with no regions, and the pre-#1086 boots' 1 +in 46. The row that arms the actuator rides a boot of its own, so a halt costs +only that boot. **Exit**: this kernel's hand-over sequence is fixed so that the measurement's off-on cycles fault no more than its switch cycles, or a documented erratum diff --git a/kernel/pure/dmaguard.rs b/kernel/pure/dmaguard.rs index b4bd1d44ef0..77a531a7b44 100644 --- a/kernel/pure/dmaguard.rs +++ b/kernel/pure/dmaguard.rs @@ -1,11 +1,4 @@ -//! What a remapping unit's protected memory regions cover while every unit is -//! handed over, and the order the hand-over takes. -//! -//! A unit has one low region, below 4 GiB, and one high region. Each is the -//! largest stretch of memory the firmware map calls RAM, with no reserved -//! region in it and no gap, since a gap may decode as MMIO, above a floor the -//! caller's platform sets. Each is shrunk to -//! the units' alignment, so what is protected is never more than that. +//! Where a remapping unit's protected memory regions go while it is handed over. #![forbid(unsafe_code)] @@ -20,10 +13,24 @@ pub struct Span { pub end: u64, } +impl Span { + pub fn holds(self, (start, end): (u64, u64)) -> bool { + self.base <= start && end <= self.end + } +} + /// The largest part of `ram` inside `[lo, hi)` that touches no `avoid` range -/// and no gap between `ram`'s ranges, with its base rounded up and its end -/// rounded down to `align` (a power of two). `None` if nothing is left. -pub fn largest(ram: &[(u64, u64)], lo: u64, hi: u64, avoid: &[(u64, u64)], align: u64) -> Option { +/// and spans no gap between `ram`'s ranges, with its base rounded up and its +/// end rounded down to `align` (a power of two), among the parts holding every +/// `within` range that starts inside `[lo, hi)`. `None` if no part does. +pub fn largest( + ram: &[(u64, u64)], + lo: u64, + hi: u64, + avoid: &[(u64, u64)], + align: u64, + within: &[(u64, u64)], +) -> Option { assert!(align.is_power_of_two(), "a protected region's alignment {align:#x} is not a power of two"); let mut ram: Vec<(u64, u64)> = ram.iter().copied().filter(|&(s, e)| s < e).collect(); ram.sort_unstable(); @@ -37,6 +44,7 @@ pub fn largest(ram: &[(u64, u64)], lo: u64, hi: u64, avoid: &[(u64, u64)], align } let mut avoid: Vec<(u64, u64)> = avoid.iter().copied().filter(|&(s, e)| s < e).collect(); avoid.sort_unstable(); + let needed: Vec<(u64, u64)> = within.iter().copied().filter(|&(s, _)| lo <= s && s < hi).collect(); let mut best: Option = None; for (start, end) in runs { let mut piece = start.max(lo); @@ -49,8 +57,12 @@ pub fn largest(ram: &[(u64, u64)], lo: u64, hi: u64, avoid: &[(u64, u64)], align if cut > piece { let base = piece.div_ceil(align).saturating_mul(align); let end = cut / align * align; - if base < end && best.is_none_or(|b| end - base > b.end - b.base) { - best = Some(Span { base, end }); + let span = Span { base, end }; + if base < end + && needed.iter().all(|&r| span.holds(r)) + && best.is_none_or(|b| end - base > b.end - b.base) + { + best = Some(span); } } if b >= stop { @@ -62,35 +74,6 @@ pub fn largest(ram: &[(u64, u64)], lo: u64, hi: u64, avoid: &[(u64, u64)], align best } -/// One step of handing every unit over, units named by index. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum Step { - /// Every unit's protected regions on. - Arm, - /// Translation off, this kernel's root pointed and invalidated through the - /// unit's registers, translation on. - Translate(usize), - /// Every unit's protected regions off. - Release, - /// The invalidation queue and interrupt remapping, which need memory the - /// regions may cover. - Finish(usize), -} - -/// The hand-over of `units` units, between `Arm` and `Release` where `guarded`. -pub fn order(units: usize, guarded: bool) -> Vec { - let mut steps = Vec::with_capacity(2 * units + 2); - if guarded { - steps.push(Step::Arm); - } - steps.extend((0..units).map(Step::Translate)); - if guarded { - steps.push(Step::Release); - } - steps.extend((0..units).map(Step::Finish)); - steps -} - #[cfg(test)] mod tests { use super::*; @@ -98,43 +81,17 @@ mod tests { const MIB: u64 = 1 << 20; - #[test] - fn every_translation_happens_inside_the_guard_and_nothing_needs_memory_until_it_is_gone() { - for units in 0..5 { - let steps = order(units, true); - let at = |s: Step| steps.iter().position(|&t| t == s).unwrap(); - let arm = at(Step::Arm); - let release = at(Step::Release); - for unit in 0..units { - assert!(arm < at(Step::Translate(unit)), "{steps:?}"); - assert!(at(Step::Translate(unit)) < release, "{steps:?}"); - assert!(release < at(Step::Finish(unit)), "{steps:?}"); - } - assert_eq!(steps.len(), 2 * units + 2); - } - } - - #[test] - fn an_unguarded_hand_over_still_translates_every_unit_before_finishing_any() { - let steps = order(3, false); - assert_eq!(steps.iter().filter(|s| matches!(s, Step::Arm | Step::Release)).count(), 0); - let last_translate = steps.iter().rposition(|s| matches!(s, Step::Translate(_))).unwrap(); - let first_finish = steps.iter().position(|s| matches!(s, Step::Finish(_))).unwrap(); - assert!(last_translate < first_finish); - } - - /// The T14's shape: RAM up to its graphics region, which an RMRR names, - /// and RAM again above 4 GiB. + /// RAM up to a graphics region an RMRR names, and RAM again above 4 GiB. #[test] fn a_region_stops_at_the_reserved_region_and_at_4_gib() { let ram = [(0x10_0000, 0x9c00_0000), (0x1_0000_0000, 0x4_5f80_0000)]; let avoid = [(0x9c00_0000, 0xa080_0000)]; assert_eq!( - largest(&ram, 16 * MIB, FOUR_GIB, &avoid, MIB), - Some(Span { base: 16 * MIB, end: 0x9c00_0000 }) + largest(&ram, 0, FOUR_GIB, &avoid, MIB, &[]), + Some(Span { base: 0x10_0000, end: 0x9c00_0000 }) ); assert_eq!( - largest(&ram, FOUR_GIB, u64::MAX, &avoid, MIB), + largest(&ram, FOUR_GIB, u64::MAX, &avoid, MIB, &[]), Some(Span { base: FOUR_GIB, end: 0x4_5f80_0000 }) ); } @@ -142,29 +99,50 @@ mod tests { #[test] fn a_gap_in_the_map_ends_a_stretch_and_adjacent_ranges_join() { let ram = [(0x0100_0000, 0x2000_0000), (0x2000_0000, 0x3000_0000), (0x3010_0000, 0x3800_0000)]; - assert_eq!(largest(&ram, 0, FOUR_GIB, &[], MIB), Some(Span { base: 0x0100_0000, end: 0x3000_0000 })); + assert_eq!(largest(&ram, 0, FOUR_GIB, &[], MIB, &[]), Some(Span { base: 0x0100_0000, end: 0x3000_0000 })); } #[test] fn a_reserved_region_inside_a_stretch_leaves_its_larger_side() { let ram = [(0x0100_0000, 0x8000_0000)]; let avoid = [(0x2000_0000, 0x2010_0000)]; - assert_eq!(largest(&ram, 0, FOUR_GIB, &avoid, MIB), Some(Span { base: 0x2010_0000, end: 0x8000_0000 })); + assert_eq!(largest(&ram, 0, FOUR_GIB, &avoid, MIB, &[]), Some(Span { base: 0x2010_0000, end: 0x8000_0000 })); + } + + #[test] + fn a_region_is_the_part_that_holds_what_it_must_cover_even_when_smaller() { + let ram = [(0x0100_0000, 0x2000_0000), (0x2010_0000, 0x8000_0000)]; + let tables = [(0x0160_0000, 0x0180_0000)]; + assert_eq!(largest(&ram, 0, FOUR_GIB, &[], MIB, &tables), Some(Span { base: 0x0100_0000, end: 0x2000_0000 })); + // Ranges that start in the other window bind nothing here. + assert_eq!( + largest(&ram, 0, FOUR_GIB, &[], MIB, &[(0x1_0000_0000, 0x1_0020_0000)]), + Some(Span { base: 0x2010_0000, end: 0x8000_0000 }) + ); + } + + #[test] + fn what_no_one_part_holds_leaves_no_region() { + let ram = [(0x0100_0000, 0x2000_0000), (0x2010_0000, 0x8000_0000)]; + let split = [(0x0160_0000, 0x0180_0000), (0x5000_0000, 0x5020_0000)]; + assert!(largest(&ram, 0, FOUR_GIB, &[], MIB, &split).is_none()); + let under_an_rmrr = [(0x3000_0000, 0x3020_0000)]; + assert!(largest(&ram, 0, FOUR_GIB, &[(0x3000_0000, 0x3100_0000)], MIB, &under_an_rmrr).is_none()); } #[test] fn alignment_only_ever_shrinks_a_region() { let ram = [(0x0123_4000, 0x0987_6000)]; - let span = largest(&ram, 0, FOUR_GIB, &[], 16 * MIB).unwrap(); + let span = largest(&ram, 0, FOUR_GIB, &[], 16 * MIB, &[]).unwrap(); assert_eq!(span, Span { base: 0x0200_0000, end: 0x0900_0000 }); - assert!(largest(&ram, 0, FOUR_GIB, &[], 1 << 30).is_none()); + assert!(largest(&ram, 0, FOUR_GIB, &[], 1 << 30, &[]).is_none()); } #[test] fn nothing_is_protected_where_nothing_is_ram() { - assert!(largest(&[], 0, FOUR_GIB, &[], MIB).is_none()); + assert!(largest(&[], 0, FOUR_GIB, &[], MIB, &[]).is_none()); let ram = vec![(0x1000_0000, 0x2000_0000)]; - assert!(largest(&ram, 0, FOUR_GIB, &[(0, u64::MAX)], MIB).is_none()); - assert!(largest(&ram, FOUR_GIB, u64::MAX, &[], MIB).is_none()); + assert!(largest(&ram, 0, FOUR_GIB, &[(0, u64::MAX)], MIB, &[]).is_none()); + assert!(largest(&ram, FOUR_GIB, u64::MAX, &[], MIB, &[]).is_none()); } } diff --git a/kernel/src/arch/aarch64/smmu/mod.rs b/kernel/src/arch/aarch64/smmu/mod.rs index d4e6c89a37d..e62c981607c 100644 --- a/kernel/src/arch/aarch64/smmu/mod.rs +++ b/kernel/src/arch/aarch64/smmu/mod.rs @@ -210,7 +210,13 @@ pub fn drivers_settled() {} /// The SMMUv3, armed; a refusal leaves every unit the IORT names aborting /// what it is given, or says why one could not be told to. -pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[toyos_abi::boot::RootBridgeWindow]>) { +/// `_kernel` is the VT-d hand-over's to guard; this backend has no use for it. +pub fn init( + rsdp_addr: u64, + devices: &[PciDevice], + windows: Option<&[toyos_abi::boot::RootBridgeWindow]>, + _kernel: (u64, u64), +) { let iort = match toyos_acpi::iort(direct_phys(), rsdp_addr) { Ok(iort) => iort, Err(IortRefused::Table(TableError::Absent)) => { diff --git a/kernel/src/arch/x86_64/iommu_unit.rs b/kernel/src/arch/x86_64/iommu_unit.rs index b34cbc47ba7..0eab569ca86 100644 --- a/kernel/src/arch/x86_64/iommu_unit.rs +++ b/kernel/src/arch/x86_64/iommu_unit.rs @@ -35,8 +35,8 @@ fn backend() -> Backend { const MAX_PHYS: u64 = 1 << 52; /// The units of whichever table firmware published. -pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeWindow]>) { - let found = if super::vtd::init(rsdp_addr, devices, windows) { +pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeWindow]>, kernel: (u64, u64)) { + let found = if super::vtd::init(rsdp_addr, devices, windows, kernel) { Backend::Vtd } else if super::amdvi::init(rsdp_addr, devices, windows) { Backend::AmdVi diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 984702b71e1..bbe4cefaaf7 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -31,7 +31,6 @@ use crate::sync::Lock; use crate::time::{Duration, Tripwire}; use super::iommu_unit::{yn, MAX_UNITS}; -use kernel::dmaguard::Step; use dmar::{Dmar, Malformed, Scope, Scopes, Structure}; use queue::Queue; use table::{Table, Tables}; @@ -94,8 +93,8 @@ static TABLES: Lock = Lock::new(Tables::new()); /// queue: a queue has one tail, so a second holder would submit behind its back. static UNITS: Lock> = Lock::new(Vec::new()); -/// A unit past `enable`: its window, its queue, and the root table its context -/// entries live in. +/// A unit past its hand-over: its window, its queue, and the root table its +/// context entries live in. pub(super) struct Live { regs: Mmio, queue: Queue, @@ -127,7 +126,7 @@ pub(super) fn invalidate_interrupt_entries() { /// `windows` and every region firmware reserved are what no domain's addresses /// reach. `false` where firmware published no DMAR. -pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeWindow]>) -> bool { +pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeWindow]>, kernel: (u64, u64)) -> bool { let dmar = match Dmar::open(rsdp_addr) { Ok(dmar) => dmar, Err(TableError::Absent) => return false, @@ -248,53 +247,88 @@ pub fn init(rsdp_addr: u64, devices: &[PciDevice], windows: Option<&[RootBridgeW // 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]; - let mut steps: Vec> = ready.into_iter().map(Some).collect(); // Before any table is built: what a domain of a driver's own can be is the // narrowest thing every translating unit on this machine agrees to. - for (unit, plan) in steps.iter().flatten() { + for (unit, plan) in &ready { domain::unit_agrees(plan.width, unit.caps.domains(), unit.caps.mgaw()); } #[cfg(feature = "boot-actuators")] if crate::actuator::iommu_firmware_left() { - for (unit, plan) in steps.iter().flatten() { - unit.leave_on(devices, identity_domain(&mut domains, plan.width), plan.width); + for (unit, plan) in &ready { + let domain = identity_domain(&mut TABLES.lock(), &mut domains, plan.width); + unit.leave_on(devices, domain, plan.width); } } - // A unit whose translation goes off passes DMA untranslated until it is - // back on, so the protected regions cover every unit's window at once. - let mut guard = { - let units: Vec<(usize, Mmio, bool)> = steps + for (unit, _) in &ready { + pmr::hand_over(unit.index, unit.regs); + } + // Built before the regions are chosen, so they can be chosen to hold them. + let ready: Vec<(Unit, Plan, Table)> = ready + .into_iter() + .map(|(unit, plan)| { + let mut tables = TABLES.lock(); + let domain = identity_domain(&mut tables, &mut domains, plan.width); + let root = identity_root(&mut tables, devices, domain, plan.width); + (unit, plan, root) + }) + .collect(); + let guard = { + let candidates: Vec = ready .iter() - .flatten() - .map(|(unit, _)| (unit.index, unit.regs, unit.caps.protected_memory())) + .map(|(unit, _, _)| pmr::Candidate { + index: unit.index, + regs: unit.regs, + regions: unit.caps.protected_memory(), + registers: unit.invalidates_through_registers(), + }) .collect(); + // What the OS owns once firmware is done with it: the pmm's memory and + // ACPI's reclaimable tables, never firmware's NVS. let ram: Vec<(u64, u64)> = crate::mm::firmware_map() .iter() - .filter(|e| toyos_bootmap::is_read_as_memory(e.uefi_type)) + .filter(|e| toyos_bootmap::is_usable_type(e.uefi_type) || e.uefi_type == toyos_bootmap::EFI_ACPI_RECLAIM_MEMORY) .map(|e| (e.start, e.end)) .collect(); - if units.is_empty() { None } else { pmr::program(&units, &ram, &rmrrs, crate::mm::pmm::top()) } - }; - let mut translating: Vec> = Vec::new(); - for step in kernel::dmaguard::order(steps.len(), guard.is_some()) { - match step { - Step::Arm => guard.as_ref().expect("an armed step without a guard").arm(), - Step::Translate(at) => { - let (unit, plan) = steps[at].take().expect("a unit translated twice"); - translating.push(Some(translate(unit, plan, devices, &mut domains))); - } - Step::Release => guard.take().expect("a release without a guard").release(), - Step::Finish(at) => finish(translating[at].take().expect("a unit finished twice"), devices, remap), + let cover: Vec<(u64, u64)> = TABLES.lock().pages().chain([kernel]).collect(); + if candidates.is_empty() { + None + } else { + pmr::program(&candidates, units, &ram, &rmrrs, &cover, crate::mm::pmm::top()) } + }; + if let Some(guard) = &guard { + log_bus_masters(devices); + guard.arm(); + } + let via = if guard.is_some() { Invalidation::Registers } else { Invalidation::Queue }; + let translating: Vec = + ready.into_iter().map(|(unit, plan, root)| translate(unit, plan, root, via)).collect(); + if let Some(guard) = guard { + guard.release(); + } + for translating in translating { + finish(translating, devices, remap); } true } +/// Every function mastering the bus as the regions go on: what they block, +/// they block without a record (§11.4.8.1). +fn log_bus_masters(devices: &[PciDevice]) { + const BUS_MASTER: u16 = 1 << 2; + let masters: Vec = devices + .iter() + .filter(|d| d.read_config_u16(0x04) & BUS_MASTER != 0) + .map(|d| alloc::format!("{}", StreamId::pci(d.bus, d.dev, d.func))) + .collect(); + log!("iommu: functions mastering the bus as protected memory goes on: [{}]", masters.join(" ")); +} + /// The identity domain at `width`, built the first time it is asked for. -fn identity_domain(domains: &mut [Option
; 2], width: AddressWidth) -> Table { +fn identity_domain(tables: &mut Tables, domains: &mut [Option
; 2], width: AddressWidth) -> Table { *domains[domain_slot(width)].get_or_insert_with(|| { let top = crate::mm::pmm::top(); - let (domain, frames) = table::identity_domain(&mut TABLES.lock(), width, top); + let (domain, frames) = table::identity_domain(tables, width, top); log!("iommu: identity domain aw={} covers 0x0..{top:#x} in {frames} 2 MiB leaves", width.bits()); domain }) @@ -401,11 +435,11 @@ impl Unit { let root = identity_root(&mut tables, devices, domain, width); (queue, root, tables.alloc()) }; + self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); self.regs.write_u64(RTADDR_REG, root.phys()); self.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - self.invalidate_through_registers(); + queue.invalidate_all(self.regs); self.command(TRANSLATION_ENABLE, true, "translation"); - self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); if self.caps.interrupt_remapping() { self.regs.write_u64(interrupt::IRTA_REG, remap.phys()); self.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); @@ -416,10 +450,17 @@ impl Unit { } } + /// Whether this unit takes register-based invalidation: a unit of major + /// version 6 or later refuses every request (§11.4.6.1), and the IOTLB + /// registers must lie inside the window this kernel maps. + fn invalidates_through_registers(&self) -> bool { + self.major < 6 && self.caps.iotlb_registers() + 16 <= REGISTER_WINDOW + } + /// A global context-cache invalidation, then a global IOTLB one draining /// reads and writes, through the unit's registers (§6.5.1); valid only with /// queued invalidation off, whatever `TES` says. A unit that reports either - /// done at no granularity refused it, as every unit from version 6 does. + /// done at no granularity refused it. fn invalidate_through_registers(&self) { const START: u64 = 1 << 63; const GLOBAL_CONTEXT: u64 = 0b01 << 61; @@ -568,66 +609,69 @@ fn plan(unit: &Unit) -> Option { ); return None; } - // The hand-over invalidates through these registers (`Unit::invalidate_through_registers`). - if unit.major >= 6 { - log!( - "iommu: unit{index} is version {}, which refuses register-based invalidation (§11.4.6.1) \ - and so the hand-over's — not programmed", - unit.major - ); - return None; - } - if unit.caps.iotlb_registers() + 16 > REGISTER_WINDOW { - log!( - "iommu: unit{index} puts its IOTLB registers at {:#x}, past its own {REGISTER_WINDOW}-byte \ - window (ECAP={:#018x}) — not programmed", - unit.caps.iotlb_registers(), - unit.caps.ecap - ); - return None; - } Some(Plan { width, records }) } -/// A unit translating through this kernel's root, its queue and interrupt -/// remapping not yet programmed. +/// How a hand-over invalidates after pointing the unit at this kernel's root. +#[derive(Clone, Copy)] +enum Invalidation { + /// Through the unit's registers, inside protected memory: the queue's + /// memory is no remapping structure (Appendix A, `ECAP.C`), so the regions + /// may cover it. + Registers, + /// Through the queue, enabled first. + Queue, +} + +/// A unit translating through this kernel's root, its interrupt remapping not +/// yet programmed. struct Translating { unit: Unit, width: AddressWidth, root: Table, + /// Enabled already where the hand-over invalidated through it. + queue: Option, } -/// Whatever firmware left on, off; then this kernel's root pointed, the global -/// invalidation §6.6 owes it, and translation on. The invalidation goes through -/// the unit's registers, since the queue's memory is no remapping structure -/// (Appendix A) and may lie under a protected region. -fn translate(unit: Unit, plan: Plan, devices: &[PciDevice], domains: &mut [Option
; 2]) -> Translating { +/// Whatever firmware left on, off; then `root` pointed, the global +/// invalidation §6.6 owes it, and translation on. +fn translate(unit: Unit, plan: Plan, root: Table, via: Invalidation) -> Translating { let Plan { width, records } = plan; - // Built before `TABLES` is held here: building it takes that lock too. - let domain = identity_domain(domains, width); - let root = identity_root(&mut TABLES.lock(), devices, domain, width); // After the tables are built, so a unit firmware left translating passes - // DMA untranslated only while it is programmed. + // DMA untranslated only while it is programmed, and before the queue is + // pointed, which §6.5.2 does only with it off. unit.hand_over(); // Before `TE`: the first blocked transaction must be reportable, not merely counted. fault::arm(unit.index, unit.regs, records, crate::arch::idt::DMA_FAULT_VECTOR); + let mut queue = match via { + Invalidation::Registers => None, + Invalidation::Queue => Some(enable_queue(&unit)), + }; unit.regs.write_u64(RTADDR_REG, root.phys()); unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); - unit.invalidate_through_registers(); + match queue.as_mut() { + Some(queue) => queue.invalidate_all(unit.regs), + None => unit.invalidate_through_registers(), + } unit.command(TRANSLATION_ENABLE, true, "translation"); - Translating { unit, width, root } + Translating { unit, width, root, queue } } -/// The invalidation queue and interrupt remapping, each confirmed in `GSTS` -/// before the next. +fn enable_queue(unit: &Unit) -> Queue { + let queue = Queue::new(&mut TABLES.lock(), unit.regs); + unit.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); + queue +} + +/// The queue where it is not on yet, then interrupt remapping, each confirmed +/// in `GSTS` before the next. fn finish(translating: Translating, devices: &[PciDevice], remap: Option) { - let Translating { unit, width, root } = translating; + let Translating { unit, width, root, queue } = translating; let index = unit.index; - let mut queue = Queue::new(&mut TABLES.lock(), unit.regs); + let mut queue = queue.unwrap_or_else(|| enable_queue(&unit)); // 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); - unit.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); if let Some(irta) = irta { unit.regs.write_u64(interrupt::IRTA_REG, irta); unit.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); @@ -795,7 +839,7 @@ impl Capabilities { self.ecap & (1 << 0) != 0 } - /// `ECAP.QI`; absent falls back to `CCMD_REG`/`IOTLB_REG`, correct but slower. + /// `ECAP.QI`; a unit without it is left unprogrammed (`plan`). fn queued_invalidation(&self) -> bool { self.ecap & (1 << 1) != 0 } diff --git a/kernel/src/arch/x86_64/vtd/pmr.rs b/kernel/src/arch/x86_64/vtd/pmr.rs index 7e0d6a5f8b0..d78d241943f 100644 --- a/kernel/src/arch/x86_64/vtd/pmr.rs +++ b/kernel/src/arch/x86_64/vtd/pmr.rs @@ -1,11 +1,13 @@ -//! Protected memory regions (VT-d §11.4.8): while a unit's translation is off, -//! it blocks DMA to them, and a unit's own reads of its remapping structures -//! are exempt (§11.4.8.1, `EPM`). They hold while every unit is handed over, -//! so a unit with translation off passes no DMA to the memory they cover. +//! Protected memory regions (VT-d §11.4.8). On a unit of major version 2 or +//! later, and on every one since Skylake, they block DMA to the memory they +//! cover whether or not the unit translates, while the unit's own reads of its +//! remapping structures are exempt (§11.4.8.1, `EPM`). So holding them from +//! before the first unit's translation goes off until the last unit translates +//! keeps the hand-over's tables, and this kernel, out of every device's reach. //! //! The same regions go on every unit, as §11.4.8.1 requires for them to -//! protect anything. They avoid every RMRR, so firmware's own DMA to its -//! reserved memory runs on through the hand-over. +//! protect anything. They avoid every RMRR, so firmware's DMA to its reserved +//! memory runs on through the hand-over. use alloc::vec::Vec; @@ -14,11 +16,6 @@ use crate::mm::Mmio; use crate::time::{Duration, Tripwire}; use kernel::dmaguard::{self, Span}; -/// Below 16 MiB an Intel host bridge may decode legacy ranges and the optional -/// ISA hole as MMIO (11th Generation Intel Core datasheet, 631122 §2.5.1), and -/// no protected region may cover MMIO (§11.4.8.1). -const LOW_FLOOR: u64 = 16 << 20; - const PMEN_REG: u64 = 0x64; const PLMBASE_REG: u64 = 0x68; const PLMLIMIT_REG: u64 = 0x6C; @@ -34,45 +31,103 @@ const STATUS_TIMEOUT: Tripwire = Tripwire::absurd( "a unit whose protected regions never report their state is one whose reach nothing can state", ); +/// Protected memory firmware left on, off: its bases and limits may not be +/// rewritten while it is (§11.4.8.2-§11.4.8.5), and left on it would block +/// DMA after the hand-over. +pub fn hand_over(index: usize, regs: Mmio) { + if regs.read_u32(PMEN_REG) & STATUS != 0 { + log!("iommu: unit{index} was handed over with protected memory on; it goes off first"); + set(index, regs, false); + } +} + +/// One unit the hand-over would guard. +pub struct Candidate { + pub index: usize, + pub regs: Mmio, + /// `CAP.PLMR` and `CAP.PHMR` both. + pub regions: bool, + /// It invalidates through its registers, which the window needs. + pub registers: bool, +} + /// Every unit's protected regions, programmed and not yet on. pub struct Guard { units: Vec<(usize, Mmio)>, } -/// Programs the same regions on every unit in `units`, each `(index, registers, -/// supported)`, or says why it cannot. `ram` is firmware's RAM, `avoid` -/// every RMRR, `top` the highest memory the identity domain maps. -pub fn program(units: &[(usize, Mmio, bool)], ram: &[(u64, u64)], avoid: &[(u64, u64)], top: u64) -> Option { - if let Some(&(index, _, _)) = units.iter().find(|(_, _, supported)| !supported) { - log!( - "iommu: unit{index} has no protected memory regions (CAP.PLMR or CAP.PHMR clear), so while \ - the units are handed over every function behind one reaches all of memory" - ); - return None; +/// The same regions programmed on every unit, covering every `cover` range, +/// or `None` with the reason logged: the hand-over is then unguarded. `ram` is +/// what firmware's map says the OS owns, `avoid` every RMRR, `top` the end of +/// what the identity domain maps. +pub fn program( + units: &[Candidate], + described: usize, + ram: &[(u64, u64)], + avoid: &[(u64, u64)], + cover: &[(u64, u64)], + top: u64, +) -> Option { + fn unguarded(why: core::fmt::Arguments) -> Option { + log!("iommu: the hand-over is unguarded: {why}, so every function reaches all of memory while the units are programmed"); + None } - // The coarsest alignment any unit decodes, so one pair of regions suits them all. + if units.len() < described { + return unguarded(format_args!( + "{} of {described} units are programmed, and the regions protect only where every unit holds them", + units.len() + )); + } + if let Some(unit) = units.iter().find(|u| !u.regions) { + return unguarded(format_args!("unit{} has no protected memory regions (CAP.PLMR or CAP.PHMR clear)", unit.index)); + } + if let Some(unit) = units.iter().find(|u| !u.registers) { + return unguarded(format_args!( + "unit{} cannot invalidate through its registers, and its queue's memory may lie under the regions", + unit.index + )); + } + // The coarsest alignment any base or limit decodes (§11.4.8.2-§11.4.8.5), + // so one pair of regions suits every unit. let mut low_align = 1u64; let mut high_align = 1u64; - for &(index, regs, _) in units { + for unit in units { + let regs = unit.regs; regs.write_u32(PLMBASE_REG, u32::MAX); + regs.write_u32(PLMLIMIT_REG, u32::MAX); regs.write_u64(PHMBASE_REG, u64::MAX); - let (low, high) = (regs.read_u32(PLMBASE_REG), regs.read_u64(PHMBASE_REG)); - if low == 0 || high == 0 { - log!("iommu: unit{index} decodes no protected region base (PLMBASE={low:#x} PHMBASE={high:#x})"); - return None; + regs.write_u64(PHMLIMIT_REG, u64::MAX); + let low = [regs.read_u32(PLMBASE_REG), regs.read_u32(PLMLIMIT_REG)]; + let high = [regs.read_u64(PHMBASE_REG), regs.read_u64(PHMLIMIT_REG)]; + if low.contains(&0) || high.contains(&0) { + return unguarded(format_args!( + "unit{} decodes no protected region (PLMBASE/LIMIT={low:x?} PHMBASE/LIMIT={high:x?})", + unit.index + )); + } + for bits in low { + low_align = low_align.max(1 << bits.trailing_zeros()); + } + for bits in high { + high_align = high_align.max(1 << bits.trailing_zeros()); } - low_align = low_align.max(1 << low.trailing_zeros()); - high_align = high_align.max(1 << high.trailing_zeros()); } - let low = dmaguard::largest(ram, LOW_FLOOR, dmaguard::FOUR_GIB, avoid, low_align); - let high = dmaguard::largest(ram, dmaguard::FOUR_GIB, top, avoid, high_align); + let low = dmaguard::largest(ram, 0, dmaguard::FOUR_GIB, avoid, low_align, cover); + let high = dmaguard::largest(ram, dmaguard::FOUR_GIB, top, avoid, high_align, cover); log!( "iommu: protected memory for the hand-over on {} unit(s): low {} high {}, aligned to {low_align:#x} and {high_align:#x}", units.len(), Shown(low), Shown(high), ); - for &(_, regs, _) in units { + for &range in cover { + if !low.is_some_and(|s| s.holds(range)) && !high.is_some_and(|s| s.holds(range)) { + return unguarded(format_args!("{:#x}..{:#x}, which it must protect, lies outside both regions", range.0, range.1)); + } + } + log!("iommu: the regions cover the hand-over's tables and this kernel: {:x?}", cover); + for unit in units { + let regs = unit.regs; // A limit below its base is a region switched off (§11.4.8.3, §11.4.8.5). let (base, limit) = low.map_or((low_align, 0), |s| (s.base, s.end - 1)); regs.write_u32(PLMBASE_REG, base as u32); @@ -81,7 +136,7 @@ pub fn program(units: &[(usize, Mmio, bool)], ram: &[(u64, u64)], avoid: &[(u64, regs.write_u64(PHMBASE_REG, base); regs.write_u64(PHMLIMIT_REG, limit); } - Some(Guard { units: units.iter().map(|&(index, regs, _)| (index, regs)).collect() }) + Some(Guard { units: units.iter().map(|u| (u.index, u.regs)).collect() }) } impl Guard { @@ -89,6 +144,7 @@ impl Guard { for &(index, regs) in &self.units { set(index, regs, true); } + log!("iommu: protected memory on for {} unit(s)", self.units.len()); } pub fn release(self) { diff --git a/kernel/src/arch/x86_64/vtd/queue.rs b/kernel/src/arch/x86_64/vtd/queue.rs index 9737347e5c5..22230e98582 100644 --- a/kernel/src/arch/x86_64/vtd/queue.rs +++ b/kernel/src/arch/x86_64/vtd/queue.rs @@ -1,9 +1,9 @@ //! Queued invalidation: submits a descriptor and an Invalidation Wait //! descriptor, then polls the wait's status write to completion. //! Acknowledgement timeout is a panic — the kernel cannot say what a device -//! can reach past it. Every invalidation after the hand-over goes through -//! here; the hand-over's own goes through the unit's registers, before the -//! queue exists. A unit without `ECAP.QI` is left unprogrammed. +//! can reach past it. A hand-over inside protected memory invalidates through +//! the unit's registers instead, before the queue is enabled. A unit without +//! `ECAP.QI` is left unprogrammed. use crate::mm::Mmio; use crate::time::{Duration, Tripwire}; @@ -20,7 +20,9 @@ const IOTLB: u64 = 0x2; const INTERRUPT_ENTRY_CACHE: u64 = 0x4; const WAIT: u64 = 0x5; -/// Granularity field, bits 5:4, `10`: everything cached for one domain id, whatever address or requester it came from. +/// Granularity field, bits 5:4: `01` is global — every domain, every entry. +const GLOBAL: u64 = 1 << 4; +/// `10`: everything cached for one domain id, whatever address or requester it came from. const DOMAIN: u64 = 2 << 4; /// `11`, context cache only: one requester id, Section 6.5.2.1. const DEVICE: u64 = 3 << 4; @@ -62,6 +64,18 @@ impl Queue { queue } + /// Every cached translation and context entry, gone, acknowledged by the + /// unit before this returns. + pub fn invalidate_all(&mut self, regs: Mmio) { + self.submit( + regs, + &[ + (CONTEXT_CACHE | GLOBAL, 0), + (IOTLB | GLOBAL | DRAIN_WRITES | DRAIN_READS, 0), + ], + ); + } + /// Every translation cached for one domain, gone. Owed after any change to /// its second-level tables, a mapping becoming present included, because /// `CAP.CM` says the unit may have cached the absence. diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index 115414e579f..ed913d9689e 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -46,6 +46,14 @@ impl Tables { Self { pages: Vec::new(), used: PAGE_2M as usize } } + /// Every 2 MiB page tables have been carved from, `[start, end)` physical. + pub fn pages(&self) -> impl Iterator + '_ { + self.pages.iter().map(|page| { + let start = page.direct_map().phys(); + (start, start + PAGE_2M) + }) + } + /// Returns one zeroed 4 KiB table, usable as a root, context, second-level, or invalidation-queue table. pub fn alloc(&mut self) -> Table { if self.used + TABLE_BYTES > PAGE_2M as usize { diff --git a/kernel/src/iommu/mod.rs b/kernel/src/iommu/mod.rs index 96397140197..4a5ef200fc1 100644 --- a/kernel/src/iommu/mod.rs +++ b/kernel/src/iommu/mod.rs @@ -278,12 +278,15 @@ impl core::fmt::Display for StreamId { /// /// No domain's addresses reach into one of `windows`, the root bridges' /// declared memory, or `None` where no declaration is held ([`window`]). +/// `kernel` is this kernel's image, `[start, end)` physical: what a unit's +/// hand-over keeps out of every device's reach where it can. pub fn init( rsdp_addr: u64, devices: &[crate::drivers::pci::PciDevice], windows: Option<&toyos_pci::aperture::Declaration>>, + kernel: (u64, u64), ) { - unit::init(rsdp_addr, devices, windows.map(|declared| declared.windows())); + unit::init(rsdp_addr, devices, windows.map(|declared| declared.windows()), kernel); } /// Once, after every kernel driver has taken its functions from firmware and diff --git a/kernel/src/main.rs b/kernel/src/main.rs index ce9eda9a7b5..f4e21e76433 100644 --- a/kernel/src/main.rs +++ b/kernel/src/main.rs @@ -432,7 +432,12 @@ pub(crate) unsafe extern "C" fn kernel_main(loader_args: &mut 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, pcidev::declared().as_ref()); + iommu::init( + kernel_args.rsdp_addr, + &pci_devices, + pcidev::declared().as_ref(), + (kernel_args.kernel_memory_addr, kernel_args.kernel_memory_addr + kernel_args.kernel_memory_size), + ); // 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/lib/bootmap/src/lib.rs b/lib/bootmap/src/lib.rs index 689c5ee6c02..c9884816552 100644 --- a/lib/bootmap/src/lib.rs +++ b/lib/bootmap/src/lib.rs @@ -105,7 +105,7 @@ pub const EFI_LOADER_DATA: u32 = 2; const EFI_BOOT_SERVICES_CODE: u32 = 3; const EFI_BOOT_SERVICES_DATA: u32 = 4; const EFI_CONVENTIONAL_MEMORY: u32 = 7; -const EFI_ACPI_RECLAIM_MEMORY: u32 = 9; +pub const EFI_ACPI_RECLAIM_MEMORY: u32 = 9; const EFI_ACPI_MEMORY_NVS: u32 = 10; /// Whether a UEFI memory type becomes free RAM the pmm hands out. diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 9dca9463103..20a5cafd8e7 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -353,17 +353,16 @@ pub(crate) fn amdvi_left_on(unit: &str) -> [String; 3] { /// the T14 takes (`iommu_firmware_left`'s row), logged as such. Under /// `iommu-firmware-left` the unit is left queueing, translating and remapping; /// each goes off by its own write, the root is set and invalidated through -/// the unit's registers, and the unit ends translating with the boot stick's -/// DMA behind it. +/// the queue, and the unit ends translating with the boot stick's DMA behind it. pub fn vtd_firmware_left(test_config: &Path) -> Result<(), String> { let options = BootOptions { profile: Profile::Metal, kernel_params: &["iommu-firmware-left"], ..Default::default() }; let qemu = QemuInstance::boot_with_options(test_config, &[], &[], options); let log = Serial::boot(&qemu); log.must_be_clean()?; - let unguarded = log.must_say("iommu: unit0 has no protected memory regions")?; + let unguarded = log.must_say("iommu: the hand-over is unguarded: unit0 has no protected memory regions")?; let off = log.must_say("iommu: unit0 was handed over with translation on; it goes off first")?; log.must_say("iommu: unit0 was handed over with queued invalidation on; it goes off first")?; - log.must_not_say("iommu: protected memory for the hand-over")?; + log.must_not_say("iommu: protected memory on for")?; let translating = log.must_say(" translating gsts=")?; if !translating.contains(" tes=y ") { return Err(format!("unit0 was programmed and is not translating: {translating:?}")); diff --git a/tests/toyos.rs b/tests/toyos.rs index 6a11590d55e..2e045eb87ab 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1303,7 +1303,7 @@ const METAL: &[(&str, metal::Metal)] = &[ // log running, on real silicon: each field goes off by its own write // before the unit is programmed or left off, and the boot goes on. "iommu_firmware_left", - metal::Metal { arms: SELFTESTS, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, + metal::Metal { arms: FIRMWARE_LEFT, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, ), // ---- a claimed function at the unit, on tests/images/testcases.toml ---- ( @@ -1806,7 +1806,6 @@ const SELFTEST_PARAMS: &[&str] = &[ "lapic-spurious-selftest", "unclaimed-vector-selftest", "sysret-ss-probe", - "iommu-firmware-left", ]; const SELFTESTS: &[metal::Arm] = &[metal::Arm { @@ -1814,6 +1813,15 @@ const SELFTESTS: &[metal::Arm] = &[metal::Arm { ..metal::once("shared-debug", "tests/images/testcases.toml", SELFTEST_PARAMS, &[]) }]; +/// Its own boot: on the T14 the hand-over it stages can halt the machine on +/// the iGPU's fault +/// (`issues/the-t14s-igpu-faults-read-permission-after-translation-goes-off-and-on-under-its-scanout.md`), +/// which would take every row batched with it. +const FIRMWARE_LEFT: &[metal::Arm] = &[metal::Arm { + features: ACTUATOR_KERNEL, + ..metal::once("iommu-firmware-left", "tests/images/testcases.toml", &["iommu-firmware-left"], &[]) +}]; + /// The two boots every discovered member rides on the T14, each behind its /// rows' jobs: `testcases`, the shipping kernel's, carries every discovered /// Rust binary, then the C corpus, then [`LAST_MEMBERS`]; `shared-debug` @@ -5988,7 +5996,9 @@ fn sysret_ss(log: &str) -> Result<(), String> { /// An AMD-Vi unit's lines are on the boot stick only because the switch-off /// let the stick's DMA through the unit the actuator left blocking it. fn iommu_firmware_left(log: &str) -> Result<(), String> { - let guarded = log.lines().filter(|l| l.contains(" cap=") && l.contains("iommu: unit")).all(|l| l.contains(" pmr=y")); + // VT-d unit lines alone: an AMD-Vi unit's read `iommu: amdvi unit`. + let vtd: Vec<&str> = log.lines().filter(|l| l.contains("iommu: unit") && l.contains(" cap=")).collect(); + let guarded = !vtd.is_empty() && vtd.iter().all(|l| l.contains(" pmr=y")); let mut units = 0; for line in log.lines().filter(|l| l.contains(" translating gsts=")) { let unit = line @@ -6037,13 +6047,18 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { if units == 0 { return Err(format!("no unit was programmed or switched off, so nothing was handed over:\n{log}")); } - let armed = log.lines().position(|l| l.contains("iommu: protected memory for the hand-over on")); - let released = log.lines().position(|l| l.contains("iommu: protected memory released on")); - let first_off = log.lines().position(|l| l.contains("was handed over with translation on; it goes off first")); - if guarded && !matches!((armed, first_off, released), (Some(a), Some(o), Some(r)) if a < o && o < r) { + let at = |needle: &str| log.lines().position(|l| l.contains(needle)); + let armed = at("iommu: protected memory on for"); + let first_off = at("was handed over with translation on; it goes off first"); + let released = at("iommu: protected memory released on"); + let first_translating = at(" translating gsts="); + if guarded + && !matches!((armed, first_off, released, first_translating), + (Some(a), Some(o), Some(r), Some(t)) if a < o && r < t) + { return Err(format!( - "every unit has protected memory regions, and the hand-over did not take translation \ - off between their going on and their release:\n{log}" + "every unit has protected memory regions, and they did not go on before translation \ + first went off and come off before the first unit was programmed:\n{log}" )); } let passed = log.matches("handed over with compatibility-format pass-through on").count();