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

Updates for CLIC spec (august 23) #928

Merged
Show file tree
Hide file tree
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
55 changes: 19 additions & 36 deletions rtl/cv32e40x_controller_fsm.sv
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
input align_status_e align_status_wb_i, // Aligned status (atomics) in WB
input logic [31:0] wpt_match_wb_i, // LSU watchpoint trigger (WB)


// From LSU (WB)
input logic data_stall_wb_i, // WB stalled by LSU
input logic lsu_valid_wb_i, // LSU instruction in WB is valid
Expand Down Expand Up @@ -166,6 +165,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;

logic branch_taken_n;
logic branch_taken_q;
logic clic_ptr_in_ex;

// WB signals
logic exception_in_wb;
Expand Down Expand Up @@ -281,8 +281,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// index 0 for non-vectored CLINT mode and CLIC mode, 0xF for vectored CLINT mode
assign ctrl_fsm_o.nmi_mtvec_index = (mtvec_mode_i == 2'b01) ? 5'hF : 5'h0;



////////////////////////////////////////////////////////////////////
// ID stage

Expand All @@ -291,8 +289,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Using the ID stage local instr_valid would bring halt_id and kill_id into the equation
// causing a path from data_rvalid to instr_addr_o/instr_req_o/instr_memtype_o via pc_set.



assign sys_mret_id = sys_en_id_i && sys_mret_id_i && if_id_pipe_i.instr_valid;
assign jmp_id = alu_en_id_i && alu_jmp_id_i && if_id_pipe_i.instr_valid;

Expand Down Expand Up @@ -407,6 +403,9 @@ 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);

// Regular CLIC pointer in EX (not caused by mret)
assign clic_ptr_in_ex = id_ex_pipe_i.instr_meta.clic_ptr && id_ex_pipe_i.instr_valid;

// Regular CLIC pointer in WB (not caused by mret)
assign clic_ptr_in_wb = ex_wb_pipe_i.instr_meta.clic_ptr && ex_wb_pipe_i.instr_valid;

Expand All @@ -425,7 +424,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// This CSR bit shall not be gated by debug mode or step without stepie
assign ctrl_fsm_o.pending_nmi = nmi_pending_q;

// Debug //
// Debug

// Single step will need to finish insn in WB, including LSU
// LSU will now set valid_1_o only for second part of misaligned instructions.
Expand Down Expand Up @@ -461,16 +460,13 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// The signal pending_single_step should never be used outside of the FUNCTIONAL state.
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_nmi && nmi_allowed)));


// Detect if there is a live CLIC pointer in the pipeline
// This should block debug and interrupts
generate
if (CLIC) begin : gen_clic_pointer_flag
// A CLIC pointer may be in the pipeline from the moment we start fetching (clic_ptr_in_progress_id == 1)
// or while a pointer is in the EX or WB stages.
assign clic_ptr_in_pipeline = (id_ex_pipe_i.instr_valid && id_ex_pipe_i.instr_meta.clic_ptr) ||
(ex_wb_pipe_i.instr_valid && ex_wb_pipe_i.instr_meta.clic_ptr) ||
clic_ptr_in_progress_id;
assign clic_ptr_in_pipeline = clic_ptr_in_ex || clic_ptr_in_wb || clic_ptr_in_progress_id;
end else begin : gen_basic_pointer_flag
assign clic_ptr_in_pipeline = 1'b0;
end
Expand Down Expand Up @@ -681,8 +677,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.csr_save_cause = 1'b0;
ctrl_fsm_o.csr_cause = 32'h0;

ctrl_fsm_o.csr_clear_minhv = 1'b0;

pipe_pc_mux_ctrl = PC_WB;

exc_cause = 11'b0;
Expand Down Expand Up @@ -745,15 +739,14 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.csr_cause.irq = 1'b1;
ctrl_fsm_o.csr_cause.exception_code = nmi_is_store_q ? INT_CAUSE_LSU_STORE_FAULT : INT_CAUSE_LSU_LOAD_FAULT;

// Keep mcause.minhv when taking exceptions and interrupts, only cleared on successful pointer fetches or CSR writes.
ctrl_fsm_o.csr_cause.minhv = mcause_i.minhv;
// Clear mcause.minhv when taking an NMI (only set when taking exceptions on CLIC/mret pointers)
ctrl_fsm_o.csr_cause.minhv = 1'b0;

if (CLIC) begin
// Keep current interrupt level when taking NMIs
ctrl_fsm_o.irq_level = mintstatus_i.mil;
end


// Save pc from oldest valid instruction
if (ex_wb_pipe_i.instr_valid) begin
pipe_pc_mux_ctrl = PC_WB;
Expand Down Expand Up @@ -796,10 +789,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.csr_save_cause = 1'b1;
ctrl_fsm_o.csr_cause.irq = 1'b1;

// Default to keeping mcause.minhv. It will only be set to 1 when taking a CLIC SHV interrupt (or by a CSR write).
// For all other cases it keeps its value.
ctrl_fsm_o.csr_cause.minhv = mcause_i.minhv;

// Clear mcause.minhv when taking an interrupt (only set when taking exceptions on CLIC/mret pointers)
ctrl_fsm_o.csr_cause.minhv = 1'b0;

if (CLIC) begin
ctrl_fsm_o.csr_cause.exception_code = {1'b0, irq_id_ctrl_i};
Expand All @@ -810,9 +801,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.pc_mux = PC_TRAP_CLICV;
clic_ptr_in_progress_id_set = 1'b1;
ctrl_fsm_o.pc_set_clicv = 1'b1;
// When taking an SHV interrupt, always set minhv.
// Mcause.minhv will only be cleared when a successful pointer fetch is done, or when it is cleared by a CSR write.
ctrl_fsm_o.csr_cause.minhv = 1'b1;
end else begin
ctrl_fsm_o.pc_mux = PC_TRAP_IRQ;
end
Expand Down Expand Up @@ -864,8 +852,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.csr_save_cause = !debug_mode_q; // Do not update CSRs if in debug mode
ctrl_fsm_o.csr_cause.exception_code = exception_cause_wb;

// Keep mcause.minhv when taking exceptions and interrupts, only cleared on successful pointer fetches or CSR writes.
ctrl_fsm_o.csr_cause.minhv = mcause_i.minhv;
// Set mcause.minhv if exception is for a CLIC or mret pointer. Otherwise clear it
ctrl_fsm_o.csr_cause.minhv = clic_ptr_in_wb || mret_ptr_in_wb;

// Special insn
end else if (wfi_in_wb || wfe_in_wb) begin
Expand Down Expand Up @@ -987,14 +975,13 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.kill_if = 1'b1;

if (sys_mret_id) begin
// If the xcause.minhv bit is set for the previous privilege level (mcause.mpp) when an mret is in ID,
// the mret should restart the CLIC pointer fetch using mepc as a pointer to the pointer instead of jumping to
// the address in mepc. If mpp==PRIV_LVL_U, the (not existing) ucause.uinhv bit is assumed to be 0.
// If the mcause.minhv bit is set when an mret is in ID, the mret should restart the CLIC pointer fetch using mepc as
// a pointer to the pointer instead of jumping to the address in mepc.
// This is done below by signalling pc_set_clicv along with pc_mux=PC_MRET. This will
// treat the mepc as an address to a CLIC pointer. The minhv flag will only be cleared
// when a pointer reaches the WB stage with no faults from fetching.
// ID stage is halted while it contains an mret and at the same time there are CSR writes (including CLIC pointers) EX or WB, hence it is safe to use mcause here.
if (mcause_i.minhv && (mcause_i.mpp == PRIV_LVL_M)) begin
// treat the mepc as an address to a CLIC pointer. The minhv flag will only be set when an exception is taken on a
// CLIC or mret pointer, and cleared when a trap for any other cause except debug is taken. It is also writeable by SW.
// ID stage is halted while it contains an mret and at the same time there are CSR writes (including CLIC pointers) in EX or WB, hence it is safe to use mcause here.
if (mcause_i.minhv) begin
// mcause.minhv set, exception occured during last pointer fetch (or SW wrote it)
// Do another pointer fetch from the address stored in mepc.
ctrl_fsm_o.pc_set = 1'b1;
Expand Down Expand Up @@ -1063,11 +1050,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
ctrl_fsm_o.csr_restore_mret_ptr = !debug_mode_q;
end

// CLIC pointer in WB
if (clic_ptr_in_wb && !ctrl_fsm_o.kill_wb && !ctrl_fsm_o.halt_wb && !exception_in_wb) begin
// Clear minhv if no exceptions are associated with the pointer
ctrl_fsm_o.csr_clear_minhv = 1'b1;
end
end // !debug or interrupts

// Single step debug entry or etrigger debug entry
Expand Down Expand Up @@ -1466,6 +1448,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
assign ctrl_fsm_o.debug_halted = debug_fsm_cs[HALTED_INDEX];



//---------------------------------------------------------------------------
// eXtension interface
//---------------------------------------------------------------------------
Expand Down
23 changes: 1 addition & 22 deletions rtl/cv32e40x_cs_registers.sv
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ module cv32e40x_cs_registers import cv32e40x_pkg::*;
cause : ctrl_fsm_i.debug_cause,
default : 'd0
};
dcsr_we = 1'b1;
dcsr_we = 1'b1;

dpc_n = ctrl_fsm_i.pipe_pc;
dpc_we = 1'b1;
Expand Down Expand Up @@ -1106,11 +1106,6 @@ dcsr_we = 1'b1;
mintthresh_n = 32'h00000000;
mintthresh_we = 1'b1;
end

if (ctrl_fsm_i.csr_restore_mret_ptr) begin
// Clear mcause.minhv if the mret also caused a successful CLIC pointer fetch
mcause_n.minhv = 1'b0;
end
end
end //ctrl_fsm_i.csr_restore_mret

Expand All @@ -1134,22 +1129,6 @@ dcsr_we = 1'b1;

end //ctrl_fsm_i.csr_restore_dret

// Clear mcause.minhv on successful CLIC pointer fetches
// This only happens for CLIC pointer that did not originate from an mret.
// In the case of mret restarting CLIC pointer fetches, minhv is cleared while
// ctrl_fsm_i.csr_restore_mret_ptr is asserted.
ctrl_fsm_i.csr_clear_minhv: begin
if (CLIC) begin
// Keep mcause values, only clear minhv bit.
mcause_n = mcause_rdata;
mcause_n.minhv = 1'b0;
mcause_we = 1'b1;

// Not really needed, but allows for asserting mstatus_we == mcause_we to check aliasing formally
mstatus_n = mstatus_rdata;
mstatus_we = 1'b1;
end
end
default:;
endcase

Expand Down
1 change: 0 additions & 1 deletion rtl/include/cv32e40x_pkg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,6 @@ typedef struct packed {
logic csr_restore_mret_ptr; // Restore CSR due to mret followed by CLIC
logic csr_restore_dret; // Restore CSR due to dret
logic csr_save_cause; // Update CSRs
logic csr_clear_minhv; // Clear the mcause.minhv field
logic pending_nmi; // An NMI is pending (for dcsr.nmip)

// Performance counter events
Expand Down
1 change: 0 additions & 1 deletion sva/cv32e40x_controller_fsm_sva.sv
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,6 @@ end else begin // CLIC
|->
(clic_ptr_in_progress_id_set != 1'b1) &&
!ctrl_fsm_o.csr_cause.minhv &&
!ctrl_fsm_o.csr_clear_minhv &&
!mcause_i.minhv &&
!if_id_pipe_i.instr_meta.clic_ptr &&
!id_ex_pipe_i.instr_meta.clic_ptr &&
Expand Down
56 changes: 35 additions & 21 deletions sva/cv32e40x_cs_registers_sva.sv
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,41 @@ module cv32e40x_cs_registers_sva
a_mcause_mstatus_alias: assert property(p_mcause_mstatus_alias)
else `uvm_error("cs_registers", "mcause.mpp and mcause.mpie not aliased correctly")

// Whenever mepc is written by HW, mcause.minhv must also be written with the correct value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Prove other way around as well (if minhv is written mepc should be written)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added assert to prove that whenever mcause.minhv changes mepc must also be written.

// mcause.minhv is only set when an exception is taken on a CLIC or mret pointer
// SW can still write both mepc and mcause independently
property p_mepc_minhv_write;
@(posedge clk) disable iff (!rst_n)
(
mepc_we // mepc is written
|->
(mcause_we && // mcause is written
mcause_n.minhv == ((ex_wb_pipe_i.instr_valid && (ex_wb_pipe_i.instr_meta.clic_ptr || ex_wb_pipe_i.instr_meta.mret_ptr)) && // pointer in WB
(ctrl_fsm_i.pc_set && ctrl_fsm_i.pc_mux == PC_TRAP_EXC))) // exception taken
or
csr_we_int // SW writes mepc
);
endproperty;

a_mepc_minhv_write: assert property(p_mepc_minhv_write)
else `uvm_error("cs_registers", "mcause.minhv not updated correctly on mepc write.")

// Whenever mcause.minhv is changed mepc should also be written (or SW writes to mcause.minhv)
property p_minhv_mepc_write;
@(posedge clk) disable iff (!rst_n)
(
mcause_we &&
(mcause_n.minhv != mcause_q.minhv)
|->
mepc_we
or
csr_we_int
);
endproperty;

a_minhv_mepc_write: assert property(p_minhv_mepc_write)
else `uvm_error("cs_registers", "mcause written without mepc written")

end

// Check that no csr instruction can be in WB during sleep when ctrl_fsm.halt_limited_wb is set
Expand All @@ -174,27 +209,6 @@ module cv32e40x_cs_registers_sva
a_halt_limited_wb: assert property(p_halt_limited_wb)
else `uvm_error("cs_registers", "CSR in WB while halt_limited_wb is set");


// Check csr_clear_minhv cannot happen at the same time as csr_save_cause or csr_restore_dret (would cause mcause_we conflict)
sequence seq_csr_clear_minhv;
ctrl_fsm_i.csr_clear_minhv;
endsequence

property p_minhv_unique;
@(posedge clk) disable iff (!rst_n)
( seq_csr_clear_minhv |-> !(ctrl_fsm_i.csr_save_cause || ctrl_fsm_i.csr_restore_dret));
endproperty;

if (CLIC) begin
a_minhv_unique: assert property(p_minhv_unique)
else `uvm_error("cs_registers", "csr_save_cause at the same time as csr_clear_minhv.");

end else begin
a_minhv_unique_nocover: assert property(
@(posedge clk) disable iff (!rst_n)
not seq_csr_clear_minhv);
end

/////////////////////////////////////////////////////////////////////////////////////////
// Asserts to check that the CSR flops remain unchanged if a set/clear has all_zero rs1
/////////////////////////////////////////////////////////////////////////////////////////
Expand Down