hpcp: gate per-counter enables on combinational cnt_mode_dis_pre#53
Open
flaviens wants to merge 1 commit into
Open
hpcp: gate per-counter enables on combinational cnt_mode_dis_pre#53flaviens wants to merge 1 commit into
flaviens wants to merge 1 commit into
Conversation
The `pmdm`/`pmds`/`pmdu` bits silence the hardware perf counters in M, S,
and U mode respectively. Today every per-counter enable in
`ct_hpcp_top.v` gates on the registered `cnt_mode_dis`, which lags the
combinational `cnt_mode_dis_pre` by one cycle on every privilege-mode
change. Each transition into a disabled mode counts one extra event that
should have been silenced, and each transition out misses one event that
should have been counted.
Drive the per-counter enables and the exported `hpcp_xx_cnt_en` from
`cnt_mode_dis_pre` directly. The clock-gating logic at line 1086 already
uses `(cnt_mode_dis_pre ^ cnt_mode_dis)` to keep the block clocked
across the transition cycle, so removing the one-cycle lag from the
counter enables does not change clock activity. The registered
`cnt_mode_dis` is left in place because that XOR depends on it.
Affected sites in `C910_RTL_FACTORY/gen_rtl/pmu/rtl/ct_hpcp_top.v`:
- 18 per-counter enables (mcycle_en, minstret_en, mhpmcnt3..mhpmcnt18)
- 4 L2 counter enables (l2cnt_{ra,rm,wa,wm}_cnt_en)
- the exported global enable hpcp_xx_cnt_en
flaviens
force-pushed
the
hpcp-priv-gate-fix
branch
from
25b226f to
dd22d9d
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes a one-cycle lag in how the PMU’s per-counter enables react to privilege-mode changes by gating enables directly on the combinational cnt_mode_dis_pre (instead of the registered cnt_mode_dis). This ensures counters are silenced/enabled on the correct transition cycle when pmdm/pmds/pmdu disable counting in M/S/U mode.
Changes:
- Drive all per-counter enables (
mcycle,minstret,mhpmcnt3..18) from!cnt_mode_dis_preinstead of!cnt_mode_dis. - Drive the L2 counter enables (
l2cnt_{ra,rm,wa,wm}_cnt_en) from!cnt_mode_dis_preinstead of!cnt_mode_dis. - Drive the exported global enable
hpcp_xx_cnt_enfrom!cnt_mode_dis_preinstead of!cnt_mode_dis.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Hi!
The
pmdm/pmds/pmdubits silence the hardware perf counters in M, S, and U mode respectively. Today every per-counter enable inct_hpcp_top.vgates on the registeredcnt_mode_dis, which lags the combinationalcnt_mode_dis_preby one cycle on every privilege-mode change. Each transition into a disabled mode counts one extra event that should have been silenced, and each transition out misses one event that should have been counted.Drive the per-counter enables and the exported
hpcp_xx_cnt_enfromcnt_mode_dis_predirectly. The clock-gating logic at line 1086 already uses(cnt_mode_dis_pre ^ cnt_mode_dis)to keep the block clocked across the transition cycle, so removing the one-cycle lag from the counter enables does not change clock activity. The registeredcnt_mode_disis left in place because that XOR depends on it.Affected sites in
C910_RTL_FACTORY/gen_rtl/pmu/rtl/ct_hpcp_top.v:Thanks!
Flavien