[lc_ctrl] Use unique diversification values #21372
Merged
Conversation
This file contains 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
msfschaffner
requested review from
a-will,
andreaskurth,
moidx,
gdessouky and
vogelpi
msfschaffner
requested review from
marnovandermaas
and removed request for
a team
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
2 times, most recently
from
6948d41 to
c3140fd
Compare
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
c3140fd to
010f7d4
Compare
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
010f7d4 to
3345df8
Compare
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
5 times, most recently
from
7d17c8d to
09c1060
Compare
a-will
approved these changes
Feb 14, 2024
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
4 times, most recently
from
0f8ec7d to
a191e1c
Compare
This addresses the design side of lowRISC#18250 Signed-off-by: Michael Schaffner <[email protected]>
This addresses the block-level DV parts of lowRISC#18250. Signed-off-by: Michael Schaffner <[email protected]>
When we are in RMA or PROD_END state, we can't inject a transition error since the only valid transition is into SCRAP (and that transition is always allowed, no matter the transition counter). While we make sure to not initialize the DUT with RMA or PROD_END if we intend to inject such an error, it can happen that we end up in one of these states when running multiple transactions back-to-back. In those cases we need to catch the corner case and make sure we skip the life cycle transition attempt. Signed-off-by: Michael Schaffner <[email protected]>
Some CSR accesses via JTAG take a long time, hence the timeout in wait_to_issue_reset() needs to be higher so that the function does not error out unexpectedly. Signed-off-by: Michael Schaffner <[email protected]>
Besides being able to test the volatile RAW unlock mechanism, we should also be able to test whether the case where that mechanism has been disabled in HW. This patch adds a plusarg that allows to parameterize the test with our expectation. In production silicon this mechanism should be disabled, for example. Signed-off-by: Michael Schaffner <[email protected]>
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
a191e1c to
a9f3cf5
Compare
gdessouky
approved these changes
Feb 15, 2024
| parameter lc_keymgr_div_t RndCnstLcKeymgrDivProduction = LcKeymgrDivWidth'(3), | ||
| parameter lc_keymgr_div_t RndCnstLcKeymgrDivRma = LcKeymgrDivWidth'(4), | ||
| parameter lc_token_mux_t RndCnstInvalidTokens = {TokenMuxBits{1'b1}}, | ||
| parameter bit SecVolatileRawUnlockEn = 0 |
There was a problem hiding this comment.
I guess the parameters for these diversification values are defined here and input into lc_ctrl_fsm and are just not in the package file, because they will be fed from the netlist constants?
There was a problem hiding this comment.
Yes, in order to use the random constant mechanism, these need to be defined as instantiation parameters so that we can feed them in from the top-level. This way, we can also make sure that different instances of the same block get different values. This is not important for LC_CTRL, but it is useful for other modules like ROM_CTRL or SRAM_CTRL that may be instantiated multiple times.
hw/ip/lc_ctrl/data/lc_ctrl.hjson
Outdated
| @@ -77,7 +77,13 @@ | |||
| randcount: "128", | |||
| randtype: "data", | |||
| } | |||
| { name: "RndCnstLcKeymgrDivTestDevRma", | |||
| { name: "RndCnstLcKeymgrDivTestUnlocked", | |||
| desc: "Compile-time random bits for lc state group diversification value", | |||
There was a problem hiding this comment.
Nit: it's intuitive from the name, but can probably also add the name of the lc state in the description of each of these.
There was a problem hiding this comment.
Yes good point, let me make those descriptions a bit more informative.
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
2 times, most recently
from
9f8b4c4 to
1a1c45b
Compare
vogelpi
approved these changes
Feb 15, 2024
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
1a1c45b to
9e2942c
Compare
msfschaffner
commented
Feb 15, 2024
| // Check for any error bits set - however, we exclude the status that | ||
| // we are looking for in this comparison, since otherwise this | ||
| // function would just bail. | ||
| if polled_status.intersects(LcCtrlStatus::ERRORS & !status) { |
There was a problem hiding this comment.
@timothytrippel FYI - this is needed so that we can wait for specific error statuses.
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
9e2942c to
28b959d
Compare
This updates the manuf_cp_volatile_unlock_raw test so that we can indicate whether we expect the target to support volatile RAW unlock or not. Signed-off-by: Michael Schaffner <[email protected]>
This makes the diversification values fed to the keymgr unique for each group of life cycle states. Fixes lowRISC#14047 Signed-off-by: Michael Schaffner <[email protected]>
msfschaffner
force-pushed
the
lc-ctrl-div-const
branch
from
28b959d to
138cc3d
Compare
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.
This makes the diversification values fed to the keymgr unique for each group of life cycle states.
Fixes #14047
This is dependent on #21320 - only the last commit is relevant. We merge all commits from #21320 with this PR however, since CI load is currently very high and that reduces turnaround time for the collection of changes. Note that #21320 has been approved separately already.