[DRAFT] tdisp: Add support for TDX Connect - #4414
Closed
Michael Frohlich (mfrohlich-msft) wants to merge 99 commits into
Closed
Michael Frohlich (mfrohlich-msft) wants to merge 99 commits into
Michael Frohlich (mfrohlich-msft) wants to merge 99 commits into
Conversation
…gher level interfaces
…t to confirm this works
…to tdisp state in vpci
Parse the TDI interface report during attest() and save it in the mutable state so subsequent operations can consult it. Use the report's MMIO range flags (is_non_tee_mem) to decide whether each BAR's MMIO range must be validated via tdisp_unblock_mmio: only protected (TEE) ranges are unblocked; non-TEE ranges are skipped.
The Linux sev-guest driver's TIO_GUEST_REQUEST ioctl returns 0 from the syscall even when the ASP or VMM rejected the request; the real status is reported out-of-band via exitinfo1/exitinfo2 on the in/out struct (and the driver also disables VMPCK0 to prevent IV reuse). We were ignoring those fields and returning the zero-initialized response body as if the request had succeeded, which led to misleading 'tio_guest_request completed successfully' traces followed by a guest with a dead VMPCK0. Check exitinfo1/exitinfo2 after the ioctl and return a new TioGuestRequestFirmware error variant when either is non-zero.
sevtio::tdisp_unblock_mmio was only passing the base PFN to modify_gpa_visibility(PRIVATE, &[pfn]) — trailing pages of a multi-page BAR were left host-visible, so subsequent PSP validate / rmpadjust ran against partially-converted memory. Build the full PFN list from [base_pfn, base_pfn + length_in_pages) and hand it to the hypercall in one call; also log the range being flipped so partition-terminating failures have a visible last breadcrumb. vpci_client tdisp state now tracks a set of paravisor-intercepted BARs. MSI-X table / PBA BARs are backed by BarMemoryKind::Intercept on the paravisor side, so there is no convertible guest-RAM page for modify_gpa_visibility to convert — trying to do so silently terminates the partition. Expose mark_bar_intercepted / is_bar_intercepted on the TDISP state, surface the setter through TdispVpciAttestationInterface as tdisp_mark_bar_intercepted, and auto-mark any MMIO range whose TDI interface report flags report range_maps_msix_table or range_maps_msix_pba. tdisp_on_mmio_reconfigured short-circuits on those BARs before the unblock path runs.
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
Copilot started reviewing on behalf of
Michael Frohlich (mfrohlich-msft)
September 10, 2026 21:50
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
One or more issues must be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds TDX Connect support and extends TDISP/VPCI resource-isolation handling for TDX and SEV-TIO.
Changes:
- Adds TDX Connect ABI definitions and TDCALL wrappers.
- Adds TDISP MMIO commands, VPCI protocol negotiation, and isolation reporting.
- Adds platform resource validators and supporting tests.
File summaries
| File | Description |
|---|---|
| vm/x86/x86defs/src/tdx.rs | Updated as part of this pull request. |
| vm/x86/tdcall/src/lib.rs | Updated as part of this pull request. |
| vm/hv1/hvdef/src/lib.rs | Updated as part of this pull request. |
| vm/hv1/hvdef/Cargo.toml | Updated as part of this pull request. |
| vm/devices/tdisp/src/tests/statemachine_tests.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/tests/serialize_tests.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/tests/mocks.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/tests/endtoend_tests.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/test_helpers.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/serialize_proto.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/lib.rs | Updated as part of this pull request. |
| vm/devices/tdisp/src/devicereport.rs | Updated as part of this pull request. |
| vm/devices/tdisp_proto/src/tdisp.proto | Updated as part of this pull request. |
| vm/devices/tdisp_proto/src/lib.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci/src/device.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci_relay/src/lib.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci_relay/Cargo.toml | Updated as part of this pull request. |
| vm/devices/pci/vpci_protocol/src/lib.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci_client/src/tests.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci_client/src/lib.rs | Updated as part of this pull request. |
| vm/devices/pci/vpci_client/Cargo.toml | Updated as part of this pull request. |
| vm/chipset_device/src/lib.rs | Updated as part of this pull request. |
| vm/chipset_device_resources/src/lib.rs | Updated as part of this pull request. |
| support/sev_guest_device/src/lib.rs | Updated as part of this pull request. |
| support/sev_guest_device/sev_guest_device_tio/src/lib.rs | Updated as part of this pull request. |
| support/sev_guest_device/Cargo.toml | Updated as part of this pull request. |
| openhcl/underhill_core/src/worker.rs | Updated as part of this pull request. |
| openhcl/underhill_core/Cargo.toml | Updated as part of this pull request. |
| openhcl/openhcl_tdisp/src/tdxconnect.rs | Updated as part of this pull request. |
| openhcl/openhcl_tdisp/src/sevtio.rs | Updated as part of this pull request. |
| openhcl/openhcl_tdisp/src/noop.rs | Updated as part of this pull request. |
| openhcl/openhcl_tdisp/src/lib.rs | Updated as part of this pull request. |
| openhcl/openhcl_tdisp/Cargo.toml | Updated as part of this pull request. |
| openhcl/hcl/src/ioctl/tdx.rs | Updated as part of this pull request. |
| openhcl/hcl/src/ioctl.rs | Updated as part of this pull request. |
| openhcl/hcl/Cargo.toml | Updated as part of this pull request. |
| Cargo.lock | Updated as part of this pull request. |
Review details
Suppressed comments (12)
openhcl/openhcl_tdisp/src/noop.rs:182
- The no-op validator is shared by all relayed devices, but this removes every recorded range with the same BAR/range ID, regardless of
device_id. Teardown of BAR 0 on one device therefore erases another device's recorded unblock, making the validator's state and tests incorrect. Match both the TDI device ID and range ID.
self.unblocked_mmio_ranges
.lock()
.retain(|r| r.range_id != range_id);
openhcl/openhcl_tdisp/src/sevtio.rs:521
- The DMA block operation has the same panic path: a firmware or ioctl failure from
tio_msg_sdte_write_reqis unwrapped during unbind. Propagate the error so cleanup can retain/report the failed state rather than crashing OpenHCL.
.unwrap();
openhcl/openhcl_tdisp/src/sevtio.rs:357
- When
vtomis absent, the worker passes zero to this validator.vtom_high - 1then underflows in debug builds (and becomes an all-ones mask in release), before the SDTE request can report a useful configuration error. Use checked subtraction and return an error for an invalid VTOM.
let vtom_high = (self.vtom >> SHIFT_2MB) as u32;
// Subtract 1 to create the mask for the non-VTOM bit parts of the address
let vtom = vtom_high - 1;
openhcl/openhcl_tdisp/src/tdxconnect.rs:535
- Accepting one page per TDCALL makes a 64 MiB BAR require 16,384 ioctl/TDCALL operations here, followed by another page-by-page attribute loop. That can make guest activation take an unbounded amount of time under load and exposes a large denial-of-service surface to device BAR sizes; plumb the R9 resume output or otherwise batch/limit the operation before treating large MMIO ranges as supported.
// TDISP TODO: this accepts one 4K page per call because
// `tdcall_tdi_mmio_accept` requires it: the leaf reports its resume
// cursor in R9 and the ioctl cannot read R9 back. EAS 4.3.5.1 says an
// interrupted accept resumes the TD with only RCX and R9 updated and
// that "guest TD software is not directly involved", which suggests
openhcl/openhcl_tdisp/src/tdxconnect.rs:51
- This module now issues
TDG.TDI.MMIO.ACCEPTandTDG.DMAR.ACCEPTintdisp_unblock_mmio/tdisp_unblock_dma, so the statement that the unblock paths are stubs that do not issue accept TDCALLs is no longer true and will mislead anyone debugging resource setup. Update this documentation to describe the implemented accept path and the host-side-only block path.
/// After a device has been attested and placed in the Run state, this struct
/// will issue guest-side TDCALLs to the TDX Module to make device resources
/// (MMIO, DMA) accessible to the guest. Note that the unblock paths are
/// currently stubs that validate the TDI but do not yet issue the accept
/// TDCALLs, so no resource is actually made accessible.
openhcl/openhcl_tdisp/src/tdxconnect.rs:96
- The TDISP report permits multiple MMIO entries with the same
range_idfor grouped subranges, but this always selects the first entry. For such a BAR,TDG.TDI.MMIO.ACCEPTreceives the wrongMMIO_RANGE_IDXand the report's subrange offset is ignored, so pages in the requested subrange may remain unaccepted. Resolve the report index using the requested GPA/offset and length, not onlyrange_id.
let index = ranges
.iter()
.position(|r| r.range_id == range_id)
.with_context(|| {
vm/devices/pci/vpci_client/src/tdisp.rs:677
- DMA cleanup has the same VTL mismatch as the MMIO cleanup above: DMA was unblocked using
self.target_vtl, but is re-blocked usingVtl::Vtl2. On the normal VPCI path this blocks the wrong VTL/VMPL and can leave the guest's DMA mapping active. Useself.target_vtlfor the block call.
if let Err(e) = validator.tdisp_block_dma(Vtl::Vtl2, raw_device_id) {
vm/devices/pci/vpci_client/src/tdisp.rs:672
- The BAR is removed from
validated_mmio_barsafter platform blocking succeeds even when the host-side block command fails. In that failure case the host still considers the range unblocked, but the record needed to retry the host block has been discarded. Only remove the entry after both platform and host blocking succeed.
// Successful re-block, remove the bar from the validated list.
self.mutable_state.validated_mmio_bars.remove(&bar_id);
vm/devices/pci/vpci_client/src/tdisp.rs:1073
- This early return makes the documented reconfiguration path incorrect: if a BAR is programmed to a new base or length after its first validation, the old mapping remains private while the new range is never host-unblocked or platform-validated. Track the previously validated range and tear it down/revalidate, or reject BAR changes while resources are active instead of skipping solely by
bar_id.
if self.mutable_state.validated_mmio_bars.contains_key(&bar_id) {
tracing::debug!(
bar_id,
"skipping MMIO unblock for BAR that has already been validated"
);
vm/devices/pci/vpci_relay/src/lib.rs:389
- A successful mock TDISP flow is not reflected in
tdisp_capable. The resulting relay bypasses the TDISP command-register interception and reportsNotTdispCapablefor the mock device, so this test scenario does not exercise the behavior it just initialized. Set the flag after the mock flow succeeds.
if self.options.test_tdisp_flow {
Self::tdisp_test_mock_flow(vpci_device.clone())
.await
.expect("failed to exercise TDISP flow test");
vm/devices/pci/vpci_relay/src/lib.rs:144
VpciClientTdispState::newinitializes every device toUnlocked, including devices whose capability probe failed. This makes the condition true for non-TDISP devices and sendstdisp_unbindeven though no TDI was bound; retain the successful capability result (or track a completed bind) inRelayedDeviceand gate teardown on that state.
// Only devices that actually completed at least a Bind have a
// TDI on the host side to unbind. Non-TDISP devices stay in
// `Uninitialized` and must be left alone. `tdisp_unbind` on
// them would return a host error.
if self.vpci_device.tdisp_tdi_state().await != TdispTdiState::Uninitialized {
vm/x86/x86defs/src/tdx.rs:276
DMAR_TARGET.VM_IDXis a 2-bit field in the TDX Connect layout, but usingu8without a#[bits]attribute allocates all 8 low bits and leaves only 56 reserved bits. Valid current values happen to be below 4, but the public bitfield can encode values into bits that the ABI reserves; model the field as 2 bits and reserve 62 bits.
- Files reviewed: 37/38 changed files
- Comments generated: 15
- 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
+68
to
+72
| // Leaving pages stuck immutable is unrecoverable. Matches the | ||
| // PagesAccessibleToLowerVtl precedent in lower_vtl_permissions_guard. | ||
| panic!( | ||
| "failed to roll back immutable bit on {} PFNs ({processed} cleared before failure): {e:?}", | ||
| self.pfns.len() |
Comment on lines
+213
to
+217
| let length_in_pages = length_in_bytes / hvdef::HV_PAGE_SIZE; | ||
|
|
||
| // Build the full list of PFNs covered by the MMIO range. | ||
| let pfns: Vec<u64> = (0..length_in_pages).map(|i| base_pfn + i).collect(); | ||
|
|
| .sev_guest | ||
| .tio_msg_sdte_write_req(device_id, true, vtom, Self::vtl_to_vmpl(target_vtl)) | ||
| .context("failed to send SDTE write request") | ||
| .unwrap(); |
Comment on lines
+546
to
+550
| for i in 0..page_count { | ||
| let page_gpa = base_gpa + (u64::from(i) << hvdef::HV_PAGE_SHIFT); | ||
|
|
||
| let gpa_base_and_level = TdgTdiMmioAcceptRcx::new() | ||
| .with_level(TdgMemPageLevel::Size4k) |
Comment on lines
+384
to
+391
| let tdisp = VpciClientTdispState::new( | ||
| req.clone(), | ||
| id.slot.into_bits() as u64, | ||
| resource_validator, | ||
| isolation_type, | ||
| vtom, | ||
| target_vtl, | ||
| implemented_bars(&requirements.bars), |
Comment on lines
+657
to
+667
| // Once MMIO is on the TDI is bound and its ranges have been | ||
| // unblocked and accepted into the guest. Drop the write rather | ||
| // than letting the guest walk that back: the disable edge would | ||
| // unbind the device and re-block every range. | ||
| tracing::warn!( | ||
| ?offset, | ||
| ?value, | ||
| "dropping a config space write that would disable MMIO; the command \ | ||
| register does not transition back to off once it is on" | ||
| ); | ||
| return IoResult::Ok; |
| pub fn new(vtom: u64) -> anyhow::Result<Self> { | ||
| let sev_guest = SevGuestDevice::open() | ||
| .context("failed to open /dev/sev-guest") | ||
| .unwrap(); |
| worker_req, | ||
| vpci_device_id: device_id, | ||
| mutable_state: VpciClientTdispMutableState { | ||
| tdi_state: TdispTdiState::Unlocked, |
Comment on lines
+693
to
+697
| // Clear every per-attest field. All of these will be fetched cleanly on | ||
| // the next re-attest cycle. | ||
| self.mutable_state.tdi_report = None; | ||
| self.mutable_state.guest_device_id = TdispDeviceId::Invalid; | ||
| self.mutable_state.intercepted_bars.clear(); |
Comment on lines
+1153
to
+1156
| self.resource_validator | ||
| .tdisp_unblock_mmio(self.target_vtl, device_id, base_address, 0, length, bar_id) | ||
| .await | ||
| .context("tdisp_on_mmio_reconfigured: failed to unblock MMIO")?; |
Member
|
Michael Frohlich (@mfrohlich-msft), this monolithic TDX Connect prototype appears superseded by the split follow-up work and has unresolved trust-boundary issues. Could you close it, or identify anything still worth reviving? Thanks. |
Contributor
Author
|
This is the stacked PR on top of the other PR that's under review: #4416 I don't need this reviewed right now (which is why it's in Draft) but I'll close it just so it doesn't show up on any dashboards. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft PR for all changes to support TDX Connect and recent VPCI infrastructure.