diff --git a/bhv/cv32e40x_rvfi.sv b/bhv/cv32e40x_rvfi.sv index aa0f8c41..23587733 100644 --- a/bhv/cv32e40x_rvfi.sv +++ b/bhv/cv32e40x_rvfi.sv @@ -104,7 +104,6 @@ module cv32e40x_rvfi input ctrl_state_e ctrl_fsm_cs_i, input ctrl_state_e ctrl_fsm_ns_i, input logic pending_single_step_i, - input logic pending_single_step_ptr_i, input logic single_step_allowed_i, input logic nmi_pending_i, // regular NMI pending input logic nmi_is_store_i, // regular NMI type @@ -775,7 +774,7 @@ module cv32e40x_rvfi end - if((pending_single_step_i || pending_single_step_ptr_i) && single_step_allowed_i) begin + if(pending_single_step_i && single_step_allowed_i) begin // The timing of the single step debug entry does not allow using pc_mux for detection rvfi_trap_next.debug = 1'b1; rvfi_trap_next.debug_cause = DBG_CAUSE_STEP; diff --git a/bhv/cv32e40x_wrapper.sv b/bhv/cv32e40x_wrapper.sv index 19fc26fc..70a3a7d1 100644 --- a/bhv/cv32e40x_wrapper.sv +++ b/bhv/cv32e40x_wrapper.sv @@ -280,6 +280,7 @@ module cv32e40x_wrapper .ctrl_debug_allowed (core_i.controller_i.controller_fsm_i.debug_allowed), .ctrl_pending_interrupt (core_i.controller_i.controller_fsm_i.pending_interrupt), .ctrl_interrupt_allowed (core_i.controller_i.controller_fsm_i.interrupt_allowed), + .ctrl_debug_cause_n (core_i.controller_i.controller_fsm_i.debug_cause_n), .id_stage_multi_cycle_id_stall (core_i.id_stage_i.multi_cycle_id_stall), .id_stage_id_valid (core_i.id_stage_i.id_valid_o), @@ -484,7 +485,6 @@ endgenerate .ctrl_fsm_cs_i ( core_i.controller_i.controller_fsm_i.ctrl_fsm_cs ), .ctrl_fsm_ns_i ( core_i.controller_i.controller_fsm_i.ctrl_fsm_ns ), .pending_single_step_i ( core_i.controller_i.controller_fsm_i.pending_single_step ), - .pending_single_step_ptr_i( core_i.controller_i.controller_fsm_i.pending_single_step_ptr ), .single_step_allowed_i ( core_i.controller_i.controller_fsm_i.single_step_allowed ), .nmi_pending_i ( core_i.controller_i.controller_fsm_i.nmi_pending_q ), .nmi_is_store_i ( core_i.controller_i.controller_fsm_i.nmi_is_store_q ), diff --git a/rtl/cv32e40x_controller_fsm.sv b/rtl/cv32e40x_controller_fsm.sv index 28e367eb..9c99f184 100644 --- a/rtl/cv32e40x_controller_fsm.sv +++ b/rtl/cv32e40x_controller_fsm.sv @@ -172,12 +172,12 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; logic ebreak_in_wb; logic trigger_match_in_wb; logic xif_in_wb; + logic clic_ptr_in_wb; logic pending_nmi; logic pending_nmi_early; logic pending_debug; logic pending_single_step; - logic pending_single_step_ptr; logic pending_interrupt; // Flags for allowing interrupt and debug @@ -349,6 +349,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; // An offloaded instruction is in WB assign xif_in_wb = (ex_wb_pipe_i.xif_en && ex_wb_pipe_i.instr_valid); + assign clic_ptr_in_wb = (ex_wb_pipe_i.instr_meta.clic_ptr && ex_wb_pipe_i.instr_valid); + // Pending NMI // Using flopped version to avoid paths from data_err_i/data_rvalid_i to instr_* outputs assign pending_nmi = nmi_pending_q; @@ -385,28 +387,23 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; For any interruptible instructions (non-LSU), at any stage, we would kill the instruction and jump to debug mode without executing any instructions. Interrupt handler's first instruction will be in dpc. - For LSU instructions that may not be killed (if they reach WB of stay in EX for >1 cycles), + For LSU instructions that may not be killed (if they reach WB or stay in EX for >1 cycles), we are not allowed to take interrupts, and we will re-enter debug mode after finishing the LSU. Interrupt will then be taken when we enter the next step. */ assign non_shv_irq_ack = ctrl_fsm_o.irq_ack && !irq_clic_shv_i; - // single step becomes pending when the last operation of an instruction is done in WB, or we ack a non-shv interrupt. - // If a CLIC SHV interrupt is taken during single step, the flag 'pending_single_step_ptr' will be used once the pointer - // reaches the ID stage. If the pointer fetch fails, an exception will be taken from WB and debug mode is entered with dpc - // pointing at the first instruction of the exception handler. + // single step becomes pending when the last operation of an instruction is done in WB (including CLIC pointers), or we ack a non-shv interrupt. + // If a CLIC SHV interrupt is taken during single step, a pointer that reaches WB will trigger the debug entry. + // - For un-faulted pointer fetches, the second fetch of the CLIC vectoring took place in ID, and the final SHV handler target address will be available from IF. + // - A faulted pointer fetch does not perform the second fetch. Instead the exception handler fetch will occur before entering debug due to stepping. + // todo: Likely flop the wb_valid/last_op/abort_op to be able to evaluate all debug reasons (debug_req when pointer is in WB is not allowed, while single step is allowed) assign pending_single_step = (!debug_mode_q && dcsr_i.step && ((wb_valid_i && (last_op_wb_i || abort_op_wb_i)) || non_shv_irq_ack)) && !pending_debug; - // Separate flag for pending single step when doing CLIC SHV. - // When a successfully fetched pointer reaches the ID stage, the controller will perform the pc_set of the target instruction, - // and then go to the DEBUG_TAKEN state with single step as the cause. The dpc CSR will point at the first instruction - // of the interrupt handler. No instruction will be retired during the step. - assign pending_single_step_ptr = !debug_mode_q && dcsr_i.step && (wb_valid_i || 1'b1) && !pending_debug; - // Detect if there is a live CLIC pointer in the pipeline - // This should block debug + // This should block debug and interrupts generate if (SMCLIC) begin : gen_clic_pointer_flag // We only need to check EX and WB, as the FSM will only be in FUNCTIONAL state @@ -422,9 +419,10 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; // LSU will not be interruptible if the outstanding counter != 0, or // a trans_valid has been clocked without ex_valid && wb_ready handshake. // 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) + // Any multi operation instruction (table jumps, push/pop and double moves) may not be interrupted once the first operation has completed its operation in WB. + // - This is guarded with using the sequence_interruptible, which tracks sequence progress through the WB stage. + // When a CLIC pointer is in the pipeline stages EX or WB, we must block debug. + // - Debug would otherwise kill the pointer and use the address of the pointer for dpc. A following dret would then return to the mtvt table, losing program progress. 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 @@ -436,8 +434,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; // Determine cause of debug // pending_single_step may only happen if no other causes for debug are true. // The flopped version of this is checked during DEBUG_TAKEN state (one cycle delay) + // todo: update priority according to updated debug spec assign debug_cause_n = pending_single_step ? DBG_CAUSE_STEP : - pending_single_step_ptr ? DBG_CAUSE_STEP : trigger_match_in_wb ? DBG_CAUSE_TRIGGER : (ebreak_in_wb && dcsr_i.ebreakm && !debug_mode_q) ? DBG_CAUSE_EBREAK : DBG_CAUSE_HALTREQ; @@ -456,13 +454,15 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; // will be interruptable as they were converted to NOP in ID stage. // The cycle after fencei enters WB, the fencei handshake will be initiated. This must complete and the fencei instruction must retire before allowing interrupts. // TODO:OK:low May allow interuption of Zce to idempotent memories - // Once the first part of a table jump has finished in WB, we are not allowed to take interrupts before the last part finishes. This can be detected when the last - // part of a table jump is in either EX or WB. + // Any multi operation instruction (table jumps, push/pop and double moves) may not be interrupted once the first operation has completed its operation in WB. + // - This is guarded with using the sequence_interruptible, which tracks sequence progress through the WB stage. + // When a CLIC pointer is in the pipeline stages EX or WB, we must block interrupts. + // - Interrupt would otherwise kill the pointer and use the address of the pointer for mepc. A following mret would then return to the mtvt table, losing program progress. - assign interrupt_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !xif_in_wb && sequence_interruptible && !interrupt_blanking_q; + assign interrupt_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !xif_in_wb && !clic_ptr_in_pipeline && sequence_interruptible && !interrupt_blanking_q; // Allowing NMI's follow the same rule as regular interrupts, except we don't need to regard blanking of NMIs after a load/store. - assign nmi_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !xif_in_wb && sequence_interruptible; + assign nmi_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !xif_in_wb && !clic_ptr_in_pipeline && sequence_interruptible; // Do not allow interrupts if in debug mode, or single stepping without dcsr.stepie set. assign debug_interruptible = !(debug_mode_q || (dcsr_i.step && !dcsr_i.stepie)); @@ -874,6 +874,15 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; if (mret_in_wb && !ctrl_fsm_o.kill_wb) begin ctrl_fsm_o.csr_restore_mret = !debug_mode_q; end + + // CLIC pointer in WB + if(clic_ptr_in_wb && !ctrl_fsm_o.kill_wb) begin + // Clear minhv if no exceptions are associated with the pointer + // todo: deal with integrity related faults for E40S. + if(!((ex_wb_pipe_i.instr.mpu_status != MPU_OK) || ex_wb_pipe_i.instr.bus_resp.err)) begin + ctrl_fsm_o.csr_clear_minhv = 1'b1; + end + end end // !debug or interrupts // Single step debug entry @@ -901,7 +910,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; ctrl_fsm_ns = FUNCTIONAL; ctrl_fsm_o.ctrl_busy = 1'b1; // Keep IF/ID halted while waking up (EX contains a bubble) - // Any jump/tablejump/mret which is in ID in this cycle must also remain in ID + // Any jump/table jump/mret which is in ID in this cycle must also remain in ID // the next cycle for their side effects to be taken during the FUNCTIONAL state in case the interrupt is not actually taken. ctrl_fsm_o.halt_id = 1'b1; @@ -973,33 +982,24 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; // is waiting for the pointer to arrive in the decode stage. POINTER_FETCH: begin if (if_id_pipe_i.instr_meta.clic_ptr && if_id_pipe_i.instr_valid) begin - // Function pointer reached ID stage, do another jump - // if no faults happened during pointer fetch. (mcause.minhv will stay high for faults) + // Function pointer reached ID stage, do another jump if no faults happened during pointer fetch. // todo: deal with integrity related faults for E40S. if(!((if_id_pipe_i.instr.mpu_status != MPU_OK) || if_id_pipe_i.instr.bus_resp.err)) begin ctrl_fsm_o.pc_set = 1'b1; ctrl_fsm_o.pc_mux = PC_POINTER; ctrl_fsm_o.kill_if = 1'b1; - ctrl_fsm_o.csr_clear_minhv = 1'b1; - - // Jump to debug taken in case of a pending single step. - // We should enter debug without executing any instruction, and let dpc - // point to the first instruction in the handler - ctrl_fsm_ns = pending_single_step_ptr ? DEBUG_TAKEN : FUNCTIONAL; - end else begin - // If the pointer fetch faulted, we don't jump to the target and return to FUNCTIONAL. - // Any pending single step will not be taken (the irq handler instruction fetch faulted), - // and the associated exception or NMI will be taken in FUNCTIONAL along with the single step once - // the pointer reaches WB. Dpc will then point to the first instruction in the exception/NMI handler. - ctrl_fsm_ns = FUNCTIONAL; end - // Note: If the pointer fetch faulted (pma/pmp/bus error), an exception or NMI will - // be taken once the pointer fetch reachces WB (two cycles after the current) - // The FSM must be in the FUNCTIONAL state to take the exception or NMI. - // A faulted pointer (in ID) should not cause debug entry either, + // Return to FUNCTIONAL once the pointer reaches the ID stage. + // If the pointer has any exceptions, the exception will be handled from the WB stage in the FUNCTIONAL state. + // If single stepping with dcsr.stepie, the single step debug entry will happen from FUNCTIONAL once the + // pointer reaches the WB stage. + ctrl_fsm_ns = FUNCTIONAL; end // Mret in WB restores CSR regs + // If the pointer fetch was (re)started by an mret with mcause.minhv set, the mret may + // be in the WB stage while in the POINTER_FETCH stage. The CSR side effects of the mret + // must still take place. if (mret_in_wb && !ctrl_fsm_o.kill_wb) begin ctrl_fsm_o.csr_restore_mret = !debug_mode_q; end diff --git a/rtl/cv32e40x_cs_registers.sv b/rtl/cv32e40x_cs_registers.sv index ad692538..1a34778e 100644 --- a/rtl/cv32e40x_cs_registers.sv +++ b/rtl/cv32e40x_cs_registers.sv @@ -1060,27 +1060,19 @@ module cv32e40x_cs_registers import cv32e40x_pkg::*; mstatus_we = 1'b1; end //ctrl_fsm_i.csr_restore_dret + + ctrl_fsm_i.csr_clear_minhv: begin + if (SMCLIC) begin + // Keep mcause values, only clear minhv bit. + mcause_n = mcause_rdata; + mcause_n.minhv = 1'b0; + mcause_we = 1'b1; + end // SMCLIC + end // ctrl_fsm_i.csr_clear_minhv 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 diff --git a/sva/cv32e40x_controller_fsm_sva.sv b/sva/cv32e40x_controller_fsm_sva.sv index 3c465ff4..3a17e324 100644 --- a/sva/cv32e40x_controller_fsm_sva.sv +++ b/sva/cv32e40x_controller_fsm_sva.sv @@ -96,7 +96,8 @@ module cv32e40x_controller_fsm_sva input logic irq_wu_ctrl_i, input logic wu_wfe_i, input logic sys_en_id_i, - input logic sys_mret_id_i + input logic sys_mret_id_i, + input logic clic_ptr_in_wb ); @@ -604,6 +605,13 @@ if (SMCLIC) begin !(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") + a_clic_ptr_functional_only: + assert property (@(posedge clk) disable iff (!rst_n) + ((ctrl_fsm_cs != FUNCTIONAL) + |-> + !(clic_ptr_in_wb && !ctrl_fsm_o.kill_wb))) + else `uvm_error("controller", "clic_ptr_in_wb && !kill_wb while not in FUNCTIONAL state.") + end else begin // SMCLIC // Check that CLIC related signals are inactive when CLIC is not configured. a_clic_inactive: @@ -769,19 +777,22 @@ end $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") -*/ + + + // mret CSR restores are done in parallell with other events in the controller except for nmi/interrupt/debug entries. + // Check that CSR restores for mret cannot happen while the WB stage is halted. + a_mret_restore_halt: + assert property (@(posedge clk) disable iff (!rst_n) + ctrl_fsm_o.csr_restore_mret + |-> + !ctrl_fsm_o.halt_wb) + else `uvm_error("controller", "csr_restore_mret when WB is halted") endmodule diff --git a/sva/cv32e40x_core_sva.sv b/sva/cv32e40x_core_sva.sv index 94f2cf7e..f0213c61 100644 --- a/sva/cv32e40x_core_sva.sv +++ b/sva/cv32e40x_core_sva.sv @@ -80,6 +80,7 @@ module cv32e40x_core_sva input logic ctrl_interrupt_allowed, input logic ctrl_pending_interrupt, input ctrl_byp_t ctrl_byp, + input logic [2:0] ctrl_debug_cause_n, // probed cs_registers signals input logic [31:0] cs_registers_mie_q, input logic [31:0] cs_registers_mepc_n, @@ -346,18 +347,27 @@ if (SMCLIC) begin else `uvm_error("core", "Assertion a_single_step_with_irq_nonshv failed") // An SHV CLIC interrupt will first do one fetch to get a function pointer, - // then a second fetch to the actual interrupt handler. If this second fetch has - // no faults, debug is entered with dpc pointing to the handler entry. - // Otherwise, if the pointer fetch failed, we will eventually end up in debug mode - // with dpc pointing to the respective exception/NMI handler. - // In any way, wb_valid shall remain low until we retire the first instruction in debug mode. + // then a second fetch to the actual interrupt handler. If the first fetch has + // no faults, debug is entered with dpc pointing to the handler entry when the pointer reaches the WB stage. + // Otherwise, if the pointer fetch failed, we will start fetching the appropriate exception handler + // before entering debug with DPC pointing to the first exception handler instruction. + // External debug entry and interrupts (including NMIs) are not allowed to be taken while there is + // a live pointer in WB (IF-ID: guarded by POINTER_FETCH STATE, EX-WB: guarded by clic_ptr_in_pipeline). + // - this could cause the address of the pointer to end up in DPC, making dret jumping to a mtvt entry instead of an instruction. + /* + todo: Reintroduce (and update) when debug single step logic has been updated. + -should likely flop the event that causes single step entry to evaluate all debug reasons + when the pipeline is guaranteed to not disallow any debug reason to enter debug. a_single_step_with_irq_shv : assert property (@(posedge clk) disable iff (!rst_ni) (dcsr.step && !ctrl_fsm.debug_mode && irq_ack && irq_clic_shv) |-> - !wb_valid until (wb_valid && ctrl_fsm.debug_mode && dcsr.step)) + (!wb_valid until (wb_valid && ex_wb_pipe.instr_meta.clic_ptr) // CLIC pointer in WB, enter DEBUG_TAKEN + ##1 ctrl_debug_mode_n) // Debug mode from next cycle + or + (!wb_valid until (ctrl_debug_mode_n && (ctrl_debug_cause_n == DBG_CAUSE_HALTREQ)))) // external debug happened before pointer reached WB else `uvm_error("core", "Assertion a_single_step_with_irq_shv failed") - +*/ end else begin // Interrupt taken during single stepping. diff --git a/sva/cv32e40x_cs_registers_sva.sv b/sva/cv32e40x_cs_registers_sva.sv index af8f8ddd..93b9ee79 100644 --- a/sva/cv32e40x_cs_registers_sva.sv +++ b/sva/cv32e40x_cs_registers_sva.sv @@ -158,28 +158,5 @@ module cv32e40x_cs_registers_sva 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