From cdb3afd1f30a8cf0fa7706b0b53b89817c8f009b Mon Sep 17 00:00:00 2001 From: Flavien Solt Date: Fri, 8 May 2026 23:58:44 +0800 Subject: [PATCH] idma_channel_coupler: don't consume coupled-AW credit when popping a decoupled head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The credit counter `aw_to_send_q` in `proc_credit_cnt` tracks coupled AWs whose `first` R beat has arrived but that have not yet been handshaken on the AW master channel. The default `aw_sent = aw_decoupled_head & aw_valid` correctly pops a decoupled AW from the head without touching the counter, but the surrounding if/else-if chain does not check whether the head is decoupled. This leads to two related bugs: 1. The third else-if (`aw_ready_decoupled & !first & aw_to_send_q != 0`) decrements the counter and asserts `aw_sent` whenever the downstream is ready and a credit is pending — even when the AW currently being popped is a decoupled head. The credit (which was earned by a *coupled* AW further back in the FIFO) is consumed by the decoupled pop, so when the coupled AW reaches the head later it has neither a credit nor an arriving `first` and the AW master channel deadlocks. 2. The first if-branch (`aw_ready_decoupled & first`) consumes a `first` even when the head is decoupled. The `first` belonged to a future coupled AW; it is silently lost. Reproducer (seclang `examples/idma/i003_verilator/`): 1. Push AW0 (decoupled). FIFO=[AW0(dec)] ready=0 2. first arrives for upcoming AW1. ready=0, credit++ -> 1 3. Push AW1 (coupled). FIFO=[AW0(dec), AW1(coup)] ready=0 4. Release ready=1. Pre-fix: cycle 0 handshakes AW0 and decrements credit to 0. cycle 1+ AW1 head, no credit, no `first` -> deadlock. Post-fix: cycle 0 handshakes AW0 (default path), credit stays at 1. cycle 1 handshakes AW1, credit drains to 0. Fix: gate the first-consuming branch and the credit-decrement branch on `~aw_decoupled_head`, and rephrase the increment branch to fire on any unhandled `first`. Decoupled AWs are popped only by the default `aw_sent = aw_decoupled_head & aw_valid` path and do not interact with the credit counter. A more comprehensive fix exists upstream on branch `decouple_rw_fix` (commit a7bbc83, "Fix channel_coupler to handle decoupled", 2025-08-21) which introduces a separate `aw_to_stall_q` counter; that branch also touches other parts of the legalizer and has not been merged. This patch is the minimal change that resolves the deadlock against `master` without the larger refactor. --- src/backend/idma_channel_coupler.sv | 30 +++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/backend/idma_channel_coupler.sv b/src/backend/idma_channel_coupler.sv index 1491716a..e901d56a 100644 --- a/src/backend/idma_channel_coupler.sv +++ b/src/backend/idma_channel_coupler.sv @@ -127,29 +127,39 @@ module idma_channel_coupler #( .ready_i ( aw_ready ) ); - // use a credit counter to keep track of AWs to send + // use a credit counter to keep track of coupled AWs to send. + // + // The credit `aw_to_send_q` represents *coupled* AWs whose `first` R + // beat has arrived but that have not yet been handshaken. Decoupled + // AWs do not earn a credit. Pop a decoupled AW that happens to sit at + // the head separately (via the default `aw_sent = aw_decoupled_head & + // aw_valid`) so it does not consume a coupled credit reserved for a + // coupled AW further back in the FIFO. always_comb begin : proc_credit_cnt // defaults aw_to_send_d = aw_to_send_q; - // if we bypass the logic + // bypass: a decoupled head can always be sent aw_sent = aw_decoupled_head & aw_valid; - // first is asserted and aw is ready -> just send AW out - // without changing the credit counter value - if (aw_ready_decoupled & first) begin + // First arrives and the *coupled* head is ready -> send it + // without changing the credit counter. + if (aw_ready_decoupled & first & ~aw_decoupled_head) begin aw_sent = 1'b1; end - // if first is asserted and aw is not ready -> increment - // credit counter - else if (!aw_ready_decoupled & first) begin + // First arrives but we cannot send the corresponding coupled AW + // *this cycle* (either downstream isn't ready, or the FIFO head is + // currently a decoupled AW that has not yet been popped). Save the + // first as a credit for the future coupled AW. + else if (first) begin aw_to_send_d = aw_to_send_q + 1; end - // if not first, aw is ready and we have credit -> count down - else if (aw_ready_decoupled & !first & aw_to_send_q != '0) begin + // No new first this cycle, downstream ready, head is coupled, + // credit available -> drain one credit. + else if (aw_ready_decoupled & ~aw_decoupled_head & aw_to_send_q != '0) begin aw_sent = 1'b1; aw_to_send_d = aw_to_send_q - 1; end