[Work in Progress] AXI4 bundle definitions for the standard library

[ekiwi]

Very much work in progress, just collecting some thought for now.

Existing implementations:

• riscv-min
• rocket-chip
• chisel-bfm-tester
• tensil
• axi-in-chisel
• fpga-tidbits
• axi-nodes this library is quite new and using DataView :tada:
• antmicro/fastvdma

Important Decisions

• should the name of the bundles end in IO (like in DecoupledIO, QueueIO etc.) or Bundle (as is common in the rocket chip code base) or something else?
• should there be separate classes for the read and write address channels? Their fields are essentially the same.
• How do we expose the optional region field in the address bundles?
• aw or addrWriteChannel or addressWriteChannel or addressWrite or addrWrite? (similar decision for the other channels)

Contributor Checklist

• [ ] Did you add Scaladoc to every public function/method?
• [ ] Did you add at least one test demonstrating the PR?
• [ ] Did you delete any extraneous printlns/debugging code?
• [ ] Did you specify the type of improvement?
• [ ] Did you add appropriate documentation in docs/src?
• [ ] Did you state the API impact?
• [ ] Did you specify the code generation impact?
• [ ] Did you request a desired merge strategy?
• [ ] Did you add text to be included in the Release Notes for this change?

Type of Improvement

API Impact

Backend Code Generation Impact

Desired Merge Strategy

Release Notes

Reviewer Checklist (only modified by reviewer)

• [ ] Did you add the appropriate labels?
• [ ] Did you mark the proper milestone (Bug fix: 3.4.x, [small] API extension: 3.5.x, API modification or big change: 3.6.0)?
• [ ] Did you review?
• [ ] Did you check whether all relevant Contributor checkboxes have been checked?
• [ ] Did you mark as Please Merge?

[jerryz123]

should the name of the bundles end in IO (like in DecoupledIO, QueueIO etc.) or Bundle (as is common in the rocket chip code base) or something else?

I prefer the rocket-chip style, as the bundle doesn't have to be used as an IO, its a bit more intuitive.

should there be separate classes for the read and write address channels? Their fields are essentially the same.

I think these can inherit the same AXI4ABundle class.

aw or addrWriteChannel or addressWriteChannel or addressWrite or addrWrite? (similar decision for the other channels)

Concise naming is easier to read and match with the spec. AXI4AW/AXI4AR/AXI4W/AXI4R/AXI4B.

I think at a minimum, this needs enough functionality to replace the rocket-chip AXI4 implementations.

[ekiwi]

TODO from @jwright6323 : Turns out that the Axi4MemoryType Bundle needs to be different for the read vs. the write direction.

[sequencer]

@ekiwi would you mind cherry-picking chisel.std definition in build systems(sbt mill) firstly, creating another PR, and #2518 can directly use that?

[jiegec]

FWIW, I wrote a Bundle with DataView to convert AXI4 bundle (from rocket chip) to standard names:

// use standard names
class StandardAXI4Bundle(
    val addrBits: Int,
    val dataBits: Int,
    val idBits: Int
) extends Bundle {
  val AWREADY = Input(Bool())
  val AWVALID = Output(Bool())
  val AWID = Output(UInt(idBits.W))
  val AWADDR = Output(UInt(addrBits.W))
  val AWLEN = Output(UInt(8.W))
  val AWSIZE = Output(UInt(3.W))
  val AWBURST = Output(UInt(2.W))
  val AWLOCK = Output(UInt(1.W))
  val AWCACHE = Output(UInt(4.W))
  val AWPROT = Output(UInt(3.W))
  val AWQOS = Output(UInt(4.W))

  val WREADY = Input(Bool())
  val WVALID = Output(Bool())
  val WDATA = Output(UInt(dataBits.W))
  val WSTRB = Output(UInt((dataBits / 8).W))
  val WLAST = Output(Bool())

  val BREADY = Output(Bool())
  val BVALID = Input(Bool())
  val BID = Input(UInt(idBits.W))
  val BRESP = Input(UInt(2.W))

  val ARREADY = Input(Bool())
  val ARVALID = Output(Bool())
  val ARID = Output(UInt(idBits.W))
  val ARADDR = Output(UInt(addrBits.W))
  val ARLEN = Output(UInt(8.W))
  val ARSIZE = Output(UInt(3.W))
  val ARBURST = Output(UInt(2.W))
  val ARLOCK = Output(UInt(1.W))
  val ARCACHE = Output(UInt(4.W))
  val ARPROT = Output(UInt(3.W))
  val ARQOS = Output(UInt(4.W))

  val RREADY = Output(Bool())
  val RVALID = Input(Bool())
  val RID = Input(UInt(idBits.W))
  val RDATA = Input(UInt(dataBits.W))
  val RRESP = Input(UInt(2.W))
  val RLAST = Input(Bool())
}

object StandardAXI4Bundle {
  implicit val axiView = DataView[StandardAXI4Bundle, AXI4Bundle](
    vab =>
      new AXI4Bundle(
        AXI4BundleParameters(vab.addrBits, vab.dataBits, vab.idBits)
      ),
    // AW
    _.AWREADY -> _.aw.ready,
    _.AWVALID -> _.aw.valid,
    _.AWID -> _.aw.bits.id,
    _.AWADDR -> _.aw.bits.addr,
    _.AWLEN -> _.aw.bits.len,
    _.AWSIZE -> _.aw.bits.size,
    _.AWBURST -> _.aw.bits.burst,
    _.AWLOCK -> _.aw.bits.lock,
    _.AWCACHE -> _.aw.bits.cache,
    _.AWPROT -> _.aw.bits.prot,
    _.AWQOS -> _.aw.bits.qos,
    // W
    _.WREADY -> _.w.ready,
    _.WVALID -> _.w.valid,
    _.WDATA -> _.w.bits.data,
    _.WSTRB -> _.w.bits.strb,
    _.WLAST -> _.w.bits.last,
    // B
    _.BREADY -> _.b.ready,
    _.BVALID -> _.b.valid,
    _.BID -> _.b.bits.id,
    _.BRESP -> _.b.bits.resp,
    // AR
    _.ARREADY -> _.ar.ready,
    _.ARVALID -> _.ar.valid,
    _.ARID -> _.ar.bits.id,
    _.ARADDR -> _.ar.bits.addr,
    _.ARLEN -> _.ar.bits.len,
    _.ARSIZE -> _.ar.bits.size,
    _.ARBURST -> _.ar.bits.burst,
    _.ARLOCK -> _.ar.bits.lock,
    _.ARCACHE -> _.ar.bits.cache,
    _.ARPROT -> _.ar.bits.prot,
    _.ARQOS -> _.ar.bits.qos,
    // R
    _.RREADY -> _.r.ready,
    _.RVALID -> _.r.valid,
    _.RID -> _.r.bits.id,
    _.RDATA -> _.r.bits.data,
    _.RRESP -> _.r.bits.resp,
    _.RLAST -> _.r.bits.last
  )
  implicit val axiView2 = StandardAXI4Bundle.axiView.invert(ab =>
    new StandardAXI4Bundle(
      ab.params.addrBits,
      ab.params.dataBits,
      ab.params.idBits
    )
  )
}

I hope it helps!

[ekiwi]

FWIW, I wrote a Bundle with DataView to convert AXI4 bundle (from rocket chip) to standard names:

Cool. Is that code licensed under Apache 2.0 ?

[jiegec]

FWIW, I wrote a Bundle with DataView to convert AXI4 bundle (from rocket chip) to standard names:

Cool. Is that code licensed under Apache 2.0 ?

Sure.

[chiplet]

FWIW here's a library of AXI4-Lite types, some interconnect, and some peripherals: https://gitlab.com/a-core/a-core_chisel/amba/-/tree/master/src/main/scala/axi4l

I agree that it would be a good idea to add the bundle definitions to stdlib since currently everyone seems to be writing their own types.

One thing to consider is how the interface clock and reset signals are defined. IIRC the spec names them ACLK and ARESETn respectively. The spec also states that the reset signal is asynchronous with active-low polarity, which is currently quite painful to implement in Chisel.

Should the bundle definitions for the standard library types aim for full spec compliance, or would it be more useful to take an opinionated approach when it comes to clocks / resets? Using Chisel's implicit clock and reset would simplify implementations but it would also make it more difficult to integrate existing implementations written in e.g. verilog/VHDL that are fully spec compliant.

[ekiwi]

Should the bundle definitions for the standard library types aim for full spec compliance, or would it be more useful to take an opinionated approach when it comes to clocks / resets? Using Chisel's implicit clock and reset would simplify implementations but it would also make it more difficult to integrate existing implementations written in e.g. verilog/VHDL that are fully spec compliant.

I think we should rely on Chisel's implicit clock and reset whenever possible and not artificially limit ourselves. In order to connect to external components, we will provide adapters that ensure that you will get ports that are named according to the AXI standard. I think - as part of this adapter - we can also include some glue logic to expose the clock and reset with the standard names and polarities.

[jwright6323]

Even though ACLK and ARESETn are marked as required, they're frequently omitted or named something else when there are multiple buses on the same clock/reset domain

[chiplet]

I think we should rely on Chisel's implicit clock and reset whenever possible and not artificially limit ourselves.

Now that I think about it, another good reason for going with implicit clocks is that it dramatically simplifies testing from Chisel. IIRC chiseltest only supports implicit clocks and resets for its testers so defining custom clock and reset would force designs to use some other verification framework.

[unlsycn]

It seems to be impossible to convert the Axi4IO to ExternalAxi4IO by using DataView without subword assignment, since bundles such as Axi4MemoryType cannot be viewed as a UInt.