From acd0ee0295ec0a7470955d6e065f1ce226372f56 Mon Sep 17 00:00:00 2001 From: Mark Gentry Date: Fri, 14 Aug 2026 07:51:55 -0500 Subject: [PATCH 1/5] fix: re-read the VM profile from context between steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit execute_test assigned the loop's local `profile` onto `ctx.profile` at the top of every iteration. A callable step that replaces the profile — `dataclasses.replace` on a frozen VMProfile returns a new object, so mutating it in place is not possible — had that replacement silently overwritten before the next step ran. Any handler that reconfigures the guest for subsequent steps was therefore a no-op. Read the profile out of the context instead of writing the stale local back over it. --- sev_verify/cli.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sev_verify/cli.py b/sev_verify/cli.py index 1e7677eb..8b26b5d1 100644 --- a/sev_verify/cli.py +++ b/sev_verify/cli.py @@ -378,7 +378,11 @@ def execute_test( for i, step in enumerate(steps): is_last = i == total_steps - 1 - ctx.profile = profile + # Re-read the profile from the context rather than overwriting it. + # A callable step may replace ctx.profile (dataclasses.replace on a + # frozen VMProfile yields a new object); assigning the stale local + # back over it silently discarded that change for every later step. + profile = ctx.profile ctx.launch = launch if _IS_TTY: From 11fd7c27d6a564a7ebe9c9a79d83fbacce21844f Mon Sep 17 00:00:00 2001 From: Mark Gentry Date: Fri, 14 Aug 2026 07:52:20 -0500 Subject: [PATCH 2/5] fix: honour expected_result when a VM launch raises run_vm_launch_step reported "error" unconditionally when vm_launch raised VMLaunchError, ignoring the step's expected_result. That made it impossible to write a step that expects a launch to be refused: QEMU exiting immediately raises rather than returning ok=False, so a step declaring expected_result="exit_code:1" errored on exactly the outcome it was asserting. Evaluate expected_result against exit code 1 and the exception text, and record exit_code=1 so the result is self-describing. Steps that do not declare an expectation still default to "exit_code:0" and so still report error, leaving existing behavior unchanged. --- sev_verify/runner.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sev_verify/runner.py b/sev_verify/runner.py index d019bda8..9a8fd56e 100644 --- a/sev_verify/runner.py +++ b/sev_verify/runner.py @@ -175,10 +175,16 @@ def run_vm_launch_step( launch = profile.vm_launch() except VMLaunchError as exc: duration_ms = int((time.monotonic() - start) * 1000) + # A launch that fails hard is still a launch outcome, so honour the + # step's expected_result. Reporting "error" unconditionally made it + # impossible to write a step that expects a launch to be refused — + # QEMU exiting immediately raises rather than returning ok=False. + passed = _check_expected_values(step, 1, str(exc)) return ( StepResult( step=step, - result="error", + result="pass" if passed else "error", + exit_code=1, stderr=str(exc), duration_ms=duration_ms, ), From 07723960a2b4f660147dab88ae60235a7268d0d3 Mon Sep 17 00:00:00 2001 From: Mark Gentry Date: Fri, 14 Aug 2026 07:52:55 -0500 Subject: [PATCH 3/5] fix: stop a later test result masking a worse earlier one execute_certification folded per-test results with if tr.result != "pass": overall = tr.result which is last-writer-wins, not worst-wins. A run where the first test errors and a later one merely fails reported the certification as "fail", losing the error. The bug is latent today because both outcomes are non-passing, but it silently downgrades severity in reports, and any new non-passing state makes it worse. Fold with an explicit severity ordering instead. --- sev_verify/cli.py | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/sev_verify/cli.py b/sev_verify/cli.py index 8b26b5d1..93c5dde3 100644 --- a/sev_verify/cli.py +++ b/sev_verify/cli.py @@ -496,6 +496,20 @@ def execute_test( stop_vm(launch) +#: Ordering used to fold many test results into one certification result. +#: A later test must never mask a worse earlier one. "skip" is included for +#: forward compatibility — StepResult already has that state, and a test-level +#: skip belongs between pass and fail. +_RESULT_SEVERITY = {"pass": 0, "skip": 1, "fail": 2, "error": 3} + + +def _worse_result(current: str, candidate: str) -> str: + """Return whichever of the two results is more severe.""" + if _RESULT_SEVERITY.get(candidate, 0) > _RESULT_SEVERITY.get(current, 0): + return candidate + return current + + def execute_certification( cert: CertificationDefinition, guest_path: Path, @@ -533,8 +547,7 @@ def execute_certification( environment=environment, ) test_results.append(tr) - if tr.result != "pass": - overall = tr.result + overall = _worse_result(overall, tr.result) _flush("") icon = _RESULT_LABEL.get(overall, "????") From dccad7e3241099bdae3294cc179df973e5c0fff2 Mon Sep 17 00:00:00 2001 From: Mark Gentry Date: Thu, 13 Aug 2026 11:15:09 -0500 Subject: [PATCH 4/5] fix: stop waiting for a guest whose QEMU has already exited MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vm_launch checks whether QEMU is still alive exactly once, at wait_ready_seconds, and never again. wait_for_guest then polls vsock until vsock_boot_timeout with no reference to the process at all. If QEMU exits after that single check, the harness spends the rest of the boot timeout pinging a VM that no longer exists. Measured on an EPYC 9654 with a deliberately corrupted ID block measurement, sampling the process at 1 Hz: +14.6s .. +17.2s QEMU alive +17.2s .. gone step ran +13.8s .. +195.9s QEMU lived 2.6s; the step took 182.1s. ~179s was spent waiting on a dead process. Policy violations are rejected at SNP_LAUNCH_START before guest memory is loaded, so QEMU dies inside the initial check and those launches already fail in ~2s — which is why only the measurement case was slow. Pass the process handle into wait_for_guest and abort as soon as it exits. The step drops from 182.1s to ~3s and the id-block suite from ~200s to ~21s. The diagnostics matter more than the speed. Firmware had already reported the exact cause: SNP_LAUNCH_FINISH ret=-5 fw_error=11 'Bad measurement' and the harness discarded it in favour of "Vsock agent on CID ... not ready after 180.0s". A negative test asserting exit_code:1 is satisfied by that timeout just as well as by a real rejection, so it would pass identically if the guest merely hung or if the ID block were ignored entirely. The failure message now carries QEMU's exit code and its stderr tail, so a rejected launch states the firmware's reason. process defaults to None, preserving the old behavior for any caller that does not supply it. --- sev_verify/guest_vsock.py | 22 +++++++++++++++++++++- sev_verify/vm_profile.py | 12 +++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/sev_verify/guest_vsock.py b/sev_verify/guest_vsock.py index 6604a95d..9fff3b70 100644 --- a/sev_verify/guest_vsock.py +++ b/sev_verify/guest_vsock.py @@ -14,6 +14,7 @@ import re import shlex import socket +import subprocess import time from pathlib import Path from typing import TYPE_CHECKING, Any @@ -142,9 +143,15 @@ def wait_for_guest( *, timeout: float | None = None, poll_interval: float = 2.0, + process: subprocess.Popen | None = None, ) -> None: """ Block until the guest vsock agent responds or timeout is reached. + + Pass ``process`` (the QEMU handle) to stop waiting the moment QEMU exits. + Without it, a guest that never starts — or one the firmware refuses to + launch — costs the full boot timeout and is then reported as an + indistinguishable "agent not ready", discarding the reason QEMU gave. """ boot_timeout = timeout if timeout is not None else profile.vsock_boot_timeout deadline = time.monotonic() + boot_timeout @@ -156,6 +163,11 @@ def wait_for_guest( return except GuestVsockError as exc: last_error = str(exc) + if process is not None and process.poll() is not None: + raise GuestVsockError( + f"QEMU exited with code {process.returncode} before the " + f"guest became ready" + ) from exc time.sleep(poll_interval) raise GuestVsockError( @@ -169,12 +181,20 @@ def check_guest_ready( *, timeout: float | None = None, poll_interval: float = 2.0, + process: subprocess.Popen | None = None, ) -> tuple[bool, str]: """ Return whether the guest vsock agent responds. + + ``process`` is forwarded to :func:`wait_for_guest`; see its docstring. """ try: - wait_for_guest(profile, timeout=timeout, poll_interval=poll_interval) + wait_for_guest( + profile, + timeout=timeout, + poll_interval=poll_interval, + process=process, + ) except GuestVsockError as exc: return False, str(exc) diff --git a/sev_verify/vm_profile.py b/sev_verify/vm_profile.py index ed500ac5..d042efb3 100644 --- a/sev_verify/vm_profile.py +++ b/sev_verify/vm_profile.py @@ -228,10 +228,20 @@ def vm_launch( # Import here to avoid circular import with guest_vsock. from .guest_vsock import check_guest_ready - booted, boot_error = check_guest_ready(self) + # Passing the process lets the wait abort as soon as QEMU dies, + # instead of polling vsock until the boot timeout expires against + # a VM that no longer exists. + booted, boot_error = check_guest_ready(self, process=process) if not booted: ok = False message = f"Guest did not boot: {boot_error}" + if process.poll() is not None: + # QEMU's own diagnosis is far more useful than "agent not + # ready" — for a rejected launch it names the firmware + # error, e.g. "SNP_LAUNCH_FINISH ... 'Bad measurement'". + stderr_tail = _read_guest_errors(self.guest_error_log).strip() + if stderr_tail: + message = f"{message}\nQEMU stderr:\n{stderr_tail}" elif message == "VM launch verified": message = "VM launched and guest booted" From 732ae03cbc170b4efd0141d6c6f8ff5c6f66d575 Mon Sep 17 00:00:00 2001 From: markg-github Date: Fri, 14 Aug 2026 13:33:21 -0500 Subject: [PATCH 5/5] fix: make error masking fix more robust MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> fix: parameterize the QEMU process type annotation guest_vsock used a bare subprocess.Popen while VMLaunchResult.process is annotated subprocess.Popen[bytes]. Bare Popen reads as Popen[Any] to a type checker, losing the stream element type for no benefit — only .poll() and .returncode are used. Match the existing annotation. refactor: split the severity comparison into named locals The single-expression comparison ran to 107 characters and read poorly, with _RESULT_SEVERITY.get(..., unknown_severity) appearing twice on one line. Name both severities and compare them. Behavior is identical: an unrecognised state still ranks above every known one, so a typo surfaces loudly rather than masking a real failure. That rationale moves from a comment into the docstring, where it is visible to callers. --- sev_verify/cli.py | 13 +++++++++---- sev_verify/guest_vsock.py | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/sev_verify/cli.py b/sev_verify/cli.py index 93c5dde3..81db41d5 100644 --- a/sev_verify/cli.py +++ b/sev_verify/cli.py @@ -504,10 +504,15 @@ def execute_test( def _worse_result(current: str, candidate: str) -> str: - """Return whichever of the two results is more severe.""" - if _RESULT_SEVERITY.get(candidate, 0) > _RESULT_SEVERITY.get(current, 0): - return candidate - return current + """Return whichever of the two results is more severe. + + An unrecognised state ranks above every known one, so a typo surfaces + loudly rather than silently masking a real failure. + """ + unknown = max(_RESULT_SEVERITY.values()) + 1 + current_severity = _RESULT_SEVERITY.get(current, unknown) + candidate_severity = _RESULT_SEVERITY.get(candidate, unknown) + return candidate if candidate_severity > current_severity else current def execute_certification( diff --git a/sev_verify/guest_vsock.py b/sev_verify/guest_vsock.py index 9fff3b70..c8cf6464 100644 --- a/sev_verify/guest_vsock.py +++ b/sev_verify/guest_vsock.py @@ -143,7 +143,7 @@ def wait_for_guest( *, timeout: float | None = None, poll_interval: float = 2.0, - process: subprocess.Popen | None = None, + process: subprocess.Popen[bytes] | None = None, ) -> None: """ Block until the guest vsock agent responds or timeout is reached. @@ -181,7 +181,7 @@ def check_guest_ready( *, timeout: float | None = None, poll_interval: float = 2.0, - process: subprocess.Popen | None = None, + process: subprocess.Popen[bytes] | None = None, ) -> tuple[bool, str]: """ Return whether the guest vsock agent responds.