l2,axi_to_arb: Move be (wstrb) to data FIFO

Fix: If the AXI bus has WREADY=0 and AWREADY=1,
  the AW handshake (single beat) that CVA5 starts
  concurrently with the corresponding W handshake
  will complete first.
 CVA5 stores `be` (-> WSTRB) in the FIFO for AW requests,
  even though WSTRB is part of the W handshake.
 The result is that WSTRB becomes unstable once the AW request finishes.
-> The fix moves `be` to the write data FIFO.

Signed-off-by: Florian Meisel <meisel@esa.tu-darmstadt.de>
This commit is contained in:
Florian Meisel 2022-09-05 10:59:59 +02:00
parent 3239e20360
commit 6b1d2280d8
5 changed files with 43 additions and 37 deletions

View file

@ -182,7 +182,7 @@ module axi_to_arb
assign axi_wdata = read_modify_write ? amo_result_r : l2.wr_data;
assign axi_wstrb =read_modify_write ? '1 : l2.be;
assign axi_wstrb = read_modify_write ? '1 : l2.wr_data_be;
//Done when read request sent, or slave ack on write data

View file

@ -78,6 +78,7 @@ module l1_arbiter
//Dcache Specific
assign l2.wr_data_push = CONFIG.INCLUDE_DCACHE & (push_ready & l1_request[L1_DCACHE_ID].request & ~l1_request[L1_DCACHE_ID].rnw); //Assumes data cache has highest priority
assign l2.wr_data = l1_request[L1_DCACHE_ID].data;
assign l2.wr_data_be = l1_request[L1_DCACHE_ID].be;
assign l2.inv_ack = CONFIG.DCACHE.USE_EXTERNAL_INVALIDATIONS ? l1_response[L1_DCACHE_ID].inv_ack : l2.inv_valid;
assign l1_response[L1_DCACHE_ID].inv_addr = l2.inv_addr;
@ -89,7 +90,6 @@ module l1_arbiter
always_comb begin
l2_requests[i].addr = l1_request[i].addr[31:2];
l2_requests[i].rnw = l1_request[i].rnw;
l2_requests[i].be = l1_request[i].be;
l2_requests[i].is_amo = l1_request[i].is_amo;
l2_requests[i].amo_type_or_burst_size = l1_request[i].size;
l2_requests[i].sub_id = L2_SUB_ID_W'(i);
@ -113,7 +113,6 @@ module l1_arbiter
assign l2.addr = l2_requests[arb_sel].addr;
assign l2.rnw = l2_requests[arb_sel].rnw;
assign l2.be = l2_requests[arb_sel].be;
assign l2.is_amo = l2_requests[arb_sel].is_amo;
assign l2.amo_type_or_burst_size = l2_requests[arb_sel].amo_type_or_burst_size;
assign l2.sub_id = l2_requests[arb_sel].sub_id;

View file

@ -38,19 +38,20 @@ module l2_arbiter
//FIFO interfaces
fifo_interface #(.DATA_WIDTH($bits(l2_request_t))) input_fifos [L2_NUM_PORTS-1:0]();
fifo_interface #(.DATA_WIDTH(32)) input_data_fifos [L2_NUM_PORTS-1:0]();
fifo_interface #(.DATA_WIDTH($bits(l2_data_request_t))) input_data_fifos [L2_NUM_PORTS-1:0]();
fifo_interface #(.DATA_WIDTH(30)) inv_response_fifos [L2_NUM_PORTS-1:0]();
fifo_interface #(.DATA_WIDTH(32 + L2_SUB_ID_W)) returndata_fifos [L2_NUM_PORTS-1:0]();
fifo_interface #(.DATA_WIDTH($bits(l2_mem_request_t))) mem_addr_fifo();
fifo_interface #(.DATA_WIDTH(32)) mem_data_fifo();
fifo_interface #(.DATA_WIDTH($bits(l2_data_request_t))) mem_data_fifo();
fifo_interface #(.DATA_WIDTH($bits(l2_data_attributes_t))) data_attributes();
fifo_interface #(.DATA_WIDTH(32 + L2_ID_W)) mem_returndata_fifo();
l2_mem_request_t mem_addr_fifo_data_out;
l2_request_t requests_in[L2_NUM_PORTS-1:0];
l2_data_request_t data_requests_in[L2_NUM_PORTS-1:0];
logic advance;
l2_request_t arb_request;
@ -70,7 +71,7 @@ module l2_arbiter
l2_data_attributes_t new_attr;
l2_data_attributes_t current_attr;
logic [31:0] input_data [L2_NUM_PORTS-1:0];
l2_data_request_t input_data [L2_NUM_PORTS-1:0];
l2_mem_return_data_t mem_return_data;
l2_return_data_t return_data [L2_NUM_PORTS-1:0];
@ -93,11 +94,10 @@ module l2_arbiter
//Repack input attributes
assign requests_in[i].addr = request[i].addr;
assign requests_in[i].be = request[i].be;
assign requests_in[i].rnw = request[i].rnw;
assign requests_in[i].is_amo = request[i].is_amo;
assign requests_in[i].amo_type_or_burst_size = request[i].amo_type_or_burst_size;
assign requests_in[i].sub_id = request[i].sub_id;
assign requests_in[i].rnw = request[i].rnw;
assign requests_in[i].is_amo = request[i].is_amo;
assign requests_in[i].amo_type_or_burst_size = request[i].amo_type_or_burst_size;
assign requests_in[i].sub_id = request[i].sub_id;
assign input_fifos[i].data_in = requests_in[i];
assign input_fifos[i].pop = input_fifos[i].valid & arb.grantee_v[i] & ~mem_addr_fifo.full;
@ -121,12 +121,14 @@ module l2_arbiter
assign input_data_fifos[i].push = request[i].wr_data_push;
assign input_data_fifos[i].potential_push = request[i].wr_data_push;
assign input_data_fifos[i].data_in = request[i].wr_data;
assign data_requests_in[i].data = request[i].wr_data;
assign data_requests_in[i].be = request[i].wr_data_be;
assign input_data_fifos[i].data_in = data_requests_in[i];
assign request[i].data_full = input_data_fifos[i].full;
//FIFO instantiation
cva5_fifo #(.DATA_WIDTH(32), .FIFO_DEPTH(L2_INPUT_FIFO_DEPTHS)) input_data_fifo (.*, .fifo(input_data_fifos[i]));
cva5_fifo #(.DATA_WIDTH($bits(l2_data_request_t)), .FIFO_DEPTH(L2_INPUT_FIFO_DEPTHS)) input_data_fifo (.*, .fifo(input_data_fifos[i]));
//Arbiter FIFO side
assign input_data_fifos[i].pop = (data_attributes.valid && (current_attr.id == i) && ~mem_data_fifo.full);
@ -152,7 +154,6 @@ module l2_arbiter
assign arb_request = requests[arb.grantee_i];
assign mem_request.addr = arb_request.addr;
assign mem_request.be = arb_request.be;
assign mem_request.rnw = arb_request.rnw;
assign mem_request.is_amo = arb_request.is_amo;
assign mem_request.amo_type_or_burst_size = arb_request.amo_type_or_burst_size;
@ -162,11 +163,10 @@ module l2_arbiter
//unpack memory request attributes
assign mem_addr_fifo_data_out = mem_addr_fifo.data_out;
assign mem.addr = mem_addr_fifo_data_out.addr;
assign mem.rnw = mem_addr_fifo_data_out.rnw;
assign mem.be = mem_addr_fifo_data_out.be;
assign mem.is_amo = mem_addr_fifo_data_out.is_amo;
assign mem.amo_type_or_burst_size = mem_addr_fifo_data_out.amo_type_or_burst_size;
assign mem.addr = mem_addr_fifo_data_out.addr;
assign mem.rnw = mem_addr_fifo_data_out.rnw;
assign mem.is_amo = mem_addr_fifo_data_out.is_amo;
assign mem.amo_type_or_burst_size = mem_addr_fifo_data_out.amo_type_or_burst_size;
assign mem.id = mem_addr_fifo_data_out.id;
cva5_fifo #(.DATA_WIDTH($bits(l2_mem_request_t)), .FIFO_DEPTH(L2_MEM_ADDR_FIFO_DEPTH)) input_fifo (.*, .fifo(mem_addr_fifo));
@ -261,14 +261,17 @@ module l2_arbiter
assign write_done = data_attributes.valid & ~mem_data_fifo.full & (burst_count == current_attr.burst_size);
cva5_fifo #(.DATA_WIDTH($bits(32)), .FIFO_DEPTH(L2_MEM_ADDR_FIFO_DEPTH)) mem_data (.*, .fifo(mem_data_fifo));
cva5_fifo #(.DATA_WIDTH($bits($bits(l2_data_request_t))), .FIFO_DEPTH(L2_MEM_ADDR_FIFO_DEPTH)) mem_data (.*, .fifo(mem_data_fifo));
assign mem_data_fifo.push = data_attributes.valid & ~mem_data_fifo.full & ~current_attr.abort_request;
assign mem_data_fifo.potential_push = data_attributes.valid & ~mem_data_fifo.full & ~current_attr.abort_request;
assign mem_data_fifo.data_in = input_data[current_attr.id];
assign mem.wr_data = mem_data_fifo.data_out;
l2_data_request_t mem_data_request;
assign mem_data_request = mem_data_fifo.data_out;
assign mem.wr_data = mem_data_request.data;
assign mem.wr_data_be = mem_data_request.be;
assign mem.wr_data_valid = mem_data_fifo.valid;
assign mem_data_fifo.pop = mem.wr_data_read;
@ -306,4 +309,4 @@ module l2_arbiter
end
endgenerate
endmodule
endmodule

View file

@ -48,13 +48,17 @@ package l2_config_and_types;
typedef struct packed{
logic [29:0] addr;
logic [3:0] be;
logic rnw;
logic is_amo;
logic [4:0] amo_type_or_burst_size;
logic [L2_SUB_ID_W-1:0] sub_id;
} l2_request_t;
typedef struct packed{
logic [31:0] data;
logic [3:0] be;
} l2_data_request_t;
typedef struct packed{
logic [29:0] addr;
logic [3:0] be;

View file

@ -25,7 +25,6 @@ interface l2_requester_interface;
//l2_request_t request;
logic [29:0] addr;
logic [3:0] be;
logic rnw;
logic is_amo;
logic [4:0] amo_type_or_burst_size;
@ -42,6 +41,7 @@ interface l2_requester_interface;
logic con_valid;
logic [31:0] wr_data;
logic [3:0] wr_data_be;
logic wr_data_push;
logic data_full;
@ -50,26 +50,26 @@ interface l2_requester_interface;
logic rd_data_valid;
logic rd_data_ack;
modport master (output addr, be, rnw, is_amo, amo_type_or_burst_size, sub_id,
modport master (output addr, rnw, is_amo, amo_type_or_burst_size, sub_id,
output request_push, input request_full,
input inv_addr, inv_valid, output inv_ack,
input con_result, con_valid,
output wr_data, wr_data_push, input data_full,
output wr_data, wr_data_be, wr_data_push, input data_full,
input rd_data, rd_sub_id, rd_data_valid, output rd_data_ack);
modport slave (input addr, be, rnw, is_amo, amo_type_or_burst_size, sub_id,
modport slave (input addr, rnw, is_amo, amo_type_or_burst_size, sub_id,
input request_push, output request_full,
output inv_addr, inv_valid, input inv_ack,
output con_result, con_valid,
input wr_data, wr_data_push, output data_full,
input wr_data, wr_data_be, wr_data_push, output data_full,
output rd_data, rd_sub_id, rd_data_valid, input rd_data_ack);
`ifdef __CVA5_FORMAL__
modport formal (input addr, be, rnw, is_amo, amo_type_or_burst_size, sub_id,
modport formal (input addr, rnw, is_amo, amo_type_or_burst_size, sub_id,
request_push, output request_full,
inv_addr, inv_valid, input inv_ack,
con_result, con_valid,
wr_data, wr_data_push, output data_full,
wr_data, wr_data_be, wr_data_push, output data_full,
rd_data, rd_sub_id, rd_data_valid, input rd_data_ack);
`endif
@ -81,7 +81,6 @@ interface l2_memory_interface;
localparam L2_ID_W = $clog2(L2_NUM_PORTS) + L2_SUB_ID_W;
logic [29:0] addr;
logic [3:0] be;
logic rnw;
logic is_amo;
logic [4:0] amo_type_or_burst_size;
@ -93,6 +92,7 @@ interface l2_memory_interface;
logic abort_request;
logic [31:0] wr_data;
logic [3:0] wr_data_be;
logic wr_data_valid;
logic wr_data_read;
@ -100,20 +100,20 @@ interface l2_memory_interface;
logic [L2_ID_W-1:0] rd_id;
logic rd_data_valid;
modport master (output addr, be, rnw, is_amo, amo_type_or_burst_size, id,
modport master (output addr, rnw, is_amo, amo_type_or_burst_size, id,
output request_valid, abort_request, input request_pop,
output wr_data, wr_data_valid, input wr_data_read,
output wr_data, wr_data_be, wr_data_valid, input wr_data_read,
input rd_data, rd_id, rd_data_valid);
modport slave (input addr, be, rnw, is_amo, amo_type_or_burst_size, id,
modport slave (input addr, rnw, is_amo, amo_type_or_burst_size, id,
input request_valid, abort_request, output request_pop,
input wr_data, wr_data_valid, output wr_data_read,
input wr_data, wr_data_be, wr_data_valid, output wr_data_read,
output rd_data, rd_id, rd_data_valid);
`ifdef __CVA5_FORMAL__
modport formal (input addr, be, rnw, is_amo, amo_type_or_burst_size, id,
modport formal (input addr, rnw, is_amo, amo_type_or_burst_size, id,
request_valid, abort_request, output request_pop,
wr_data, wr_data_valid, output wr_data_read,
wr_data, wr_data_be, wr_data_valid, output wr_data_read,
rd_data, rd_id, rd_data_valid);
`endif