-
Notifications
You must be signed in to change notification settings - Fork 746
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
[sram_ctrl,dv] Add readback_regwen test #24596
Conversation
89e66a1
to
efeba07
Compare
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.
A couple of nitty comments. The CI failure is because you need to regenerate some stuff from sram_ctrl.hjson
(an easy fix!)
b6c0326
to
c680d6e
Compare
Thanks for your review, @rswarbrick. I've addressed your comments. |
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.
Thanks for the tweaks: this looks good to me.
CHANGE AUTHORIZED: hw/ip/sram_ctrl/data/sram_ctrl.hjson (The change here is to add a description of a countermeasure, so there is no potential to break the design) |
CHANGE AUTHORIZED: hw/ip/sram_ctrl/data/sram_ctrl.hjson (No functional change, only adds a countermeasure description.) |
To close a coverage gap, this commit enhances the regwen_vseq to also cover the readback_regwen register. To reflect this change, the `sec_cm_readback_config_regwen` label is added. No functional RTL change. Now, EXPRESSION (readback_we & readback_regwen_qs) gets full cond. coverage. Closes lowRISC#24358. Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
c680d6e
to
b2058de
Compare
CHANGE AUTHORIZED: hw/ip/sram_ctrl/data/sram_ctrl.hjson (No functional change, only adds a countermeasure description.) |
To close a coverage gap, this commit enhances the regwen_vseq to also cover the readback_regwen register.
Closes #24358.