sail-riscv
sail-riscv copied to clipboard
riscv
Don't read or write 8 bytes for 4-byte PTEs
For Sv32 Page Table Entries are only 4 bytes, but the old code was unconditionally reading and writing 8 bytes.
Fixes #459
Test Results
712 tests ±0 712 :white_check_mark: ±0 0s :stopwatch: ±0s 6 suites ±0 0 :zzz: ±0 1 files ±0 0 :x: ±0
Results for commit 4d607432. ± Comparison against base commit e1242d85.
:recycle: This comment has been updated with latest results.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Actually it's worse than I thought; it also writes 8 bytes for Sv32 when updating the accessed/dirty bits. This pretty much means Sv32 is broken at the moment.
Are you planning on making a separate PR for the writes, or adjust this one? Otherwise LGTM, and we should get this fix in quickly.
How was the original PR tested if we’re finding this now?
We ran it on the tests in this repo, which I thought does include RV32 with virtual memory enabled. We also had a member of the golden model meeting run it on an extra set of virtual memory tests their company had, and we re-ran the basic 'boot FreeBSD' smoke test. It might be the case that there are no tests that actually do anything with the PTE dirty bits though.
How was the original PR tested if we’re finding this now?
We ran it on the tests in this repo, which I thought does include RV32 with virtual memory enabled. We also had a member of the golden model meeting run it on an extra set of virtual memory tests their company had, and we re-ran the basic 'boot FreeBSD' smoke test. It might be the case that there are no tests that actually do anything with the PTE dirty bits though.
--enable-dirty-update is off by default
Are you planning on making a separate PR for the writes, or adjust this one?
I'll update this one.
Ok I updated it to include the writes too. I am also working on refactoring the virtual memory code to use Sail's type system properly in all of this code, rather than resorting to bit(64).