Relay TimeAlarm timer values - #87
Merged
dymk merged 4 commits intoJul 21, 2026
Merged
Conversation
Allow service shims to validate canonical success response discriminants that differ from their request IDs while preserving the existing equal-ID convenience method and remote-error ordering. Assisted-by: GitHub Copilot:gpt-5.6-sol Copilot-Session: d2609ceb-d6c1-4dab-a4bd-3e4c6eb98def
Add typed timer set/get requests, canonical response-discriminant validation, exact body parsing, and the existing AML raw scalar ABI. Assisted-by: GitHub Copilot:gpt-5.6-sol Copilot-Session: d2609ceb-d6c1-4dab-a4bd-3e4c6eb98def
Assisted-by: GitHub Copilot:gpt-5.6-sol Copilot-Session: d2609ceb-d6c1-4dab-a4bd-3e4c6eb98def
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the TimeAlarm EC relay service to support relaying SetTimerValue and GetTimerValue over the EC with explicit request/response discriminant validation, while keeping the existing AML-facing raw ABI (status/value as a 32-bit scalar) and adding focused protocol contract tests to prevent wire-format drift.
Changes:
- Add
SetTimerValue/GetTimerValuecommand handling inTimeAlarm, using typed zerocopy request layouts and exact response parsing. - Introduce
Relay::invoke_request_with_response_idto validate success responses against an explicit expected discriminant (while preserving remote-error precedence). - Add protocol-level tests that round-trip request/response bytes via the EC-side serializer/deserializer to lock the canonical wire contract.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| ec-service-lib/src/services/time_alarm.rs | Adds timer value relay support with typed request parsing, explicit response-discriminant validation, and contract-focused tests preserving the existing AML ABI. |
| ec-service-lib/src/services/ec_relay.rs | Factors out invoke_request_with_response_id to support commands whose success response discriminant differs from the request message id, plus targeted tests for discriminant validation and error precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Move the checked command-byte-skipping zerocopy parser into the services module so Thermal and TimeAlarm use one implementation and one pair of boundary tests. Assisted-by: GitHub Copilot:gpt-5.6-sol Copilot-Session: d2609ceb-d6c1-4dab-a4bd-3e4c6eb98def
philgweber
approved these changes
Jul 21, 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.
Summary
SetTimerValueandGetTimerValuethrough the EC6 -> 6,7 -> 5)Architecture
Relay::invoke_request_with_response_idkeeps remote-error handling ahead of success-discriminant validation while preserving the existing equal-ID helper. TimeAlarm uses typed zerocopy request layouts and exact empty/four-byte response parsing.Code-judo review
An independent simplification pass consolidated duplicate positive assurance into FFA-to-wire contract tests: 47 net test LOC removed, no production churn, with malformed-body, wrong-ID, remote-error, oversized-parser, and GetRealTime boundaries retained.
Validation
cargo test --locked -p ec-service-lib— 116 passedcargo clippy --locked -p ec-service-lib --tests -- -D warningscargo check --locked -p ec-service-lib --target aarch64-unknown-nonecargo check --locked -p ec-service-lib --target aarch64-unknown-none-softfloatDownstream
Dependent ArmVirt PR will track:
OpenDevicePartnership/odp-platform-qemu-arm-virt#127