Release Candidate v2.73.0 - #1441
Merged
Merged
Conversation
Appears to work fine.
Add a generic to allow for disabling of the safe buffering scheme. This reduces the number of buffers from three to one, saving on resources. However, it becomes possible to write to/read from the same buffer at the same time, possibly corrupting the data. This option may be used if the user knows that write/read never happens at the same time and would like to save on resources.
Supported in newer VHDL standards but not the one used for vivado build it seems.
The non-oneshot one would drop triggers it seems (on hardware). Not sure if this is really due to the synchronizer or some more intricate timing issue that just happens to be resolved from switching to the oneshot synchronizer...
Implement handshake between clock domains to make sure no signals are missed. Allow trigger signal synchronous to either data or axil clock.
Does not test async clock case. Would be harder to validate and not sure if necessary.
They will be mentioned in PR thread.
Returning from wait to idle state before rdMoveDoneSync is de-asserted leads to the FSM immediately moving back to wait and thus jumping between wait and idle until rdMoveDoneSync returns to 0. Now the FSM waits until rdMoveDoneSync returns to 0, then moves back to idle.
…dRxReg The 0x24 address decode had its read and write actions swapped: the write branch drove rdata from r.rxPowerDown while the read branch loaded v.rxPowerDown from wdata. Swap them so AXI-Lite writes set rxPowerDown and reads return its current value.
…eadback The read decode never cleared v.axilReadSlave.rdata before the address case, so bits left over from a previous read of a wider register could leak into the readback of a narrower field. Zero rdata at the start of every read transaction, matching the JesdRxReg decode.
… multiframe Per JESD204B section 8.2 (Figure 50) and 8.3 (Tables 20/21), the second ILAS multiframe must carry /Q/ (K28.4) at octet 1 followed by 14 link-configuration octets including the FCHK checksum. JesdIlasGen previously emitted only the /R/ and /A/ frame delimiters. - JesdIlasGen: add mfCnt/wordCnt counters and emit /Q/ plus the 14 config octets during the second multiframe (mfCnt=1) in GT byte order. FCHK is the mod-256 sum of config octets 0-12. - Add Q_CHAR_C (K28.4, 0x9C) to Jesd204bPkg. - Thread the ILAS link-config generics (DID_G, BID_G, M_G, N_G, NPRIME_G, CS_G, S_G, HD_G, CF_G) through JesdTxLane and Jesd204bTx, and drive per-lane LID via a new lid_i port. All generics and the lid_i port are defaulted so existing instantiations compile unchanged. The SURF RX ILA state counts multiframes and does not parse config octets, so the added content cannot break SURF-to-SURF links.
…mments - Jesd204bPkg: drop the unused jesdScrambler procedure (no references anywhere in the library; the per-lane scrambling lives in JesdScrCmb/descrambler logic). - JesdSyncFsmRx: correct the comma-detect comment (the AND chain spans four consecutive cycles, not three) and remove the stale readBuff TODO note. - Jesd204bTx: replace the 'scrambling not tested' header warning now that TX scrambling is exercised end-to-end by the cocotb loopback bench.
…s plumbing Add a cocotb-based regression suite for the JESD204B library under tests/protocols/jesd204b: - jesd204b_test_utils.py: golden models (LFSR scrambler, LMFC, ILAS timeline builders) and shared testbench helpers - Unit benches: JesdLmfcGen, JesdScramblerWrapper, Jesd16bTo32b, Jesd32bTo16b, JesdSyncFsmTx, JesdIlasGen, JesdTxLane, JesdSyncFsmRx, JesdAlignFrRepCh, JesdRxLane - Register benches: JesdTxReg and JesdRxReg full map walks - test_Jesd204bLoopback: end-to-end TX-to-RX loopback across parameter cases with golden scrambler cross-checks - README with the coverage model and bench-to-module map Add cocotb-facing wrappers under protocols/jesd204b/wrappers that flatten record ports and expose AXI-Lite/GT interfaces for simulation (JesdTxLaneWrapper, JesdRxLaneWrapper, JesdScramblerWrapper, Jesd204bTxWrapper, Jesd204bRxWrapper, Jesd204bLoopbackWrapper), and load them from ruckus.tcl.
The SimLink models talk to Rogue over ZeroMQ, so the conda environment needs zeromq and pkg-config to build the simulator adapters, and both the conda and pip requirement sets need pyzmq for the test peers. Bump the ruckus lock to 4.30.1 for the mixed-language and xsim simulation support the new backends depend on.
…d C core Move the simulation link models out of axi/simlink into a top-level simlink/ module, and split the C sources into a simulator-neutral core plus thin per-simulator adapters: - simlink/shared/ holds the protocol state machines (Stream, Memory, SideBand), the instance registry, and a bounded ZeroMQ transport that every backend shares. - simlink/vcs/ keeps the VCS VHPI adapter, reduced to argument marshalling on top of the shared core. - simlink/sim/ holds the simulator-neutral wrappers plus the new RogueTcpStreamPacer for deterministic stream pacing. - simlink/ruckus.tcl picks the backend from RUCKUS_SIM_BACKEND and purges sibling-backend sources on regeneration, so switching simulators does not leave duplicate RogueTcp* entities in an existing Vivado project. axi/ruckus.tcl no longer loads simlink; the root ruckus.tcl loads it directly. The RTL instantiation graph deferral pattern follows the new path.
Add a VHPIDIRECT adapter so the SimLink models run under GHDL without a commercial VHPI implementation. Per-instance state lives in a registry keyed by the VHDL-supplied instance handle, so multiple RogueTcpStream, RogueTcpMemory, and RogueSideBand instances can coexist in a single elaboration. The Makefile rebuilds the shared object when the shared headers change, and boundary cases for zero-size and sparse frames are handled explicitly.
Add a DPI-C adapter and SystemVerilog shims so the SimLink models run under Vivado xsim. RogueDpiInstance maps each DPI import call onto a registry entry, which is what allows more than one SimLink instance per elaboration. The VHDL entities wrap the SystemVerilog DPI modules so existing VHDL testbenches instantiate them unchanged.
…ends Add the test-only HDL the pytest suites drive: flat harnesses for each protocol, a multi-instance harness, a memory relaunch harness, the VCS VPI bridges, and xsim tops covering multi-instance elaboration, duplicate port rejection, and eight-instance live traffic. These live under simlink/test/ and are supplied explicitly by the runners, so a normal SURF import never pulls them in.
Document the module for users and integrators: getting started, HDL integration, the wire protocol, the architecture and its shared-core split, Rogue client usage, migration from the old VCS-only flow, and troubleshooting.
Add the common layer the per-simulator suites build on: the Rogue-TCP ZeroMQ peer, peer orchestration, port allocation that is safe under pytest-xdist, the protocol encoder and decoder, the stream pacing reference model, and the multi-instance scenario definitions. The peer waits for its PUSH socket to report a connection before the first send, which closes the ZeroMQ slow-joiner window that made the live-traffic tests flaky.
Cover the GHDL backend end to end: Stream, Memory, SideBand, the wrappers, the pacer, multi-instance elaboration, simulator relaunch, and the VHPIDIRECT lifecycle. Add native ctypes suites that exercise the shared transport, the DPI instance registry, and stream overload handling without a simulator, including malformed-request, fabricated-pointer, and stale-pointer cases. The native suites carry local svdpi.h and vhpi_user.h stubs so they build with gcc alone.
Add VCS multi-instance and relaunch tests plus xsim multi-instance and eight-instance live-traffic tests, with shared helpers for building, elaborating, and running each simulator. Peers are spawned after elaboration so the ZeroMQ handshake does not race the simulator start, and the traffic test checks per-instance isolation across Stream, Memory, and SideBand.
Exercise the Stream, Memory, and SideBand models against real Rogue client objects instead of the synthetic ZeroMQ peer, so the wire contract is validated against the library the models exist to serve. Includes the memory error contract and the readiness probe path.
Add run.sh plus one runner per layer (native, ghdl, rogue, vcs, xsim) sharing a common helper file, so each layer runs on its own or all at once. The licensed VCS and xsim runners clean sim_build before running, derive the VCS version from VCS_HOME, and print the captured build and run logs even on success. env.example.sh documents the configuration and env.local.sh is ignored.
Install libzmq3-dev and valgrind for the SimLink native builds, and exclude tests/simlink from the unbounded "-n auto" integration invocation. The multi-instance ZeroMQ traffic test is timing sensitive and an unbounded worker count starves its peer handshake, so it runs in its own step with a bounded worker count on every push. Document that exception in tests/README.md.
The JESD204B wrappers are simulation-only helpers for the cocotb benches, so load them from the test files rather than the ruckus import. Drops the wrappers directory from protocols/jesd204b/ruckus.tcl.
Add the task planning, progress, and findings notes under docs/plans/ as AGENTS.md prescribes, covering the SimLink alignment work, the VHPIDIRECT readiness plan, the xsim multi-instance co-simulation and live-traffic plans, the user documentation plan, and the review findings tracker. simlink/README.md links to the alignment plan.
build_and_stage_so() staged the VHPIDIRECT library with a plain copy, which truncates and rewrites the destination in place. The ctypes lifecycle tests dlopen that exact path, and under --dist=worksteal a worker can leave the module and come back, re-running the module-scoped staging while its earlier handle is still mapped. The rewrite then segfaults the next dlsym. Stage through a temporary name and os.replace() it so live mappings stay valid. The multi-instance traffic scenario stopped clocking as soon as every peer process had exited, but a peer exits once it has pushed its last frame, before the model presents that frame to the DUT. The inbound assertions then sampled None on whichever lane had not landed. Keep clocking until every inbound observation has arrived, and report the outstanding peers and lanes on timeout. The persistent relaunch peer waited a fixed 30 s for the first simulator connection and 60 s for a response. A VCS phase spends minutes in vlogan and elaboration before simv binds, so the peer was always dead by then. Make the budget an env override and have the VCS relaunch test derive it from the vlogan/elaborate/run timeouts it already enforces. Also widen the peer scripts' import fallback: it keyed on the exact name 'tests', so a site-packages top-level 'tests' package shadowing this repo's raised 'tests.simlink' and re-raised instead of falling back to the sibling modules.
The multi-instance traffic test intermittently found one lane with no inbound frame at all: the peer reported a successful round trip and exited, the model logged its own outbound traffic on the same port pair, and nothing was ever logged as received. Which lane lost the frame varied per run. _await_push_connected() waited on ZMQ_EVENT_CONNECTED. That fires when the TCP connection completes, which is before the ZMTP handshake attaches the pipe to the socket, so a send issued right after it can still land on a pipe that is discarded rather than written. Wait on ZMQ_EVENT_HANDSHAKE_SUCCEEDED instead, which is the point the pipe is actually attached. _close_peer() then closed with LINGER=0, which is a second, independent discard: send() only queues into the outbound pipe, and closing before the I/O thread writes it throws the frame away. Stream and SideBand are fire and forget with nothing that would retransmit. Close with a bounded linger so the frame is flushed, still bounded so teardown cannot hang on an endpoint that never drains. Measured over 200 trials of the connect-before-bind ordering this suite uses: CONNECTED plus LINGER=0 lost 4 of 400 frames, matching the observed CI rate. HANDSHAKE_SUCCEEDED alone, bounded linger alone, and both together each lost none.
The runner targeted only test_RogueVcsMultiInstance.py and so missed test_RogueVcsRelaunch.py. Point it at tests/simlink/vcs so every VCS test is covered (and any added later). Verified under VCS X-2025.06: both tests run and pass (2 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Align and harden SimLink simulation backends
Both models are vendor Verilog/SystemVerilog with no VHDL equivalent, so they cannot be simulated by the GHDL/cocotb flow and cannot be reduced to a subset of their files: arch_package.sv includes arch_defines.v, and the four *_ddr3_parameters.vh headers exist only to be included by ddr3.v. Delete devices/Micron/ddr3 and devices/Micron/ddr4 in full, including the per-device manifests that carried the Verilog-specific USED_IN and FILE_TYPE properties, and drop the two now-dangling loadRuckusTcl lines from devices/Micron/ruckus.tcl. mt28ew, n25q, and p30 are unchanged. Projects whose testbenches include ddr3.v or arch_package.sv from surf/devices/Micron/ddr3/tb or ddr4/tb must vendor those Micron models locally.
The SaciSlaveAnalog module has no instantiation anywhere in the repository. It reached builds only through the loadSource -sim_only -dir glob over protocols/saci/saci1/sim, so no manifest change is needed.
The four UdpDebugBridge_stub.v files are Vivado synth-stub byproducts that no manifest loads: xilinx/xvc-udp/ruckus.tcl loads only UdpDebugBridge.dcp, and the 7Series and UltraScale DCP-regeneration manifests load only dcp/core/*.vhd. The sibling UdpDebugBridge_stub.vhd and .vho reference files are kept.
Remove the remaining non-RoCEv2 Verilog from surf
resetIn, resetDone and resetErr are all asynchronous to axilClk but were read directly by the combinational process. resetDone and resetErr come from the GT wizard buffer bypass helper block and are in the rxusrclk2 domain; resetIn is driven either from an axilClk combinational net or, in the PGP2fc wrappers, from the stableClk domain. Add a synchronizer on each and use the synchronized versions in the state machine, the user reset condition and the retry counter gate. Annotate the port list with the domain of each reset signal. The error is qualified with resetDone in the GT's RX clock domain so a single composite event crosses into axilClk. Synchronizing the two levels separately and combining them afterwards makes the condition depend on relative synchronizer latency: with a one-shot on the error the pulse is dropped whenever done rises more than about one axilClk cycle later, and with level synchronization on both the reset is held until the helper block clears its sticky error rather than for a bounded number of cycles. Gating ahead of the crossing removes the ordering dependence, and the one-shot then gives exactly one reset per error event. Synchronizer is used for the levels rather than RstSync so they have no asynchronous assertion path. resetInSync fans out to rst, state, rstcnt and retryCnt in the same combinational cone, and RstSync would also latch glitches from the combinational net driving resetIn.
COMMA_ALIGN_LATENCY is read as a 7-bit value, so the index can be 0 to 127, but the sample array only holds 40 counters. Any read above 39, including garbage returned during reset or on a DRP fault, indexed past the end of the array: a bound check failure in simulation and a lost increment in hardware. Guard the increment with the array length. LastPhase and the byte alignment comparison are left outside the guard so an out-of-range read is still reported and still triggers the mismatch reset.
retryCnt is a 16-bit counter incremented on each internally triggered reset with no upper bound, so it wrapped. A wrapped value is actively misleading for a link-health register: 65536 retries read back as 0, which looks like a link that never retried. Hold at 0xFFFF instead, which matches the "Does not roll-over" contract already documented for RetryCnt in the PyRogue model. Software can still clear the counter through the RstRetryCnt strobe.
PhaseTarget, Mask and ResetLen all share offset 0x100 with the sampler clear, so a write that only updates those fields also clears all 40 phase histogram bins. Nothing in the register map said so, so a configuration write silently discarded the histogram. Name the side effect on the three writable fields and name what clears them on PhaseHistRaw and the PhaseHist bins. State in the RTL that the fields keep their newly written values, so the note is not read as a reset of tgt/mask/rstlen. Also correct PhaseHistRaw's description, which did not mention that it is a histogram.
… the GHDL build GtRxAlignCheck had no automated coverage, so the reset synchronization, sample index bound, and retryCnt saturation fixes in this branch had nothing to protect them. The non-Vivado branch of xilinx/ruckus.tcl loaded only SelectIoRxGearboxAligner plus the dummy directory, so GtRxAlignCheck was absent from the GHDL build. It was therefore never reached by the `make analysis` syntax check in CI either, and was unavailable to cocotb. It has no unisim dependency, so load it alongside SelectIoRxGearboxAligner. Add a pytest/cocotb/GHDL regression built the same way as the rest of tests/: a thin wrapper under xilinx/general/wrappers/ flattens the slave register-map and master DRP records through the existing SlaveAxiLiteIpIntegrator and MasterAxiLiteIpIntegrator shims and exposes the DRP base address as an integer generic, and the bench reuses axil_read_u32/axil_write_u32 from tests/axi/utils.py and the runner helpers in tests/common/regression_utils.py. Two models stand in for the GT wizard, following how TimingGtCoreWrapper wires a GTHE3 core: a scripted AXI-Lite slave answers the COMMA_ALIGN_LATENCY read, and an RX-clock-domain model reproduces the buffer bypass done and error levels reacting to resetOut, with controllable ordering and skew between them. Coverage is 16 cases across three elaborations: lock on a masked phase match, retry and count on mismatch, histogram accumulation and its clear-on-config-write side effect, target and mask reprogramming, the override register and the SIMULATION_G generic, retry counter clearing, programmable reset length, asynchronous resetIn restart, axilRst return to reset values, and the OK response on unmapped offsets. Both GT_TYPE_G branches of the COMMA_ALIGN_LATENCY offset are checked against the observed DRP read address. Two cases are regression tests, both confirmed to fail when the corresponding fix is reverted: - An error asserted 12 rxClk ahead of done must still produce exactly one reset. Reverting to separately synchronized levels makes only this test fail, and the zero-skew case still passes, so it is specific to the skew rather than to any change in that area. - A 7-bit phase of 0x7F must not disturb the 40-bin histogram. Removing the bound check makes GHDL abort with "index (127) out of bounds (39 downto 0)". The three clock frequency registers are left uncovered and documented as such: the RTL fixes SyncClockFreq at REFRESH_RATE_G => 1.0, so one measurement window is a second of simulated time, and tests/base/sync/test_SyncClockFreq.py already proves that measurement. The retryCnt saturation edge is also out of reach, since it needs 65535 reset-and-reread cycles.
Fix GtRxAlignCheck reset clock domain crossings and add cocotb regression coverage
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description