From db433b33946858501b8bd46f9afeafeeef0a23f4 Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Mon, 12 Feb 2024 16:14:30 -0800 Subject: [PATCH 1/6] [top_earlgrey] Disable RAW unlock feature This addresses the design side of #18250 Signed-off-by: Michael Schaffner --- hw/top_earlgrey/rtl/top_pkg.sv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/top_earlgrey/rtl/top_pkg.sv b/hw/top_earlgrey/rtl/top_pkg.sv index 2e800ab053635..59e3c4210e4ad 100644 --- a/hw/top_earlgrey/rtl/top_pkg.sv +++ b/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 From 8b280c9f3f2322e63b72026cc10e67f62db0c630 Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Mon, 12 Feb 2024 16:15:43 -0800 Subject: [PATCH 2/6] [lc_ctrl/dv] Add build modes with volatile unlock enabled and disabled This addresses the block-level DV parts of #18250. Signed-off-by: Michael Schaffner --- hw/ip/lc_ctrl/dv/README.md | 4 ++-- hw/ip/lc_ctrl/dv/env/lc_ctrl_scoreboard.sv | 7 ++++++- .../lc_ctrl_volatile_unlock_smoke_vseq.sv | 16 +++++++++++++++- ...m_cfg.hjson => lc_ctrl_base_sim_cfg.hjson} | 17 ++++++++++++----- ...trl_volatile_unlock_disabled_sim_cfg.hjson | 19 +++++++++++++++++++ ...ctrl_volatile_unlock_enabled_sim_cfg.hjson | 19 +++++++++++++++++++ hw/ip/lc_ctrl/dv/tb.sv | 10 +--------- .../dv/top_earlgrey_sim_cfgs.hjson | 3 ++- 8 files changed, 76 insertions(+), 19 deletions(-) rename hw/ip/lc_ctrl/dv/{lc_ctrl_sim_cfg.hjson => lc_ctrl_base_sim_cfg.hjson} (95%) create mode 100644 hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim_cfg.hjson create mode 100644 hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_enabled_sim_cfg.hjson diff --git a/hw/ip/lc_ctrl/dv/README.md b/hw/ip/lc_ctrl/dv/README.md index 5c9fc05c66356..b71e1a81dbda8 100644 --- a/hw/ip/lc_ctrl/dv/README.md +++ b/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 diff --git a/hw/ip/lc_ctrl/dv/env/lc_ctrl_scoreboard.sv b/hw/ip/lc_ctrl/dv/env/lc_ctrl_scoreboard.sv index 7cdab8b6b50ee..f215b676592eb 100644 --- a/hw/ip/lc_ctrl/dv/env/lc_ctrl_scoreboard.sv +++ b/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. diff --git a/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_volatile_unlock_smoke_vseq.sv b/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_volatile_unlock_smoke_vseq.sv index e40f41f33081f..97fbd427493da 100644 --- a/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_volatile_unlock_smoke_vseq.sv +++ b/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); diff --git a/hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson b/hw/ip/lc_ctrl/dv/lc_ctrl_base_sim_cfg.hjson similarity index 95% rename from hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson rename to hw/ip/lc_ctrl/dv/lc_ctrl_base_sim_cfg.hjson index 740fc0059d759..acc497d747c6b 100644 --- a/hw/ip/lc_ctrl/dv/lc_ctrl_sim_cfg.hjson +++ b/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 } diff --git a/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim_cfg.hjson b/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_disabled_sim_cfg.hjson new file mode 100644 index 0000000000000..42ba62aedc872 --- /dev/null +++ b/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"] +} diff --git a/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_enabled_sim_cfg.hjson b/hw/ip/lc_ctrl/dv/lc_ctrl_volatile_unlock_enabled_sim_cfg.hjson new file mode 100644 index 0000000000000..82c249f73da95 --- /dev/null +++ b/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"] +} diff --git a/hw/ip/lc_ctrl/dv/tb.sv b/hw/ip/lc_ctrl/dv/tb.sv index 2fe596517f405..37bb0245cd448 100644 --- a/hw/ip/lc_ctrl/dv/tb.sv +++ b/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), diff --git a/hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson b/hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson index 73023d8a9489d..0602527e8a235 100644 --- a/hw/top_earlgrey/dv/top_earlgrey_sim_cfgs.hjson +++ b/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", From bb01ebd858ea1a9fde4049e0625bce84378940be Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Tue, 13 Feb 2024 14:21:44 -0800 Subject: [PATCH 3/6] [lc_ctrl/dv] Stop checking if no transition can be performed 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 --- .../dv/env/seq_lib/lc_ctrl_errors_vseq.sv | 117 +++++++++--------- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_errors_vseq.sv b/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_errors_vseq.sv index 865f1be80c757..c156b8fab8def 100644 --- a/hw/ip/lc_ctrl/dv/env/seq_lib/lc_ctrl_errors_vseq.sv +++ b/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(); From 4c59f993601e9fe51a5ac16efe934d0a06598bbd Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Tue, 13 Feb 2024 22:55:24 +0100 Subject: [PATCH 4/6] [cip/dv] Increase timeout when waiting for access 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 --- hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv b/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv index be90f174d7f35..c88de17bc3102 100644 --- a/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv +++ b/hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv @@ -746,10 +746,10 @@ class cip_base_vseq #( // Wait a random number of cycles (up to reset_delay_bound) before triggering the reset. cfg.clk_rst_vif.wait_clks(rand_reset_delay); - // If there is an outstanding access, wait up to 1000 more cycles to allow it to clear. If it + // If there is an outstanding access, wait up to 10000 more cycles to allow it to clear. If it // doesn't clear, something has gone wrong: we don't expect there to permanently be CSR // accesses. - for (int i = 0; i < 1000; i++) begin + for (int i = 0; i < 10000; i++) begin if (!has_outstanding_access()) break; cfg.clk_rst_vif.wait_clks(1); end From 9c780615a1ac0da95a320f5f48c71a945a7fb8b1 Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Tue, 13 Feb 2024 14:52:59 -0800 Subject: [PATCH 5/6] [top/dv] Make volatile RAW unlock test parameteric 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 --- .../data/ip/chip_lc_ctrl_testplan.hjson | 11 ++- hw/top_earlgrey/dv/chip_sim_cfg.hjson | 11 ++- .../dv/env/seq_lib/chip_sw_base_vseq.sv | 36 +++++-- .../chip_sw_lc_volatile_raw_unlock_vseq.sv | 94 +++++++++++-------- 4 files changed, 104 insertions(+), 48 deletions(-) diff --git a/hw/top_earlgrey/data/ip/chip_lc_ctrl_testplan.hjson b/hw/top_earlgrey/data/ip/chip_lc_ctrl_testplan.hjson index 5b8ed309d97ff..a37f8fa181d9e 100644 --- a/hw/top_earlgrey/data/ip/chip_lc_ctrl_testplan.hjson +++ b/hw/top_earlgrey/data/ip/chip_lc_ctrl_testplan.hjson @@ -297,12 +297,21 @@ - Pre-load OTP image with RAW lc_state. - Initiate the LC transition to test_unlocked0 state using the VOLATILE_RAW_UNLOCK mode of operation. + + If this top-level is expected to support VOLATILE_RAW_UNLOCK: + - As part of the transition to test_unlocked0, switch the TAP interface to rv_dm. - Enable ROM execution via rv_dm, and perform POR. - Initiate a second transition to test_unlocked0 using VOLATILE_RAW_UNLOCK. - Verify that the CPU is able to execute. + - Test ext_clk injection before enabling ROM execution. + + If this top-level is NOT expected to support VOLATILE_RAW_UNLOCK: - Test ext_clk injection before enabling ROM execution. + - Check that the VOLATILE_RAW_UNLOCK bit in the transition control register stays 0 + when it is programmed to 1. + - Check that the transition results in a token error (the real RAW unlock transition + expects the unhashed token instead of the hashed token supplied for volatile RAW unlock). ''' features: [ "LC_CTRL.STATE.RAW", diff --git a/hw/top_earlgrey/dv/chip_sim_cfg.hjson b/hw/top_earlgrey/dv/chip_sim_cfg.hjson index 379e254e6a68e..ebf4606cd2aa5 100644 --- a/hw/top_earlgrey/dv/chip_sim_cfg.hjson +++ b/hw/top_earlgrey/dv/chip_sim_cfg.hjson @@ -925,7 +925,11 @@ uvm_test_seq: chip_sw_lc_volatile_raw_unlock_vseq sw_images: ["//sw/device/tests/sim_dv:lc_ctrl_volatile_raw_unlock_test:1:new_rules"] en_run_modes: ["sw_test_mode_test_rom"] - run_opts: ["+use_otp_image=OtpTypeLcStRaw"] + run_opts: [ + "+use_otp_image=OtpTypeLcStRaw", + // We set this to zero if the volatile RAW unlock mechanism is expected + // to be disabled (this should be the case in production silicon). + "+exp_volatile_raw_unlock_en=0"] run_timeout_mins: 120 } { @@ -935,7 +939,10 @@ en_run_modes: ["sw_test_mode_test_rom"] run_opts: [ "+use_otp_image=OtpTypeLcStRaw", - "+chip_clock_source=ChipClockSourceExternal48Mhz"] + "+chip_clock_source=ChipClockSourceExternal48Mhz", + // We set this to zero if the volatile RAW unlock mechanism is expected + // to be disabled (this should be the case in production silicon). + "+exp_volatile_raw_unlock_en=0"] run_timeout_mins: 120 } { diff --git a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_base_vseq.sv b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_base_vseq.sv index e73670285f225..007c4fbc9cbb5 100644 --- a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_base_vseq.sv +++ b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_base_vseq.sv @@ -801,9 +801,18 @@ class chip_sw_base_vseq extends chip_base_vseq; cfg.m_jtag_riscv_agent_cfg.allow_errors = 0; endtask + virtual task wait_lc_token_error(bit allow_err = 1, int max_attempt = 5000); + cfg.m_jtag_riscv_agent_cfg.allow_errors = allow_err; + wait_lc_status(LcTokenError, max_attempt); + cfg.m_jtag_riscv_agent_cfg.allow_errors = 0; + endtask + // Use JTAG interface to transit LC_CTRL from RAW to TEST_UNLOCKED* states // using the VOLATILE_RAW_UNLOCK mode of operation. - virtual task jtag_lc_state_volatile_raw_unlock(chip_jtag_tap_e target_strap); + // If this operation is expected to fail due to the absence of the mechanism in HW, set the + // expect_success argument to 0. + virtual task jtag_lc_state_volatile_raw_unlock(chip_jtag_tap_e target_strap, + bit expect_success = 1); bit [TL_DW-1:0] current_lc_state; bit [TL_DW-1:0] transition_ctrl; bit use_ext_clk = 1'b0; @@ -840,8 +849,15 @@ class chip_sw_base_vseq extends chip_base_vseq; ral.lc_ctrl.transition_ctrl.get_offset(), p_sequencer.jtag_sequencer_h, transition_ctrl); - `DV_CHECK_FATAL(transition_ctrl & (1 << 1), {"VOLATILE_RAW_UNLOCK is not supported by this ", - "top level. Check the SecVolatileRawUnlockEn parameter configuration."}) + // In this case we expect the transition_ctrl bit to stay 0. + if (expect_success) begin + `DV_CHECK_FATAL(transition_ctrl & (1 << 1), {"VOLATILE_RAW_UNLOCK is not supported by this ", + "top level. Check the SecVolatileRawUnlockEn parameter configuration."}) + end else begin + `DV_CHECK_FATAL(!(transition_ctrl & (1 << 1)), {"VOLATILE_RAW_UNLOCK is not expected to be ", + "supported by this top-level. Check the SecVolatileRawUnlockEn parameter ", + "configuration."}) + end if (use_ext_clk) begin wait_lc_ext_clk_switched(); @@ -869,10 +885,16 @@ class chip_sw_base_vseq extends chip_base_vseq; 1); if (target_strap == JtagTapLc) begin - wait_lc_transition_successful(.max_attempt(max_attempt)); - jtag_riscv_agent_pkg::jtag_write_csr(ral.lc_ctrl.claim_transition_if.get_offset(), - p_sequencer.jtag_sequencer_h, - prim_mubi_pkg::MuBi8False); + if (expect_success) begin + wait_lc_transition_successful(.max_attempt(max_attempt)); + jtag_riscv_agent_pkg::jtag_write_csr(ral.lc_ctrl.claim_transition_if.get_offset(), + p_sequencer.jtag_sequencer_h, + prim_mubi_pkg::MuBi8False); + end else begin + // The hashed token should be invalid if volatile unlock is not supported, since the + // regular transition expects that the unhashed token is provided. + wait_lc_token_error(.max_attempt(max_attempt)); + end end else begin cfg.clk_rst_vif.wait_clks($urandom_range(10000, 20000)); end diff --git a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_lc_volatile_raw_unlock_vseq.sv b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_lc_volatile_raw_unlock_vseq.sv index dec13dd8f1cad..ca9e18e4775e8 100644 --- a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_lc_volatile_raw_unlock_vseq.sv +++ b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_lc_volatile_raw_unlock_vseq.sv @@ -11,8 +11,12 @@ class chip_sw_lc_volatile_raw_unlock_vseq extends chip_sw_base_vseq; // image. bit rom_prod_mode = 1'b0; + // Indicates whether we expect the volatile RAW unlock feature to be enabled in HW or not. + bit exp_volatile_raw_unlock_en = 1'b0; + virtual task pre_start(); void'($value$plusargs("rom_prod_mode=%0d", rom_prod_mode)); + void'($value$plusargs("exp_volatile_raw_unlock_en=%0d", exp_volatile_raw_unlock_en)); cfg.chip_vif.tap_straps_if.drive(JtagTapLc); super.pre_start(); endtask @@ -33,49 +37,63 @@ class chip_sw_lc_volatile_raw_unlock_vseq extends chip_sw_base_vseq; super.body(); - // Since super.body only does backdoor operatoin, + // Since super.body only does backdoor operation, // add wait for clock task before the test uses jtag polling task. wait_rom_check_done(); wait_lc_ready(1); - // VOLATILE_RAW_UNLOCK does not require a reset after completion. - // Applying a reset will move the device back to RAW state in this case. - jtag_lc_state_volatile_raw_unlock(JtagTapRvDm); - reset_jtag_tap(); - - if (rom_prod_mode) begin - // The production ROM is not instrumented to report `sw_test_status`, so - // we wait for the PC to advance before continuing with the test. The - // PC was taken from the ROM disassembly. - `DV_WAIT(cfg.chip_vif.probed_cpu_pc.pc_wb >= 8194) + // If we're expecting the feature to be enabled, the device should boot. + if (exp_volatile_raw_unlock_en) begin + // VOLATILE_RAW_UNLOCK does not require a reset after completion. + // Applying a reset will move the device back to RAW state in this case. + jtag_lc_state_volatile_raw_unlock(.target_strap(JtagTapRvDm), .expect_success(1)); + reset_jtag_tap(); + + if (rom_prod_mode) begin + // The production ROM is not instrumented to report `sw_test_status`, so + // we wait for the PC to advance before continuing with the test. The + // PC was taken from the ROM disassembly. + `DV_WAIT(cfg.chip_vif.probed_cpu_pc.pc_wb >= 8194) + end else begin + // In RAW state the ROM should halt as RomExecEn is not set yet. + `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInBootRomHalt) + end + + // Use the frontend interface to configure the RomExecEn OTP value. A + // reset is required to have otp_ctrl sample the new OTP value. + `uvm_info(`gfn, "Configuring RomExecEng", UVM_LOW) + jtag_dm_activation_seq.start(p_sequencer.jtag_sequencer_h); + `uvm_info(`gfn, $sformatf("rv_dm_activated: %0d", cfg.m_jtag_riscv_agent_cfg.rv_dm_activated), + UVM_LOW) + cfg.m_jtag_riscv_agent_cfg.is_rv_dm = 1; + jtag_otp_program32(otp_ctrl_reg_pkg::CreatorSwCfgRomExecEnOffset, 1); + + cfg.chip_vif.tap_straps_if.drive(JtagTapLc); + cfg.m_jtag_riscv_agent_cfg.is_rv_dm = 0; + apply_reset(); + reset_jtag_tap(); + + // Wait for `rom_ctrl` to complete the ROM check. This will give the dut + // enough time to configure the TAP interface before any JTAG agents send + // any commands. + wait_rom_check_done(); + + // Second VOLATILE_RAW_UNLOCK does not change the TAP interface to rv_dm + // so that we can check the completion status through that interface. + // After this, the rest of the test should proceed. + jtag_lc_state_volatile_raw_unlock(JtagTapLc); end else begin - // In RAW state the ROM should halt as RomExecEn is not set yet. - `DV_WAIT(cfg.sw_test_status_vif.sw_test_status == SwTestStatusInBootRomHalt) + // We expect this to fail in this case. + jtag_lc_state_volatile_raw_unlock(.target_strap(JtagTapLc), .expect_success(0)); end - - // Use the frontend interface to configure the RomExecEn OTP value. A - // reset is required to have otp_ctrl sample the new OTP value. - `uvm_info(`gfn, "Configuring RomExecEng", UVM_LOW) - jtag_dm_activation_seq.start(p_sequencer.jtag_sequencer_h); - `uvm_info(`gfn, $sformatf("rv_dm_activated: %0d", cfg.m_jtag_riscv_agent_cfg.rv_dm_activated), - UVM_LOW) - cfg.m_jtag_riscv_agent_cfg.is_rv_dm = 1; - jtag_otp_program32(otp_ctrl_reg_pkg::CreatorSwCfgRomExecEnOffset, 1); - - cfg.chip_vif.tap_straps_if.drive(JtagTapLc); - cfg.m_jtag_riscv_agent_cfg.is_rv_dm = 0; - apply_reset(); - reset_jtag_tap(); - - // Wait for `rom_ctrl` to complete the ROM check. This will give the dut - // enough time to configure the TAP interface before any JTAG agents send - // any commands. - wait_rom_check_done(); - - // Second VOLATILE_RAW_UNLOCK does not change the TAP interface to rv_dm - // so that we can check the completion status through that interface. - // After this, the rest of the test should proceed. - jtag_lc_state_volatile_raw_unlock(JtagTapLc); - endtask + + task post_start(); + // In case the volatile RAW unlock feature is not supported by HW, the transition is not going + // to succeed and SW will never be able to run. + if (!exp_volatile_raw_unlock_en) begin + override_test_status_and_finish(.passed(1)); + end + super.post_start(); + endtask : post_start endclass From db8f9b83a1eef6e063071cebd4cd9b3eb6ee853e Mon Sep 17 00:00:00 2001 From: Michael Schaffner Date: Wed, 14 Feb 2024 15:52:41 -0800 Subject: [PATCH 6/6] [ottool] Support chips with volatile RAW unlock disabled 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 --- .../src/test_utils/lc_transition.rs | 36 ++++++++++++++----- sw/host/opentitantool/src/command/lc.rs | 3 ++ .../manuf_cp_volatile_unlock_raw/src/main.rs | 6 ++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/sw/host/opentitanlib/src/test_utils/lc_transition.rs b/sw/host/opentitanlib/src/test_utils/lc_transition.rs index ed4d557b92ce0..8e7a885a8873f 100644 --- a/sw/host/opentitanlib/src/test_utils/lc_transition.rs +++ b/sw/host/opentitanlib/src/test_utils/lc_transition.rs @@ -31,6 +31,8 @@ pub enum LcTransitionError { FailedToClaimMutex, #[error("Volatile raw unlock is not supported on this chip.")] VolatileRawUnlockNotSupported, + #[error("Volatile raw unlock is unexpectedly supported on this chip.")] + VolatileRawUnlockSupported, #[error("LC transition target programming failed (target state: 0x{0:x}).")] TargetProgrammingFailed(u32), #[error("LC transition failed (status: 0x{0:x}).")] @@ -216,6 +218,12 @@ pub fn trigger_lc_transition( /// provided (a pre-requisite of the volatile operation. The device will NOT be reset into the /// new lifecycle state as TAP straps are sampled again on a successfull transition. However, /// the TAP can be switched from LC to RISCV on a successfull transition. +/// +/// If the feature is not present in HW we expect the transition to fail with +/// a token error since the token is invalid for a real RAW unlock +/// transition. Use the expect_raw_unlock_supported argument to indicate +/// whether we expect this transition to succeed or not. +#[allow(clippy::too_many_arguments)] pub fn trigger_volatile_raw_unlock<'t>( transport: &'t TransportWrapper, mut jtag: Box, @@ -224,6 +232,7 @@ pub fn trigger_volatile_raw_unlock<'t>( use_external_clk: bool, post_transition_tap: JtagTap, jtag_params: &JtagParams, + expect_raw_unlock_supported: bool, ) -> Result> { // Wait for the lc_ctrl to become initialized, claim the mutex, and program the target state // and token CSRs. @@ -237,8 +246,11 @@ pub fn trigger_volatile_raw_unlock<'t>( jtag.write_lc_ctrl_reg(&LcCtrlReg::TransitionCtrl, ctrl.bits())?; // Read back the volatile raw unlock bit to see if the feature is supported in the silicon. - if jtag.read_lc_ctrl_reg(&LcCtrlReg::TransitionCtrl)? < 2u32 { + let read = jtag.read_lc_ctrl_reg(&LcCtrlReg::TransitionCtrl)?; + if read < 2u32 && expect_raw_unlock_supported { return Err(LcTransitionError::VolatileRawUnlockNotSupported.into()); + } else if read >= 2u32 && !expect_raw_unlock_supported { + return Err(LcTransitionError::VolatileRawUnlockSupported.into()); } // Select the requested JTAG TAP to connect to post-transition. @@ -258,13 +270,21 @@ pub fn trigger_volatile_raw_unlock<'t>( jtag = transport.jtag(jtag_params)?.connect(JtagTap::RiscvTap)?; } - wait_for_status( - &mut *jtag, - Duration::from_secs(3), - LcCtrlStatus::TRANSITION_SUCCESSFUL, - ) - .context("failed waiting for TRANSITION_SUCCESSFUL status.")?; - + if expect_raw_unlock_supported { + wait_for_status( + &mut *jtag, + Duration::from_secs(3), + LcCtrlStatus::TRANSITION_SUCCESSFUL, + ) + .context("failed waiting for TRANSITION_SUCCESSFUL status.")?; + } else { + let mut status = LcCtrlStatus::INITIALIZED | LcCtrlStatus::TOKEN_ERROR; + if use_external_clk { + status |= LcCtrlStatus::EXT_CLOCK_SWITCHED; + } + wait_for_status(&mut *jtag, Duration::from_secs(3), status) + .context("failed waiting for TOKEN_ERROR status.")?; + } Ok(jtag) } diff --git a/sw/host/opentitantool/src/command/lc.rs b/sw/host/opentitantool/src/command/lc.rs index 14fa37509a1b4..ccdf50619d102 100644 --- a/sw/host/opentitantool/src/command/lc.rs +++ b/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, )?; jtag = self diff --git a/sw/host/tests/manuf/manuf_cp_volatile_unlock_raw/src/main.rs b/sw/host/tests/manuf/manuf_cp_volatile_unlock_raw/src/main.rs index bc44c027b4e56..8c52ac43f0d14 100644 --- a/sw/host/tests/manuf/manuf_cp_volatile_unlock_raw/src/main.rs +++ b/sw/host/tests/manuf/manuf_cp_volatile_unlock_raw/src/main.rs @@ -57,6 +57,9 @@ fn volatile_raw_unlock_with_reconnection_to_lc_tap( /*use_external_clk=*/ true, JtagTap::LcTap, &opts.init.jtag_params, + // whether we expect the RAW unlock feature to be present or not. + // on prod silicon this should be disabled. + false, ) .context("failed to transition to TEST_UNLOCKED0")?; @@ -102,6 +105,9 @@ fn volatile_raw_unlock_with_reconnection_to_rv_tap( /*use_external_clk=*/ true, JtagTap::RiscvTap, &opts.init.jtag_params, + // whether we expect the RAW unlock feature to be present or not. + // on prod silicon this should be disabled. + false, ) .context("failed to transition to TEST_UNLOCKED0")?;