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

Further removal of CLIC pointers using data access. #548

Merged
merged 2 commits into from
May 23, 2022
Merged

Further removal of CLIC pointers using data access. #548

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e2f2cdf
Further removal of CLIC pointers using data access.
silabs-oysteink May 20, 2022
c2b13bb
Removed comments related to CLIC data fetches and NMIs.
silabs-oysteink May 23, 2022
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
6 changes: 2 additions & 4 deletions bhv/cv32e40x_rvfi.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ module cv32e40x_rvfi
input logic single_step_allowed_i,
input logic nmi_pending_i, // regular NMI pending
input logic nmi_is_store_i, // regular NMI type
input logic clic_nmi_pending_i, // NMI due to CLIC vector load is pending
input logic clic_nmi_is_store_i, // NMI type due to CLIC vector load
input logic pending_debug_i,
input logic debug_mode_q_i,

Expand Down Expand Up @@ -639,7 +637,7 @@ module cv32e40x_rvfi
rvfi_trap_next.trap = rvfi_trap_next.exception || rvfi_trap_next.debug;
end

// WFI instructions retire when their wake-up condition is present.
// WFI instructions retire when their wake-up condition is present.
// The wake-up condition is only checked in the SLEEP state of the controller FSM.
assign wb_valid_adjusted = (sys_en_wb_i && sys_wfi_insn_wb_i) ? (ctrl_fsm_cs_i == SLEEP) && (ctrl_fsm_ns_i == FUNCTIONAL) : wb_valid_i;

Expand Down Expand Up @@ -846,7 +844,7 @@ module cv32e40x_rvfi
end
end // always_ff @

assign rvfi_nmip = {(nmi_is_store_i || clic_nmi_is_store_i), (nmi_pending_i || clic_nmi_pending_i)};
assign rvfi_nmip = {nmi_is_store_i, nmi_pending_i};

// Capture possible performance counter writes during WB, before wb_valid
// If counter write happens before wb_valid (e.g. LSU stalled waiting for rvalid or WFI that is in WB multiple cycles),
Expand Down
2 changes: 0 additions & 2 deletions bhv/cv32e40x_wrapper.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,6 @@ module cv32e40x_wrapper
.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 ),
.clic_nmi_pending_i ( core_i.controller_i.controller_fsm_i.pending_clic_nmi ),
.clic_nmi_is_store_i ( 1'b0 /* CLIC NMI can only be due to a load*/ ),
.pending_debug_i ( core_i.controller_i.controller_fsm_i.pending_debug ),
.debug_mode_q_i ( core_i.controller_i.controller_fsm_i.debug_mode_q ),
.irq_i ( core_i.irq_i & IRQ_MASK ),
Expand Down
2 changes: 0 additions & 2 deletions rtl/cv32e40x_controller.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
input logic [1:0] lsu_err_wb_i, // LSU bus error in WB stage
input logic lsu_busy_i, // LSU is busy with outstanding transfers
input logic lsu_interruptible_i, // LSU may be interrupted
input logic lsu_write_buffer_empty_i, // LSU write buffer state

// jump/branch signals
input logic branch_decision_ex_i, // branch decision signal from EX ALU
Expand Down Expand Up @@ -162,7 +161,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
.wb_valid_i ( wb_valid_i ),

.lsu_interruptible_i ( lsu_interruptible_i ),
.lsu_write_buffer_empty_i ( lsu_write_buffer_empty_i ),
// Interrupt Controller Signals
.irq_req_ctrl_i ( irq_req_ctrl_i ),
.irq_id_ctrl_i ( irq_id_ctrl_i ),
Expand Down
44 changes: 9 additions & 35 deletions rtl/cv32e40x_controller_fsm.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
input logic lsu_busy_i, // LSU is busy with outstanding transfers

input logic lsu_interruptible_i, // LSU can be interrupted
input logic lsu_write_buffer_empty_i, // LSU write buffer state
// Interrupt Controller Signals
input logic irq_req_ctrl_i, // irq requst
input logic [9:0] irq_id_ctrl_i, // irq id
Expand Down Expand Up @@ -159,7 +158,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;

logic pending_nmi;
logic pending_nmi_early;
logic pending_clic_nmi;
logic pending_debug;
logic pending_single_step;
logic pending_single_step_ptr;
Expand All @@ -176,9 +174,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Used to block debug until pointer
logic pointer_in_pipeline;

// Flag for checking if we can to a CLIC pointer fetch
logic wbuf_irq_ok;

// Internal irq_ack for use when a (clic) pointer reaches ID stage and
// we have single stepping enabled.
logic non_shv_irq_ack;
Expand Down Expand Up @@ -249,24 +244,20 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
assign branch_taken_ex = branch_in_ex && !branch_taken_q;

// Exception in WB if the following evaluates to 1
// CLIC: bus errors for pointer fetches are treated as NMI, not exceptions.
assign exception_in_wb = ((ex_wb_pipe_i.instr.mpu_status != MPU_OK) ||
(!ex_wb_pipe_i.instr_meta.clic_ptr && ex_wb_pipe_i.instr.bus_resp.err) ||
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) ||
(lsu_mpu_status_wb_i != MPU_OK)) && ex_wb_pipe_i.instr_valid;

// Set exception cause
// For CLIC: Pointer fetches with PMA/PMP errors will get the exception code converted to LOAD_FAULT
// Bus errors will be converted to NMI as for regular loads.
assign exception_cause_wb = (!ex_wb_pipe_i.instr_meta.clic_ptr && (ex_wb_pipe_i.instr.mpu_status != MPU_OK)) ? EXC_CAUSE_INSTR_FAULT :
(!ex_wb_pipe_i.instr_meta.clic_ptr && ex_wb_pipe_i.instr.bus_resp.err) ? EXC_CAUSE_INSTR_BUS_FAULT :
(ex_wb_pipe_i.instr_meta.clic_ptr && (ex_wb_pipe_i.instr.mpu_status != MPU_OK)) ? EXC_CAUSE_LOAD_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 :
(lsu_mpu_status_wb_i == MPU_WR_FAULT) ? EXC_CAUSE_STORE_FAULT :
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 :
(lsu_mpu_status_wb_i == MPU_WR_FAULT) ? EXC_CAUSE_STORE_FAULT :
EXC_CAUSE_LOAD_FAULT; // (lsu_mpu_status_wb_i == MPU_RE_FAULT)

// For now we are always allowed to take exceptions once they arrive in WB.
Expand Down Expand Up @@ -303,11 +294,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Using flopped version to avoid paths from data_err_i/data_rvalid_i to instr_* outputs
// Gating the pending signal instead of the allowed signal for debug related conditions, otherwise a pending NMI during debug mode
// or single stepping with dcsr.stepie==0 would stall ID stage and we would never get out of debug, resulting in a deadlock.
// CLIC pointer fetches with associated bus errors are treated as NMI. The error bit is taken from ex_wb_pipe and not flopped further.
// This preserves the address of the pointer (ex_wb_pipe.pc) that must be stored to mepc.
assign pending_clic_nmi = (ex_wb_pipe_i.instr_valid && ex_wb_pipe_i.instr_meta.clic_ptr && ex_wb_pipe_i.instr.bus_resp.err);
assign pending_nmi = (nmi_pending_q || pending_clic_nmi) &&
!debug_mode_q && !(dcsr_i.step && !dcsr_i.stepie);
assign pending_nmi = nmi_pending_q && !debug_mode_q && !(dcsr_i.step && !dcsr_i.stepie);
Copy link
Contributor

Choose a reason for hiding this comment

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

Update comment above as well


// Early version of the pending_nmi signal, using the unflopped lsu_err_wb_i[0]
// This signal is used for halting the ID stage in the same cycle as the bus error arrives.
Expand All @@ -320,9 +307,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;

// dcsr.nmip will always see a pending nmi if nmi_pending_q is set.
// This CSR bit shall not be gated by debug mode or step without stepie
// NMI's related to clic pointer load bus errors are not signaled through dcsr.nmip
// This can only happen during machine mode as interrupts are blocked during debug,
// and the dcsr is not readable from machine mode. No need to expose this bit as it will never be visible.
assign ctrl_fsm_o.pending_nmi = nmi_pending_q;

// Debug //
Expand Down Expand Up @@ -407,17 +391,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// 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

// todo: generate CLIC, check shv and write buffer status
generate
if (SMCLIC) begin : gen_clic_wbuf_check
// If an interrupt is SHV, the write buffer must be empty before we allow to take the interrupt
// The content in the write buffer could be a pointer update.
assign wbuf_irq_ok = irq_clic_shv_i ? lsu_write_buffer_empty_i : 1'b1;
end else begin : gen_basic_wbuf_check
assign wbuf_irq_ok = 1'b1;
end
endgenerate
assign interrupt_allowed = lsu_interruptible_i && !fencei_ongoing && !xif_in_wb && wbuf_irq_ok;
assign interrupt_allowed = lsu_interruptible_i && !fencei_ongoing && !xif_in_wb;

// Allowing NMI's follow the same rule as regular interrupts.
assign nmi_allowed = interrupt_allowed;
Expand Down
3 changes: 0 additions & 3 deletions rtl/cv32e40x_core.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,6 @@ module cv32e40x_core import cv32e40x_pkg::*;
logic if_busy;
logic lsu_busy;
logic lsu_interruptible;
logic lsu_write_buffer_empty;

// ID/EX pipeline
id_ex_pipe_t id_ex_pipe;
Expand Down Expand Up @@ -575,7 +574,6 @@ module cv32e40x_core import cv32e40x_pkg::*;
// Control signals
.busy_o ( lsu_busy ),
.interruptible_o ( lsu_interruptible ),
.write_buffer_empty_o ( lsu_write_buffer_empty ),

// Stage 0 outputs (EX)
.lsu_split_0_o ( lsu_split_ex ),
Expand Down Expand Up @@ -784,7 +782,6 @@ module cv32e40x_core import cv32e40x_pkg::*;
.lsu_err_wb_i ( lsu_err_wb ),
.lsu_busy_i ( lsu_busy ),
.lsu_interruptible_i ( lsu_interruptible ),
.lsu_write_buffer_empty_i ( lsu_write_buffer_empty ),

// jump/branch control
.branch_decision_ex_i ( branch_decision_ex ),
Expand Down
7 changes: 1 addition & 6 deletions rtl/cv32e40x_load_store_unit.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;
// Control outputs
output logic busy_o,
output logic interruptible_o,
output logic write_buffer_empty_o,

// Stage 0 outputs (EX)
output logic lsu_split_0_o, // Misaligned access is split in two transactions (to controller)
Expand Down Expand Up @@ -95,8 +94,6 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;
logic buffer_trans_ready;
obi_data_req_t buffer_trans;

logic buffer_empty;

logic filter_trans_valid;
logic filter_trans_ready;
obi_data_req_t filter_trans;
Expand Down Expand Up @@ -665,11 +662,9 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;

.valid_o ( bus_trans_valid ),
.ready_i ( bus_trans_ready ),
.trans_o ( bus_trans ),
.empty_o ( buffer_empty )
.trans_o ( bus_trans )
);

assign write_buffer_empty_o = buffer_empty;
//////////////////////////////////////////////////////////////////////////////
// OBI interface
//////////////////////////////////////////////////////////////////////////////
Expand Down
8 changes: 2 additions & 6 deletions rtl/cv32e40x_write_buffer.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ module cv32e40x_write_buffer import cv32e40x_pkg::*;
// outputs
output logic valid_o,
output obi_data_req_t trans_o,
output logic ready_o,

output logic empty_o
);
output logic ready_o
);

// local signals
write_buffer_state_e state, next_state;
Expand Down Expand Up @@ -115,6 +113,4 @@ module cv32e40x_write_buffer import cv32e40x_pkg::*;
assign valid_o = (state == WBUF_FULL) || valid_i;
assign trans_o = (state == WBUF_FULL) ? trans_q : trans_i;

assign empty_o = (state == WBUF_EMPTY);

endmodule
6 changes: 0 additions & 6 deletions sva/cv32e40x_controller_fsm_sva.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ module cv32e40x_controller_fsm_sva
input logic nmi_is_store_q,
input logic nmi_pending_q,
input dcsr_t dcsr_i,
input logic lsu_write_buffer_empty_i,
input logic irq_clic_shv_i
);

Expand Down Expand Up @@ -514,11 +513,6 @@ endgenerate
else `uvm_error("controller", "NMI handler not taken within two instruction retirements")

if (SMCLIC) begin
// When a CLIC SHV interrupt is taken, the write buffer must be empty
a_clic_shv_wbuf_empty:
assert property (@(posedge clk) disable iff (!rst_n)
(irq_clic_shv_i && ctrl_fsm_o.irq_ack) |-> lsu_write_buffer_empty_i)
else `uvm_error("controller", "LSU write buffer not empty when fetching CLIC pointer")

// After a pc_set to PC_TRAP_CLICV, only the following jump targets are allowed:
// PC_POINTER : Normal execution, the pointer target is being fetched
Expand Down
4 changes: 1 addition & 3 deletions sva/cv32e40x_core_sva.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,8 @@ always_ff @(posedge clk , negedge rst_ni)
expected_instr_err_mepc <= ex_wb_pipe.pc;
end

// CLIC pointers generate data errors, exluding to avoid cause mismatch. todo: make separate asserts for clicptr exceptions.
// todo: if CLIC spec changes from data to instruction fetch for pointer, this must change again.
if (!first_instr_mpuerr_found && ex_wb_pipe.instr_valid && !irq_ack && !(ctrl_pending_debug && ctrl_debug_allowed) &&
!(ctrl_fsm.pc_mux == PC_TRAP_NMI) && !(ex_wb_pipe.instr_meta.clic_ptr) &&
!(ctrl_fsm.pc_mux == PC_TRAP_NMI) &&
(ex_wb_pipe.instr.mpu_status != MPU_OK) && !ctrl_debug_mode_n) begin
first_instr_mpuerr_found <= 1'b1;
expected_instr_mpuerr_mepc <= ex_wb_pipe.pc;
Expand Down