aarch64: advertise host SSID/ATS for accelerated SMMU - #4344
aarch64: advertise host SSID/ATS for accelerated SMMU#4344Henry Li (henryli001) wants to merge 2 commits into
Conversation
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
2922fbe to
c210d1d
Compare
There was a problem hiding this comment.
Pull request overview
Enables PASID/ATS-related plumbing for accelerated aarch64 VFIO guests by (1) reserving/relocating guest RAM to keep host-reserved IOVA regions unbacked while still booting UEFI, and (2) propagating host SMMUv3 + per-endpoint PASID capabilities into the guest-visible vSMMU, ACPI IORT, and PCI config emulation.
Changes:
- Add an aarch64 UEFI-compatible RAM layout: keep a small low RAM window at GPA 0 and place bulk RAM above 1 GiB to avoid the iommufd MSI doorbell reserved IOVA hole.
- Extend iommufd capability querying and propagate host ATS/SSID (SMMU) plus per-device PASID capabilities through the vSMMU and into PCI endpoint emulation (synthetic PASID ext-capability).
- Mark IORT PCI root complexes as ATS-capable when configured, and preserve ATS/SSID-related fields in nested STE canonicalization when advertised.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vmm_core/src/acpi_builder.rs | Threads per-RC ATS support into IORT RC node construction. |
| vm/devices/user_driver/vfio_sys/src/iommufd.rs | Extends IOMMU_GET_HW_INFO results to include max PASID width + capability bits. |
| vm/devices/pci/vfio_assigned_device/src/resolver.rs | Captures per-device PASID caps from nesting and passes into device construction. |
| vm/devices/pci/vfio_assigned_device/src/manager.rs | Plumbs (host_caps, device_caps) from iommufd nesting query into nesting output. |
| vm/devices/pci/vfio_assigned_device/src/lib.rs | Synthesizes a PASID extended capability and hooks it into config-space read/write/reset; adds tests. |
| vm/devices/pci/vfio_assigned_device/src/iommufd_nesting.rs | Splits host SMMU caps vs per-endpoint PASID caps from get_hw_info. |
| vm/devices/pci/pci_core/src/spec.rs | Adds PASID to the extended capability ID list. |
| vm/devices/iommu/smmu/src/shared.rs | Tracks advertised SSID/ATS in shared state and preserves related STE fields when enabled. |
| vm/devices/iommu/smmu/src/emulator.rs | Decodes SSIDSIZE/ATS from host IDR and advertises them via effective IDR0/IDR1; gates CR0.ATSCHK and ATC_INV. |
| vm/acpi_spec/src/iort.rs | Extends IORT PCI RC node constructor to carry ATS attribute. |
| openvmm/openvmm_core/src/worker/memory_layout.rs | Adds low-RAM window support and ensures the MSI doorbell hole remains unbacked. |
| openvmm/openvmm_core/src/worker/dispatch/smmu_wiring.rs | Populates ACPI SMMU config with ATS support flag (currently derived from accel). |
| openvmm/openvmm_core/src/worker/dispatch.rs | Selects (ram_start_address, low_ram_window_size) for aarch64 (Linux vs UEFI). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| configs.push(vmm_core::acpi_builder::AcpiSmmuConfig { | ||
| rc_index: pcie_host_bridges[rc_pos].index, | ||
| segment: pcie_host_bridges[rc_pos].segment, | ||
| base: smmu.base, | ||
| event_gsiv: smmu.evtq_intid, | ||
| gerr_gsiv: smmu.gerr_intid, | ||
| ats_supported: accel, | ||
| reserved_iova_ranges, |
There was a problem hiding this comment.
Addressed in 4b5f4f6. Cold-plug VFIO binding resolves HostSmmuCaps before firmware construction; the SMMU ACPI configs are now refreshed from SmmuSharedState::ats_supported(), which follows the physical IDR0.ATS bit. The new regression test covers both ATS-present and ATS-absent hosts.
| .count(); | ||
|
|
||
| // On aarch64 Linux direct boot, start RAM at 1 GiB to avoid the low GPA | ||
| // On aarch64, place the bulk of RAM at 1 GiB to avoid the low GPA |
There was a problem hiding this comment.
Hm, should we do this all the time? I'm not against it, and I don't think we have any compat concerns yet. I'm just wondering if we should have a mode where we don't do this?
There was a problem hiding this comment.
I split the memory-layout change into #4372 so this policy can be reviewed independently. It remains scoped to ARM64 UEFI boots; direct Linux boot is unchanged. I added the rationale and compatibility-mode question to that PR.
c210d1d to
4b5f4f6
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The synthetic PASID capability header DWORD is currently writable-through to the physical device config space, which can allow guest writes to mutate host config state and desync the synthetic capability chain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 1
- Review effort level: Lite
|
Fixed the quick-check failure in 5949152 by making the synthetic PASID capability bitfield precedence explicit. Validated with Rust 1.95 |
There was a problem hiding this comment.
🔵 Needs a closer look
The synthetic PASID capability currently allows guest writes to the synthetic header DWORD to fall through to hardware config-space writes, which should be intercepted to avoid undefined host-device behavior.
Review details
Suppressed comments (1)
vm/devices/pci/vfio_assigned_device/src/lib.rs:1816
- Synthetic PASID capability writes are intercepted only for the control DWORD at
pasid.offset + 4; a guest write to the synthetic capability header DWORD atpasid.offsetwill currently be forwarded to the physical device viawrite_phys_config. Since this offset is being repurposed to hold a synthetic extended capability header, forwarding guest writes there can cause undefined behavior on the host device. Consider intercepting and dropping writes to the synthetic header DWORD as well (it should be read-only).
_ if self
.synthetic_pasid
.as_ref()
.is_some_and(|pasid| offset == pasid.offset + 4) =>
{
self.synthetic_pasid.as_mut().unwrap().write(value);
return IoResult::Ok;
}
// All other registers: pass through to physical device.
_ => self.write_phys_config(offset, value),
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Lite
Decode SSIDSIZE and ATS from the physical SMMU, expose them through the accelerated vSMMU before start, gate ATSCHK and ATC invalidation accordingly, and preserve PASID/ATS-dependent nested STE fields only when advertised. Add coverage for capability advertisement, ATS-disabled invalidation, and conditional STE canonicalization.
Carry ATS support through the IORT root-complex node and derive it from the physical SMMU's resolved IDR0.ATS capability before firmware construction. This keeps ACPI consistent with what the accelerated vSMMU advertises to the guest.
5949152 to
f13a40d
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes low-level aarch64 IOMMU/ACPI capability advertisement and ATS behavior in ways that are hardware- and firmware-interaction sensitive and warrants final human review.
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
Advertise physical SMMUv3 PASID/SSID width and ATS support through an accelerated ARM vSMMU, and report the same ATS capability in ACPI IORT.
Changes
IDR1.SSIDSIZEandIDR0.ATSfromIOMMU_GET_HW_INFO.IDR0andIDR1.CR0.ATSCHKand forwardATC_INVonly when the physical SMMU supports ATS.S1Fmt,S1CDMax, andS1DSSin nested STEs when SSID support is advertised, and preserveEATSwhen ATS is advertised.IDR0.ATScapability before firmware construction.Split PRs
Validation
cargo +1.95.0 test -p smmu --lib(172 passed)cargo +1.95.0 test -p openvmm_core --lib(56 passed)OPENVMM_GUEST_TARGET=aarch64 RUSTFLAGS='-D warnings' cargo +1.95.0 clippy -p smmu -p openvmm_core --all-targets --all-featurescargo xtask fmt --only-diffed