Skip to content

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

Open
davidharrishmc wants to merge 3 commits into
openhwfoundation:mainfrom
davidharrishmc:dh/fp-ad-update-pte
Open

davidharrishmc wants to merge 3 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 fpADUpdate test (fsd/fld, rv64gc) and fpADUpdate32 (fsw/flw) in a new coverage64f suite that f_rv64gc and fh_rv64gc run in the nightly regression; the suite is self-checked, unlike coverage64gc. 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>
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