Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a 'first_op' to track the first operation of multi operation in… #605

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions bhv/cv32e40x_rvfi.sv
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ module cv32e40x_rvfi
input logic [4:0] rs2_addr_id_i,
input logic [31:0] operand_a_fw_id_i,
input logic [31:0] operand_b_fw_id_i,
input logic first_op_id_i,

// EX probes
input logic ex_ready_i,
Expand Down Expand Up @@ -873,8 +874,11 @@ module cv32e40x_rvfi
instr_pmp_err[STAGE_EX] <= instr_pmp_err[STAGE_ID];
rs1_addr [STAGE_EX] <= rs1_addr_id;
rs2_addr [STAGE_EX] <= rs2_addr_id;
rs1_rdata [STAGE_EX] <= rs1_rdata_id;
rs2_rdata [STAGE_EX] <= rs2_rdata_id;
// Only update rs1/rs2 on the first part of a multi operation instruction.
// Jumps may actually use rs1 before (id_valid && ex_ready), an assertion exists to check that
// the jump target is stable and it should be safe to use rs1/2_rdata at the time of the pipeline handshake.
rs1_rdata [STAGE_EX] <= first_op_id_i ? rs1_rdata_id : rs1_rdata[STAGE_EX];
Silabs-ArjanB marked this conversation as resolved.
Show resolved Hide resolved
rs2_rdata [STAGE_EX] <= first_op_id_i ? rs2_rdata_id : rs1_rdata[STAGE_EX];
mem_rmask [STAGE_EX] <= (lsu_en_id_i && !lsu_we_id_i) ? rvfi_mem_mask_int : '0;
mem_wmask [STAGE_EX] <= (lsu_en_id_i && lsu_we_id_i) ? rvfi_mem_mask_int : '0;
end else begin
Expand Down
3 changes: 3 additions & 0 deletions bhv/cv32e40x_wrapper.sv
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,14 @@ module cv32e40x_wrapper
bind cv32e40x_id_stage:
core_i.id_stage_i cv32e40x_id_stage_sva id_stage_sva
(
.jmp_taken_id_ctrl_i (core_i.controller_i.controller_fsm_i.jump_taken_id),
.*
);

bind cv32e40x_ex_stage:
core_i.ex_stage_i cv32e40x_ex_stage_sva #(.X_EXT(X_EXT)) ex_stage_sva
(
.branch_taken_ex_ctrl_i (core_i.controller_i.controller_fsm_i.branch_taken_ex),
.*
);

Expand Down Expand Up @@ -406,6 +408,7 @@ module cv32e40x_wrapper
.rs2_addr_id_i ( core_i.register_file_wrapper_i.raddr_i[1] ),
.operand_a_fw_id_i ( core_i.id_stage_i.operand_a_fw ),
.operand_b_fw_id_i ( core_i.id_stage_i.operand_b_fw ),
.first_op_id_i ( core_i.id_stage_i.if_id_pipe_i.first_op ),

// EX Probes
.ex_ready_i ( core_i.ex_stage_i.ex_ready_o ),
Expand Down
3 changes: 2 additions & 1 deletion rtl/cv32e40x_controller_fsm.sv
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Blocking on branch_taken_q, as a jump has already been taken
assign jump_taken_id = jump_in_id && !branch_taken_q;

// todo: RVFI does not use jump_taken_id (which is not in itself an issue); we should have an assertion showing that the target address remains constant during jump_in_id; same remark for branches
// Note: RVFI does not use jump_taken_id (which is not in itself an issue); An assertion in id_stage_sva checks that the jump target remains stable;
// todo: Do we need a similar stability check for branches?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; just add it now (will be less work than picking it up a month from now)


// EX stage
// Branch taken for valid branch instructions in EX with valid decision
Expand Down
8 changes: 7 additions & 1 deletion rtl/cv32e40x_core.sv
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,8 @@ module cv32e40x_core import cv32e40x_pkg::*;

// LSU
logic lsu_split_ex;
logic lsu_first_op_ex;
logic lsu_last_op_ex;
mpu_status_e lsu_mpu_status_wb;
logic [31:0] lsu_rdata_wb;
logic [1:0] lsu_err_wb;
Expand Down Expand Up @@ -556,6 +558,8 @@ module cv32e40x_core import cv32e40x_pkg::*;
.lsu_valid_o ( lsu_valid_ex ),
.lsu_ready_i ( lsu_ready_0 ),
.lsu_split_i ( lsu_split_ex ),
.lsu_last_op_i ( lsu_last_op_ex ),
.lsu_first_op_i ( lsu_first_op_ex ),

// Pipeline handshakes
.ex_ready_o ( ex_ready ),
Expand Down Expand Up @@ -601,11 +605,13 @@ module cv32e40x_core import cv32e40x_pkg::*;

// Stage 0 outputs (EX)
.lsu_split_0_o ( lsu_split_ex ),
.lsu_mpu_status_1_o ( lsu_mpu_status_wb ),
.lsu_first_op_0_o ( lsu_first_op_ex ),
.lsu_last_op_0_o ( lsu_last_op_ex ),

// Stage 1 outputs (WB)
.lsu_err_1_o ( lsu_err_wb ), // To controller (has WB timing, but does not pass through WB stage)
.lsu_rdata_1_o ( lsu_rdata_wb ),
.lsu_mpu_status_1_o ( lsu_mpu_status_wb ),

// Valid/ready
.valid_0_i ( lsu_valid_ex ), // First LSU stage (EX)
Expand Down
11 changes: 10 additions & 1 deletion rtl/cv32e40x_ex_stage.sv
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
output logic lsu_valid_o,
input logic lsu_ready_i,
input logic lsu_split_i, // LSU is performing first part of a misaligned/split instruction
input logic lsu_last_op_i,
input logic lsu_first_op_i,

// Stage ready/valid
output logic ex_ready_o, // EX stage is ready for new data
Expand Down Expand Up @@ -118,6 +120,9 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
// Detect if we get an illegal CSR instruction
logic csr_is_illegal;

// Local first_op signal
logic first_op;

assign instr_valid = id_ex_pipe_i.instr_valid && !ctrl_fsm_i.kill_ex && !ctrl_fsm_i.halt_ex;

assign mul_en_gated = id_ex_pipe_i.mul_en && instr_valid; // Factoring in instr_valid to kill mul instructions on kill/halt
Expand Down Expand Up @@ -166,7 +171,9 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;

// Detect last operation
// Both parts of a split misaligned load/store will reach WB, but only the second half will be marked with "last_op"
assign last_op_o = (id_ex_pipe_i.lsu_en && lsu_split_i) ? 1'b0 : id_ex_pipe_i.last_op;
assign last_op_o = id_ex_pipe_i.lsu_en ? (lsu_last_op_i && id_ex_pipe_i.last_op) : id_ex_pipe_i.last_op;

assign first_op = id_ex_pipe_i.lsu_en ? (lsu_first_op_i && id_ex_pipe_i.first_op) : id_ex_pipe_i.first_op;

////////////////////////////
// _ _ _ _ //
Expand Down Expand Up @@ -344,6 +351,7 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
ex_wb_pipe_o.csr_mnxti_access <= 1'b0;
ex_wb_pipe_o.xif_en <= 1'b0;
ex_wb_pipe_o.xif_meta <= '0;
ex_wb_pipe_o.first_op <= 1'b0;
ex_wb_pipe_o.last_op <= 1'b0;
end
else
Expand All @@ -352,6 +360,7 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;
ex_wb_pipe_o.instr_valid <= 1'b1;

ex_wb_pipe_o.last_op <= last_op_o;
ex_wb_pipe_o.first_op <= first_op;
// Deassert rf_we in case of illegal csr instruction or
// when the first half of a misaligned/split LSU goes to WB.
// Also deassert if CSR was accepted both by eXtension if and pipeline
Expand Down
2 changes: 2 additions & 0 deletions rtl/cv32e40x_id_stage.sv
Original file line number Diff line number Diff line change
Expand Up @@ -543,12 +543,14 @@ module cv32e40x_id_stage import cv32e40x_pkg::*;
id_ex_pipe_o.instr_meta <= '0;
id_ex_pipe_o.trigger_match <= 1'b0;

id_ex_pipe_o.first_op <= 1'b0;
id_ex_pipe_o.last_op <= 1'b0;
end else begin
// normal pipeline unstall case
if (id_valid_o && ex_ready_i) begin
id_ex_pipe_o.instr_valid <= 1'b1;
id_ex_pipe_o.last_op <= if_id_pipe_i.last_op;
id_ex_pipe_o.first_op <= if_id_pipe_i.first_op;

// Operands
if (alu_op_a_mux_sel != OP_A_NONE) begin
Expand Down
13 changes: 11 additions & 2 deletions rtl/cv32e40x_if_stage.sv
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
// eXtension interface signals
logic [X_ID_WIDTH-1:0] xif_id;

// Flag for last operation - used by Zc*
// Flags for first and last operation of an instruction
logic first_op;
logic last_op;

// ready signal for predecoder, tied to id_ready_i
Expand All @@ -132,6 +133,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
// Zc* sequencer signals
logic seq_valid;
logic seq_ready;
logic seq_first;
logic seq_last;
inst_resp_t seq_instr;

Expand Down Expand Up @@ -301,6 +303,10 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
// todo: Factor in last operation of sequenced Zc* (push/pop)
assign last_op = tbljmp ? 1'b0 : 1'b1;

// Flag first operation of a sequence.
// todo: factor in seq_first. For now only tablejump pointer may be the only non-first operation.
assign first_op = prefetch_is_tbljmp_ptr ? 1'b0 : 1'b1;
Silabs-ArjanB marked this conversation as resolved.
Show resolved Hide resolved

// Populate instruction meta data
// Fields 'compressed' and 'tbljmp' keep their old value by default.
// - In case of a table jump we need the fields to stay as 'compressed=1' and 'tbljmp=1'
Expand Down Expand Up @@ -329,6 +335,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
if_id_pipe_o.xif_id <= '0;
if_id_pipe_o.ptr <= '0;
if_id_pipe_o.last_op <= 1'b0;
if_id_pipe_o.first_op <= 1'b0;
end else begin
// Valid pipeline output if we are valid AND the
// alignment buffer has a valid instruction
Expand All @@ -343,6 +350,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
if_id_pipe_o.trigger_match <= trigger_match_i;
if_id_pipe_o.xif_id <= xif_id;
if_id_pipe_o.last_op <= last_op;
if_id_pipe_o.first_op <= first_op;

// No PC update for tablejump pointer, PC of instruction itself is needed later.
// No update to the meta compressed, as this is used in calculating the link address.
Expand Down Expand Up @@ -420,9 +428,10 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
);
end else begin : gen_no_seq
assign seq_valid = 1'b0;
assign seq_last = 1'b0;
assign seq_last = 1'b0;
assign seq_instr = '0;
assign seq_ready = 1'b1;
assign seq_first = 1'b0;
end
endgenerate

Expand Down
7 changes: 7 additions & 0 deletions rtl/cv32e40x_load_store_unit.sv
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;

// Stage 0 outputs (EX)
output logic lsu_split_0_o, // Misaligned access is split in two transactions (to controller)
output logic lsu_first_op_0_o, // First operation is active in EX
output logic lsu_last_op_0_o, // Last operation is active in EX

// Stage 1 outputs (WB)
output logic [1:0] lsu_err_1_o,
Expand Down Expand Up @@ -387,6 +389,11 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;
end
end

// Set flags for first and last op
// Only valid when id_ex_pipe.lsu_en == 1
assign lsu_last_op_0_o = !lsu_split_0_o;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment that these signals are only valid when lsu_en = 1 (I think)

assign lsu_first_op_0_o = !split_q;

// Busy if there are ongoing (or potentially outstanding) transfers
// In the case of mpu errors, the LSU control logic can have outstanding transfers not seen by the response filter.
assign busy_o = filter_resp_busy || (cnt_q > 0) || trans_valid;
Expand Down
7 changes: 5 additions & 2 deletions rtl/include/cv32e40x_pkg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,7 @@ typedef struct packed {
logic trigger_match;
logic [31:0] xif_id; // ID of offloaded instruction
logic [31:0] ptr; // Flops to hold 32-bit pointer
logic first_op; // First part of multi operation instruction
logic last_op; // Last part of multi operation instruction
} if_id_pipe_t;

Expand Down Expand Up @@ -1113,7 +1114,8 @@ typedef struct packed {
logic xif_en; // Instruction has been offloaded via eXtension interface
xif_meta_t xif_meta; // xif meta struct

logic last_op;
logic first_op; // First part of multi operation instruction
logic last_op; // Last part of multi operation instruction

} id_ex_pipe_t;

Expand Down Expand Up @@ -1160,7 +1162,8 @@ typedef struct packed {
logic xif_en; // Instruction has been offloaded via eXtension interface
xif_meta_t xif_meta; // xif meta struct

logic last_op;
logic first_op; // First part of multi operation instruction
logic last_op; // Last part of multi operation instruction
} ex_wb_pipe_t;

// Performance counter events
Expand Down
25 changes: 20 additions & 5 deletions sva/cv32e40x_ex_stage_sva.sv
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Copyright 2021 Silicon Labs, Inc.
//
//
// This file, and derivatives thereof are licensed under the
// Solderpad License, Version 2.0 (the "License");
// Use of this file means you agree to the terms and conditions
// of the license and are in full compliance with the License.
// You may obtain a copy of the License at
//
//
// https://solderpad.org/licenses/SHL-2.0/
//
//
// Unless required by applicable law or agreed to in writing, software
// and hardware implementations thereof
// distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -26,7 +26,7 @@
module cv32e40x_ex_stage_sva
import uvm_pkg::*;
import cv32e40x_pkg::*;
#(
#(
parameter bit X_EXT = 1'b0
)
(
Expand All @@ -41,7 +41,10 @@ module cv32e40x_ex_stage_sva
input id_ex_pipe_t id_ex_pipe_i,
input ex_wb_pipe_t ex_wb_pipe_o,
input logic lsu_split_i,
input logic csr_illegal_i
input logic csr_illegal_i,

input logic [31:0] branch_target_o,
input logic branch_taken_ex_ctrl_i
);

// Halt implies not ready and not valid
Expand Down Expand Up @@ -111,4 +114,16 @@ endgenerate
id_ex_pipe_i.csr_en, id_ex_pipe_i.sys_en, id_ex_pipe_i.lsu_en, id_ex_pipe_i.xif_en}))
else `uvm_error("ex_stage", "Multiple functional units enabled")

// Check that branch target remains constant while a branch instruction is in EX
property p_bch_target_stable;
logic [31:0] bch_target;
@(posedge clk) disable iff (!rst_n)
(branch_taken_ex_ctrl_i && !ctrl_fsm_i.kill_ex, bch_target=branch_target_o)
|->
(bch_target == branch_target_o) until_with ((ex_valid_o && wb_ready_i) || ctrl_fsm_i.kill_ex);
endproperty

a_bch_target_stable: assert property (p_bch_target_stable)
else `uvm_error("ex_stage", "Branch target not stable")

endmodule // cv32e40x_ex_stage_sva
36 changes: 35 additions & 1 deletion sva/cv32e40x_id_stage_sva.sv
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,17 @@ module cv32e40x_id_stage_sva
input logic id_ready_o,
input logic id_valid_o,
input ctrl_fsm_t ctrl_fsm_i,
input logic xif_insn_accept
input logic xif_insn_accept,
input logic [31:0] jalr_fw,
input logic [31:0] operand_a_fw,
input ctrl_byp_t ctrl_byp_i,
input logic alu_jmp,
input logic alu_jmpr,
input logic [31:0] jmp_target_o,
input logic jmp_taken_id_ctrl_i



);

/* todo: check and fix/remove
Expand Down Expand Up @@ -125,5 +135,29 @@ module cv32e40x_id_stage_sva
$onehot0({alu_en, div_en, mul_en, csr_en, sys_en, lsu_en, xif_en}))
else `uvm_error("id_stage", "Multiple functional units enabled")

// Assert that jalr_fw has the same value as operand_a_fw when a jump is taken
// Only checking for JALR, as regular JAL do not use any RF or bypass operands for the jump target.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add reason why this needs to be checked (i.e. because RVFI uses alu_jmpr instead of operand_a_fw)

// Checked because RVFI is using operand_a_fw only, even for JALR instructions. With this assert proven,
// there is no need to mux in jalr_fw inside RVFI.
a_jalr_fw_match :
assert property (@(posedge clk) disable iff (!rst_n)
jmp_taken_id_ctrl_i && alu_jmpr
|->
(jalr_fw == operand_a_fw))
else `uvm_error("id_stage", "jalr_fw does not match operand_a_fw")

// Assert stable jump target for a jump instruction that stays multiple cycles in ID
// Target must remain stable until instruction exits ID (id_valid && ex_ready, or
// instructions is killed.
property p_jmp_target_stable;
logic [31:0] jmp_target;
@(posedge clk) disable iff (!rst_n)
(jmp_taken_id_ctrl_i && !ctrl_fsm_i.kill_id, jmp_target=jmp_target_o)
Silabs-ArjanB marked this conversation as resolved.
Show resolved Hide resolved
|->
(jmp_target == jmp_target_o) until_with ((id_valid_o && ex_ready_i) || ctrl_fsm_i.kill_id);
endproperty

a_jmp_target_stable: assert property (p_jmp_target_stable)
else `uvm_error("id_stage", "Jump target not stable")
endmodule // cv32e40x_id_stage_sva