Skip to content

fix(qemu-acpi): emit ACPI PCI hotplug AML for root-port buses - #1203

Draft
kvinwang wants to merge 4 commits into
nextfrom
fix/qemu-acpi-root-port-hotplug
Draft

kvinwang wants to merge 4 commits into
nextfrom
fix/qemu-acpi-root-port-hotplug

Conversation

@kvinwang

Copy link
Copy Markdown
Collaborator

Problem

A TDX CVM with GPUs behind pcie-root-ports and ACPI PCI hotplug enabled (the default; hotplug_off = false) fails lite ACPI verification. A captured attestation from an 8×B200 CVM (QEMU 10.2.1, dstack-0.6.0-rc3) fails verifier-v0.6.0-rc3 with:

TDX lite acpi-loader digest mismatch:
  expected adae98e24401f6c8d6f3f2c24a78023ebc3c82ed951a92dde7958e5eae1fe1b4352ad1075aec822b687469075027b82c
  reported 01f02cba34d8f7213872ce341a587c636016a6268d0b9c2d2198058a8a709fe38cde80f67c808067444cb8fa11cdd27c

Root cause: once a root port's secondary bus is ACPI-hotplug capable, QEMU gets a BSEL for it (acpi_set_bsel) and adds hotplug AML to the DSDT. The generator emitted none of it:

  • per root port: BSEL, a Scope (S00_) with ASUN/_DSM/_SUN/_EJ0, and a DVNT method (build_pci_bridge_amlbuild_append_pcihp_slots);
  • a \_SB.PCI0.PCNT tree that selects each bus and notifies it (build_append_notification_callback);
  • a non-empty _GPE._E01 that calls \_SB.PCI0.PCNT under BLCK whenever a PCNT exists.

Fix

  • notify.rs: when hotplug is on, emit the hotplug AML for each pcie.0 root port, plus the PCNT scope. BSEL values run in reverse slot order because QEMU inserts each secondary bus at the head of the parent's child list (QLIST_INSERT_HEAD in pci_bridge_initfn) and assigns BSEL depth-first.
  • gpe.rs: _E01 acquires BLCK, calls PCNT, and releases BLCK when a PCNT exists. Otherwise it stays empty, as before.
  • PXB-attached root ports (hugepage/NUMA mode) get no AML. QEMU's expander-bridge DSDT node does not enumerate children ("Expander bridges do not have ACPI PCI Hot-plug enabled").
  • The helper documents the dstack-vmm device order the BSEL mapping depends on: ascending root-port slots, one function per port, and PXBs created before root ports.

QEMU version coverage

I extracted every QEMU function involved from the upstream tags and compared them after stripping comments and whitespace:

Function v8.2.0 → v11.1.0
build_append_pcihp_slots, build_append_pcihp_notify_entry, is_devfn_ignored_{generic,hotplug} identical
build_pci_bridge_aml, build_append_pci_bus_devices identical
aml_pci_device_dsm moved to hw/acpi/pcihp.c in 10.1, body identical
build_append_notification_callback (PCNT) moved and typo-renamed in 10.1, body identical
_E01 / has_pcnt block in build_dsdt identical apart from that rename
acpi_set_bsel, pci_for_each_bus_depth_first, child-bus QLIST_INSERT_HEAD, pcie_cap_slot_init identical
ICH9 acpi-pci-hotplug-with-bridge-support default for current q35 on in every version (11.1 only moved where the default is set)

So the new AML is version-independent across the supported range and needs no compatibility-profile gating.

Verification

  • Replayed the captured 8×B200 attestation through verifier-v0.6.0-rc3 plus this change. Only one other input differs from the capture: pci_hole64_size is set to the 64-bit window the guest actually saw (see the note below). With that, all four checks pass: quote, event log, OS image hash, and ACPI (is_valid: true, tcb_status: UpToDate).
  • tdx_lite_acpi_matches_captured_eight_gpu_qemu_10_2_vm pins all three RTMR0 ACPI digests (loader, RSDP, tables) from that CVM's event log. The loader digest alone only proves table sizes and offsets; the tables digest proves the AML bytes.
  • cargo test -p qemu-acpi (46 passed), cargo test -p dstack-verifier tdx_lite_acpi (5 passed), clippy clean.

Not covered: no QEMU-generated fixture with root ports and endpoints exists yet. The byte-level evidence for versions other than 10.2.1 is the source comparison above. A follow-up should add a root-port + endpoint topology to scripts/differential-upstream.sh.

Known gap (follow-up, not in this PR)

The DSDT _CRS 64-bit window is max(firmware-assigned BAR range, start + pci-hole64-size) (q35_host_get_pci_hole64_end). With the default pci_hole64_size = 0, QEMU uses a 32 GiB hole, but eight B200s need roughly 3.75 TiB of BAR space. The real window (0x380000000000-0x3bc006013fff) therefore cannot be derived from vm_config, and the tables digest still mismatches in production. Possible fixes:

  • GPU hosts configure a 1 GiB-aligned qemu_pci_hole64_size larger than the BAR span;
  • the VMM sizes the hole automatically;
  • the verifier rejects num_gpus > 0 && pci_hole64_size == 0 with a clear error.

Separately, the generator ignores QEMU's 1 GiB round-up of start + pci_hole64_size.

Co-authored with @Leechael, who wrote the qemu-acpi change.

Copilot AI lite review requested due to automatic review settings September 11, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved multi-PXB mapping and production BAR-window issues can cause ACPI measurement divergence.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds QEMU-compatible ACPI PCI hotplug AML for root-port buses to fix TDX lite ACPI measurements.

Changes:

  • Generates root-port hotplug AML, BSEL mappings, and PCNT notifications.
  • Updates _GPE._E01 handling.
  • Adds golden and eight-GPU regression tests.
File summaries
File Summary Final review comments
dstack/verifier/src/verification.rs Adds captured eight-GPU ACPI regression coverage. Moderate, 1 vote: Test supplies a realized BAR-window size and masks the production default failure.
dstack/crates/qemu-acpi/src/golden_tests.rs Updates hotplug-related golden tests and hashes. No final comments.
dstack/crates/qemu-acpi/src/dsdt/notify.rs Generates root-port hotplug and PCNT AML. Critical, 1 vote: Multi-PXB topologies can map PCNT scopes to incorrect devfns. Moderate, 2 votes: PXB handling lacks QEMU-generated coverage.
dstack/crates/qemu-acpi/src/dsdt/mod.rs Integrates hotplug AML and PCNT generation. No final comments.
dstack/crates/qemu-acpi/src/dsdt/gpe.rs Adds conditional PCI hotplug scanning through _E01. No final comments.
Review details

Suppressed comments (1)

dstack/verifier/src/verification.rs:2482

  • This regression test supplies the realized GPU BAR-window length instead of the production default (pci_hole64_size = 0/None). With the default, crs::build still emits the fixed 32 GiB window while QEMU derives _CRS from the firmware-assigned GPU BARs, so the real eight-B200 configuration continues to fail the tables digest; this test masks that failure. Please wire the realized window into measurement or reject GPU configurations without an explicit size before treating the verifier path as fixed.
            "pci_hole64_size": 0x3c0_0601_4000u64,
  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +121 to +125
pub(crate) fn pcnt(
slot_count: u32,
root_port_count: u32,
pxb_devfn: Option<u8>,
) -> Option<Vec<u8>> {
Comment on lines +164 to +168
fn root_ports(slot_count: u32, count: u32, pxb_devfn: Option<u8>) -> Vec<(u8, u32)> {
let mut ports = Vec::with_capacity(count as usize);
let mut slot = slot_count;
for index in 0..count {
if pxb_devfn == Some((slot * 8) as u8) {
@kvinwang
kvinwang marked this pull request as draft September 11, 2026 17:02
The oracle dumped the ACPI tables built inside acpi_setup(), a
machine_init_done notifier. QEMU assigns the ACPI hotplug BSEL property to
PCI bridge secondary buses in acpi_set_pci_info(), reached from the ICH9 PM
reset handler through acpi_pcihp_reset(), and machine reset runs after that
notifier. QEMU then rebuilds the tables via acpi_build_update(), so the
blob a guest reads -- and measures into RTMR0 -- is the rebuilt one.

Sampling before reset produced a DSDT in which no bus had a BSEL, and so
none of the root-port hotplug AML this branch adds. The oracle agreed with
any generator that omitted those terms: 17 of its 61 cases could not fail,
namely every case with bridge hotplug enabled and a root port on pcie.0.
That is why this branch's differential job reported the generator as wrong
when it was the reference that was wrong.

The fix lives in the QEMU compatibility fork, which now performs the one
reset side effect that changes the generated AML before dumping, and folds
the directory output mode in so both consumers sample at the same point.
Bump to that revision, drop the patch it replaces, and repin the image.

Also cross the axes that had only ever run GPU-less. Every per-version and
large-CPU fixed case ran with no GPUs, so no version tier and no CPU-count
boundary was pinned against a topology with root ports. Add the host
sharing mode as an explicit axis too: dstack-mr accepts "9p", "vvfat" and
"vhd" and models none of them, on the theory that each contributes exactly
one PCI slot and the DSDT records only _ADR; QEMU confirms all three are
byte-identical. Fixed cases go from 29 to 40.

Document the two measured inputs the oracle structurally cannot reach, so a
green run is not read as covering them: the 64-bit PCI window when
pci_hole64_size is 0, which QEMU derives from BARs the guest firmware has
assigned and no dump-and-exit oracle can observe, and multi-PXB topologies,
which MachineConfig cannot express.
@kvinwang
kvinwang force-pushed the fix/qemu-acpi-root-port-hotplug branch from fb366e2 to b6535e4 Compare September 12, 2026 06:43
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.

2 participants