Repository navigation
cva6: D-cache data RAM drops its byte enables, so every cache write corrupts the other way #4769
Description
Where: flow/designs/src/cva6/core/cache_subsystem/hpdcache/rtl/src/common/hpdcache_sram_wbyteenable.sv (unchanged
since cb73ed6; same at 26Q4 ecb3cfdeb), used by designs/asap7/cva6.
What: the wrapper replaces upstream's hpdcache_sram_wbyteenable_1rw with fakeram7_128x64, which has no write
mask, and leaves its wbyteenable input unconnected. cv32a65x packs both ways of a D-cache set into each 64-bit RAM
word (cva6_hpdcache_subsystem.sv: dataWaysPerRamWord = min(2, 128/32) = 2). hpdcache_memctrl.sv replicates the
write data into both 32-bit lanes and selects the way only through the byte enables. With them dropped, every refill and
every store hit, full-word ones included, also overwrites the other way's word at that address. Any program that fills
both ways of a set reads wrong data. The I-cache is not affected (one macro per way, byte enables all ones).
Shown in simulation: upstream cva6 at 3a389af (the commit designs/src/cva6 is based on) with this tree's changed
files, upstream's Verilator testharness, target cv32a65x, Verilator 5.008, behavioural models of the four fakeram7
macros (read on a rising clock with ce_in high, write when we_in, hold otherwise). A 40-line C program writes two
16-byte lines 1 KB apart (one set, both ways), then reads them back alternately and returns the mismatch count:
| RTL | result | cva6 iss_cmp against Spike |
|---|---|---|
upstream's hpdcache_sram_wbyteenable_1rw |
pass, 525 cycles | 100 GPR updates matched |
this hpdcache_sram_wbyteenable.sv |
fail (tohost = 1), 525 cycles | first mismatch at c.lw a2, 8(a5): Spike 0x11110002, RTL 0x22220002 (the other way's word) |
Consequence: the cva6 netlist the flow builds does not compute correctly; its timing and area are not affected, but
any gate-level or activity-based use of it is.
Possible fixes: a data-RAM macro with a byte write mask connected to wbyteenable, or one macro per way.
Activity
oharboe commentedon Oct 30, 2026
@maliberty FYI.... generate_ram and patches carried in bazel-orfs will fix this hope. LEC could help too ...
saikatchatrg commentedon Oct 31, 2026
As far as I can see, none of the bazel-orfs patches replaces
hpdcache_sram_wbyteenableyet. A generated data RAM with a byte mask onwbyteenablewould fix it. LEC between flow stages wouldn't catch this one, because the netlists match the RTL; it shows up in simulation, or against upstream's byte-enabled wrapper.oharboe commentedon Oct 31, 2026
You're right, and sorry: I misremembered. I thought I had used cva6 in bazel-orfs's CoreMark/Joule study, where a simulation would have caught this, but cva6 is only a literature point there. No carried patch replaced this wrapper, and LEC between flow stages can't see it: the netlists match the RTL, and the RTL is what's wrong.
A fix, carried in bazel-orfs for now: The-OpenROAD-Project/bazel-orfs#1484 (draft). It puts
hpdcache_sram_wbyteenable.svback to upstream's, unchanged, with upstream's behaviouralhpdcache_sram_wbyteenable_1rw, and letsAUTO_MEMORIESmake that RAM a 128 x 64 macro with 8 byte-mask lanes, its pins named by a.memoriesoverride asasap7/coralnpudoes for itsSram. cv32a65x's MSHR uses the flip-flop register bank, so the data RAM is the only instance, and the I-cache is unaffected as you said.It is not tested yet, not even built past analysis. Feel free to try your two-way test program on it.
saikatchatrg commentedon Oct 31, 2026
Ran the two-way test on #1484's RTL. The two files patch 0202 restores,
hpdcache_sram_wbyteenable.svandmacros/behav/hpdcache_sram_wbyteenable_1rw.sv, are byte-identical to the ones in the tree the test passed on (upstream cva6 3a389af with ORFS's other changes todesigns/src/cva6, upstream's Verilator testharness, cv32a65x, behavioural models of the other fakeram7 macros). Re-run today: SUCCESS (tohost = 0) after 525 cycles, and cva6'siss_cmpagainst Spike: 100 matched. So with the behavioural RAM the core computes correctly. Not covered here: the generated 128 x 64 macro's own model in place of the behavioural RAM, which your kepler-formal check covers at depths 2 and 4.saikatchatrg commentedon Nov 1, 2026
Thank you for turning this around so quickly and so thoroughly: all four RAMs back to upstream's RTL, each macro checked by LEC with controls, and the change upstreamed as #4776 and #4777. It was a pleasure to check against.
maliberty commentedon Nov 4, 2026
Resolved?
oharboe commentedon Nov 4, 2026
The PRs are in flight. I would normally close the issue as I'm happy using only PRs to track work, but this project generally keeps issues open until PRs are merged. To me this is a matter of preferences.
asap7/cva6: the D-cache RAMs as upstream has them, made macros
saikatchatrg commentedon Nov 6, 2026
Thanks @oharboe for the fix, and @maliberty for the review.
We reran the test from this issue on #4777's merge (
8d33f937f): ORFS master'sdesigns/src/cva6withasap7/cva6/surgery.pyapplied to the techno cut, the rest as above (upstream cva6 at 3a389af, its Verilator testharness, cv32a65x, Verilator 5.008). The six RAM files in master are byte-identical to upstream's.iss_cmp100 GPR updates matched; RVFI trace identical to the upstream-wrapper run aboveiss_cmp191,314 matched, 9 mismatches, allmcycle/minstretreads around the timed loop (Spike's counters are not cycle-accurate)ecb3cfdeb), as a controlSo the RTL the flow now synthesizes keeps the D-cache byte enables. We simulated the RTL only, not the macros AUTO_MEMORIES makes from it.