cmd_interface: Distinguish poll-again from stop-polling for the initiator

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>

Author: chrysh

pldm-interface/src/cmd_interface.rs

@@ -28,6 +28,19 @@ use pldm_common::util::mctp_transport::{
 
 pub type PldmCompletionErrorCode = u8;
 
+/// What the initiator polling loop should do next, as reported by
+/// [`CmdInterface::generate_initiator_request`].
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum InitiatorAction {
+    /// A request was generated: transmit `msg_buf[..n]`.
+    Request(usize),
+    /// Nothing due — a request is in flight or the current operation is
+    /// still running. Poll again.
+    Waiting,
+    /// The FD left initiator mode; stop polling.
+    Complete,
+}
+
 // Helper function to write a failure response message into payload
 pub(crate) fn generate_failure_response(
     payload: &mut [u8],
@@ -86,17 +99,31 @@ impl<'a, O: FdOps> CmdInterface<'a, O> {
     /// Sets the MCTP message-type byte at `msg_buf[0]` and writes the PLDM
     /// request starting at `msg_buf[1]`.
     ///
-    /// Returns `Ok(None)` when no request is pending (nothing to send).
-    /// Returns `Ok(Some(n))` where `n` is the total number of bytes written
-    /// to `msg_buf` (1 MCTP header byte + `n - 1` PLDM bytes) when a request
-    /// was generated.
+    /// Returns [`InitiatorAction::Request`]`(n)` when a request was
+    /// generated; transmit `msg_buf[..n]` (1 MCTP header byte plus the
+    /// encoded PLDM request). [`InitiatorAction::Waiting`] means nothing is
+    /// due yet — poll again. [`InitiatorAction::Complete`] means the FD
+    /// left initiator mode — stop polling.
+    ///
+    /// # Errors
+    ///
+    /// `T1Timeout`: no UA response arrived within T1 — the update was
+    /// cancelled and the FD is back in Idle. A protocol outcome to report,
+    /// not a transport fault to retry; the next poll returns `Complete`.
     pub fn generate_initiator_request(
         &mut self,
         msg_buf: &mut [u8],
-    ) -> Result<Option<usize>, MsgHandlerError> {
+    ) -> Result<InitiatorAction, MsgHandlerError> {
+        if self.fd_ctx.should_stop_initiator_mode() {
+            return Ok(InitiatorAction::Complete);
+        }
         let payload = construct_mctp_pldm_msg(msg_buf).map_err(MsgHandlerError::Util)?;
         let pldm_len = self.fd_ctx.fd_progress(payload)?;
-        Ok((pldm_len > 0).then_some(pldm_len))
+        Ok(if pldm_len > 0 {
+            InitiatorAction::Request(pldm_len + PLDM_MSG_OFFSET)
+        } else {
+            InitiatorAction::Waiting
+        })
     }
 
     /// Process a received FD-initiated PLDM response from `msg_buf`.

pldm-interface/src/firmware_device/fd_context.rs

@@ -868,7 +868,9 @@ impl<'a, O: FdOps> FirmwareDeviceContext<'a, O> {
 
     fn fd_progress_download(&mut self, payload: &mut [u8]) -> Result<usize, MsgHandlerError> {
         if !self.should_send_fd_request() {
-            return Err(MsgHandlerError::FdInitiatorModeError);
+            // A request is in flight (or nothing is due yet): not an
+            // error, the caller polls again.
+            return Ok(0);
         }
 
         let instance_id = self.internal.alloc_next_instance_id().unwrap();
@@ -941,7 +943,9 @@ impl<'a, O: FdOps> FirmwareDeviceContext<'a, O> {
 
     fn pldm_fd_progress_verify(&mut self, _payload: &mut [u8]) -> Result<usize, MsgHandlerError> {
         if !self.should_send_fd_request() {
-            return Err(MsgHandlerError::FdInitiatorModeError);
+            // A request is in flight (or nothing is due yet): not an
+            // error, the caller polls again.
+            return Ok(0);
         }
 
         let mut res = VerifyResult::default();
@@ -985,7 +989,9 @@ impl<'a, O: FdOps> FirmwareDeviceContext<'a, O> {
 
     fn pldm_fd_progress_apply(&mut self, _payload: &mut [u8]) -> Result<usize, MsgHandlerError> {
         if !self.should_send_fd_request() {
-            return Err(MsgHandlerError::FdInitiatorModeError);
+            // A request is in flight (or nothing is due yet): not an
+            // error, the caller polls again.
+            return Ok(0);
         }
 
         let mut res = ApplyResult::default();
@@ -1596,4 +1602,56 @@ mod tests {
             Some(GetStatusReasonCode::DownloadTimeout)
         );
     }
+
+    // A request in flight with T2 not yet elapsed is the normal polling
+    // state: Ok(0), not FdInitiatorModeError.
+    #[test]
+    fn test_fd_progress_waiting_is_not_an_error() {
+        let mut fd_ctx = new_test_fd_ctx();
+        let mut buffer = [0u8; 256];
+        let now = fd_ctx.ops.now();
+
+        fd_ctx.internal.set_fd_state(FirmwareDeviceState::Download);
+        fd_ctx.internal.set_fd_req(
+            FdReqState::Sent,
+            true,
+            Some(TransferResult::TransferSuccess as u8),
+            Some(0),
+            Some(FwUpdateCmd::TransferComplete as u8),
+            Some(now),
+        );
+        fd_ctx.internal.set_fd_t1_update_ts(now);
+
+        assert!(matches!(fd_ctx.fd_progress(&mut buffer), Ok(0)));
+        assert_eq!(
+            fd_ctx.internal.get_fd_state(),
+            FirmwareDeviceState::Download
+        );
+    }
+
+    // T1 must fire in the waiting state too, not only on the T2 resend
+    // path: request in flight, T2 not elapsed, but the last UA response
+    // is older than T1.
+    #[test]
+    fn test_fd_progress_t1_fires_while_waiting() {
+        let mut fd_ctx = new_test_fd_ctx();
+        let mut buffer = [0u8; 256];
+        let now = fd_ctx.ops.now();
+
+        fd_ctx.internal.set_fd_state(FirmwareDeviceState::Download);
+        fd_ctx.internal.set_fd_req(
+            FdReqState::Sent,
+            true,
+            Some(TransferResult::TransferSuccess as u8),
+            Some(0),
+            Some(FwUpdateCmd::TransferComplete as u8),
+            Some(now),
+        );
+        fd_ctx.internal.set_fd_t1_update_ts(0);
+
+        let result = fd_ctx.fd_progress(&mut buffer);
+
+        assert!(matches!(result, Err(MsgHandlerError::T1Timeout)));
+        assert_eq!(fd_ctx.internal.get_fd_state(), FirmwareDeviceState::Idle);
+    }
 }