Fix for issue #507 #655
Merged
Fix for issue #507 #655
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
Blanking interrupts for one cycle after a load or store has left WB. Refactored some assertions for CLIC, added a separate file for CLIC asserts. Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
Silabs-ArjanB
requested changes
Aug 29, 2022
rtl/cv32e40x_controller_bypass.sv
Outdated
| @@ -144,7 +144,12 @@ module cv32e40x_controller_bypass import cv32e40x_pkg::*; | |||
| // This is needed because the data bypass from EX uses csr_rdata, and for mnxti this is actually mstatus and not the result | |||
| // that will be written to the register file. Could be optimized to only stall when the result from the CSR instruction is used in ID, | |||
| // but the common usecase is a CSR access followed by a branch using the mnxti result in the RF, so it would likely stall in most cases anyway. | |||
| assign ctrl_byp_o.mnxti_stall = csr_mnxti_read_i; | |||
| assign ctrl_byp_o.mnxti_stall_id = csr_mnxti_read_i; | |||
There was a problem hiding this comment.
I would prefer the following renaming as _id, _ex postfixes are normally used to indicate signal timing:
mnxti_stall_id -> mnxti_id_stall
mnxti_stall_ex -> mnxti_ex_stall
| interrupt_blanking_q <= 1'b0; | ||
| end else begin | ||
| interrupt_blanking_q <= ex_wb_pipe_i.instr_valid && ex_wb_pipe_i.lsu_en && lsu_valid_wb_i; | ||
| end |
There was a problem hiding this comment.
Is the use of lsu_valid_wb_i even needed? Can you check if it is SEC clean to remove it? (Interrupting WB while waiting for an rvalid should not be allowed anyway)
There was a problem hiding this comment.
It is in fact not needed, we are SEC clean without it. Removed.
rtl/include/cv32e40x_pkg.sv
Outdated
| @@ -1204,7 +1204,8 @@ typedef struct packed { | |||
| logic load_stall; // Stall due to load operation | |||
| logic csr_stall; | |||
| logic wfi_stall; | |||
| logic mnxti_stall; // Stall due to mnxti CSR access in EX | |||
| logic mnxti_stall_id; // Stall ID due to mnxti CSR access in EX | |||
sva/cv32e40x_core_sva.sv
Outdated
| @@ -72,24 +72,21 @@ module cv32e40x_core_sva | |||
| input logic data_req_o, | |||
| input logic data_we_o, | |||
| input logic [5:0] data_atop_o, | |||
| input logic data_rvalid_i, | |||
There was a problem hiding this comment.
Is this signal still used? If not, then remove
Signed-off-by: Oystein Knauserud <[email protected]>
Silabs-ArjanB
approved these changes
Aug 29, 2022
Silabs-ArjanB
added
the
Component:RTL
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.
Blanking interrupts for one cycle after a Load/Store instruction leaves WB.
Refactored some CLIC assertions (new file for clic_int_controller_sva)