Skip to content

Commit c850006

Browse files
marcos-mendezMarcos
andauthored
test(rtl): canary for back-to-back contr_rd_en single-deep latch limitation (#40)
Adds `test_back_to_back_contr_rd_drops_second` to verif/global_mem_controller/test_global_mem_controller.py — the CANARY demanded by MAST issue #21. The arbiter in src/global_mem_controller.sv defers a contr_rd_en pulse that arrives during a busy cycle into a single-deep `contr_rd_pending` latch. Today's caller (gpu_controller.sv) holds each request pending its own ack handshake, so depth-1 is sufficient AT THIS MOMENT. When gpu_controller.sv migrates to AXI4 (Phase 3 of the parameter-taxonomy plan), or any future caller can issue back-to-back contr_rd_en pulses, this assumption breaks: a second pulse arriving while pending is set will silently overwrite the first. The new test pins this behaviour: * Pre-loads distinct sentinels at addr_a / addr_b / addr_c * Holds the AXI4 adapter busy with a core1 read at addr_c * Pulses contr_rd_en for addr_a then addr_b on consecutive cycles * Asserts BOTH acks land — the post-fix behaviour * `@cocotb.test(expect_fail=True)` flips the assertion failure into a regression PASS today, so the suite stays green while the gap is documented in-place The test fires the assertion exactly as predicted (saw 1 ack carrying word_b=0xBBBBBBBB; word_a=0xAAAAAAAA was lost), proving the depth-1 gap empirically. After a future fix widens the latch to a FIFO (depth N>=2) or adds a contr_rd_busy back-pressure output, the test will start passing functionally — at which point the expect_fail=True marker must be removed (instructions are in the test docstring). Out of scope per issue #21: widening the latch, redesigning the arbiter, ADR work. This PR is canary-only. Refs: MAST #21, MAST #19 (the merged arbiter), MAST #24/#26 (the SVA invariant that already guards the inflight/pending mutual exclusion). TESTS=11 PASS=11 FAIL=0 SKIP=0 Authored by Agent 1 (RTL Architect). Signed-off-by: Marcos <m@pop.coop> Co-authored-by: Marcos <m@pop.coop>
1 parent b5be635 commit c850006

1 file changed

Lines changed: 153 additions & 0 deletions

File tree

‎verif/global_mem_controller/test_global_mem_controller.py‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,3 +603,156 @@ async def test_m_araddr_upper_bits_zero_on_contr_read(dut):
603603
await RisingEdge(dut.clk)
604604
if dut.contr_rd_ack.value == 1:
605605
break
606+
607+
608+
# ----------------------------------------------------------------------------
609+
# CANARY: back-to-back contr_rd_en pulses expose single-deep pending latch
610+
# (MAST issue #21)
611+
# ----------------------------------------------------------------------------
612+
#
613+
# This test is the CANARY for the depth-1 limitation of `contr_rd_pending`
614+
# in `src/global_mem_controller.sv`. Today's caller (`gpu_controller.sv`)
615+
# holds each contr_rd request pending its own ack handshake, so depth-1 is
616+
# sufficient AT THIS MOMENT. Issue #21 demands a test that demonstrates the
617+
# gap so a future migration (e.g. AXI4 caller, or anything that can issue
618+
# back-to-back `contr_rd_en` pulses) does not silently lose reads.
619+
#
620+
# Failure mode being captured:
621+
#
622+
# Cycle T0: contr_rd_en=1, contr_rd_addr=A, core1 still busy in adapter
623+
# → grant_contr_rd=0 → pending<=1, pending_addr<=A
624+
# Cycle T1: contr_rd_en=1, contr_rd_addr=B, core1 still busy in adapter
625+
# → grant_contr_rd=0 → pending<=1, pending_addr<=B (A LOST)
626+
# Cycle T2..: cm_busy drops → grant fires → reads B → contr_rd_ack pulses
627+
# EXACTLY ONCE. The first pulse (addr A) was silently dropped.
628+
#
629+
# Marked `expect_fail=True` so the cocotb regression bookkeeping treats the
630+
# test as PASS today (the assertion fires, proving the gap), and as FAIL the
631+
# day someone widens the latch to a FIFO without removing the marker. When
632+
# the latch is widened (FIFO depth N≥2 OR a `contr_rd_busy` back-pressure
633+
# output), update this test by:
634+
# 1. Removing `expect_fail=True` from the decorator
635+
# 2. Asserting that BOTH addr-A and addr-B reads landed in order
636+
# 3. Updating the docstring to reference the fix PR/ADR
637+
# ----------------------------------------------------------------------------
638+
639+
@cocotb.test(expect_fail=True)
640+
async def test_back_to_back_contr_rd_drops_second(dut):
641+
"""CANARY for MAST #21: two contr_rd_en pulses on consecutive cycles
642+
while the adapter is busy → only ONE contr_rd_ack fires.
643+
644+
The single-deep `contr_rd_pending` latch in
645+
`src/global_mem_controller.sv` overwrites its captured address on every
646+
new `contr_rd_en` pulse that cannot be granted. When two pulses arrive
647+
back-to-back while the arbiter cannot service either, the FIRST pulse's
648+
address is silently overwritten by the second — the first read is lost.
649+
650+
This is the pre-fix CANARY: today the test FAILS the functional
651+
assertion (only 1 of 2 acks observed), and `@cocotb.test(expect_fail=
652+
True)` flips that into a regression PASS. After the latch is widened
653+
to a FIFO (depth N≥2) OR a `contr_rd_busy` back-pressure output is
654+
added, this test will start passing functionally and the
655+
`expect_fail=True` marker will need to be removed (the test will then
656+
flip to FAIL on the regression, prompting an update). See MAST #21
657+
for the gating decision.
658+
659+
Stimulus shape:
660+
* Pre-load distinct values at addr_a and addr_b via the loader.
661+
* Issue a core1 read at addr_c to occupy the AXI4 adapter
662+
(cm_busy stays high for several cycles after we drop core1_rd_req).
663+
* On cycle T0 (one cycle after dropping core1_rd_req, when core1
664+
is no longer driving the bus but cm_busy is still high), pulse
665+
`contr_rd_en=1, contr_rd_addr=addr_a`.
666+
* On cycle T1, pulse `contr_rd_en=1, contr_rd_addr=addr_b`.
667+
* Wait for the system to drain.
668+
* Count `contr_rd_ack` pulses and capture data on each pulse.
669+
670+
Expected behaviour POST-FIX (FIFO widened): two acks, first==word_a,
671+
second==word_b.
672+
Actual behaviour TODAY (depth-1 latch): exactly one ack carrying
673+
word_b — addr_a was overwritten in the pending register and never
674+
issued.
675+
"""
676+
cocotb.start_soon(Clock(dut.clk, CLK_PERIOD_NS, unit="ns").start())
677+
await reset_dut(dut)
678+
679+
# Pre-load distinct sentinels at three cache-line-aligned addresses.
680+
addr_a, word_a = 0x0000_0200, 0xAAAA_AAAA
681+
addr_b, word_b = 0x0000_0240, 0xBBBB_BBBB
682+
addr_c, word_c = 0x0000_0280, 0xCCCC_CCCC
683+
await contr_loader_write(dut, addr_a, word_a)
684+
await contr_loader_write(dut, addr_b, word_b)
685+
await contr_loader_write(dut, addr_c, word_c)
686+
await RisingEdge(dut.clk) # let the loader NBAs commit
687+
688+
# Kick off a core1 read at addr_c. After we drop core1_rd_req on the
689+
# next cycle, the AXI4 adapter is still busy completing the read
690+
# (cm_busy high), so the arbiter cannot grant any contr_rd that
691+
# arrives during this window.
692+
dut.core1_addr.value = addr_c
693+
dut.core1_rd_req.value = 1
694+
await RisingEdge(dut.clk)
695+
dut.core1_rd_req.value = 0
696+
# Now core1_active=0 but cm_busy=1. Any contr_rd_en pulse here will
697+
# be deferred into the single-deep pending latch.
698+
699+
# Cycle T0: first contr_rd_en pulse (addr_a). Cannot be granted
700+
# (cm_busy is high), so pending<=1 and pending_addr<=addr_a.
701+
dut.contr_rd_addr.value = addr_a
702+
dut.contr_rd_en.value = 1
703+
await RisingEdge(dut.clk)
704+
705+
# Cycle T1: second contr_rd_en pulse (addr_b), still cannot be
706+
# granted (cm_busy still high or grant blocked by inflight). The
707+
# depth-1 latch overwrites pending_addr with addr_b — addr_a is
708+
# now LOST.
709+
dut.contr_rd_addr.value = addr_b
710+
dut.contr_rd_en.value = 1
711+
await RisingEdge(dut.clk)
712+
dut.contr_rd_en.value = 0
713+
714+
# Drain: wait long enough for the core1 read AND any contr_rd_ack
715+
# pulses to land. Count ack pulses and capture data.
716+
contr_acks = []
717+
saw_core1_ack = False
718+
for _ in range(800):
719+
await RisingEdge(dut.clk)
720+
if dut.contr_rd_ack.value == 1:
721+
contr_acks.append(int(dut.contr_rd_data.value))
722+
if dut.core1_ack.value == 1:
723+
saw_core1_ack = True
724+
# Stop once everything has settled and we're past the expected
725+
# post-fix ack window. 800 cycles is generous (a single AXI4
726+
# round-trip on the simple master takes ~10 cycles).
727+
if saw_core1_ack and len(contr_acks) >= 2:
728+
break
729+
730+
# Sanity: the core1 read must have completed (otherwise something
731+
# unrelated to issue #21 is broken).
732+
assert saw_core1_ack, (
733+
"core1 read at addr_c never acked — testbench setup is wrong, "
734+
"this is not the depth-1 bug under test."
735+
)
736+
737+
# CANARY assertion: post-fix we expect TWO contr_rd_ack pulses
738+
# carrying word_a then word_b. Pre-fix we observe exactly one carrying
739+
# word_b. The expect_fail=True decorator inverts the regression result
740+
# so the suite is GREEN today (gap demonstrated) and turns RED the day
741+
# someone widens the latch without updating this test.
742+
assert len(contr_acks) == 2, (
743+
f"single-deep contr_rd_pending latch dropped a request: expected "
744+
f"2 contr_rd_ack pulses (one per contr_rd_en pulse), saw "
745+
f"{len(contr_acks)}. Captured data: "
746+
f"{[f'0x{x:08x}' for x in contr_acks]}. "
747+
f"Expected post-fix: [0x{word_a:08x}, 0x{word_b:08x}]. "
748+
f"This is the documented gap from MAST #21 — see the test "
749+
f"docstring for how to update this test once the latch is widened."
750+
)
751+
assert contr_acks[0] == word_a, (
752+
f"first contr_rd_ack should carry word_a=0x{word_a:08x}, "
753+
f"got 0x{contr_acks[0]:08x}"
754+
)
755+
assert contr_acks[1] == word_b, (
756+
f"second contr_rd_ack should carry word_b=0x{word_b:08x}, "
757+
f"got 0x{contr_acks[1]:08x}"
758+
)

0 commit comments

Comments
 (0)