Changed logic for sequence progress detection

bhv/cv32e40x_wrapper.sv

@@ -210,6 +210,10 @@ module cv32e40x_wrapper
                               .xif_commit_kill     (core_i.xif_commit_if.commit.commit_kill),
                               .xif_commit_valid    (core_i.xif_commit_if.commit_valid),
                               .last_op_id_i        (core_i.if_id_pipe.last_op),
+                              .first_op_if_i       (core_i.first_op_if),
+                              .first_op_ex_i       (core_i.first_op_ex),
+                              .instr_valid_if_i    (core_i.if_stage_i.prefetch_valid),
+                              .ptr_in_if_i         (core_i.if_stage_i.prefetch_is_tbljmp_ptr),
                               .*);
   bind cv32e40x_cs_registers:        core_i.cs_registers_i              cv32e40x_cs_registers_sva  #(.SMCLIC(SMCLIC)) cs_registers_sva (.*);
 

rtl/cv32e40x_controller.sv

@@ -47,7 +47,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
 
   // From IF stage
   input  logic [31:0] pc_if_i,
-  input  logic        first_op_if_i,
   input  logic        last_op_if_i,
 
   // from IF/ID pipeline
@@ -60,9 +59,9 @@ module cv32e40x_controller import cv32e40x_pkg::*;
   input  logic        csr_en_raw_id_i,
   input  csr_opcode_e csr_op_id_i,
   input  logic        first_op_id_i,
+  input  logic        last_op_id_i,
 
   input  id_ex_pipe_t id_ex_pipe_i,
-  input  logic        first_op_ex_i,
 
   input  ex_wb_pipe_t ex_wb_pipe_i,
 
@@ -146,7 +145,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
 
     .if_valid_i                  ( if_valid_i               ),
     .pc_if_i                     ( pc_if_i                  ),
-    .first_op_if_i               ( first_op_if_i            ),
     .last_op_if_i                ( last_op_if_i             ),
 
     // From ID stage
@@ -158,14 +156,14 @@ module cv32e40x_controller import cv32e40x_pkg::*;
     .alu_en_id_i                 ( alu_en_id_i              ),
     .sys_en_id_i                 ( sys_en_id_i              ),
     .first_op_id_i               ( first_op_id_i            ),
+    .last_op_id_i                ( last_op_id_i             ),
 
     // From EX stage
     .id_ex_pipe_i                ( id_ex_pipe_i             ),
     .branch_decision_ex_i        ( branch_decision_ex_i     ),
     .ex_ready_i                  ( ex_ready_i               ),
     .ex_valid_i                  ( ex_valid_i               ),
     .last_op_ex_i                ( last_op_ex_i             ),
-    .first_op_ex_i               ( first_op_ex_i            ),
 
     // From WB stage
     .ex_wb_pipe_i                ( ex_wb_pipe_i             ),

rtl/cv32e40x_controller_fsm.sv

@@ -48,7 +48,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
 
   // From IF stage
   input  logic [31:0] pc_if_i,
-  input  logic        first_op_if_i,              // IF stage is handling the first operation of a sequence.
   input  logic        last_op_if_i,               // IF stage is handling the last operation of a sequence.
 
   // From ID stage
@@ -58,12 +57,12 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
   input  logic        alu_en_id_i,                // alu_en qualifier for jumps
   input  logic        sys_en_id_i,                // sys_en qualifier for mret
   input  logic        first_op_id_i,              // ID stage is handling the first operation of a sequence
+  input  logic        last_op_id_i,               // ID stage is handling the last operation of a sequence
 
   // From EX stage
   input  id_ex_pipe_t id_ex_pipe_i,
   input  logic        branch_decision_ex_i,       // branch decision signal from EX ALU
   input  logic        last_op_ex_i,               // EX stage contains the last operation of an instruction
-  input  logic        first_op_ex_i,              // EX stage is handling the first operation of a sequence
 
   // From WB stage
   input  ex_wb_pipe_t ex_wb_pipe_i,
@@ -218,12 +217,18 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
   logic       csr_flush_ack_q;
 
   // Flag for checking if multi op instructions are in an interruptible state
+  logic       sequence_in_progress_wb;
   logic       sequence_interruptible;
 
   // Flag for checking if ID stage can be halted
   // Used to not halt sequences in the middle, potentially causing deadlocks
+  logic       sequence_in_progress_id;
   logic       id_stage_haltable;
 
+  assign sequence_interruptible = !sequence_in_progress_wb;
+
+  assign id_stage_haltable = !sequence_in_progress_id;
+
   assign fencei_ready = !lsu_busy_i;
 
   // Once the fencei handshake is initiated, it must complete and the instruction must retire.
@@ -463,36 +468,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
   assign ctrl_fsm_o.mhpmevent.id_ld_stall   = ctrl_byp_i.load_stall && !id_valid_i && ex_ready_i;
   assign ctrl_fsm_o.mhpmevent.wb_data_stall = data_stall_wb_i;
 
-  // Detect if we are in the middle of a multi operation sequence
-  // Basically check if the oldest instruction in the pipeline is a 'first_op'
-  // - if not, we have an unfinished sequence and cannot interrupt it.
-  // Used to determine if we are allowed to take debug or interrupts
-  always_comb begin
-    if (ex_wb_pipe_i.instr_valid) begin
-      sequence_interruptible = ex_wb_pipe_i.first_op;
-    end else if (id_ex_pipe_i.instr_valid) begin
-      sequence_interruptible = first_op_ex_i;
-    end else if (if_id_pipe_i.instr_valid) begin
-      sequence_interruptible = first_op_id_i;
-    end else begin
-      sequence_interruptible = first_op_if_i; // todo: first/last op should always be classified with instr_vali; if not it needs proper explanation.
-    end
-  end
 
-  // Check if we are allowed to halt the ID stage.
-  // If ID stage contains a non-first operation, we cannot halt ID as that
-  // could cause a deadlock because a sequence cannot finish through ID.
-  // If ID stage does not contain a valid instruction, the same check is performed
-  // for the IF stage (although this is likely not needed).
-  always_comb begin
-    if (if_id_pipe_i.instr_valid) begin
-      id_stage_haltable = first_op_id_i;
-    end else begin
-      // IF stage first_op defaults to 1'b1 if the stage does not hold a valid instruction or
-      // table jump pointer. Table jump pointers will have first_op set to 0.
-      id_stage_haltable = first_op_if_i; // todo: first/last op should always be classified with instr_valid; if not it needs proper explanation (and I would prefer if we get rid of this special reliance on the 1'b1 default)
-    end
-  end
 
   // Mux used to select PC from the different pipeline stages
   always_comb begin
@@ -1082,6 +1058,53 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
     end
   end
 
+  // Instruction sequences may not be interrupted/killed if the first operation has already been retired.
+  // Flop set when first_op is done in WB, and cleared when last_op is done, or WB is killed
+  always_ff @(posedge clk, negedge rst_n) begin
+    if (rst_n == 1'b0) begin
+      sequence_in_progress_wb <= 1'b0;
+    end else begin
+      if (!sequence_in_progress_wb) begin
+        if (wb_valid_i && ex_wb_pipe_i.first_op && !last_op_wb_i) begin
+          sequence_in_progress_wb <= 1'b1;
+        end
+      end else begin
+        // sequence_in_progress_wb is set, clear when last_op retires
+        if (wb_valid_i && last_op_wb_i) begin
+          sequence_in_progress_wb <= 1'b0;
+        end
+        // Needed in the future, but commented out to make a SEC clean PR.
+        //if (ctrl_fsm_o.kill_wb) begin
+        //  sequence_in_progress_wb <= 1'b0;
+        //end
+      end
+    end
+  end
+
+  // Helper logic to track first_op and last_op through the ID stage to detect unhaltable sequences
+  // Flop set when first_op is done in ID, and cleared when last_op is done, or ID is killed
+  always_ff @(posedge clk, negedge rst_n) begin
+    if (rst_n == 1'b0) begin
+      sequence_in_progress_id <= 1'b0;
+    end else begin
+      if (!sequence_in_progress_id) begin
+        if (id_valid_i && ex_ready_i && first_op_id_i && !last_op_id_i) begin
+          sequence_in_progress_id <= 1'b1;
+        end
+      end else begin
+        // sequence_in_progress_id is set, clear when last_op retires
+        if (id_valid_i && ex_ready_i && last_op_id_i) begin
+          sequence_in_progress_id <= 1'b0;
+        end
+      end
+
+      // Reset flag if ID stage is killed
+      if (ctrl_fsm_o.kill_id) begin
+        sequence_in_progress_id <= 1'b0;
+      end
+    end
+  end
+
 
   /////////////////////
   // Debug state FSM //

rtl/cv32e40x_core.sv

@@ -181,6 +181,7 @@ module cv32e40x_core import cv32e40x_pkg::*;
 
   // Detect last_op
   logic        last_op_if;
+  logic        last_op_id;
   logic        last_op_ex;
   logic        last_op_wb;
 
@@ -506,6 +507,7 @@ module cv32e40x_core import cv32e40x_pkg::*;
     .sys_en_o                     ( sys_en_id                 ),
 
     .first_op_o                   ( first_op_id               ),
+    .last_op_o                    ( last_op_id                ),
 
     .rf_re_o                      ( rf_re_id                  ),
     .rf_raddr_o                   ( rf_raddr_id               ),
@@ -800,7 +802,6 @@ module cv32e40x_core import cv32e40x_pkg::*;
 
     // From ID/EX pipeline
     .id_ex_pipe_i                   ( id_ex_pipe             ),
-    .first_op_ex_i                  ( first_op_ex            ),
 
     .csr_counter_read_i             ( csr_counter_read       ),
     .csr_mnxti_read_i               ( csr_mnxti_read         ),
@@ -809,12 +810,12 @@ module cv32e40x_core import cv32e40x_pkg::*;
     .ex_wb_pipe_i                   ( ex_wb_pipe             ),
 
     // last_op bits
+    .last_op_id_i                   ( last_op_id             ),
     .last_op_ex_i                   ( last_op_ex             ),
     .last_op_wb_i                   ( last_op_wb             ),
 
     .if_valid_i                     ( if_valid               ),
     .pc_if_i                        ( pc_if                  ),
-    .first_op_if_i                  ( first_op_if            ),
     .last_op_if_i                   ( last_op_if             ),
     // from IF/ID pipeline
     .if_id_pipe_i                   ( if_id_pipe             ),

rtl/cv32e40x_id_stage.sv

@@ -77,6 +77,7 @@ module cv32e40x_id_stage import cv32e40x_pkg::*;
   output logic        sys_en_o,
 
   output logic        first_op_o,
+  output logic        last_op_o,
 
   // RF interface -> controller
   output logic [REGFILE_NUM_READ_PORTS-1:0] rf_re_o,
@@ -679,6 +680,7 @@ module cv32e40x_id_stage import cv32e40x_pkg::*;
   assign id_valid_o = (instr_valid && !xif_waiting) || (multi_cycle_id_stall && !ctrl_fsm_i.kill_id && !ctrl_fsm_i.halt_id);
 
   assign first_op_o = if_id_pipe_i.first_op;
+  assign last_op_o  = if_id_pipe_i.last_op;
   //---------------------------------------------------------------------------
   // eXtension interface
   //---------------------------------------------------------------------------

rtl/cv32e40x_if_stage.sv

@@ -291,8 +291,6 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
 
   // Acknowledge prefetcher when IF stage is ready. This factors in seq_ready to avoid ack'ing the
   // prefetcher in the middle of a Zc sequence.
-  // No need to ack the prefetcher when tbljmp==1, as this is will generate a pc_set and kill_if when
-  // the table jump is taken in ID and the pointer fetch is initiated (or the table jump is killed).
   assign prefetch_ready = if_ready && !tbljmp;
 
   // Last operation of table jumps are set when the pointer is fed to ID stage
@@ -315,7 +313,8 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
   // todo: The treatement of trigger match causes 1 instruction to possibly have first/last op set multiple times. Can this be avoided (it messes up the semantics of first/last op)?
   // todo: factor in CLIC pointers?
   assign first_op_o = prefetch_is_tbljmp_ptr ? 1'b0 :
-                      trigger_match_i        ? 1'b1 : seq_first;
+                      trigger_match_i        ? 1'b1 :
+                      seq_valid              ? seq_first : 1'b1;
 
   // todo: first/last op need to be treated in the same manner (assignments need to look more similar). Also seq_first/seq_last need cleaner semantics with respect ot how they relate to seq_valid.
 

sva/cv32e40x_controller_fsm_sva.sv

@@ -79,11 +79,15 @@ module cv32e40x_controller_fsm_sva
   input logic           csr_wr_in_wb_flush_i,
   input logic [2:0]     debug_cause_q,
   input logic           id_valid_i,
+  input logic           first_op_if_i,
   input logic           first_op_id_i,
+  input logic           first_op_ex_i,
   input logic           last_op_id_i,
   input logic           ex_ready_i,
   input logic           sequence_interruptible,
-  input logic           id_stage_haltable
+  input logic           id_stage_haltable,
+  input logic           instr_valid_if_i,
+  input logic           ptr_in_if_i
 );
 
 
@@ -538,73 +542,72 @@ if (SMCLIC) begin
 
 end // SMCLIC
 
-  // Instruction sequences may not be interrupted/killed if the first operation has already been retired.
-  // Helper logic to track first_op and last_op through the WB stage to detect uninterruptible sequences
-  logic first_op_done_wb;
-  always_ff @(posedge clk, negedge rst_n) begin
-    if (rst_n == 1'b0) begin
-      first_op_done_wb <= 1'b0;
+
+
+  // Detect if we are in the middle of a multi operation sequence
+  // Basically check if the oldest instruction in the pipeline is a 'first_op'
+  // - if not, we have an unfinished sequence and cannot interrupt it.
+  // Used to determine if we are allowed to take debug or interrupts
+  logic sequence_interruptible_alt;
+  always_comb begin
+    if (ex_wb_pipe_i.instr_valid) begin
+      sequence_interruptible_alt = ex_wb_pipe_i.first_op;
+    end else if (id_ex_pipe_i.instr_valid) begin
+      sequence_interruptible_alt = first_op_ex_i;
+    end else if (if_id_pipe_i.instr_valid) begin
+      sequence_interruptible_alt = first_op_id_i;
+    end else if (instr_valid_if_i) begin
+      sequence_interruptible_alt = first_op_if_i;
     end else begin
-      if (!first_op_done_wb) begin
-        if (wb_valid_i && ex_wb_pipe_i.first_op && !last_op_wb_i) begin
-          first_op_done_wb <= 1'b1;
-        end
-      end else begin
-        // first_op_done_wb is set, clear when last_op retires
-        if (wb_valid_i && last_op_wb_i) begin
-          first_op_done_wb <= 1'b0;
-        end
-      end
+      // If no instruction is ready in the whole pipeline (including IF), then there are nothing in progress
+      // and we should safely be able to interrupt unless the IF stage is waiting for a table jump pointer
+      sequence_interruptible_alt = !ptr_in_if_i;
     end
   end
 
-// todo: make above code look the same as below code (add kill_wb related logic)
-
-  // Helper logic to track first_op and last_op through the ID stage to detect unhaltable sequences
-  logic first_op_done_id;
-  always_ff @(posedge clk, negedge rst_n) begin
-    if (rst_n == 1'b0) begin
-      first_op_done_id <= 1'b0;
+  // Check if we are allowed to halt the ID stage.
+  // If ID stage contains a non-first operation, we cannot halt ID as that
+  // could cause a deadlock because a sequence cannot finish through ID.
+  // If ID stage does not contain a valid instruction, the same check is performed
+  // for the IF stage (although this is likely not needed).
+  logic id_stage_haltable_alt;
+  always_comb begin
+    if (if_id_pipe_i.instr_valid) begin
+      id_stage_haltable_alt = first_op_id_i;
+    end else if (instr_valid_if_i) begin
+      id_stage_haltable_alt = first_op_if_i;
     end else begin
-      if (!first_op_done_id) begin
-        if (id_valid_i && ex_ready_i && first_op_id_i && !last_op_id_i) begin
-          first_op_done_id <= 1'b1;
-        end
-      end else begin
-        // first_op_done_id is set, clear when last_op retires
-        if (id_valid_i && ex_ready_i && last_op_id_i) begin
-          first_op_done_id <= 1'b0;
-        end
-      end
-      // Reset flag if ID stage is killed
-      if (ctrl_fsm_o.kill_id) begin
-        first_op_done_id <= 1'b0;
-      end
+      // If no instruction is ready in the whole pipeline (including IF), then there are nothing in progress
+      // and we should safely be able to halt ID unless the IF stage is waiting for a table jump pointer
+      id_stage_haltable_alt = !ptr_in_if_i;
     end
   end
 
   a_no_sequence_interrupt:
   assert property (@(posedge clk) disable iff (!rst_n)
-                    (first_op_done_wb |-> !ctrl_fsm_o.irq_ack))
+                    (!sequence_interruptible_alt |-> !ctrl_fsm_o.irq_ack))
     else `uvm_error("controller", "Sequence broken by interrupt")
 
-// todo: add equivalency check related to first_op_done_wb computation and sequence_interruptible
+  a_interruptible_equivalency:
+  assert property (@(posedge clk) disable iff (!rst_n)
+                    (sequence_interruptible_alt == sequence_interruptible))
+    else `uvm_error("controller", "first_op_done_wb not matching !sequence_interruptible")
 
   a_no_sequence_nmi:
   assert property (@(posedge clk) disable iff (!rst_n)
-                    (first_op_done_wb |-> !(ctrl_fsm_o.pc_set && (ctrl_fsm_o.pc_mux == PC_TRAP_NMI))))
+                    (!sequence_interruptible_alt |-> !(ctrl_fsm_o.pc_set && (ctrl_fsm_o.pc_mux == PC_TRAP_NMI))))
     else `uvm_error("controller", "Sequence broken by NMI")
 
   a_no_sequence_ext_debug:
   assert property (@(posedge clk) disable iff (!rst_n)
-                    (first_op_done_wb |-> !(ctrl_fsm_o.dbg_ack && (debug_cause_q == DBG_CAUSE_HALTREQ)))) // todo: timing of (debug_cause_q == DBG_CAUSE_HALTREQ) seems wrong (and whole calsuse should not be needed)
+                    (!sequence_interruptible_alt |-> !(ctrl_fsm_o.dbg_ack && (debug_cause_q == DBG_CAUSE_HALTREQ)))) // todo: timing of (debug_cause_q == DBG_CAUSE_HALTREQ) seems wrong (and whole calsuse should not be needed)
     else `uvm_error("controller", "Sequence broken by external debug")
 
   // Check that we do not allow ID stage to be halted for pending interrupts/debug if a sequence is not done
   // in the ID stage.
   a_id_stage_haltable:
   assert property (@(posedge clk) disable iff (!rst_n)
-                    (first_op_done_id |-> !id_stage_haltable)) // todo: proof equivalency, not just implication
+                    (id_stage_haltable_alt == id_stage_haltable))
     else `uvm_error("controller", "id_stage_haltable not correct")
 
   // Assert that we have no pc_set in the same cycle as a CSR write in WB requires flushing of the pipeline