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.

Sign up for GitHub

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

Misaligned pointer update. #812

Merged
merged 5 commits into from
Mar 27, 2023
Merged

Misaligned pointer update. #812

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b0bdad
Detecting misaligned pointers in the IF stage. Propagating alignment …
silabs-oysteink Mar 22, 2023
1902f4a
Clearing pointer flags from alignment buffer when pointers are consumed.
silabs-oysteink Mar 24, 2023
5b62839
Fixed MPU_OK -> ALIGN_OK where needed. Added alignment status check f…
silabs-oysteink Mar 24, 2023
5dfd05b
Avoiding setting instr_valid (alignment buffer) twice for pointers.
silabs-oysteink Mar 24, 2023
0f5c6a6
Fixed issue where the alignment buffer would not set instr_valid_o fo…
silabs-oysteink Mar 24, 2023
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
2 changes: 2 additions & 0 deletions bhv/cv32e40x_wrapper.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ endgenerate
.write_buffer_valid_o ('0),
.write_buffer_txn_bufferable ('0),
.write_buffer_txn_cacheable ('0),
.obi_if_state (core_i.if_stage_i.instruction_obi_i.state_q),
.*);

bind cv32e40x_mpu:
Expand All @@ -444,6 +445,7 @@ endgenerate
.write_buffer_valid_o (core_i.load_store_unit_i.write_buffer_i.valid_o),
.write_buffer_txn_bufferable (core_i.load_store_unit_i.write_buffer_i.trans_o.memtype[0]),
.write_buffer_txn_cacheable (core_i.load_store_unit_i.write_buffer_i.trans_o.memtype[1]),
.obi_if_state (cv32e40x_pkg::TRANSPARENT),
.*);

bind cv32e40x_lsu_response_filter :
Expand Down
3 changes: 0 additions & 3 deletions rtl/cv32e40x_align_check.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ module cv32e40x_align_check import cv32e40x_pkg::*;
// Interface towards bus interface
input logic bus_trans_ready_i,
output logic bus_trans_valid_o,
output logic bus_trans_pushpop_o,
output CORE_REQ_TYPE bus_trans_o,

input logic bus_resp_valid_i,
Expand All @@ -50,7 +49,6 @@ module cv32e40x_align_check import cv32e40x_pkg::*;
// Interface towards core (MPU)
input logic core_trans_valid_i,
output logic core_trans_ready_o,
input logic core_trans_pushpop_i,
input CORE_REQ_TYPE core_trans_i,

output logic core_resp_valid_o,
Expand Down Expand Up @@ -156,7 +154,6 @@ module cv32e40x_align_check import cv32e40x_pkg::*;
// Forward transaction request towards MPU
assign bus_trans_valid_o = core_trans_valid_i && !align_block_bus;
assign bus_trans_o = core_trans_i;
assign bus_trans_pushpop_o = core_trans_pushpop_i;


// Forward transaction response towards core
Expand Down
37 changes: 27 additions & 10 deletions rtl/cv32e40x_alignment_buffer.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
output logic [31:0] instr_addr_o,
output privlvl_t instr_priv_lvl_o,
output logic instr_is_clic_ptr_o, // True CLIC pointer after taking a CLIC SHV interrupt
output logic instr_is_mret_ptr_o, // CLIC pointer due to restarting pionter fetch during mret
output logic instr_is_mret_ptr_o, // CLIC pointer due to restarting pointer fetch during mret
output logic instr_is_tbljmp_ptr_o,
output logic [ALBUF_CNT_WIDTH-1:0] outstnd_cnt_q_o
);
Expand Down Expand Up @@ -97,9 +97,10 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
// Store number of responses to flush when get get a branch
logic [1:0] n_flush_n, n_flush_q, n_flush_branch;

// Error propagation signals for bus and mpu
// Error propagation signals for bus, mpu and alignment checker (pointers)
logic bus_err_unaligned, bus_err;
mpu_status_e mpu_status_unaligned, mpu_status;
align_status_e align_status_unaligned, align_status;

// resp_valid gated while flushing
logic resp_valid_gated;
Expand Down Expand Up @@ -163,9 +164,11 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;

// Aligned instructions will either be fully in index 0 or incoming data
// This also applies for the bus_error and mpu_status
assign instr = (valid_q[rptr]) ? resp_q[rptr].bus_resp.rdata : resp_i.bus_resp.rdata;
assign bus_err = (valid_q[rptr]) ? resp_q[rptr].bus_resp.err : resp_i.bus_resp.err;
assign mpu_status = (valid_q[rptr]) ? resp_q[rptr].mpu_status : resp_i.mpu_status;
assign instr = (valid_q[rptr]) ? resp_q[rptr].bus_resp.rdata : resp_i.bus_resp.rdata;
assign bus_err = (valid_q[rptr]) ? resp_q[rptr].bus_resp.err : resp_i.bus_resp.err;
assign mpu_status = (valid_q[rptr]) ? resp_q[rptr].mpu_status : resp_i.mpu_status;
assign align_status = (valid_q[rptr]) ? resp_q[rptr].align_status : resp_i.align_status;


// Unaligned instructions will either be split across index 0 and 1, or index 0 and incoming data
assign instr_unaligned = (valid_q[rptr2]) ? {resp_q[rptr2].bus_resp.rdata[15:0], instr[31:16]} : {resp_i.bus_resp.rdata[15:0], instr[31:16]};
Expand All @@ -182,9 +185,10 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
assign aligned_is_compressed = instr[1:0] != 2'b11;


// Set mpu_status and bus error for unaligned instructions
// Set mpu_status, align_status and bus error for unaligned instructions
always_comb begin
mpu_status_unaligned = MPU_OK;
align_status_unaligned = ALIGN_OK;
bus_err_unaligned = 1'b0;
// There is valid data in q1 (valid q0 is implied)
if(valid_q[rptr2]) begin
Expand All @@ -195,12 +199,18 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
mpu_status_unaligned = MPU_RE_FAULT;
end

if((resp_q[rptr2].align_status != MPU_OK) || (resp_q[rptr].align_status != MPU_OK)) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

MPU_OK -> ALIGN_OK

align_status_unaligned = ALIGN_RE_ERR;
end

// Bus error from either entry
bus_err_unaligned = (resp_q[rptr2].bus_resp.err || resp_q[rptr].bus_resp.err);
end else begin
// Compressed, use only mpu_status from q0
mpu_status_unaligned = resp_q[rptr].mpu_status;

align_status_unaligned = resp_q[rptr].align_status;

// bus error from q0
bus_err_unaligned = resp_q[rptr].bus_resp.err;
end
Expand All @@ -214,19 +224,24 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
mpu_status_unaligned = MPU_RE_FAULT;
end

if((resp_q[rptr].align_status != MPU_OK) || (resp_i.align_status != MPU_OK)) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

MPU_OK -> ALIGN_OK

align_status_unaligned = ALIGN_RE_ERR;
end

// Bus error from q0 and resp_i
bus_err_unaligned = (resp_q[rptr].bus_resp.err || resp_i.bus_resp.err);
end else begin
// There is unaligned data in q0 and it is compressed
mpu_status_unaligned = resp_q[rptr].mpu_status;

align_status_unaligned = resp_q[rptr].align_status;
// Bus error from q0
bus_err_unaligned = resp_q[rptr].bus_resp.err;
end
end else begin
// There is no data in the buffer, use input
mpu_status_unaligned = resp_i.mpu_status;
bus_err_unaligned = resp_i.bus_resp.err;
mpu_status_unaligned = resp_i.mpu_status;
bus_err_unaligned = resp_i.bus_resp.err;
align_status_unaligned = resp_i.align_status;
end
end
end
Expand All @@ -238,6 +253,7 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
instr_instr_o.bus_resp.rdata = instr;
instr_instr_o.bus_resp.err = bus_err;
instr_instr_o.mpu_status = mpu_status;
instr_instr_o.align_status = align_status;
instr_valid_o = 1'b0;

// Invalidate output if we get killed
Expand All @@ -248,14 +264,15 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
instr_instr_o.bus_resp.rdata = instr_unaligned;
instr_instr_o.bus_resp.err = bus_err_unaligned;
instr_instr_o.mpu_status = mpu_status_unaligned;
instr_instr_o.align_status = align_status_unaligned;
// No instruction valid
if (!valid) begin
instr_valid_o = 1'b0;
// Unaligned instruction is compressed, we only need 16 upper bits from index 0
end else if (unaligned_is_compressed) begin
instr_valid_o = valid;
end else begin
// Unaligned is not compressed, we need data form either index 0 and 1, or 0 and input
// Unaligned is not compressed, we need data from either index 0 and 1, or 0 and input
instr_valid_o = valid_unaligned_uncompressed;
end
end else begin
Expand Down
2 changes: 1 addition & 1 deletion rtl/cv32e40x_controller_bypass.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ module cv32e40x_controller_bypass import cv32e40x_pkg::*;
// Also deassert for trigger match, as with dcsr.timing==0 we do not execute before entering debug mode
// CLIC pointer fetches go through the pipeline, but no write enables should be active.
if (if_id_pipe_i.instr.bus_resp.err || !(if_id_pipe_i.instr.mpu_status == MPU_OK) || if_id_pipe_i.trigger_match ||
if_id_pipe_i.instr_meta.clic_ptr || if_id_pipe_i.instr_meta.mret_ptr) begin
if_id_pipe_i.instr_meta.clic_ptr || if_id_pipe_i.instr_meta.mret_ptr || !(if_id_pipe_i.instr.align_status == ALIGN_OK)) begin
ctrl_byp_o.deassert_we = 1'b1;
end

Expand Down
4 changes: 3 additions & 1 deletion rtl/cv32e40x_controller_fsm.sv
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

align_status should be checked wherever mpu_status is checked. Seems to be missing on line 1028:
if (!(if_id_pipe_i.instr.bus_resp.err || (if_id_pipe_i.instr.mpu_status != MPU_OK))) begin // todo: add check for alignment check in IF (mret pointer)

Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// M | 1 | 1 | Debug entry (restart from dm_halt_addr_i)
//
assign exception_in_wb = ((ex_wb_pipe_i.instr.mpu_status != MPU_OK) ||
(ex_wb_pipe_i.instr.align_status != ALIGN_OK) ||
ex_wb_pipe_i.instr.bus_resp.err ||
ex_wb_pipe_i.illegal_insn ||
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ||
Expand All @@ -349,7 +350,8 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
assign ctrl_fsm_o.exception_in_wb = exception_in_wb;

// Set exception cause
assign exception_cause_wb = (ex_wb_pipe_i.instr.mpu_status != MPU_OK) ? EXC_CAUSE_INSTR_FAULT : // todo: add code from align_check in IF
assign exception_cause_wb = (ex_wb_pipe_i.instr.mpu_status != MPU_OK) ? EXC_CAUSE_INSTR_FAULT :
(ex_wb_pipe_i.instr.align_status != ALIGN_OK) ? EXC_CAUSE_INSTR_MISALIGNED :
ex_wb_pipe_i.instr.bus_resp.err ? EXC_CAUSE_INSTR_BUS_FAULT :
ex_wb_pipe_i.illegal_insn ? EXC_CAUSE_ILLEGAL_INSN :
(ex_wb_pipe_i.sys_en && ex_wb_pipe_i.sys_ecall_insn) ? EXC_CAUSE_ECALL_MMODE :
Expand Down
9 changes: 5 additions & 4 deletions rtl/cv32e40x_ex_stage.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,11 @@ module cv32e40x_ex_stage import cv32e40x_pkg::*;

// Exception happened during IF or ID, or trigger match in ID (converted to NOP).
// signal needed for ex_valid to go high in such cases
assign previous_exception = (id_ex_pipe_i.illegal_insn ||
id_ex_pipe_i.instr.bus_resp.err ||
(id_ex_pipe_i.instr.mpu_status != MPU_OK) ||
id_ex_pipe_i.trigger_match) &&
assign previous_exception = (id_ex_pipe_i.illegal_insn ||
id_ex_pipe_i.instr.bus_resp.err ||
(id_ex_pipe_i.instr.mpu_status != MPU_OK) ||
(id_ex_pipe_i.instr.align_status != ALIGN_OK) ||
id_ex_pipe_i.trigger_match) &&
id_ex_pipe_i.instr_valid;

// ALU write port mux
Expand Down
1 change: 1 addition & 0 deletions rtl/cv32e40x_id_stage.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,7 @@ module cv32e40x_id_stage import cv32e40x_pkg::*;
id_ex_pipe_o.instr.bus_resp.rdata <= {16'h0, if_id_pipe_i.compressed_instr};
id_ex_pipe_o.instr.bus_resp.err <= if_id_pipe_i.instr.bus_resp.err;
id_ex_pipe_o.instr.mpu_status <= if_id_pipe_i.instr.mpu_status;
id_ex_pipe_o.instr.align_status <= if_id_pipe_i.instr.align_status;
end else begin
id_ex_pipe_o.instr <= if_id_pipe_i.instr;
end
Expand Down
18 changes: 14 additions & 4 deletions rtl/cv32e40x_if_stage.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
logic prefetch_trans_valid;
logic prefetch_trans_ready;
logic [31:0] prefetch_trans_addr;
logic prefetch_trans_ptr;
inst_resp_t prefetch_inst_resp;
logic prefetch_one_txn_pend_n;
logic [ALBUF_CNT_WIDTH-1:0] prefetch_outstnd_cnt_q;
Expand All @@ -134,6 +135,9 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
logic alcheck_trans_ready;
obi_inst_req_t alcheck_trans;

logic align_check_en;
logic address_misaligned;

// Local instr_valid
logic instr_valid;

Expand Down Expand Up @@ -214,6 +218,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
.trans_valid_o ( prefetch_trans_valid ),
.trans_ready_i ( prefetch_trans_ready ),
.trans_addr_o ( prefetch_trans_addr ),
.trans_ptr_o ( prefetch_trans_ptr ),

.resp_valid_i ( prefetch_resp_valid ),
.resp_i ( prefetch_inst_resp ),
Expand Down Expand Up @@ -274,6 +279,9 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
);


assign align_check_en = prefetch_trans_ptr;
assign address_misaligned = |prefetch_trans_addr[1:0];

cv32e40x_align_check
#(
.IF_STAGE ( 1 ),
Expand All @@ -285,8 +293,8 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
(
.clk ( clk ),
.rst_n ( rst_n ),
.align_check_en_i ( 1'b0 ), // todo: enable check for pointers
.misaligned_access_i ( 1'b0 ),
.align_check_en_i ( align_check_en ),
.misaligned_access_i ( address_misaligned ),

.core_one_txn_pend_n ( prefetch_one_txn_pend_n ),
.core_align_err_wait_i( 1'b1 ),
Expand Down Expand Up @@ -372,7 +380,8 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;


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

// Signal current privilege level of IF
assign priv_lvl_if_o = prefetch_priv_lvl;
Expand Down Expand Up @@ -437,7 +446,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
// For mret pointers, the pointer address is only needed downstream if the pointer fetch fails.
// If the pointer fetch is successful, the address of the mret (i.e. the previous PC) is needed.
if(prefetch_is_mret_ptr ?
(instr_decompressed.bus_resp.err || (instr_decompressed.mpu_status != MPU_OK)) :
(instr_decompressed.bus_resp.err || (instr_decompressed.mpu_status != MPU_OK) || (instr_decompressed.align_status != ALIGN_OK)) :
1'b1) begin
if_id_pipe_o.pc <= pc_if_o;
end
Expand All @@ -461,6 +470,7 @@ module cv32e40x_if_stage import cv32e40x_pkg::*;
// Need to update bus error status and mpu status, but may omit the 32-bit instruction word
if_id_pipe_o.instr.bus_resp.err <= instr_decompressed.bus_resp.err;
if_id_pipe_o.instr.mpu_status <= instr_decompressed.mpu_status;
if_id_pipe_o.instr.align_status <= instr_decompressed.align_status;
end else begin
// Regular instruction, update the whole instr field
if_id_pipe_o.instr <= seq_valid ? seq_instr : instr_decompressed;
Expand Down
4 changes: 3 additions & 1 deletion rtl/cv32e40x_prefetch_unit.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ module cv32e40x_prefetch_unit import cv32e40x_pkg::*;
output logic trans_valid_o,
input logic trans_ready_i,
output logic [31:0] trans_addr_o,
output logic trans_ptr_o,

input logic resp_valid_i,
input inst_resp_t resp_i,
Expand Down Expand Up @@ -99,7 +100,8 @@ module cv32e40x_prefetch_unit import cv32e40x_pkg::*;
.fetch_priv_lvl_access_o ( fetch_priv_lvl_resp ),
.trans_valid_o ( trans_valid_o ),
.trans_ready_i ( trans_ready_i ),
.trans_addr_o ( trans_addr_o )
.trans_addr_o ( trans_addr_o ),
.trans_ptr_o ( trans_ptr_o )
);


Expand Down
7 changes: 4 additions & 3 deletions rtl/cv32e40x_prefetcher.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ module cv32e40x_prefetcher import cv32e40x_pkg::*;
// Transaction request interface
output logic trans_valid_o, // Transaction request valid (to bus interface adapter)
input logic trans_ready_i, // Transaction request ready (transaction gets accepted when trans_valid_o and trans_ready_i are both 1)
output logic [31:0] trans_addr_o // Transaction address (only valid when trans_valid_o = 1). No stability requirements.


output logic [31:0] trans_addr_o, // Transaction address (only valid when trans_valid_o = 1). No stability requirements.
output logic trans_ptr_o // Transaction is fetching a pointer
);


Expand All @@ -80,6 +79,8 @@ module cv32e40x_prefetcher import cv32e40x_pkg::*;
// and will always be ready to accept responses.
assign trans_valid_o = fetch_valid_i;

assign trans_ptr_o = fetch_ptr_access_o;

assign fetch_ready_o = trans_valid_o && trans_ready_i;

// FSM (state_q, next_state) to control trans_addr_o
Expand Down
Loading