Updates to mret/xinhv handling. #331
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…nhv assumes for mpp==PRIV_LVL_U. Unique case violation in cs_registers due to clearing of minhv fixed, assertions added to make sure there is no conflicting writes to mcause. Signed-off-by: Oystein Knauserud <Oystein.Knauserud@silabs.com> Added comments and assertions to explain why the clearing of mcause.minhv is allowed to happen at the same time as an mret updates CSRs from the WB stage. Other updates to comments to clarify minhv handling. Signed-off-by: Oystein Knauserud <Oystein.Knauserud@silabs.com>
silabs-oysteink
added
the
Component:RTL
Silabs-ArjanB
requested changes
Oct 26, 2022
| @@ -753,6 +761,9 @@ module cv32e40s_controller_fsm import cv32e40s_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; | |||
There was a problem hiding this comment.
Can we have an assertion checking that various CLIC related signals are inactive when SMCLIC=0?
E.g.FSM is never in POINTER_FETCH state, ctrl_fsm_o.csr_cause.minhv is 1'b0, mcause_i.minhv = 1'b0, *.instr_meta.clic_ptr = 1'b0, etc.
There was a problem hiding this comment.
Added assertion.
rtl/cv32e40s_cs_registers.sv
Outdated
| // The fields mstatus.mpp and mstatus.mpie er aliased between mcause and mstatus. The mcause write | ||
| // due to csr_celar_minhv will however only write to mcause.minhv, and no updates to mstatus.mpp/mpie. | ||
| if (ctrl_fsm_i.csr_clear_minhv) begin | ||
| if (SMCLIC) begin |
There was a problem hiding this comment.
Can if (SMCLI) be made the 'outer if' instead (for code coverage reasons)?
There was a problem hiding this comment.
Order of if's fixed.
…CLIC=0. Refactored order of if statement for coverage purposes. Signed-off-by: Oystein Knauserud <Oystein.Knauserud@silabs.com>
Silabs-ArjanB
approved these changes
Oct 27, 2022
silabs-oysteink
pushed a commit
to silabs-oysteink/cv32e40s
that referenced
this pull request
Nov 2, 2022
Updates to mret/xinhv handling. Signed-off-by: Oystein Knauserud <Oystein.Knauserud@silabs.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updated handling of mret related to mcause.mpp and mcause.minhv. No inhv assumes for mpp==PRIV_LVL_U. Unique case violation in cs_registers due to clearing of minhv fixed, assertions added to make sure there is no conflicting writes to mcause.
When merged, this PR will be cherry picked to the E40X to apply the changes there. Should fix issue #498 on CV32E40X when done.