fix(axi4): correct WRAP burst handling for mid-line and page-edge fills

The OpenC906 is the first master to issue WRAP bursts (cache line fills
start at the critical word). Two latent path bugs made every such fill
fail or corrupt data:

- sdram_axi4 reconstructed the WRAP wrap-back address from the
  decrementing remaining-beat count, which cannot reproduce the wrap
  boundary; mid-line starts read the second half from wrong addresses.
  Beat stepping now uses the registered original geometry through the
  proven shared axi4_addr_gen, on both the read and write paths.
- axi4_data_crossbar burst_legal computed a WRAP burst's last address
  as if it were INCR, so fills starting in the last 64 bytes of a 4 KiB
  page were faulted as FaultProtocol and answered with DECERR. The WRAP
  last address is now computed from the wrap boundary.

Locked by new directed cases: mid-line WRAP read/write with
address-encoded data in tests/rtl/sdram_data_tb.sv and a page-edge WRAP
read routing case in tests/rtl/axi4_data_crossbar_tb.sv. Both fail on
the pre-fix RTL and pass after; full test_axi4/test_sdram (17/17),
hp-smoke-sim, hp-rtthread-sim and the PR ci_smoke all pass with the fix.
This commit is contained in:
maksyuki committed 2026-10-11 08:14:57 +08:00
1 parent ef8f7459a3
commit 489ff284e1
4 files changed
+220 -17

No files matched your search

+48 -16
View File
@@ -128,6 +128,11 @@ module sdram_axi4 (
logic s_rd_id_d;
logic [ 1:0] s_rd_resp_q;
logic [ 1:0] s_rd_resp_d;
// Original burst length, registered at accept: WRAP stepping needs the
// total geometry, which the decrementing remaining-beat count cannot
// reconstruct.
logic [ 7:0] s_rd_len_q;
logic [ 7:0] s_rd_len_d;
sdram_wr_state_e s_wr_state_q;
sdram_wr_state_e s_wr_state_d;
@@ -145,6 +150,8 @@ module sdram_axi4 (
logic s_wr_id_d;
logic [ 1:0] s_wr_resp_q;
logic [ 1:0] s_wr_resp_d;
logic [ 7:0] s_wr_len_q;
logic [ 7:0] s_wr_len_d;
logic s_rd_err_event_d;
logic s_wr_err_event_d;
@@ -307,6 +314,31 @@ module sdram_axi4 (
assign error_code_o = s_err_code_q;
assign error_addr_o = s_err_addr_q;
// Per-beat address stepping uses the registered original burst geometry so
// WRAP boundaries survive mid-burst: the previous remaining-beats-based
// mask arithmetic produced wrong wrap-back addresses for mid-line starts.
logic [31:0] s_rd_step_addr;
logic [31:0] s_wr_step_addr;
axi4_addr_gen #(
.ADDR_WIDTH(32)
) u_rd_addr_gen (
.alen_i (s_rd_len_q),
.asize_i (s_rd_size_q),
.aburst_i(s_rd_burst_q),
.addr_i (s_rd_addr_q),
.addr_o (s_rd_step_addr)
);
axi4_addr_gen #(
.ADDR_WIDTH(32)
) u_wr_addr_gen (
.alen_i (s_wr_len_q),
.asize_i (s_wr_size_q),
.aburst_i(s_wr_burst_q),
.addr_i (s_wr_addr_q),
.addr_o (s_wr_step_addr)
);
fifo #(
.DATA_WIDTH (CmdWidth),
.BUFFER_DEPTH(2)
@@ -396,6 +428,7 @@ module sdram_axi4 (
s_rd_frag_d = s_rd_frag_q;
s_rd_size_d = s_rd_size_q;
s_rd_burst_d = s_rd_burst_q;
s_rd_len_d = s_rd_len_q;
s_rd_id_d = s_rd_id_q;
s_rd_resp_d = s_rd_resp_q;
s_ar_pop = 1'b0;
@@ -436,6 +469,7 @@ module sdram_axi4 (
s_rd_left_d = s_ar_len + 8'd1;
s_rd_size_d = s_ar_size;
s_rd_burst_d = s_ar_burst;
s_rd_len_d = s_ar_len;
s_rd_id_d = s_ar_id;
rd_frag = fragment_beats(s_ar_addr, s_ar_len + 8'd1, s_ar_len, s_ar_size, s_ar_burst);
s_rd_frag_d = rd_frag;
@@ -461,19 +495,11 @@ module sdram_axi4 (
end
s_r_push = 1'b1;
s_r_push_data = {rd_data_rdata_i, (s_rd_left_q == 8'd1), s_rd_resp_d, s_rd_id_q};
if (s_rd_burst_q == `AXI4_BURST_TYPE_FIXED) begin
next_rd_addr = s_rd_addr_q;
end else begin
next_rd_addr = s_rd_addr_q + (32'd1 << s_rd_size_q);
if ((s_rd_burst_q == `AXI4_BURST_TYPE_WRAP) && (beats_to_wrap(
s_rd_addr_q, s_rd_left_q - 8'd1, s_rd_size_q, s_rd_burst_q
) == 8'd1)) begin
next_rd_addr = s_rd_addr_q & ~((32'd1 << s_rd_size_q) - 1'b1);
next_rd_addr = next_rd_addr - (((32'(s_rd_left_q) - 32'd1) << s_rd_size_q));
end
end
s_rd_addr_d = next_rd_addr;
s_rd_left_d = s_rd_left_q - 8'd1;
// Step with the original geometry: FIXED holds, INCR increments,
// WRAP wraps at the total-burst boundary (mid-line starts included).
next_rd_addr = s_rd_step_addr;
s_rd_addr_d = next_rd_addr;
s_rd_left_d = s_rd_left_q - 8'd1;
if (s_rd_frag_q != 4'd0) begin
s_rd_frag_d = s_rd_frag_q - 4'd1;
end else if (s_rd_left_q == 8'd1) begin
@@ -512,6 +538,7 @@ module sdram_axi4 (
s_wr_frag_d = s_wr_frag_q;
s_wr_size_d = s_wr_size_q;
s_wr_burst_d = s_wr_burst_q;
s_wr_len_d = s_wr_len_q;
s_wr_id_d = s_wr_id_q;
s_wr_resp_d = s_wr_resp_q;
s_aw_pop = 1'b0;
@@ -553,6 +580,7 @@ module sdram_axi4 (
s_wr_left_d = s_aw_len + 8'd1;
s_wr_size_d = s_aw_size;
s_wr_burst_d = s_aw_burst;
s_wr_len_d = s_aw_len;
s_wr_id_d = s_aw_id;
wr_frag = fragment_beats(s_aw_addr, s_aw_len + 8'd1, s_aw_len, s_aw_size, s_aw_burst);
s_wr_frag_d = wr_frag;
@@ -574,9 +602,9 @@ module sdram_axi4 (
if (s_w_data[0] != (s_wr_left_q == 8'd1)) begin
s_wr_resp_d = `AXI4_RESP_SLAVE_ERROR;
end
if (s_wr_burst_q != `AXI4_BURST_TYPE_FIXED) begin
s_wr_addr_d = s_wr_addr_q + (32'd1 << s_wr_size_q);
end
// Step with the original geometry; this also gives WRAP writes a
// correct wrap-back instead of running past the boundary.
s_wr_addr_d = s_wr_step_addr;
s_wr_left_d = s_wr_left_q - 8'd1;
if (s_wr_frag_q != 4'd0) begin
s_wr_frag_d = s_wr_frag_q - 4'd1;
@@ -632,6 +660,7 @@ module sdram_axi4 (
s_rd_frag_q <= '0;
s_rd_size_q <= '0;
s_rd_burst_q <= '0;
s_rd_len_q <= '0;
s_rd_id_q <= '0;
s_rd_resp_q <= `AXI4_RESP_OKAY;
s_wr_state_q <= WrIdle;
@@ -640,6 +669,7 @@ module sdram_axi4 (
s_wr_frag_q <= '0;
s_wr_size_q <= '0;
s_wr_burst_q <= '0;
s_wr_len_q <= '0;
s_wr_id_q <= '0;
s_wr_resp_q <= `AXI4_RESP_OKAY;
s_err_event_q <= 1'b0;
@@ -652,6 +682,7 @@ module sdram_axi4 (
s_rd_frag_q <= s_rd_frag_d;
s_rd_size_q <= s_rd_size_d;
s_rd_burst_q <= s_rd_burst_d;
s_rd_len_q <= s_rd_len_d;
s_rd_id_q <= s_rd_id_d;
s_rd_resp_q <= s_rd_resp_d;
s_wr_state_q <= s_wr_state_d;
@@ -660,6 +691,7 @@ module sdram_axi4 (
s_wr_frag_q <= s_wr_frag_d;
s_wr_size_q <= s_wr_size_d;
s_wr_burst_q <= s_wr_burst_d;
s_wr_len_q <= s_wr_len_d;
s_wr_id_q <= s_wr_id_d;
s_wr_resp_q <= s_wr_resp_d;
s_err_event_q <= s_rd_err_event_d || s_wr_err_event_d;
+6 -1
View File
@@ -282,12 +282,17 @@ module axi4_data_crossbar #(
logic [32:0] beat_bytes;
logic [32:0] burst_bytes;
logic [32:0] last_addr;
logic [32:0] wrap_base;
logic [32:0] window_limit;
beat_bytes = 33'd1 << size;
burst_bytes = beat_bytes * ({25'd0, len} + 1'b1);
// WRAP bursts end at their wrap boundary, not at addr + burst_bytes: a
// mid-line start otherwise overshoots and fails the 4 KiB page check.
wrap_base = {1'b0, addr} & ~(burst_bytes - 1'b1);
last_addr = (burst == 2'b00) ?
({1'b0, addr} + beat_bytes - 1'b1) :
({1'b0, addr} + burst_bytes - 1'b1);
((burst == 2'b10) ? (wrap_base + burst_bytes - 1'b1) :
({1'b0, addr} + burst_bytes - 1'b1));
window_limit = {1'b0, addr[31:12], 12'd0} + 33'd4096;
return !lock && (size <= 3'd3) &&
((burst == 2'b00) || (burst == 2'b01) ||
+73
View File
@@ -940,6 +940,79 @@ module axi4_data_crossbar_tb;
join
end
// C906 cache fills issue WRAP bursts whose critical word can start near
// the end of a 4 KiB page: the burst must be routed to SDRAM with its
// real (wrapped) last address, not faulted by an INCR-style overshoot.
wrap_page_edge_read : begin
logic collect_done;
logic seen_sdram_ar;
logic seen_error_ar;
logic seen_fault;
logic [31:0] seen_addr;
logic [ 7:0] seen_len;
logic [ 1:0] seen_burst;
collect_done = 1'b0;
seen_sdram_ar = 1'b0;
seen_error_ar = 1'b0;
seen_fault = 1'b0;
seen_addr = '0;
seen_len = '0;
seen_burst = '0;
fork
begin
@(negedge clk_i);
masters[1].arid = 7'h09;
masters[1].araddr = 32'h3800_0FF0;
masters[1].arlen = 8'd7;
masters[1].arsize = 3'd3;
masters[1].arburst = 2'b10;
masters[1].arvalid = 1'b1;
do @(posedge clk_i); while (!masters[1].arready);
@(negedge clk_i);
masters[1].arvalid = 1'b0;
masters[1].arlen = '0;
masters[1].arburst = 2'b01;
end
begin
while (!collect_done) begin
@(posedge clk_i);
if (targets[1].arvalid && targets[1].arready) begin
seen_sdram_ar = 1'b1;
seen_addr = targets[1].araddr;
seen_len = targets[1].arlen;
seen_burst = targets[1].arburst;
end
if (targets[5].arvalid && targets[5].arready) seen_error_ar = 1'b1;
if (fault_valid_o) seen_fault = 1'b1;
end
end
return_target1_burst(7'h09, 64'hACE0_5555_0000_0000);
begin
for (int beat = 0; beat < 8; beat++) begin
do @(posedge clk_i); while (!masters[1].rvalid);
if (masters[1].rid != 7'h09)
$fatal(1, "WRAP page-edge read beat %0d ID mismatch", beat);
if (masters[1].rdata !== (64'hACE0_5555_0000_0000 + 64'(beat))) begin
$fatal(1, "WRAP page-edge read beat %0d data mismatch", beat);
end
if (masters[1].rresp !== 2'b00) $fatal(1, "WRAP page-edge read beat %0d faulted", beat);
if (masters[1].rlast !== (beat == 7)) begin
$fatal(1, "WRAP page-edge read beat %0d RLAST mismatch", beat);
end
@(negedge clk_i);
end
collect_done = 1'b1;
end
join
if (!seen_sdram_ar || seen_error_ar || seen_fault) begin
$fatal(1, "WRAP page-edge read was not cleanly routed to SDRAM");
end
if ((seen_addr != 32'h3800_0FF0) || (seen_len != 8'd7) || (seen_burst != 2'b10)) begin
$fatal(1, "WRAP page-edge read metadata was not forwarded intact");
end
$display("AXI4 data crossbar WRAP page-edge read routed to SDRAM intact");
end
@(negedge clk_i);
if (!idle_o || (outstanding_read_o != 8'd0) || (outstanding_write_o != 8'd0)) begin
$fatal(1, "ACL and QoS transactions did not drain the crossbar");
+93
View File
@@ -264,6 +264,87 @@ module sdram_data_tb;
end
endtask
// Byte address of beat `beat` in a 4-byte WRAP burst starting mid-line.
function automatic logic [31:0] wrap_beat_addr(input logic [31:0] start, input integer beat,
input logic [7:0] length);
logic [31:0] burst_bytes;
logic [31:0] wrap_base;
logic [31:0] addr;
begin
burst_bytes = ({24'd0, length} + 32'd1) * 32'd4;
wrap_base = start & ~(burst_bytes - 32'd1);
addr = start + (32'(beat) * 32'd4);
if (addr >= (wrap_base + burst_bytes)) addr = addr - burst_bytes;
return addr;
end
endfunction
// WRAP write starting mid-line; beat data encodes the beat's own byte
// address so a plain sequential INCR readback catches any wrap-back error.
task automatic write_wrap(input logic [31:0] address, input logic [7:0] length);
integer beat;
begin
@(negedge clk_i);
axi4.awaddr = address;
axi4.awlen = length;
axi4.awburst = `AXI4_BURST_TYPE_WRAP;
axi4.awvalid = 1'b1;
do @(posedge clk_i); while (!axi4.awready);
@(negedge clk_i);
axi4.awvalid = 1'b0;
for (beat = 0; beat <= length; beat++) begin
axi4.wdata = 32'h1000_0000 + wrap_beat_addr(address, beat, length);
axi4.wstrb = 4'hF;
axi4.wlast = (beat == length);
axi4.wvalid = 1'b1;
do @(posedge clk_i); while (!axi4.wready);
@(negedge clk_i);
end
axi4.wvalid = 1'b0;
axi4.wlast = 1'b0;
axi4.bready = 1'b1;
do @(posedge clk_i); while (!axi4.bvalid);
if (axi4.bresp != `AXI4_RESP_OKAY) begin
$fatal(1, "AXI4 SDRAM WRAP write failed at %08x", address);
end
@(negedge clk_i);
axi4.bready = 1'b0;
end
endtask
// WRAP read starting mid-line; every beat must carry the data of its own
// wrap-ordered byte address, with RLAST only on the final beat.
task automatic read_wrap(input logic [31:0] address, input logic [7:0] length);
integer beat;
logic [31:0] beat_addr;
begin
@(negedge clk_i);
axi4.araddr = address;
axi4.arlen = length;
axi4.arburst = `AXI4_BURST_TYPE_WRAP;
axi4.arvalid = 1'b1;
do @(posedge clk_i); while (!axi4.arready);
@(negedge clk_i);
axi4.arvalid = 1'b0;
axi4.rready = 1'b1;
for (beat = 0; beat <= length; beat++) begin
beat_addr = wrap_beat_addr(address, beat, length);
do @(posedge clk_i); while (!axi4.rvalid);
if (axi4.rdata !== (32'h1000_0000 + beat_addr)) begin
$fatal(1, "WRAP read mismatch beat %0d addr %08x got %08x", beat, beat_addr, axi4.rdata);
end
if (axi4.rresp != `AXI4_RESP_OKAY) begin
$fatal(1, "WRAP read SLVERR beat %0d", beat);
end
if (axi4.rlast !== (beat == length)) begin
$fatal(1, "WRAP RLAST mismatch beat %0d", beat);
end
@(negedge clk_i);
end
axi4.rready = 1'b0;
end
endtask
initial begin
#2_000_000;
$fatal(1, "simulation timeout");
@@ -324,6 +405,18 @@ module sdram_data_tb;
write_incr(`SOC_ADDR_SDRAM_BASE + 32'h300, 8'd15);
read_incr(`SOC_ADDR_SDRAM_BASE + 32'h300, 8'd15);
// C906 cache fills issue WRAP bursts whose critical word can start
// mid-line: cover a 64-byte line entered at +0x30 in both directions.
write_incr(`SOC_ADDR_SDRAM_BASE + 32'h400, 8'd15);
read_wrap(`SOC_ADDR_SDRAM_BASE + 32'h430, 8'd15);
write_wrap(`SOC_ADDR_SDRAM_BASE + 32'h530, 8'd15);
read_incr(`SOC_ADDR_SDRAM_BASE + 32'h500, 8'd15);
// A second mid-line offset guards against offset-dependent mask math.
write_incr(`SOC_ADDR_SDRAM_BASE + 32'h600, 8'd15);
read_wrap(`SOC_ADDR_SDRAM_BASE + 32'h610, 8'd15);
write_wrap(`SOC_ADDR_SDRAM_BASE + 32'h720, 8'd15);
read_incr(`SOC_ADDR_SDRAM_BASE + 32'h700, 8'd15);
@(negedge clk_i);
axi4.araddr = `SOC_ADDR_SDRAM_BASE;
axi4.arlen = 8'd0;