Updates for CLIC spec (august 23) #928
Conversation
mcause.minhv only set when taking an exception for a CLIC pointer or mret pointer restart. When jumping due to an mret, mcause.minhv is checked regardless of mcause.mpp. Signed-off-by: Oystein Knauserud <[email protected]>
silabs-oysteink
added
the
Component:RTL
…its fan-in. The signal clic_ptr_in_wb is used by RVFI but not by the controller. Signed-off-by: Oystein Knauserud <[email protected]>
rtl/cv32e40x_controller_fsm.sv
Outdated
| // 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 = (ex_wb_pipe_i.instr_meta.clic_ptr || ex_wb_pipe_i.instr_meta.mret_ptr); |
There was a problem hiding this comment.
Why using the unqualified signals here?
There was a problem hiding this comment.
'exception_in_wb' on line 846 is already qualified with ex_wb_pipe_i.instr_valid. It shouldn't be needed to also factor it into line 865.
There was a problem hiding this comment.
It is not strictly needed indeed, but it would be more readable
There was a problem hiding this comment.
Updated code
rtl/cv32e40x_controller_fsm.sv
Outdated
| @@ -1465,6 +1456,9 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*; | |||
| assign ctrl_fsm_o.debug_running = debug_fsm_cs[RUNNING_INDEX]; | |||
| assign ctrl_fsm_o.debug_halted = debug_fsm_cs[HALTED_INDEX]; | |||
|
|
|||
| // Tie off unused signals | |||
| assign unused_signals = clic_ptr_in_wb; | |||
There was a problem hiding this comment.
I think this signal should actually be used
There was a problem hiding this comment.
Agreed, it could be used for detecting 'clic_ptr_in_pipeline'. I will refactor code to use this signal.
There was a problem hiding this comment.
Removed the 'unused_signal'
sva/cv32e40x_cs_registers_sva.sv
Outdated
| mepc_we // mepc is written | ||
| |-> | ||
| (mcause_we && // mcause is written | ||
| mcause_n.minhv == ((ex_wb_pipe_i.instr_meta.clic_ptr || ex_wb_pipe_i.instr_meta.mret_ptr) && // pointer in WB |
There was a problem hiding this comment.
Use qualified signals
| @@ -163,6 +163,25 @@ 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. | |||
There was a problem hiding this comment.
Prove other way around as well (if minhv is written mepc should be written)
There was a problem hiding this comment.
Added assert to prove that whenever mcause.minhv changes mepc must also be written.
Added assert for checking that mepc is always written when mcause.minhv is changed. Refactored some code in controller_fsm. SEC clean. Signed-off-by: Oystein Knauserud <[email protected]>
mcause.minhv only set when taking an exception for a CLIC pointer or mret pointer restart. When jumping due to an mret, mcause.minhv is checked regardless of mcause.mpp.