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

hw/ip/lc_ctrl/dv/README.md

@@ -101,9 +101,9 @@ It also updates the UVM register model.
 ## Building and running tests
 We are using our in-house developed [regression tool](../../../../util/dvsim/README.md) for building and running our tests and regressions.
 Please take a look at the link for detailed information on the usage, capabilities, features and known issues.
-Here's how to run a smoke test:
+Here's how to run a smoke test for the LC_CTRL variant with volatile RAW unlock disabled:
 ```console
-$ $REPO_TOP/util/dvsim/dvsim.py $REPO_TOP/hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson -i lc_ctrl_smoke
+$ $REPO_TOP/util/dvsim/dvsim.py $REPO_TOP/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim.hjson -i lc_ctrl_smoke
 ```
 
 ## Testplan

hw/ip/lc_ctrl/dv/env/lc_ctrl_scoreboard.sv

@@ -251,7 +251,6 @@ class lc_ctrl_scoreboard extends cip_base_scoreboard #(
     bit            write = item.is_write();
     uvm_reg_addr_t csr_addr = cfg.ral_models[ral_name].get_word_aligned_addr(item.a_addr);
     lc_outputs_t   exp = '{default: lc_ctrl_pkg::Off};
-
     bit            addr_phase_read = (!write && channel == AddrChannel);
     bit            addr_phase_write = (write && channel == AddrChannel);
     bit            data_phase_read = (!write && channel == DataChannel);
@@ -277,6 +276,12 @@ class lc_ctrl_scoreboard extends cip_base_scoreboard #(
               else                exp_clk_byp_req = lc_ctrl_pkg::Off;
             end
           end
+          // In case the volatile unlock feature is disabled, the register is hardwired to zero.
+          // Since the RAL models this as an RW register, we have to override that model here.
+          if (!`SEC_VOLATILE_RAW_UNLOCK_EN) begin
+            bit [DataWidth-1:0] mask = 'h2;
+            item.a_data &= ~mask;
+          end
         end
         default: begin
          // Do nothing.

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

@@ -355,75 +355,76 @@ class lc_ctrl_errors_vseq extends lc_ctrl_smoke_vseq;
         cfg.set_test_phase(LcCtrlWaitTransition);
         sw_transition_req(next_lc_state, token_val);
         cfg.set_test_phase(LcCtrlTransitionComplete);
-      end else begin
-        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
-      // verilog_format: on
 
-      if (cfg.err_inj.token_mux_ctrl_redun_err || cfg.err_inj.token_mux_digest_err) begin
-        // Allow the FSM to get to a terminal state
-        wait(cfg.lc_ctrl_vif.lc_ctrl_fsm_state inside {PostTransSt, InvalidSt, EscalateSt});
-        disable token_mux_ctrl_err_inject;
-        disable token_mux_digest_err_inject;
-      end
+        // verilog_format: on
 
-      // Allow volatile registers to settle
-      cfg.clk_rst_vif.wait_clks($urandom_range(15, 10));
+        if (cfg.err_inj.token_mux_ctrl_redun_err || cfg.err_inj.token_mux_digest_err) begin
+          // Allow the FSM to get to a terminal state
+          wait(cfg.lc_ctrl_vif.lc_ctrl_fsm_state inside {PostTransSt, InvalidSt, EscalateSt});
+          disable token_mux_ctrl_err_inject;
+          disable token_mux_digest_err_inject;
+        end
 
-      cfg.set_test_phase(LcCtrlReadState1);
-      // Check count and state before escalate is generated
-      // Skip this if we are injecting clock bypass error responses as the KMAC
-      // may or may not respond leaving the FSM stuck in TokenHashSt
-      // Also Token Mux select error injection leads to unpredicable results
-      if (!err_inj.clk_byp_error_rsp && !err_inj.token_mux_ctrl_redun_err) begin
-        rd_lc_state_and_cnt_csrs();
-      end else begin
-        `uvm_info(`gfn, "Skipped read of lc state & lc_count because of error injection",
-                  UVM_MEDIUM)
+        // Allow volatile registers to settle
         cfg.clk_rst_vif.wait_clks($urandom_range(15, 10));
-      end
 
-      // Allow escalate to be generated if we have received an alert
-      cfg.set_test_phase(LcCtrlEscalate);
+        cfg.set_test_phase(LcCtrlReadState1);
+        // Check count and state before escalate is generated
+        // Skip this if we are injecting clock bypass error responses as the KMAC
+        // may or may not respond leaving the FSM stuck in TokenHashSt
+        // Also Token Mux select error injection leads to unpredicable results
+        if (!err_inj.clk_byp_error_rsp && !err_inj.token_mux_ctrl_redun_err) begin
+          rd_lc_state_and_cnt_csrs();
+        end else begin
+          `uvm_info(`gfn, "Skipped read of lc state & lc_count because of error injection",
+                    UVM_MEDIUM)
+          cfg.clk_rst_vif.wait_clks($urandom_range(15, 10));
+        end
+
+        // Allow escalate to be generated if we have received an alert
+        cfg.set_test_phase(LcCtrlEscalate);
 
-      // Wait before re-checking lc_state to allow escalate to be accepted
-      cfg.clk_rst_vif.wait_clks($urandom_range(150, 100));
-      cfg.set_test_phase(LcCtrlReadState2);
-      // Check count and state after escalate is generated
-      // Skip if token mux select line error injection
-      if (!err_inj.token_mux_ctrl_redun_err) rd_lc_state_and_cnt_csrs();
+        // Wait before re-checking lc_state to allow escalate to be accepted
+        cfg.clk_rst_vif.wait_clks($urandom_range(150, 100));
+        cfg.set_test_phase(LcCtrlReadState2);
+        // Check count and state after escalate is generated
+        // Skip if token mux select line error injection
+        if (!err_inj.token_mux_ctrl_redun_err) rd_lc_state_and_cnt_csrs();
 
-      cfg.set_test_phase(LcCtrlPostTransition);
+        cfg.set_test_phase(LcCtrlPostTransition);
 
-      // Delay a few extra cycles if we are doing escalate injection
-      if (err_inj.security_escalation_err) begin
-        cfg.clk_rst_vif.wait_clks(security_escalation_err_inj_delay + 10);
-      end
+        // Delay a few extra cycles if we are doing escalate injection
+        if (err_inj.security_escalation_err) begin
+          cfg.clk_rst_vif.wait_clks(security_escalation_err_inj_delay + 10);
+        end
 
-      // Attempt a second transition post transition if enabled
-      // verilog_format: off - avoid bad formatting
-      if (err_inj.post_trans_err) begin
-        `uvm_info(`gfn, "Attempting second transition post transition", UVM_LOW)
-        `DV_CHECK_RANDOMIZE_FATAL(this)
-        // Clear all error injections except post_trans_err
-        err_inj = '{post_trans_err: 1, default: 0};
-        // SW transition request
-        if ((err_inj.state_err || valid_state_for_trans(lc_state)) &&
-            (err_inj.count_err || err_inj.transition_count_err || lc_cnt != LcCnt24)) begin
-          lc_ctrl_state_pkg::lc_token_t token_val = get_random_token();
-          randomize_next_lc_state(dec_lc_state(lc_state));
-          set_hashed_token();
-          set_otp_prog_rsp();
-          sw_transition_req(next_lc_state, token_val);
-        end else begin
-          // wait at least two clks for scb to finish checking lc outputs
-          cfg.clk_rst_vif.wait_clks($urandom_range(10, 2));
+        // Attempt a second transition post transition if enabled
+        // verilog_format: off - avoid bad formatting
+        if (err_inj.post_trans_err) begin
+          `uvm_info(`gfn, "Attempting second transition post transition", UVM_LOW)
+          `DV_CHECK_RANDOMIZE_FATAL(this)
+          // Clear all error injections except post_trans_err
+          err_inj = '{post_trans_err: 1, default: 0};
+          // SW transition request
+          if ((err_inj.state_err || valid_state_for_trans(lc_state)) &&
+              (err_inj.count_err || err_inj.transition_count_err || lc_cnt != LcCnt24)) begin
+            lc_ctrl_state_pkg::lc_token_t token_val = get_random_token();
+            randomize_next_lc_state(dec_lc_state(lc_state));
+            set_hashed_token();
+            set_otp_prog_rsp();
+            sw_transition_req(next_lc_state, token_val);
+          end else begin
+            // wait at least two clks for scb to finish checking lc outputs
+            cfg.clk_rst_vif.wait_clks($urandom_range(15, 6));
+          end
+          cfg.set_test_phase(LcCtrlPostTransTransitionComplete);
         end
-        cfg.set_test_phase(LcCtrlPostTransTransitionComplete);
+        // verilog_format: on
+      end else begin
+        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(15, 6));
       end
-      // verilog_format: on
 
       // Sample coverage if enabled
       sample_cov();

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

@@ -46,7 +46,21 @@ class lc_ctrl_volatile_unlock_smoke_vseq extends lc_ctrl_smoke_vseq;
       end
       csr_wr(ral.transition_cmd, 'h01);
 
-      if (next_state == DecLcStTestUnlocked0) begin
+      if (!`SEC_VOLATILE_RAW_UNLOCK_EN) begin
+        // We expect the VOLATILE_RAW_UNLOCK bit to stay at zero in this case.
+        cfg.clk_rst_vif.wait_clks(2);
+        csr_rd_check(.ptr(ral.transition_ctrl.volatile_raw_unlock), .compare_value(0));
+        cfg.clk_rst_vif.wait_clks(10);
+        // Since we're performing a real transition in this case with a hashed token, we should
+        // be getting a token error since a real transition expects an unhashed token.
+        csr_spinwait(.ptr(ral.status.token_error), .exp_data(1), .timeout_ns(10_000_000));
+        // The strap sampling override signal should be zero.
+        `DV_CHECK_EQ(cfg.lc_ctrl_vif.strap_en_override_o, 0);
+        if (cfg.en_cov) cov.volatile_raw_unlock_cg.sample(0);
+      end else if (next_state == DecLcStTestUnlocked0) begin
+        // We expect the VOLATILE_RAW_UNLOCK bit to go to one in this case.
+        cfg.clk_rst_vif.wait_clks(2);
+        csr_rd_check(.ptr(ral.transition_ctrl.volatile_raw_unlock), .compare_value(1));
         csr_spinwait(.ptr(ral.status.transition_successful), .exp_data(1), .timeout_ns(100_000));
         cfg.clk_rst_vif.wait_clks(10);
         `DV_CHECK_EQ(cfg.lc_ctrl_vif.strap_en_override_o, 1);

hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson → hw/ip/lc_ctrl/dv/lc_ctrl_base_sim_cfg.hjson

@@ -59,9 +59,16 @@
     }
   ]
 
-  // Add LC_CTRL specific exclusion files.
-  vcs_cov_excl_files: ["{proj_root}/hw/ip/lc_ctrl/dv/cov/lc_ctrl_cov_unr_excl.el",
-                       "{proj_root}/hw/ip/lc_ctrl/dv/cov/lc_ctrl_terminal_st_excl.el"]
+  build_modes: [
+    {
+      name: volatile_unlock_disabled
+      build_opts: ["+define+SEC_VOLATILE_RAW_UNLOCK_EN=0"]
+    }
+    {
+      name: volatile_unlock_enabled
+        build_opts: ["+define+SEC_VOLATILE_RAW_UNLOCK_EN=1"]
+    }
+  ]
 
   // Default UVM test and seq class name.
   uvm_test: lc_ctrl_base_test
@@ -157,14 +164,14 @@
     {
       name: lc_ctrl_jtag_access
       uvm_test_seq: lc_ctrl_jtag_access_vseq
-      run_opts: ["+en_scb=0", "+create_jtag_riscv_map=1"]
+      run_opts: ["+create_jtag_riscv_map=1"]
       reseed: 50
     }
 
     {
       name: lc_ctrl_jtag_priority
       uvm_test_seq: lc_ctrl_jtag_priority_vseq
-      run_opts: ["+en_scb=0", "+create_jtag_riscv_map=1"]
+      run_opts: ["+create_jtag_riscv_map=1"]
       reseed: 10
     }
 

hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim_cfg.hjson

@@ -0,0 +1,19 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// sim cfg file for the variant of LC_CTRL without volatile unlock
+{
+  // Name of the sim cfg variant
+  variant: volatile_unlock_disabled
+
+  // Import additional common sim cfg files.
+  import_cfgs: ["{proj_root}/hw/ip/lc_ctrl/dv/lc_ctrl_base_sim_cfg.hjson"]
+
+  // Enable this build mode for all tests
+  en_build_modes: ["volatile_unlock_disabled"]
+
+  // exclusion files
+  // TODO: redo UNR
+  vcs_cov_excl_files: ["{proj_root}/hw/ip/lc_ctrl/dv/cov/lc_ctrl_terminal_st_excl.el"]
+}

hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_enabled_sim_cfg.hjson

@@ -0,0 +1,19 @@
+// Copyright lowRISC contributors.
+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+// sim cfg file for the variant of LC_CTRL with volatile unlock
+{
+  // Name of the sim cfg variant
+  variant: volatile_unlock_enabled
+
+  // Import additional common sim cfg files.
+  import_cfgs: ["{proj_root}/hw/ip/lc_ctrl/dv/lc_ctrl_base_sim_cfg.hjson"]
+
+  // Enable this build mode for all tests
+  en_build_modes: ["volatile_unlock_enabled"]
+
+  // exclusion files
+  // TODO: redo UNR
+  vcs_cov_excl_files: ["{proj_root}/hw/ip/lc_ctrl/dv/cov/lc_ctrl_terminal_st_excl.el"]
+}

hw/ip/lc_ctrl/dv/tb.sv

@@ -26,14 +26,6 @@ module tb;
     LcKeymgrDivWidth'({(LcKeymgrDivWidth/8){8'h5a}});
   parameter lc_keymgr_div_t RndCnstLcKeymgrDivProduction =
     LcKeymgrDivWidth'({(LcKeymgrDivWidth/8){8'ha5}});
-  // ---------- VOLATILE_TEST_UNLOCKED CODE SECTION START ----------
-  // NOTE THAT THIS IS A FEATURE FOR TEST CHIPS ONLY TO MITIGATE
-  // THE RISK OF A BROKEN OTP MACRO. THIS WILL BE DISABLED VIA
-  // SecVolatileRawUnlockEn AT COMPILETIME FOR PRODUCTION DEVICES.
-  // ---------------------------------------------------------------
-  // ICEBOX(#18250): The SecVolatileRawUnlockEn configuration should be tested separately for PROD.
-  parameter bit SecVolatileRawUnlockEn = 1;
-  // ----------- VOLATILE_TEST_UNLOCKED CODE SECTION END -----------
 
   // macro includes
   `include "uvm_macros.svh"
@@ -108,7 +100,7 @@ module tb;
     .SiliconCreatorId(LcCtrlSiliconCreatorId[lc_ctrl_reg_pkg::SiliconCreatorIdWidth-1:0]),
     .ProductId(LcCtrlProductId[lc_ctrl_reg_pkg::ProductIdWidth-1:0]),
     .RevisionId(LcCtrlRevisionId[lc_ctrl_reg_pkg::RevisionIdWidth-1:0]),
-    .SecVolatileRawUnlockEn(SecVolatileRawUnlockEn)
+    .SecVolatileRawUnlockEn(`SEC_VOLATILE_RAW_UNLOCK_EN)
   ) dut (
     .clk_i (clk),
     .rst_ni(rst_n),

hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson

@@ -34,7 +34,8 @@
              "{proj_root}/hw/ip/keymgr/dv/keymgr_sim_cfg.hjson",
              "{proj_root}/hw/ip/kmac/dv/kmac_masked_sim_cfg.hjson",
              "{proj_root}/hw/ip/kmac/dv/kmac_unmasked_sim_cfg.hjson",
-             "{proj_root}/hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson",
+             "{proj_root}/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim_cfg.hjson",
+             "{proj_root}/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_enabled_sim_cfg.hjson",
              "{proj_root}/hw/ip/otbn/dv/uvm/otbn_sim_cfg.hjson",
              "{proj_root}/hw/ip/otp_ctrl/dv/otp_ctrl_sim_cfg.hjson",
              "{proj_root}/hw/ip/pattgen/dv/pattgen_sim_cfg.hjson",

hw/top_earlgrey/rtl/top_pkg.sv

@@ -17,6 +17,6 @@ localparam int TL_SZW=$clog2($clog2(TL_DBW)+1);
 // NOTE THAT THIS IS A FEATURE FOR TEST CHIPS ONLY TO MITIGATE
 // THE RISK OF A BROKEN OTP MACRO. THIS WILL BE DISABLED FOR
 // PRODUCTION DEVICES.
-localparam int SecVolatileRawUnlockEn = 1;
+localparam int SecVolatileRawUnlockEn = 0;
 
 endpackage