From affb0702cf262dbab9bd427e3eb0e888150ef723 Mon Sep 17 00:00:00 2001 From: Halfdan Bechmann Date: Mon, 28 Mar 2022 17:47:52 +0200 Subject: [PATCH] Account for multicycle WFIs in rvfi Signed-off-by: Halfdan Bechmann --- bhv/cv32e40x_rvfi.sv | 37 +++++++++++++++++++++++++++++-------- bhv/cv32e40x_wrapper.sv | 2 ++ sva/cv32e40x_rvfi_sva.sv | 8 ++++---- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/bhv/cv32e40x_rvfi.sv b/bhv/cv32e40x_rvfi.sv index d5dcd6bd..995d7112 100644 --- a/bhv/cv32e40x_rvfi.sv +++ b/bhv/cv32e40x_rvfi.sv @@ -75,6 +75,7 @@ module cv32e40x_rvfi input logic ebreak_in_wb_i, input logic [31:0] instr_rdata_wb_i, input logic csr_en_wb_i, + input logic sys_wfi_insn_wb_i, // Register writes input logic rf_we_wb_i, input logic [4:0] rf_addr_wb_i, @@ -98,6 +99,7 @@ module cv32e40x_rvfi input logic debug_mode_q_i, // Interrupt Controller probes + input logic [31:0] irq_i, input logic irq_wu_ctrl_i, input logic [4:0] irq_id_ctrl_i, @@ -528,6 +530,9 @@ module cv32e40x_rvfi logic [63:0] data_wdata_ror; // Intermediate rotate signal, as direct part-select not supported in all tools + logic wb_valid_wfi_delayed; + logic wb_valid_adjusted; + logic pc_mux_debug; logic pc_mux_dret; logic pc_mux_exception; @@ -578,23 +583,25 @@ module cv32e40x_rvfi logic in_trap_clr; // Clear in trap pipeline when it reaches rvfi_intr // This is done to avoid reporting already signaled triggers as supressed during by debug - assign in_trap_clr = wb_valid_i && in_trap[STAGE_WB].intr; + assign in_trap_clr = wb_valid_adjusted && in_trap[STAGE_WB].intr; - // Store information about wakeup until fist instruction is retired + // Store information about wakeup until first instruction is retired // This information needs to be stored at the wakeup because other interrupts // can be triggered between wakeup and the first instruction executed after wakeup rvfi_wu_t rvfi_wu_next; logic store_irq_wu_cause; - assign rvfi_wu_next.wu = rvfi_sleep; // If sleep is still set, it is the firs instruction after wakeup + assign rvfi_wu_next.wu = rvfi_sleep; // If sleep is still set, it is the first instruction after wakeup always_ff @(posedge clk_i or negedge rst_ni) begin if (!rst_ni) begin + wb_valid_wfi_delayed <= 1'b0; store_irq_wu_cause <= 1'b0; rvfi_wu_next.interrupt <= 1'b0; rvfi_wu_next.debug <= 1'b0; rvfi_wu_next.cause <= '0; end else begin + wb_valid_wfi_delayed <= wb_valid_i && (ctrl_fsm_ns_i == SLEEP); // Store wake-up sources when core exits sleep if ((ctrl_fsm_cs_i == SLEEP) && (ctrl_fsm_ns_i != SLEEP)) begin rvfi_wu_next.interrupt <= irq_wu_ctrl_i; @@ -662,6 +669,15 @@ module cv32e40x_rvfi rvfi_trap_next.trap = rvfi_trap_next.exception || rvfi_trap_next.debug; end + + // WFI instructions can use two cycles in WB if there are no flopped pending wakeup sources. + // In this case their retirement happens in the second cycle, but becuase it makes no difference functionally in rtl, + // wb_valid is set in the first WFI cycle to avoid introducing support logic for a multicycle WB stage. + // Moving wb_valid to second cycle of WFI instructions in these cases is therefore needed. + assign wb_valid_adjusted = (sys_wfi_insn_wb_i && ((ctrl_fsm_ns_i == SLEEP) || (ctrl_fsm_cs_i == SLEEP))) ? + wb_valid_wfi_delayed : + wb_valid_i; + // Pipeline stage model // always_ff @(posedge clk_i or negedge rst_ni) begin @@ -824,8 +840,9 @@ module cv32e40x_rvfi //// WB Stage //// - rvfi_valid <= wb_valid_i; - if (wb_valid_i) begin + rvfi_valid <= wb_valid_adjusted; + if (wb_valid_adjusted) begin + rvfi_order <= rvfi_order + 64'b1; rvfi_pc_rdata <= pc_wb_i; rvfi_insn <= instr_rdata_wb_i; @@ -865,8 +882,8 @@ module cv32e40x_rvfi rvfi_pc_wdata <= (pc_mux_debug || pc_mux_exception) ? branch_addr_n_i & ~32'b1 : (pc_mux_dret) ? csr_dpc_q_i : pc_wdata[STAGE_WB] & ~32'b1; - end else begin // if (wb_valid_i) - rvfi_sleep <= rvfi_sleep; // Keep sleep signal asserted until next valid WB + end else begin + rvfi_sleep <= rvfi_sleep; // Keep sleep signal asserted until next valid WB end end @@ -1009,8 +1026,12 @@ module cv32e40x_rvfi assign rvfi_csr_wdata_d.mtval = '0; // Not implemented, read 0 assign rvfi_csr_wmask_d.mtval = '0; + // MIP is read in EX by CSR instructions, evaluated combinatorically in WB by the WFI instruction, + // and is evaluated in WB for all other instructions assign ex_csr_rdata_d.mip = csr_mip_q_i; - assign rvfi_csr_rdata_d.mip = csr_en_wb_i ? ex_csr_rdata.mip : csr_mip_q_i; + assign rvfi_csr_rdata_d.mip = csr_en_wb_i ? ex_csr_rdata.mip : + sys_wfi_insn_wb_i ? irq_i : + csr_mip_q_i; assign rvfi_csr_wdata_d.mip = csr_mip_n_i; assign rvfi_csr_wmask_d.mip = csr_mip_we_i ? '1 : '0; diff --git a/bhv/cv32e40x_wrapper.sv b/bhv/cv32e40x_wrapper.sv index 9d8661b9..552939a1 100644 --- a/bhv/cv32e40x_wrapper.sv +++ b/bhv/cv32e40x_wrapper.sv @@ -367,6 +367,7 @@ module cv32e40x_wrapper .wb_ready_i ( core_i.wb_stage_i.wb_ready_o ), .instr_rdata_wb_i ( core_i.wb_stage_i.ex_wb_pipe_i.instr.bus_resp.rdata ), .csr_en_wb_i ( core_i.wb_stage_i.ex_wb_pipe_i.csr_en ), + .sys_wfi_insn_wb_i ( core_i.wb_stage_i.ex_wb_pipe_i.sys_wfi_insn ), .ebreak_in_wb_i ( core_i.controller_i.controller_fsm_i.ebreak_in_wb ), .rs1_addr_id_i ( core_i.register_file_wrapper_i.raddr_i[0] ), @@ -424,6 +425,7 @@ module cv32e40x_wrapper .nmi_is_store_i ( core_i.controller_i.controller_fsm_i.nmi_is_store_q ), .pending_debug_i ( core_i.controller_i.controller_fsm_i.pending_debug ), .debug_mode_q_i ( core_i.controller_i.controller_fsm_i.debug_mode_q ), + .irq_i ( core_i.irq_i & IRQ_MASK ), .irq_wu_ctrl_i ( core_i.irq_wu_ctrl ), .irq_id_ctrl_i ( core_i.irq_id_ctrl ), // CSRs diff --git a/sva/cv32e40x_rvfi_sva.sv b/sva/cv32e40x_rvfi_sva.sv index dd73de56..835ae90e 100644 --- a/sva/cv32e40x_rvfi_sva.sv +++ b/sva/cv32e40x_rvfi_sva.sv @@ -198,11 +198,11 @@ module cv32e40x_rvfi_sva (rvfi_wu.wu && rvfi_valid) |-> previous_instruction_was_sleep) else `uvm_error("rvfi", "rvfi_wu asserted without a previous rvfi_sleep") - // Check that the last instruction before core_sleep_o had rvfi_sleep set - a_always_core_sleep_always_preceeded_by_instr_with_rvfi_sleep: + // Check that the last instruction before (or at the same time as) core_sleep_o goes high has rvfi_sleep set + a_core_sleep_always_preceeded_by_instr_with_rvfi_sleep: assert property (@(posedge clk_i) disable iff (!rst_ni) - core_sleep_i |-> previous_instruction_was_sleep) - else `uvm_error("rvfi", "The instruction before asserted core_sleep_o did not have rvfi_sleep set") + core_sleep_i |-> (rvfi_sleep || previous_instruction_was_sleep)) + else `uvm_error("rvfi", "The last instruction before asserted core_sleep_o did not have rvfi_sleep set") // Check that rvfi_sleep is always set when the core is sleeping // This assertion is stricter than needed (behavior only specified for valid insructions)