Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single step and debug cause cleanup #719

Merged
merged 1 commit into from
Dec 8, 2022
Merged

Single step and debug cause cleanup #719

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion bhv/cv32e40x_rvfi.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ module cv32e40x_rvfi
input logic nmi_pending_i, // regular NMI pending
input logic nmi_is_store_i, // regular NMI type
input logic debug_mode_q_i,
input logic [2:0] debug_cause_n_i,

// Interrupt Controller probes
input logic [31:0] irq_i,
Expand Down Expand Up @@ -792,7 +793,9 @@ module cv32e40x_rvfi

end

if(pending_single_step_i && single_step_allowed_i) begin
// Check for single step debug entry, need to include the actual debug_cause_n, as single step has the lowest priority
// to enter debug and any higher priority cause could be active at the same time.
if((pending_single_step_i && single_step_allowed_i) && (debug_cause_n_i == DBG_CAUSE_STEP)) 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;
Expand Down
1 change: 1 addition & 0 deletions bhv/cv32e40x_wrapper.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,7 @@ endgenerate
.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 ),
.debug_mode_q_i ( core_i.controller_i.controller_fsm_i.debug_mode_q ),
.debug_cause_n_i ( core_i.controller_i.controller_fsm_i.debug_cause_n ),
.irq_i ( core_i.irq_i & IRQ_MASK ),
.irq_wu_ctrl_i ( core_i.irq_wu_ctrl ),
.irq_id_ctrl_i ( core_i.irq_id_ctrl ),
Expand Down
20 changes: 11 additions & 9 deletions rtl/cv32e40x_controller_fsm.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// - 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)
// todo: can this be merged with pending_sync_debug in the future?
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_async_debug || pending_sync_debug);
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));


// Detect if there is a live CLIC pointer in the pipeline
Expand Down Expand Up @@ -476,8 +476,9 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Debug pending for external debug request
assign pending_async_debug = ((debug_req_i || debug_req_q) && !debug_mode_q);

// Determine cause of debug
// pending_single_step may only happen if no other causes for debug are true.
// Determine cause of debug. Set for all causes of debug entry.
// In case of ebreak during debug mode, the entry code in DEBUG_TAKEN will
// make sure not to update any CSRs.
// The flopped version of this is checked during DEBUG_TAKEN state (one cycle delay)
// todo: update priority according to updated debug spec
// 1: resethaltreq (0x5)
Expand All @@ -486,10 +487,11 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// 4: trigger match (0x2)
// 5: ebreak (0x1)
// 6: single step (0x4)
assign debug_cause_n = pending_single_step ? DBG_CAUSE_STEP :
(trigger_match_in_wb || etrigger_wb_i) ? DBG_CAUSE_TRIGGER :
(ebreak_in_wb && dcsr_i.ebreakm && !debug_mode_q) ? DBG_CAUSE_EBREAK :
DBG_CAUSE_HALTREQ;
assign debug_cause_n = (trigger_match_in_wb || etrigger_wb_i) ? DBG_CAUSE_TRIGGER : // Etrigger will enter DEBUG_TAKEN as a single step (no halting), but kill pipeline as non-stepping entries.
(ebreak_in_wb && dcsr_i.ebreakm && !debug_mode_q) ? DBG_CAUSE_EBREAK : // Ebreak during machine mode
(ebreak_in_wb && debug_mode_q) ? DBG_CAUSE_EBREAK : // Ebreak during debug mode
(pending_async_debug && async_debug_allowed) ? DBG_CAUSE_HALTREQ :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had expected debug_cause_q here, but this works as well

(pending_single_step && single_step_allowed) ? DBG_CAUSE_STEP : DBG_CAUSE_NONE;

// Debug cause to CSR from flopped version (valid during DEBUG_TAKEN)
assign ctrl_fsm_o.debug_cause = debug_cause_q;
Expand Down Expand Up @@ -1027,8 +1029,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Neither ebreak nor trigger match have any state updates in WB. For trigger match, all write enables are suppressed in the ID stage.
// Thus this change is not visible to core state, only for RVFI use.
// todo: Move some logic to RVFI instead?
ctrl_fsm_o.kill_wb = !((debug_cause_q == DBG_CAUSE_EBREAK) || (debug_cause_q == DBG_CAUSE_TRIGGER) ||
(debug_mode_q && ebreak_in_wb));
ctrl_fsm_o.kill_wb = !((debug_cause_q == DBG_CAUSE_EBREAK) || (debug_cause_q == DBG_CAUSE_TRIGGER));


// Save pc from oldest valid instruction
if (ex_wb_pipe_i.instr_valid) begin
Expand Down