Complete the Thermal relay command family#85
Merged
dymk merged 8 commits intoJul 18, 2026
Conversation
Recognize ODP error responses before validating success command ids, reject response packets marked as requests, and keep TimeAlarm's public invalid-timestamp fallback unchanged. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Return the raw DeciKelvin value at FFA payload offset zero, reject trailing EC response bytes, and move Thermal's growing host tests behind a focused test-module boundary. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Forward canonical SetThrs and GetThrs bodies, use the AML-compatible u32 timeout request layout, return raw threshold values, and reject non-exact EC responses. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Forward the canonical cooling-policy request and preserve the EC's InvalidParameter discriminant as raw FFA status 1 while keeping local relay failures distinct as all ones. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Forward fixed-DWORD GetVar and SetVar requests with raw MPTF UUID bytes, return raw values/status, and reject non-DWORD FFA lengths without touching the EC channel. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Keep unit coverage focused on shared relay invariants, SetScp error mapping, variable length validation, and a small GetTmp smoke check. Rely on the two-QEMU suite for command-family correctness. Assisted-by: GitHub Copilot:gpt-5.6-sol Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the Thermal service shim to relay the full Thermal command family over the shared EC MCTP channel, while tightening relay response-envelope validation and aligning FF-A payload layouts with the expected AML-style ABI.
Changes:
- Extend
Thermalto relaySetThrs,GetThrs,SetScp,GetVar, andSetVar, and update FF-A request/response packing accordingly. - Strengthen
ec_relayto reject malformed ODP responses (responses marked as requests) and preserve EC remote error discriminants via a dedicatedEcRelayError::Remote(_). - Add focused unit tests for Thermal wire-format invariants and FF-A status/value prefix behavior; update TimeAlarm test to use the new EC-error header helper.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ec-service-lib/src/services/time_alarm.rs | Updates a unit test to construct synthetic EC error headers via the shared helper. |
| ec-service-lib/src/services/thermal.rs | Implements relaying for all Thermal commands, enforces exact EC response lengths, and updates FF-A ABI mapping. |
| ec-service-lib/src/services/thermal/tests.rs | Adds shared Thermal test harness helpers for framing/transport and FF-A request construction. |
| ec-service-lib/src/services/thermal/tests/get_tmp.rs | Adds Thermal GetTmp wire-format and exact-length response-body validation tests. |
| ec-service-lib/src/services/thermal/tests/set_scp.rs | Adds SetScp wire-format round-trip validation and remote-error discriminant propagation test. |
| ec-service-lib/src/services/thermal/tests/variables.rs | Adds FF-A rejection tests for non-canonical variable-length requests (no relay side effects). |
| ec-service-lib/src/services/ec_relay.rs | Improves ODP response validation and preserves EC remote error codes; adds test-only helper to build EC error headers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
philgweber
reviewed
Jul 16, 2026
philgweber
previously approved these changes
Jul 16, 2026
rogurr
previously approved these changes
Jul 16, 2026
Replace anonymous FFA payload offsets with typed little-endian request layouts shared by FFA decoding and EC request serialization. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
rogurr
previously approved these changes
Jul 17, 2026
parse_request indexed the payload via DirectMessagePayload::slice, which panics if a future request type's 1 + size_of::<T>() prefix runs past the 112-byte payload. Add a bounds-checked get mirroring slice::get and use it with ? before ref_from_bytes, so an oversized layout returns None. Assisted-by: GitHub Copilot:claude-opus-4.8 Copilot-Session: f36aff10-2137-4e4c-a1dd-d6ee0f6f982d
rogurr
approved these changes
Jul 17, 2026
williampMSFT
approved these changes
Jul 17, 2026
This was referenced Jul 18, 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
Verification
cargo test --locked -p ec-service-lib(106 passed)cargo clippy --locked -p ec-service-lib --tests -- -D warningsReview context
Closes #86