[rtl] Harden lockstep enable against FI

nasahlpa · vogelpi · commit 8ec0c6f18ec2 · 2024-01-23T09:14:45.000Z
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 &lt;nasahlpa@lowrisc.org&gt;
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -120,44 +120,71 @@ 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),
     .d_i   (rst_shadow_set_d),
     .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
@@ -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
