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..7750b7ff414 --- /dev/null +++ b/issues/kernel/a-function-refused-under-translation-faults-read-permission-after-the-hand-over.md @@ -0,0 +1,69 @@ +--- +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/kernel/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/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..7e10da87b20 --- /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 from no +release). The slot's next holder is attached to that same domain +(`slot_space`), so the released function's requests translate through the +next holder's grants for as long as it is that holder's. Nothing but the +released function's own obedience to `BME` stands between them: a function +that masters the bus with `BME` clear reaches another process's memory. + +**Exit**: a released function's context entry names no domain once it is +quiet, read back off the unit's table on the T14 after its slot's next claim. diff --git a/issues/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/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..2b69d91e2be 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,85 @@ 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. + /// + /// 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], domain: Table, width: AddressWidth) { + let (mut queue, root, remap) = { + let mut tables = TABLES.lock(); + let queue = Queue::new(&mut tables, self.regs); + 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.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 +406,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 +437,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 +499,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], @@ -447,72 +511,44 @@ fn enable( // narrowest thing every translating unit on this machine agrees to. domain::unit_agrees(width, unit.caps.domains(), unit.caps.mgaw()); - let (root, queue) = { - 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); - } - - 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_ENABLE, - "queued invalidation", - ); - unit.regs.write_u64(RTADDR_REG, root.phys()); - unit.command(SET_ROOT_TABLE_POINTER, false, ROOT_TABLE_SET, "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", - ); - 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. + 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 + // (`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::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. @@ -537,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 { 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/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 486edcdcd90..f79acc8f893 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,46 @@ 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() + .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")); + 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> {