[rtl] Various security feature bugfixes

- Document that SecureIbex cannot be used without a multiplier and add
  an assertion in the rtl. This fixes #1080.
- Move the PC checking hardware onto its own parameter to match all the
  other individual security features.
- Make the PC increment behavior more sensible on fetch errors (and make
  it match the icache behavior). Factor this into the PC increment check
  to prevent false triggering, fixes #1094.
- Stop the PC mismatch checker firing on dummy instructions, fixes
  #1095.

Signed-off-by: Tom Roberts <tomroberts@lowrisc.org>

doc/02_user/integration.rst

@@ -123,7 +123,8 @@ Parameters
 | ``BranchPrediction``         | bit                 | 0          | *EXPERIMENTAL* Enable Static branch prediction                        |
 +------------------------------+---------------------+------------+-----------------------------------------------------------------------+
 | ``SecureIbex``               | bit                 | 0          | *EXPERIMENTAL* Enable various additional features targeting           |
-|                              |                     |            | secure code execution.                                                |
+|                              |                     |            | secure code execution. Note: SecureIbex == 1'b1 and                   |
+|                              |                     |            | RV32M == ibex_pkg::RV32MNone is an illegal combination.               |
 +------------------------------+---------------------+------------+-----------------------------------------------------------------------+
 | ``DbgTriggerEn``             | bit                 | 0          | Enable debug trigger support (one trigger only)                       |
 +------------------------------+---------------------+------------+-----------------------------------------------------------------------+

doc/03_reference/security.rst

@@ -52,6 +52,9 @@ The interval between instruction insertion is randomized in the core using an LF
 Sofware can periodically re-seed this LFSR with true random numbers (if available) via the **secureseed** CSR.
 This will make the insertion interval of dummy instructions much harder for an attacker to predict.
 
+Note that the dummy instruction feature inserts multiply and divide instructions.
+The core must be configured with a multiplier (`RV32M != ibex_pkg::RV32MNone`) or errors will occur using this feature.
+
 Register file ECC
 -----------------
 

rtl/ibex_core.sv

@@ -111,6 +111,7 @@ module ibex_core #(
   localparam int unsigned PMP_NUM_CHAN      = 2;
   localparam bit          DataIndTiming     = SecureIbex;
   localparam bit          DummyInstructions = SecureIbex;
+  localparam bit          PCIncrCheck       = SecureIbex;
   // Speculative branch option, trades-off performance against timing.
   // Setting this to 1 eases branch target critical paths significantly but reduces performance
   // by ~3% (based on CoreMark/MHz score).
@@ -396,7 +397,7 @@ module ibex_core #(
       .DummyInstructions ( DummyInstructions ),
       .ICache            ( ICache            ),
       .ICacheECC         ( ICacheECC         ),
-      .SecureIbex        ( SecureIbex        ),
+      .PCIncrCheck       ( PCIncrCheck       ),
       .BranchPredictor   ( BranchPredictor   )
   ) if_stage_i (
       .clk_i                    ( clk                    ),
@@ -1431,4 +1432,7 @@ module ibex_core #(
 
 `endif
 
+  // Certain parameter combinations are not supported
+  `ASSERT_INIT(IllegalParamSecure, !(SecureIbex && (RV32M == RV32MNone)))
+
 endmodule

rtl/ibex_fetch_fifo.sv

@@ -103,7 +103,7 @@ module ibex_fetch_fifo #(
                                         (valid_q[0] & in_valid_i);
 
   // If there is an error, rdata is unknown
-  assign unaligned_is_compressed = (rdata[17:16] != 2'b11) | err;
+  assign unaligned_is_compressed = (rdata[17:16] != 2'b11) & ~err;
   assign aligned_is_compressed   = (rdata[ 1: 0] != 2'b11) & ~err;
 
   ////////////////////////////////////////

rtl/ibex_if_stage.sv

@@ -18,7 +18,7 @@ module ibex_if_stage #(
     parameter bit          DummyInstructions = 1'b0,
     parameter bit          ICache            = 1'b0,
     parameter bit          ICacheECC         = 1'b0,
-    parameter bit          SecureIbex        = 1'b0,
+    parameter bit          PCIncrCheck       = 1'b0,
     parameter bit          BranchPredictor   = 1'b0
 ) (
     input  logic                   clk_i,
@@ -372,13 +372,14 @@ module ibex_if_stage #(
   end
 
   // Check for expected increments of the PC when security hardening enabled
-  if (SecureIbex) begin : g_secure_pc
+  if (PCIncrCheck) begin : g_secure_pc
     logic [31:0] prev_instr_addr_incr;
     logic        prev_instr_seq_q, prev_instr_seq_d;
 
     // Do not check for sequential increase after a branch, jump, exception, interrupt or debug
-    // request, all of which will set branch_req. Also do not check after reset.
-    assign prev_instr_seq_d = (prev_instr_seq_q | instr_new_id_d) & ~branch_req;
+    // request, all of which will set branch_req. Also do not check after reset or for dummys.
+    assign prev_instr_seq_d = (prev_instr_seq_q | instr_new_id_d) &
+        ~branch_req & ~stall_dummy_instr;
 
     always_ff @(posedge clk_i or negedge rst_ni) begin
       if (!rst_ni) begin
@@ -388,7 +389,8 @@ module ibex_if_stage #(
       end
     end
 
-    assign prev_instr_addr_incr = pc_id_o + (instr_is_compressed_id_o ? 32'd2 : 32'd4);
+    assign prev_instr_addr_incr = pc_id_o + ((instr_is_compressed_id_o && !instr_fetch_err_o) ?
+                                             32'd2 : 32'd4);
 
     // Check that the address equals the previous address +2/+4
     assign pc_mismatch_alert_o = prev_instr_seq_q & (pc_if_o != prev_instr_addr_incr);