From f8fd54c2e3f983b791b9d7bbb1f56b843f2a8090 Mon Sep 17 00:00:00 2001 From: Greg Chadwick Date: Tue, 17 Sep 2024 13:51:01 +0100 Subject: [PATCH] [rtl] Flush pipe on all CSR modifications This fixes #2193, an issue that meant bit clears in PMP related CSRs didn't immediately apply to an instruction already in the fetch stage due to a lack of a pipeline flush. With this change the pipeline will flush in that scenario, fixing the issue. It now flushes the pipeline on all CSR modifications as this makes the pipeline more resliant against similar issues in the future (where the list of CSRs to flush on should have been updated but wasn't). --- rtl/ibex_id_stage.sv | 37 +++++++------------------------------ 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/rtl/ibex_id_stage.sv b/rtl/ibex_id_stage.sv index 3a358af7ab..d84613c606 100644 --- a/rtl/ibex_id_stage.sv +++ b/rtl/ibex_id_stage.sv @@ -509,36 +509,13 @@ module ibex_id_stage #( .branch_in_dec_o(branch_in_dec) ); - ///////////////////////////////// - // CSR-related pipeline flushes // - ///////////////////////////////// - always_comb begin : csr_pipeline_flushes - csr_pipe_flush = 1'b0; - - // A pipeline flush is needed to let the controller react after modifying certain CSRs: - // - When enabling interrupts, pending IRQs become visible to the controller only during - // the next cycle. If during that cycle the core disables interrupts again, it does not - // see any pending IRQs and consequently does not start to handle interrupts. - // - When modifying any PMP CSR, PMP check of the next instruction might get invalidated. - // Hence, a pipeline flush is needed to instantiate another PMP check with the updated CSRs. - // - When modifying debug CSRs. - if (csr_op_en_o == 1'b1 && (csr_op_o == CSR_OP_WRITE || csr_op_o == CSR_OP_SET)) begin - if (csr_num_e'(instr_rdata_i[31:20]) == CSR_MSTATUS || - csr_num_e'(instr_rdata_i[31:20]) == CSR_MIE || - csr_num_e'(instr_rdata_i[31:20]) == CSR_MSECCFG || - // To catch all PMPCFG/PMPADDR registers, get the shared top most 7 bits. - instr_rdata_i[31:25] == 7'h1D) begin - csr_pipe_flush = 1'b1; - end - end else if (csr_op_en_o == 1'b1 && csr_op_o != CSR_OP_READ) begin - if (csr_num_e'(instr_rdata_i[31:20]) == CSR_DCSR || - csr_num_e'(instr_rdata_i[31:20]) == CSR_DPC || - csr_num_e'(instr_rdata_i[31:20]) == CSR_DSCRATCH0 || - csr_num_e'(instr_rdata_i[31:20]) == CSR_DSCRATCH1) begin - csr_pipe_flush = 1'b1; - end - end - end + // Flush pipe on any CSR modification. Some CSR modifications alter how instructions execute (e.g. + // the PMP CSRs) so this ensures all instructions always see the latest architectural state when + // entering the fetch stage. This causes some needless flushes but performance impact is + // limited. We have a single fetch stage to flush not many stages of a deep pipeline and CSR + // instructions are in general rare and not part of performance critical parts of the code. + assign csr_pipe_flush = + (csr_op_en_o == 1) && (csr_op_o inside {CSR_OP_WRITE, CSR_OP_SET, CSR_OP_CLEAR}); //////////////// // Controller //