Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase TLB size to 64 #498

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Jun 18, 2024

This has a very significant effect on the emulator performance with virtual memory enabled (e.g. when booting Linux).

Having a more realistic TLB can also help expose bugs, like the recent bug in #496. For example with a larger TLB you can add an assertion in translate(). It also allows you to detect missing sfence.vmas in software by asserting that the TLB translation is the same as the uncached translation. This is not implemented in this change but we can do it in future (or you can just hack it in when needed). The detection will never be 100% but the bigger the TLB the more missing fences you will detect.

64 was chosen based on benchmarks I did last year. Performance when booting Linux keeps going up until you get to 64.

@Timmmm Timmmm requested a review from rsnikhil June 18, 2024 15:44
@Timmmm
Copy link
Collaborator Author

Timmmm commented Jun 18, 2024

Shouldn't be merged before #496 is merged, otherwise it makes the bug that that PR fixes way more likely (which is how I discovered it).

Copy link

github-actions bot commented Jun 18, 2024

Test Results

712 tests  ±0   712 ✅ ±0   0s ⏱️ ±0s
  6 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit be3f3b1. ± Comparison against base commit eeb9270.

♻️ This comment has been updated with latest results.

@Timmmm Timmmm mentioned this pull request Jun 28, 2024
This has a very significant effect on the emulator performance with virtual memory enabled (e.g. when booting Linux).

Having a more realistic TLB can also help expose bugs, like the recent bug in riscv#496. For example with a larger TLB you can add an assertion in `translate()`. It also allows you to detect missing `sfence.vma`s in software by asserting that the TLB translation is the same as the uncached translation. This is not implemented in this change but we can do it in future (or you can just hack it in when needed). The detection will never be 100% but the bigger the TLB the more missing fences you will detect.

64 was chosen based on benchmarks I did last year. Performance when booting Linux keeps going up until you get to 64.
@Timmmm
Copy link
Collaborator Author

Timmmm commented Jul 15, 2024

@billmcspadden-riscv this should be ok to merge now that the fix in #496 is merged.

@billmcspadden-riscv billmcspadden-riscv merged commit db5f430 into riscv:master Jul 15, 2024
2 checks passed
@Timmmm Timmmm deleted the user/timh/bigger_tlb_2 branch August 30, 2024 15:34
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.

3 participants