Merge minhv handling from CV32E40S

rtl/cv32e40x_alignment_buffer.sv

@@ -100,7 +100,7 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
   // resp_valid gated while flushing
   logic resp_valid_gated;
 
-  // CLIC vectoring (and Zc table jumps)
+  // CLIC vectoring
   // Flag for signalling that results is a CLIC function pointer
   logic is_clic_ptr_q;
   // Flag for table jump pointer

rtl/cv32e40x_controller_bypass.sv

@@ -123,7 +123,8 @@ module cv32e40x_controller_bypass import cv32e40x_pkg::*;
   //TODO:OK:low This CSR stall check is very restrictive
   //         Should only check EX vs WB, and also CSR/rd addr
   //         Also consider whether ID or EX should be stalled
-  // Detect when a CSR insn is in ID
+  // The controller mechanism for checking mcause.mpp/mcause.minhv when an mret is in the ID stage depends on this stall.
+  // Detect when a CSR insn is in ID (including WFI which reads mstatus.tw and priv level)
   // Note that hazard detection uses the registered instr_valid signals. Usage of the local
   // instr_valid signals would lead to a combinatorial loop via the halt signal.
 

rtl/cv32e40x_controller_fsm.sv

@@ -424,6 +424,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
   // The cycle after fencei enters WB, the fencei handshake will be initiated. This must complete and the fencei instruction must retire before allowing debug.
   // Once the first part of a table jump has finished in WB, we are not allowed to take debug before the last part finishes. This can be detected when the last
   // part of a table jump is in either EX or WB. // todo: update comments related to table jump (explain general concept and to which instructions it applies)
+  // todo: why is clic_ptr_in_pipeline included here (and not in interrupt_allowed)
   assign debug_allowed = lsu_interruptible_i && !fencei_ongoing && !xif_in_wb && !clic_ptr_in_pipeline && sequence_interruptible;
 
   // Debug pending for any other reason than single step
@@ -565,6 +566,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
 
     ctrl_fsm_o.csr_save_cause   = 1'b0;
     ctrl_fsm_o.csr_cause        = 32'h0;
+
     ctrl_fsm_o.csr_clear_minhv  = 1'b0;
 
     pipe_pc_mux_ctrl            = PC_WB;
@@ -618,6 +620,9 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
           ctrl_fsm_o.csr_cause.irq = 1'b1;
           ctrl_fsm_o.csr_cause.exception_code = nmi_is_store_q ? INT_CAUSE_LSU_STORE_FAULT : INT_CAUSE_LSU_LOAD_FAULT;
 
+          // Keep mcause.minhv when taking exceptions and interrupts, only cleared on successful pointer fetches or CSR writes.
+          ctrl_fsm_o.csr_cause.minhv  = mcause_i.minhv;
+
           // Save pc from oldest valid instruction
           if (ex_wb_pipe_i.instr_valid) begin
             pipe_pc_mux_ctrl = PC_WB;
@@ -657,6 +662,9 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
           ctrl_fsm_o.csr_save_cause  = 1'b1;
           ctrl_fsm_o.csr_cause.irq = 1'b1;
 
+          // Keep mcause.minhv when taking exceptions and interrupts, only cleared on successful pointer fetches or CSR writes.
+          ctrl_fsm_o.csr_cause.minhv  = mcause_i.minhv;
+
 
           if (SMCLIC) begin
             ctrl_fsm_o.csr_cause.exception_code = {1'b0, irq_id_ctrl_i};
@@ -705,6 +713,10 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
             pipe_pc_mux_ctrl = PC_WB;
             ctrl_fsm_o.csr_save_cause = !debug_mode_q; // Do not update CSRs if in debug mode
             ctrl_fsm_o.csr_cause.exception_code = exception_cause_wb;
+
+            // Keep mcause.minhv when taking exceptions and interrupts, only cleared on successful pointer fetches or CSR writes.
+            ctrl_fsm_o.csr_cause.minhv  = mcause_i.minhv;
+
           // Special insn
           end else if (wfi_in_wb || wfe_in_wb) begin
             // Halt the entire pipeline
@@ -807,13 +819,14 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
             ctrl_fsm_o.kill_if = 1'b1;
 
             if (sys_mret_id) begin
-              // When mcause.minhv is set, it signals that the previous pointer fetch faulted in IF.
-              // When the mret it execured with mcause.minhv set, the pointer fetch should be restarted
-              // instead of returning to the address in mepc.
+              // If the xcause.minhv bit is set for the previous privilege level (mcause.mpp) when an mret is in ID,
+              // the mret should restart the CLIC pointer fetch using mepc as a pointer to the pointer instead of jumping to
+              // the address in mepc. If mpp==PRIV_LVL_U, the (not existing) ucause.uinhv bit is assumed to be 0.
               // This is done below by signalling pc_set_clicv along with pc_mux=PC_MRET. This will
-              // treat the mepc as an address of a CLIC pointer. The minhv flag will only be cleaned
+              // treat the mepc as an address to a CLIC pointer. The minhv flag will only be cleaned
               // when a pointer reaches the ID stage with no faults from fetching.
-              if (mcause_i.minhv) begin
+              // ID stage is halted while it contains an mret and at the same time there are CSR writes EX or WB, hence it is safe to use mcause here.
+              if (mcause_i.minhv && (mcause_i.mpp == PRIV_LVL_M)) begin
                 // mcause.minhv set, exception occured during last pointer fetch (or SW wrote it)
                 // Must wait until EX and WB are empty as they can cause exceptions
                 if (!(id_ex_pipe_i.instr_valid || ex_wb_pipe_i.instr_valid)) begin
@@ -831,7 +844,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
                   ctrl_fsm_o.halt_id = 1'b1;
                 end
               end else begin
-                // mcause.minhv not set, do regular mret
+                // xcause.xinhv not set for the previous privilege level, do regular mret
                 ctrl_fsm_o.pc_mux = PC_MRET;
                 ctrl_fsm_o.pc_set = 1'b1;
 
@@ -955,7 +968,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
         debug_mode_n = 1'b1;
         ctrl_fsm_ns = FUNCTIONAL;
       end
-      // State for CLIC vectoring (and Zc table jumps)
+      // State for CLIC vectoring
       // In this state a fetch has been ordered, and the controller
       // is waiting for the pointer to arrive in the decode stage.
       POINTER_FETCH: begin

rtl/cv32e40x_cs_registers.sv

@@ -1008,6 +1008,7 @@ module cv32e40x_cs_registers import cv32e40x_pkg::*;
 
           mcause_n       = ctrl_fsm_i.csr_cause;
 
+
           if (SMCLIC) begin
             // mpil is saved from mintstatus
             mcause_n.mpil = mintstatus_rdata.mil;
@@ -1027,7 +1028,6 @@ module cv32e40x_cs_registers import cv32e40x_pkg::*;
           end else begin
             mcause_n.mpil = '0;
           end
-
           mcause_we = 1'b1;
 
         end
@@ -1060,18 +1060,27 @@ module cv32e40x_cs_registers import cv32e40x_pkg::*;
         mstatus_we     = 1'b1;
 
       end //ctrl_fsm_i.csr_restore_dret
-
-      // mcause.minhv shall be cleared if vector fetch is successful
-      ctrl_fsm_i.csr_clear_minhv: begin
-        if (SMCLIC) begin
-          mcause_n = mcause_rdata;
-          mcause_n.minhv = 1'b0;
-          mcause_we = 1'b1;
-        end
-      end
-
       default:;
     endcase
+
+    // mcause.minhv shall be cleared if vector fetch is successful
+    // Not part of the above unique case, as csr_clear_minv and csr_restore_mret
+    // may happen at the same time when an mret is executed when mcause.minhv==1
+    // -- In this case, the mret may be in WB while the controller FSM is in POINTER state
+    //    and a successful pointer fetch is in the ID stage (clears mcause.minhv).
+    // The fields mstatus.mpp and mstatus.mpie er aliased between mcause and mstatus. The mcause write
+    // due to csr_celar_minhv will however only write to mcause.minhv, and no updates to mstatus.mpp/mpie.
+    if (SMCLIC) begin
+      if (ctrl_fsm_i.csr_clear_minhv) begin
+        // Keep mcause values, only clear minhv bit.
+        // Note that mcause_rdata may have the wrong values for mpp and mpie if an mret is also in WB.
+        //  - This is ok as the aliased mpp/mpie bits are stored in mstatus and not mcause, and the clearing
+        //    of minhv does not attempt to change mcause.mpp/mpie.
+        mcause_n = mcause_rdata;
+        mcause_n.minhv = 1'b0;
+        mcause_we = 1'b1;
+      end
+    end
   end
 
   // Mirroring mstatus_n to mnxti_n for RVFI

sva/cv32e40x_controller_fsm_sva.sv

@@ -94,7 +94,9 @@ module cv32e40x_controller_fsm_sva
   input mcause_t        mcause_i,
   input logic           lsu_trans_valid_i,
   input logic           irq_wu_ctrl_i,
-  input logic           wu_wfe_i
+  input logic           wu_wfe_i,
+  input logic           sys_en_id_i,
+  input logic           sys_mret_id_i
 );
 
 
@@ -593,14 +595,34 @@ if (SMCLIC) begin
                                                                       (ctrl_fsm_o.pc_mux == PC_TRAP_NMI))))
     else `uvm_error("controller", "Illegal pc_mux after pointer fetch")
 
-  // Check that EX and WB are empty when an mret does it's jump when mcause.minhv is set
-  a_minhv_ex_wb_pipeline_empty:
+  // Check that EX and WB does not contain any instruction with possible exceptions when a mret which restarts a pointer fetch is performed
+  a_jump_no_exceptions:
   assert property (@(posedge clk) disable iff (!rst_n)
-                  (ctrl_fsm_o.pc_set && ctrl_fsm_o.pc_mux == PC_MRET) && mcause_i.minhv
-                  |-> !(id_ex_pipe_i.instr_valid || ex_wb_pipe_i.instr_valid))
-    else `uvm_error("controller", "EX and WB not empty on mret with mcause.minhv set")
-
-end // SMCLIC
+                  (ctrl_fsm_o.pc_set && ctrl_fsm_o.pc_mux == PC_MRET) && ctrl_fsm_o.pc_set_clicv
+                  |->
+                  !(id_ex_pipe_i.instr_valid && (id_ex_pipe_i.abort_op)) &&
+                  !(ex_wb_pipe_i.instr_valid && (ex_wb_pipe_i.abort_op || lsu_err_wb_i || (lsu_mpu_status_wb_i != MPU_OK))))
+    else `uvm_error("controller", "EX and WB may cause exceptions when mret with mcause.minhv is performed")
+
+end else begin // SMCLIC
+  // Check that CLIC related signals are inactive when CLIC is not configured.
+  a_clic_inactive:
+  assert property (@(posedge clk) disable iff (!rst_n)
+                  1'b1
+                  |->
+                  (ctrl_fsm_cs != POINTER_FETCH)    &&
+                  !ctrl_fsm_o.csr_cause.minhv       &&
+                  !ctrl_fsm_o.csr_clear_minhv       &&
+                  !mcause_i.minhv                   &&
+                  !if_id_pipe_i.instr_meta.clic_ptr &&
+                  !id_ex_pipe_i.instr_meta.clic_ptr &&
+                  !ex_wb_pipe_i.instr_meta.clic_ptr &&
+                  !ctrl_fsm_o.pc_set_clicv          &&
+                  !(|ctrl_fsm_o.irq_level)          &&
+                  !ctrl_fsm_o.irq_shv               &&
+                  !(|ctrl_fsm_o.irq_priv) )
+    else `uvm_error("controller", "CLIC signals active when CLIC is not configured.")
+end
 
 
 
@@ -737,5 +759,29 @@ end // SMCLIC
   assert property (@(posedge clk) disable iff (!rst_n)
                     (ctrl_fsm_cs == SLEEP) |-> !ctrl_byp_i.irq_enable_stall)
     else `uvm_error("controller", "irq_enable_stall while SLEEPING should not happen.")
+
+  // Handling of mret in ID requires mcause.minhv to be stable
+  // This assertion checks that both operations of secure mrets in ID see the same mcause.minhv.
+  a_mret_id_ex_minhv_stable:
+  assert property (@(posedge clk) disable iff (!rst_n)
+                    (id_valid_i && ex_ready_i && sys_en_id_i && sys_mret_id_i)
+                    |=>
+                    $stable(mcause_i.minhv))
+    else `uvm_error("controller", "mcause.minhv not stable when mret goes from ID to EX")
+
+/* todo: Fix or remove assertion. Will fail for the following scenario:
+    1: mret (1/2) in ID restarts pointer fetch due to mpp and minhv conditions.
+    2: mret (1/2) ID->EX, mret (2/2) in ID  [pointer may be in IF]
+    3: mret (1/2) EX->WB, mret (2/2) in EX  [pointer may be in ID, if successful minhv is cleared]
+    4: mret (2/2) EX->WB, minhv not stable due to clearing by pointer in previous cycle.
+    The not stable minhv is a side effect of the mret itself, so it could be considered a self-stall which
+    normally will not cause halts.
+  a_mret_ex_wb_minhv_stable:
+  assert property (@(posedge clk) disable iff (!rst_n)
+                    (ex_valid_i && wb_ready_i && id_ex_pipe_i.sys_en && id_ex_pipe_i.sys_mret_insn)
+                    |=>
+                    $stable(mcause_i.minhv))
+    else `uvm_error("controller", "mcause.minhv not stable when mret goes from EX to WB")
+*/
 endmodule
 

sva/cv32e40x_cs_registers_sva.sv

@@ -138,5 +138,48 @@ module cv32e40x_cs_registers_sva
 
   a_halt_limited_wb: assert property(p_halt_limited_wb)
     else `uvm_error("cs_registers", "CSR in WB while halt_limited_wb is set");
+
+  // Check that mcause is not written when an mret or dret is in WB
+  property p_mret_dret_wb_mcause_write;
+    @(posedge clk) disable iff (!rst_n)
+    (  (ctrl_fsm_i.csr_restore_mret || ctrl_fsm_i.csr_restore_dret) && !ctrl_fsm_i.csr_clear_minhv |-> !mcause_we);
+  endproperty;
+
+  a_mret_dret_wb_mcause_write: assert property(p_mret_dret_wb_mcause_write)
+    else `uvm_error("cs_registers", "mcause written when MRET or DRET is in WB.");
+
+  // Check csr_clear_minhv cannot happen at the same time as csr_save_cause or csr_restore_dret (would cause mcause_we conflict)
+  property p_minhv_unique;
+    @(posedge clk) disable iff (!rst_n)
+    (  ctrl_fsm_i.csr_clear_minhv -> !(ctrl_fsm_i.csr_save_cause || ctrl_fsm_i.csr_restore_dret));
+  endproperty;
+
+  a_minhv_unique: assert property(p_minhv_unique)
+    else `uvm_error("cs_registers", "csr_save_cause at the same time as csr_clear_minhv.");
+
+
+  // Check EX is empty or contains last part of mret when minhv is cleared
+  property p_ex_empty_minhv_clear;
+    @(posedge clk) disable iff (!rst_n)
+    (  ctrl_fsm_i.csr_clear_minhv
+       -> !id_ex_pipe_i.instr_valid       // No valid instruction in EX
+       or
+          (id_ex_pipe_i.instr_valid && id_ex_pipe_i.sys_en && id_ex_pipe_i.sys_mret_insn && id_ex_pipe_i.last_op)); // mret in EX
+  endproperty;
+
+  a_ex_empty_minhv_clear: assert property(p_ex_empty_minhv_clear)
+    else `uvm_error("cs_registers", "EX not empty when csr_clear_minhv is active.");
+
+  // Check WB is empty or contains any part of mret when minhv is cleared
+  property p_wb_empty_minhv_clear;
+    @(posedge clk) disable iff (!rst_n)
+    (  ctrl_fsm_i.csr_clear_minhv
+        -> !ex_wb_pipe_i.instr_valid       // No valid instruction in WB
+        or
+          (ex_wb_pipe_i.instr_valid && ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_mret_insn)); // mret in WB
+  endproperty;
+
+  a_wb_empty_minhv_clear: assert property(p_wb_empty_minhv_clear)
+    else `uvm_error("cs_registers", "WB not empty when csr_clear_minhv is active.");
 endmodule