Issue #1766: defer ITLB-miss walk until M-stage data access leaves M#1
Open
davidharrishmc wants to merge 48 commits into
Open
Issue #1766: defer ITLB-miss walk until M-stage data access leaves M#1davidharrishmc wants to merge 48 commits into
davidharrishmc wants to merge 48 commits into
Conversation
This reverts commit 3a73587.
… cache line size violates other cache geometry constraints and causes assertion failure
* ebuarbfsm cannot reach transition from ARBITRATE to ARBITRATE via both assertion. With only 2 managers is it not possible for this transaction. The fsm is designed to be generic and support future multilayer interconnects with more than 2 managers. Adding an exclusion rather than changing the RTL. * Simplified reset on genesys2 board. Added small ILA debugger. * Fixes performance issue with fence.i bug openhwgroup#1714. A trapped fence.i would still invalidate the icache cand flush the data cache. * Removed unused constraint from genesys2. * Updated comment reset in genesys2 fpga top for clarity. * Added back in ILA debugger to genesys2. * Fixed performance bug in cacheLRU. * Fixes issue openhwgroup#1170 with suggestion form Claud, THRE interrupt clearing was incorrect. In order to reproduce the bug, an old version of buildroot from May 2025 was required. The current version does not trigger the "UART slow down."
…roup#1766) A load that hits in the D$ can read the wrong value when its access coincides with an ITLB miss. Sequence (sv32, from the issue): - A wrong-path fetch (branch mispredict) raises an instruction page fault; the HPTW reaches FAULT and returns to IDLE, but the faulting fetch is not yet flushed, so its ITLB miss is still pending. - The lw advances into M and, after the issue openhwgroup#1538 TagSetStale re-read, HITS in the D$. Its read data is only valid combinationally while the load still owns the cache. - The load cannot retire because HPTWStall (asserted by the pending ITLB miss itself) holds the pipeline. The previous deferral guard (ITLBMissReady = ITLBMissOrUpdateAF & ~DCacheBusStallM, openhwgroup#1538) only waits while the cache/bus is busy; once the load hits, DCacheBusStallM drops and the walk fires, re-pointing the cache SRAMs at the PTE sets. - When the stalled load finally reaches W it captures the HPTW's stale read data instead of its own. Fix: also defer the ITLB walk while a committed M-stage data access (load/store/AMO/CMO) is present, not just while the cache/bus is busy. MStageMemPending = |MemRWM | |CMOpM holds the walk until the data op leaves the M stage, so the load retires with correct data and the walk then starts against an idle cache. Cannot deadlock: the data op's retirement never depends on the younger instruction-fetch walk, and DTLB-miss walks are unaffected. Verified: sv32_nleaf_pte_DAU_Umode (issue reproducer) passes ImperasDV lockstep with 0 mismatches (was 6). Regression clean on Verilator: rv64gc arch64vm_sv39/sv48/sv48_b/sv57, arch64i/priv/m/c, wally64priv; rv32gc arch32i/priv/c/m, wally32priv. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Close remaining rv64gc code-coverage gaps that are unreachable by design: - I$ AHBBuscachefsm: exclude the writeback/atomic/back-to-back-fetch branch, condition, and expression rows (HREADY1, FetchWait, SelBusBeat). The I$ is a read-only cache with HREADY always 1, so these write/atomic/ writeback paths never activate. - D$ cachefsm LoadMiss: exclude the ~InvalidateCache expression term; D$ ties InvalidateCache to 1'b0, so the term is dead. Anchored on "assign LoadMiss" since the existing AnyMiss tag resolves earlier. - HPTW: exclude the UPDATE_PTE self-loop (the just-read PTE is resident and writable, so the store hits and DCacheBusStallM cannot assert) and the SvMode!=SV57 condition on the PPN-source mux (L4 states only exist under SV57). - vm64check.S: add check15a/b/c to exercise the three SV39 upper-bit field patterns (all0_55_47_0, all1_46_38_0, all1_55_47_0) that no existing VA covered. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Exclude three more dead rows in the read-only instruction-cache bus FSM (viewcov-validated against the merged rv64gc cov.ucdb): - HREADY1 / FetchWait: widen -item b 1 to -item bs 1 so the dead statements (L91 CACHE_WRITEBACK assign, L104 back-to-back FetchWait assign) are excluded along with their already-excluded branches. - AtomicElse (L98): exclude the ATOMIC_READ_DATA_PHASE self-loop else branch. The state is dead in a read-only cache (no atomics) and is already -fstate excluded, so the else is unreachable too. Anchored on AtomicElse (L98), not AtomicReadData (L97) which -fstate already covers. I$ AHBBuscachefsm Branch -> 100% (22/23 -> 23/23) and Statement -> 100% (29/31 -> 31/31). Remaining I$ condition/expression rows and the D$ cachefsm/AHBBuscachefsm rows are left open (HREADY-term rows pending a buildroot-inclusive check; CMO/TagSetStale rows need directed tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Close the remaining rv64gc code-coverage gaps that are unreachable by construction (all viewcov-validated against the merged cov.ucdb). bpred -> 100%: - bpred.sv:177 BPWrongE InstrValidD_0. Single-issue invariant (RTL comment L174-175): when a branch mispredicts in E the next instruction in D is always valid, since no flush can invalidate D without also flushing the branch in E. InstrValidD is constant-1 here, so InstrValidD_0 is unreachable. D$ cachefsm -> Cond 100%, Expr 134/135: - cachefsm.sv:132 (|CMOpM & ~CMOWriteback) CMOWriteback_1. Priority- masked: the higher-priority else-if L131 (AnyMiss | CMOWriteback) catches CMOWriteback=1 first (D$ READ_ONLY_CACHE=0), so L132 is only ever evaluated with CMOWriteback=0. Verified by waveform that a cbo.clean/flush of a dirty line does assert CMOWriteback yet never covers this row. - cachefsm.sv:213 CacheBusRW[0] term (STATE_WRITEBACK & (CMOpM[1]| CMOpM[2]) & ~CacheBusAck). Logically redundant: fully subsumed by the earlier OR term L211 (STATE_WRITEBACK & ~CacheBusAck), so CMOpM[1]/[2] can never independently drive the output. (TagSetStale_1 remains open -- the issue openhwgroup#1538 stale-tag scenario.) I$ AHBBuscachefsm Branch + Statement -> 100%: - HREADY1 / FetchWait: widen -item b to -item bs to also exclude the dead CACHE_WRITEBACK (L91) and back-to-back FetchWait (L104) statements. - AtomicElse (L98): exclude the ATOMIC_READ_DATA_PHASE self-loop else; the state is dead in a read-only cache (already -fstate excluded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Statements 92.5% -> 100%, Conditions 64.3% -> 94.7% (viewcov-validated). - WritebackWriteback / FetchWriteback / FetchWait: widen the existing -item bc exclusions to -item bcs so the dead L103/L104/L107 statements are excluded too. The whole line is dead (the cache never issues back-to-back pipelined writeback/fetch bus operations), so its statement and all condition rows are unreachable. - CACHE_FETCH fetch-done branch (HREADY & FinalBeatCount & ~|CacheBusRW -> ADR_PHASE): exclude its three _0 condition rows. HREADY is always 1 for the D$ cache states (ram_ahb never stalls and the D$ is the never-gated high-priority bus manager), and a pending request at or after the final beat would be the back-to-back Fetch/Writeback paths above, which never occur. Remaining D$ AHBBuscachefsm rows (cond L140 CacheAccess_0; expr L118/119 HRESETn_0, L123, L127 Flush_1) are reachable-rare corners left open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix a branch-coverage regression and exclude the remaining unreachable
buscachefsm rows (all viewcov-validated across branch/cond/expr/stmt).
D$ AHBBuscachefsm:
- The three dead back-to-back lines (WritebackWriteback / FetchWriteback /
FetchWait) now use a bare -linerange (whole-line) exclusion instead of
-item bcs 1. bcs 1 only caught one branch direction, leaving branch
item 2 of L103/L107 uncovered (Branch had regressed to 31/33); the
whole-line form excludes both directions, all condition rows, and the
statement -> Branch + Condition + Statement back to 100%.
- Additional unreachable rows -> Expressions 26/27 (only Flush_1 left):
* BeatCountReg/BeatCountDelayedReg reset HRESETn_0 (no D$ transaction is
in flight at power-on reset; the I$ covers this, the D$ cannot)
* BeatCntEn (NextState==ADR_PHASE)_0 (priority-masked: a pending cached
request forces NextState to a cache state)
* HTRANS pipelined-request CacheAccess_0 (FinalBeatCount only asserts at
the end of a cache burst, so CacheAccess=0 & FinalBeatCount can't occur)
* CACHE_FETCH fetch-done HREADY/FinalBeatCount/~|CacheBusRW _0 rows
I$ AHBBuscachefsm: mirror the D$ dead rows that also apply to a read-only
cache -- CACHE_FETCH fetch-done _0 rows and the HTRANS (CacheAccess &
|BeatCount) CacheAccess_0. Left uncovered (reachable): L90 HREADY_0 (the
genuine ADR_PHASE IFU/LSU-contention row, IFUHREADY is gated by non-grant)
and the L147/L127 flush-mid-transaction corners.
Still open and intentionally NOT excluded (reachable, need buildroot/nightly
or are hard to isolate deterministically): D$ cachefsm TagSetStale_1 (the
issue openhwgroup#1538 stale-tag case), I$ AHBBuscachefsm L90/L147/L127, D$
AHBBuscachefsm L127 Flush_1.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The D$ bus-FSM CaptureEn (buscachefsm.sv:127) Flush_1 row, ((~Flush & DATA_PHASE) & BusRW[1]), is unreachable. A D$ DATA_PHASE access with BusRW[1] is a committed, non-speculative, post-translation uncached load read, and the D$ flush input LSUFlushW = HPTWFlushW | FlushW has no branch-mispredict term (unlike the I$'s FlushD). During the load's bus data phase CommittedM masks TrapM, StallW zeros FlushW's LatestUnstalledW, and HPTWFlushW only fires on a new TrapM, StallW zeros FlushW's LatestUnstalledW, and HPTWFlushW only fires on a new DTLB miss at the walker -- so Flush is always 0 here. (The I$ copy of this row is reachable via a younger speculative fetch squashed by an older branch's BPWrongE, so only the D$ instance is excluded.) Viewcov-validated on the merged cov.ucdb: D$ AHBBuscachefsm Expressions -> 100%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…quiring lock, allowing many more jobs to run concurrently. Runtime reduces from 25 to 10 minutes for code coverage
…tion-flush rows CVW is in-order, so the I$ never takes a flush during an in-flight bus transaction (BPWrongE frozen by IFUStall, interrupts deferred by CommittedF, M-stage CSRWriteFenceM/TrapM/RetM gate off IFUStallF and preempt at ADR_PHASE/beat 0). Exclude the rows requiring such a flush: HBURST (CacheAccess & |BeatCount) CacheAccess_0/_1 + |BeatCount_1 (feccondrow 5,6,8), and CaptureEn Flush_1 (drops the whole CaptureEn expr incl. the rare reachable BusRW[1]_0, still tracked via L130/L161 + FSM). Viewcov-validated: I$ AHBBuscachefsm Expressions 100%, only L90 HREADY_0 left. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ance 100%
CVW is in-order, so the I$ bus FSM never sees these conditions; exclude with rationale:
- HBURST (CacheAccess & |BeatCount) CacheAccess_0/_1 + |BeatCount_1 (feccondrow 5,6,8)
and CaptureEn Flush_1: require a flush mid in-flight bus transaction. In-order this
never happens -- during CACHE_FETCH/DATA_PHASE the whole pipe stalls
(StallE<=StallM<=StallW<=IFUStallF) so BPWrongE can't fire, interrupts are deferred
(CommittedF), and an M-stage CSRWriteFenceM/TrapM/RetM gates off IFUStallF and so
preempts at ADR_PHASE/beat 0. (CaptureEn fecexprrow isn't surgical so the whole expr
drops, also taking the rare reachable BusRW[1]_0; states/terms remain tracked via
L130/L161 + FSM.)
- ADR_PHASE (HREADY & |BusRW) HREADY_0: the IFU stalled in ADR_PHASE with a pending
*uncached* fetch. An uncached fetch's request is a single ADR cycle; in-order, while
the LSU holds the bus the front-end is stalled and not initiating a fetch, so that
1-cycle request never overlaps an LSU grant (a directed uncached+missing-load sliding
test produced zero ADR_PHASE->ADR_PHASE self-loops).
Viewcov-validated: I$ AHBBuscachefsm Branches/Conditions/Expressions/Statements all 100%.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…roup#1773) * Initial plan * Remove obsolete deriv config from regression-wally nightly list --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Remove malformed derived configuration
…-sfence-global-hptw mmu: preserve global TLB entries on ASID sfence.vma; harden HPTW vs ITLB-miss store drop (openhwgroup#1538)
…PTW vs ITLB-miss store drop (openhwgroup#1538)" (openhwgroup#1774)
Adds the new code-coverage-targeted assembly tests from PR openhwgroup#1771, independent of the RTL/Issue-1766 changes in that PR: cacheInval.S, decompReserved.S, fpuReservedRM.S, pmpTOR7.S, tlbGLBASID.S, tlbGLBHIT.S, and vm64check.S (SV39 FEC upper-bit cases). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- tests.vh: register new coverage tests (tlbGLBASID, tlbGLBHIT,
fpuReservedRM, decompReserved, pmpTOR7, cacheInval) in coverage64gc;
add arch64vm_sv48_a/_b/_sv39_isolate/_sv48_mxr_isolate test lists.
sv57 A_and_D tests left disabled (base reverts Issue openhwgroup#1538).
- testbench.sv: dispatch the new arch64vm_sv48_a/_b/_sv39_isolate/
_sv48_mxr_isolate suites.
- wsim: check design freshness before acquiring the compile lock so
more jobs run concurrently.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s-from-1771 coverage: add tests/coverage tests from PR openhwgroup#1771
regression-wally cleanup for coverage with ACT4
…eing created accidentally where a block device was supposed to be.
…device type is a whole-disk. Cleaned up checks.
* Bump whisper to 9f99b7c * Correct date in install script * Bump whisper to 321a7da Signed-off-by: Ryan Wolk <rwolk@hmc.edu> * Bump whipser to a53d0f3 Signed-off-by: Ryan Wolk <rwolk@hmc.edu> * Bump whisper to e4316b4 Signed-off-by: Ryan Wolk <rwolk@hmc.edu> * Bump whisper to e27141d Signed-off-by: Ryan Wolk <rwolk@hmc.edu> --------- Signed-off-by: Ryan Wolk <rwolk@hmc.edu>
…ritical commands.
Brought over robust exclusions from openhwgroup#1771
…check fix: Added many checks to flash-sd.sh to prevent regular files from being created accidentally where a block device was supposed to be.
Removed dead code
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.
Issue openhwgroup#1766 — LW reads wrong value when a D$ access overlaps an ITLB miss
Fixes openhwgroup#1766.
Stacked on top of openhwgroup#1538 (
dh/issue-1538-sfence-global-hptw, PR openhwgroup#1768) — this PR is based on that branch and contains only the incrementalhptw.svchange. Merge openhwgroup#1768 first.Symptom
lw x9,4(x6)at PA0x800012a0reads0x11instead of0x0000000c(sv32). Reproducer:sv32_nleaf_pte_DAU_Umodefrom the issue.Root cause
A wrong-path fetch (branch mispredict) raises an instruction page fault; the HPTW reaches FAULT and returns to IDLE, but the faulting fetch is not yet flushed, so its ITLB miss is still pending. The
lwadvances into M and—after the openhwgroup#1538TagSetStalere-read—hits in the D$. Its read data is only valid combinationally while the load still owns the cache. The load cannot retire becauseHPTWStall(asserted by the pending ITLB miss itself) holds the pipeline. The previous deferral guardITLBMissReady = ITLBMissOrUpdateAF & ~DCacheBusStallM(openhwgroup#1538) only waits while the cache/bus is busy; once the load hits,DCacheBusStallMdrops and the walk fires, re-points the cache SRAMs at the PTE sets, and the stalled load captures HPTW residue when it finally reaches W.Fix
Also defer the ITLB walk while a committed M-stage data access is present, not just while the cache/bus is busy:
The load retires with correct data, then the walk starts against an idle cache. Cannot deadlock: a committed M-stage op's retirement never depends on the younger instruction-fetch walk; DTLB-miss walks are unaffected.
Verification
arch64vm_sv39/sv48/sv48_b/sv57,arch64i/priv/m/c,wally64priv; rv32gc:arch32i/priv/c/m,wally32priv.🤖 Generated with Claude Code