Revert #1875: spurious load page fault after a wrong-path ITLB walk fault (#1885) - #1886
Merged
rosethompson merged 1 commit intoSep 17, 2026
Merged
Conversation
…e bug in that PR (openhwfoundation#1875)" This reverts commit 310dbfabe75b9ccd08f5e83a52d5e4f6a8c62d2f. openhwfoundation#1875 removed the walker/LSU arbitration guards added by openhwfoundation#1871: ITLBReq no longer defers behind MemAccessInFlightM, HPTWFlushW no longer spares an access that has already been performed and captured, and lsu.sv lost the MemAccessDone / ReadDataHold state. With those guards gone, a wrong-path fetch whose ITLB walk ends in FAULT can leave a spurious load page fault on an unrelated M-stage load: the load hits in the D$ and returns the correct data in the very cycle the core traps on it (issue openhwfoundation#1885). Three Sv arch tests livelock in a recursive trap as a result. Reverting restores the openhwfoundation#1871 behavior. The sequencing argument in openhwfoundation#1875 may still be right, but something has to cover the window where the M-stage access has been performed and an ITLB request arrives in the next cycle. Verified on this revert: the three openhwfoundation#1885 reproducers pass (Verilator and ImperasDV lockstep), the openhwfoundation#1766 ELF and arch64vm_sv48_a/_b still pass, and the ACT Sv suites pass on rv32gc and rv64gc. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Both runs were done in fresh worktrees prepared identically ( |
Contributor
|
I will merge this, but I suspect there is a deeper issue in the control logic that needs to be debugged. |
Contributor
Author
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.
Reverts #1875 (
310dbfabe) to fix #1885.Why
#1875 removed the walker/LSU arbitration guards #1871 had added:
plus the
MemAccessDone/ReadDataHoldstate inlsu.sv. Without them, an M-stage access thathas already been performed can be flushed by an ITLB request arriving the next cycle, and a
wrong-path fetch whose walk ends in FAULT leaves a spurious load page fault on an unrelated load:
the load hits in the D$ and returns the correct data in the very cycle the core traps on it
(
scause=13,stval= the load's own address). ThreeSvarch tests then livelock in a recursivetrap. Full trace and reproducer ELFs are in #1885.
The sequencing argument in #1875 may well be right — but something still has to cover the window
where the M-stage access has been performed and an ITLB request arrives in the next cycle. This
revert is the conservative step while that is worked out; happy to close it in favour of a narrower
fix.
Verification
Verilator 5.036, plus Questa + ImperasDV where noted. "main" =
ff84ec108, "#1871" =b3d3ac1a1.arch64vm_sv48_a/_b,arch64vm_sv39,arch64vm_sv48,arch32vm_sv32Svat act4 HEAD (rv32gc 30, rv64gc 97)Svat22c01e88b1(rv32gc 30, rv64gc 97)lint-wally(
lint-wallyalso reportsfdqh_rv64gc failed linton both main and this branch in a freshworktree — missing generated
config/deriv, not a code difference.)regression-wallyresults to follow in a comment.🤖 Generated with Claude Code