diff --git a/src/commit_stage.sv b/src/commit_stage.sv index e91be4e68..57f2bed02 100644 --- a/src/commit_stage.sv +++ b/src/commit_stage.sv @@ -119,6 +119,7 @@ module commit_stage #( // CSR Logic // --------- // check whether the instruction we retire was a CSR instruction + // interrupts are never taken on CSR instructions if (commit_instr_i[0].fu == CSR) begin // write the CSR file commit_csr_o = 1'b1; @@ -129,6 +130,8 @@ module commit_stage #( // ------------------ // SFENCE.VMA Logic // ------------------ + // sfence.vma is idempotent so we can safely re-execute it after returning + // from interrupt service routine // check if this instruction was a SFENCE_VMA if (commit_instr_i[0].op == SFENCE_VMA) begin // no store pending so we can flush the TLBs and pipeline @@ -139,6 +142,8 @@ module commit_stage #( // ------------------ // FENCE.I Logic // ------------------ + // fence.i is idempotent so we can safely re-execute it after returning + // from interrupt service routine // Fence synchronizes data and instruction streams. That means that we need to flush the private icache // and the private dcache. This is the most expensive instruction. if (commit_instr_i[0].op == FENCE_I || (flush_dcache_i && commit_instr_i[0].fu != STORE)) begin @@ -149,6 +154,8 @@ module commit_stage #( // ------------------ // FENCE Logic // ------------------ + // fence is idempotent so we can safely re-execute it after returning + // from interrupt service routine if (commit_instr_i[0].op == FENCE) begin commit_ack_o[0] = no_st_pending_i; // tell the controller to flush the D$ @@ -192,6 +199,7 @@ module commit_stage #( // ----------------------------- // Exception & Interrupt Logic // ----------------------------- + // TODO(zarubaf): Move interrupt handling to commit stage. // here we know for sure that we are taking the exception always_comb begin : exception_handling // Multiple simultaneous interrupts and traps at the same privilege level are handled in the following decreasing @@ -227,8 +235,15 @@ module commit_stage #( // ------------------------ // check for CSR interrupts (e.g.: normal interrupts which get triggered here) // by putting interrupts here we give them precedence over any other exception - // Don't take the interrupt if we are committing an AMO. - if (csr_exception_i.valid && csr_exception_i.cause[63] && !amo_valid_commit_o) begin + // Don't take the interrupt if we are committing an AMO or a CSR. + // - Atomics because they are atomic in their nature and should not be interrupted + // - CSRs because it makes the implementation easier as CSRs are figured out at the same + // time as interrupts (here in the commit stage). By not allowing them on CSRs we + // reduce the potential critical path length. As all CSRs are single-cycle (plus a + // potential pipeline flush) this only impacts interrupt latency in a couple of cycles. + if (csr_exception_i.valid && csr_exception_i.cause[63] + && !amo_valid_commit_o + && commit_instr_i[0].fu != CSR) begin exception_o = csr_exception_i; exception_o.tval = commit_instr_i[0].ex.tval; end @@ -240,4 +255,4 @@ module commit_stage #( end end -endmodule \ No newline at end of file +endmodule