-
Notifications
You must be signed in to change notification settings - Fork 547
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
[dv] Fix performance counter printing in simple system #2102
Conversation
2258690
to
d626b4c
Compare
d626b4c
to
4aa6879
Compare
if (index == 0 || index == 2) | ||
return true; | ||
|
||
// The "NONE" counter is fake. That never exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// The "NONE" counter is fake. That never exists. | |
// The "NONE" counter is a placeholder for the "MTIME" CSR. That never exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that makes sense. I couldn't figure this out: how can we tell that index 1 corresponds to the MTIME CSR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the order of MCYCLE, MTIME, MINSTRET, MHPMCOUNTER3, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm clearly being slow here. Where does that list of CSRs come from? Is it a standard RISC-V thing? My web searches find lots of Ibex...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at e.g. the RISC-V privileged spec, I see MCYCLE, (a gap here), MINSTRET, MHPMCOUNTER3, ... in table 2.5.
Is there a spec that explains that the gap is MTIME?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MTIME was once an actual CSR but later removed from the spec since people want it to be MMIO instead of actual CSR. But you can guess that this counter is reserved for MTIME by looking at the list of user-mode CSRs, which is CYCLE, TIME, INSTRET, HPMCOUNTER3, ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahah! Thanks for the explanation. I'll tweak this PR to match.
4aa6879
to
8b0f482
Compare
This will avoid printing out a load of spurious zeros if the Ibex config doesn't enable the corresponding counter.
8b0f482
to
2bfb499
Compare
This comes in two commits, but the first is a silly typo fix. The commit message for the second is:
Partial fix for #2101.