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

Add support for HPM counters #404

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Feb 7, 2024

Implements Hardware Performance Monitoring counters, but only by making the registers available. Their values only change if explicitly written to and all values are legal.

All 32 HPM counters are always implemented (the spec says this "should" be the case), and a platform callback sys_writable_hpm_counters is added to allow making any of them read-only. I haven't hooked sys_writable_hpm_counters up to CLI flags because Codasip does not use the CLI flags and it is quite tedious, and hopefully going to change when riscv-config is used.

Finally this adds menvcfg and senvcfg to the CSR name map which I forgot to do previously.

Copy link

github-actions bot commented Feb 7, 2024

Test Results

396 tests  ±0   396 ✅ ±0   0s ⏱️ ±0s
  4 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 01b63cd. ± Comparison against base commit fd1be4b.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"High Performance Counters" should be "Hardware Performance Counters"?

Also do we need to guard these with a (always-true) haveZihpm() function?

@Timmmm
Copy link
Collaborator Author

Timmmm commented Feb 7, 2024

"High Performance Counters" should be "Hardware Performance Counters"?

Also do we need to guard these with a (always-true) haveZihpm() function?

Good call, I'll add that too.

model/prelude.sail Outdated Show resolved Hide resolved
Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but would be good to have someone else double-check

@Timmmm Timmmm force-pushed the user/timh/hpmcounters branch 2 times, most recently from 661228a to 549f55f Compare February 13, 2024 12:01
@Timmmm
Copy link
Collaborator Author

Timmmm commented Apr 9, 2024

@billmcspadden-riscv I think it's ok if you double check this since you've also done a lot of HPM stuff.

@Timmmm Timmmm added the tgmm-agenda Tagged for the next Golden Model meeting agenda. label May 14, 2024
@Timmmm
Copy link
Collaborator Author

Timmmm commented Jul 23, 2024

@billmcspadden-riscv I've rebased this.

model/riscv_sys_regs.sail Outdated Show resolved Hide resolved
@Timmmm
Copy link
Collaborator Author

Timmmm commented Sep 6, 2024

@billmcspadden-riscv please can you merge this? It's ready to go.

Implements Hardware Performance Monitoring counters, but only by making the registers available. Their values only change if explicitly written to and all values are legal.

All 32 HPM counters are always implemented (the spec says this "should" be the case), and a platform callback `sys_writable_hpm_counters` is added to allow making any of them read-only. I haven't hooked `sys_writable_hpm_counters` up to CLI flags because Codasip does not use the CLI flags and it is quite tedious, and hopefully going to change when riscv-config is used.
@billmcspadden-riscv billmcspadden-riscv merged commit 5a841c9 into riscv:master Sep 24, 2024
2 checks passed
@Timmmm Timmmm deleted the user/timh/hpmcounters branch September 25, 2024 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tgmm-agenda Tagged for the next Golden Model meeting agenda.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants