uefi/perf: Add QEMU Q35 boot harness

[kat-perez]

Adds uefi/perf/qemu/run-q35-boot.sh, which boots a Patina Q35 firmware pair under QEMU and reports whether it reaches BDS. This is the bringup step for measuring Patina boot behavior on a reproducible machine rather than on hardware.

Closes #118.

What it does

• Boots QEMUQ35_CODE.fd / QEMUQ35_VARS.fd with the machine wiring the Patina Q35 platform expects, copying the variable store so the firmware directory stays reusable.
• Watches the debug console for the BDS entry marker and stops QEMU as soon as it appears, since the firmware keeps running when no boot device is attached.
• Keeps boot-debugcon.log as the artifact to inspect on failure.

Exit codes

Code	Meaning
0	The BDS entry marker appeared on the debug console.
1	The firmware did not reach BDS: the timeout expired, or the guest reset or shut down first.
2	A setup problem: bad arguments, a missing file or tool, an output directory that cannot be prepared, or QEMU itself failing to run.

This keeps a firmware regression distinguishable from a broken invocation. When QEMU ends on its own, its exit status decides which of the two it was: a clean exit means the guest ended the run, so that is a boot failure, while a non-zero exit means QEMU itself would not run.

CI

uefi-perf-check.yml runs shellcheck and bash -n over every script under uefi/perf/, plus python3 -m py_compile on the Python helpers.

Nothing under uefi/perf/ was covered by any existing workflow: every path filter in the repository targets ec/, common/ or uefi/crates/, so these scripts had no checks at all. Booting the firmware for real needs QEMU and a firmware build, which is more than a hosted runner should take on, so that stays out of CI.

shellcheck is pinned by version and sha256 because it is not part of the hosted runner image.

Verification

Run against a locally built patina-qemu v4.0.5 firmware:

PASS: reached BDS
939:INFO - DXE Core Platform Binary v3.1.2
2769:[Bds] Entry...

Every exit path exercised against a stub QEMU:

reaches BDS                       -> 0
guest resets before BDS           -> 1
no marker before the timeout      -> 1
QEMU exits non-zero               -> 2
QEMU not installed                -> 2
bad argument / missing firmware   -> 2

Notes

• The README documents why PERF_TRACE_ENABLE must be set at build time. The platform PEI always publishes the Patina performance configuration HOB, and read_performance_config returns it whenever the HOB exists, so a present-but-disabled HOB outranks the DXE Core's default. Replacing only the DXE Core binary cannot turn measurement on.
• *.sh text eol=lf is added because this repository is cloned on Windows with core.autocrlf=true, which yields a CRLF shebang that neither Linux CI nor WSL will run.

[Copilot]

🟡 Changes recommended

The new harness has confirmed argument/timeout handling bugs that can cause unintended exit codes and confusing failures under common CLI misuse (missing or non-numeric flag values).

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

Pull request overview

Adds a reproducible QEMU Q35 boot bringup harness for Patina firmware, intended as the first step toward consistent performance/boot-behavior measurement in CI by detecting when the firmware reaches BDS via debug console output.

Changes:

• Introduces run-q35-boot.sh to boot the Patina Q35 firmware pair under QEMU, monitor the debug console for the BDS entry marker, and stop QEMU once reached.
• Adds a README documenting prerequisites, how to obtain/build the firmware, how to run the harness, and how to confirm performance tracing is enabled.
• Pins *.sh files to LF line endings via .gitattributes to avoid CRLF-shebang execution failures on Linux/WSL.

File summaries

File	Description
uefi/perf/qemu/run-q35-boot.sh	New QEMU Q35 boot harness that detects BDS via debugcon log and returns structured exit codes.
uefi/perf/qemu/README.md	Usage and firmware acquisition/build documentation for the new harness.
.gitattributes	Enforces LF EOL for shell scripts to keep shebangs runnable in Linux CI/WSL.

Review details

Suppressed comments (1)

uefi/perf/qemu/run-q35-boot.sh:126

• timeout_seconds is used in arithmetic expansion ($((SECONDS + timeout_seconds))). If --timeout is non-numeric (or empty due to a missing value), the script will exit due to set -e with a confusing error and exit code 1 instead of EXIT_USAGE (2). Consider validating --timeout before computing the deadline.

deadline=$((SECONDS + timeout_seconds))

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

The script’s documented exit-code contract isn’t met for common setup/invocation failures (e.g., missing QEMU or early QEMU exit), which will misclassify errors and hinder regression triage.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

uefi/perf/qemu/run-q35-boot.sh:123

• The PR description/README state that setup problems should exit with code 2, but if qemu-system-x86_64 is missing this script will currently exit with bash's command not found status (127) due to set -e. Add an explicit PATH check and use EXIT_USAGE so callers can reliably distinguish setup failures from firmware timeouts.

This issue also appears on line 155 of the same file.

uefi/perf/qemu/run-q35-boot.sh:158

• When QEMU exits early (e.g., unsupported flags/options, missing KVM/TCG features, etc.), the loop breaks but the script still reports a timeout-style failure and exits 1 later. That contradicts the documented exit code contract (2 for setup/invocation problems) and makes regressions harder to triage. Exit EXIT_USAGE immediately when QEMU is gone, while still pointing to the boot log artifact.

  if ! kill -0 "$qemu_pid" 2>/dev/null; then
    echo "QEMU exited before reaching BDS" >&2
    break
  fi

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The harness’ documented exit-code contract isn’t consistently enforced for setup/environment failures, which can misclassify harness problems as firmware timeouts/regressions.

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

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

uefi/perf/qemu/run-q35-boot.sh:121

• Several setup steps rely on set -e and will exit with the underlying command status (often 1) instead of the documented EXIT_USAGE=2. For example, a non-writable --out-dir, a failed mktemp, or a failed cp of the VARS image will currently be indistinguishable from a firmware timeout (exit 1). Consider explicitly mapping these setup failures to exit 2 with a clear error message.

This issue also appears in the following locations of the same file:

• line 127
• line 146

uefi/perf/qemu/run-q35-boot.sh:147

• If QEMU fails immediately due to an invocation/setup issue (unsupported flags, bad -global, etc.), the loop treats it as a firmware failure (exit 1). Since the PR description calls out exit 2 for setup problems, consider detecting an immediate QEMU exit with no debugcon output and returning EXIT_USAGE=2 in that case.

  -no-reboot &
qemu_pid=$!

uefi/perf/qemu/run-q35-boot.sh:131

• cp/chmod of the writable VARS image and creation of boot-debugcon.log can still fail with exit code 1 due to set -e, even though these are setup/environment problems (and the PR description says setup problems should be exit 2). Consider explicitly mapping these failures to EXIT_USAGE=2 so callers can reliably distinguish harness failures from firmware timeouts.

vars_fd="${out_dir}/QEMUQ35_VARS.writable.fd"
cp "$vars_fd_source" "$vars_fd"
chmod u+w "$vars_fd"

: > "$boot_log"

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The harness can misclassify an early QEMU exit as a firmware timeout/failure (exit 1) rather than a setup/invocation problem (exit 2), which undermines the stated exit-code contract for regression detection.

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

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[rogurr]

Do we need to state there are published builds?

It looks like to get the performance data, it is a compilation time change using the 'BLD_*_PERF_TRACE_ENABLE=TRUE' flag. So maybe just show them how to compile?

[kat-perez]

Dropped this part, since I later realized that with more testing your point was stronger than I assumed. This section now just shows the build with a link to the upstream building doc, with just a note about the BLD_*_PERF_TRACE_ENABLE=TRUE flag

[rogurr]

I'm a little confused by the title, is this for running a performance measurement?

[kat-perez]

Yeah, changed this to name the boot check and performance capture that happens in the script

[rogurr]

This looks a lot like it was AI generated since it tends to be kind of wordy on minute details not really relavent. Is there already a build README.md in the QEMU repo? If this tool is dependent on that build, maybe just say build using the instructions in [README.md] (http:// ) with the BLD switch?

And the next paragraph I think is good, but it appears AI targeted why we need to use the switch, not really what the new HOB is providing during boot.

[rogurr]

Is this only for the QEMU Q35 build from the ODP repository? Or can it work on other QEMU builds? What about Radxa? Or the other internal platform builds?

[kat-perez]

Yes, Q35 only for now

[Copilot]

🟡 Changes recommended

Several setup/QEMU-invocation failure modes can incorrectly return exit code 1 (firmware timeout/regression) instead of the documented/setup exit code 2, making failures ambiguous.

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

Review details

Suppressed comments (2)

uefi/perf/qemu/run-q35-boot.sh:137

• If QEMU fails immediately due to an invalid option / missing acceleration support / host incompatibility, the script will later report a firmware timeout (exit 1). Capture QEMU stderr and do a quick post-launch liveness check so a failed QEMU invocation is reported as EXIT_USAGE (2) with a usable artifact.

"$QEMU_COMMAND" \
  -debugcon "file:${boot_log}" \
  -global "isa-debugcon.iobase=${DEBUGCON_IO_PORT}" \
  -global ICH9-LPC.disable_s3=1 \
  -device "isa-debug-exit,iobase=${DEBUG_EXIT_IO_PORT},iosize=${DEBUG_EXIT_IO_SIZE}" \

uefi/perf/qemu/run-q35-boot.sh:168

• When QEMU exits before the BDS marker is seen, the script currently still returns 1 (interpreted as a firmware timeout/regression). That makes a broken QEMU invocation indistinguishable from a real boot failure. Treat early QEMU exit as a setup problem and return EXIT_USAGE (2).

  if ! kill -0 "$qemu_pid" 2>/dev/null; then
    echo "QEMU exited before reaching BDS" >&2
    break
  fi

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The option parsing still accepts a following flag as a “value” (e.g. --timeout --out-dir ...), which can shift past options and produce confusing failures instead of clearly rejecting misuse at the point of parsing.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

uefi/perf/qemu/run-q35-boot.sh:77

• require_value only checks for an empty "$2". If a user accidentally provides another flag next (e.g. --timeout --out-dir ...), the parser will treat --out-dir as the timeout value and shift past it, leading to confusing follow-on errors. Consider rejecting values that look like flags so the error is reported at the point of misuse.

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The harness’ documented exit-code contract and CI behavior claims don’t fully match the current implementation/docs, and QEMU termination can potentially hang without a bounded wait.

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

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

uefi/perf/qemu/run-q35-boot.sh:126

• Because the script runs with set -e, failures in mktemp/mkdir will currently propagate their native exit status (often 1) rather than the documented usage/setup exit code 2. This makes environment/setup failures indistinguishable from a firmware boot failure for callers relying on the exit codes.

This issue also appears on line 132 of the same file.
uefi/perf/qemu/run-q35-boot.sh:159

• stop_qemu calls wait unconditionally after sending SIGTERM. If QEMU fails to terminate (e.g., stuck in an uninterruptible state), wait can block indefinitely and hang the harness/CI job. Consider adding a bounded wait and escalating to SIGKILL so the script always makes forward progress.

uefi/perf/qemu/run-q35-boot.sh:136

• With set -e, failures writing artifacts (copying/chmod-ing the variable store or creating the boot log) will currently exit with the underlying command's status (often 1), not the documented setup/usage exit code 2. These are setup/environment errors and should consistently map to exit 2 so regressions can be distinguished from invocation problems.

vars_fd="${out_dir}/QEMUQ35_VARS.writable.fd"
cp "$vars_fd_source" "$vars_fd"
chmod u+w "$vars_fd"

: > "$boot_log"

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new harness script has a few verified exit-code and argument-handling edge cases that can yield confusing errors or the wrong status code relative to the documented contract.

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

Review details

Suppressed comments (1)

uefi/perf/qemu/run-q35-boot.sh:136

• Similarly, failures when copying/making the VARS image writable or creating boot-debugcon.log will currently exit with a generic non-2 status due to set -e. Wrapping these steps keeps setup failures consistently reported as exit 2 as documented.

cp "$vars_fd_source" "$vars_fd"
chmod u+w "$vars_fd"

: > "$boot_log"

• Files reviewed: 3/3 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The harness currently treats any early QEMU exit as a setup error (exit 2), which can misclassify a clean guest reboot/shutdown before BDS as an invocation problem rather than a firmware boot failure.

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

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are self-contained (new harness + documentation + line-ending rule) and the behavior/contract described in the PR matches the implementation.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new script can unconditionally kill a reused PID after QEMU exits, which can terminate an unrelated process on busy systems.

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

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The changes are self-contained, match the stated behavior/exit-code contract, and add appropriate documentation plus non-invasive CI validation.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟢 Approval recommended

The harness logic, exit-code contract, documentation, and CI checks are consistent and self-contained with no verified functional issues in the changed files.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The boot harness can misclassify late QEMU exits as a timeout, potentially returning the wrong exit code (firmware failure vs QEMU/setup failure).

Review details

Suppressed comments (1)

uefi/perf/qemu/run-q35-boot.sh:201

• If QEMU exits between the final in-loop kill -0 check and the loop terminating due to the timeout, qemu_exited stays false and the script reports a timeout (exit 1) even though QEMU actually ended. In particular, a late non-zero QEMU exit would be misclassified as a firmware timeout (exit 1) instead of a setup/QEMU failure (exit 2). Add a post-loop liveness check before stop_qemu to capture QEMU’s real exit status when the loop ends due to the deadline.

while [ "$SECONDS" -lt "$deadline" ]; do
  if grep -qF "$BDS_READY_MARKER" "$boot_log" 2>/dev/null; then
    reached_bds=true
    break
  fi
  # QEMU ending on its own is either the guest resetting or shutting down under
  # -no-reboot, which is a firmware failure, or QEMU refusing to run at all.
  # Its exit status tells the two apart: clean means the guest ended it.
  if ! kill -0 "$qemu_pid" 2>/dev/null; then
    qemu_exited=true
    qemu_reaped=true
    wait "$qemu_pid" 2>/dev/null || qemu_status=$?
    break
  fi
  sleep "$POLL_INTERVAL_SECONDS"
done

stop_qemu
trap - EXIT

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite