Implementation of mnxti #506
Conversation
Refactored some CSR write code that access more than one CSR. Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
There was a problem hiding this comment.
(We will discuss #507 separately as that issue was not introduced with this PR)
rtl/cv32e40x_core.sv
Outdated
| logic [31:0] csr_rdata; | ||
| logic csr_counter_read; | ||
|
|
||
| // CLIC signals for returning pointer to handles | ||
| // when mnxti is accessed | ||
| logic csr_clic_pa_valid; |
There was a problem hiding this comment.
What does 'pa' stand for?
There was a problem hiding this comment.
PointerAddress, I tried to make a short name, but this is probably too short. Would 'csr_clic_ptr_addr_valid' be better perhaps?
There was a problem hiding this comment.
You can keep this name; just add a comment on the lines where these signals are declared that contains at least the words 'pointer address'
rtl/cv32e40x_cs_registers.sv
Outdated
| // Write mpie and mpp as aliased through mcause | ||
| mstatus_n.mpie = csr_wdata_int[MCAUSE_MPIE_BIT]; | ||
| mstatus_n.mpp = PRIV_LVL_M; // todo: handle priv mode for E40S | ||
| end else if (mnxti_we) begin |
There was a problem hiding this comment.
If there is a mnxti write with side effects, then both mcause_we = 1 and mnxti_we = 1 and this block of code will not get executed.
There was a problem hiding this comment.
That's right, we'll have to check the state for both write enables in the first if block I think.
There was a problem hiding this comment.
Maybe you can handle mnxti_we in the top/first if to get it correct?
rtl/cv32e40x_cs_registers.sv
Outdated
| mstatus_n.mpp = PRIV_LVL_M; // todo: handle priv mode for E40S | ||
| end else if (mnxti_we) begin | ||
| // A mnxti write writes to mstatus.mie | ||
| mstatus_n = mstatus_q; |
There was a problem hiding this comment.
I don't think these two lines are correct. The spec says 'All CSR instructions may be used with xnxti. The operation will be the same as the corresponding CSR instruction used with xstatus.', so shouldn't the already computed mstatus_n from line 571 do the trick?
There was a problem hiding this comment.
Agreed, fixed.
Connected irq_level from clic_interrupt_controller to cs_registers, as this was left floating in the previous commit. Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Silabs-ArjanB
added
the
Component:RTL
MNXTI functionality should now be in.
Tested basic functionality of reading correct pointer addresses for pending interrupts when changing levels in the memory mapped registers.
SEC clean for SMCLIC=0