[otbn,rtl] Pass an escalation signal to start/stop controller

Otherwise, an escalation that happens when we're in e.g. state
OtbnStartStopStateUrndRefresh leaves us in a weird half-running state.
The rest of OTBN knows that we're stopping immediately in this case:
make sure that the start/stop controller knows too!

The other change with this PR is that we change the
OtbnStartStopStateError state to OtbnStartStopStateLocked. An FSM
glitch will still ping state_error_o for a cycle, causing a fatal
alert, but now we have a nice terminal state for local escalation.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>

hw/ip/otbn/rtl/otbn_core.sv

diff --git a/hw/ip/otbn/rtl/otbn_core.sv b/hw/ip/otbn/rtl/otbn_core.sv
index 9a641e2..005da02 100644
--- a/hw/ip/otbn/rtl/otbn_core.sv
+++ b/hw/ip/otbn/rtl/otbn_core.sv
@@ -214,7 +214,7 @@
   logic [ImemAddrWidth:0]   prefetch_loop_end_addr;
   logic [ImemAddrWidth-1:0] prefetch_loop_jump_addr;
 
-  mubi4_t               controller_escalate_en;
+  mubi4_t               controller_escalate_en, start_stop_escalate_en;
   controller_err_bits_t controller_err_bits;
 
   core_err_bits_t err_bits_q, err_bits_d;
@@ -232,6 +232,7 @@
     .rst_ni,
 
     .start_i,
+    .escalate_en_i(start_stop_escalate_en),
 
     .controller_start_o(controller_start),
 
@@ -438,6 +439,12 @@
   `ASSERT(InsnDataStableInStall, u_otbn_controller.state_q == OtbnStateStall |->
                                  insn_fetch_resp_data == $past(insn_fetch_resp_data))
 
+  // Spot the fatal error bits from the controller
+  logic controller_fatal_err;
+  assign controller_fatal_err = |{controller_err_bits.fatal_software,
+                                  controller_err_bits.bad_internal_state,
+                                  controller_err_bits.reg_intg_violation};
+
   // Generate an err_bits output by combining errors from all the blocks in otbn_core
   assign err_bits_d = '{
     fatal_software:      controller_err_bits.fatal_software,
@@ -472,12 +479,16 @@
   // 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)
-  assign controller_escalate_en = mubi4_or_hi(
-      escalate_en_i, mubi4_bool_to_mubi(
-          |{start_stop_state_error, urnd_all_zero, rf_base_rd_data_err, lsu_rdata_err,
-            insn_fetch_err}
-      )
-  );
+  assign controller_escalate_en =
+      mubi4_or_hi(escalate_en_i,
+                  mubi4_bool_to_mubi(|{start_stop_state_error, urnd_all_zero,
+                                       rf_base_rd_data_err, lsu_rdata_err, insn_fetch_err}));
+
+  // Similarly for the start/stop controller
+  assign start_stop_escalate_en =
+      mubi4_or_hi(escalate_en_i,
+                  mubi4_bool_to_mubi(|{urnd_all_zero, rf_base_rd_data_err,
+                                       lsu_rdata_err, insn_fetch_err, controller_fatal_err}));
 
   assign insn_cnt_o = insn_cnt;
 
@@ -734,4 +745,10 @@
           mubi4_test_true_loose(controller_escalate_en) && mubi4_test_false_strict(escalate_en_i)
           |=> err_bits_q)
 
+  // Similarly, if we pass an escalation signal down to the start/stop controller then we should
+  // also be setting an error flag, unless the signal came from above.
+  `ASSERT(ErrBitsIfStartStopEscalate_A,
+          mubi4_test_true_loose(start_stop_escalate_en) && mubi4_test_false_strict(escalate_en_i)
+          |=> err_bits_q)
+
 endmodule

hw/ip/otbn/rtl/otbn_pkg.sv

diff --git a/hw/ip/otbn/rtl/otbn_pkg.sv b/hw/ip/otbn/rtl/otbn_pkg.sv
index 3eb6515..7d4529d 100644
--- a/hw/ip/otbn/rtl/otbn_pkg.sv
+++ b/hw/ip/otbn/rtl/otbn_pkg.sv
@@ -481,7 +481,7 @@
     OtbnStartStopSecureWipeAccModBaseUrnd = 6'b100101,
     OtbnStartStopSecureWipeAllZero        = 6'b111110,
     OtbnStartStopSecureWipeComplete       = 6'b001011,
-    OtbnStartStopStateError               = 6'b000110
+    OtbnStartStopStateLocked              = 6'b000110
   } otbn_start_stop_state_e;
 
 // Encoding generated with:

hw/ip/otbn/rtl/otbn_start_stop_control.sv

diff --git a/hw/ip/otbn/rtl/otbn_start_stop_control.sv b/hw/ip/otbn/rtl/otbn_start_stop_control.sv
index 44054e3..1e61e1f 100644
--- a/hw/ip/otbn/rtl/otbn_start_stop_control.sv
+++ b/hw/ip/otbn/rtl/otbn_start_stop_control.sv
@@ -24,14 +24,16 @@
 
 module otbn_start_stop_control
   import otbn_pkg::*;
-  #(
+  import prim_mubi_pkg::*;
+#(
   // Enable internal secure wipe
   parameter bit                SecWipeEn  = 1'b0
 )(
   input  logic clk_i,
   input  logic rst_ni,
 
-  input  logic start_i,
+  input  logic  start_i,
+  input mubi4_t escalate_en_i,
 
   output logic controller_start_o,
 
@@ -62,6 +64,27 @@
   logic addr_cnt_inc;
   logic [4:0] addr_cnt_q, addr_cnt_d;
 
+  // There are two ways in which the start/stop controller can be told to stop. Either
+  // start_secure_wipe_i comes from the controller (which means "I've run some instructions and I've
+  // hit an ECALL or error"). Or escalate_en_i can be asserted (which means "Someone else has told
+  // us to stop immediately"). If running, both can be true at once.
+  //
+  // An escalation signal gets latched into should_lock. If we were running some instructions, we'll
+  // go through the secure wipe process, but we'll see the should_lock_q signal when done and go
+  // into the local locked state.
+  logic esc_request, should_lock_d, should_lock_q, stop;
+  assign esc_request   = mubi4_test_true_loose(escalate_en_i);
+  assign stop          = esc_request | start_secure_wipe_i;
+  assign should_lock_d = should_lock_q | esc_request;
+
+  always_ff @(posedge clk_i or negedge rst_ni) begin
+    if (!rst_ni) begin
+      should_lock_q <= 1'b0;
+    end else begin
+      should_lock_q <= should_lock_d;
+    end
+  end
+
   // SEC_CM: START_STOP_CTRL.FSM.SPARSE
   `PRIM_FLOP_SPARSE_FSM(u_state_regs, state_d, state_q,
       otbn_start_stop_state_e, OtbnStartStopStateHalt)
@@ -85,7 +108,9 @@
 
     unique case (state_q)
       OtbnStartStopStateHalt: begin
-        if (start_i) begin
+        if (stop) begin
+          state_d = OtbnStartStopStateLocked;
+        end else if (start_i) begin
           urnd_reseed_req_o = 1'b1;
           ispr_init_o       = 1'b1;
           state_reset_o     = 1'b1;
@@ -93,13 +118,15 @@
         end
       end
       OtbnStartStopStateUrndRefresh: begin
-        if (!urnd_reseed_busy_i) begin
+        if (stop) begin
+          state_d = OtbnStartStopStateLocked;
+        end else if (!urnd_reseed_busy_i) begin
           state_d     = OtbnStartStopStateRunning;
         end
       end
       OtbnStartStopStateRunning: begin
         urnd_advance_o = 1'b1;
-        if (start_secure_wipe_i) begin
+        if (stop) begin
           if (SecWipeEn) begin
             state_d = OtbnStartStopSecureWipeWdrUrnd;
           end
@@ -148,20 +175,21 @@
           state_d = OtbnStartStopSecureWipeComplete;
         end
       end
-       OtbnStartStopSecureWipeComplete: begin
+      OtbnStartStopSecureWipeComplete: begin
         urnd_advance_o = 1'b1;
         secure_wipe_running_o = 1'b1;
-        state_d = OtbnStartStopStateHalt;
+        state_d = should_lock_d ? OtbnStartStopStateLocked : OtbnStartStopStateHalt;
       end
-      OtbnStartStopStateError: begin
+      OtbnStartStopStateLocked: begin
         // SEC_CM: START_STOP_CTRL.FSM.LOCAL_ESC
-        // Terminal error state
-        state_error_o = 1'b1;
+        //
+        // Terminal state. This is either accessed by glitching state_q (and going through the
+        // default case below) or by getting an escalation signal
       end
       default: begin
         // We should never get here. If we do (e.g. via a malicious glitch), error out immediately.
         state_error_o = 1'b1;
-        state_d = OtbnStartStopStateError;
+        state_d = OtbnStartStopStateLocked;
       end
     endcase
   end
@@ -182,13 +210,17 @@
 
   assign sec_wipe_addr_o = addr_cnt_q;
 
-  `ASSERT(StartStopStateValid,
+  `ASSERT(StartStopStateValid_A,
       state_q inside {OtbnStartStopStateHalt,
                       OtbnStartStopStateUrndRefresh,
                       OtbnStartStopStateRunning,
                       OtbnStartStopSecureWipeWdrUrnd,
                       OtbnStartStopSecureWipeAccModBaseUrnd,
                       OtbnStartStopSecureWipeAllZero,
-                      OtbnStartStopSecureWipeComplete})
+                      OtbnStartStopSecureWipeComplete,
+                      OtbnStartStopStateLocked})
+
+  `ASSERT(StartSecureWipeImpliesRunning_A,
+          start_secure_wipe_i |-> (state_q == OtbnStartStopStateRunning))
 
 endmodule