diff --git a/rtl/ibex_controller.sv b/rtl/ibex_controller.sv index afd60cca..ccb6bdcb 100644 --- a/rtl/ibex_controller.sv +++ b/rtl/ibex_controller.sv @@ -91,6 +91,7 @@ module ibex_controller #( input logic csr_mstatus_tw_i, // stall & flush signals + input logic lsu_req_in_id_i, input logic stall_id_i, input logic stall_wb_i, output logic flush_id_o, @@ -130,6 +131,7 @@ module ibex_controller #( logic stall; logic halt_if; + logic retain_id; logic flush_id; logic illegal_dret; logic illegal_umode; @@ -379,6 +381,7 @@ module ibex_controller #( ctrl_busy_o = 1'b1; halt_if = 1'b0; + retain_id = 1'b0; flush_id = 1'b0; debug_csr_save_o = 1'b0; @@ -483,7 +486,7 @@ module ibex_controller #( // Halt IF but don't flush ID. This leaves a valid instruction in // ID so controller can determine appropriate action in the // FLUSH state. - halt_if = 1'b1; + retain_id = 1'b1; // Wait for the writeback stage to either be ready for a new instruction or raise its own // exception before going to FLUSH. If the instruction in writeback raises an exception it @@ -511,11 +514,11 @@ module ibex_controller #( // If entering debug mode or handling an IRQ the core needs to wait // until the current instruction has finished executing. Stall IF // during that time. - if ((enter_debug_mode || handle_irq) && stall) begin + if ((enter_debug_mode || handle_irq) && (stall || lsu_req_in_id_i)) begin halt_if = 1'b1; end - if (!stall && !special_req_all) begin + if (!stall && !lsu_req_in_id_i && !special_req_all) begin if (enter_debug_mode) begin // enter debug mode ctrl_fsm_ns = DBG_TAKEN_IF; @@ -777,14 +780,14 @@ module ibex_controller #( assign stall = stall_id_i | stall_wb_i; // signal to IF stage that ID stage is ready for next instr - assign id_in_ready_o = ~stall & ~halt_if; + assign id_in_ready_o = ~stall & ~halt_if & ~retain_id; // kill instr in IF-ID pipeline reg that are done, or if a // multicycle instr causes an exception for example - // halt_if is another kind of stall, where the instr_valid bit must remain + // retain_id is another kind of stall, where the instr_valid bit must remain // set (unless flush_id is set also). It cannot be factored directly into // stall as this causes a combinational loop. - assign instr_valid_clear_o = ~(stall | halt_if) | flush_id; + assign instr_valid_clear_o = ~(stall | retain_id) | flush_id; // update registers always_ff @(posedge clk_i or negedge rst_ni) begin : update_regs diff --git a/rtl/ibex_core.sv b/rtl/ibex_core.sv index 006ae57e..2303813b 100644 --- a/rtl/ibex_core.sv +++ b/rtl/ibex_core.sv @@ -163,7 +163,6 @@ module ibex_core #( // Core busy signals logic ctrl_busy; logic if_busy; - logic lsu_load; logic lsu_busy; logic core_busy_d, core_busy_q; @@ -230,7 +229,7 @@ module ibex_core #( logic id_in_ready; logic ex_valid; - logic lsu_data_valid; + logic lsu_resp_valid; // Signals between instruction core interface and pipe (if and id stages) logic instr_req_int; // Id stage asserts a req to instruction core interface @@ -495,9 +494,7 @@ module ibex_core #( // Stalls .ex_valid_i ( ex_valid ), - .lsu_valid_i ( lsu_data_valid ), - .lsu_load_i ( lsu_load ), - .lsu_busy_i ( lsu_busy ), + .lsu_resp_valid_i ( lsu_resp_valid ), .alu_operator_ex_o ( alu_operator_ex ), .alu_operand_a_ex_o ( alu_operand_a_ex ), @@ -693,13 +690,12 @@ module ibex_core #( .addr_last_o ( lsu_addr_last ), - .lsu_resp_valid_o ( lsu_data_valid ), + .lsu_resp_valid_o ( lsu_resp_valid ), // exception signals .load_err_o ( lsu_load_err ), .store_err_o ( lsu_store_err ), - .load_o ( lsu_load ), .busy_o ( lsu_busy ), .perf_load_o ( perf_load ), @@ -734,7 +730,7 @@ module ibex_core #( .rf_wdata_wb_o ( rf_wdata_wb ), .rf_we_wb_o ( rf_we_wb ), - .lsu_data_valid_i ( lsu_data_valid ), + .lsu_resp_valid_i ( lsu_resp_valid ), .instr_done_wb_o ( instr_done_wb ) ); @@ -1162,7 +1158,7 @@ module ibex_core #( // Capture read data from LSU when it becomes valid always_comb begin - if (lsu_data_valid) begin + if (lsu_resp_valid) begin rvfi_mem_rdata_d = rf_wdata_lsu; end else begin rvfi_mem_rdata_d = rvfi_mem_rdata_q; diff --git a/rtl/ibex_id_stage.sv b/rtl/ibex_id_stage.sv index 26ed52a9..f976b775 100644 --- a/rtl/ibex_id_stage.sv +++ b/rtl/ibex_id_stage.sv @@ -61,10 +61,8 @@ module ibex_id_stage #( input logic [31:0] pc_id_i, // Stalls - input logic ex_valid_i, // EX stage has valid output - input logic lsu_valid_i, // LSU has valid output, or is done - input logic lsu_load_i, // LSU is performing a load - input logic lsu_busy_i, // LSU is busy + input logic ex_valid_i, // EX stage has valid output + input logic lsu_resp_valid_i, // LSU has valid output, or is done // ALU output ibex_pkg::alu_op_e alu_operator_ex_o, output logic [31:0] alu_operand_a_ex_o, @@ -209,6 +207,7 @@ module ibex_id_stage #( logic controller_run; logic stall_ld_hz; logic stall_mem; + logic lsu_req_in_id; logic stall_multdiv; logic stall_branch; logic stall_jump; @@ -601,6 +600,7 @@ module ibex_id_stage #( .trigger_match_i ( trigger_match_i ), // stall signals + .lsu_req_in_id_i ( lsu_req_in_id ), .stall_id_i ( stall_id ), .stall_wb_i ( stall_wb ), .flush_id_o ( flush_id ), @@ -819,7 +819,7 @@ module ibex_id_stage #( if (WritebackStage) begin assign multicycle_done = lsu_req_dec ? ~stall_mem : ex_valid_i; end else begin - assign multicycle_done = lsu_req_dec ? lsu_valid_i : ex_valid_i; + assign multicycle_done = lsu_req_dec ? lsu_resp_valid_i : ex_valid_i; end // Signal instruction in ID is in it's first cycle. It can remain in its @@ -830,8 +830,6 @@ module ibex_id_stage #( assign instr_first_cycle_id_o = instr_first_cycle; if (WritebackStage) begin : gen_stall_mem - logic unused_lsu_busy; - // Register read address matches write address in WB logic rf_rd_a_wb_match; logic rf_rd_b_wb_match; @@ -839,33 +837,16 @@ module ibex_id_stage #( logic rf_rd_a_hz; logic rf_rd_b_hz; - logic data_req_complete_d; - logic data_req_complete_q; - logic outstanding_memory_access; logic instr_kill; - assign unused_lsu_busy = lsu_busy_i; - - assign data_req_complete_d = - (data_req_complete_q | (lsu_req & lsu_req_done_i)) & ~instr_id_done_o; - - always_ff @(posedge clk_i or negedge rst_ni) begin - if (~rst_ni) begin - data_req_complete_q <= 1'b0; - end else begin - data_req_complete_q <= data_req_complete_d; - end - end - // Is a memory access ongoing that isn't finishing this cycle assign outstanding_memory_access = (outstanding_load_wb_i | outstanding_store_wb_i) & - ~lsu_valid_i; + ~lsu_resp_valid_i; // Can start a new memory access if any previous one has finished or is finishing - // Don't start a new memory access if this instruction has already done it's request - assign data_req_allowed = ~outstanding_memory_access & ~data_req_complete_q; + assign data_req_allowed = ~outstanding_memory_access; // Instruction won't execute because: // - There is a pending exception in writeback @@ -896,12 +877,20 @@ module ibex_id_stage #( instr_valid_i & ~instr_kill & ~instr_executing |-> stall_id) // Stall for reasons related to memory: - // * Requested by LSU (load/store in ID/EX needs to be held in ID/EX whilst a data request or - // granted or the second half of a misaligned access is sent out) - // * LSU access required but data requests aren't currently allowed // * There is an outstanding memory access that won't resolve this cycle (need to wait to allow // precise exceptions) - assign stall_mem = instr_valid_i & ((lsu_req_dec & (~data_req_allowed | (~data_req_complete_q & ~lsu_req_done_i))) | outstanding_memory_access); + // * There is a load/store request not being granted or which is unaligned and waiting to issue + // a second request (needs to stay in ID for the address calculation) + assign stall_mem = instr_valid_i & (outstanding_memory_access | (lsu_req_dec & ~lsu_req_done_i)); + + // If we stall a load in ID for any reason, it must not make an LSU request + // (otherwide we might issue two requests for the same instruction) + `ASSERT(IbexStallMemNoRequest, + instr_valid_i & lsu_req_dec & ~instr_done |-> ~lsu_req_done_i) + + // Indicate to the controller that an lsu req is in ID stage - we cannot handle interrupts or + // debug requests until the load/store completes + assign lsu_req_in_id = instr_valid_i & lsu_req_dec; assign rf_rd_a_wb_match = (rf_waddr_wb_i == rf_raddr_a_o) & |rf_raddr_a_o; assign rf_rd_b_wb_match = (rf_waddr_wb_i == rf_raddr_b_o) & |rf_raddr_b_o; @@ -917,7 +906,7 @@ module ibex_id_stage #( assign rf_rdata_a_fwd = rf_rd_a_wb_match & rf_write_wb_i ? rf_wdata_fwd_wb_i : rf_rdata_a_i; assign rf_rdata_b_fwd = rf_rd_b_wb_match & rf_write_wb_i ? rf_wdata_fwd_wb_i : rf_rdata_b_i; - assign stall_ld_hz = outstanding_load_wb_i & lsu_load_i & (rf_rd_a_hz | rf_rd_b_hz) & rf_write_wb_i; + assign stall_ld_hz = outstanding_load_wb_i & (rf_rd_a_hz | rf_rd_b_hz); assign instr_type_wb_o = ~lsu_req_dec ? WB_INSTR_OTHER : lsu_we ? WB_INSTR_STORE : @@ -937,10 +926,11 @@ module ibex_id_stage #( // Without Writeback Stage always stall the first cycle of a load/store. // Then stall until it is complete - assign stall_mem = instr_valid_i & (lsu_busy_i | (lsu_req_dec & (~lsu_valid_i | instr_first_cycle))); + assign stall_mem = instr_valid_i & (lsu_req_dec & (~lsu_resp_valid_i | instr_first_cycle)); // No load hazards without Writeback Stage - assign stall_ld_hz = 1'b0; + assign stall_ld_hz = 1'b0; + assign lsu_req_in_id = 1'b0; // Without writeback stage any valid instruction that hasn't seen an error will execute assign instr_executing = instr_valid_i & ~instr_fetch_err_i & controller_run; @@ -967,7 +957,6 @@ module ibex_id_stage #( logic [31:0] unused_rf_wdata_fwd_wb; assign unused_data_req_done_ex = lsu_req_done_i; - assign unused_lsu_load = lsu_load_i; assign unused_rf_waddr_wb = rf_waddr_wb_i; assign unused_rf_write_wb = rf_write_wb_i; assign unused_outstanding_load_wb = outstanding_load_wb_i; @@ -980,7 +969,7 @@ module ibex_id_stage #( assign instr_type_wb_o = WB_INSTR_OTHER; assign stall_wb = 1'b0; - assign perf_dside_wait_o = instr_executing & lsu_req_dec & ~lsu_valid_i; + assign perf_dside_wait_o = instr_executing & lsu_req_dec & ~lsu_resp_valid_i; assign en_wb_o = 1'b0; assign instr_id_done_o = instr_done; diff --git a/rtl/ibex_load_store_unit.sv b/rtl/ibex_load_store_unit.sv index 2af0b55b..4d89b257 100644 --- a/rtl/ibex_load_store_unit.sv +++ b/rtl/ibex_load_store_unit.sv @@ -59,7 +59,6 @@ module ibex_load_store_unit output logic load_err_o, output logic store_err_o, - output logic load_o, output logic busy_o, output logic perf_load_o, @@ -494,7 +493,6 @@ module ibex_load_store_unit assign store_err_o = data_or_pmp_err & data_we_q & lsu_resp_valid_o; assign busy_o = (ls_fsm_cs != IDLE); - assign load_o = ~data_we_q; //////////////// // Assertions // diff --git a/rtl/ibex_wb_stage.sv b/rtl/ibex_wb_stage.sv index 8c66e758..ffe380ff 100644 --- a/rtl/ibex_wb_stage.sv +++ b/rtl/ibex_wb_stage.sv @@ -42,7 +42,7 @@ module ibex_wb_stage #( output logic [31:0] rf_wdata_wb_o, output logic rf_we_wb_o, - input logic lsu_data_valid_i, + input logic lsu_resp_valid_i, output logic instr_done_wb_o ); @@ -74,7 +74,7 @@ module ibex_wb_stage #( // Writeback for non load/store instructions always completes in a cycle (so instantly done) // Writeback for load/store must wait for response to be received by the LSU // Signal only relevant if wb_valid_q set - assign wb_done = (wb_instr_type_q == WB_INSTR_OTHER) | lsu_data_valid_i; + assign wb_done = (wb_instr_type_q == WB_INSTR_OTHER) | lsu_resp_valid_i; always_ff @(posedge clk_i or negedge rst_ni) begin if(~rst_ni) begin @@ -132,14 +132,14 @@ module ibex_wb_stage #( logic unused_en_wb; wb_instr_type_e unused_instr_type_wb; logic [31:0] unused_pc_id; - logic unused_lsu_data_valid; + logic unused_lsu_resp_valid; assign unused_clk = clk_i; assign unused_rst = rst_ni; assign unused_en_wb = en_wb_i; assign unused_instr_type_wb = instr_type_wb_i; assign unused_pc_id = pc_id_i; - assign unused_lsu_data_valid = lsu_data_valid_i; + assign unused_lsu_resp_valid = lsu_resp_valid_i; assign outstanding_load_wb_o = 1'b0; assign outstanding_store_wb_o = 1'b0;