Skip to content

rewrite the axi fabric to support pipelines. - #513

Merged
nathanaelhuffman merged 10 commits into
mainfrom
ndh/axi-pipes
Aug 17, 2026
Merged

rewrite the axi fabric to support pipelines.#513
nathanaelhuffman merged 10 commits into
mainfrom
ndh/axi-pipes

Conversation

@nathanaelhuffman

@nathanaelhuffman nathanaelhuffman commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

As #480 mentions, our somewhat naive axi fabric has some improvement opportunities.

This PR rewrites the bock to support user-specified pipeline stages between each of the axi responders, and adds more test coverage since the block is not as simple. Some additional logic improvements are integrated in the rewrite including a one-hot decode methodolgy to reduce logic and increase fmax.

This also fixes a couple of bugs in the axi8 responder that we weren't experiencing in hardware given the previous design but would experience now given the implementation removed some of the simplifications:

  • responder read address was driven from the write address
  • decode teardown was unreachable while AWVALID stayed asserted, so the next read decoded against a stale address
  • a request held one cycle past its handshake re-armed the fabric and issued a duplicate transaction
  • and axil8_resizer had four reversed assignments and reads were't simulated so this was not caught in simulation.

This is running successfully on cosmo hw.

Fixes #480

Summary of new functionality:

  • Decode/mux rework: range membership became an equality compare on the address bits above each responder's span (bases are span-aligned, spans are powers of two), replacing two 32-bit magnitude compares per responder and their carry chains.
  • The dynamic address mask which was implemented as a variable-shift mux shared by every responder path is now compile-time constant per responder. The responder select became a registered one-hot instead of an integer index, so the return path is a flat AND-OR tree.
  • Configurable pipelining: pipe_stages per responder in the config record. Since the fabric admits one transaction at a time, axil_pipe serializes it into one request bundle out and one response bundle back rather than five channel register slices. This new implementation uses about a third of the flops, one 1-bit token chain, no timing exceptions required (everything is register-register timed). If you set pipe_stages = 0, you get a free pass-through.

A fun timing comparison:

image before after
cosmo_seq WNS 0.100 ns 0.120 ns (worst path moved out of eSPI)
grapefruit WNS 0.038 ns 0.226 ns
cosmo_hp 86.2 MHz 101.5 MHz

Note that most of this gain is from the consolidated logic: only cosmo_seq's eSPI responder is piped, at one stage.

Aaron-Hartwig
Aaron-Hartwig approved these changes Aug 12, 2026
View reviewed changes

@Aaron-Hartwig Aaron-Hartwig left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits but this LGTM. I know you've had a bit of runtime through other testing as well.

Comment thread hdl/ip/vhd/axi_blocks/axil_pipe.vhd Outdated
Comment thread hdl/ip/vhd/axi_blocks/axil_pipe.vhd Outdated
Comment thread hdl/ip/vhd/axi_blocks/sims/axil_interconnect_sim_pkg.vhd Outdated
Comment thread hdl/ip/vhd/axi_blocks/sims/axil_slow_responder.vhd Outdated
@nathanaelhuffman
nathanaelhuffman force-pushed the ndh/axi-pipes branch 2 times, most recently from 95d1d37 to 20bb778 Compare August 12, 2026 20:12
@nathanaelhuffman
axi_blocks: fix reversed view assignments in axil8_resizer
608e78a 
The read-address valid, read-data ready, and read-data resp/valid assignments
drove the wrong side of the interface view, so the responder never saw
ARVALID and the fabric never saw RVALID/RRESP. Only spi_nor_th used this
entity and spi_nor_tb never reads, which is why no read had ever actually
traversed it.
@nathanaelhuffman
axil_interconnect: rework address decode and transaction muxing
962746d 
The fabric was a purely combinational crossbar behind a one cycle decode, and
the decode itself built two 32 bit magnitude compares per responder plus a
variable width address mask shared by every responder path. On cosmo_hp that
mask and its carry chains are the reported critical path.

Spans are powers of two and bases are aligned to their own span, so range
membership is just an equality compare on the address bits above the span. The
compare stays 32 bits wide against a resized address so a narrow initiator
cannot match a base it has no way to reach; the extra bits are constant zeros
and fold away. Each responder now masks the address to its own span with a
compile time constant, which also lets the fabric side of each responder's
address bus collapse to the bits it actually decodes.

The responder select becomes a registered one-hot instead of an integer index,
so the return path is a flat AND-OR tree rather than an integer to one-hot
decode buried inside combinational logic.

Three behavioural fixes fall out of the rework:

- The responder read address was driven from the initiator *write* address. This
  only worked because both initiators drive AWADDR and ARADDR from the same
  register.
- Teardown now takes priority over arming, and a write is only decoded once AW
  and W are both present, matching the condition every responder already applies
  before asserting AWREADY. Previously an initiator that left AWVALID asserted
  pinned the fabric to the completed transaction's responder, so the next read
  decoded against a stale address.
- Re-arming is blocked per channel until the initiator drops the request it just
  completed. Initiators here deassert VALID a cycle after the handshake, so
  without this a stale request re-armed the fabric and a duplicate transaction
  went out behind the initiator's back.

The error responder also only answers the channel that was actually decoded, so
an unmapped read can no longer hand an AWREADY to a write that has not presented
its data yet.

Add span_mask/bases_aligned/ranges_disjoint/bases_reachable to axil_common_pkg
along with elaboration asserts for the invariants the decode now relies on.
@nathanaelhuffman
axil_interconnect: add a VUnit testbench
8830a09 
The interconnect had no testbench, and because axil8_resizer drove four
assignments in the wrong direction and spi_nor_tb never reads, no read had ever
traversed this fabric in simulation.

The harness instantiates the flat port axil_interconnect_2k8 with a mixed
responder map that includes an unmapped hole, and can be driven either by
vunit_lib.axi_lite_master or by hand so the testbench can reproduce initiator
handshake patterns the bus functional model never generates.

Two responder models, deliberately with different handshake shapes:

- axil_sram_responder wraps the production axil_target_txn, so it reproduces the
  contract every register block in the tree presents: a registered AWREADY pulse
  gated on AWVALID and WVALID, combinational ARREADY, and a single cycle BVALID
  pulse when BREADY is already asserted.
- axil_slow_responder accepts AW and W independently after LFSR driven stalls and
  holds its responses until READY, which is the shape nothing in the tree
  currently exercises.

The harness also counts handshakes on both sides of the fabric so a duplicated
transaction is caught even when the data happens to land correctly, and checks
that a stalled channel's payload stays put.

Nine of the ten cases fail against the fabric as it was before the previous
commit.
@nathanaelhuffman
axil_interconnect: add a pipe_stages field to the responder config
4dcc96a 
Adds the knob that the next commit implements. Every site is set to 0, so this is
functionally and structurally a no-op.

VHDL records have no field defaults and aggregates must be complete, so adding a
field breaks every config aggregate in the tree. Since they all had to be touched
anyway, they now go through a resp_cfg() constructor with defaulted arguments, so
the next field addition will not break them.
@nathanaelhuffman
axil_interconnect: add configurable per responder pipeline stages
3f8b5de 
axil_pipe inserts config_array(i).pipe_stages register stages in each direction
between the fabric and one responder, so a responder that sits a long way from
the fabric no longer has to be reached and answered inside a single clock period.

Because the fabric admits one transaction at a time, this does not need five
independent AXI channel register slices. The whole transaction serializes into
one request bundle out and one response bundle back, which is roughly a third of
the flops a pair of full register slices costs and needs one token chain instead
of five sets of valid/ready control. Only the bits a responder actually decodes
are carried, so an 8 bit responder pipes 45 bits rather than 32 bit addresses.

Each payload stage advances only behind its own token, so every stage holds what
it was given until the next transaction pushes through it. That keeps the far end
stable across a multi-cycle handshake and removes the need for a separate capture
register at either end. Gating the chain on the OR of all tokens instead would
clobber the last stage on the cycle the token reached it.

Deliberately no timing exceptions are required, because cosmo_hp goes through
yosys and nextpnr where there is no way to express one.

The sink side asserts AWREADY and WREADY together as a registered one-shot, never
combinationally off AWVALID, and will not re-arm until the request that just
completed is off the bus. The source side samples all the responder handshakes in
one state, because axil_target_txn presents BVALID as a single cycle pulse when
BREADY is already asserted and ARREADY combinationally.

stages = 0 generates a plain pass-through, so it is free and the netlist is
unchanged where the knob is left alone.

The testbench now runs a mixed 0/1/3/2 stage map and adds a latency case that
proves the stages are really in the path: reads answer in 2, 6 and 8 clocks at 0,
1 and 2 stages respectively.

Note: multitool format could not be run here, vsg is not installed in this
environment.
@nathanaelhuffman
cosmo_seq: pipeline the eSPI responder by one stage
44da2da 
eSPI is the largest register file and the most distant block in the design, and
it owned the worst clk_125m path. With one stage in each direction WNS goes from
-0.003ns (failing) to +0.120ns and the worst path moves out of eSPI entirely into
the DIMM SPD proxy.

The other responders stay unpiped. cosmo_hp and grapefruit need no stages at all
after the decode rework.
@nathanaelhuffman
axil_interconnect: document the fabric and the pipeline
f377686 
Covers the single-outstanding-transaction property everything leans on, the
equality-based address decode and its elaboration-checked invariants, the
one-hot select, the error responder, and how axil_pipe serializes a transaction
into one request bundle out and one response bundle back rather than five
independent channel register slices.

Two draw.io diagrams, sources embedded in the SVG content attribute as with the
other drawings in the tree.
@nathanaelhuffman
axi_blocks: drop --! doc comment markers in the interconnect sources
cd28964 
Plain -- everywhere in this block, matching axil_common_pkg. Comment text only,
no functional change.
@nathanaelhuffman
axil_interconnect: give the testbench helpers descriptive names
f352869 
ba/w32 were too terse to read at the call sites. responder_addr also drops the
overload with bus_addr, since the two do quite different things.

  ba(idx, offset) -> responder_addr(idx, offset)   34 uses
  ba(addr)        -> bus_addr(addr)                 8 uses
  w32(value)      -> data_word(value)              10 uses

Kept rather than inlined as to_std_logic_vector(). responder_addr does a config
array lookup, so it is not a conversion at all. The other two are thin wrappers,
but inlining them would push ten lines past the 120 column limit (worst case 133),
and bus_addr keeps the initiator width in one place. Also wrapped three lines that
were already over the limit.
@nathanaelhuffman
Implement review fixes
7a18476 
Make the slow responder's longest stall a generic and drive it, along with the
deepest pipe_stages in the responder map, from a single MAX_DELAY constant in
axil_interconnect_sim_pkg. Previously the responder hard-coded 3 internally while
the map separately said pipe_stages => 3, so the two could drift apart silently.

The LFSR stall fields are only two bits, so delay_of now clamps to max_delay and
the reset values go through the same clamp, which keeps the counters in range for
a max_delay below 3. Verified the suite passes with MAX_DELAY at 1, 3 and 5.
@nathanaelhuffman
nathanaelhuffman merged commit 3ad17b4 into main Aug 17, 2026
10 checks passed
@nathanaelhuffman
nathanaelhuffman deleted the ndh/axi-pipes branch August 17, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Aaron-Hartwig Aaron-Hartwig Aaron-Hartwig approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

We're starting to feel timing pressure on the axi interconnect as the design scales

2 participants

@nathanaelhuffman @Aaron-Hartwig