From 3abd1e06b92bf10a4bdceb356aee75c0e9d24bf3 Mon Sep 17 00:00:00 2001 From: Flavien Solt Date: Fri, 8 May 2026 22:00:33 +0800 Subject: [PATCH] idma_error_handler: gate WAIT_LAST_W transitions on eh_valid_i The error-handler FSM has two structurally identical wait states. In WAIT, the decision logic is correctly gated on eh_valid_i: WAIT : begin if (eh_valid_i) begin if (eh_i == idma_pkg::CONTINUE) ... if (eh_i == idma_pkg::ABORT) ... end end WAIT_LAST_W is missing this guard: WAIT_LAST_W : begin if (eh_i == idma_pkg::CONTINUE) ... if (eh_i == idma_pkg::ABORT) ... end idma_eh_req_t is a 1-bit signal whose enum (idma_pkg.sv:27) is {CONTINUE = 0, ABORT = 1}. With eh_valid_i deasserted (the typical idle bus condition), eh_i defaults to 0 = CONTINUE, so the FSM auto-CONTINUEs the cycle it enters WAIT_LAST_W and never waits for the user's actual answer. The user has no way to ABORT a transfer after a last-burst write error: by the time they decode the error, the FSM has already taken the CONTINUE branch. Reproducer (Verilator 5.046, MetaFifoDepth=4): release reset, drive one write-error response with w_last_burst_i=1 (which IDLE catches and routes to WAIT_LAST_W), keep eh_valid_i=0, observe the FSM. Without the fix the FSM goes IDLE -> WAIT_LAST_W -> EMIT_EXTRA_RSP -> IDLE in three cycles regardless of the user; with the fix it stays in WAIT_LAST_W indefinitely until eh_valid_i is asserted. Wrap the WAIT_LAST_W body in `if (eh_valid_i)`, mirroring WAIT. --- src/backend/idma_error_handler.sv | 47 ++++++++++++++++--------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/backend/idma_error_handler.sv b/src/backend/idma_error_handler.sv index ad9abcce..ccd93252 100644 --- a/src/backend/idma_error_handler.sv +++ b/src/backend/idma_error_handler.sv @@ -311,29 +311,32 @@ module idma_error_handler #( // error happened on the last write burst of a 1D transfer. We need to emit an extra // response post error handling. WAIT_LAST_W : begin - // continue case (~error reporting) - if (eh_i == idma_pkg::CONTINUE) begin - eh_ready_o = 1'b1; - state_d = EMIT_EXTRA_RSP; - end - // abort - if (eh_i == idma_pkg::ABORT) begin - // in the case we have multiple outstanding 1D transfers in the datapath: - // - the transfers are small no flush required - // - some transfers might complete properly so no flush allowed! - // in this case just continue - if (num_outst_q > 'd1) begin - eh_ready_o = 1'b1; - state_d = EMIT_EXTRA_RSP; - // we are aborting a long transfer (it is still in the legalizer and - // therefore the only active transfer in the datapath) - end else if (num_outst_q == 'd1) begin + // answer arrives + if (eh_valid_i) begin + // continue case (~error reporting) + if (eh_i == idma_pkg::CONTINUE) begin eh_ready_o = 1'b1; - state_d = LEG_FLUSH; - // the counter is 0 -> no transfer in the datapath. This is an impossible - // state - end else begin - `ASSERT_I(inactive_tf_wait_last_w, rst_ni !== 1'b1) + state_d = EMIT_EXTRA_RSP; + end + // abort + if (eh_i == idma_pkg::ABORT) begin + // in the case we have multiple outstanding 1D transfers in the datapath: + // - the transfers are small no flush required + // - some transfers might complete properly so no flush allowed! + // in this case just continue + if (num_outst_q > 'd1) begin + eh_ready_o = 1'b1; + state_d = EMIT_EXTRA_RSP; + // we are aborting a long transfer (it is still in the legalizer and + // therefore the only active transfer in the datapath) + end else if (num_outst_q == 'd1) begin + eh_ready_o = 1'b1; + state_d = LEG_FLUSH; + // the counter is 0 -> no transfer in the datapath. This is an impossible + // state + end else begin + `ASSERT_I(inactive_tf_wait_last_w, rst_ni !== 1'b1) + end end end end