[otbn,rtl] Register local escalation signals for timing optimization

Signals contributing to the local controller escalation signal directly factor into the
insn_executing signal inside the otbn_controller.sv. This then drives various commit signals
as well as the DMEM request signal (lsu_load_req_o). By cutting these escalation signals,
these paths are shortened which results in better timing/area results.

Signed-off-by: Pascal Etterli <[email protected]>

Author: etterli

hw/ip/otbn/rtl/otbn_core.sv

@@ -113,7 +113,7 @@ module otbn_core
   logic [31:0]              insn_fetch_resp_data;
   logic                     insn_fetch_resp_clear;
   logic                     insn_fetch_err;
-  logic                     insn_addr_err;
+  logic                     insn_addr_err_d, insn_addr_err;
 
   rf_predec_bignum_t        rf_predec_bignum;
   alu_predec_bignum_t       alu_predec_bignum;
@@ -156,7 +156,7 @@ module otbn_core
   logic                     rf_base_rd_commit;
   logic                     rf_base_call_stack_sw_err;
   logic                     rf_base_call_stack_hw_err;
-  logic                     rf_base_intg_err;
+  logic                     rf_base_intg_err_d, rf_base_intg_err;
   logic                     rf_base_spurious_we_err;
   logic                     rf_base_sec_wipe_err;
 
@@ -175,7 +175,7 @@ module otbn_core
 
   logic [BaseIntgWidth-1:0] lsu_base_rdata;
   logic [ExtWLEN-1:0]       lsu_bignum_rdata;
-  logic                     lsu_rdata_err;
+  logic                     lsu_rdata_err_d, lsu_rdata_err;
 
   logic [WdrAw-1:0]   rf_bignum_wr_addr;
   logic [WdrAw-1:0]   rf_bignum_wr_addr_ctrl;
@@ -241,7 +241,7 @@ module otbn_core
   logic            urnd_advance;
   logic            urnd_advance_start_stop_control;
   logic [WLEN-1:0] urnd_data;
-  logic            urnd_all_zero;
+  logic            urnd_all_zero_d, urnd_all_zero;
 
   logic        controller_start;
 
@@ -281,7 +281,7 @@ module otbn_core
   logic start_stop_fatal_error;
   logic rf_bignum_predec_error, alu_bignum_predec_error, ispr_predec_error, mac_bignum_predec_error;
   logic controller_predec_error;
-  logic rd_predec_error, predec_error;
+  logic rd_predec_error, predec_error_d, predec_error;
 
   logic req_sec_wipe_urnd_keys_q;
 
@@ -361,7 +361,7 @@ module otbn_core
     .insn_fetch_resp_data_o (insn_fetch_resp_data),
     .insn_fetch_resp_clear_i(insn_fetch_resp_clear),
     .insn_fetch_err_o       (insn_fetch_err),
-    .insn_addr_err_o        (insn_addr_err),
+    .insn_addr_err_o        (insn_addr_err_d),
 
     .rf_predec_bignum_o       (rf_predec_bignum),
     .alu_predec_bignum_o      (alu_predec_bignum),
@@ -416,7 +416,7 @@ module otbn_core
                              rf_predec_bignum.rf_ren_b,
                              ispr_predec_bignum.ispr_rd_en} & ~insn_valid;
 
-  assign predec_error =
+  assign predec_error_d =
     ((alu_bignum_predec_error | mac_bignum_predec_error | controller_predec_error) & insn_valid) |
      rf_bignum_predec_error                                                                      |
      ispr_predec_error                                                                           |
@@ -589,10 +589,100 @@ module otbn_core
                                   controller_err_bits.bad_internal_state,
                                   controller_err_bits.reg_intg_violation};
 
-  logic non_controller_reg_intg_violation;
-  assign non_controller_reg_intg_violation =
-      |{alu_bignum_reg_intg_violation_err, mac_bignum_reg_intg_violation_err, rf_base_intg_err};
-
+  logic non_controller_reg_intg_violation_d, non_controller_reg_intg_violation;
+  assign non_controller_reg_intg_violation_d =
+      |{alu_bignum_reg_intg_violation_err, mac_bignum_reg_intg_violation_err, rf_base_intg_err_d};
+
+  ////////////////////////////////////////////////////////////////
+  // Register local escalation signals for timinig optimization //
+  ////////////////////////////////////////////////////////////////
+  // Signals contributing to the local controller escalation signal directly factor into the
+  // insn_executing signal inside the otbn_controller.sv. This then drives various commit signals
+  // as well as the DMEM request signal (lsu_load_req_o). By cutting these escalation signals,
+  // these paths are shortened which results in better timing/area results. Signals contributing to
+  // the start/stop escalation are also cut to synchronize the escalation such that transitioning
+  // into LOCKED and the start of wiping activities happen at the same time. The error bits are
+  // also updated delayed to have the same behaviour as without a cut.
+  //
+  // Registered signals:
+  // - urnd_all_zero:    Is high when the local PRNG is all zero and locks up.
+  //                     Potentially timing critical.
+  // - predec_error:     Generated by checking all the registered, predecoded signals with decoded
+  //                     signals and combining these.
+  //                     Potentially timing critical.
+  // - lsu_rdata_err:    DMEM ECC errors of data read from DMEM.
+  //                     Timing critical because source is in memory.
+  // - insn_addr_err:    Depends on insn_fetch_req_addr_o from the controller which factors in
+  //                     lots of combinatorial signals.
+  //                     Likely timing critical.
+  // - rf_base_intg_err: ECC errors triggered with in the base register file. The control signals
+  //                     for the base register file are not predecoded.
+  //                     So this is likely timing critical.
+  // - non_controller_reg_intg_violation: Based on alu_bignum_reg_intg_violation_err,
+  //                                      mac_bignum_reg_intg_violation_err and rf_base_intg_err.
+  //                                      The former two are generated by checking the ECC of the
+  //                                      MOD / ACC values. The checks depends on the actual
+  //                                      ALU / MAC datapath outputs.
+  //                                      **Highly** timing critical.
+  //
+  // Not registered signals:
+  // - escalate_en_i:           This must NOT be not cut. It stems from the system wide escalation
+  //                            control mechanism and such a global escalation must be as fast as
+  //                            possible.
+  // - start_stop_fatal_error:  The error signal of the start stop FSM. Based on EDN urnd ack
+  //                            signal and internal state checking. This error is not cut to
+  //                            synchronize the escalation of the controller and start/stop
+  //                            control.
+  // - controller_fatal_err:    The error signal of the controller towards the start/stop logic.
+  //                            This error is not cut to synchronize the escalation of the
+  //                            controller and start/stop control.
+  // - rf_base_spurious_we_err: This error is already registered at the source.
+  // - insn_fetch_err:          IMEM ECC errors, not computed on the IMEM output but on a flopped
+  //                            signal.
+  //
+  // Security considerations:
+  // With the registering, the reaction to escalation sources is delayed by one cycle. Any faulty
+  // instruction therefore still commits and updates registers or the memory. This does not affect
+  // security because:
+  // - While executing a program, the IMEM and DMEM are locked and cannot be read by Ibex.
+  // - They become readable once the execution finishes without a fatal error. In addition, only a
+  //   part of the DMEM is readable by Ibex.
+  // - If a fatal error occurs OTBN locks up and none of the memories become readable again.
+  // - SCA and FI combined attacks are out of scope. As of this, any leakage due to FI attacks is
+  //   considered unusable because the OpenTitan chip scraps itself when too many FI attacks were
+  //   detected.
+  // - When escalating, OTBN asserts locking_o and therefore any memory reads are killed
+  //   immediately. This handles the case where the final instruction is targeted. An error during
+  //   the final instruction does not prevent the OTBN of going into IDLE and thus the memories are
+  //   theoretically unlocked for one cycle. However, OTBN will escalate immediately and also kill
+  //   any read requests. The memories are therefore never readable except maybe some glitching
+  //   activities on the port. However, assuming a single fault per execution, when targeting the
+  //   last instruction, any program should have run correctly up to this point and thus should not
+  //   expose any secrets in the readable DMEM section. Additionally, converting the ECALL
+  //   instruction into a meaningful memory instruction is infeasible with only 1 fault. Also, a
+  //   BN.SID instruction takes two cycles to execute and therefore only a SW instruction could
+  //   be performed.
+
+  // TODO: Does this need a prim_flop or is a regular flop sufficient?
+  // We drop the _q suffix for the registered signals such that DV and tracing can stay the same
+  // whether there is a cut or not.
+  always_ff @(posedge clk_i or negedge rst_ni) begin
+    if (!rst_ni) begin
+      urnd_all_zero                     <= '0;
+      predec_error                      <= '0;
+      rf_base_intg_err                  <= '0;
+      lsu_rdata_err                     <= '0;
+      non_controller_reg_intg_violation <= '0;
+      insn_addr_err                     <= '0;
+    end else begin
+      urnd_all_zero                     <= urnd_all_zero_d;
+      predec_error                      <= predec_error_d;
+      rf_base_intg_err                  <= rf_base_intg_err_d;
+      lsu_rdata_err                     <= lsu_rdata_err_d;
+      non_controller_reg_intg_violation <= non_controller_reg_intg_violation_d;
+      insn_addr_err                     <= insn_addr_err_d;
+    end
+  end
 
   // Generate an err_bits output by combining errors from all the blocks in otbn_core
   assign err_bits_d = '{
@@ -633,11 +723,12 @@ module otbn_core
 
   // Pass an "escalation" signal down to the controller by ORing in error signals from the other
   // modules in otbn_core. Note that each error signal except escalate_en_i that appears here also
-  // appears somewhere in err_bits_o above (checked in ErrBitsIfControllerEscalate_A)
+  // appears somewhere in err_bits_o above (checked in ErrBitsIfControllerEscalate_A).
+  // The error rf_base_intg_err is already factored into non_controller_reg_intg_violation.
   assign controller_fatal_escalate_en =
       mubi4_or_hi(escalate_en_i,
                   mubi4_bool_to_mubi(|{start_stop_fatal_error, urnd_all_zero, predec_error,
-                                       rf_base_intg_err, rf_base_spurious_we_err, lsu_rdata_err,
+                                       rf_base_spurious_we_err, lsu_rdata_err,
                                        insn_fetch_err, non_controller_reg_intg_violation,
                                        insn_addr_err}));
 
@@ -684,7 +775,7 @@ module otbn_core
 
     .lsu_base_rdata_o  (lsu_base_rdata),
     .lsu_bignum_rdata_o(lsu_bignum_rdata),
-    .lsu_rdata_err_o   (lsu_rdata_err)
+    .lsu_rdata_err_o   (lsu_rdata_err_d)
   );
 
   // Base Instruction Subset =======================================================================
@@ -716,7 +807,7 @@ module otbn_core
 
     .call_stack_sw_err_o(rf_base_call_stack_sw_err),
     .call_stack_hw_err_o(rf_base_call_stack_hw_err),
-    .intg_err_o         (rf_base_intg_err),
+    .intg_err_o         (rf_base_intg_err_d),
     .spurious_we_err_o  (rf_base_spurious_we_err),
     .sec_wipe_err_o     (rf_base_sec_wipe_err)
   );
@@ -910,7 +1001,7 @@ module otbn_core
     .urnd_reseed_ack_o (urnd_reseed_ack),
     .urnd_advance_i    (urnd_advance),
     .urnd_data_o       (urnd_data),
-    .urnd_all_zero_o   (urnd_all_zero),
+    .urnd_all_zero_o   (urnd_all_zero_d),
 
     .edn_rnd_req_o,
     .edn_rnd_ack_i,