Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss by davidharrishmc · Pull Request #1771 · openhwgroup/cvw

davidharrishmc · 2026-05-29T13:29:46Z

Requires 1768 to be merged first.

Many code coverage improvements, especially exclusions based on Claude reasoning.

Improved parallelism for regression-wally speeds up --ccov from 25 to 10 minutes

mmu: defer ITLB-miss walk until M-stage data access leaves M (#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 Found some bugs while running regression for SV48 #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, Found some bugs while running regression for SV48 #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

…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>

Copilot

Pull request overview

This PR addresses virtual-memory/cache ordering hazards where instruction-side page walks can interfere with in-flight data-cache accesses, and also adjusts related TLB flush behavior and regression coverage.

Changes:

Defers ITLB HPTW walks while M-stage data memory operations are pending.
Updates cache FSM handling for stale tag-set reads and stalled store hits.
Adds sfence.vma global-entry flush control plus targeted VM regression test lists.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`src/mmu/hptw.sv`	Defers ITLB walks and narrows LSU flushes to DTLB walks/faults.
`src/cache/cache.sv`	Tracks previous HPTW tag set to detect stale cache reads.
`src/cache/cachefsm.sv`	Suppresses stale hit/miss decisions and extends cache enable for stalled store hits.
`src/lsu/lsu.sv`	Wires new cache fault/flush controls into D$ and DTLB paths.
`src/ifu/ifu.sv`	Wires TLB flush-all control and I$ cache fault tie-off.
`src/mmu/mmu.sv`	Adds TLB flush-all plumbing.
`src/mmu/tlb/tlb.sv`	Passes flush-all control into TLB CAM.
`src/mmu/tlb/tlbcam.sv`	Passes flush-all control into CAM lines.
`src/mmu/tlb/tlbcamline.sv`	Preserves global entries for ASID-scoped TLB flushes.
`src/privileged/privileged.sv`	Exposes `sfencevmaAllM` from privileged decode.
`src/privileged/privdec.sv`	Decodes whether `sfence.vma` should flush global entries.
`src/wally/wallypipelinedcore.sv`	Connects `sfencevmaAllM` between core units.
`testbench/tests.vh`	Adds isolated/order-sensitive VM test sets and re-enables SV57 tests.
`testbench/testbench.sv`	Adds selectors for new VM test lists.
`sim/questa/coverage-exclusions-rv64gc.do`	Updates coverage exclusions for new cache FSM terms.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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>

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>

coverage: add tests/coverage tests from PR #1771

Brought over robust exclusions from #1771

davidharrishmc · 2026-06-12T04:12:51Z

After merging recent changes, coverage is still nearly complete.

Instance                                Assertions          Branches            Conditions          Covergroup Bins     Directives          Expressions         FSM States          FSM Transitions     Statments           Toggles             
----------                              ---------------     ---------------     ---------------     ---------------     ---------------     ---------------     ---------------     ---------------     ---------------     ---------------     
-core                                         -                   -                   -                   -                   -                   -                   -                   -                   -                   -             
--priv/priv                                   -                   -                   -                   -                   -                   -                   -                   -                   -                   -             
---pmd                                        -                   -                   -                   -                   -              96.66%(58/60)            -                   -                   -                   -             
--lsu                                         -                   -                   -                   -                   -              96.29%(26/27)            -                   -                   -                   -

jordancarlin · 2026-06-14T20:29:06Z

Looks like the PR title and description need to be updated. This no longer does anything to code coverage or regression-wally.

Removed dead code

davidharrishmc · 2026-06-14T20:47:27Z

Coverage fixes were already merged (#1775), so all that remains is the hptw fix for #1766.

davidharrishmc · 2026-06-14T21:09:48Z

Updated title.

rosethompson · 2026-06-15T05:43:30Z

I did an extensive review of this change and the bug. I'm not sure how this slipped through. This bug and this cover point was one of the original cases I considered in the design of the HPTW. It is the concurrent ITLB miss with D$ miss. I am honestly surprised Wally boots Linux at all with this bug.

There are two ways to fix it. One is the proposed solution which gives priority to the D$ miss, the other prioritizes the ITLB miss delaying the D$ transaction until the ITLB is fulled. Unfortunately, the proposed solution does not work. The FPGA fails to boot Linux with this PR.

davidharrishmc force-pushed the issue-1766-fix branch from 8dce2c1 to 8100f87 Compare May 29, 2026 13:46

davidharrishmc closed this May 29, 2026

davidharrishmc reopened this May 29, 2026

davidharrishmc changed the title ~~Issue 1766 fix~~ Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss May 29, 2026

davidharrishmc requested review from Copilot and rosethompson May 29, 2026 13:59

Copilot started reviewing on behalf of davidharrishmc May 29, 2026 13:59 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread src/cache/cachefsm.sv

davidharrishmc and others added 13 commits May 29, 2026 11:01

AI close camline coverage

355e63e

coverage improvements

7b43acf

Code coverage exclusions

7f60446

coverage improvements

ab2fa18

Speed up regression by facilitating wsim checking freshness before ac…

353a51e

…quiring lock, allowing many more jobs to run concurrently. Runtime reduces from 25 to 10 minutes for code coverage

davidharrishmc changed the title ~~Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss~~ Close most code coverage, speed up regression-wally, Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss May 31, 2026

davidharrishmc mentioned this pull request Jun 2, 2026

coverage: add tests/coverage tests from PR #1771 #1775

Merged

rosethompson added a commit that referenced this pull request Jun 2, 2026

Merge pull request #1775 from davidharrishmc/coverage-tests-from-1771

1a9cc37

coverage: add tests/coverage tests from PR #1771

davidharrishmc added a commit to davidharrishmc/cvw that referenced this pull request Jun 2, 2026

Brought over robust exclusions from openhwgroup#1771

db71c58

rosethompson added a commit that referenced this pull request Jun 11, 2026

Merge pull request #1777 from davidharrishmc/dev

7f85a24

Brought over robust exclusions from #1771

merge

81bffd7

jordancarlin reviewed Jun 14, 2026

View reviewed changes

Comment thread sim/questa/coverage-exclusions-rv64gc.do Outdated

Update coverage-exclusions-rv64gc.do

23c4862

Removed dead code

davidharrishmc changed the title ~~Close most code coverage, speed up regression-wally, Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss~~ Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss Jun 14, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss#1771

Issue 1766 fix: lw returns wrong value when access coincides with ITLB miss#1771
davidharrishmc wants to merge 16 commits into
openhwgroup:mainfrom
davidharrishmc:issue-1766-fix

davidharrishmc commented May 29, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

davidharrishmc commented Jun 12, 2026

Uh oh!

Uh oh!

jordancarlin commented Jun 14, 2026 •

edited

Loading

Uh oh!

davidharrishmc commented Jun 14, 2026

Uh oh!

davidharrishmc commented Jun 14, 2026 •

edited

Loading

Uh oh!

rosethompson commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

davidharrishmc commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

davidharrishmc commented Jun 12, 2026

Uh oh!

Uh oh!

jordancarlin commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidharrishmc commented Jun 14, 2026

Uh oh!

davidharrishmc commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rosethompson commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davidharrishmc commented May 29, 2026 •

edited

Loading

jordancarlin commented Jun 14, 2026 •

edited

Loading

davidharrishmc commented Jun 14, 2026 •

edited

Loading