dm_csrs: fix dmstatus.stickyunavail per spec #520 (revert of closed PR#2)#3
Open
jahanzebkhalid-10Xe wants to merge 7 commits into
Open
dm_csrs: fix dmstatus.stickyunavail per spec #520 (revert of closed PR#2)#3jahanzebkhalid-10Xe wants to merge 7 commits into
jahanzebkhalid-10Xe wants to merge 7 commits into
Conversation
…ffsets Fixes pulp-platform#104 (riscv-dbg-vip). 17e912c ("Updated 1.0 debug module") moved dm_mem.sv's flag-write addresses to the v1.0 spec layout: GoingAddr: 0x108 -> 0x104 ResumingAddr: 0x110 -> 0x108 ExceptionAddr: 0x118 -> 0x10C but debug_rom.S was never updated to match, so the hart's debug-mode code kept writing "going" to the old 0x108 offset -- an address dm_mem.sv's FSM no longer watches. going never asserted, and the abstract-command FSM parked in Go forever (abstractcs.busy stuck at 1). Updated the GOING/RESUMING/EXCEPTION defines in debug_rom.S to match, and regenerated debug_rom.sv/debug_rom_one_scratch.sv (+ .h) via `make -C debug_rom` (riscv64-unknown-elf-gcc 15.2.0). Diff is minimal and exactly localized to the changed store-immediate encodings, nothing else. Verified against real Questa CVA6 sim (riscv-dbg-vip's pydebug suite): halt_uvm 8/8 (was stuck at step 04, "Read PC"), csr_access_uvm 4/4, gpr_write_uvm 4/4 -- all previously timed out on the first abstract command. One new, separate, unrelated finding surfaced now that the Program Buffer path is reachable for the first time (progbuf0/1 write/read-back returns 0) -- filed as riscv-dbg-vip#121, not touched here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes riscv-dbg-vip#117 (RTL side). stickyunavail_q/stickyunavail_d (added for spec 1.0 #520, just above) already correctly implement per-hart sticky-unavailable tracking -- set when a hart goes unavailable, held sticky, cleared by ackunavail. But the dmstatus.stickyunavail output field itself was hardcoded to 1'b1, completely bypassing that logic: permanently asserted regardless of whether any hart was ever actually unavailable. Wired to stickyunavail_q[selected_hart], the same pattern already used correctly for allunavail/anyunavail two lines below. Verified against real Questa (riscv-dbg-vip's pydebug suite, paired with that repo's dm_ref_model dut_version parameterization): CVA6 halt_uvm/ csr_access_uvm/gpr_write_uvm all MODEL_CHECK Mismatches=0 (were showing this bit's contribution to the mismatch before). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
stickyunavail (dmstatus bit 23, spec dm_registers.xml) is a global
mode/capability declaration ("this DM implements sticky unavail
semantics for allunavail/anyunavail"), not a per-hart status bit.
024efc2 mistakenly wired it to stickyunavail_q[selected_hart], which
would report "not sticky" for any hart that has never gone
unavailable even though the implementation always applies sticky
behavior (via unavailable_effective feeding allunavail/anyunavail).
Reverts to the hardcoded 1'b1, keeping the stickyunavail_q tracking
and unavailable_effective wiring from 024efc2 -- that part correctly
implements #520 sticky semantics for allunavail/anyunavail.
Per review on riscv-dbg-vip#117 / riscv-dbg PR#2 by tufailrizvi-debug.
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
2 participants
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.
Redo of #2 (closed after review) addressing tufailrizvi-debug's comment.
What #2 got wrong
#2 wired
dmstatus.stickyunavailtostickyunavail_q[selected_hart](per-hart).Per spec (
dm_registers.xml, dmstatus bit 23),stickyunavailis a globalcapability/mode bit -- it declares that this DM's
allunavail/anyunavailbehave sticky (value 1) vs. current-state-only (value 0). It is not itself a
per-hart status bit; that job belongs to
allunavail/anyunavail. Wiring itper-hart would falsely report "not sticky" for any hart that has never gone
unavailable, even though the implementation applies sticky semantics
uniformly.
This PR
Reverts
dmstatus.stickyunavailto the hardcoded1'b1(declaring stickysemantics are always active), while keeping the
stickyunavail_q/unavailable_effectivetracking machinery from #2 that correctly implements#520 for
allunavail/anyunavail-- that part of #2's change was right.Fixes riscv-dbg-vip#117 (RTL half; DV half already merged via
riscv-dbg-vip#125).
cc @tufailrizvi-debug