Skip to content

AMD-Vi translates: a blocked device table, identity and own domains, the event log, and one domain registry for every backend - #909

Merged
Japabu merged 6 commits into
mainfrom
wt/toyos-amdvi
Oct 14, 2026
Merged

Japabu merged 6 commits into
mainfrom
wt/toyos-amdvi

Conversation

@Japabu

@Japabu Japabu commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator

Stage 5 of issues/an-amd-vi-machine-isolates-no-device.md: AMD-Vi translation, built on the design accepted here with review blockers B1–B5 as named changes. Stage 6, interrupt remapping, is a second PR. Draft. The AMD bench rows ran green at this tree, and the control went red (below).

What changes

On a machine whose IVRS describes AMD-Vi units, the kernel programs every unit it can use instead of leaving it off:

  • One device table every unit reads (2 MiB, 65,536 entries), each requester id blocked (V, TV, Mode 000b, IR=IW=0, domain 0) but every enumerated function's, which names the identity domain (1) over [0, pmm::top()). All written before IommuEn (S1).
  • Each unit gets a command buffer, an event log, its exclusion range cleared, and its own MSI armed through its PCI function. That is the one message no unit remaps (Capability Offset 10h); the bench's unit has MSI and no MSI-X. Then CONTROL, then INVALIDATE_IOMMU_ALL and one INVALIDATE_DEVTAB_ENTRY per enumerated function (B4), behind a completion wait under a Tripwire.
  • Kernel drivers get domains of their own (the bench's xHCIs 03:00.3/03:00.4). Every map, unmap and attach is published by INVALIDATE_IOMMU_PAGES, plus INVALIDATE_DEVTAB_ENTRY for a move (§2.4.2). The move swaps the entry's translation half in one cmpxchg16b (§2.2.2.2).
  • The event log drains on DMA_FAULT_VECTOR into iommu::fault. Every record a requester can cause counts against that requester and never stops the machine on its own: IO_PAGE_FAULT, IOTLB_INV_TIMEOUT, INVALID_DEVICE_REQUEST, and INVALID_PPR_REQUEST with RX clear (Figure 64, Table 67). Only a record of this driver's tables or commands, or of the unit's own hardware, panics, because past it nothing can say what the unit lets through. An overflow restarts the log (§2.5.1).
  • A function no programmed unit serves keeps the machine's addresses (DeviceSpace::create answers Untranslated) and the log says why, so one refused unit costs isolation only behind itself (orchestrator's ruling on review B3).
  • Refused by name, unit left off:
    • a segment other than 0;
    • no IASup;
    • a reserved HATS;
    • device entries refused;
    • an IVRS alias that makes an enumerated function request under another id;
    • a unit function without an MSI capability.
  • IVMD (B3), VT-d's RMRR policy:
    • A unity range is logged inside or OUTSIDE the identity domain.
    • Its requesters keep the identity domain: DeviceSpace::create now takes the function and answers Untranslated for them.
    • An IVMD with ExclusionRange set, or with Unity clear (Table 113: "any virtual address translation may be used"), leaves every unit off, logged by range and by which of the two it is.
    • The bench declares no IVMD, so none of this runs there.

Files: kernel/amdvi/ (new pure crate toyos-amdvi), kernel/src/arch/x86_64/amdvi/{mod,domain,fault,selftest}.rs, kernel/src/arch/x86_64/iommu_unit.rs (backend dispatch for domain and fault; the shared cmpxchg16b and MAX_UNITS), kernel/src/iommu/domains.rs.

Commits

  1. b3e77cac4: the design round's measurement line (the unit's function caps, misc0, exclusion range).
  2. 810626cf1 (B1): one domain registry, iommu::domains::Domains<S>, for VT-d and the SMMU. Ids, windows, the alignment refusal, the handed-out check and the records move into it; each backend keeps its tables, invalidation and lock. Net −10 lines (232+/242−). One behaviour moves: a VT-d unmap refused part-way now invalidates what it already cleared, as the SMMU's did.
  3. 9173c339b: AMD-Vi as the third backend of that registry.

Net for the branch: +2,054 / −529:

  • toyos-amdvi: 307 lines of source and 172 of host tests;
  • the kernel backend and dispatch (amdvi/, iommu_unit.rs): 998+/134−;
  • the harness (guest test, profile, row): 159+/31−.

Decisions carried from review

  • B2 (claims clear pass bits and SysMgt) is stage 6's: no claim is handed over on AMD-Vi until remapping exists (NotRemapped). Stage 5 entries carry the IVHD byte's SysMgt alone (Table 106 → DTE bits 105:104), which the IOAPIC's 00:14.0 needs (data=0xd7).
  • B5 closed. 48882 rev 3.11 was fetched from AMD's own documentation hub (docs.amd.com Fluid Topics API, document 2oKZlc87KNCKiT3B~xJjHg, 48882_3.11_IOMMU_PUB.pdf) with the build user-agent. The owner downloaded it from AMD as well. Both have SHA-256 f7c375a15db5ed63de760356867211063d164a2ed59f2d38613daec95894ce22, equal to the copy the design read (record). 48882 is the host tests' oracle, cited per assertion.
  • S2 declined. QEMU's IVHD carries flags 0x11, Coherent clear, so refusing such a unit would refuse the only AMD-Vi unit CI has. CONTROL.Coherent's reset value and the value Table 97 recommends are both 1, and the driver sets it. The bench's IVHD has it set (0xb0).
  • S1, S4 and S5 taken. FIRMWARE_LEFT's order is now stated by the specification's reason, and describe_function is renamed. The figure citation for the 128-bit IRTE goes to stage 6.
  • Leaves are written FC=1, so a device's no-snoop cannot read stale memory. This is AMD's equivalent of what VT-d's snoop control gives HDA.

Checks (head 9173c339b; tree equal to a2e53ebfa, which every run below except the first three used)

command target exit log
cargo run -- --build-only / --arch aarch64 at 810626cf1 both images 0 / 0 x86, arm
cargo test --test toyos-build -- virt_smmu usbd_drives_the_spare iommu_virtio_platform at 810626cf1 QEMU (VT-d, SMMUv3) 0 guest-b1.log
cargo test -p toyos-amdvi (inside --ci host) host, 12 encoding tests 0 s5-ci-host2.log
cargo run -- --ci host host, 76 steps 0 s5-ci-host2.log (the first run was red on the bus-master gate and three clippy lints, all fixed)
cargo test --test toyos-build -- --nocapture amdvi_selftest amdvi_firmware_left usbd_drives_the_spare iommu_virtio_platform virt_smmu QEMU 11.1.1 amd-iommu, intel-iommu, SMMUv3 0 guest-final.log
cargo run -- --build-only --arch aarch64 aarch64 image 0 final-arm.log
AMD bench (orchestrator), images built at a2e53ebfa: amdvi_translates_the_stick_controller, domain_ends_below_the_host_bridges, iommu_firmware_left AMD bench, 2 boots 3 passed, 0 failed judge, testcases kernel log, shared-debug kernel log, runner
AMD bench, control units-left-off.patch: amdvi_translates_the_stick_controller AMD bench, 1 boot 1 (red: "no AMD-Vi unit is translating") judge, kernel log

Negative controls:

  • no-per-device-invalidation.patch: INVALIDATE_IOMMU_ALL alone at boot. amdvi_selftest red, exit 1 (log). The device answered 0xdead0002 and no record was read. On this 40-bit guest a write passed through untranslated to 2^46 also reaches nothing, so the answer cannot tell translation from pass-through; the missing record is the discriminator. This matches the QEMU issue's premise (pass-through until an INVALIDATE_DEVTAB_ENTRY names the function), so issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md stays open because the defect persists, and the boot's per-function commands are there for it alone (§2.4.8 needs INVALIDATE_IOMMU_ALL only).
  • ppr-is-the-units.patch: INVALID_PPR_REQUEST (RX=0) decoded as the unit's. a_record_a_requester_causes_is_its_own red, exit 101 (log).
  • units-left-off.patch: every unit left off, the base's behaviour. On the AMD bench, amdvi_translates_the_stick_controller is red with "no AMD-Vi unit is translating", and the log says AMD-Vi isolates no device this boot: 1 unit(s) described, none usable. The stick still binds there, so the row's red comes from the unit lines and not from the stick.

The head's bench log against the design (testcases kernel log):

  • Unit translating: control=0x000022000000140d (IommuEn, EventLogEn, EventIntEn, Coherent, CmdBufEn) and status=0x00000018 (CmdBufRun, EventLogRun), so the command buffer and event log are running. Its MSI is armed at 0xfee00000/0x0025 (DMA_FAULT_VECTOR) on its function 00:00.2. 28 enumerated functions have identity entries.
  • The identity domain covers 0x0..0x40f200000.
  • The stick's controller 03:00.3 moves to domain2 and is mapped at 0x400000000000 (2^46), with addresses from 2^46 to 2^48. 03:00.4 is on domain3. The stick binds (usb-storage: 1 device(s)) through that translation, and no DMA FAULT appears in either boot.
  • No IVMD: the bench declares none, so the IVMD handling did not run there; only a host-level read covers it.

Fix round (head e7ab4648c)

Build review B1–B4 and the orchestrator's rulings: requester records never panic (B1); the control's reading corrected and the track issue's QEMU constraint restored (B2); unserved functions keep machine addresses (B3, ruled); the IVMD label follows Table 113 (B4). Also: CONTROL written whole rather than OR-ed onto firmware's fields; untranslated (formerly firmware_reserves) documents that VT-d answers None for every function, an RMRR's included; the device table keeps its own page field; the domain width comes from the root level.

command target exit log
cargo run -- --build-only x86_64 image 0 r2-x86.log
cargo test --test toyos-build -- --nocapture amdvi_selftest amdvi_firmware_left QEMU 11.1.1 amd-iommu 0 r2-guest.log
cargo run -- --ci host (13 toyos-amdvi tests among 76 steps) host 0 r2-ci-host.log

| AMD bench (orchestrator), image built at e7ab4648c: amdvi_translates_the_stick_controller, domain_ends_below_the_host_bridges | AMD bench, 1 boot | 2 passed, 0 failed | judge, kernel log, runner |

The head's bench reading sits beside the a2e53ebfa set above. CONTROL is now written whole, so the unit reads translating control=0x000000000000140d status=0x00000018: firmware's bits 57 and 61 are gone, and the unit, its command buffer and its event log are running.

Merges of origin/main (head bbc9b924a)

Neither merge changes kernel/src/arch/x86_64/amdvi/ or the AMD-Vi code in iommu/, so no bench boot is owed.

command revision exit log
cargo run -- --build-only, x86_64 and aarch64 4cd56a35b, bbc9b924a 0, 0 each m-build, m2-build
cargo test --test toyos-build -- amdvi_selftest amdvi_firmware_left virt_smmu virt_claim_lpi 4cd56a35b 0 m-guest
cargo test --test toyos-build -- amdvi_selftest amdvi_firmware_left bbc9b924a 0 m2-guest
cargo run -- --ci host, 77 steps 4cd56a35b, bbc9b924a 0, 0 m-ci-host, m2-ci-host

Tests

  • Host (kernel/amdvi/tests/encodings.rs): every word against 48882's own field positions. Covered: the blocked and translated DTE, the IVHD byte's SysMgt, the device-table and ring registers, level indices, directory and 2 MiB leaf entries, the five commands, IO_PAGE_FAULT/INVALID_DEVICE_REQUEST decode with every other code named, and IASup/HATS decoded on the bench's EFR.

  • Guest amdvi_firmware_left (changed): the unit is programmed after the switch-off, and the boot stick's controller moves to domain2 and binds, in both arms.

  • Guest amdvi_selftest (new; profile MetalAmdViTestdev: Profile::MetalAmdVi plus two iommu-testdev, shared with the SMMU's selftest through kernel/src/iommu/testdev.rs):

    • one testdev on its own domain lands a write where its domain maps it;
    • the other, on the identity domain, lands one in memory;
    • that one is then refused where the identity domain maps nothing, recorded owner=kernel as IO_PAGE_FAULT, and the machine halts.

    Why a guest: a kernel-owned record halts the machine, so no metal row reaches the event log of a unit that translates for the kernel alone, and only iommu-testdev aims a DMA where nothing maps. QEMU's record carries no address, so the test holds the record to its requester (filed: issues/qemus-amd-iommu-page-fault-record-carries-no-address.md, raw words in guest-s5-debug.log).

  • Metal row amdvi_translates_the_stick_controller (AMD bench, testcases boot): every unit translating, every xHCI the kernel drives moved to a domain of its own, the stick bound, no DMA FAULT, no "isolates no device". domain_ends_below_the_host_bridges and iommu_firmware_left ride the same request.

Risks

  • The blocked default for unenumerated requesters has no oracle on any target: QEMU passes Mode 0 through, and no known requester on the bench DMAs unenumerated.
  • claim_reuses_its_remapping_entry stays T14-only (issues/a-metal-row-names-no-hardware-and-a-bench-without-it-reds.md), and a T14 run of amdvi_translates_the_stick_controller reds for want of an AMD unit, for the same reason.

Design-round evidence: d1.

🤖 Generated with Claude Code

https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy

…irmware left

The AMD-Vi translation and remapping design (issues/an-amd-vi-machine-isolates-no-device.md,
stages 5 and 6) arms the unit's event interrupt through its own PCI
function, which AMD 48882 §3.1 lets carry MSI, MSI-X or both, and must
clear an exclusion range firmware may leave, inside which DMA is neither
translated nor checked (§3.4.1, MMIO 0020h and 0028h). Neither is read
today, so the bench's answer is unknown.

Each unit now logs its function's capability ids and its Miscellaneous
Information Register 0 (Capability Offset 10h: VAsize, PAsize, MsiNum),
and the hand-over line carries the exclusion base and limit. Nothing the
unit does changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator on the design's open decisions.

  1. Two PRs: accepted. Stage 5 lands and is read on the bench before stage 6 changes interrupts.
  2. Refusing a unit without INVALIDATE_IOMMU_ALL, and the 128-bit remapping entry alone: accepted provided the AMD bench's unit has both. The bench reading is being taken now and will be posted here; if either is missing there, the design takes the longer path, since QEMU never degrades what metal gets and the bench is the metal.
  3. xhci-leave= by function address: routine; do it.
  4. One domain registry for VT-d, AMD-Vi and the SMMU in iommu::: authorized as the first commit of stage 5, if it deletes more than it adds. The reviewer judges that by the diff.

@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

AMD bench reading for the design (orchestrator; image b09c2e45…, built at b3e77cac4; blackbox_unclaimed_page judged, exit 0; the readback stays in the orchestrator's scratchpad until the build round publishes it). Every iommu: amdvi unit line of the kernel log:

iommu: amdvi unit0 @0xfdf00000 seg=0 dev=00:00.2 ivhd=0x40 flags=0xb0 cap=0x40 info=0x0000 attributes=0x00040200 efr_image=0x206d73ef22254ade efr2_image=0x0000000000000000
iommu: amdvi unit0 function 00:00.2 caps=[0x0f 0x05 0x08] misc0=0x00203040 va=64 pa=48 msinum=0
iommu: amdvi unit0 serves 00:01.0..=ff:1f.6 data=0x00
iommu: amdvi unit0 serves ff:00.0..=ff:1f.7 as 00:14.5 data=0x00
iommu: amdvi unit0 serves special variety=2 handle=0 as 00:14.0 data=0x00
iommu: amdvi unit0 serves special variety=1 handle=33 as 00:14.0 data=0xd7
iommu: amdvi unit0 serves special variety=1 handle=34 as 00:00.1 data=0x00
iommu: amdvi unit0 serves acpi "AMDI0020" cid="" uid="\\_SB.FUR0" as 00:14.5 data=0x40
iommu: amdvi unit0 serves acpi "AMDI0020" cid="" uid="\\_SB.FUR1" as 00:14.5 data=0x40
iommu: amdvi unit0 serves acpi "AMDI0020" cid="" uid="\\_SB.FUR2" as 00:14.5 data=0x40
iommu: amdvi unit0 serves acpi "AMDI0020" cid="" uid="\\_SB.FUR3" as 00:14.5 data=0x40
iommu: amdvi unit0 handed over control=0x0000220000000400 status=0x00000000 efr=0x206d73ef22254ade exclusion=0x0000000000000000..0x0000000000000000
iommu: amdvi unit0 @0xfdf00000 switched off control=0x0000220000000400 status=0x00000000

For ruling 2: the unit's own EFR reads 0x206d73ef22254ade; the designer and the reviewer read IASup, GASup and XTSup off it against 48882 Table 25 rather than take my decoding. The unit's own function carries capabilities [0x0f 0x05 0x08], so MSI and no MSI-X, with msinum=0; firmware left no exclusion range.

@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Design review: #909 at b3e77cac4 (reviewer; base ef684e4fc; judged against reviewer.md on main after #908, root CLAUDE.md, and #907's trust model on wt/toyos-trust-model. I checked every cited spec fact below against the designer's page text of 48882 rev 3.11.)

Assessment

The architecture is right. The unit picks the remapping table from the requester's own DTE, so giving each kept requester a table indexed by vector verifies the source structurally. It needs no handle allocator, and the device keeps its plain compat message. The cost is 4 KiB per requester with a remapped interrupt, a handful on the bench, plus one INVALIDATE_INTERRUPT_TABLE per change. One shared table would remove the source check, so I do not see a cheaper design that keeps the guarantee. The 2 MiB device table is a fixed cost that buys one constant. The toyos-amdvi pure crate copies the toyos-smmu pattern without duplicating it. DTEs, AMD host PTEs/PDEs with NextLevel and IR/IW, commands and event records are AMD's alone. The only thing shared with toyos-smmu's plan is nine-bit index arithmetic, which is not worth a shared crate. It should take toyos_phys::Phys<N> for addresses, as toyos-smmu does. What it does duplicate is the kernel-side domain registry (see B1). Splitting the work into two PRs is sound and already ruled on.

Ruling 2: the bench's EFR, decoded against Table 25 (MMIO 0030h, pp. 225-228)

0x206d73ef22254ade, low word 0x22254ade:

  • bit 0 PreFSup = 0; bit 1 PPRSup = 1; bit 2 XTSup = 1; bit 3 NXSup = 1; bit 4 GTSup = 1; bit 5 GAPPISup = 0; bit 6 IASup = 1; bit 7 GASup = 1; bit 8 HESup = 0; bit 9 PCSup = 1
  • HATS[11:10] = 10b (6 levels, so Mode 4 is within limit); GATS[13:12] = 00b; GLXSup[15:14] = 01b; SmiFSup[17:16] = 01b; SmiFRC[20:18] = 001b; GAMSup[23:21] = 001b; DualPprLogSup[25:24] = 10b; DualEventLogSup[29:28] = 10b; SATSSup bit 31 = 0

High word 0x206d73ef:

  • PASmax = 0x0F; USSup = 1; DevTblSegSup = 11b; bits 40, 41, 44, 45, 46 (MsiCapMmioSup) set; bits 48, 50, 51, 53, 54 set; bit 61 set
  • SNPSup bit 63 = 0

Table 21 (p. 93) selects the 128-bit IRTE for remapping when GASup = 1, GAEn = 1 and GAMEn = 000b, whatever GAMSup is. The bench therefore has both INVALIDATE_IOMMU_ALL (IASup) and the 128-bit format (GASup), plus XTSup should it ever run in x2APIC mode. Ruling 2's condition holds. SNPSup = 0, so the SNP-specific restrictions on the exclusion register (p. 223) and on Table 21's rows do not apply on the bench.

The other new readings agree with the spec and with an independent source on the same machine. misc0=0x00203040 decodes by Capability Offset 10h (p. 209: VAsize 21:15, PAsize 14:8, MsiNum 4:0) to va=64, pa=48, msinum=0, matching the IVRS IVinfo's pa=48 va=64. The caps [0x0f 0x05 0x08] mean MSI and no MSI-X, which §3.1 (p. 205) permits. The exclusion range reads 0..0. The IVHD flags 0xb0 set Coherent (bit 5), and control=…400 reads Coherent = 1 back.

Spec facts checked (all hold)

  • Blocked DTE V=1, TV=1, Mode 000b, IR=IW=0. Table 7 bits 11:9 say "Translation disabled (Access controlled by IR and IW bits)". IR and IW at 61/62 target-abort reads and writes, and with both 0 even zero-byte reads. Table 44 logs a read or write to a protected page as IO_PAGE_FAULT. Table 8 confirms that V=0 passes everything, as the issue says.
  • DTE layout as in Figure 7. One 128-bit write plus INVALIDATE_DEVTAB_ENTRY for translation changes, and one 64-bit write for the interrupt half (§2.2.2.2, p. 77).
  • MMIO 0000h Size = 0x1FF gives 2 MiB, base 4 KiB-aligned (p. 214).
  • Remapping:
    • The IRT index is MSI data[10:0] (Figure 14), so index = vector holds only for Fixed delivery (data[10:8] = 000b). The kernel arms Fixed; say so in the code that arms the message.
    • IntCtl and IntTabLen encodings: Table 7, pp. 65/67.
    • XTEn needs GAEn (§2.2.5.3, p. 97).
    • RemapEn = 0 produces IO_PAGE_FAULT (p. 94 step 7; Table 44).
  • The software note under §2.1.3 (p. 52) allows a faulting translation to be cached, which justifies invalidating on map.
  • Exclusion register Allow/ExEn: p. 223.
  • One citation is wrong. The 128-bit GuestMode = 0 entry is Figure 17 / Table 22 (p. 95). Figure 19 (p. 98) is the same format with the x2APIC destination; cite both, or Figure 17 for xAPIC.

Findings

B1 BLOCKER: the stage 5 plan adds a third copy of the domain registry, contrary to ruling 4. PR body "Stage 5 → domain.rs" and "What it deletes → Not deleted, and proposed separately".

  • Problem. The planned create/map/map_at/place/unmap/attach is the shape kernel/src/arch/x86_64/vtd/domain.rs (180 lines) and kernel/src/arch/aarch64/smmu/domain.rs (204 lines) already implement: a Vec<Domain> by id minus FIRST, a Window each, and an invalidate after each change. The orchestrator authorized lifting one registry into iommu:: as stage 5's first commit, provided it deletes more than it adds, and the body still says the work lies outside the track.
  • Consequence. As written, stage 5 lands a third copy, and the 1,500-2,000-line estimate includes it.
  • Fix. Rewrite the stage 5 plan:
    1. The first commit lifts the registry into iommu:: behind a small per-backend table/invalidate surface. Its net line count shows it removes more than it adds.
    2. AMD-Vi is then a backend of that registry.
    3. The estimate changes to match.

B2 BLOCKER: a claimed function inherits the IVHD pass bits. PR body "Stage 6": "Pass bits (NMI, INIT, ExtInt, LINT0/1) and SysMgt come from the IVHD byte".

  • Problem. Table 10 (p. 75) forwards a requester's NMI, INIT, ExtInt and LINT messages unmapped when its pass bit is set. SysMgt = 01b/10b forwards system-management messages (Table 7, p. 68).
  • Consequence. On a machine whose IVHD range entry gives a claimable function a nonzero DTE byte, a process-driven device could raise an NMI or an INIT, which resets a CPU, against the kernel. That breaks the stated outcome that the MSI "reaches only its own claim slot's vector". Under CLAUDE.md: ToyOS's trust model, the privilege rule and the source order; issues: Thunderbolt and USB4 is a track #907, programs and their devices are isolated even though the firmware that wrote the byte is trusted. On the bench every PCI function's range byte is 0x00, so no row would show the problem.
  • Fix. claim refuses, by name, a function whose DTE carries any pass bit or SysMgt ≠ 00b, or clears those bits for the claim's lifetime. Hold it with a host test of the DTE the claim writes.

B3 BLOCKER: the design does not say what happens to IVMD unity and exclusion ranges. PR body "Stage 5": windows clear of "every IVMD range"; "exclusion range cleared".

  • Problem. An IVMD range is firmware's declaration that the named requesters must reach it. It may be a unity map or an exclusion request (kernel/src/arch/x86_64/amdvi/mod.rs already logs unity and exclusion). The design only keeps windows away from these ranges. It then clears the exclusion register and moves devices into domains that do not map them. VT-d handles RMRRs this way (vtd/mod.rs:174-207): a range inside the identity domain is held "for free", one outside it is logged OUTSIDE, and a userspace hand-over of a scoped device is refused.
  • Consequence. On another AMD machine a device named by an IVMD would fault after being moved to its own domain or claimed. The bench declares no IVMD, so no row would catch it.
  • Fix. State the same policy for IVMD:
    • A range below pmm::top() is held by the identity domain; one outside it is logged as such.
    • A requester an IVMD names is not moved to its own domain or claimed.
    • An IVMD with the exclusion flag is honoured or refused by name rather than cleared silently.

B4 BLOCKER: the QEMU issue is not answered by this design, so the body must not say it is. PR body "What it deletes": "issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md is answered (attach invalidates per requester)".

  • Problem. The issue (and the track issue's constraint) records that QEMU 11.1.1 moves a function to translation only on an INVALIDATE_DEVTAB_ENTRY that names it. Boot under this design sends only INVALIDATE_IOMMU_ALL, so under QEMU every identity-domain kernel driver stays on QEMU's pass-through. Only a function attach moves is translated.
  • Consequence.
    • The guest boot establishes nothing about the identity tables.
    • The QEMU deficiency remains true.
    • Closing the issue would drop the record that the guest suite cannot see a broken identity domain.
  • Fix. Either:
    • keep the issue open, with its exit restated for the new code; or
    • write each enumerated function's identity DTE through the same per-requester INVALIDATE_DEVTAB_ENTRY that attach sends. That is spec-legal, costs one command per enumerated function at boot and degrades no metal. Then name the guest assertion that shows the identity domain translating under QEMU.

B5 BLOCKER: the spec copy's provenance is knowable and the body misstates it. PR body "Source"; evidence d1/spec/README.txt.

  • Problem. The body calls the PDF "a local one already on the development machine". Its SHA-256 f7c375a1…ce22 equals the copy committed in a third-party GitHub repository, matotubol/svmvisor docs/48882-3.11.pdf at b93b652, which another agent cloned on 2026-10-12. Under CLAUDE.md: ToyOS's trust model, the privilege rule and the source order; issues: Thunderbolt and USB4 is a track #907's source order that is a vendor document received through someone else's redistribution, not AMD's bytes.
  • Adequacy for this round. The copy is adequate for the design: its metadata names AMD rev 3.11, the facts above are internally consistent, and the bench's EFR/misc0/IVinfo readings corroborate the field positions a bench can reach.
  • Adequacy for stage 5. It is not adequate as the cited oracle of stage 5's host tests until one of the following holds:
    1. its hash is matched against bytes from an AMD host. The docs.amd.com/v/u/en-US/48882_3.10_PUB fetch returned 200 with a 2,575-byte page; follow the download link that page names, or another AMD documentation-hub URL, with the build user-agent.
    2. the owner supplies AMD's download and its hash.
    3. failing both, the evidence records the true origin and the stage 5 PR cites only facts the bench also confirms or that a second AMD publication states.
  • Fix now. The orchestrator corrects the Source section to name the origin.

S1 SUGGESTION: state the order and the reserved DomainID. Write identity DTEs for every enumerated function before IommuEn, so firmware-left DMA in flight cannot meet a blocked entry and halt as a kernel-owned fault. Give blocked DTEs one DomainID that no translated domain uses (for example 0, with identity at 1 as VT-d's KERNEL_DOMAIN). Table 7 requires DomainID to be valid when V=1 and ties shared IDs to identical Mode and root.

S2 SUGGESTION: refuse a unit whose IVHD Coherent flag is clear. The design sets CONTROL.Coherent = 1 and never flushes. IVHD flag bit 5 is firmware's recommendation for that control (p. 296), so refusing such a unit by name is cheaper than adding a clflush path.

S3 SUGGESTION: read QEMU's I/O APIC special entry now. One amd-iommu,intremap=on boot reading the existing serves special lines settles whether stage 6's machine-wide rule ("every I/O APIC named by an IVHD special entry") can ever pass under QEMU. If it cannot, the issue's own exit cannot be met in a guest. The assumption is cheap to check before stage 6 is built on it.

S4 SUGGESTION: reword "most of the risk" in Tests. Host tests hold the encodings. Stage 5's most consequential risks are the register sequence and the identity coverage ("if it is wrong the bench returns no log"), and only the bench row holds those.

S5 SUGGESTION: tidy the measurement code.

  • kernel/src/arch/x86_64/amdvi/mod.rs:166: fn function is a vague name for a function that logs; describe_function matches describe.
  • mod.rs:37: when stage 5 rewrites this module, replace "in Linux's iommu_disable order" with the specification's own reason for the order. Root's rules do not let Linux source be the authority.

Bench rows and isolation

Taken together, the rows prove isolation for a claimed function:

  • translated DMA: row 1 (the stick through its own domain at addresses ≥ 2^46, which fails if the unit passes through) and row 2;
  • faulted DMA: row 2's DMA FAULT owner=slot at the aimed address;
  • remapped interrupt: row 2's No-Op through the claim's own entry;
  • refused foreign vector: row 3.

Stage 5 alone proves translation only. A kernel-owned fault halts, so no stage 5 row reaches the event-log drain on metal. The blocked default for unenumerated requesters has no oracle on any target. The body says this honestly, and I am not asking for more.

Evidence

All four checks show exit codes and logs at 8a0ab6fb9. The guest log shows the new lines (caps=[0x05 0x08 0x05 0x0f] misc0=0x00182840, exclusion=0..0) and test result: ok. Its supervisor stamp reads ef684e4fc … dirty, so it ran on the uncommitted change rather than the committed head. The diff is one log line, so nothing invalidates that. Host CI: 76 steps green. The bench reading is the orchestrator's comment, image b09c2e45… built at b3e77cac4.

SEND BACK

@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator: design accepted with the five blockers as named changes; stage 5 builds now, on this branch.

The ownership stands: one device table, blocked by default; identity domains for kernel drivers, a domain per claimed function; a remapping table per requester, indexed by vector. The bench's EFR settles ruling 2: IASup, GASup and XTSup are set, so the short path holds.

Named changes, carried in the build:

  1. B1: the shared domain registry for VT-d, AMD-Vi and the SMMU is stage 5's first commit, per ruling 4; the estimate follows.
  2. B2: a claim clears the pass bits (NMI, INIT, ExtInt, LINT) and SysMgt in the claimed function's entry; kernel-driven functions keep the IVHD's byte. A process never gains a path to an NMI or INIT.
  3. B3: IVMD ranges get VT-d's RMRR policy: the identity domain holds a named range below the top of memory, a requester an IVMD names is neither moved nor claimed, and an exclusion IVMD is honoured or refused by name.
  4. B4: at boot every function gets its own INVALIDATE_DEVTAB_ENTRY after INVALIDATE_IOMMU_ALL, and a guest assertion holds that the identity domain translates under QEMU; the QEMU issue closes on that assertion, not on a claim.
  5. B5: the Source section says where the copy came from. The host tests' oracle is 48882 only once its hash matches bytes from AMD's own host, or a copy the owner downloads; until then the body says so, and every fact the host tests encode is one the bench confirmed or a second AMD publication states.

Suggestions S1–S5 are taken; S3's one boot is the orchestrator's to run and will be posted here. The figure citation is corrected.

VT-d and the SMMU each kept the same bookkeeping for the domains a unit
translates for: ids from a first one up, a Window of device addresses
each, the 2 MiB alignment refusal, the handed-out check before a place,
and the records a create, a map and a map-again write. AMD-Vi, the next
backend (issues/an-amd-vi-machine-isolates-no-device.md, stage 5), would
have been a third copy.

`iommu::domains::Domains<S>` now holds it, generic over the backend's
own name for a domain's tables. A backend keeps its tables, its
invalidation and its lock, and holds the registry under that lock: VT-d
its `DOMAINS` (with the width agreement, which is VT-d's alone), the SMMU
inside `Live`. VT-d's table module loses `Domain` and its window for a
plain `Space` of root and width, with per-leaf `put` and `take`.

One behaviour moves: a VT-d unmap refused part-way now invalidates what
it cleared before the refusal, as the SMMU's already did; it returned
before invalidating. The create record's backend fields now precede
"addresses from .. to ..", which ends every such line.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

B5 is closed. The owner downloaded 48882_3.11_IOMMU_PUB.pdf from AMD on 2026-10-13; its SHA-256 is f7c375a15db5ed63de760356867211063d164a2ed59f2d38613daec95894ce22, byte-identical to the copy the design was read from. 48882 rev 3.11 is the oracle for the host tests; the body's Source section records the owner's download and this hash.

…and the event log

Stage 5 of issues/an-amd-vi-machine-isolates-no-device.md, the design
accepted on #909. On a machine whose IVRS describes AMD-Vi units the kernel
now programs each unit it can use instead of leaving it off.

- toyos-amdvi (kernel/amdvi), pure and host-tested against AMD 48882 rev
  3.11's field positions: device table entries (blocked: V, TV, Mode 000b,
  IR=IW=0; translated: Mode 100b over a 4-level root), the IVHD DTE
  setting's SysMgt, host page table directory and 2 MiB leaf entries
  (FC set, so a device's no-snoop cannot read stale memory), the
  commands, the event records, and the register fields.
- One device table every unit reads, all 65,536 entries blocked under
  domain 0 but each enumerated function's, which names the identity
  domain (1) over [0, pmm::top()); written before IommuEn. Each unit gets a
  command buffer and event log, its exclusion range cleared, its own MSI
  armed through its PCI function (the one message no unit remaps), then
  CONTROL; then INVALIDATE_IOMMU_ALL and one INVALIDATE_DEVTAB_ENTRY per
  enumerated function, behind a completion wait.
- A unit is left off, by name, on a segment other than 0, without IASup,
  with a reserved HATS, where the IVRS has an enumerated function issue its
  requests under another id, where its device entries are refused, or
  where its function has no MSI capability. An IVMD that asks for an
  exclusion range or memory no device may reach leaves every unit off; a
  unity IVMD's requesters keep the identity domain (DeviceSpace::create now
  takes the function and answers Untranslated for them).
- Domains go through iommu::domains, with map, unmap and attach each
  published by INVALIDATE_IOMMU_PAGES (and INVALIDATE_DEVTAB_ENTRY for a
  move) behind a completion wait; attach swaps the entry's translation half
  with the cmpxchg16b VT-d already had, now shared in iommu_unit.
- The event log is drained on DMA_FAULT_VECTOR into iommu::fault;
  IO_PAGE_FAULT and INVALID_DEVICE_REQUEST are a requester's, any other
  record panics; an overflow restarts the log by §2.5.1.
- iommu_unit dispatches domain and fault on the backend init found.
- The firmware-left actuator is rebuilt from toyos-amdvi's encodings, and
  FIRMWARE_LEFT's order is stated by the specification's reason.
- amdvi-selftest and the MetalAmdViTestdev profile: two iommu-testdev,
  one on its own domain landing a write where it maps, the other on the
  identity domain landing one in memory and refused one past it, recorded
  as the kernel's, and the machine halts. QEMU 11.1.1's record of that
  write carries no address, filed as
  issues/qemus-amd-iommu-page-fault-record-carries-no-address.md.

- The AMD bench's row amdvi_translates_the_stick_controller: every unit
  translating, every xHCI the kernel drives moved to a domain of its own,
  the stick bound, no DMA refused. build/sourcegate.rs declares the
  unit's function's enable_bus_master, after the unit's refusals.

Declined: refusing a unit whose IVHD leaves Coherent clear (review S2).
QEMU's IVHD says flags 0x11, and CONTROL.Coherent's reset value and
Table 97's recommended value are both 1; this driver sets it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
@Japabu Japabu changed the title AMD-Vi translation and interrupt remapping: design, and the bench reading it needs AMD-Vi translates: a blocked device table, identity and own domains, the event log, and one domain registry for every backend Oct 13, 2026
@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator: S2 stays declined, as built: QEMU's unit leaves Coherent clear and refusing it would refuse the only AMD-Vi unit CI has; the driver sets Coherent, which is the reset value and the spec's recommendation. B2 moving to stage 6 is right: nothing on AMD-Vi can be claimed before remapping. The three AMD bench boots are running now.

@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Build review: #909 at 9173c339b (reviewer; base ef684e4fc; judged against reviewer.md, root CLAUDE.md, the accepted design 6152903557 with B1–B5 as named changes, and the orchestrator's later rulings: 48882 hash match, S2 declined, B2 deferred to stage 6). Oracle: 48882 rev 3.11. The local copy's SHA-256 is f7c375a1…ce22, which matches. I read each encoding against the page text.

Assessment

The architecture is the one the design accepted, and it is built cleanly:

  • One device table, blocked by default.
  • An identity domain written before IommuEn (S1 holds: build runs before program).
  • Own domains through the lifted registry.
  • An event log drained without locks into iommu::fault.

B1 holds:

  • iommu::domains::Domains<S> removes the duplicated ids, window, alignment, handed-out check and records from VT-d and the SMMU (232+/242−).
  • exchange16 and the testdev registers are lifted, not copied.
  • VT-d's only behavioural change is the stated one. A partly refused unmap now invalidates what it cleared. The create record's field order and the map record's timing (now logged before the leaves are written) are cosmetic.
  • SMMU create's asid(id) refusal became an expect. That is sound, because prepare bounds ids by the same ASID ceiling.

The host tests hold every word I checked against 48882:

  • DTE: Fig. 7 / Table 7, including SysMgt 105:104 from Table 106 bits 5:4.
  • INVALIDATE_IOMMU_PAGES all-domain address 7_FFFF_FFFF_FFFFh with S/PDE (§2.4.3).
  • IO_PAGE_FAULT I@51 and RW@53 (Fig. 56).
  • Device-table Size 1FFh.
  • Ring length 1000b.
  • PTE NextLevel, FC, IR and IW.
  • HATS and IASup.

The bench log matches the design:

  • control=…140d: IommuEn, EventLogEn, EventIntEn, Coherent and CmdBufEn. The upper 0x22… is firmware's bit 61 (SNPAVICEn 001b) and reserved bit 57, both carried over.
  • status=0x18: CmdBufRun and EventLogRun.
  • The MSI is at 0x25.
  • 03:00.3 is on domain2 from 2^46.
  • Images were built at a2e53ebfa, whose tree equals the head's (b178aed5…).

The metal control units-left-off is a real red.

The remaining problems:

  • One device-originated event code panics the kernel.
  • A misread negative control rewrote a true QEMU constraint out of the record.
  • A machine with one refused unit hands out IOVAs that no unit translates.
  • One false IVMD description.

Findings

B1 BLOCKER: INVALID_PPR_REQUEST is caused by a requester, but the kernel panics on it.

  • Where: kernel/amdvi/src/queue.rs:47, kernel/src/arch/x86_64/amdvi/fault.rs:6-8, 90-95.
  • Problem: Table 52 (p. 149) says the unit raises code 1001b when a "PRI request [is] received when … PPREn=0 or … PPRLogEn=0". That is this driver's configuration. Figure 64 / Table 67 (RX=0) put the "DeviceID of the peripheral issuing the invalid PPR request" at [15:0]. The record is device input that the unit has already target-aborted.
  • Consequence: The decoder files it under Other, documented as "a table or command of this driver's the unit refused, or the unit's own hardware failing", and drain panics. Any requester can crash the kernel by sending one PRI message, including one nobody enumerated, which iommu::fault says is "counted and the machine goes on". That contradicts root's "a userland bug never crashes the kernel / external input refused at the boundary", and stage 6 inherits it for claimed functions.
  • Fix: Decode 1001b with RX=0 as a requester record, the same way as INVALID_DEVICE_REQUEST. Add it to encodings.rs and correct the two comments.

B2 BLOCKER: the per-function-invalidation control is misread, and a true QEMU constraint was removed on that basis.

  • Where: issues/an-amd-vi-machine-isolates-no-device.md:28-29, tests/common/iommu.rs:450-453, kernel/src/arch/x86_64/amdvi/selftest.rs:11-12, kernel/src/arch/x86_64/amdvi/mod.rs:431-432.
  • Problem: The control (mut-no-per-device-invalidation.log) shows 00:06.0's write at 0x400000000000 answering 0xdead0002, with no record. The body and the harness doc read that answer as "QEMU still refused" it, meaning the unit translated. They conclude the QEMU issue's premise "is not what this configuration showed". The same log reports the guest's physical width as pa=40. A write passed through untranslated to 2^46 reaches no memory either. iommu-testdev reports a failed DMA as a 0xDEAD000X error whatever the cause. So 0xdead0002 cannot distinguish translation from pass-through. The issue's premise (iommu_nodma until amdvi_inval_devtab_entry) predicts exactly what was observed: a failed write and no record. The selftest's comment "A unit passing its DMA untranslated lands it instead" is false on this machine for the same reason. The record is the only discriminator, which is why the control still reds. The control is real, but its explanation is wrong.
  • Consequence:
    • The track issue loses a constraint the evidence supports, "only once an INVALIDATE_DEVTAB_ENTRY names it".
    • The kernel comment ("a unit may track an entry by the invalidation that names it") hides the real reason for the 28 boot commands.
    • Two test docs teach a false oracle.
  • Fix:
    • Restore the constraint in the track issue.
    • Have mod.rs:431-432 name the reason: QEMU 11.1.1, citing issues/qemus-invalidate-iommu-all-leaves-every-function-untranslated.md. The spec needs only INVALIDATE_IOMMU_ALL (§2.4.8).
    • Correct the selftest and harness docs: the record, not the verdict, establishes translation.
    • Keeping the QEMU issue open is right, because the defect persists. Its stated reason in the body is the orchestrator's to correct.

B3 BLOCKER: when one unit is refused, functions it serves still get domains that no unit translates.

  • Where: kernel/src/arch/x86_64/amdvi/mod.rs:189-191, 231-255, kernel/src/arch/x86_64/amdvi/domain.rs:68-74, kernel/src/iommu/mod.rs:174.

  • Problem: Units are refused individually: alias, no MSI, IASup, HATS, segment, or device entries refused. The backend nevertheless becomes AmdVi as soon as one unit is ready. create and attach do not ask whether any live unit serves the requester. A kernel driver for a function behind a refused unit, or behind no IVHD at all, is given IOVAs from 2^46 and writes them into its device. That unit was switched off, so the DMA goes untranslated to physical 2^46+ and the device fails. This is not the documented "left off" result, in which DMA reaches memory. The single-unit bench and QEMU cannot show it, but a multi-IVHD machine would.

  • Fix: Pick one:

    • The cheapest: refuse machine-wide. Any described unit unusable leaves every unit off, as vtd::remappable already reasons for interrupts.
    • Have DeviceSpace::create answer Untranslated (logged) for a requester that no programmed unit's device entries name. declare already walks those entries.

    VT-d appears to share the latent per-unit gap. That is pre-existing and can be recorded rather than fixed here.

B4 BLOCKER: an IVMD with Unity clear is described as something it is not.

  • Where: kernel/src/arch/x86_64/amdvi/mod.rs:221-228.
  • Problem: Table 113 (p. 309) defines Unity=0 as "any virtual address translation may be used". It does not mean "memory no device may reach". The code refuses on Unity == 0, and the PR body says the refusal is for an IVMD "forbidding all access" (IR=IW=0), so the line and the body describe two different conditions. Refusing what this kernel does not honour is the accepted policy; the false text is the problem.
  • Fix: Make the log name the actual condition (a range without Unity), or implement the condition the body states.

S1 SUGGESTION: set CONTROL explicitly instead of OR-ing onto firmware's value.

  • Where: mod.rs:424.
  • switch_off clears only the enable bits. Fields such as DevTblSegEn (36:34, p. 218), GAEn/XTEn and HtTunEn survive. A nonzero DevTblSegEn would make the unit index this single table as segments. The bench leaves only bits 57 and 61.
  • Writing the fields this driver means, and logging what firmware had, removes that dependency.

S2 SUGGESTION: firmware_reserves always answers false on VT-d.

  • Where: kernel/src/arch/x86_64/iommu_unit.rs (domain::firmware_reserves).
  • Its name asks a generic question, but VT-d answers false even where an RMRR scopes the function.
  • Either note that it covers IVMDs only, or record that VT-d moves RMRR-scoped kernel functions off identity. That behaviour is pre-existing and outside this PR.

S3 SUGGESTION: minor simplifications.

  • amdvi/domain.rs:71: derive 48 from ROOT_LEVEL instead of repeating it.
  • Event::PageFault { domain, interrupt } are decoded and tested, but nothing reads them.
  • build's tables.pages.insert(0, page) keeps last() pointing at the carving page. That is a subtle invariant; a separate field for the device table page would be plainer.
  • drain W1Cs EventOverflow before restart does it again in §2.5.1's order. The first clear is redundant.

Evidence

  • The host CI log (s5-ci-host2.log, exit 0, 76 steps) runs toyos_amdvi's encodings.
  • Guest amdvi_selftest passed in guest-final.log at a2e53ebfa, with exactly one owner=kernel … stream=00:06.0 … IO_PAGE_FAULT before the halt.
  • The QEMU record-address issue is backed by the raw 0x200a000000000030 0x0 in guest-s5-debug.log.
  • The bench judge shows 3 passed. The control judge shows exit 1 on "no AMD-Vi unit is translating".

None of the fixes above calls for a metal rerun:

  • B1 needs a host test.
  • B2 and B4 change text.
  • B3's machine-wide variant changes no behaviour on single-unit machines, but the amdvi_selftest/amdvi_firmware_left guest pair should be rerun after it.

SEND BACK

@Japabu

Japabu commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator on the build review (6154133561): fix round, with one ruling.

  • B1: a device's own fault, INVALID_PPR_REQUEST included, is counted against that device and never panics; a device is outside the trusted base, and the kernel never crashes from one. Host test.
  • B2: the control is re-read as the reviewer says: the mutation's 0xdead0002 cannot tell translation from pass-through on a 40-bit guest; the track issue gets its constraint back, the two test docs and the mod.rs comment say what is true, and the body's reason for keeping the QEMU issue open is corrected in the same push.
  • B3, ruled: a device no programmed unit serves answers Untranslated and is logged by name, so one refused unit costs isolation only behind itself. Leaving every unit off would make one bad unit disable every other.
  • B4: the IVMD label and the body follow 48882 Table 113.
  • Suggestions: the control register is written whole, not OR-ed onto firmware's leftovers; firmware_reserves' VT-d behaviour gets its doc line; the tidy-ups are the implementer's call.

No bench rerun is owed; amdvi_selftest and amdvi_firmware_left rerun on QEMU after B3.

…machine addresses, and the control's reading is corrected

Build review of #909 (B1-B4) and the orchestrator's rulings.

- B1: every record a requester can cause counts against it and never
  stops the machine on its own: IO_PAGE_FAULT, IOTLB_INV_TIMEOUT,
  INVALID_DEVICE_REQUEST, and INVALID_PPR_REQUEST with RX clear, whose
  DeviceID is the peripheral's (48882 Figure 64, Table 67). Only a record
  of this driver's tables or commands, or of the unit's own hardware,
  panics. `Event` is now `Requester` or `Unit`; host tests hold both, and
  decoding INVALID_PPR_REQUEST as the unit's reds them.
- B2: the per-function-invalidation control's `0xdead0002` cannot tell
  translation from pass-through on a 40-bit guest, so the record is the
  only discriminator. The track issue gets back QEMU's "only once an
  INVALIDATE_DEVTAB_ENTRY names it", the boot's per-function commands are
  commented by that QEMU defect (the specification needs
  INVALIDATE_IOMMU_ALL alone), and the selftest's and harness's docs say
  the record, not the answer, establishes translation.
- B3 (ruled): a function no programmed unit's device entries name keeps
  the machine's addresses and says why, so one refused unit costs
  isolation only behind itself. `firmware_reserves` becomes
  `untranslated`, answering the reason; its doc states VT-d answers None
  for every function, an RMRR's included.
- B4: an IVMD without Unity is logged as that (Table 113: any translation
  may be used), not as memory no device may reach.
- CONTROL is written whole rather than OR-ed onto firmware's fields, so a
  DevTblSegEn left over cannot change how the unit reads the table; the
  device table keeps a page field of its own; the domain width is derived
  from the root level; the event drain no longer clears EventOverflow
  ahead of the restart that does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
@Japabu

Japabu commented Oct 13, 2026 •

Copy link
Copy Markdown
Collaborator Author

Build review round 2: #909 at e7ab4648c (reviewer; fix commit only, +167/−105 across 11 files; judged against round 1 (6154133561), the orchestrator's rulings (6154144473) and 48882 rev 3.11, read from the copy whose hash matches. Evidence: pr-909 at f778bff27 (d4/) and d64a174fc (d5/).)

Assessment

The fix round stays within the accepted design and the rulings. The decoder now has two outcomes: Requester (counted through iommu::fault) or Unit (panic). That is simpler than the three variants it replaces, and the change removes the dead domain/interrupt fields. B3 is implemented as ruled:

  • A ready unit's declared ids are collected.
  • untranslated answers both reasons by name.
  • attach asserts the same predicate.

So DeviceSpace::create is the one gate. No code blocker remains. The one blocker is a false statement about the bench reading, which repeats my own round-1 misreading.

Round-1 blockers

B1: closed as ruled.

  • kernel/amdvi/src/queue.rs:52-67 decodes these as Requester:
    • IO_PAGE_FAULT;
    • IOTLB_INV_TIMEOUT;
    • INVALID_DEVICE_REQUEST;
    • INVALID_PPR_REQUEST with RX clear.
  • I checked the positions against Figure 64 and Table 67 (p. 166-167). RX is +04[16] = bit 48, WP is +04[21] = bit 53, and with RX=0 the DeviceID is "of the peripheral issuing the invalid PPR request".
  • RX=1 stays Unit. That is correct: the record reports a failed COMPLETE_PPR_REQUEST, and this driver issues none.
  • The remaining codes are all host software's or the unit's own (Table 42, p. 143):
    • 0001, 0011-0110;
    • 1010 EVENT_COUNTER_ZERO, with no counters armed;
    • the reserved codes.
  • Host tests:
    • a_record_a_requester_causes_is_its_own covers 1000, 0111, PPR with RX=0 and PPR with RX=1.
    • every_other_record_is_the_units covers the rest.
  • Control: ppr-is-the-units.patch reds that test with exit 101 (d4/logs/mut-ppr-is-the-units.log, 12 passed, 1 failed).
  • A requester record on a kernel-driven function still halts through iommu::fault's existing policy. That policy is unchanged, applies to every backend, and is not the decoder panic B1 was about.

B2: closed.

  • The track issue's QEMU constraint is restored, citing the QEMU issue.
  • mod.rs:501-504 gives the true reason: §2.4.8 needs INVALIDATE_IOMMU_ALL alone, and the per-function commands exist for QEMU 11.1.1.
  • selftest.rs and tests/common/iommu.rs now say the record, not the device's answer, establishes translation.
  • The body's reason for keeping qemus-invalidate-iommu-all-… open is corrected.

B3: closed as ruled.

  • declare returns each entry's served range. translated is extended only when usable_unit succeeds (mod.rs:376-379), and is handed to domain::start.
  • untranslated (amdvi/domain.rs:68-79) answers "no unit this kernel programmed serves it" before the IVMD reason. DeviceSpace::create logs it by name and answers Untranslated.
  • Alias entries contribute only used. Their other ids are non-enumerated by construction, because declare refuses otherwise.
  • The rename to untranslated is applied on all three backends.
  • The VT-d doc line (iommu_unit.rs:140-143) states the pre-existing RMRR behaviour truthfully.
  • The guest pair amdvi_selftest and amdvi_firmware_left reran green (d4/logs/r2-guest.log, exit 0). The single-unit QEMU and bench machines cannot reach the unserved branch. Inspection establishes it, since it is a range-membership test.

B4: closed.

  • The label is now "a range without Unity". Its comment matches Table 113 (p. 309): ExclusionRange makes IR, IW and Unity ignored, and "0b=any virtual address translation may be used".
  • The body matches.

Suggestions taken

  • Control written whole (mod.rs:487-492): correct against 48882, but the reading of what it clears is wrong (see B5).
    • The driver's value 0x140d sets IommuEn, EventLogEn, EventIntEn, Coherent (bit 10) and CmdBufEn. Every other field defaults to off, and none of those fields is one this stage uses (p. 217-220).
    • The bench shows firmware's 0x0000220000000400 (d5/testcases/kernel.log:117) becoming 0x000000000000140d with status=0x18 (:121). Firmware's own bit 10 (Coherent) is kept.
  • Device-table page field: taken. The insert(0, …) invariant is gone.
  • Width from the root level: taken. 9 * ROOT_LEVEL + 12 = 48 at level 4.
  • Overflow: drain clears EVENT_LOG_INT alone, and restart keeps §2.5.1's order. Taken.

Findings

B5 BLOCKER (body only, the orchestrator's): the firmware bits the whole write drops are misnamed.

  • Where: the PR body's "Fix round" paragraph, "firmware's bits 57 and 61 are gone", which the coordinator's message repeats.
  • Problem: 0x0000220000000400 has bits 45, 41 and 10 set, not 57 and 61. Round 1 introduced the error ("bit 61 (SNPAVICEn) and reserved bit 57"), and I retract it. Per p. 218:
    • bit 45 is EPHEn, Enhanced PPR handling;
    • bit 41 is BlkStopMrkEn, "Reset: 1b … enabled by default" (§2.6.4.5, p. 186).
  • Consequence: The record says the whole write dropped a reserved bit and an SNP vAPIC mode. In fact it turns off a default-on feature, the silent drop of PRI stop markers.
    • Practically harmless here. No kernel driver enables PRI, PPREn is 0, and after B1 a stray stop marker becomes a requester record rather than a panic.
    • It is still a false statement about the evidence.
  • Fix: Correct the sentence to name bits 41 (BlkStopMrkEn) and 45 (EPHEn). No code change or rerun is needed for this.

S1 SUGGESTION: decide BlkStopMrkEn deliberately.

  • The comment justifies Coherent by its reset value, yet the write drops this field's reset value of 1.
  • Either set it when BlkStopMrkSup (MMIO 0030h) says it exists, or add a few words that stop markers are left to be recorded because no driver enables PRI.

S2 SUGGESTION, for stage 6: turn the unserved check into a refusal on the claim path.

  • amdvi/domain.rs:146-149 asserts that an attached function is served. Today only DeviceSpace::create and the selftest reach attach, and pcidev claims are NotRemapped on AMD-Vi.
  • When stage 6 opens claims, OwnSpace::create/attach for a function behind a refused unit must be refused at the claim (Refusal), not reach that assert from a process's request.
  • One line in the track issue's stage 6 would carry this.

S3 SUGGESTION: rewrap selftest.rs:15. It is one 124-column doc line where the edit joined two sentences.

S4 SUGGESTION (evidence): none of the d4/ logs or the d5/ runner log records the tested revision.

  • The guest and mutation runs are stamped 22:09 local, before the commit's 22:18.
  • Their content shows they ran the fix-round code: control=…140d in QEMU, and the patch context matches queue.rs:59-65.
  • Next time, put git rev-parse HEAD and git write-tree at the top of each log.

Evidence checked

Check Result
d4/logs/r2-ci-host.log exit 0; toyos-amdvi encodings: 13
d4/logs/r2-x86.log exit 0
d4/logs/r2-guest.log amdvi_selftest and amdvi_firmware_left PASS; 2 of 2; exit 0
d5/judge-r2.log 2 passed, 0 failed, 1 boot; amdvi_translates_the_stick_controller reports "2 controller(s) on domains of their own, the stick bound"; domain_ends_below_the_host_bridges passes; runner rc=0, judge EXIT=0

SEND BACK

@Japabu
Japabu marked this pull request as ready for review October 13, 2026 20:30
@Japabu
Japabu enabled auto-merge October 13, 2026 20:30
Japabu and others added 2 commits October 13, 2026 22:42
One conflict, tests/common/qemu.rs. #912 gave `Smmu` a third state,
`WithIts` (virt's SMMUv3 with the ITS behind it, an `edu` and an
`e1000e` past a bypassing root port) and built the machine argument from
`Arch::irqchip`; this branch had renamed `Smmu` to `Testdevs { Absent, Two }`
so a q35 behind AMD-Vi could carry two `iommu-testdev`. Both are kept as
one enum, `Probes { Absent, Testdevs, Its }`, under the field `probes`:
main's machine-argument code is taken whole with `shape.smmu` read as
`shape.probes`, its ITS devices emitted on `Probes::Its`, and the q35
assertion widened to admit `Probes::Testdevs` behind a unit and nothing
else. No file under kernel/src/arch/x86_64/amdvi, kernel/src/iommu,
kernel/src/arch/x86_64/vtd or kernel/src/arch/aarch64/smmu changed on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
One conflict, kernel/src/drivers/hda.rs. #877 lists every HDA controller
at boot and makes one device space before any is chosen, shared by
whichever one a bind attaches; this branch had `DeviceSpace::create` take
the one function its space is for, so AMD-Vi can answer the machine's own
addresses for a function no programmed unit serves or an IVMD names. Main's
side is taken whole, and `create` now takes every function that may be
attached: the space is the machine's own where any of them keeps it, so
whichever is bound reaches what its device addresses name. The other three
callers pass their one function.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
@Japabu
Japabu added this pull request to the merge queue Oct 14, 2026
Merged via the queue into main with commit 9b2b128 Oct 14, 2026
3 checks passed
@Japabu
Japabu deleted the wt/toyos-amdvi branch October 14, 2026 00:17
Japabu added a commit that referenced this pull request Oct 14, 2026
kernel/src/arch/x86_64/amdvi/mod.rs takes main's FIRMWARE_LEFT doc from #909,
which cites 48882 §2.4 and §2.5.1 and names no Linux; this branch's hunk there
goes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
Japabu added a commit that referenced this pull request Oct 14, 2026
…cpufacts

One conflict: the root `Cargo.toml`'s members, where main added
`kernel/amdvi` beside the `kernel/cpuvuln` this branch deletes; the
resolution keeps `kernel/amdvi` alone. Every other file both sides changed
merged cleanly: `Cargo.lock`, `kernel/Cargo.toml` and `tests/toyos.rs`, where
main adds rows and `RUST_SKIP` entries and this branch drops
`counters_on_metal`'s Linux request. Main removed `amdvi/mod.rs`'s Linux
citation, so `the-tree-names-linux-as-an-authority.md` no longer lists it
among the sweep's lines. Main touches none of `kernel/pure/cpu.rs`,
`kernel/pure/xsave.rs`, the HWP path in `control_regs.rs`, `counters.rs`,
`cpu.rs`, `microcode.rs`, `toyos-microcode`'s source or the judged rows.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
Japabu added a commit that referenced this pull request Oct 14, 2026
The shared domain registry and AMD-Vi's own domains (#909) take the
optional declaration as vtd and the SMMUv3 do: Domains::avoid holds None
where no declaration is accepted, and amdvi::init reserves its IVMDs only
beside declared windows, as vtd its RMRRs. pcidev hands the IOMMU a
validated toyos_pci::aperture::Declaration, which only check() makes.
PARange 0b0111 is FEAT_D128's 56 bits; the window header names no machine.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
Japabu added a commit that referenced this pull request Oct 14, 2026
…le is asserted, and every boot says the CPU's width

The AMD bench's recorded root-bridge list (#909's bench log) joins the
T14's as a held case of `check`. Its AMD-Vi domains are 48 bits (mode-4
host tables, 48882 §2.2.3) and its IVinfo's PASize is 48; if its CPU also
issues 48-bit addresses it keeps the declared rule, from 2^46 to 2^48 with
no window above 2^46, which a const assertion now holds. The CPU's width
was never in a boot log, so `iommu::init` says it once and the bench's own
boot decides which rule it is on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015xhaydXCVRNscoccHYFUWy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant