Repository navigation
Verify EC timer wake from CPU standby - #182
Merged
dymk merged 6 commits intoOct 16, 2026
Merged
Conversation
This was referenced Oct 1, 2026
Contributor
Author
|
Reduced this PR from 1,223 to 923 added lines and from 15 to 12 files. Removed 70 production lines, 192 test lines, and 38 documentation lines. The separate evidence, protocol, and GPIO-wrapper files are gone. Fixture construction and cleanup now share one lifecycle, and four named regressions replace the verbose predicate matrices. The new mocked launcher test file was removed. Re-ran all four real AC/DC connected/disconnected cases. Also exercised real quoted executable/ELF paths, a deliberately failing non-wake EC run, socket/process cleanup, and rejection of incomplete result summaries. EFI build and strict Clippy pass. The dependent source-switch cases also pass with this reduction. The remaining bulk is the interrupt/timer admission and restoration code needed to leave firmware state intact; those checks and real wake-cause validation are unchanged. |
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 4
Open (6)
EC_QEMUis currently expanded with${EC_QEMU-qemu-system-riscv32}, which does not fall back… · NewEC_QEMUis currently expanded with${EC_QEMU-qemu-system-riscv32}, which does not fall back… · Newrmdir "$sockets"can fail (e.g., if additional files are created in that temp dir), and failures… · NewResults, which eagerly executes all three cleanup calls before the loop… · NewEC_QEMUcan be an absolute path and therefore does not… · New
What changed in this PR
Adds an opt-in “TimeAlarm CPU-retention” end-to-end fixture that validates firmware CPU standby behavior (not OS suspend/resume), including new UEFI-side test logic, an execution script, and build/run targets; also updates EC QEMU orchestration to support a configurable QEMU binary and a wake socket.
Changes:
- Introduce a new
time-alarm-retentionRust e2e test (UEFI app) with platform interrupt/timer validation helpers and unit tests for core validation logic. - Add a retention runner script plus
make retention-*targets to build/run the fixture with AC/DC and connected/disconnected cases. - Update EC-QEMU launcher to honor
EC_QEMUand optionally exposeEC_WAKE_SOCK; bump related submodules and document the new workflow.
| File | Description |
|---|---|
| scripts/test-time-alarm-retention.sh | New runner script wiring artifacts, sockets, and QEMU args for retention runs. |
| scripts/lib/ec-qemu.sh | Allow overriding EC QEMU binary (EC_QEMU) and pass optional wake socket chardev. |
| mod/uefi/patina-qemu | Submodule bump to a Patina/QEMU revision needed for the fixture. |
| mod/ec | Submodule bump for wake-capable EC support. |
| e2e-tests/tests/time-alarm-retention/src/platform.rs | New low-level fixture to claim/route IRQs, enter standby, and restore device/IRQ state. |
| e2e-tests/tests/time-alarm-retention/src/main.rs | New retention test program coordinating EC TimeAlarm commands with standby observations. |
| e2e-tests/tests/time-alarm-retention/src/interrupt.rs | New shared constants/validation logic with unit tests. |
| e2e-tests/tests/time-alarm-retention/Cargo.toml | New crate manifest for the retention test. |
| e2e-tests/Makefile | New retention-* targets and options for building/running the fixture. |
| e2e-tests/Cargo.toml | Add the new retention test crate to the workspace. |
| docs/src/development/e2e-tests.md | Document how to build/run the opt-in retention fixture and its prerequisites. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Add an opt-in UEFI retention fixture that identifies GPIO1 wake versus the timer guard and restores firmware interrupt state. Exercise AC/DC selection, inactive and disconnected cases, and clear/rearm through the existing relay. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Real retention runs already perform the readiness checks. Remove the extra mode and target while retaining rejection of incomplete test results. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the real wake checks while consolidating interrupt state helpers, simplifying fixture lifetime management, and replacing duplicated test scaffolding with focused regressions. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Treat an empty optional EC_QEMU override as unset. Preserve an existing failure status during socket cleanup and report cleanup errors without deleting unexpected files. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the required QEMU argument and record each cleanup result before running the next step. Keep all cleanup attempts and preserve the first failure. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dymk
force-pushed
the
dymk/time-alarm-retention-uefi
branch
from
October 13, 2026 19:11
551d978 to
485d9b2
Compare
Identify PL061 and GIC registers, architectural control bits, and PSCI values by their documented names. Separate QEMU platform addresses and fixture timing policy without changing values or access order. Assisted-by: GitHub Copilot:gpt-6-astra Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
rogurr
approved these changes
Oct 16, 2026
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.
Add an opt-in UEFI test for EC-triggered CPU retention wake. The test programs the existing TimeAlarm relay, enters PSCI CPU standby, and checks the interrupt and GPIO state on return. It distinguishes GPIO1/INTID39 wake from the CNTV/INTID27 timeout guard, then clears the alarm and verifies interrupt-state restoration.
AC/DC sources are explicit test fixtures. This does not implement Windows sleep/resume, physical power detection, or asynchronous OS notifications. Part of OpenDevicePartnership/embedded-services#689.
Dependencies
The submodule commits are publicly fetchable through their upstream PRs but are not merged. The HAL fix is temporarily pinned to its approved personal fork. The retention target requires the new EC QEMU binary, selected with RETENTION_EC_QEMU; the default devcontainer image has not yet incorporated the builder prerequisite. Existing E2E targets are unchanged.
Results on the stacked commits
The low-level fixture includes explicit ownership, routing, guard, and restoration checks; it does not replace firmware exception vectors. Pinned Patina IRQ-query defects are bypassed with checked register snapshots. Leave this draft pending review and dependency approval.
Rebased onto the HID TimeAlarm integration from #184. The EC pin is 514fdbfeb43462000e28e490b2204429273fe8aa, retaining upstream HID/MCTP support plus the wake fixture. The new upstream QEMU image and RTC enumeration settings are preserved. All four AC/DC connected/disconnected retention runs passed with the updated host QEMU.