diff --git a/rtl/ibex_controller.sv b/rtl/ibex_controller.sv index 1803c44d7..9d37a259f 100644 --- a/rtl/ibex_controller.sv +++ b/rtl/ibex_controller.sv @@ -194,7 +194,17 @@ module ibex_controller #( assign dret_insn = dret_insn_i & instr_valid_i; assign wfi_insn = wfi_insn_i & instr_valid_i; assign ebrk_insn = ebrk_insn_i & instr_valid_i; - assign csr_pipe_flush = csr_pipe_flush_i & instr_valid_i; + // csr_pipe_flush_i is already qualified by instr_valid_i at the source + // (id_stage builds it from csr_op_en_o = csr_access_o & instr_executing & + // instr_id_done_o, and instr_executing already AND-includes instr_valid_i), + // so the extra `& instr_valid_i` here is redundant. Dropping it removes one + // gate from the long combinational chain + // csr_op_en_o -> csr_pipe_flush_i -> csr_pipe_flush -> + // special_req_flush_only -> special_req -> halt_if -> + // id_in_ready_o -> ctrl_fsm_ns + // which is the dominant post-stall_id portion of the longest topological + // path on the small-config Ibex. + assign csr_pipe_flush = csr_pipe_flush_i; assign instr_fetch_err = instr_fetch_err_i & instr_valid_i; // This is recorded in the illegal_insn_q flop to help timing. Specifically diff --git a/rtl/ibex_decoder.sv b/rtl/ibex_decoder.sv index 843eb05f0..324497086 100644 --- a/rtl/ibex_decoder.sv +++ b/rtl/ibex_decoder.sv @@ -662,7 +662,25 @@ module ibex_decoder #( jump_in_dec_o = 1'b0; jump_set_o = 1'b0; branch_in_dec_o = 1'b0; - csr_access_o = 1'b0; + // csr_access_o intentionally NOT masked here: doing so creates a + // critical-path arc from decoder/illegal_insn through + // ibex_cs_registers/illegal_csr_insn_o back into id_stage/illegal_insn_o. + // Functional safety is preserved by downstream gating: + // * csr_op_en_o = csr_access_o & instr_executing & instr_id_done_o, + // and instr_executing is forced low whenever an illegal_insn raises + // id_exception, so no CSR side-effect can occur. + // * Any illegal_insn_dec=1 case is OR-merged into illegal_insn_o + // regardless of what illegal_csr_insn_o reports. + // + // SECURITY NOTE: leaving csr_access_o asserted for an illegal + // instruction means cs_registers performs its internal address decode + // for the illegal opcode's CSR field even though no architectural CSR + // side-effect occurs (csr_op_en_o is gated low). In threat models that + // include timing or power side-channels on illegal-instruction handling, + // this is a side-channel surface that the original csr_access_o = 1'b0 + // mask did not expose. Deployments where this is a concern (e.g. + // OpenTitan-style security-sensitive cores) may prefer to retain the + // mask and accept the gate cost. end end