opentmk: petri integration and opentmk_core lib split - #4211
opentmk: petri integration and opentmk_core lib split#4211mayank-microsoft wants to merge 27 commits into
Conversation
Resolve conflicts and re-integrate the opentmk feature against upstream changes: - Re-wire opentmk artifact build into the new vmm_tests_run resolve_artifact model (upstream removed artifact_to_build_mapping.rs). - Adapt UefiGuest::OpenTmk to upstream disk_path/DiskPath boot-drive model. - Move CommonArch/CommonProfile imports to crate::common. - Simplify: switch the opentmk guest logger from COM2 to COM1 and scan COM1 on the Hyper-V backend, dropping all com_2 plumbing and the unused OpenVMM opentmk serial-capture path. - Keep petri disk_image.rs unchanged (ESP/VHD logic lives in opentmk_disk). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Update comments/log messages that still said COM2 to COM1, matching the guest logger and Hyper-V scan now using COM1. - Downgrade dangling intra-doc links to opentmk_protocol::TestConfig to plain inline code in petri and vmm_tests (neither depends on opentmk_protocol directly; patching lives in opentmk_disk). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the extern "x86-interrupt" IDT handlers (which require the unstable abi_x86_interrupt feature) with assembly interrupt entry stubs that funnel into a single extern "C" handler, adapting the stable approach used by tmk_core. Stub addresses are installed into the IDT via the stable Entry::set_handler_addr. This lets OpenTMK build interrupt handling and its 5 interrupt tests (hv_memory_protect_read/write, hv_register_intercept, hv_tpm_read/write_cvm) on stable Rust. Removes all #[cfg(nightly)] gates, the abi_x86_interrupt feature, and the now-unused build.rs cfg(nightly) declaration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add petri vmm_tests entries for the 5 interrupt scenarios (3 on a non-isolated OpenHCL VTL guest, 2 TPM scenarios on SNP/TDX CVMs) via an opentmk_test! macro. Fix the interrupt handler ABI: x86_64-unknown-uefi uses the Win64 calling convention for extern "C", but the isr_common trampoline passes the saved frame in rdi with no shadow space (SysV). Mark isr_handler extern "sysv64" so it reads the argument from rdi. The 3 non-isolated interrupt tests pass on Hyper-V. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The hv_memory_protect_read/write tests require InterruptPlatformTrait and SecureInterceptPlatformTrait, which are only implemented for HvTestCtx on x86_64. Gate their tmk_tests! entries on target_arch = "x86_64" (matching the other interrupt/CVM tests) so opentmk compiles on aarch64 targets. Fixes the aarch64 clippy E0277 failures in CI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix the valid issues raised in Copilot review of PR microsoft#3254: - interrupt dispatch: an unhandled vector previously fell through to iretq silently. Log the vector when no handler is registered, and halt for the non-resumable vectors 8 (#DF) and 18 (#MC) so they cannot triple-fault into a silent reset without diagnostics (restores the old divergent-handler behavior lost in the nightly-drop refactor). - run_opentmk_uefi: capture the wait/evaluate result instead of using ?, so vm.teardown() always runs and a failed run no longer leaks a running VM. - wait_for_opentmk: return an error on timeout rather than Ok(default), which had masked the timeout as a misleading "never reported TEST_START". - read_capped_line: use saturating_add for the consumed counter to harden the untrusted-guest byte accounting against overflow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
get_xsdt_ptr used .ok_or(AcpiWrapError::InvalidXsdt.into()), whose argument is evaluated eagerly. Because From<AcpiWrapError> for TmkError logs via log::error!, every call logged "ACPI error: InvalidXsdt" even when the XSDT pointer was valid and the call returned Ok. The XSDT/MADT parse actually succeeds (vp_count == 4), so the test passed while emitting a misleading error line. Use ok_or_else so the error (and its logging From conversion) is only built on the None path. Verified on Hyper-V: opentmk_hyperv_openhcl_uefi_x64 still passes and the guest log no longer contains any ACPI error line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- opentmk_config_json: build the embedded config with serde_json instead of string formatting so the test name is escaped and the payload stays well-formed. - scan_opentmk_serial: log serial read errors instead of silently treating them as EOF, so broken-pipe/permission failures are diagnosable rather than surfacing only as a generic "never reported TEST_*". - interrupt_handler_register docs: fix a stale extern "C" reference (the handler is extern "sysv64") and document that, like tmk_core, isr_common preserves only integer GPRs (not SIMD/FPU), so handlers must stay minimal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The assembly ISR trampoline saved only the integer GPRs, so a handler that touched x87/SSE state (or Rust glue that emits XMM moves) could corrupt the interrupted context on iretq. Wrap the handler call in fxsave64/fxrstor64 to save and restore the x87, MXCSR, and XMM0-15 state in a 16-byte-aligned, 512-byte stack area. The x86_64-unknown-uefi target is the SSE2 baseline (no AVX), so fxsave64 captures the complete vector state. Validated on Hyper-V: hv_processor plus the three interrupt scenarios (memory_protect_read/write, register_intercept), which drive the trampoline through real page/GP faults, all pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- wait_for_opentmk: on timeout, race the deadline against a borrow of the scan task and explicitly cancel().await it (matching teardown at line 513) so the COM1 pipe and log file are closed deterministically instead of relying on drop-cancellation timing. - register_interrupt_handler: document why the Intel-reserved vectors (15, 22-27, 31) are left unset -- the x86_64 crate cannot set them and a spurious delivery faults through the handled #GP stub rather than silently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Build and publish the opentmk x86_64 UEFI binary (opentmk.efi) in the shared-linux job and consume it from the Windows VMM test jobs, so the opentmk tests can actually resolve their guest artifact and run in CI. Remove the opentmk exclusion from the standard filter. Give the isolated opentmk test variants an isolation host requirement so the snp/tdx/cvm variants only run on isolation-capable runners and are skipped elsewhere (matching the openvmm_test attribute macro behavior). Attach a vTPM for the hv_tpm_read_cvm / hv_tpm_write_cvm scenarios (via a tpm flag threaded through the opentmk_test! macro -> resolve -> OpentmkArtifacts -> PetriVmBuilder::with_tpm), which expect UEFI to expose the TCG protocol and a TPM command buffer. Exclude the opentmk CVM TPM tests from the CI CVM runners for now (they are not yet reliable there) via !test(opentmk_hyperv_openhcl_tpm) in the cvm_filter; they remain runnable locally via `cargo xflowey vmm-tests-run`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34167341-e52b-4594-9f87-97bec95ae81c
Rework the opentmk_test! macro to take a base name, scenario, tpm flag, and a list of isolation tokens ([none, vbs, snp, tdx]), generating one petri test per isolation with the matching name suffix. This removes the per-test boilerplate while keeping the generated test names identical. Add VBS support: the vbs token maps to IsolationType::Vbs, and opentmk_requirements now requires a baremetal host for VBS (Hyper-V VBS does not work nested), matching the #[vmm_test] macro behavior. Add a VBS variant to the baseline hv_processor scenario. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 34167341-e52b-4594-9f87-97bec95ae81c
…split # Conflicts: # Cargo.lock # flowey/flowey_hvlite/src/pipelines/build_opentmk.rs # flowey/flowey_lib_hvlite/src/_jobs/local_build_opentmk.rs # flowey/flowey_lib_hvlite/src/build_opentmk.rs # opentmk/opentmk_disk/src/lib.rs # opentmk/src/arch/x86_64/interrupt.rs # opentmk/src/arch/x86_64/interrupt_handler_register.rs # opentmk/src/tests/hyperv/hv_memory_protect_read.rs # opentmk/src/tests/hyperv/hv_memory_protect_write.rs # opentmk/src/tests/hyperv/mod.rs # opentmk/src/tests/mod.rs # opentmk_protocol/Cargo.toml # opentmk_protocol/src/lib.rs
Keep Cargo.toml and flowey_lib_hvlite/Cargo.toml byte-identical to main; the merge had reordered the opentmk workspace deps for no reason. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
opentmk_protocol was the only protocol crate at the repo root; every other one in the workspace nests under its owning component (e.g. tmk/tmk_protocol, petri/pipette_protocol, vm/devices/get/get_protocol). Its sibling opentmk_disk was already correctly placed at opentmk/opentmk_disk in the same change that introduced it. There is no build reason for the root placement: opentmk/ has no local cargo config, and the crate is a transitively-discovered library rather than a workspace member, so its directory is unconstrained. Pure directory move; only the workspace path entry changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Empty library that will hold the reusable OpenTMK harness. Deps mirror the opentmk binary; registered as a workspace path dependency rather than a members entry, since it is discovered transitively. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Pure git mv with no content edits, so this commit can be verified as a no-op: git diff -M reports 33 files, 0 insertions, 0 deletions. Path and visibility fixups follow in the next commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Adds lib.rs module declarations, the uefi submodule root, and dispatch.rs
holding opentmk_tests!/opentmk_backends! plus run_test().
run_test now takes the config region and the generated dispatch fn as
parameters: the region stays in the test binary so its dedicated linker
section is never garbage collected.
tmk_assert::{format_assert_json_string, write_str} become pub (doc-hidden)
since the exported tmk_assert! macro now expands across a crate boundary.
format! and asm! inside exported macros are qualified so they no longer
depend on what the caller has imported.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
The binary now holds only the test scenarios plus what must live in the artifact: the uefi entrypoint, the panic handler, and the config region. src/uefi/mod.rs becomes src/entry.rs and src/uefi/rt.rs becomes src/rt.rs, since the rest of the uefi module moved to core. OPENTMK_CONFIG moves to src/config.rs with its linker-section attributes intact. The core::arch::asm! qualification in create_function_with_restore! made two use core::arch::asm imports redundant; they are removed. Verified the .tmkcfg section survives the split: it is present in the built .efi with its magic, and opentmk_disk patches it and produces a bootable VHD end to end. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Intra-doc links to the exported macros resolve at the crate root, not inside dispatch. xtask fmt --fix dropped the dependencies the binary no longer uses now that the harness lives in opentmk_core. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
opentmk_core should be the platform library; test registration, dispatch, assertions, and test authoring helpers belong to the test binary. Pure git mv with no content edits: 3 files, 0 insertions, 0 deletions. Boundary fixups follow in the next commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
opentmk_core is now the platform library only: arch, context, devices,
platform, tmk_logger, tmkdefs, and the uefi runtime. It drops its
opentmk_protocol dependency, which only run_test used.
The binary picks up dispatch, test_helpers, and tmk_assert, and gains a
serde dependency for the assertion JSON derive.
Since the macros no longer cross a crate boundary, the accommodations for
that are reverted: tmk_assert::{format_assert_json_string, write_str} go
back to pub(crate) without #[doc(hidden)], and the test files return to
crate::-relative macro paths. The ::alloc::format! and ::core::arch::asm!
qualifications are kept so the macros do not depend on caller imports.
Re-verified the .tmkcfg section and the opentmk_disk patch round-trip.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Upstream replaced the six-argument SimpleTest::new with a three-argument constructor plus builder methods. The opentmk tests were written against the old signature and broke on the merge; the ignored and remote-access arguments are now defaults, so only requirements needs setting. This only failed on Windows: the Hyper-V test module is cfg(windows), so a Linux-only check compiles none of it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
There was a problem hiding this comment.
Pull request overview
Integrates OpenTMK guest tests into the Petri/vmm_tests infrastructure by adding an OpenTMK UEFI guest type, serial-result scanning/evaluation, and CI/artifact plumbing. It also splits OpenTMK into an opentmk_core library plus a shared opentmk_protocol crate for the patchable config region format.
Changes:
- Add OpenTMK UEFI guest support in Petri, including COM1 JSON scanning and run evaluation.
- Introduce
opentmk_coreandopentmk_protocolcrates; refactor the OpenTMK test binary to use them. - Wire new OpenTMK artifacts into vmm_tests, known-path resolution, and Flowey/CI pipelines.
Reviewed changes
Copilot reviewed 41 out of 73 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/opentmk.rs | New Petri-based OpenTMK guest test entrypoint (Hyper-V/OpenHCL) with isolation variants. |
| vmm_tests/vmm_tests/Cargo.toml | Registers the new opentmk test binary. |
| vmm_tests/petri_artifacts_vmm_test/src/lib.rs | Declares OpenTMK .efi artifacts (x64/aarch64). |
| vmm_tests/petri_artifact_resolver_openvmm_known_paths/src/lib.rs | Adds known-path resolution for opentmk.efi. |
| petri/src/vm/mod.rs | Adds UefiGuest::OpenTmk, lazy boot-disk construction, and firmware helpers. |
| petri/src/vm/hyperv/mod.rs | Adds Hyper-V COM1 scanning task and runtime API for collecting OpenTMK results. |
| petri/src/opentmk.rs | New serial JSON parser/scanner and pass/fail evaluation for OpenTMK runs. |
| petri/src/lib.rs | Exposes the new petri::opentmk module. |
| petri/Cargo.toml | Adds opentmk_disk dependency used to build the OpenTMK boot disk. |
| opentmk/src/tmk_assert.rs | Updates assertion logging to use opentmk_core logger and alloc::format!. |
| opentmk/src/tests/mod.rs | Refactors dispatch/config usage to new dispatch + config modules and opentmk_core ctx. |
| opentmk/src/tests/hyperv/mod.rs | Updates Hyper-V test ctx type to opentmk_core. |
| opentmk/src/tests/hyperv/hv_tpm_write_cvm.rs | Updates imports to opentmk_core for TPM plumbing. |
| opentmk/src/tests/hyperv/hv_tpm_read_cvm.rs | Updates imports to opentmk_core for TPM plumbing. |
| opentmk/src/tests/hyperv/hv_register_intercept.rs | Updates imports to opentmk_core for platform traits/tokens. |
| opentmk/src/tests/hyperv/hv_processor.rs | Updates imports to opentmk_core for platform traits/tokens. |
| opentmk/src/tests/hyperv/hv_memory_protect_write.rs | Updates imports to opentmk_core and removes unused asm import. |
| opentmk/src/tests/hyperv/hv_memory_protect_read.rs | Updates imports to opentmk_core and removes unused asm import. |
| opentmk/src/tests/hyperv/hv_error_vp_start.rs | Updates imports/error type reference to opentmk_core. |
| opentmk/src/test_helpers.rs | Adds module docs and uses fully-qualified core::arch::asm!. |
| opentmk/src/rt.rs | Adds UEFI panic handler that emits OpenTMK markers over serial. |
| opentmk/src/main.rs | Restructures OpenTMK binary into config/dispatch/entry/rt + helpers. |
| opentmk/src/entry.rs | UEFI entrypoint now initializes via opentmk_core and emits lifecycle markers. |
| opentmk/src/dispatch.rs | New macros + shared dispatch runner for patched config region. |
| opentmk/src/config.rs | New #[used] .tmkcfg config region kept in the test binary. |
| opentmk/opentmk_protocol/src/lib.rs | New shared protocol crate for config region layout + PE patching. |
| opentmk/opentmk_protocol/Cargo.toml | New crate manifest for opentmk_protocol. |
| opentmk/opentmk_core/src/uefi/mod.rs | Introduces uefi module surface in opentmk_core. |
| opentmk/opentmk_core/src/uefi/init.rs | Documents init flow (heap/logger/ACPI/VTL protection). |
| opentmk/opentmk_core/src/uefi/alloc.rs | New UEFI allocator wrapper with optional capped heap. |
| opentmk/opentmk_core/src/uefi/acpi_wrap.rs | Small ACPI wrapper tweak (ok_or_else). |
| opentmk/opentmk_core/src/tmkdefs.rs | Moves TMK error/result types into opentmk_core. |
| opentmk/opentmk_core/src/tmk_logger.rs | Switches x86_64 logger to COM1. |
| opentmk/opentmk_core/src/platform/mod.rs | New platform module root. |
| opentmk/opentmk_core/src/platform/hyperv/mod.rs | New Hyper-V platform module root. |
| opentmk/opentmk_core/src/platform/hyperv/ctx.rs | New Hyper-V test context implementation (shared across arch backends). |
| opentmk/opentmk_core/src/platform/hyperv/arch/x86_64/mod.rs | New x86_64 Hyper-V arch module root. |
| opentmk/opentmk_core/src/platform/hyperv/arch/x86_64/hypercall.rs | New x86_64 Hyper-V hypercall helpers (start/enable VP, VTL call/return). |
| opentmk/opentmk_core/src/platform/hyperv/arch/x86_64/ctx.rs | New x86_64 implementations of platform traits for Hyper-V tests. |
| opentmk/opentmk_core/src/platform/hyperv/arch/mod.rs | New arch selection layer for Hyper-V platform. |
| opentmk/opentmk_core/src/platform/hyperv/arch/hypercall.rs | New arch-neutral hypercall infrastructure for Hyper-V. |
| opentmk/opentmk_core/src/platform/hyperv/arch/aarch64/mod.rs | New aarch64 Hyper-V arch module root (stubbed). |
| opentmk/opentmk_core/src/platform/hyperv/arch/aarch64/hypercall.rs | aarch64 hypercall wrappers and placeholder VTL call/return. |
| opentmk/opentmk_core/src/platform/hyperv/arch/aarch64/ctx.rs | aarch64 platform trait impl stubs. |
| opentmk/opentmk_core/src/lib.rs | New opentmk_core crate root and module exports. |
| opentmk/opentmk_core/src/devices/tpm/mod.rs | New TPM device trait + utilities. |
| opentmk/opentmk_core/src/devices/mod.rs | New devices module root. |
| opentmk/opentmk_core/src/context.rs | New platform abstraction traits (VtlPlatformTrait, etc.). |
| opentmk/opentmk_core/src/arch/x86_64/tpm.rs | New x86_64 TPM MMIO/IO-port implementation. |
| opentmk/opentmk_core/src/arch/x86_64/serial.rs | New x86_64 serial writer implementation. |
| opentmk/opentmk_core/src/arch/x86_64/rtc.rs | New x86_64 RTC utilities (read + delay). |
| opentmk/opentmk_core/src/arch/x86_64/mod.rs | New x86_64 arch module root. |
| opentmk/opentmk_core/src/arch/x86_64/io.rs | New x86_64 port I/O helpers. |
| opentmk/opentmk_core/src/arch/x86_64/interrupt.rs | New x86_64 interrupt dispatch/IDT setup. |
| opentmk/opentmk_core/src/arch/x86_64/interrupt_handler_register.rs | New x86_64 global-asm ISR stubs and IDT registration. |
| opentmk/opentmk_core/src/arch/x86_64/hypercall.rs | New x86_64 hypercall-page/MSR initialization helpers. |
| opentmk/opentmk_core/src/arch/mod.rs | New arch selection layer for opentmk_core. |
| opentmk/opentmk_core/src/arch/aarch64/mod.rs | New aarch64 arch module root. |
| opentmk/opentmk_core/src/arch/aarch64/hypercall.rs | New aarch64 hypercall init/uninit helpers. |
| opentmk/opentmk_core/Cargo.toml | New crate manifest for opentmk_core. |
| opentmk/Cargo.toml | Updates opentmk binary crate deps to use opentmk_core + protocol split. |
| flowey/flowey_lib_hvlite/src/init_vmm_tests_env.rs | Copies opentmk.efi into test content dir when present. |
| flowey/flowey_lib_hvlite/src/_jobs/local_build_and_run_nextest_vmm_tests.rs | Adds OpenTMK to local build selections and build graph. |
| flowey/flowey_lib_hvlite/src/_jobs/consume_and_test_nextest_vmm_tests_archive.rs | Adds OpenTMK to vmm_tests archive artifact dependencies. |
| flowey/flowey_hvlite/src/pipelines/vmm_tests_run.rs | Triggers OpenTMK build when OpenTMK artifacts are requested. |
| flowey/flowey_hvlite/src/pipelines/checkin_gates.rs | Publishes OpenTMK artifact and excludes flaky OpenTMK TPM CVM cases in gates. |
| ci-flowey/openvmm-pr.yaml | Updates generated CI pipeline to publish/consume the new OpenTMK artifact. |
| Cargo.toml | Adds workspace paths for opentmk_protocol and new opentmk_core. |
| Cargo.lock | Adds opentmk_core and updates dependency graph (incl. opentmk_disk use in petri). |
| .github/workflows/openvmm-pr.yaml | Updates GH Actions workflow to download/use x64-opentmk artifact. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Several comments enumerated the current test matrix or asserted that OpenTMK is Hyper-V and x86_64 only. Three were already wrong: they listed the isolation variants as snp/tdx and predate vbs. Drops those claims and the comments that only restate the code beneath them. Kept the ones that explain something non-obvious: the .tmkcfg linker-section rationale, the black_box load, the bounded serial read, the std::fs over fs_err choice, and why VBS additionally needs a baremetal host. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
Behaviour-preserving: pointer::cast over an as-cast, core::ptr::from_ref over a reference as-cast, and is_multiple_of over a manual modulo. These predate the split but land in a crate this change introduces, so clean them up here. x86_64-unknown-uefi is now warning-free; the remaining aarch64 dead-code warnings are arch-conditional (the items are live on x86_64) and are left alone. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dbd798c5-1fba-419d-b75a-a47e8a4cdeb7
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
|
This is pretty big, can we split this into separate smaller PRs? |
No description provided.