hpcp: gate counter enables on combinational cnt_mode_dis_pre#54
Open
flaviens wants to merge 1 commit into
Open
hpcp: gate counter enables on combinational cnt_mode_dis_pre#54flaviens wants to merge 1 commit into
flaviens wants to merge 1 commit into
Conversation
The per-counter enables in ct_hpcp_top.v (mcycle_en, minstret_en, mhpmcnt3_en..mhpmcnt18_en, the four L2 counter enables, and the exported hpcp_xx_cnt_en) gate on the registered cnt_mode_dis. That register lags the combinational cnt_mode_dis_pre by one cycle, and cnt_mode_dis_pre already reflects the new privilege mode and the pmdm/pmds/pmdu bits on a privilege-transition cycle. So the first cycle after a transition into a disabled mode still increments the counters: every entry into a disabled mode over-counts by one event and every exit under-counts by one. Drive the 23 enables from cnt_mode_dis_pre so the per-mode disable takes effect on the transition cycle. The registered cnt_mode_dis is retained for the clock-gating term at line 1086 (cnt_mode_dis_pre ^ cnt_mode_dis), which keeps the block clocked across the transition cycle and is unchanged. The change is one operand per line on 23 lines and adds no flop. Security relevance: a deployment that uses pmdm/pmds/pmdu to keep higher-privilege events out of a counter a lower-privilege reader can sample no longer fully seals those events across a privilege boundary, leaking about one bit per transition. A Yosys SAT miter and a Verilator reproducer are available on request.
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 per-counter enables in ct_hpcp_top.v (mcycle_en, minstret_en, mhpmcnt3_en..mhpmcnt18_en, the four L2 counter enables, and the exported hpcp_xx_cnt_en) gate on the registered cnt_mode_dis. That register lags the combinational cnt_mode_dis_pre by one cycle, and cnt_mode_dis_pre already reflects the new privilege mode and the pmdm/pmds/pmdu bits on a privilege-transition cycle. So the first cycle after a transition into a disabled mode still increments the counters: every entry into a disabled mode over-counts by one event and every exit under-counts by one.
Drive the 23 enables from cnt_mode_dis_pre so the per-mode disable takes effect on the transition cycle. The registered cnt_mode_dis is retained for the clock-gating term at line 1086 (cnt_mode_dis_pre ^ cnt_mode_dis), which keeps the block clocked across the transition cycle and is unchanged. The change is one operand per line on 23 lines and adds no flop.
Security relevance: a deployment that uses pmdm/pmds/pmdu to keep higher-privilege events out of a counter a lower-privilege reader can sample no longer fully seals those events across a privilege boundary, leaking about one bit per transition.