test: Watch the real INIT handshake and skip zero-length stream jobs - #188
Conversation
DanielKellerM
left a comment
DanielKellerM
left a comment
There was a problem hiding this comment.
check and verify
| // Handle writes | ||
| while(axis_jobs.size() > 0) begin | ||
| current_job = axis_jobs.pop_front(); | ||
| // a zero-length transfer emits no beats; waiting for one would steal the next job's |
There was a problem hiding this comment.
there is a parameter for what behaviour we want to regarding zero length transfers, is this aligned?
|
Checked, and you are right that it is not aligned. It is correct only for I built the directed case your question implies - a zero-length AXI-Stream transfer followed by a real 64 B one - and ran it both ways, with and without this PR:
So Two things I deliberately did not do without your call:
Note the CI matrix only ever runs Happy to add the parameter gate to this PR so the assumption is at least explicit and |
The write-side watchdog generator had no init case, so INIT fell through to an AXI-shaped binding on a wire the testbench ties to zero; the watchdog expired after 100 cycles on any INIT-destination transfer while the DMA was making progress. The AXI-Stream write model also popped zero-length jobs and then blocked waiting for beats that a rejected transfer never emits, so it consumed the next job's packet and mis-attributed every later one.
|
Agreed, and dropped. The guard did not fix anything; it only made a wrong configuration fail more legibly, and that does not belong in a PR that is otherwise fixing real failures. Issue #194 now tracks what What is left here is two measured fixes, 6 lines: INIT write watchdog watched a dead wire. The generator has no AXI-Stream model consumed the wrong job after a zero-length transfer. It popped zero-length jobs and blocked on Measured effect on the red
That is 10 of the 11 red legs, and with #193 the eleventh. Also rebased onto devel, since the previous base predated #185 and the diff would otherwise have reverted it. |
bd85a62 to
5c93bab
Compare
Two independent testbench defects behind the red
vsim-sim-randomlegs. Neither is an RTL problem; the RTL bug that accounts for the remaining leg is filed separately.1. The INIT write watchdog watched a dead wire
The write-side
stream_watchdoggenerator has noinitcase, so INIT fell through to the generic AXI-shaped bindinginit_axi_req.w_valid/init_axi_rsp.w_ready. The testbench ties bothinit_axi_read_reqandinit_axi_write_reqto'0before joining them, so that valid is a compile-time constant 0. The real INIT write handshake isinit_write_req.req_valid/init_write_rsp.req_ready.Effect: on any INIT-destination transfer the watchdog expired after
WatchDogNumCycles(100) while the DMA was working perfectly. Measured on the minimal case (one 4096 B transfer, OBI to INIT): it trips at exactly 1020 ns, whilei_idma_init_write.write_happeningtoggles 88 times and the OBI read memory 82 times, and the read watchdog counter stays healthy.This is not a weakening: the watchdog still fires after 100 genuinely idle cycles, it just observes the handshake it was always meant to.
2. The AXI-Stream write model mis-attributed jobs after a zero-length transfer
The model pops every job whose destination is AXI Stream, including zero-length ones, then blocks on
wait(axis_write_req.tvalid). A rejected zero-length transfer emits no beats, so the model consumed the next job's packet and wrote it at the zero-length job's destination, desynchronising by one job for the rest of the run. The read model was never affected because itswhile (address < src_addr + length)loop is a natural no-op at length 0; that asymmetry is the bug.Worth noting why it hid for so long:
compare_memskips any byte reading0xff, so a mis-attributed job usually compares vacuously against untouched memory and passes. It only becomes a hardMismatch!once a later, longer job has already overwritten the region.Verified
Regenerated and recompiled clean, then re-ran the previously failing legs with the CI gate (
! grep Error:and! grep Fatal:):snitch_writeseed 0: PASS (was tripping the write watchdog at 1020 ns)rw_axi_rw_axisseeds 0 and 2: PASSrw_axi_rw_axisseed 1: still fails, and correctly so. It no longer produces the bogusMismatch!; it now hangs honestly on the RTL defect filed separately, so this PR deliberately does not turn it green.10 of the 11 red legs are fixed here. No seed was pinned or skipped, no watchdog relaxed, no gate weakened, and the random job generator is untouched.