From 401d6f7d58cdbb6abcbcad9aa3dc5af0490c2f13 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 3 Oct 2026 23:46:57 +0200 Subject: [PATCH 1/7] =?UTF-8?q?VT-d:=20a=20moved=20context=20entry=20is=20?= =?UTF-8?q?invalidated=20under=20the=20domain=20it=20held,=20and=20GCMD=20?= =?UTF-8?q?follows=20=C2=A711.4.4.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving a function's context entry onto another domain invalidated the context cache and the IOTLB under the domain it was moving TO. VT-d Rev. 4.0 §6.5.2.1 invalidates a device-selective context-cache entry only where both the source-id and the domain-id match, and §6.2.1 has the IOTLB look a translation up by requester while keeping it under the domain it was made in. So the entry the unit held for the old domain, and every translation the function made through it, survived the move: a function released from slot 0, whose slot G then claimed, re-claimed into slot 1 could still translate through slot 0's domain, which now maps G's grants. QEMU ignores the domain-id of a device-selective context invalidation (intel_iommu.c, vtd_process_context_cache_desc), so no guest could see it. Linux reads the id off the entry it replaces (domain_context_clear_one, intel_context_flush_no_pasid) and so does this now: `table::bind` reads the entry, `rebind` decides the new entry and a `Displaced` that only the old entry's id goes into, and `Queue::invalidate_context` takes nothing else. A const block holds `rebind` to it: a move off the identity domain onto domain 0xffff is invalidated under domain 1. The entry itself was rewritten as two 8-byte stores, the domain-id half first, which leaves a present entry naming the new id over the old table. §6.2.2.1 asks a present entry to change by a 16-byte atomic write or through states each self-consistent; it now changes in one `lock cmpxchg16b`, which also proves the entry replaced is the one whose id is invalidated. Every GCMD write started from a shadow that began at 0, so the first one (`QIE`) also dropped any `TE`, `IRE` or `CFI` firmware had left on, several fields in one write where §11.4.4.1 asks one. `Unit::command` is now the section's recipe: GSTS & 0x96FFFFFF with one field changed. What firmware left on is then switched off deliberately, one field per write, before the unit is pointed anywhere, because §6.6 and §6.7 let a live unit's tables move only onto tables that translate alike and §6.5.2 sets a queue up only while queued invalidation is off: translation, compatibility-format pass-through (while GSTS still reports it, §11.4.4.2), interrupt remapping, then queued invalidation once its queue is empty. Linux does the same for a handover it cannot copy (init_dmars, dmar_disable_qi, intel_setup_irq_remapping). The unit line now carries GSTS as firmware left it. `iommu-firmware-left` stages that handover — every unit queueing, translating and remapping through tables of its own, as this kernel leaves one — and the selftests boot's `iommu_firmware_left` row judges it on the T14: every unit that translates said it switched each of those off first. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/actuator.rs | 5 + kernel/src/arch/x86_64/vtd/domain.rs | 3 +- kernel/src/arch/x86_64/vtd/interrupt.rs | 2 +- kernel/src/arch/x86_64/vtd/mod.rs | 147 ++++++++++++++++-------- kernel/src/arch/x86_64/vtd/queue.rs | 21 ++-- kernel/src/arch/x86_64/vtd/table.rs | 121 ++++++++++++++++--- src/metal.rs | 5 + tests/toyos.rs | 44 +++++++ 8 files changed, 264 insertions(+), 84 deletions(-) diff --git a/kernel/src/actuator.rs b/kernel/src/actuator.rs index 31a902a95b8..a345b957429 100644 --- a/kernel/src/actuator.rs +++ b/kernel/src/actuator.rs @@ -112,6 +112,11 @@ actuators! { /// Withhold `VIRTIO_F_ACCESS_PLATFORM` from every virtio device but the console, staging a function whose addresses the unit never translates. virtio_no_access_platform = "virtio-no-access-platform"; + /// Leave every IOMMU unit queueing, translating and remapping through + /// tables of its own, as firmware may hand one over, just before this + /// kernel programs it. Judged by `iommu_firmware_left`. + iommu_firmware_left = "iommu-firmware-left"; + /// Leave every AP holding the CR0/CR4 that INIT left it. no_ap_control_regs = "no-ap-control-regs"; diff --git a/kernel/src/arch/x86_64/vtd/domain.rs b/kernel/src/arch/x86_64/vtd/domain.rs index 9a6bd647cf2..e40b7d1734d 100644 --- a/kernel/src/arch/x86_64/vtd/domain.rs +++ b/kernel/src/arch/x86_64/vtd/domain.rs @@ -168,8 +168,7 @@ pub fn attach(stream: StreamId, id: DomainId) { let domain = *domains.at(id); let mut units = UNITS.lock(); for unit in units.iter_mut() { - table::bind(&mut TABLES.lock(), unit.root(), stream, &domain); - unit.invalidate_context(domain.id(), stream.requester()); + unit.attach(stream, &domain); } super::fault::attached(stream, domain.id()); log!("iommu: {stream} moves to domain{}", domain.id()); diff --git a/kernel/src/arch/x86_64/vtd/interrupt.rs b/kernel/src/arch/x86_64/vtd/interrupt.rs index 7f349087fbf..62cd1ae9c53 100644 --- a/kernel/src/arch/x86_64/vtd/interrupt.rs +++ b/kernel/src/arch/x86_64/vtd/interrupt.rs @@ -40,7 +40,7 @@ pub const IRTA_REG: u64 = 0xB8; pub const INTERRUPT_REMAPPING_ENABLE: u32 = 1 << 25; pub const SET_TABLE_POINTER: u32 = 1 << 24; -/// Set means compatibility-format messages bypass remapping; `GCMD.CFI` is never written, so it reads clear. +/// Set means compatibility-format messages bypass remapping. pub const COMPATIBILITY_FORMAT: u32 = 1 << 23; /// `2^(7+1)` entries of 128 bits is exactly the 4 KiB [`Tables::alloc`] hands out. diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index a9e313f5c15..2b0d282d329 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -44,6 +44,7 @@ const FECTL_REG: u64 = 0x38; const FEDATA_REG: u64 = 0x3C; const FEADDR_REG: u64 = 0x40; const FEUADDR_REG: u64 = 0x44; +const IQH_REG: u64 = 0x80; const IQT_REG: u64 = 0x88; const IQA_REG: u64 = 0x90; @@ -51,8 +52,23 @@ const IQA_REG: u64 = 0x90; const TRANSLATION_ENABLE: u32 = 1 << 31; const SET_ROOT_TABLE_POINTER: u32 = 1 << 30; const QUEUED_INVALIDATION_ENABLE: u32 = 1 << 26; -/// `GSTS.RTPS`: the root table pointer has been taken; shares `GCMD`'s `SRTP` bit position. -const ROOT_TABLE_SET: u32 = 1 << 30; + +/// `GSTS` less its one-shot bits: what §11.4.4.1's recipe carries into every +/// `GCMD` write, so that each write changes exactly one field. +const PERSISTENT: u32 = 0x96FF_FFFF; + +/// What firmware may leave on, switched off one field per write before the +/// unit is pointed at anything: §6.6 and §6.7 move a live unit only onto +/// tables that translate alike, §6.5.2 points a queue only while queued +/// invalidation is off, and `CFI` would ride every later write into the `IRE` +/// this kernel sets. `CFI` before `IRE`, while `GSTS` still reports it +/// (§11.4.4.2). +const FIRMWARE_LEFT: [(u32, &str); 4] = [ + (TRANSLATION_ENABLE, "translation"), + (interrupt::COMPATIBILITY_FORMAT, "compatibility-format pass-through"), + (interrupt::INTERRUPT_REMAPPING_ENABLE, "interrupt remapping"), + (QUEUED_INVALIDATION_ENABLE, "queued invalidation"), +]; /// How long a `GCMD` write is given to appear in `GSTS` before the kernel /// panics: a half-enabled unit's reach cannot be stated. @@ -88,16 +104,15 @@ pub(super) struct Live { } impl Live { - pub(super) fn root(&self) -> Table { - self.root - } - pub(super) fn invalidate_domain(&mut self, domain: u16) { self.queue.invalidate_domain(self.regs, domain); } - pub(super) fn invalidate_context(&mut self, domain: u16, stream: u16) { - self.queue.invalidate_context(self.regs, domain, stream); + /// Moves `stream` onto `domain` in this unit's tables, and out of + /// whatever this unit still holds for it under its old one. + pub(super) fn attach(&mut self, stream: StreamId, domain: &table::Domain) { + let displaced = table::bind(self.root, stream, domain); + self.queue.invalidate_context(self.regs, displaced); } } @@ -274,37 +289,79 @@ fn remappable(ready: &[(Unit, Plan)], described: usize) -> Option { Some(ready.iter().all(|(u, _)| u.caps.extended_interrupt_mode())) } -/// A unit whose register window decodes, with its capabilities and `GCMD` state. +/// A unit whose register window decodes, with its capabilities. struct Unit { index: usize, base: u64, regs: Mmio, caps: Capabilities, - /// Bits switched on in `GCMD`; the register is not read-modify-write safe, so this is the only record. - gcmd: u32, } impl Unit { - /// Set one `GCMD` bit and wait for `GSTS` to agree — one bit at a time. - fn command(&mut self, bit: u32, persistent: bool, status: u32, what: &str) { - self.regs.write_u32(GCMD_REG, self.gcmd | bit); - if persistent { - self.gcmd |= bit; - } + /// 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(); - loop { - let gsts = self.regs.read_u32(GSTS_REG); - if gsts & status != 0 { - return; - } + while !done() { assert!( crate::clock::nanos_since_boot() < deadline, - "iommu: unit{} never reported {what}, GSTS={gsts:#010x}", - self.index + "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`] + /// is for, left the way this kernel leaves a unit, each batch ended by a + /// wait. `CFI` is written as firmware writes it, unconfirmed: whether + /// `GSTS` reports it is the unit's to say. + #[cfg(feature = "boot-actuators")] + fn leave_on(&self) { + let (mut queue, root, remap) = { + let mut tables = TABLES.lock(); + (Queue::new(&mut tables, self.regs), tables.alloc(), 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.command(TRANSLATION_ENABLE, true, "translation"); + 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"); + queue.invalidate_interrupts(self.regs); + self.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); + let status = self.regs.read_u32(GSTS_REG) & PERSISTENT; + self.regs.write_u32(GCMD_REG, status | interrupt::COMPATIBILITY_FORMAT); + } + } + + /// 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; + } + 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", || { + self.regs.read_u64(IQH_REG) == self.regs.read_u64(IQT_REG) + }); + } + self.command(bit, false, what); + } + } } /// Firmware's register base, mapped only if 4 KiB-aligned and within the @@ -343,14 +400,15 @@ fn describe_unit(index: usize, drhd: &dmar::Drhd) -> Option { let caps = Capabilities { cap: regs.read_u64(CAP_REG), ecap: regs.read_u64(ECAP_REG) }; log!( "iommu: unit{index} @{base:#x} seg={} pci_all={} ver={}.{} cap={:#018x} ecap={:#018x} \ - aw={} sagaw={:#04x} mgaw={} nd={} sps2m={} cm={} psi={} nfr={} fro={:#x} qi={} ir={} \ - eim={} pt={} coherent={} sc={}", + gsts={:#010x} aw={} sagaw={:#04x} mgaw={} nd={} sps2m={} cm={} psi={} nfr={} fro={:#x} \ + qi={} ir={} eim={} pt={} coherent={} sc={}", drhd.segment(), yn(drhd.include_pci_all()), (version >> 4) & 0xF, version & 0xF, caps.cap, caps.ecap, + regs.read_u32(GSTS_REG), // Widest of 48/39 the unit advertises; a unit offering neither is refused. match caps.address_width() { Some(aw) => aw.bits(), @@ -373,7 +431,7 @@ fn describe_unit(index: usize, drhd: &dmar::Drhd) -> Option { ); describe_scopes("unit", index, drhd.scopes()); - Some(Unit { index, base, regs, caps, gcmd: 0 }) + Some(Unit { index, base, regs, caps }) } /// What a unit's capabilities let this kernel program, or `None` with the @@ -435,7 +493,7 @@ fn plan(unit: &Unit) -> Option { /// pointer, global invalidation, then `TE` — each confirmed in `GSTS` before /// the next. fn enable( - mut unit: Unit, + unit: Unit, plan: Plan, devices: &[PciDevice], domains: &mut [Option; 2], @@ -443,6 +501,11 @@ fn enable( ) { let index = unit.index; let Plan { width, records } = plan; + #[cfg(feature = "boot-actuators")] + if crate::actuator::iommu_firmware_left() { + unit.leave_on(); + } + unit.hand_over(); // 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()); @@ -480,39 +543,21 @@ fn enable( // 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_ENABLE, - "queued invalidation", - ); + unit.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); unit.regs.write_u64(RTADDR_REG, root.phys()); - unit.command(SET_ROOT_TABLE_POINTER, false, ROOT_TABLE_SET, "the root table pointer"); + unit.command(SET_ROOT_TABLE_POINTER, true, "the root table pointer"); if let Some(irta) = irta { unit.regs.write_u64(interrupt::IRTA_REG, irta); - unit.command( - interrupt::SET_TABLE_POINTER, - false, - interrupt::SET_TABLE_POINTER, - "the interrupt remap table pointer", - ); + unit.command(interrupt::SET_TABLE_POINTER, true, "the interrupt remap table pointer"); queue.invalidate_interrupts(unit.regs); } queue.invalidate_all(unit.regs); (root, queue) }; - unit.command(TRANSLATION_ENABLE, true, TRANSLATION_ENABLE, "translation"); - // `GCMD.CFI` is never among the bits `command` writes, so every write leaves - // it clear and `GSTS.CFIS` reads clear: a compatibility-format message is - // blocked from here on, which is the whole point of the step. + unit.command(TRANSLATION_ENABLE, true, "translation"); if remap.is_some() { - unit.command( - interrupt::INTERRUPT_REMAPPING_ENABLE, - true, - interrupt::INTERRUPT_REMAPPING_ENABLE, - "interrupt remapping", - ); + unit.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); } // After `IRE`, so nothing is invalidated on a unit that is not yet reading // the table, and after `TE`, so nothing reaches a unit that is not on. diff --git a/kernel/src/arch/x86_64/vtd/queue.rs b/kernel/src/arch/x86_64/vtd/queue.rs index 3fa4b1a0493..4715dbb0c27 100644 --- a/kernel/src/arch/x86_64/vtd/queue.rs +++ b/kernel/src/arch/x86_64/vtd/queue.rs @@ -8,7 +8,7 @@ use crate::mm::Mmio; use crate::time::{Duration, Tripwire}; -use super::table::{Table, Tables}; +use super::table::{Displaced, Table, Tables}; use super::{FSTS_REG, IQA_REG, IQT_REG}; /// 4 KiB of descriptors: 256 entries of 16 bytes (`IQA.QS = 0`). @@ -90,25 +90,22 @@ impl Queue { ); } - /// One requester's context entry, then everything cached for the domain it - /// now names — Section 6.5.2.1's order, and the IOTLB half is not optional: - /// a translation cached under the old context entry outlives it. - pub fn invalidate_context(&mut self, regs: Mmio, domain: u16, stream: u16) { + /// A replaced context entry, then everything cached under the domain it + /// named, in Section 6.5.2.1's order: the unit looks a translation up by + /// requester and keeps it under that id (Section 6.2.1). + pub fn invalidate_context(&mut self, regs: Mmio, displaced: Displaced) { + let domain = (displaced.domain() as u64) << DOMAIN_SHIFT; self.submit( regs, &[ ( CONTEXT_CACHE | DEVICE - | ((domain as u64) << DOMAIN_SHIFT) - | ((stream as u64) << SOURCE_SHIFT), - 0, - ), - ( - IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS - | ((domain as u64) << DOMAIN_SHIFT), + | domain + | ((displaced.requester() as u64) << SOURCE_SHIFT), 0, ), + (IOTLB | DOMAIN | DRAIN_WRITES | DRAIN_READS | domain, 0), ], ); } diff --git a/kernel/src/arch/x86_64/vtd/table.rs b/kernel/src/arch/x86_64/vtd/table.rs index a3cb7812d96..84a2dbd0e33 100644 --- a/kernel/src/arch/x86_64/vtd/table.rs +++ b/kernel/src/arch/x86_64/vtd/table.rs @@ -109,6 +109,44 @@ impl Table { (self.read(index * 2), self.read(index * 2 + 1)) } + /// Swaps the present 16-byte entry `old` at `index` for `new` in one + /// `lock cmpxchg16b`: the unit may fetch it at any moment and must read + /// the one or the other, never a half of each (§6.2.2.1). + fn replace_pair(self, index: usize, old: (u64, u64), new: (u64, u64)) { + let entry = self.window().subregion(index as u64 * 16, 16); + let (found_lo, found_hi): (u64, u64); + // SAFETY: `entry` is 16 bytes of a table `Tables::alloc` never frees, + // 16-byte aligned since a table is 4 KiB-aligned; `rbx`, which LLVM + // reserves, holds `new`'s low half only between the two moves. + unsafe { + core::arch::asm!( + "xchg {lo}, rbx", + "lock cmpxchg16b xmmword ptr [{entry}]", + "mov rbx, {lo}", + entry = in(reg) entry.addr(), + lo = inout(reg) new.0 => _, + in("rcx") new.1, + inout("rax") old.0 => found_lo, + inout("rdx") old.1 => found_hi, + options(nostack), + ); + } + let found = (found_lo, found_hi); + // `found` is `old` exactly when the swap happened; nothing else + // writes an entry under `UNITS`, so anything else is a kernel bug. + assert!( + found == old, + "iommu: entry {index} of table {:#x} held {:#x}:{:#x}, not the {:#x}:{:#x} read before \ + it was replaced", + self.phys, + found.1, + found.0, + old.1, + old.0 + ); + flush(entry.addr() as usize); + } + fn flush_all(self) { let base = self.window().addr() as usize; for offset in (0..TABLE_BYTES).step_by(LINE_BYTES) { @@ -147,7 +185,7 @@ fn flush(addr: usize) { } /// Second-level table depth for `width`; the context entry's `AW` field is this minus two. -fn levels(width: AddressWidth) -> u8 { +const fn levels(width: AddressWidth) -> u8 { match width { AddressWidth::Bits39 => 3, AddressWidth::Bits48 => 4, @@ -398,28 +436,42 @@ fn map_2m(tables: &mut Tables, root: Table, levels: u8, at: Iova, phys: u64, per table.write(index, (phys & !(PAGE_2M - 1)) | SL_LARGE | perm); } -/// Gives `stream` a context entry naming the identity domain. -pub fn bind_identity( - tables: &mut Tables, - root: Table, - stream: StreamId, - domain: Table, - width: AddressWidth, -) { - write_context(tables, root, stream, domain, KERNEL_DOMAIN, width); +/// Translation type 00: untranslated requests route through the named second-level table. +const fn context_entry(domain: Table, id: u16, width: AddressWidth) -> (u64, u64) { + (domain.phys | PRESENT, ((id as u64) << 8) | (levels(width) as u64 - 2)) +} + +/// What a unit may still hold cached for a context entry [`bind`] replaced: +/// its requester and the domain id it named, the pair §6.5.2.1 invalidates by. +#[must_use] +pub struct Displaced { + requester: u16, + domain: u16, +} + +impl Displaced { + pub fn requester(&self) -> u16 { + self.requester + } + + pub fn domain(&self) -> u16 { + self.domain + } } -/// Moves `stream` onto a domain of its own, in one unit's root table. -pub fn bind(tables: &mut Tables, root: Table, stream: StreamId, domain: &Domain) { - write_context(tables, root, stream, domain.root, domain.id, domain.width); +/// The entry that moves `requester`'s present entry `old` onto `domain`, and +/// what that leaves the unit holding: the id `old` named, never `domain`'s. +const fn rebind(old: (u64, u64), requester: u16, domain: &Domain) -> ((u64, u64), Displaced) { + let new = context_entry(domain.root, domain.id, domain.width); + (new, Displaced { requester, domain: (old.1 >> 8) as u16 }) } -fn write_context( +/// Gives `stream` its first context entry, naming the identity domain. +pub fn bind_identity( tables: &mut Tables, root: Table, stream: StreamId, domain: Table, - id: u16, width: AddressWidth, ) { let bus = stream.bus() as usize; @@ -431,12 +483,45 @@ fn write_context( root.write_pair(bus, table.phys | PRESENT, 0); table }; - // Translation type 00: untranslated requests route through the named second-level table. - let lo = domain.phys | PRESENT; - let hi = ((id as u64) << 8) | (levels(width) as u64 - 2); + let (lo, hi) = context_entry(domain, KERNEL_DOMAIN, width); context.write_pair(stream.devfn() as usize, lo, hi); } +/// Moves `stream`'s present context entry in one unit's root table onto a +/// domain of its own, and answers what that unit may still hold for the old one. +pub fn bind(root: Table, stream: StreamId, domain: &Domain) -> Displaced { + let entry = root.read(stream.bus() as usize * 2); + let context = Table { phys: entry & ADDR_MASK }; + let index = stream.devfn() as usize; + let old = if entry & PRESENT != 0 { context.read_pair(index) } else { (0, 0) }; + // `bind_identity` gave every enumerated function one before `TE`. + assert!( + old.0 & PRESENT != 0, + "iommu: {stream} has no context entry to move — it was not enumerated when its unit \ + was programmed" + ); + let (new, displaced) = rebind(old, stream.requester(), domain); + context.replace_pair(index, old, new); + displaced +} + +/// A function moved off the identity domain is invalidated under the identity +/// domain's id, the only one its cached entry can match (§6.5.2.1), and its new +/// entry names the domain it moved to. +const _: () = { + const OWN: Domain = Domain { + root: Table { phys: 0x5000 }, + id: u16::MAX, + width: AddressWidth::Bits39, + translatable: 39, + next: 0, + }; + let identity = context_entry(Table { phys: 0x3000 }, KERNEL_DOMAIN, AddressWidth::Bits48); + let (new, displaced) = rebind(identity, 0x00F8, &OWN); + assert!(displaced.domain == KERNEL_DOMAIN && displaced.requester == 0x00F8); + assert!(new.0 == 0x5000 | PRESENT && new.1 == (u16::MAX as u64) << 8 | 1); +}; + /// A unit whose `MGAW` is narrower than its `SAGAW` gets its window under the /// smaller of the two. /// diff --git a/src/metal.rs b/src/metal.rs index 9199546e3af..a88a7c8214d 100644 --- a/src/metal.rs +++ b/src/metal.rs @@ -767,6 +767,11 @@ pub const FLASHABLE: &[&str] = &[ // each printing what it asked for and what it observed. It establishes and // drops them and reaches nothing else. "sched-operation-nesting", + // Every IOMMU unit left translating, remapping and queueing just before the + // kernel programs it, which switches each of those off again first. It + // writes only the units' registers, which a reset returns to firmware, and + // tables in memory this boot owns. + "iommu-firmware-left", // It seals this boot's own record under an identity one bit from this // stick's, so the pass that finds it clears it and boots a kernel. The page // is memory the loader allocated and the machine is what it was after. diff --git a/tests/toyos.rs b/tests/toyos.rs index 486edcdcd90..d6dee46a3cd 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -639,6 +639,13 @@ const METAL: &[(&str, metal::Metal)] = &[ judge: |b| operation_nesting_log(b[0].kernel().text()), }, ), + ( + // A unit handed over translating, remapping and queueing, on real + // silicon: each field goes off by its own write before the unit is + // programmed, and the boot goes on. + "iommu_firmware_left", + metal::Metal { arms: SELFTESTS, judge: |b| iommu_firmware_left(b[0].kernel().text()) }, + ), // ---- the `isa` claim: one image whose i8042 the kernel leaves alone ---- ( // The I/O permission bitmap on the machine's own processor: the ports @@ -799,6 +806,7 @@ const SELFTESTS: &[metal::Arm] = &[metal::once( "sysret-ss-probe", "test-input-merge", "sched-operation-nesting", + "iommu-firmware-left", ], &[], )]; @@ -2789,6 +2797,42 @@ fn input_merge_ok(log: &str) -> Result<(), String> { Ok(()) } +/// Every unit this kernel went on to program said it was handed over with what +/// the actuator left on. +fn iommu_firmware_left(log: &str) -> Result<(), String> { + let mut units = 0; + for line in log.lines().filter(|l| l.contains(" translating gsts=")) { + let unit = line + .split("iommu: ") + .nth(1) + .and_then(|rest| rest.split_whitespace().next()) + .ok_or_else(|| format!("an unreadable unit line: {line:?}"))?; + let remaps = + log.lines().any(|l| l.contains(&format!("iommu: {unit} @")) && l.contains(" ir=y ")); + let left = ["translation", "queued invalidation"] + .into_iter() + .chain(remaps.then_some("interrupt remapping")); + for field in left { + if !log.contains(&format!("iommu: {unit} was handed over with {field} on")) { + return Err(format!( + "{unit} translates and never said it was handed over with {field} on, so \ + the state the actuator left was never switched off by the hand-over:\n{log}" + )); + } + } + units += 1; + } + if units == 0 { + return Err(format!("no unit was programmed, so nothing was handed over:\n{log}")); + } + let passed = log.matches("handed over with compatibility-format pass-through on").count(); + eprintln!( + " [iommu-firmware-left] {units} unit(s) programmed after switching off what they were \ + handed over with; {passed} of them reported compatibility format passed" + ); + Ok(()) +} + /// An inner `scheduler::Operation` may only narrow, and its drop restores what it displaced. fn operation_nesting_log(log: &str) -> Result<(), String> { From 55a5ccb420dbf1403c64e1d6435048547de26be1 Mon Sep 17 00:00:00 2001 From: japabu Date: Sat, 3 Oct 2026 23:48:44 +0200 Subject: [PATCH 2/7] File: a released function still names its slot's domain Found while moving context entries: release resets the function and clears its bus mastering, and leaves its context entry naming the slot's domain, which the slot's next holder is attached to. Not fixed on this branch. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...d-function-still-names-its-slots-domain.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 issues/kernel/a-released-function-still-names-its-slots-domain.md diff --git a/issues/kernel/a-released-function-still-names-its-slots-domain.md b/issues/kernel/a-released-function-still-names-its-slots-domain.md new file mode 100644 index 00000000000..a6c06a2ce6a --- /dev/null +++ b/issues/kernel/a-released-function-still-names-its-slots-domain.md @@ -0,0 +1,19 @@ +--- +status: open +kind: defect +opened: 2026-10-03 +--- + +# A released function still names its slot's domain + +`pcidev::release` (`kernel/src/pcidev/mod.rs`, `tear_down`) clears Bus Master +Enable and resets the function, and leaves its context entry naming the slot's +domain (`kernel/src/arch/x86_64/vtd/table.rs`, `bind`, is reached only from a +claim). The slot's next holder is attached to that same domain +(`slot_space`), so the released function's requests translate through the +next holder's grants for as long as it is that holder's. Nothing but the +released function's own obedience to `BME` stands between them: a function +that masters the bus with `BME` clear reaches another process's memory. + +**Exit**: a released function's context entry names no domain once it is +quiet, read back off the unit's table on the T14 after its slot's next claim. From 9be0a8ac8ac4c300053c6ff5a35c6015f3f4cd0f Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:20:44 +0200 Subject: [PATCH 3/7] VT-d: firmware's translation goes off after the unit's tables are built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answers review round 1 of #700. `hand_over` ran before the identity domain and the unit's context entries were built, so a unit firmware left translating passed DMA untranslated for the whole build: on the T14 under `iommu-firmware-left`, unit0's TE went off at 0.153 s and came back at 0.155 s around the 8956-leaf build. It now runs after the build and before `Queue::new` writes IQA (VT-d §6.5.2 points a queue only with queued invalidation off), outside the TABLES lock. Measured with the hand-over probe on QEMU 11.1.1: `identity domain ... covers` at 0.082 s, unit0's hand-over at 0.083 s, `translating` at 0.090 s; at 55a5ccb42 the hand-over was at 0.088 s and the build at 0.092 s. What stays open is filed: the window that remains while a unit firmware left translating is programmed, and a unit this kernel does not program keeping whatever firmware left on. The iommu_firmware_left judge reads `ir` through `unit_fields`, the parse tests/common/iommu.rs already has for that line, and refuses a translating unit no line describes. The released-function issue says what is true of `bind`: no release reaches it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...d-function-still-names-its-slots-domain.md | 4 +- ...asses-dma-untranslated-while-programmed.md | 32 +++++++++++++ ...not-program-keeps-what-firmware-left-on.md | 24 ++++++++++ kernel/src/arch/x86_64/vtd/mod.rs | 47 ++++++++++--------- tests/common/iommu.rs | 2 +- tests/toyos.rs | 8 +++- 6 files changed, 90 insertions(+), 27 deletions(-) create mode 100644 issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md create mode 100644 issues/kernel/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md diff --git a/issues/kernel/a-released-function-still-names-its-slots-domain.md b/issues/kernel/a-released-function-still-names-its-slots-domain.md index a6c06a2ce6a..7e10da87b20 100644 --- a/issues/kernel/a-released-function-still-names-its-slots-domain.md +++ b/issues/kernel/a-released-function-still-names-its-slots-domain.md @@ -8,8 +8,8 @@ opened: 2026-10-03 `pcidev::release` (`kernel/src/pcidev/mod.rs`, `tear_down`) clears Bus Master Enable and resets the function, and leaves its context entry naming the slot's -domain (`kernel/src/arch/x86_64/vtd/table.rs`, `bind`, is reached only from a -claim). The slot's next holder is attached to that same domain +domain (`kernel/src/arch/x86_64/vtd/table.rs`, `bind`, is reached from no +release). The slot's next holder is attached to that same domain (`slot_space`), so the released function's requests translate through the next holder's grants for as long as it is that holder's. Nothing but the released function's own obedience to `BME` stands between them: a function diff --git a/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md b/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md new file mode 100644 index 00000000000..6638fddcb42 --- /dev/null +++ b/issues/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md @@ -0,0 +1,32 @@ +--- +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, 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 (`toyos-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 +`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. + +**Exit**: under `iommu-firmware-left`, on QEMU and on the T14, no unit logs +`was handed over with translation on; it goes off first`, and every unit's +`translating` line reads `tes=y`. diff --git a/issues/kernel/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md b/issues/kernel/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md new file mode 100644 index 00000000000..f95ecaf94d1 --- /dev/null +++ b/issues/kernel/a-unit-this-kernel-does-not-program-keeps-what-firmware-left-on.md @@ -0,0 +1,24 @@ +--- +status: open +kind: defect +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 +`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 +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. + +Evidence: read off the tree; no machine here has one. The T14's four units are +all programmed, and its firmware leaves each at `gsts=0x40000000`. + +**Exit**: a boot that leaves a unit translating, remapping and queueing and +then refuses its plan logs each of those going off and boots on, judged on +QEMU. diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index 2b0d282d329..a57385dba43 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -505,12 +505,11 @@ fn enable( if crate::actuator::iommu_firmware_left() { unit.leave_on(); } - unit.hand_over(); // 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()); - let (root, queue) = { + let root = { let mut tables = TABLES.lock(); let domain = match domains[domain_slot(width)] { Some(domain) => domain, @@ -533,28 +532,32 @@ fn enable( let stream = StreamId::pci(device.bus, device.dev, device.func); table::bind_identity(&mut tables, root, stream, domain, width); } - - let mut queue = Queue::new(&mut tables, unit.regs); - drop(tables); - // 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"); - 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); - (root, queue) + root }; + // After the tables are built, so a unit firmware left translating passes DMA + // untranslated only while it is programmed + // (`issues/kernel/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(); + 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"); + 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() { unit.command(interrupt::INTERRUPT_REMAPPING_ENABLE, true, "interrupt remapping"); diff --git a/tests/common/iommu.rs b/tests/common/iommu.rs index 351c3da5d66..6f6a4ff3c9d 100644 --- a/tests/common/iommu.rs +++ b/tests/common/iommu.rs @@ -348,7 +348,7 @@ fn unit_is_first(argv: &[String], name: &str) -> Result<(), String> { /// The `key=value` pairs on a unit line. `@0xfed90000` carries no `=` and is /// skipped, which is what makes the split total rather than a parse. -fn unit_fields(line: &str) -> BTreeMap { +pub(crate) fn unit_fields(line: &str) -> BTreeMap { line.split_whitespace() .filter_map(|word| word.split_once('=')) .map(|(k, v)| (k.to_string(), v.to_string())) diff --git a/tests/toyos.rs b/tests/toyos.rs index d6dee46a3cd..f79acc8f893 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -2807,8 +2807,12 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> { .nth(1) .and_then(|rest| rest.split_whitespace().next()) .ok_or_else(|| format!("an unreadable unit line: {line:?}"))?; - let remaps = - log.lines().any(|l| l.contains(&format!("iommu: {unit} @")) && l.contains(" ir=y ")); + let remaps = log + .lines() + .filter(|l| l.contains(&format!("iommu: {unit} @"))) + .find_map(|l| common::iommu::unit_fields(l).remove("ir")) + .ok_or_else(|| format!("{unit} translates and no line describes its ir:\n{log}"))? + == "y"; let left = ["translation", "queued invalidation"] .into_iter() .chain(remaps.then_some("interrupt remapping")); From 25ab31bd57e2e1e0a73a3ad73f8b9f99f795e403 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 00:39:58 +0200 Subject: [PATCH 4/7] VT-d: the firmware-left actuator leaves a unit translating through an identity domain The T14's selftests boot at 9be0a8ac8 panicked at 0.156 s on a read fault from the integrated GPU (00:02.0, reason 0x06, 0x9c855000, inside rmrr0) once unit0 translated; at 55a5ccb42 the same boot passed all ten rows. `leave_on` left each unit translating through an empty root table: every request from every function behind it faults, the GPU's scanout out of its reserved region included, which no firmware leaves a unit doing (a reserved region stays mapped while translation is on). At 55a5ccb42 that state lasted from `leave_on` to `hand_over`, microseconds; with the hand-over after the table build (review round 1, BLOCKER 1) it lasts the 8956-leaf build, 0.153 s to 0.156 s on the T14, with the GPU faulting the whole time. The unit's fault records overflowed in it, and the fault that killed the boot came on the GPU's next scanout read once unit0 translated through this kernel's tables. `leave_on` now builds an identity domain of its own and gives every enumerated function a context entry naming it, as this kernel leaves a unit, so the state the actuator stages is one firmware may hand over and the hand-over stays after the build. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/arch/x86_64/vtd/mod.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index a57385dba43..f524964ed94 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -324,11 +324,22 @@ impl Unit { /// is for, left the way this kernel leaves a unit, each batch ended by a /// wait. `CFI` is written as firmware writes it, unconfirmed: whether /// `GSTS` reports it is the unit's to say. + /// + /// Through an identity domain of its own, never an empty root: a unit left + /// translating through tables that miss a reserved region faults its + /// device's DMA for as long as it is left so, which firmware may not do. #[cfg(feature = "boot-actuators")] - fn leave_on(&self) { + fn leave_on(&self, devices: &[PciDevice], width: AddressWidth) { let (mut queue, root, remap) = { let mut tables = TABLES.lock(); - (Queue::new(&mut tables, self.regs), tables.alloc(), tables.alloc()) + let queue = Queue::new(&mut tables, self.regs); + let (domain, _) = table::identity_domain(&mut tables, width, crate::mm::pmm::top()); + let root = tables.alloc(); + for device in devices { + let stream = StreamId::pci(device.bus, device.dev, device.func); + table::bind_identity(&mut tables, root, stream, domain, width); + } + (queue, root, tables.alloc()) }; self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); self.regs.write_u64(RTADDR_REG, root.phys()); @@ -503,7 +514,7 @@ fn enable( let Plan { width, records } = plan; #[cfg(feature = "boot-actuators")] if crate::actuator::iommu_firmware_left() { - unit.leave_on(); + unit.leave_on(devices, width); } // 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. From 5e8cbb06eaa9156be8df88a5209043f1168e1d49 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 01:10:43 +0200 Subject: [PATCH 5/7] VT-d: the firmware-left actuator and enable share one identity domain and one root builder Review round 2, BLOCKER 3: `leave_on` repeated `enable`'s identity-domain build and its per-function `bind_identity` loop, and built a fresh 8956-leaf domain for every unit, widening the T14's build window from 0.153-0.156 s to 0.153-0.158 s. `enable` now looks up (or builds) the per-width identity domain before the actuator runs and hands it to `leave_on`; both take their root table from one function, `identity_root`, which allocates a root and gives every enumerated function a context entry in that domain. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- kernel/src/arch/x86_64/vtd/mod.rs | 62 +++++++++++++------------------ 1 file changed, 25 insertions(+), 37 deletions(-) diff --git a/kernel/src/arch/x86_64/vtd/mod.rs b/kernel/src/arch/x86_64/vtd/mod.rs index f524964ed94..2b69d91e2be 100644 --- a/kernel/src/arch/x86_64/vtd/mod.rs +++ b/kernel/src/arch/x86_64/vtd/mod.rs @@ -325,20 +325,15 @@ impl Unit { /// wait. `CFI` is written as firmware writes it, unconfirmed: whether /// `GSTS` reports it is the unit's to say. /// - /// Through an identity domain of its own, never an empty root: a unit left + /// Through the identity domain, never an empty root: a unit left /// translating through tables that miss a reserved region faults its /// device's DMA for as long as it is left so, which firmware may not do. #[cfg(feature = "boot-actuators")] - fn leave_on(&self, devices: &[PciDevice], width: AddressWidth) { + fn leave_on(&self, devices: &[PciDevice], domain: Table, width: AddressWidth) { let (mut queue, root, remap) = { let mut tables = TABLES.lock(); let queue = Queue::new(&mut tables, self.regs); - let (domain, _) = table::identity_domain(&mut tables, width, crate::mm::pmm::top()); - let root = tables.alloc(); - for device in devices { - let stream = StreamId::pci(device.bus, device.dev, device.func); - table::bind_identity(&mut tables, root, stream, domain, width); - } + let root = identity_root(&mut tables, devices, domain, width); (queue, root, tables.alloc()) }; self.command(QUEUED_INVALIDATION_ENABLE, true, "queued invalidation"); @@ -512,39 +507,21 @@ fn enable( ) { let index = unit.index; let Plan { width, records } = plan; - #[cfg(feature = "boot-actuators")] - if crate::actuator::iommu_firmware_left() { - unit.leave_on(devices, width); - } // 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()); - let root = { - let mut tables = TABLES.lock(); - let domain = match domains[domain_slot(width)] { - Some(domain) => domain, - None => { - let top = crate::mm::pmm::top(); - let (domain, frames) = table::identity_domain(&mut tables, width, top); - log!( - "iommu: identity domain aw={} covers 0x0..{top:#x} in {frames} 2 MiB leaves", - width.bits() - ); - domains[domain_slot(width)] = Some(domain); - domain - } - }; - - // Every enumerated function needs a context entry before `TE`: one - // missing bricks the boot disk instead of merely faulting later. - let root = tables.alloc(); - for device in devices { - let stream = StreamId::pci(device.bus, device.dev, device.func); - table::bind_identity(&mut tables, root, stream, domain, width); - } - root - }; + 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); + } + 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 @@ -596,6 +573,17 @@ fn enable( ); } +/// A root table giving every enumerated function a context entry in `domain`: +/// one missing before `TE` bricks the boot disk instead of merely faulting later. +fn identity_root(tables: &mut Tables, devices: &[PciDevice], domain: Table, width: AddressWidth) -> Table { + let root = tables.alloc(); + for device in devices { + let stream = StreamId::pci(device.bus, device.dev, device.func); + table::bind_identity(tables, root, stream, domain, width); + } + root +} + /// Slot of the per-width domain cache; exhaustive match so a new `AddressWidth` fails to compile here. fn domain_slot(width: AddressWidth) -> usize { match width { From 6fdd9c57961652684c0324f12343a28252a603ce Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 01:48:50 +0200 Subject: [PATCH 6/7] issues: a function refused under translation faults read permission after the hand-over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round 2's BLOCKER 1 on pull request #700. The T14 probe boot at 9be0a8ac8 (FSTS probe) shows unit0's one record, before fault::arm clears it, is 00:02.0 reading 0x9cb80000 with reason 0x01 and FSTS=0x3: the empty root's refusals, overflowed. Once TE is confirmed FSTS is 0 and the record is clear, so the overflow and the fatal 0x06 at 0x9cc39000 that follow were raised through this kernel's identity domain, which grants the read. Against VT-d Rev. 4.1: 0x06 is LGN.3, a read meeting a second-stage entry without R. Every entry on the walk grants it. The context entry is TT=00, AW=2, DID=1 in every arm. A CM=0 unit caches no faulted translation (§6.2.4.1), and the SRTP is followed by acknowledged global context-cache and IOTLB invalidations (§6.6). Nothing the spec describes carries the refusal across TE off and on. 55a5ccb42 (refused only within the hand-over's millisecond, TE off for 2 ms) and 25ab31bd5/5e8cbb06e (no refusal) did not fault. Firmware leaves the T14's units off, and today's actuator refuses nothing, so the state is no longer reached. It is filed with the reproduction as its exit; two probe images at 9be0a8ac8 (raw record, PCI STATUS, the walk as DRAM holds it, and a 3 ms TE-off arm) are staged for the T14. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...lts-read-permission-after-the-hand-over.md | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md diff --git a/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md b/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md new file mode 100644 index 00000000000..4fcc27a1940 --- /dev/null +++ b/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md @@ -0,0 +1,55 @@ +--- +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. + +Ruled out by reading: the identity domain's leaf for `0x9cc00000` is +`SL_READ | SL_WRITE | SL_LARGE`, in tables built by the code the green boots +ran. Each line is written back with `clflush` (`ECAP.C=0`). The context entry +is `TT=00`, `AW=2`, `DID=1`, the same in every arm. `0x01`, not `0x06`, is +what the empty root answers. + +Staged on pull request #700, not yet run: `9be0a8ac8` plus a probe that logs +the fatal record raw (`AT`, `PP`, `PRIV`, `EXE`, `T2`), the GPU's PCI `STATUS` +before the actuator, before the hand-over, at `TE` and at the fault, and the +unit's walk for the fault from `RTADDR` down, each entry read from DRAM after +`clflush`. A second image adds 3 ms with `TE` off after the hand-over. + +**Exit**: on the T14, the reproduction (the selftests boot at `9be0a8ac8`) +reaches `Boot: complete` with no `DMA FAULT` line, with the hand-over or +programming changed to whatever the probe readings name. From 041014fdf4da57368e814b58429d3918e2442a33 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 4 Oct 2026 02:22:05 +0200 Subject: [PATCH 7/7] issues: the read-permission fault carries the walk and gap readings, and its exit refuses a timed hold The walk probe read the unit's tables from DRAM at the fault: every entry grants the read, the record is a plain untranslated read and the GPU saw no abort. The gap probe, 3 ms with TE off after the hand-over, booted green. That replaces the staged-not-run paragraph and the leaf claim made by reading. A timed hold is a flat wait no document mandates and widens the untranslated window already filed, so the exit asks for a hand-over that waits on an event instead. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01WcU2Dsw6mDYtwYfzVHPzM8 --- ...lts-read-permission-after-the-hand-over.md | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md b/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md index 4fcc27a1940..7750b7ff414 100644 --- a/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md +++ b/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md @@ -38,18 +38,32 @@ Evidence, pull request #700, T14 boots: - `25ab31bd5` and `5e8cbb06e`, no refusal, and the hand-over and `translating` logged in the same millisecond: no fault. -Ruled out by reading: the identity domain's leaf for `0x9cc00000` is -`SL_READ | SL_WRITE | SL_LARGE`, in tables built by the code the green boots -ran. Each line is written back with `clflush` (`ECAP.C=0`). The context entry -is `TT=00`, `AW=2`, `DID=1`, the same in every arm. `0x01`, not `0x06`, is -what the empty root answers. - -Staged on pull request #700, not yet run: `9be0a8ac8` plus a probe that logs -the fatal record raw (`AT`, `PP`, `PRIV`, `EXE`, `T2`), the GPU's PCI `STATUS` -before the actuator, before the hand-over, at `TE` and at the fault, and the -unit's walk for the fault from `RTADDR` down, each entry read from DRAM after -`clflush`. A second image adds 3 ms with `TE` off after the hand-over. +- `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, with the hand-over or -programming changed to whatever the probe readings name. +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/kernel/a-unit-left-translating-passes-dma-untranslated-while-programmed.md` +records, and it rests on one green boot against three reds.