diff --git a/bhv/cv32e40x_wrapper.sv b/bhv/cv32e40x_wrapper.sv index c094aa4a..c413c508 100644 --- a/bhv/cv32e40x_wrapper.sv +++ b/bhv/cv32e40x_wrapper.sv @@ -203,13 +203,17 @@ module cv32e40x_wrapper cv32e40x_controller_fsm_sva #(.X_EXT(X_EXT)) controller_fsm_sva ( - .lsu_outstanding_cnt (core_i.load_store_unit_i.cnt_q), - .rf_we_wb_i (core_i.wb_stage_i.rf_we_wb_o ), - .csr_we_i (core_i.cs_registers_i.csr_we_int ), - .csr_illegal_i (core_i.cs_registers_i.csr_illegal_o), - .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), + .lsu_outstanding_cnt (core_i.load_store_unit_i.cnt_q), + .rf_we_wb_i (core_i.wb_stage_i.rf_we_wb_o ), + .csr_we_i (core_i.cs_registers_i.csr_we_int ), + .csr_illegal_i (core_i.cs_registers_i.csr_illegal_o), + .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), + .prefetch_valid_if_i (core_i.if_stage_i.prefetch_valid), + .prefetch_is_tbljmp_ptr_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 (.*); diff --git a/rtl/cv32e40x_controller.sv b/rtl/cv32e40x_controller.sv index 64007085..ea29bf2e 100644 --- a/rtl/cv32e40x_controller.sv +++ b/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,6 +156,7 @@ 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 ), @@ -165,7 +164,6 @@ module cv32e40x_controller import cv32e40x_pkg::*; .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 ), diff --git a/rtl/cv32e40x_controller_fsm.sv b/rtl/cv32e40x_controller_fsm.sv index 635b49a5..52827e52 100644 --- a/rtl/cv32e40x_controller_fsm.sv +++ b/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 // wb_valid implies ex_wb_pipe.instr_valid + 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 + + // 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 // id_valid implies if_id_pipe.instr.valid + 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 // diff --git a/rtl/cv32e40x_core.sv b/rtl/cv32e40x_core.sv index 5c624097..4343c079 100644 --- a/rtl/cv32e40x_core.sv +++ b/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 ), diff --git a/rtl/cv32e40x_id_stage.sv b/rtl/cv32e40x_id_stage.sv index dfcbe755..987bf6a3 100644 --- a/rtl/cv32e40x_id_stage.sv +++ b/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 //--------------------------------------------------------------------------- diff --git a/rtl/cv32e40x_if_stage.sv b/rtl/cv32e40x_if_stage.sv index e87e01e5..803ee0da 100644 --- a/rtl/cv32e40x_if_stage.sv +++ b/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. diff --git a/sva/cv32e40x_controller_fsm_sva.sv b/sva/cv32e40x_controller_fsm_sva.sv index 4509e1f5..e25ca5f7 100644 --- a/sva/cv32e40x_controller_fsm_sva.sv +++ b/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 prefetch_valid_if_i, + input logic prefetch_is_tbljmp_ptr_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 (prefetch_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 = !prefetch_is_tbljmp_ptr_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 (prefetch_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 = !prefetch_is_tbljmp_ptr_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