[lc_ctrl/top_earlgrey] Disable RAW unlock feature and additional DV checks #21320

msfschaffner · 2024-02-13T00:21:02Z

This disables the volatile unlock feature in top_earlgrey for the production stepping and enhances the block-level DV environment to test both lc_ctrl variants. The PR also parameterizes the corresponding top-level test so that the expectation whether the mechanism is supported or not can be indicated via a plusarg.

This fixes part of #18250.

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]>

msfschaffner · 2024-02-14T00:10:22Z

hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_errors_vseq.sv

-        cfg.set_test_phase(LcCtrlBadNextState);
-        // wait at least two clks for scb to finish checking lc outputs
-        cfg.clk_rst_vif.wait_clks($urandom_range(10, 2));
-      end


This looks like a big change, but what it really does is move this else leg towards the end of the test, so that either we perform a transition and do all the checking, or we skip the transition and go straight to the LcCtrlBadNextState phase.

msfschaffner · 2024-02-14T00:18:44Z

hw/top_earlgrey/dv/env/seq_lib/chip_sw_lc_volatile_raw_unlock_vseq.sv

-      // In RAW state the ROM should halt as RomExecEn is not set yet.
-      `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInBootRomHalt)
+    // If we're expecting the feature to be enabled, the device should boot.
+    if (exp_volatile_raw_unlock_en) begin


no change below, just made the rest of the test conditional on exp_volatile_raw_unlock_en

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 · 2024-02-14T23:56:02Z

sw/host/opentitantool/src/command/lc.rs

@@ -478,6 +478,9 @@ impl CommandDispatch for VolatileRawUnlock {
            /*use_external_clk=*/ true,
            /*post_transition_tap=*/ JtagTap::LcTap,
            &self.jtag_params,
+            // whether we expect the RAW unlock feature to be present or not.
+            // on prod silicon this should be disabled.
+            false,


@timothytrippel can you take a look at this here?
we do not expect this to work on production silicon, and we need to test this because if it were supported, that would be a bad loophole.

We may want to feed in some global target configuration at some point so that this remains compatible with multiple top-levels/targets.

yep this looks good to keep the testing path open. Thanks @msfschaffner !

timothytrippel · 2024-02-15T00:19:54Z

sw/host/opentitantool/src/command/lc.rs

@@ -478,6 +478,9 @@ impl CommandDispatch for VolatileRawUnlock {
            /*use_external_clk=*/ true,
            /*post_transition_tap=*/ JtagTap::LcTap,
            &self.jtag_params,
+            // whether we expect the RAW unlock feature to be present or not.
+            // on prod silicon this should be disabled.
+            false,


yep this looks good to keep the testing path open. Thanks @msfschaffner !

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]>

vogelpi

Thanks for the PR and thanks for splitting it up into individual, clearly separated commits. This LGTM.

After inspecting fda744c (the commit which introduced the RAW unlock feature) I am convinced that disabling done here is done in a robust / secure way. It looks so simple because it changes just a single parameter :-)

msfschaffner · 2024-02-15T21:24:48Z

Thanks for the reviews. Due to increased CI latency, I will close this PR in favor of #21372 which adds just one commit to this chain that makes the life cycle key manager diversification constants unique per life cycle state.

This was referenced Feb 13, 2024

[lc_ctrl] Study prod netlist when SecVolatileRawUnlockEn set to 0 to verify logic is removed #18250

Closed

[lc_ctrl] D2(S) Signoff #20979

Closed

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 11 times, most recently from 55dc53f to ef8c8ea Compare February 13, 2024 03:19

msfschaffner requested review from andreaskurth and vogelpi February 13, 2024 03:20

[top_earlgrey] Disable RAW unlock feature

db433b3

This addresses the design side of lowRISC#18250 Signed-off-by: Michael Schaffner <[email protected]>

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 6 times, most recently from 5761280 to bb144fe Compare February 13, 2024 23:47

msfschaffner self-assigned this Feb 14, 2024

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 2 times, most recently from b544a9f to 3508d05 Compare February 14, 2024 00:05

msfschaffner added 3 commits February 13, 2024 16:06

[lc_ctrl/dv] Add build modes with volatile unlock enabled and disabled

8b280c9

This addresses the block-level DV parts of lowRISC#18250. Signed-off-by: Michael Schaffner <[email protected]>

[cip/dv] Increase timeout when waiting for access

4c59f99

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]>

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch from 3508d05 to 06055dc Compare February 14, 2024 00:07

msfschaffner commented Feb 14, 2024

View reviewed changes

msfschaffner requested a review from a-will February 14, 2024 00:17

msfschaffner commented Feb 14, 2024

View reviewed changes

msfschaffner marked this pull request as ready for review February 14, 2024 00:19

msfschaffner requested a review from a team as a code owner February 14, 2024 00:19

msfschaffner requested review from rswarbrick and jdonjdon and removed request for a team February 14, 2024 00:19

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 2 times, most recently from 7575851 to 2590c33 Compare February 14, 2024 00:38

a-will approved these changes Feb 14, 2024

View reviewed changes

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 3 times, most recently from c47ddbe to 8087405 Compare February 14, 2024 01:23

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch from 8087405 to 9c78061 Compare February 14, 2024 01:36

msfschaffner mentioned this pull request Feb 14, 2024

[lc_ctrl] Use unique diversification values #21372

Merged

msfschaffner requested a review from a team as a code owner February 14, 2024 23:54

msfschaffner requested review from HU90m and removed request for a team February 14, 2024 23:54

msfschaffner commented Feb 14, 2024

View reviewed changes

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch from 893b593 to 639a1a0 Compare February 14, 2024 23:58

timothytrippel approved these changes Feb 15, 2024

View reviewed changes

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch 3 times, most recently from ef0ef2f to f8101a1 Compare February 15, 2024 07:02

[ottool] Support chips with volatile RAW unlock disabled

db8f9b8

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]>

msfschaffner force-pushed the lc-ctrl-volatile-unlock branch from f8101a1 to db8f9b8 Compare February 15, 2024 17:23

vogelpi approved these changes Feb 15, 2024

View reviewed changes

msfschaffner closed this Feb 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[lc_ctrl/top_earlgrey] Disable RAW unlock feature and additional DV checks #21320

[lc_ctrl/top_earlgrey] Disable RAW unlock feature and additional DV checks #21320

msfschaffner commented Feb 13, 2024 •

edited

Loading

msfschaffner Feb 14, 2024 •

edited

Loading

msfschaffner Feb 14, 2024

msfschaffner Feb 14, 2024 •

edited

Loading

timothytrippel Feb 15, 2024

timothytrippel Feb 15, 2024

vogelpi left a comment

msfschaffner commented Feb 15, 2024

[lc_ctrl/top_earlgrey] Disable RAW unlock feature and additional DV checks #21320

[lc_ctrl/top_earlgrey] Disable RAW unlock feature and additional DV checks #21320

Conversation

msfschaffner commented Feb 13, 2024 • edited Loading

msfschaffner Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

msfschaffner Feb 14, 2024

Choose a reason for hiding this comment

msfschaffner Feb 14, 2024 • edited Loading

Choose a reason for hiding this comment

timothytrippel Feb 15, 2024

Choose a reason for hiding this comment

timothytrippel Feb 15, 2024

Choose a reason for hiding this comment

vogelpi left a comment

Choose a reason for hiding this comment

msfschaffner commented Feb 15, 2024

msfschaffner commented Feb 13, 2024 •

edited

Loading

msfschaffner Feb 14, 2024 •

edited

Loading

msfschaffner Feb 14, 2024 •

edited

Loading