amdvi: the firmware-left actuator invalidates with one INVALIDATE_IOMMU_ALL, and its metal row judges AMD-Vi units

issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md

@@ -0,0 +1,33 @@
+---
+status: open
+kind: tooling
+opened: 2026-10-12
+---
+
+# QEMU's `INVALIDATE_IOMMU_ALL` leaves every function untranslated
+
+AMD 48882 §2.4.8: once `INVALIDATE_IOMMU_ALL` completes, "all IOMMU caches
+are empty", so the unit reads every device table entry afresh. QEMU 11.1.1's
+`amd-iommu` does not. With `dma-remap=on` a function's address space starts
+on `iommu_nodma`, which passes its DMA untranslated, and moves to
+translation only in `amdvi_update_addr_translation_mode`, whose one caller is
+`amdvi_inval_devtab_entry`. `amdvi_inval_all` resets the IOTLB, notifies the
+interrupt caches and replays the address spaces that have notifiers, and
+never reads an entry's mode (`hw/i386/amd_iommu.c` at `c3d48b7d`,
+`:1249-1313` and `:1332-1351`).
+
+So under `iommu-firmware-left`, which invalidates with one
+`INVALIDATE_IOMMU_ALL` (`kernel/src/arch/x86_64/amdvi/mod.rs`, `leave_on`),
+QEMU passes every function's DMA through a unit whose every entry blocks it.
+`amdvi_firmware_left`'s second boot binds the stick whether or not the
+switch-off runs: its negative control reds only on the register half, and the
+DMA half is held by the AMD bench's `iommu_firmware_left` row alone, in
+whatever run the orchestrator stages.
+
+Owner: the orchestrator's AMD laptop track
+(`issues/an-amd-vi-machine-isolates-no-device.md`), whose translation stage
+is the next code to rest on what QEMU does on this command.
+
+**Exit:** under the QEMU `build/ci/qemu-version` names, `amdvi_firmware_left`
+with `switch_off` skipping every field reds on `usb-storage: 1 device(s)`,
+and the test's doc says that boot's stick never binds.

kernel/src/arch/x86_64/amdvi/mod.rs

@@ -88,7 +88,7 @@ pub fn init(rsdp_addr: u64) -> bool {
                 if let Some(regs) = present(units, &unit) {
                     #[cfg(feature = "boot-actuators")]
                     if crate::actuator::iommu_firmware_left() {
-                        leave_on(regs, &ivrs, &unit);
+                        leave_on(regs);
                     }
                     switch_off(units, &unit, regs);
                     off += 1;
@@ -261,15 +261,14 @@ fn switch_off(index: usize, unit: &Unit, regs: Mmio) {
 /// protecting memory before the operating system runs may leave it — every
 /// requester's device table entry translating through a table that maps
 /// nothing, so every DMA is blocked, with its command buffer and event log
-/// running. The entry of every requester the unit's IVHD names is
-/// invalidated, which is what a unit reads a changed entry on, and a
-/// completion wait says each was.
+/// running. One `INVALIDATE_IOMMU_ALL` empties every cache the unit holds,
+/// so it reads every entry as written, and a completion wait says it did.
 ///
 /// The tables are leaked, as firmware's are its own: the unit reads them
 /// until it is switched off, and freed they could read back as an entry that
 /// passes DMA untranslated.
 #[cfg(feature = "boot-actuators")]
-fn leave_on(regs: Mmio, ivrs: &Ivrs<impl Phys>, unit: &Unit) {
+fn leave_on(regs: Mmio) {
     use crate::mm::{pmm, DirectMap};
 
     const DEVICE_TABLE: u64 = 0x0000;
@@ -281,17 +280,26 @@ fn leave_on(regs: Mmio, ivrs: &Ivrs<impl Phys>, unit: &Unit) {
     const EVENT_TAIL: u64 = 0x2018;
     /// 2^8 entries of 16 bytes, 4 KiB, in bits 59:56 of either base.
     const LOG_LEN: u64 = 8 << 56;
-    const RING: usize = 1 << 8;
     const DEVICE_IDS: u64 = 1 << 16;
     const ENTRY: u64 = 32;
     /// `V`, `TV`, and a four-level table: `Mode` 100b at 11:9; `IR` and `IW`
     /// clear.
     const BLOCKED: u64 = 1 | (1 << 1) | (4 << 9);
-    const INVALIDATE_ENTRY: u64 = 2 << 60;
+    /// `IASup`, set where the unit takes `INVALIDATE_IOMMU_ALL`.
+    const IA_SUP: u64 = 1 << 6;
+    /// The opcode alone: every other bit of the command is reserved.
+    const INVALIDATE_IOMMU_ALL: u64 = 8 << 60;
     /// The opcode, and `S`: store the second word at the address.
     const COMPLETION_WAIT: u64 = (1 << 60) | 1;
     const DONE: u64 = 0x600d_f00d;
 
+    let efr = regs.read_u64(EXTENDED_FEATURES);
+    assert!(
+        efr & IA_SUP != 0,
+        "amdvi: the unit's EFR {efr:#018x} has IASup clear, so it takes no INVALIDATE_IOMMU_ALL and the actuator \
+         cannot make it read the entries it leaves"
+    );
+
     let window = |phys: u64, bytes: u64| {
         // SAFETY: `phys` is inside a page `pmm::alloc_page` handed out and
         // this function leaks, which the direct map covers for the machine's life.
@@ -318,31 +326,12 @@ fn leave_on(regs: Mmio, ivrs: &Ivrs<impl Phys>, unit: &Unit) {
     }
     regs.write_u64(CONTROL, regs.read_u64(CONTROL) | CMD_BUF_EN | EVENT_LOG_EN | IOMMU_EN);
 
-    let commands = window(commands_at, RING as u64 * 16);
-    let mut tail = 0;
-    let mut push = |low: u64, high: u64| {
-        // A ring holds one entry fewer than it has.
-        assert!(tail < (RING as u64 - 1) * 16, "amdvi: the unit's requesters overflow the actuator's {RING}-entry ring");
-        commands.write_u64(tail, low);
-        commands.write_u64(tail + 8, high);
-        tail += 16;
-    };
-    for entry in ivrs.devices(unit) {
-        let (first, last) = match entry.expect("amdvi: the actuator's unit has a device list it can walk") {
-            DeviceEntry::All { .. } => (0, u16::MAX),
-            DeviceEntry::Select { id, .. } | DeviceEntry::Extended { id, .. } | DeviceEntry::Hid { id, .. } => (id, id),
-            DeviceEntry::Range { first, last, .. } | DeviceEntry::ExtendedRange { first, last, .. } => (first, last),
-            DeviceEntry::Alias { used, .. }
-            | DeviceEntry::AliasRange { used, .. }
-            | DeviceEntry::Special { used, .. } => (used, used),
-            DeviceEntry::Other(_) => continue,
-        };
-        for id in first..=last {
-            push(INVALIDATE_ENTRY | u64::from(id), 0);
-        }
+    let commands = [INVALIDATE_IOMMU_ALL, 0, COMPLETION_WAIT | done_at, DONE];
+    let ring = window(commands_at, commands.len() as u64 * 8);
+    for (word, value) in (0..).zip(commands) {
+        ring.write_u64(word * 8, value);
     }
-    push(COMPLETION_WAIT | done_at, DONE);
-    regs.write_u64(COMMAND_TAIL, tail);
+    regs.write_u64(COMMAND_TAIL, commands.len() as u64 * 8);
     let done = window(done_at, 8);
     spin_until(
         || done.read_u64(0) == DONE,

tests/common/iommu.rs

@@ -440,15 +440,26 @@ const NO_DOMAIN: &str = "iommu: no domain of its own for a device: no unit on th
 /// which only DMA the unit lets through can do.
 const STICK_BOUND: &str = "usb-storage: 1 device(s)";
 
+/// What the actuator leaves on an AMD-Vi unit, in the kernel's words for
+/// `unit`: one line per field the hand-over switches off by its own write.
+pub(crate) fn amdvi_left_on(unit: &str) -> [String; 3] {
+    ["CmdBufEn", "EventLogEn", "IommuEn"]
+        .map(|field| format!("iommu: amdvi {unit} was handed over with {field} on; it goes off"))
+}
+
 /// **A unit an AMD machine's firmware left on is switched off, and the boot
 /// stick's DMA reaches memory.** QEMU's `amd-iommu` is the only AMD-Vi unit
-/// in reach: the T14 is an Intel machine, and the hand-over's register
-/// sequence and the DMA it frees are neither of them a host test's. Two
-/// boots: the shipped kernel finds a unit nothing left on and switches
+/// the merge queue runs: the T14 is an Intel machine, and the hand-over's
+/// register sequence and the DMA it frees are neither of them a host test's.
+/// Two boots: the shipped kernel finds a unit nothing left on and switches
 /// nothing; the test kernel's `iommu-firmware-left` leaves the unit blocking
 /// every device's DMA with its command buffer and event log running, and
 /// each goes off by its own write before the stick is driven. With the
-/// switch-off reverted that boot's stick never binds.
+/// switch-off reverted that boot reds on the fields' lines and on `control`
+/// reading back zero, and its stick still binds: QEMU 11.1.1 moves no
+/// function to translation on `INVALIDATE_IOMMU_ALL`
+/// (`issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md`),
+/// so the AMD bench's `iommu_firmware_left` row holds the DMA half.
 pub fn amdvi_firmware_left(test_config: &Path) -> Result<(), String> {
     let profile = Profile::MetalAmdVi;
     let argv = qemu::profile_argv(&BootOptions { profile, ..Default::default() });
@@ -474,12 +485,12 @@ pub fn amdvi_firmware_left(test_config: &Path) -> Result<(), String> {
         log.must_not_say("no DMAR or IVRS table")?;
         let handed = log.must_say("iommu: amdvi unit0 handed over control=")?;
         if left {
-            // `CmdBufEn`, `EventLogEn` and `IommuEn`, as the actuator wrote them.
+            // [`amdvi_left_on`]'s fields, as the actuator wrote them.
             if !handed.contains("control=0x0000000000001005") {
                 return Err(format!("the actuator's unit was handed over as {handed:?}"));
             }
-            for field in ["CmdBufEn", "EventLogEn", "IommuEn"] {
-                log.must_say(&format!("iommu: amdvi unit0 was handed over with {field} on; it goes off"))?;
+            for line in amdvi_left_on("unit0") {
+                log.must_say(&line)?;
             }
             for field in ["EventIntEn", "GALogEn", "GAIntEn", "PPRLogEn", "PPRIntEn"] {
                 log.must_not_say(&format!("with {field} on"))?;

tests/toyos.rs

@@ -992,9 +992,10 @@ const METAL: &[(&str, metal::Metal)] = &[
         },
     ),
     (
-        // 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.
+        // A unit handed over translating, remapping and queueing, or on an
+        // AMD-Vi machine blocking every DMA with its command buffer and event
+        // 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()) },
     ),
@@ -5213,8 +5214,11 @@ 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.
+/// 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.
+///
+/// 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 mut units = 0;
         for line in log.lines().filter(|l| l.contains(" translating gsts=")) {
@@ -5242,13 +5246,32 @@ fn iommu_firmware_left(log: &str) -> Result<(), String> {
             }
             units += 1;
         }
+        let amdvi = log
+            .lines()
+            .filter_map(|l| l.split_once("iommu: amdvi ")?.1.split_once(" handed over control="))
+            .map(|(unit, _)| unit);
+        for unit in amdvi {
+            for line in common::iommu::amdvi_left_on(unit) {
+                if !log.contains(&line) {
+                    return Err(format!(
+                        "amdvi {unit} never said {line:?}, so the state the actuator left was \
+                         never switched off:\n{log}"
+                    ));
+                }
+            }
+            let off = format!("iommu: amdvi {unit} @");
+            if !log.lines().any(|l| l.contains(&off) && l.contains(" switched off control=")) {
+                return Err(format!("amdvi {unit} was handed over and never said it was switched off:\n{log}"));
+            }
+            units += 1;
+        }
         if units == 0 {
-            return Err(format!("no unit was programmed, so nothing was handed over:\n{log}"));
+            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();
         eprintln!(
-            "  [iommu-firmware-left] {units} unit(s) programmed after switching 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; \
+             {passed} of them reported compatibility format passed"
         );
         Ok(())
 }