Distinguish poll-again from stop-polling in the initiator contract - #15
Open
chrysh wants to merge 2 commits into
Open
Distinguish poll-again from stop-polling in the initiator contract#15chrysh wants to merge 2 commits into
chrysh wants to merge 2 commits into
Conversation
…ator The polling caller could not implement a correct loop: a request in flight with T2 not yet elapsed surfaced as FdInitiatorModeError, the same error as calling in a non-initiator state, so waiting, done and misuse were indistinguishable. caliptra-mcu-sw grew ResponderAction and a cancellation flag for the responder side of the same problem; this puts the outcome in the return type instead. generate_initiator_request now returns InitiatorAction: - Request(n): transmit msg_buf[..n] (includes the MCTP type byte) - Waiting: request in flight or operation still running, poll again - Complete: the FD left initiator mode, stop polling The waiting case inside fd_progress becomes Ok(0) instead of an error, which also makes the T1 check reachable while waiting - before this, a silent UA could only be timed out on the T2 resend path. Errors are now real faults only, plus T1Timeout from the cancel path. Signed-off-by: Christina Quast <christina.quast@9elements.com>
The message-type byte is stamped before it is known whether a request is due, so Waiting still mutates the buffer and a too-short buffer errors on every poll. A caller reusing one TX/RX buffer and treating Waiting as buffer-untouched would read stale bytes as a frame. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
leongross
approved these changes
Aug 21, 2026
Member
|
I can't find usage of |
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
2 participants
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.
Concrete proposal for the poll-contract discussion from the #12 review — code to argue about instead of an issue thread. Stacked on #14 (retarget/rebase as things merge); only the last commit is new here.
The problem: the polling caller can't implement a correct loop. A request in flight with T2 not yet elapsed — the most common polling state — surfaces as
Err(FdInitiatorModeError), the same error as calling in a non-initiator state. Waiting, done, and misuse are indistinguishable. caliptra-mcu-sw grewResponderAction::Continue/Completeplus a cancellation flag for the responder side of the same problem; their T1 path sets neither, so outcomes keep leaking into side-channels.The proposal: put the outcome in the return type.
generate_initiator_requestreturnsInitiatorAction:Request(n)— transmitmsg_buf[..n](1 MCTP type byte + encoded PLDM request)Waiting— request in flight or operation still running; poll againComplete— the FD left initiator mode; stop pollingErris reserved for real faults, plusT1Timeoutfrom the cancel path (#12) — after which the next poll returnsComplete.Side effect worth its own look: turning the waiting case into
Ok(0)insidefd_progressmakes the T1 check reachable while waiting — previously the error short-circuited past it, so a silent UA could only be timed out on the T2 resend path. There's a test pinning that (test_fd_progress_t1_fires_while_waiting).Open questions I'd like opinions on:
FdReqState::Failedmap toWaiting(current behavior viashould_send_fd_request) or deserve its own terminal signal?Completethe right name, given it also covers "update was cancelled and FD is idle again"?Relates to #13.