From ff78d6507cf7c34b7a5e75a84f1cf9b085322ea3 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Tue, 16 Jun 2026 17:52:00 +0930 Subject: [PATCH] pldm-fw: fd: Perform state transitions when processing UA responses Implementing ReqChannel in terms of an AsyncReqChannel implementation breaks the pldm-fw fd::Responder state machine after request channel reuse behaviour was changed in 4d148c9636b2 ("mctp-estack: Clarify RouterAsyncReqChannel reuse"). Since, the FD Responder may be starved for events: In some circumstances the state machine lacks the trigger for advancement without successive calls to progress(). However in other circumstances, with successive calls to progress(), the FD may issue concurrent requests to the UA, at which point the AsyncReqChannel will drop responses due to a tag mismatch. Further, immediate transitions to states where the ReqChannel becomes unavailable through the responder's pending_reply_ep() API eliminates the event source for triggering necessary calls to progress(). Migrate relevant FD state transitions to Responder::reply_in() so that all replies are correctly delivered to the responder and the responder correctly advances its state under progress(). Signed-off-by: Andrew Jeffery --- pldm-fw/src/fd.rs | 101 ++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/pldm-fw/src/fd.rs b/pldm-fw/src/fd.rs index bfc480a..ab6627a 100644 --- a/pldm-fw/src/fd.rs +++ b/pldm-fw/src/fd.rs @@ -272,14 +272,61 @@ impl Responder { match Cmd::from_u8(rsp.cmd) { Some(Cmd::RequestFirmwareData) => self.download_response(rsp, d), + Some(Cmd::TransferComplete) => { + if matches!(&self.state, State::Download { .. }) { + self.set_state_with(|prev| { + let State::Download { + details, req_comm, .. + } = prev + else { + unreachable!() + }; + State::Verify { + verify_result: None, + request: FDReq::Ready, + details, + req_comm, + req_pending: false, + } + }); + } else { + warn!( + "TransferComplete response but not in Download state" + ); + } + Ok(()) + } - // Ignore replies to these requests. - // We may have already moved on to a later state - // and don't have any useful retry for them. - Some(Cmd::TransferComplete) - | Some(Cmd::VerifyComplete) - | Some(Cmd::ApplyComplete) => Ok(()), - + Some(Cmd::VerifyComplete) => { + if matches!(&self.state, State::Verify { .. }) { + self.set_state_with(|prev| { + let State::Verify { + details, req_comm, .. + } = prev + else { + unreachable!() + }; + State::Apply { + apply_result: None, + request: FDReq::Ready, + details, + req_comm, + req_pending: false, + } + }); + } else { + warn!("VerifyComplete response but not in Verify state"); + } + Ok(()) + } + Some(Cmd::ApplyComplete) => { + if matches!(&self.state, State::Apply { .. }) { + self.set_state(State::ReadyXfer); + } else { + warn!("ApplyComplete response but not in Apply state"); + } + Ok(()) + } _ => Err(proto_error!("Unsupported PLDM response")), } } @@ -808,23 +855,7 @@ impl Responder { if let Some(tr) = transfer_result { // transfer complete sent - if *tr == TransferResult::Success { - self.set_state_with(|prev| { - let State::Download { - details, req_comm, .. - } = prev - else { - unreachable!() - }; - State::Verify { - verify_result: None, - request: FDReq::Ready, - details, - req_comm, - req_pending: false, - } - }); - } else { + if *tr != TransferResult::Success { // idle in Download state until the UA cancels *request = FDReq::Failed((*tr).into()) } @@ -880,23 +911,7 @@ impl Responder { } *req_pending = true; - if *vr == VerifyResult::Success { - self.set_state_with(|prev| { - let State::Verify { - details, req_comm, .. - } = prev - else { - unreachable!() - }; - State::Apply { - apply_result: None, - request: FDReq::Ready, - details, - req_comm, - req_pending: false, - } - }); - } else { + if *vr != VerifyResult::Success { // on verify failure remain in State::Verify, wait for cancel *request = FDReq::Failed(u8::from(*vr)) } @@ -951,9 +966,7 @@ impl Responder { } *req_pending = true; - if *ar == ApplyResult::Success { - self.set_state(State::ReadyXfer); - } else { + if *ar != ApplyResult::Success { // on failure remain in State::Apply, wait for cancel *request = FDReq::Failed(u8::from(*ar)); }