Skip to content

lsu: don't substitute FP store data for the walker's PTE write - #1931

Merged
rosethompson merged 6 commits into
openhwfoundation:mainfrom
davidharrishmc:dh/fp-ad-update-pte
Oct 15, 2026
Merged

rosethompson merged 6 commits into
openhwfoundation:mainfrom
davidharrishmc:dh/fp-ad-update-pte

Conversation

@davidharrishmc

@davidharrishmc davidharrishmc commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

The FP store-data mux in the LSU sits after the walker's PTE write-data mux and was selected by FpLoadStoreM alone, so with menvcfg.ADUE=1 an A/D-update walk started by an FP store or load (or by an instruction fetch while fclass, fmv.x.* or an FP load/store was in M) wrote the FP register into the PTE: fsd to a D=0 page left f1's bits in the PTE instead of the PTE with D set. The TLB was filled correctly, so the corrupt PTE took effect only after eviction or SFENCE.VMA, letting user data become a leaf PTE. The same happened on F-only configs with XLEN=64 (f_rv64gc, fh_rv64gc), where the FLEN < XLEN branch of the mux was used. hptw.sv now muxes FpLoadStoreM with the walker (fpmux, next to rwmux and the other M-stage controls it overrides), and the LSU uses the resulting LSUFpLoadStoreM for both store-data mux branches, the bus size and subwordread, so no FP control reaches the walker's access. It adds the self-checking tests fpADUpdate (fsd/fld) and fpADUpdate32 (fsw/flw) to tests/coverage, where they run on rv64gc with the rest of that directory; regression-wally's nightly regression also runs fpADUpdate32 on f_rv64gc and fh_rv64gc. Part of #1921.

🤖 Generated with Claude Code

FpLoadStoreM selected the FP write data even while the HPTW owned the LSU,
so an A/D-update walk started by an FP load/store (or with fclass/fmv.x in
M) wrote the FP register into the PTE.  Gate the select with ~SelHPTW and
add the fpADUpdate test.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
The previous commit gated only the FLEN >= XLEN store-data mux.  On F-only
configs with XLEN=64 (f_rv64gc, fh_rv64gc) the FLEN < XLEN mux still
selected the FPU's store data for the walker's PTE write, so fsw/flw to a
page needing an A/D update wrote the FP register into the PTE.

Define LSUFpLoadStoreM = FpLoadStoreM & ~SelHPTW once and use it for both
store-data muxes, the bus size and subwordread, so no FP control reaches
the walker's access.  Add fpADUpdate32 (F instructions only) in a new
coverage64f suite that f_rv64gc and fh_rv64gc run in regression.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
@rosethompson

Copy link
Copy Markdown
Contributor

While this change is functionally correct, it is inconsistent with the style established with the existing control signals gated by SelHPTW. All other M stage control signals are brought into hptw.sv and muxed with the hptw's driver. For example...
mux2 #(2) rwmux(MemRWM, HPTWRW, SelHPTW, PreLSURWM);
This should be updated to match the same style.

Move the FpLoadStoreM gating from the LSU into hptw.sv as fpmux, next to rwmux, sizemux and the other
controls the walker overrides, as requested in review.  LSUFpLoadStoreM is now an hptw output (and
FpLoadStoreM itself when there is no walker).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
davidharrishmc and others added 2 commits October 10, 2026 07:00
…idth openhwfoundation#1948) into dh/fp-ad-update-pte

openhwfoundation#1895 deleted testbench/tests.vh and the testbench's named test lists, so
this branch's coverage64gc entry for fpADUpdate and its coverage64f list and
testbench case for fpADUpdate32 go away: take main's testbench.sv and drop
tests.vh.  Every ELF in tests/coverage now runs on rv64gc, which picks up
fpADUpdate and fpADUpdate32 without a list.  The next commit hooks
fpADUpdate32 into the regression for the F-only configurations.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
The coverage64f suite named in derivconfigtests no longer exists now that
tests.vh is gone.  List fpADUpdate32 per configuration in
deriv_coverage_tests instead and run it from tests/coverage as one ELF group
per configuration in the nightly regression, the way periph_tests runs the
peripheral tests.  On rv64gc it runs with the rest of tests/coverage.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
Keep main's coverage32 lockstep run and addActTests next to the
fpADUpdate32 derivative-config runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: David Harris <David_Harris@hmc.edu>
@rosethompson
rosethompson merged commit 9f672c9 into openhwfoundation:main Oct 15, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants