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.

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

Not flagging exception for ebreak if dcsr.ebreakm==1. #774

Merged
Changes from all commits
Commits
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
33 changes: 21 additions & 12 deletions rtl/cv32e40x_controller_fsm.sv
Original file line number Diff line number Diff line change
Expand Up @@ -322,21 +322,30 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Exception in WB if the following evaluates to 1
// Not checking for ex_wb_pipe_i.last_op to enable exceptions to be taken as soon as possible for
// split load/stores or Zc sequences.
assign exception_in_wb = ((ex_wb_pipe_i.instr.mpu_status != MPU_OK) ||
ex_wb_pipe_i.instr.bus_resp.err ||
ex_wb_pipe_i.illegal_insn ||
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ||
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ebrk_insn) ||
//
// For ebreak instructions, the following scenarios are possible. Only one scenario could cause an exception:
// ebreakm | debug_mode | action
//---------|------------|-----------------------------------------
// 0 | 0 | Exception
// 0 | 1 | Debug entry (restart from dm_halt_addr_i)
// 1 | 0 | Debug entry
// 1 | 1 | Debug entry (restart from dm_halt_addr_i)
//
assign exception_in_wb = ((ex_wb_pipe_i.instr.mpu_status != MPU_OK) ||
ex_wb_pipe_i.instr.bus_resp.err ||
ex_wb_pipe_i.illegal_insn ||
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ||
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ebrk_insn && !dcsr_i.ebreakm && !debug_mode_q) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

First change. This was not SEC clean as it caused a difference on etrigger vs ebreak.

(mpu_status_wb_i != MPU_OK)) && ex_wb_pipe_i.instr_valid;

assign ctrl_fsm_o.exception_in_wb = exception_in_wb;

// Set exception cause
assign exception_cause_wb = (ex_wb_pipe_i.instr.mpu_status != MPU_OK) ? EXC_CAUSE_INSTR_FAULT :
ex_wb_pipe_i.instr.bus_resp.err ? EXC_CAUSE_INSTR_BUS_FAULT :
ex_wb_pipe_i.illegal_insn ? EXC_CAUSE_ILLEGAL_INSN :
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ? EXC_CAUSE_ECALL_MMODE :
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ebrk_insn) ? EXC_CAUSE_BREAKPOINT :
assign exception_cause_wb = (ex_wb_pipe_i.instr.mpu_status != MPU_OK) ? EXC_CAUSE_INSTR_FAULT :
ex_wb_pipe_i.instr.bus_resp.err ? EXC_CAUSE_INSTR_BUS_FAULT :
ex_wb_pipe_i.illegal_insn ? EXC_CAUSE_ILLEGAL_INSN :
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ? EXC_CAUSE_ECALL_MMODE :
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ebrk_insn && !dcsr_i.ebreakm && !debug_mode_q) ? EXC_CAUSE_BREAKPOINT :
Copy link
Contributor Author

Choose a reason for hiding this comment

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

dcsr.ebreakm part of second change, along with the minstret counter change on line 562. SEC clean with correct OBI protocol. Third change was the debug_mode_q on line 348, that was SEC clean.

(mpu_status_wb_i == MPU_WR_FAULT) ? EXC_CAUSE_STORE_FAULT :
EXC_CAUSE_LOAD_FAULT; // (mpu_status_wb_i == MPU_RE_FAULT)

Expand Down Expand Up @@ -543,14 +552,14 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// 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));

// Do not count if we have an exception in WB, trigger match in WB (we do not execute the instruction at trigger address),
// Do not count if we have an exception in WB, ebreak in WB, trigger match in WB (we do not execute the instruction at trigger address),
// or WB stage is killed or halted.
// When WB is halted, we do not know (yet) if the instruction will retire or get killed.
// Halted WB due to debug will result in WB getting killed
// Halted WB due to fence.i will result in fence.i retire after handshake is done and we count when WB is un-halted
// ctrl_fsm_o.halt_limited_wb will only be set during SLEEP, and only affect the WB stage (not cs_registers)
// In terms of counter events, no event should be counted while either of the WB related halts are asserted.
assign wb_counter_event_gated = wb_counter_event && !exception_in_wb && !trigger_match_in_wb &&
assign wb_counter_event_gated = wb_counter_event && !exception_in_wb && !ebreak_in_wb && !trigger_match_in_wb &&
!ctrl_fsm_o.kill_wb && !ctrl_fsm_o.halt_wb && !ctrl_fsm_o.halt_limited_wb;

// Performance counter events
Expand Down