[I-side] - Fix issues found in tracing example

- Fixes #288
- Add missing grant qualification to stop incorrect address updates
- Make RTL robust to spurious rvalid signalling
- Make sure a request is held until granted
- Remove incorrect assertion
This commit is contained in:
Tom Roberts 2019-09-09 10:16:21 +01:00 committed by Tom Roberts
parent 6b03bc6326
commit f025236a22
3 changed files with 18 additions and 16 deletions

View file

@ -76,7 +76,7 @@ module ibex_fetch_fifo (
// Alignment is tracked with a flag, this records whether entry[0] of the FIFO has become unaligned. // Alignment is tracked with a flag, this records whether entry[0] of the FIFO has become unaligned.
// The flag is set once any compressed instruction enters the FIFO and is only cleared once a // The flag is set once any compressed instruction enters the FIFO and is only cleared once a
// a compressed instruction realigns the FIFO, or the FIFO is cleared. // a compressed instruction realigns the FIFO, or the FIFO is cleared.
// New incoming unaligned request (must be a branch) or already unaligned // New incoming unaligned request (must be a branch) or already unaligned
assign entry0_unaligned_d = ((((in_valid_i & in_addr_i[1]) | entry0_unaligned_q) & assign entry0_unaligned_d = ((((in_valid_i & in_addr_i[1]) | entry0_unaligned_q) &
// cleared by a compressed unaligned instruction // cleared by a compressed unaligned instruction

View file

@ -261,11 +261,6 @@ module ibex_if_stage #(
@(posedge clk_i) (boot_addr_i[7:0] == 8'h00) ) else @(posedge clk_i) (boot_addr_i[7:0] == 8'h00) ) else
$error("The provided boot address is not aligned to 256 bytes"); $error("The provided boot address is not aligned to 256 bytes");
// there should never be a grant when there is no request
assert property (
@(posedge clk_i) (instr_gnt_i) |-> (instr_req_o) ) else
$warning("There was a grant without a request");
// assert that the address is word aligned when request is sent // assert that the address is word aligned when request is sent
assert property ( assert property (
@(posedge clk_i) (instr_req_o) |-> (instr_addr_o[1:0] == 2'b00) ) else @(posedge clk_i) (instr_req_o) |-> (instr_addr_o[1:0] == 2'b00) ) else

View file

@ -44,6 +44,7 @@ module ibex_prefetch_buffer (
logic valid_req; logic valid_req;
logic valid_req_d, valid_req_q; logic valid_req_d, valid_req_q;
logic hold_addr_d, hold_addr_q;
logic gnt_or_pmp_err, rvalid_or_pmp_err; logic gnt_or_pmp_err, rvalid_or_pmp_err;
logic [NUM_REQS-1:0] rdata_outstanding_n, rdata_outstanding_s, rdata_outstanding_q; logic [NUM_REQS-1:0] rdata_outstanding_n, rdata_outstanding_s, rdata_outstanding_q;
logic [NUM_REQS-1:0] branch_abort_n, branch_abort_s, branch_abort_q; logic [NUM_REQS-1:0] branch_abort_n, branch_abort_s, branch_abort_q;
@ -100,8 +101,7 @@ module ibex_prefetch_buffer (
////////////// //////////////
// Make a new request any time there is space in the FIFO, and space in the request queue // Make a new request any time there is space in the FIFO, and space in the request queue
assign valid_req = req_i & (fifo_ready | branch_i) & assign valid_req = valid_req_q | (req_i & (fifo_ready | branch_i) & (~&rdata_outstanding_q));
~&rdata_outstanding_q;
// If a request address triggers a PMP error, the external bus request is suppressed. We might // If a request address triggers a PMP error, the external bus request is suppressed. We might
// therefore never receive a grant for such a request. The grant is faked in this case to make // therefore never receive a grant for such a request. The grant is faked in this case to make
@ -115,16 +115,21 @@ module ibex_prefetch_buffer (
// As with the grant, the rvalid must be faked for a PMP error, since the request was suppressed. // As with the grant, the rvalid must be faked for a PMP error, since the request was suppressed.
// Since the pmp_err_q flop is only updated when the address updates, it will always point to the // Since the pmp_err_q flop is only updated when the address updates, it will always point to the
// PMP error status of the oldest outstanding request // PMP error status of the oldest outstanding request
assign rvalid_or_pmp_err = instr_rvalid_i | pmp_err_q; assign rvalid_or_pmp_err = rdata_outstanding_q[0] & (instr_rvalid_i | pmp_err_q);
// Hold the address stable for requests that couldn't be issued, or didn't get granted // Hold the request stable for requests that didn't get granted
assign valid_req_d = (branch_i | valid_req_q) & ~(valid_req & instr_gnt_i); assign valid_req_d = valid_req & ~instr_gnt_i;
// Hold the address stable for requests that couldn't be issued, or didn't get granted.
// This is different to valid_req_q since there are cases where we must use addr+4 for
// an ungranted request rather than addr_q (where addr_q has not been updated).
assign hold_addr_d = (branch_i | hold_addr_q) & ~(valid_req & instr_gnt_i);
//////////////// ////////////////
// Fetch addr // // Fetch addr //
//////////////// ////////////////
// The address flop is used to hold the address steady for ungranted requests and also to // The address flop is used to hold the address steady for ungranted requests and also to
// push the address to the FIFO for completed requests. For this reason, the address is only // push the address to the FIFO for completed requests. For this reason, the address is only
// updated once a request is the oldest outstanding to ensure that newer requests do not // updated once a request is the oldest outstanding to ensure that newer requests do not
// overwrite the addresses of older ones. Branches are an exception to this, since all older // overwrite the addresses of older ones. Branches are an exception to this, since all older
@ -133,7 +138,7 @@ module ibex_prefetch_buffer (
// Update the addr_q flop on any branch, or // Update the addr_q flop on any branch, or
assign addr_valid = branch_i | assign addr_valid = branch_i |
// A new request which will be the oldest, or // A new request which will be the oldest, or
(req_i & fifo_ready & ~rdata_outstanding_q[0]) | (valid_req & instr_gnt_i & ~rdata_outstanding_q[0]) |
// each time a valid request becomes the oldest // each time a valid request becomes the oldest
(rvalid_or_pmp_err & ~branch_abort_q[0] & (rvalid_or_pmp_err & ~branch_abort_q[0] &
((valid_req & instr_gnt_i) | rdata_outstanding_q[1])); ((valid_req & instr_gnt_i) | rdata_outstanding_q[1]));
@ -143,7 +148,7 @@ module ibex_prefetch_buffer (
// Address mux // Address mux
assign instr_addr = branch_i ? addr_i : assign instr_addr = branch_i ? addr_i :
valid_req_q ? instr_addr_q : hold_addr_q ? instr_addr_q :
fetch_addr; fetch_addr;
assign instr_addr_w_aligned = {instr_addr[31:2], 2'b00}; assign instr_addr_w_aligned = {instr_addr[31:2], 2'b00};
@ -180,7 +185,7 @@ module ibex_prefetch_buffer (
branch_abort_n; branch_abort_n;
// Push a new entry to the FIFO once complete (and not aborted by a branch) // Push a new entry to the FIFO once complete (and not aborted by a branch)
assign fifo_valid = rdata_outstanding_q[0] & ~branch_abort_q[0] & rvalid_or_pmp_err; assign fifo_valid = rvalid_or_pmp_err & ~branch_abort_q[0];
/////////////// ///////////////
// Registers // // Registers //
@ -189,10 +194,12 @@ module ibex_prefetch_buffer (
always_ff @(posedge clk_i or negedge rst_ni) begin always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin if (!rst_ni) begin
valid_req_q <= 1'b0; valid_req_q <= 1'b0;
hold_addr_q <= 1'b0;
rdata_outstanding_q <= 'b0; rdata_outstanding_q <= 'b0;
branch_abort_q <= 'b0; branch_abort_q <= 'b0;
end else begin end else begin
valid_req_q <= valid_req_d; valid_req_q <= valid_req_d;
hold_addr_q <= hold_addr_d;
rdata_outstanding_q <= rdata_outstanding_s; rdata_outstanding_q <= rdata_outstanding_s;
branch_abort_q <= branch_abort_s; branch_abort_q <= branch_abort_s;
end end
@ -211,7 +218,7 @@ module ibex_prefetch_buffer (
///////////// /////////////
assign instr_req_o = valid_req; assign instr_req_o = valid_req;
assign instr_addr_o = instr_addr_w_aligned; assign instr_addr_o = instr_addr_w_aligned;
//////////////// ////////////////
// Assertions // // Assertions //