Skip to content

Commit bdfaab3

Browse files
committed
[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).
1 parent 257cf5c commit bdfaab3

File tree

1 file changed

+15
-30
lines changed

1 file changed

+15
-30
lines changed

rtl/ibex_id_stage.sv

+15-30
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,7 @@ module ibex_id_stage #(
291291
logic data_req_allowed;
292292

293293
// CSR control
294+
logic no_flush_csr_addr;
294295
logic csr_pipe_flush;
295296

296297
logic [31:0] alu_operand_a;
@@ -511,36 +512,20 @@ module ibex_id_stage #(
511512
.branch_in_dec_o(branch_in_dec)
512513
);
513514

514-
/////////////////////////////////
515-
// CSR-related pipeline flushes //
516-
/////////////////////////////////
517-
always_comb begin : csr_pipeline_flushes
518-
csr_pipe_flush = 1'b0;
519-
520-
// A pipeline flush is needed to let the controller react after modifying certain CSRs:
521-
// - When enabling interrupts, pending IRQs become visible to the controller only during
522-
// the next cycle. If during that cycle the core disables interrupts again, it does not
523-
// see any pending IRQs and consequently does not start to handle interrupts.
524-
// - When modifying any PMP CSR, PMP check of the next instruction might get invalidated.
525-
// Hence, a pipeline flush is needed to instantiate another PMP check with the updated CSRs.
526-
// - When modifying debug CSRs.
527-
if (csr_op_en_o == 1'b1 && (csr_op_o == CSR_OP_WRITE || csr_op_o == CSR_OP_SET)) begin
528-
if (csr_num_e'(instr_rdata_i[31:20]) == CSR_MSTATUS ||
529-
csr_num_e'(instr_rdata_i[31:20]) == CSR_MIE ||
530-
csr_num_e'(instr_rdata_i[31:20]) == CSR_MSECCFG ||
531-
// To catch all PMPCFG/PMPADDR registers, get the shared top most 7 bits.
532-
instr_rdata_i[31:25] == 7'h1D) begin
533-
csr_pipe_flush = 1'b1;
534-
end
535-
end else if (csr_op_en_o == 1'b1 && csr_op_o != CSR_OP_READ) begin
536-
if (csr_num_e'(instr_rdata_i[31:20]) == CSR_DCSR ||
537-
csr_num_e'(instr_rdata_i[31:20]) == CSR_DPC ||
538-
csr_num_e'(instr_rdata_i[31:20]) == CSR_DSCRATCH0 ||
539-
csr_num_e'(instr_rdata_i[31:20]) == CSR_DSCRATCH1) begin
540-
csr_pipe_flush = 1'b1;
541-
end
542-
end
543-
end
515+
// Flush pipe on most CSR modification. Some CSR modifications alter how instructions execute
516+
// (e.g. the PMP CSRs) so this ensures all instructions always see the latest architectural state
517+
// when entering the fetch stage. This causes some needless flushes but performance impact is
518+
// limited. We have a single fetch stage to flush not many stages of a deep pipeline and CSR
519+
// instructions are in general rare and not part of performance critical parts of the code.
520+
//
521+
// No flush is triggered for a small number of specific CSRs. These are ones that have been
522+
// specifically identified to be a) likely to be modifed in exception handlers and b) safe to
523+
// alter without a flush.
524+
assign no_flush_csr_addr = csr_addr_o inside {CSR_MSCRATCH, CSR_MEPC};
525+
526+
assign csr_pipe_flush = (csr_op_en_o == 1) &&
527+
(csr_op_o inside {CSR_OP_WRITE, CSR_OP_SET, CSR_OP_CLEAR}) &&
528+
!no_flush_csr_addr;
544529

545530
////////////////
546531
// Controller //

0 commit comments

Comments
 (0)