Skip to content

Commit

Permalink
Merge pull request #457 from silabs-oysteink/silabs-oysteink_issue-456
Browse files Browse the repository at this point in the history
Fix for issue #456
  • Loading branch information
Silabs-ArjanB committed May 24, 2023
2 parents d85b601 + dddfb76 commit 634c384
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 43 deletions.
7 changes: 6 additions & 1 deletion bhv/cv32e40s_rvfi.sv
Expand Up @@ -913,7 +913,12 @@ module cv32e40s_rvfi
if (is_dummy_instr_wb_i) begin
dummy_suppressed_intr <= in_trap[STAGE_WB] || dummy_suppressed_intr;
end else begin
dummy_suppressed_intr <= 1'b0;
// Only clear the flag once an instruction fully completes.
// Otherwise the first operation of a sequence could clear the flag, causing the
// rvfi_valid following (wb_valid && last_op) to miss its rvfi_intr.
if (last_op_wb_i) begin
dummy_suppressed_intr <= 1'b0;
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion bhv/cv32e40s_wrapper.sv
Expand Up @@ -212,7 +212,7 @@ module cv32e40s_wrapper
(
.if_valid_i ( core_i.if_valid ),
.id_ready_i ( core_i.id_ready ),
.hint_id_i ( core_i.if_id_pipe.instr_meta.hint ),
.hint_if_i ( core_i.if_stage_i.instr_hint ),
.*
);
end
Expand Down
4 changes: 0 additions & 4 deletions rtl/cv32e40s_controller.sv
Expand Up @@ -46,10 +46,8 @@ module cv32e40s_controller import cv32e40s_pkg::*;

// From IF stage
input logic [31:0] pc_if_i,
input logic first_op_nondummy_if_i,
input logic last_op_if_i,
input logic abort_op_if_i,
input logic prefetch_valid_if_i,

// from IF/ID pipeline
input if_id_pipe_t if_id_pipe_i,
Expand Down Expand Up @@ -163,10 +161,8 @@ module cv32e40s_controller import cv32e40s_pkg::*;

.if_valid_i ( if_valid_i ),
.pc_if_i ( pc_if_i ),
.first_op_nondummy_if_i ( first_op_nondummy_if_i ),
.last_op_if_i ( last_op_if_i ),
.abort_op_if_i ( abort_op_if_i ),
.prefetch_valid_if_i ( prefetch_valid_if_i ),

// From ID stage
.if_id_pipe_i ( if_id_pipe_i ),
Expand Down
7 changes: 2 additions & 5 deletions rtl/cv32e40s_controller_fsm.sv
Expand Up @@ -47,10 +47,8 @@ module cv32e40s_controller_fsm import cv32e40s_pkg::*;

// From IF stage
input logic [31:0] pc_if_i,
input logic first_op_nondummy_if_i, // IF stage is handling the first operation of a sequence
input logic last_op_if_i, // IF stage is handling the last operation of a sequence.
input logic abort_op_if_i, // IF stage contains an operation that will be aborted (bus error or MPU exception)
input logic prefetch_valid_if_i, // Prefecher in IF stage has a valid instruction (similar as if_id_pipe.instr_valid)

// From ID stage
input if_id_pipe_t if_id_pipe_i,
Expand Down Expand Up @@ -292,11 +290,10 @@ module cv32e40s_controller_fsm import cv32e40s_pkg::*;

////////////////////////////////////////////////////////////////////
// - Blocking dummy instructions during single stepping and in debug mode.
// - Blocking dummies if there is a non-first op in IF (sequencer is in the middle of a sequence)
// a_no_back_to_back_dummy in if_stage_sva checks that dummies can't come back-to-back with no other instructions in between.
assign ctrl_fsm_o.allow_dummy_instr = !dcsr_i.step && // Valid in IF because it can only be written in debug mode
!debug_mode_q && // Valid in IF because pipeline is killed when entering and exiting debug
(first_op_nondummy_if_i && prefetch_valid_if_i);
!debug_mode_q; // Valid in IF because pipeline is killed when entering and exiting debug


// ID stage

Expand Down
7 changes: 0 additions & 7 deletions rtl/cv32e40s_core.sv
Expand Up @@ -209,7 +209,6 @@ module cv32e40s_core import cv32e40s_pkg::*;
logic abort_op_wb;

// First op bits
logic first_op_nondummy_if;
logic first_op_id;
logic first_op_ex;

Expand Down Expand Up @@ -301,7 +300,6 @@ module cv32e40s_core import cv32e40s_pkg::*;
logic ex_valid;
logic wb_valid;

logic prefetch_valid_if;

// Interrupts
mstatus_t mstatus;
Expand Down Expand Up @@ -572,12 +570,9 @@ module cv32e40s_core import cv32e40s_pkg::*;
.ptr_in_if_o ( ptr_in_if ),
.priv_lvl_if_o ( priv_lvl_if ),

.first_op_nondummy_o ( first_op_nondummy_if ),
.last_op_o ( last_op_if ),
.abort_op_o ( abort_op_if ),

.prefetch_valid_o ( prefetch_valid_if ),

// Pipeline handshakes
.if_valid_o ( if_valid ),
.id_ready_i ( id_ready ),
Expand Down Expand Up @@ -1042,10 +1037,8 @@ module cv32e40s_core import cv32e40s_pkg::*;

.if_valid_i ( if_valid ),
.pc_if_i ( pc_if ),
.first_op_nondummy_if_i ( first_op_nondummy_if ),
.last_op_if_i ( last_op_if ),
.abort_op_if_i ( abort_op_if ),
.prefetch_valid_if_i ( prefetch_valid_if ),

// from IF/ID pipeline
.if_id_pipe_i ( if_id_pipe ),
Expand Down
13 changes: 11 additions & 2 deletions rtl/cv32e40s_dummy_instr.sv
Expand Up @@ -36,6 +36,9 @@ module cv32e40s_dummy_instr
(input logic clk,
input logic rst_n,
input logic instr_issued_i,
input logic first_op_nondummy_i,
input logic ptr_in_if_i,
input logic prefetch_valid_i,
input ctrl_fsm_t ctrl_fsm_i,
input xsecure_ctrl_t xsecure_ctrl_i,
output logic dummy_insert_o,
Expand Down Expand Up @@ -85,8 +88,14 @@ module cv32e40s_dummy_instr
// lfsr_cnt will update when the dummy or hint instruction leaves the ID stage
// cnt_q is updated every time an instruction goes from IF to ID
// - reset when a dummy goes from IF to ID
// - incremented when any other instruction (including hint) goes from ID to ID
assign dummy_insert_o = (cnt_q > lfsr_cnt) && dummy_en;
// - incremented when any other instruction (including hint) goes from IF to ID
// Not allowing dummy insertion when the IF stage is handling a pointer.
// Pointers are not counted as instructions in the IF stage, thus instr_issued_i will remain
// low for pointers, causing cnt_q not to reset on dummy insertion.
// If allowing dummies for pointers, pointers would also need to count as instructions.
assign dummy_insert_o = (cnt_q > lfsr_cnt) && dummy_en && // Limit reached and dummies enabled
(first_op_nondummy_i && prefetch_valid_i) && // IF stage is on instruction boundary
!ptr_in_if_i; // No pointer is in IF

assign cnt_rst = !dummy_en || // Reset counter when dummy instructions are disabled
(dummy_insert_o && instr_issued_i) || // Reset counter when inserting dummy instruction which is propagated to the ID stage
Expand Down
42 changes: 24 additions & 18 deletions rtl/cv32e40s_if_stage.sv
Expand Up @@ -79,12 +79,9 @@ module cv32e40s_if_stage import cv32e40s_pkg::*;
output logic ptr_in_if_o, // The IF stage currently holds a pointer
output privlvl_t priv_lvl_if_o, // Privilege level of the instruction currently in IF

output logic first_op_nondummy_o,
output logic last_op_o,
output logic abort_op_o,

output logic prefetch_valid_o,

// Stage ready/valid
output logic if_valid_o,
input logic id_ready_i,
Expand Down Expand Up @@ -186,6 +183,7 @@ module cv32e40s_if_stage import cv32e40s_pkg::*;
logic id_ready_no_dummy; // Ready signal to acknowledge the sequencer

logic first_op; // Local first_op, including dummies
logic first_op_nondummy; // first_op, excluding dummies

logic integrity_err_obi; // Integrity error from OBI interface
logic protocol_err_obi; // Protocol error from OBI interface
Expand Down Expand Up @@ -473,21 +471,19 @@ module cv32e40s_if_stage import cv32e40s_pkg::*;
// Any other instruction will be single operation, and gets first_op=1.
// Regular CLIC pointers are single operation with first_op == last_op == 1
// CLIC pointers that are a side effect of mret instructions will have first_op == 0 and last_op == 1
assign first_op_nondummy_o = seq_valid ? seq_first :
prefetch_is_mret_ptr ? 1'b0 :
1'b1; // Any other regular instructions are single operation.
assign first_op_nondummy = seq_valid ? seq_first :
prefetch_is_mret_ptr ? 1'b0 :
1'b1; // Any other regular instructions are single operation.

// Local first_op, including dummy instructions
assign first_op = dummy_insert ? 1'b1 : first_op_nondummy_o;
assign first_op = dummy_insert ? 1'b1 : first_op_nondummy;


// Set flag to indicate that instruction/sequence will be aborted due to known exceptions or trigger match
assign abort_op_o = dummy_insert ? 1'b0 :
(instr_decompressed.bus_resp.err || (instr_decompressed.mpu_status != MPU_OK) ||
(instr_decompressed.bus_resp.integrity_err) || (instr_decompressed.align_status != ALIGN_OK) || |trigger_match_i);

assign prefetch_valid_o = prefetch_valid;

// Signal current privilege level of IF
assign priv_lvl_if_o = prefetch_priv_lvl;

Expand Down Expand Up @@ -614,7 +610,7 @@ module cv32e40s_if_stage import cv32e40s_pkg::*;
// Predecoder is purely combinatorial and is always ready for new inputs
assign predec_ready = id_ready_i && !dummy_insert;

// Dummies are allowed when first_op_nondummy_o == 1
// Dummies are allowed when first_op_nondummy == 1
// If the first operation of a sequence is ready, we allow dummies
// but must not advance the sequencer.
assign id_ready_no_dummy = id_ready_i && !dummy_insert;
Expand Down Expand Up @@ -676,17 +672,27 @@ module cv32e40s_if_stage import cv32e40s_pkg::*;
generate
if (DUMMY_INSTRUCTIONS) begin : gen_dummy_instr
logic instr_issued; // Used to count issued instructions between dummy instructions
assign instr_issued = if_valid_o && id_ready_i;

// Count instructions when first_op==1 to not count suboperations of sequences
// Using first_op instead of last_op to not double count mrets which restarts CLIC pointer fetches.
// Mret instructions are not decoded in IF, and are signaled as single operation instructions. If it restarts
// a pointer fetch (detected in the ID stage) the mret is stretched into two operations - one for the mret
// and one for the pointer fetch. Both operations will the get last_op==1, but only the first will have first_op==1.
// Do not count pointers as instructions
assign instr_issued = if_valid_o && id_ready_i && first_op && !ptr_in_if_o;

cv32e40s_dummy_instr
dummy_instr_i
(.clk ( clk ),
.rst_n ( rst_n ),
.instr_issued_i ( instr_issued ),
.ctrl_fsm_i ( ctrl_fsm_i ),
.xsecure_ctrl_i ( xsecure_ctrl_i ),
.dummy_insert_o ( dummy_insert ),
.dummy_instr_o ( dummy_instr )
(.clk ( clk ),
.rst_n ( rst_n ),
.instr_issued_i ( instr_issued ),
.first_op_nondummy_i ( first_op_nondummy ),
.prefetch_valid_i ( prefetch_valid ),
.ptr_in_if_i ( ptr_in_if_o ),
.ctrl_fsm_i ( ctrl_fsm_i ),
.xsecure_ctrl_i ( xsecure_ctrl_i ),
.dummy_insert_o ( dummy_insert ),
.dummy_instr_o ( dummy_instr )
);

end : gen_dummy_instr
Expand Down
21 changes: 18 additions & 3 deletions sva/cv32e40s_dummy_instr_sva.sv
Expand Up @@ -40,22 +40,37 @@ module cv32e40s_dummy_instr_sva
input logic cnt_rst,
input logic if_valid_i,
input logic id_ready_i,
input logic hint_id_i
input logic hint_if_i,
input logic ptr_in_if_i
);

// Helper logic that remembers if the last instruction from IF to ID was a hint instruction.
// Hint instructions will shift LFSRs, changing the rules for allowed counter values.
// Excluding any pointers, as these are not counted as instructions.
logic last_ins_was_hint;
always_ff @(posedge clk, negedge rst_n) begin
if (rst_n == 1'b0) begin
last_ins_was_hint <= 1'b0;
end else begin
if (if_valid_i && id_ready_i && !ptr_in_if_i) begin
last_ins_was_hint <= hint_if_i;
end
end
end

// Assert that counter stopped correctly when inserting dummy instruction
// When no hint is in ID, the cnt_q should be exactly lfsr_cnt + 1
a_no_count_overflow_nohint :
assert property (@(posedge clk) disable iff (!rst_n)
dummy_insert_o && !hint_id_i |-> (cnt_q == lfsr_cnt + 1))
dummy_insert_o && !last_ins_was_hint |-> (cnt_q == lfsr_cnt + 1))
else `uvm_error("Dummy Instruction Insertion", "Counter counted further than lfsr_cnt + 1");

// When a dummy is inserted and at the same time a hint is in ID, the counter value
// may differ by more than one since the hint instruction did an lfsr shift, possibly changing the
// dummy insertion threshold.
a_no_count_overflow_hint :
assert property (@(posedge clk) disable iff (!rst_n)
dummy_insert_o && hint_id_i |-> (cnt_q >= (lfsr_cnt + 1)))
dummy_insert_o && last_ins_was_hint |-> (cnt_q >= (lfsr_cnt + 1)))
else `uvm_error("Dummy Instruction Insertion", "Counter counted further than lfsr_cnt + 1");

// Assert that we never count when counter has passed the compare value
Expand Down
4 changes: 2 additions & 2 deletions sva/cv32e40s_if_stage_sva.sv
Expand Up @@ -47,7 +47,7 @@ module cv32e40s_if_stage_sva
input logic illegal_c_insn,
input logic instr_compressed,
input logic prefetch_is_tbljmp_ptr,
input logic first_op_nondummy_o,
input logic first_op_nondummy,
input logic first_op,
input logic last_op_o,
input logic prefetch_is_clic_ptr,
Expand Down Expand Up @@ -128,7 +128,7 @@ module cv32e40s_if_stage_sva
// Assert that we do not trigger dummy instructions when the sequencer is in the middle of a sequence
a_no_dummy_mid_sequence :
assert property (@(posedge clk) disable iff (!rst_n)
!first_op_nondummy_o |-> !dummy_insert)
!first_op_nondummy |-> !dummy_insert)
else `uvm_error("if_stage", "Dummy instruction inserted mid-sequence")


Expand Down

0 comments on commit 634c384

Please sign in to comment.