From 8ec0c6f18ec22c00de7538e7dccc648e7864aa9e Mon Sep 17 00:00:00 2001 From: Pascal Nasahl Date: Wed, 17 Jan 2024 12:59:59 +0100 Subject: [PATCH] [rtl] Harden lockstep enable against FI Currently, the dual-core lockstep FI mitigation is enabled/disabled using a single bit. For transient bit-flips, this is not problematic, as one bit-flip into this signal and one bit into the Ibex is required to threaten the security of the system. However, a permanent stuck-at-0 fault could disable the lockstep completely by targeting this signal. Then, only a single, additional fault (transient or permanent) is required. This PR enhances the FI resilience of the Ibex lockstep by encoding this single bit into a ibex_mubi_t signal, i.e., a 4-bit multi-bit signal. Signed-off-by: Pascal Nasahl --- dv/uvm/core_ibex/ibex_dv.f | 1 + dv/uvm/core_ibex/tb/core_ibex_tb_top.sv | 7 +++ ibex_top.core | 1 + rtl/ibex_core.sv | 4 ++ rtl/ibex_lockstep.sv | 75 +++++++++++++++++-------- rtl/ibex_pkg.sv | 3 +- 6 files changed, 67 insertions(+), 24 deletions(-) diff --git a/dv/uvm/core_ibex/ibex_dv.f b/dv/uvm/core_ibex/ibex_dv.f index f6c54794bd..5bd9930375 100644 --- a/dv/uvm/core_ibex/ibex_dv.f +++ b/dv/uvm/core_ibex/ibex_dv.f @@ -21,6 +21,7 @@ ${PRJ_DIR}/dv/uvm/core_ibex/common/prim/prim_pkg.sv ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_assert.sv ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_util_pkg.sv +${LOWRISC_IP_DIR}/ip/prim/rtl/prim_count.sv ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_pkg.sv ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_22_16_dec.sv ${LOWRISC_IP_DIR}/ip/prim/rtl/prim_secded_22_16_enc.sv diff --git a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv index 4e75848972..a0e88710a7 100644 --- a/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv +++ b/dv/uvm/core_ibex/tb/core_ibex_tb_top.sv @@ -331,6 +331,13 @@ module core_ibex_tb_top; run_test(); end + // Manually set unused_assert_connected = 1 to disable the AssertConnected_A assertion for + // prim_count in case lockstep (set by SecureIbex) is enabled. If not disabled, DV fails. + if (SecureIbex) begin : gen_disable_count_check + assign dut.u_ibex_top.gen_lockstep.u_ibex_lockstep.u_rst_shadow_cnt. + unused_assert_connected = 1; + end + // Disable the assertion for onhot check in case WrenCheck (set by SecureIbex) is enabled. if (SecureIbex) begin : gen_disable_onehot_check assign dut.u_ibex_top.gen_regfile_ff.register_file_i.gen_wren_check.u_prim_onehot_check. diff --git a/ibex_top.core b/ibex_top.core index e8e5d011e9..38a279dfb7 100644 --- a/ibex_top.core +++ b/ibex_top.core @@ -13,6 +13,7 @@ filesets: - lowrisc:prim:and2 - lowrisc:prim:buf - lowrisc:prim:clock_mux2 + - lowrisc:prim:count - lowrisc:prim:flop - lowrisc:prim:ram_1p_scr - lowrisc:prim:onehot_check diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv index 67548fdbc2..a175efbb94 100644 --- a/rtl/ibex_core.sv +++ b/rtl/ibex_core.sv @@ -46,7 +46,11 @@ module ibex_core import ibex_pkg::*; #( ) ( // Clock and Reset input logic clk_i, + // Internally generated resets in ibex_lockstep cause IMPERFECTSCH warnings. + // TODO: Remove when upgrading Verilator #2134. + /* verilator lint_off IMPERFECTSCH */ input logic rst_ni, + /* verilator lint_on IMPERFECTSCH */ input logic [31:0] hart_id_i, input logic [31:0] boot_addr_i, diff --git a/rtl/ibex_lockstep.sv b/rtl/ibex_lockstep.sv index 7778ff74d0..b68128cb1e 100644 --- a/rtl/ibex_lockstep.sv +++ b/rtl/ibex_lockstep.sv @@ -120,33 +120,50 @@ module ibex_lockstep import ibex_pkg::*; #( // - The reset of the shadow core is synchronously released. // The comparison is started in the following clock cycle. - logic [LockstepOffsetW-1:0] rst_shadow_cnt_d, rst_shadow_cnt_q, rst_shadow_cnt_incr; - // Internally generated resets cause IMPERFECTSCH warnings - /* verilator lint_off IMPERFECTSCH */ - logic rst_shadow_set_d, rst_shadow_set_q; - logic rst_shadow_n, enable_cmp_q; - /* verilator lint_on IMPERFECTSCH */ + logic [LockstepOffsetW-1:0] rst_shadow_cnt; + logic rst_shadow_cnt_err; + ibex_mubi_t rst_shadow_set_d, rst_shadow_set_q; + logic rst_shadow_n, rst_shadow_set_single_bit; + ibex_mubi_t enable_cmp_d, enable_cmp_q; + + // This counter primitive starts counting to LockstepOffset after a system + // reset. The counter value saturates at LockstepOffset. + prim_count #( + .Width (LockstepOffsetW ), + .ResetValue (LockstepOffsetW'(1'b0) ) + ) u_rst_shadow_cnt ( + .clk_i (clk_i ), + .rst_ni (rst_ni ), + .clr_i (1'b0 ), + .set_i (1'b0 ), + .set_cnt_i ('0 ), + .incr_en_i (1'b1 ), + .decr_en_i (1'b0 ), + .step_i (LockstepOffsetW'(1'b1) ), + .cnt_o (rst_shadow_cnt ), + .cnt_next_o ( ), + .err_o (rst_shadow_cnt_err ) + ); - assign rst_shadow_cnt_incr = rst_shadow_cnt_q + 1'b1; + // When the LockstepOffset counter value is reached, activate the lockstep + // comparison. We do not explicitly check whether rst_shadow_set_q forms a valid + // multibit signal as this value is implicitly checked by the enable_cmp + // comparison below. + assign rst_shadow_set_d = + (rst_shadow_cnt >= LockstepOffsetW'(LockstepOffset - 1)) ? IbexMuBiOn : IbexMuBiOff; - assign rst_shadow_set_d = (rst_shadow_cnt_q == LockstepOffsetW'(LockstepOffset - 1)); - assign rst_shadow_cnt_d = rst_shadow_set_d ? rst_shadow_cnt_q : rst_shadow_cnt_incr; + // Enable lockstep comparison. + assign enable_cmp_d = rst_shadow_set_q; - always_ff @(posedge clk_i or negedge rst_ni) begin - if (!rst_ni) begin - rst_shadow_cnt_q <= '0; - enable_cmp_q <= '0; - end else begin - rst_shadow_cnt_q <= rst_shadow_cnt_d; - enable_cmp_q <= rst_shadow_set_q; - end - end + // This assignment is needed in order to avoid "Warning-IMPERFECTSCH" messages. + // TODO: Remove when updating Verilator #2134. + assign rst_shadow_set_single_bit = rst_shadow_set_q[0]; // The primitives below are used to place size-only constraints in order to prevent // synthesis optimizations and preserve anchor points for constraining backend tools. prim_flop #( - .Width(1), - .ResetValue(1'b0) + .Width(IbexMuBiWidth), + .ResetValue(IbexMuBiOff) ) u_prim_rst_shadow_set_flop ( .clk_i (clk_i), .rst_ni(rst_ni), @@ -154,10 +171,20 @@ module ibex_lockstep import ibex_pkg::*; #( .q_o (rst_shadow_set_q) ); + prim_flop #( + .Width(IbexMuBiWidth), + .ResetValue(IbexMuBiOff) + ) u_prim_enable_cmp_flop ( + .clk_i (clk_i), + .rst_ni(rst_ni), + .d_i (enable_cmp_d), + .q_o (enable_cmp_q) + ); + prim_clock_mux2 #( .NoFpgaBufG(1'b1) ) u_prim_rst_shadow_n_mux2 ( - .clk0_i(rst_shadow_set_q), + .clk0_i(rst_shadow_set_single_bit), .clk1_i(scan_rst_ni), .sel_i (test_en_i), .clk_o (rst_shadow_n) @@ -458,8 +485,10 @@ module ibex_lockstep import ibex_pkg::*; #( logic outputs_mismatch; - assign outputs_mismatch = enable_cmp_q & (shadow_outputs_q != core_outputs_q[0]); - assign alert_major_internal_o = outputs_mismatch | shadow_alert_major_internal; + assign outputs_mismatch = + (enable_cmp_q != IbexMuBiOff) & (shadow_outputs_q != core_outputs_q[0]); + assign alert_major_internal_o + = outputs_mismatch | shadow_alert_major_internal | rst_shadow_cnt_err; assign alert_major_bus_o = shadow_alert_major_bus; assign alert_minor_o = shadow_alert_minor; diff --git a/rtl/ibex_pkg.sv b/rtl/ibex_pkg.sv index 9a3c7faa90..bf379ad029 100644 --- a/rtl/ibex_pkg.sv +++ b/rtl/ibex_pkg.sv @@ -655,7 +655,8 @@ package ibex_pkg; // Mult-bit signal used for security hardening. For non-secure implementation all bits other than // the bottom bit are ignored. - typedef logic [3:0] ibex_mubi_t; + parameter int IbexMuBiWidth = 4; + typedef logic [IbexMuBiWidth-1:0] ibex_mubi_t; // Note that if adjusting these parameters it is assumed the bottom bit is set for On and unset // for Off. This allows the use of IbexMuBiOn/IbexMuBiOff to work for both secure and non-secure