pldm: Move the IPC server onto the shared service seam - #499
Merged
chrysh merged 3 commits intoSep 26, 2026
Merged
Conversation
chrysh
force-pushed
the
pldm-on-util-service
branch
from
September 25, 2026 13:34
54e1447 to
a8608b8
Compare
chrysh
force-pushed
the
pldm-on-util-service
branch
3 times, most recently
from
September 26, 2026 17:33
6c1d3ef to
6b407e6
Compare
Defines the binary protocol the orchestrator uses to talk to the PLDM Firmware Device over IPC. Twelve operations covering the full update lifecycle: offer accept/reject, verify/apply grant/deny, activation, SVN commit, cancel ack, and status query. Wire format: fixed 8-byte headers for both request and response, with manual byte encoding (no zerocopy dep) for no_std compatibility. The doc's response diagram adds to 6 bytes; the prose says "Fixed 8-byte header," so we pad 2 reserved bytes to match the stated intent. The doc field `gen` is renamed to `generation` because `gen` is a reserved keyword in Rust 2024 edition. All four Deny* ops carry a DenyReason byte for uniformity (the doc pins DenySvnCommit to PolicyViolation, but sending the reason on the wire keeps the decode path identical to the other denials). The FdStatus enum encodes the FD's current condition as a QueryStatus response payload, covering all DSP0267 states plus pending decisions. OfferPending carries target, total, transfer mode, and the SVN delayed flag from the UA's UpdateComponent request, so the orchestrator can validate against the floor. The Transport trait is the seam the client crate will be generic over, split into start/poll/cancel rather than one blocking round-trip: the orchestrator client runs in the event loop, which must not block. In production it is backed by util/ipc's AsyncTransaction; host tests use LoopbackTransport. Host-buildable with no kernel dependencies. All encode/decode paths have roundtrip tests. Assisted-by: Claude Code
The FD side of the IPC channel. dispatch() decodes the orchestrator's request header, matches on PldmOp, calls the corresponding FdHandler trait method, and encodes the response. FdHandler has one method per opcode so the implementation never touches wire bytes. LoopbackTransport implements pldm_ipc_api::Transport by calling dispatch directly in-process. Dispatch runs inside start(), so the response is ready on the first poll(); the split-phase shape is kept so client code written against a real channel runs unchanged here. The loopback tests show a multi-step sequence (accept offer, then query status shows ReadyXfer), proving the wire format, dispatch, and status encoding work together, plus the state rules: start while pending, poll with nothing in flight, cancel, and a short response buffer. Same pattern as services/i2c/server: dispatch generic over a trait, loopback transport in the server crate, host-buildable with no kernel dependencies. Assisted-by: Claude Code
Deletes the two local copies: ipc-api's Transport trait, which was character-identical to util_service::AsyncTransport, and ipc-server's LoopbackTransport, which was util_service::Loopback around dispatch. The api crate is now wire format only, and the transport that carries its frames is the one every IPC service shares. FdServer is the Dispatch impl. The newtype exists because Dispatch is a foreign trait and cannot be implemented for every F: FdHandler directly. dispatch() returns Result: a response buffer too small for even an error frame is the one case with nothing to send back, where it used to return a bare 0. The request length check is now per opcode. expected_request_len(op) returns the exact frame size for each operation, so a frame with trailing slack is rejected before dispatch. The match has no catch-all, so adding an opcode without choosing its length fails to compile. Assisted-by: Claude
chrysh
force-pushed
the
pldm-on-util-service
branch
from
September 26, 2026 17:38
6b407e6 to
caeabc6
Compare
chrysh
marked this pull request as ready for review
September 26, 2026 17:38
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
1 participant
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.
The first migration onto
util_service. Two local copies go away.ipc-api'sTransporttrait was character-identical toutil_service::AsyncTransport, down to the doc wording, so the file isdeleted and the api crate is wire format only.
ipc-server'sLoopbackTransportwasutil_service::Loopbackwrapped arounddispatch, so it goes too, and the tests that used it now driveLoopback<FdServer<StubFd>, MAX_RESPONSE_SIZE>instead.FdServeris theDispatchimpl. The newtype exists becauseDispatchis a foreign trait and cannot be implemented for every
F: FdHandlerdirectly.
dispatch()returnsResultnow: a response buffer too smallfor even an error frame is the one case with nothing to send back, where
it used to return a bare 0.
One behavior change worth reviewing on its own. The old loopback rejected
a request longer than
MAX_REQUEST_SIZE, and it turned out to be theonly thing doing so. The check is now per opcode:
expected_request_len(op)returns the exact frame size for each operation, with no catch-all, so
adding an opcode without choosing its length fails to compile. A frame
with trailing slack is rejected before dispatch.
26 tests in ipc-server, all passing: the existing dispatch tests plus
the ported loopback round-trips.
Not in here: i2c's migration, and the typed orchestrator-side client.
Review the last commit only; below it is #498.
Progress toward 9elements#13.