axi_isolate: fix deadlock during drain - #443
Draft
dkimTT wants to merge 3 commits into
Draft
Conversation
dkimTT
marked this pull request as draft
August 17, 2026 19:40
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
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.
Fix
Follow up to #441: Two changes, both under
TerminateTransaction = 1'b1,axi_isolate_inneris unchanged.Registered selects
The demux selects are driven by a registered
isolate_iinstead ofisolated_o. The register only updates on cycles where the channel has no request presented-and-unaccepted at a demux master port: once a request is presented,w_select_qis latched, so the select has to hold until the handshake completes. The same gate covers de-isolation: ifisolate_ifalls while a request sits unaccepted at the error slave, the select stays 1 until it is accepted; otherwise the AW would move to port 0 (inner module) while its W beats are already routed to the error slave.Inner sizing
The inner now gets a larger
NumPendingthan the demux:DemuxMaxPendingis the most transactions the demux's ID counters can have outstanding at once. With one more than that, the inner can never saturate and refuse a request the demux has already routed to port 0, so backpressure always happens at the demux's slave port, beforew_select_qis latched.Question: is the shared NumPending intentional?
The demux and inner module use
NumPendingdifferently: the demux only sizes its ID counters with it, so it admits up to2 * (2**idx_width(NumPending) - 1)transactions (possible 5 forNumPending = 4), while the inner treats it as an exact limit and cuts the AW channel atpending_aw_q >= NumPendingwhile staying inNormal, so the demux can route one more request to the inner than it will accept. Ifisolate_iis asserted in that state, the FSM entersDraindirectly (Holdis skipped, since the parked request was never forwarded downstream) and the drain runs with a request already committed at port 0 and unaccepted. Was the demux admitting more than the inner's limit intentional, and is deriving the inner's capacity from the demux ceiling the right fix, or would you rather bound the demux to exactlyNumPending?Verification
Ran a cocotb bench covering the scenario from #441, terminations during a loaded drain (W and ID-hash interlocks, W-before-AW ordering), select-hold windows on both channels in both isolate_i directions, saturation at the demux counter ceiling, ATOP AR-credit draining, and seeded random traffic. The demux's
slv_aw_select_stable/slv_ar_select_stableassertions were enabled in all runs, with no violations. Happy to contribute this bench if it's useful