[otbn,rtl] Harden acc_qw_sel in bignum MAC against FI attack

This commits predecodes the acc_qw_sel signal to avoid a tricky SW mitigation against FI attacks,
see detailed problem description below. It is predecoded by refactoring it into a 4 bit signal which
controls the MUXing of the ACC merging for each quad word (64 bits) separately.

The problem when acc_qw_sel is not predecoded:
The acc_qw_sel signal controls which quarter word of the ACC is overwritten with multiplication
results during a vectorized multiplication. These instructions process 64 bits at a time and thus
have 4 updates to the ACC WSR.

This signal did not have a redundand signal. Therefore, an attack on this signal is an issue for the
bn.mulv(m) instructions because:

1) A stuck at fault is induced into the bits such that acc_qw_sel = '0.

Yes this requires in theory two attacks as acc_qw_sel is 2 bit wide. But I think it is reasonable that both gates can be FIed
with only one attack as the signals are quite related. A multiplication thus will write all 4
results into the same QW of ACC. The other 3 QW keep the original value.
This allows that QW0 has the wrong result and the other 3 QW have a deterministic value (the value
before the instruction). This could be bad, especially if we perform a multiplication where not all
vector elements contain actual data. If only, e.g., 7 out of 8 elements contain data and element 8
is all-zero, this would allow to set a QW to zero (a deterministic value).

A SW solution solving this would be to ensure that the ACC register is cleared with a random value before
executing a instruction. This is actually supported as the bn.mulv(m) instructions clear ACC in
their last cycle and the ACC could also be cleared by using a bn.wsrw instruction.
However, with this mitigation we would still not directly detect the attack (we would only have a
wrong result). The same randomization would also be needed for unused input vector elements. This
adds considerable burdens to the programmer and is also a great source of vulnerabilities.

2) The acc_qw_sel signal is faulted transiently.

In this case only one result is written to the wrong QW and therefore one QW would keep its value.
Otherwise there are the same problems and the same SW solution could be used.

As the SW solution is cumbersome and easy to do wrong, this commit predecodes the acc_qw_sel signal
by refactoring it to a single bit MUX per QW.

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

Author: etterli

hw/ip/otbn/rtl/otbn_instruction_fetch.sv

@@ -202,6 +202,7 @@ module otbn_instruction_fetch
     mac_bignum_predec_to_fsm.mul_add_en,
     mac_bignum_predec_to_fsm.c_add_en,
     mac_bignum_predec_to_fsm.add_mod_en,
+    mac_bignum_predec_to_fsm.acc_qw_sel,
     mac_bignum_predec_to_fsm.acc_merger_en,
     mac_bignum_predec_to_fsm.mul_shift_en,
     mac_bignum_predec_to_fsm.mul_merger_en,

hw/ip/otbn/rtl/otbn_mac_bignum.sv

@@ -505,7 +505,6 @@ module otbn_mac_bignum
   ///////////////////////////////////////////
   // ACC merging for vectorized operations //
   ///////////////////////////////////////////
-  logic [1:0]       acc_qw_sel;
   logic [QWLEN-1:0] acc_new_qw;
   logic [WLEN-1:0]  acc_blanked;
   logic [WLEN-1:0]  acc_merged;
@@ -526,14 +525,10 @@ module otbn_mac_bignum
   );
 
   // Place the computed 64-bit chunk at the desired location in the ACC register.
-  assign acc_merged[0*QWLEN+:QWLEN] = (acc_qw_sel == 2'd0) ? acc_new_qw :
-                                                             acc_blanked[0*QWLEN+:QWLEN];
-  assign acc_merged[1*QWLEN+:QWLEN] = (acc_qw_sel == 2'd1) ? acc_new_qw :
-                                                             acc_blanked[1*QWLEN+:QWLEN];
-  assign acc_merged[2*QWLEN+:QWLEN] = (acc_qw_sel == 2'd2) ? acc_new_qw :
-                                                             acc_blanked[2*QWLEN+:QWLEN];
-  assign acc_merged[3*QWLEN+:QWLEN] = (acc_qw_sel == 2'd3) ? acc_new_qw :
-                                                             acc_blanked[3*QWLEN+:QWLEN];
+  for (genvar qw = 0; qw < VLEN/QWLEN; qw++) begin : gen_acc_merged
+    assign acc_merged[qw * QWLEN +: QWLEN] = predec_i.acc_qw_sel[qw] ?
+        acc_new_qw : acc_blanked[qw * QWLEN +: QWLEN];
+  end
 
   //////////////////////////////////////////////////////
   // Adder result handling for regular multiplication //
@@ -687,7 +682,6 @@ module otbn_mac_bignum
   assign tmp_clear_en  = contrl.tmp_clear_en;
   assign c_wr_en_raw   = contrl.c_wr_en_raw;
   assign c_clear_en    = contrl.c_clear_en;
-  assign acc_qw_sel    = contrl.acc_qw_sel;
   assign acc_wr_en_raw = contrl.acc_wr_en_raw;
   assign acc_clear_en  = contrl.acc_clear_en;
 
@@ -745,4 +739,10 @@ module otbn_mac_bignum
   assign sec_wipe_err_o = sec_wipe_urnd_i & ~sec_wipe_running_i;
 
   `ASSERT(NoISPRAccWrAndMacEn, ~(ispr_acc_wr_en_i & mac_en_i))
+
+  // Only one QWORD must be overwritten at the same time.
+  `ASSERT(AccQwSelOnehot_A,
+          predec_i.acc_merger_en |-> $onehot(predec_i.acc_qw_sel),
+          clk_i, !rst_ni || predec_error_o || state_err_o)
+
 endmodule

hw/ip/otbn/rtl/otbn_mac_bignum_fsm.sv

@@ -60,7 +60,7 @@ module otbn_mac_bignum_fsm
    * | op_a_qw_sel         |   0 |   1 |   2 |   3 |        yes |
    * | op_b_elem0_sel      |   0 |   2 |   4 |   6 |        yes |
    * | op_b_elem1_sel      |   1 |   3 |   5 |   7 |        yes |
-   * | acc_qw_sel          |   0 |   1 |   2 |   3 |         no |
+   * | acc_qw_sel          |   0 |   1 |   2 |   3 |        yes |
    * | acc_wr_en_raw       |   1 |   1 |   1 |   0 |         no |
    * | acc_clear_en        |   0 |   0 |   0 |   1 |         no |
    * | acc_merger_en       |   1 |   1 |   1 |   1 |        yes |
@@ -86,7 +86,7 @@ module otbn_mac_bignum_fsm
    * | tmp_clear_en        |   0 |   0 |   1 ||   0 |   0 |   1 ||     ||     |   1 |         no |
    * | c_wr_en_raw         |   1 |   0 |   0 ||   1 |   0 |   0 ||     ||     |   0 |         no |
    * | c_clear_en          |   0 |   0 |   1 ||   0 |   0 |   1 ||     ||     |   1 |         no |
-   * | acc_qw_sel          |   0 |   0 |   0 ||   1 |   1 |   1 ||     ||     |   3 |         no |
+   * | acc_qw_sel          |   0 |   0 |   0 ||   1 |   1 |   1 ||     ||     |   3 |        yes |
    * | acc_wr_en_raw       |   0 |   0 |   1 ||   0 |   0 |   1 ||     ||     |   0 |         no |
    * | acc_clear_en        |   0 |   0 |   0 ||   0 |   0 |   0 ||     ||     |   1 |         no |
    * | mul_add_en          |   0 |   0 |   1 ||   0 |   0 |   1 ||     ||     |   1 |        yes |
@@ -115,19 +115,20 @@ module otbn_mac_bignum_fsm
   localparam mac_bignum_contrl_t ControlDefault = '0;
 
   typedef struct packed {
-    logic [1:0]        op_a_qw_sel;
-    logic [2:0]        op_b_elem0_sel;
-    logic [2:0]        op_b_elem1_sel;
-    logic              mul_op_a_tmp_sel;
-    mac_mul_op_b_sel_e mul_op_b_sel;
-    logic              mul_add_en;
-    logic              c_add_en;
-    logic              add_mod_en;
-    logic              acc_merger_en;
-    logic              mul_shift_en;
-    logic              mul_merger_en;
-    logic              add_res_en;
-    logic              operation_valid_raw;
+    logic [1:0]            op_a_qw_sel;
+    logic [2:0]            op_b_elem0_sel;
+    logic [2:0]            op_b_elem1_sel;
+    logic                  mul_op_a_tmp_sel;
+    mac_mul_op_b_sel_e     mul_op_b_sel;
+    logic                  mul_add_en;
+    logic                  c_add_en;
+    logic                  add_mod_en;
+    logic [VLEN/QWLEN-1:0] acc_qw_sel;
+    logic                  acc_merger_en;
+    logic                  mul_shift_en;
+    logic                  mul_merger_en;
+    logic                  add_res_en;
+    logic                  operation_valid_raw;
   } mac_bignum_predec_dyn_t;
 
   localparam mac_bignum_predec_dyn_t PredecDynDefault = '{
@@ -139,6 +140,7 @@ module otbn_mac_bignum_fsm
     mul_add_en:          1'b0,
     c_add_en:            1'b0,
     add_mod_en:          1'b0,
+    acc_qw_sel:          '0,
     acc_merger_en:       1'b0,
     mul_shift_en:        1'b0,
     mul_merger_en:       1'b0,
@@ -174,11 +176,11 @@ module otbn_mac_bignum_fsm
     predec_vec = '{default: PredecDynDefault};
 
     for (int unsigned cycle = 0; cycle < LatencyVec; cycle++) begin
-      contrl_vec[cycle].acc_qw_sel     = 2'(cycle);
       contrl_vec[cycle].acc_wr_en_raw  = 1'b1;
       predec_vec[cycle].op_a_qw_sel    = 2'(cycle);
       predec_vec[cycle].op_b_elem0_sel = 3'(2 * cycle);
       predec_vec[cycle].op_b_elem1_sel = 3'(2 * cycle + 1);
+      predec_vec[cycle].acc_qw_sel     = (VLEN/QWLEN)'(unsigned'(1) << cycle);
       predec_vec[cycle].acc_merger_en  = 1'b1;
     end
 
@@ -223,11 +225,11 @@ module otbn_mac_bignum_fsm
     // Construct the 4 * 3 = 12 cycles and set the correct qword selection
     for (int unsigned cycle = 0; cycle < LatencyMod; cycle++) begin
       contrl_mod[cycle]                = contrl_mod_mul[cycle % LatencyMontgMul];
-      contrl_mod[cycle].acc_qw_sel     = 2'(cycle / LatencyMontgMul);
       predec_mod[cycle]                = predec_mod_mul[cycle % LatencyMontgMul];
       predec_mod[cycle].op_a_qw_sel    = 2'(cycle / LatencyMontgMul);
       predec_mod[cycle].op_b_elem0_sel = 3'(2 * (cycle / LatencyMontgMul));
       predec_mod[cycle].op_b_elem1_sel = 3'(2 * (cycle / LatencyMontgMul) + 1);
+      predec_mod[cycle].acc_qw_sel     = (VLEN/QWLEN)'(unsigned'(1)) << (cycle / LatencyMontgMul);
     end
 
     // Clear ACC in the last cycle with randomness
@@ -325,6 +327,7 @@ module otbn_mac_bignum_fsm
     mul_add_en:          predec_dyn.mul_add_en,
     c_add_en:            predec_dyn.c_add_en,
     add_mod_en:          predec_dyn.add_mod_en,
+    acc_qw_sel:          predec_dyn.acc_qw_sel,
     acc_merger_en:       predec_dyn.acc_merger_en,
     mul_shift_en:        predec_dyn.mul_shift_en,
     mul_merger_en:       predec_dyn.mul_merger_en,

hw/ip/otbn/rtl/otbn_pkg.sv

@@ -616,6 +616,7 @@ package otbn_pkg;
     logic                  mul_add_en;
     logic                  c_add_en;
     logic                  add_mod_en;
+    logic [VLEN/QWLEN-1:0] acc_qw_sel;
     logic                  acc_merger_en;
     logic                  mul_shift_en;
     logic                  mul_merger_en;
@@ -624,13 +625,12 @@ package otbn_pkg;
   } mac_bignum_predec_t;
 
   typedef struct packed {
-    logic       tmp_wr_en_raw;
-    logic       tmp_clear_en;
-    logic       c_wr_en_raw;
-    logic       c_clear_en;
-    logic [1:0] acc_qw_sel;
-    logic       acc_wr_en_raw;
-    logic       acc_clear_en;
+    logic tmp_wr_en_raw;
+    logic tmp_clear_en;
+    logic c_wr_en_raw;
+    logic c_clear_en;
+    logic acc_wr_en_raw;
+    logic acc_clear_en;
   } mac_bignum_contrl_t;
 
   typedef struct packed {

hw/ip/otbn/rtl/otbn_predecode.sv

@@ -832,6 +832,7 @@ module otbn_predecode
   assign mac_bignum_predec_raw_o.mul_add_en          = '0;
   assign mac_bignum_predec_raw_o.c_add_en            = '0;
   assign mac_bignum_predec_raw_o.add_mod_en          = '0;
+  assign mac_bignum_predec_raw_o.acc_qw_sel          = '0;
   assign mac_bignum_predec_raw_o.acc_merger_en       = '0;
   assign mac_bignum_predec_raw_o.mul_shift_en        = '0;
   assign mac_bignum_predec_raw_o.mul_merger_en       = '0;