refactor: add CHI-related code from coupledl2 by Yan-Yiming · Pull Request #144 · OpenXiangShan/Utility

Yan-Yiming · 2026-04-09T04:50:41Z

No description provided.

Copilot

Pull request overview

This PR imports/refactors a CHI (Coherent Hub Interface) utility layer (ported from coupledl2) into utility.chi, introducing CHI message definitions, opcode constants, link/network helpers, async bridging, and a CHI traffic logger.

Changes:

Add CHI flit/bundle definitions and per-issue parameterization (Message.scala) plus opcode constants/helpers (Opcode.scala).
Add link-layer interfaces and L-Credit ↔ Decoupled converters (LinkLayer.scala) and a basic System Address Map helper (NetworkLayer.scala).
Add CHI async bridge utilities/modules and a CHI logger module (AsyncBridge.scala, CHILogger.scala).

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
src/main/scala/utility/chi/Opcode.scala	Defines CHI opcode constants and opcode/issue-gating helpers.
src/main/scala/utility/chi/NetworkLayer.scala	Adds a simple System Address Map (SAM) lookup/check helper.
src/main/scala/utility/chi/Message.scala	Introduces CHI issue/config parameters, encodings, and CHI flit Bundle definitions.
src/main/scala/utility/chi/LinkLayer.scala	Adds ChannelIO/PortIO and L-Credit ↔ Decoupled conversion logic.
src/main/scala/utility/chi/CHILogger.scala	Adds a CHI traffic logger that records flits/fields into ChiselDB.
src/main/scala/utility/chi/AsyncBridge.scala	Adds async bridge bundles and source/sink modules for crossing clock domains.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T05:02:33Z

src/main/scala/utility/chi/Message.scala

+  def SC = "b001".U(width.W)
+  def UC = "b010".U(width.W)
+  def UD = "b010".U(width.W)
+  def SD = "b011".U(width.W)
+


In CHICohStates, UC and UD are both encoded as "b010", which makes UniqueClean vs UniqueDirty (and their *_PD variants) indistinguishable. If these are intended to be distinct CHI response encodings, this will break state tracking/validation; please fix the encoding or add a clarifying comment and remove the redundant alias if intentional.

Copilot · 2026-04-09T05:02:34Z

src/main/scala/utility/chi/NetworkLayer.scala

+class SAM(sam: Seq[(AddressSet, Int)]) {
+  def check(x: UInt): Bool = Cat(sam.map(_._1.contains(x))).orR
+
+  def lookup(x: UInt): UInt = {
+    ParallelPriorityMux(sam.map(m => (m._1.contains(x), m._2.U)))
+  }


SAM.check builds Cat(sam.map(...)); if sam is empty this will throw during elaboration. Either require(sam.nonEmpty) in the constructor or define an empty-SAM behavior (e.g. false.B for check, 0.U/DontCare for lookup).

Copilot · 2026-04-09T05:02:34Z

src/main/scala/utility/chi/LinkLayer.scala

+  val state = io.state.state
+  val disableFlit = state === LinkStates.STOP || state === LinkStates.ACTIVATE
+  val disableLCredit = state === LinkStates.STOP
+  val acceptLCredit = out.lcrdv && !disableLCredit


acceptLCredit is computed from out.lcrdv, but out is a locally-created wire that is never driven from io.out.lcrdv. This will make the credit pool update use an uninitialized signal. Use io.out.lcrdv directly (or explicitly connect it into the local wire) so L-Credit accounting is correct.

Suggested change

val acceptLCredit = out.lcrdv && !disableLCredit

val acceptLCredit = io.out.lcrdv && !disableLCredit

Copilot · 2026-04-09T05:02:34Z

src/main/scala/utility/chi/LinkLayer.scala

+  io.in.ready := lcreditPool =/= 0.U && !disableFlit
+
+  io.out <> out
+  out.flitpend := RegNext(true.B, init = false.B) // TODO
+  out.flitv := RegNext(flitv, init = false.B)
+  out.flit := RegEnable(Mux(io.in.valid, Cat(io.in.bits.getElements.map(_.asUInt)), 0.U /* LCrdReturn */), flitv)


io.out <> out bulk-connects two ChannelIO instances with directional fields. This pattern is very likely to produce illegal Output<>Output/Input<>Input connections (and also obscures which signals are actually driven). Prefer directly driving io.out.flitpend/flitv/flit and reading io.out.lcrdv, or create a directionless wire via Wire(chiselTypeOf(io.out)) if you really need an intermediate.

Copilot · 2026-04-09T05:02:34Z

src/main/scala/utility/chi/LinkLayer.scala

+  assert(lcreditInflight + lcreditPool === lcreditNum.U)
+
+  val ready = lcreditInflight =/= 0.U
+  val accept = ready && io.in.flitv


In non-blocking mode, accept is defined as ready && io.in.flitv and does not consider io.out.ready. When io.out.ready is low, the module can still "accept" an input flit (and potentially return credits) even though the decoupled consumer cannot take the data, which risks dropping flits / credit mismatches. Gate acceptance on io.out.ready (or add buffering) when blocking=false.

Suggested change

val accept = ready && io.in.flitv

val accept = ready && io.in.flitv && io.out.ready

Copilot · 2026-04-09T05:02:35Z

src/main/scala/utility/chi/AsyncBridge.scala

+  io.enq.rx.rsp <> FromAsyncBundle.channel(io.async.rx.rsp.flit, params, Some("rxrsp_flit"))
+  io.enq.rx.dat <> FromAsyncBundle.channel(io.async.rx.dat.flit, params, Some("rxdat_flit"))
+  io.enq.rx.snp <> FromAsyncBundle.channel(io.async.rx.snp.flit, params, Some("rxsnp_flit"))
+
+  io.async.rx.rsp.lcrdv <> ToAsyncBundle.bitPulse(io.enq.rx.rsp.lcrdv, params, Some("rxrsp_lcrdv"))
+  io.async.rx.dat.lcrdv <> ToAsyncBundle.bitPulse(io.enq.rx.dat.lcrdv, params, Some("rxdat_lcrdv"))
+  io.async.rx.snp.lcrdv <> ToAsyncBundle.bitPulse(io.enq.rx.snp.lcrdv, params, Some("rxsnp_lcrdv"))


FromAsyncBundle.channel returns a ChannelIO with flitpend/lcrdv driven to DontCare, but the callers bulk-connect the entire ChannelIO (e.g. io.enq.rx.rsp <> FromAsyncBundle.channel(...)) and also drive flitpend elsewhere. This is very likely to cause direction/connect conflicts (Output<>Output) and/or multiple drivers. Suggest returning a purpose-specific bundle (e.g. Valid(UInt)), or connect only flitv/flit fields explicitly instead of <> on the whole ChannelIO.

Copilot · 2026-04-09T05:02:35Z

src/main/scala/utility/chi/AsyncBridge.scala

+  io.deq.tx.req <> FromAsyncBundle.channel(io.async.tx.req.flit, params, Some("txreq_flit"), Some(txreq_lcrdvReady))
+  io.deq.tx.rsp <> FromAsyncBundle.channel(io.async.tx.rsp.flit, params, Some("txrsp_flit"), Some(txrsp_lcrdvReady))
+  io.deq.tx.dat <> FromAsyncBundle.channel(io.async.tx.dat.flit, params, Some("txdat_flit"), Some(txdat_lcrdvReady))


Similar to the source side, io.deq.tx.{req,rsp,dat} <> FromAsyncBundle.channel(...) bulk-connects two similarly-directed ChannelIOs (both drive flitv/flit/flitpend). This is likely an illegal connection and also ignores the fact that only flitv/flit should come from the async sink. Please wire the individual signals explicitly (and keep lcrdv handled separately).

Suggested change

io.deq.tx.req <> FromAsyncBundle.channel(io.async.tx.req.flit, params, Some("txreq_flit"), Some(txreq_lcrdvReady))

io.deq.tx.rsp <> FromAsyncBundle.channel(io.async.tx.rsp.flit, params, Some("txrsp_flit"), Some(txrsp_lcrdvReady))

io.deq.tx.dat <> FromAsyncBundle.channel(io.async.tx.dat.flit, params, Some("txdat_flit"), Some(txdat_lcrdvReady))

val async_tx_req = FromAsyncBundle.channel(io.async.tx.req.flit, params, Some("txreq_flit"), Some(txreq_lcrdvReady))

val async_tx_rsp = FromAsyncBundle.channel(io.async.tx.rsp.flit, params, Some("txrsp_flit"), Some(txrsp_lcrdvReady))

val async_tx_dat = FromAsyncBundle.channel(io.async.tx.dat.flit, params, Some("txdat_flit"), Some(txdat_lcrdvReady))

io.deq.tx.req.flitv := async_tx_req.flitv

io.deq.tx.req.flit := async_tx_req.flit

io.deq.tx.req.flitpend := false.B

io.deq.tx.rsp.flitv := async_tx_rsp.flitv

io.deq.tx.rsp.flit := async_tx_rsp.flit

io.deq.tx.rsp.flitpend := false.B

io.deq.tx.dat.flitv := async_tx_dat.flitv

io.deq.tx.dat.flit := async_tx_dat.flit

io.deq.tx.dat.flitpend := false.B

Copilot · 2026-04-09T05:02:35Z

src/main/scala/utility/chi/AsyncBridge.scala

+    val shadow_buffer = Module(new Queue(chiselTypeOf(chn.flit), 16, flow = true, pipe = false))
+    if (name.isDefined) { shadow_buffer.suggestName("shadowBuffer_" + name.get) }
+    shadow_buffer.io.enq.valid := chn.flitv
+    shadow_buffer.io.enq.bits  := chn.flit
+    /*
+     2. For rx channel (CMN->L2), send out lcrdv right after a flit entering Shadow buffer if has space
+     */
+    val deqReady = shadow_buffer.io.deq.ready
+    dontTouch(deqReady)
+    assert(!chn.flitv || shadow_buffer.io.enq.ready, s"${name.getOrElse("ToAsyncBundle")}: Shadow buffer overflow!")
+    /*
+     3. AsyncQueueSource (depth=4)
+     */
+    val source = Module(new AsyncQueueSource(chiselTypeOf(chn.flit), params))
+    if (name.isDefined) { source.suggestName("asyncQSource_" + name.get) }
+    source.io.enq <> shadow_buffer.io.deq
+
+    (source.io.async, deqReady)
+  }


ToAsyncBundleWithBuf.channel returns shadow_buffer.io.deq.ready as the "ready" signal. If this is intended to indicate shadow-buffer space for instant L-Credit return, it should be based on shadow_buffer.io.enq.ready (space to accept a new flit), not dequeue readiness. Using deq.ready here can cause credits to be returned even when the buffer is full.

Copilot · 2026-04-09T05:02:35Z

src/main/scala/utility/chi/AsyncBridge.scala

+  io.deq.rx.rsp.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.rsp.lcrdv, params, Some("rxrsp_lcrdv"))
+  io.deq.rx.dat.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.dat.lcrdv, params, Some("rxdat_lcrdv"))


io.deq.rx.{rsp,dat}.lcrdv is already driven from the async side (FromAsyncBundle.bitPulse(...)), but later the module also instantiates LCredit2Decoupled(io.deq.rx.{rsp,dat}, ...) which will try to manage/drive L-Credits for the same channels. This creates conflicting ownership of lcrdv and risks double-driving / inconsistent credit behavior; please choose a single L-Credit source and remove the other path.

Suggested change

io.deq.rx.rsp.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.rsp.lcrdv, params, Some("rxrsp_lcrdv"))

io.deq.rx.dat.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.dat.lcrdv, params, Some("rxdat_lcrdv"))

// `io.deq.rx.rsp.lcrdv` and `io.deq.rx.dat.lcrdv` are owned by the

// downstream LCredit2Decoupled(...) path. Do not also drive them from the

// async bridge, or the channels will have conflicting L-Credit sources.

Copilot · 2026-04-09T05:02:36Z

src/main/scala/utility/chi/CHILogger.scala

+class CHILogger(name: String, enable: Boolean)
+               (implicit val p: Parameters) extends Module with HasCHIOpcodes {
+
+  val io = IO(new Bundle() {
+    val up = Flipped(new PortIO)
+    val down = new PortIO
+  })
+  io.down <> io.up
+


The enable constructor parameter is currently unused, so the logger always allocates large tracking structures and always calls table.log(...) regardless of enable. Either remove the parameter or gate the logging/addr-tracking logic (or conditionally instantiate the module in CHILogger.apply) so enable=false has an effect.

Yan-Yiming added 2 commits April 1, 2026 16:19

feat: add CHI-related code

969de36

fix(chi): correct CHI-related dependencies and imports

340c28d

Yan-Yiming requested review from Copilot and linjuanZ April 9, 2026 04:53

Copilot started reviewing on behalf of Yan-Yiming April 9, 2026 04:54 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: add CHI-related code from coupledl2#144

refactor: add CHI-related code from coupledl2#144
Yan-Yiming wants to merge 2 commits intoOpenXiangShan:masterfrom
Yan-Yiming:l2-llc-merge-0402

Yan-Yiming commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Copilot AI Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	val acceptLCredit = out.lcrdv && !disableLCredit
	val acceptLCredit = io.out.lcrdv && !disableLCredit

	val accept = ready && io.in.flitv
	val accept = ready && io.in.flitv && io.out.ready

-  io.deq.tx.req <> FromAsyncBundle.channel(io.async.tx.req.flit, params, Some("txreq_flit"), Some(txreq_lcrdvReady))
-  io.deq.tx.rsp <> FromAsyncBundle.channel(io.async.tx.rsp.flit, params, Some("txrsp_flit"), Some(txrsp_lcrdvReady))
-  io.deq.tx.dat <> FromAsyncBundle.channel(io.async.tx.dat.flit, params, Some("txdat_flit"), Some(txdat_lcrdvReady))
+  val async_tx_req = FromAsyncBundle.channel(io.async.tx.req.flit, params, Some("txreq_flit"), Some(txreq_lcrdvReady))
+  val async_tx_rsp = FromAsyncBundle.channel(io.async.tx.rsp.flit, params, Some("txrsp_flit"), Some(txrsp_lcrdvReady))
+  val async_tx_dat = FromAsyncBundle.channel(io.async.tx.dat.flit, params, Some("txdat_flit"), Some(txdat_lcrdvReady))
+  io.deq.tx.req.flitv := async_tx_req.flitv
+  io.deq.tx.req.flit := async_tx_req.flit
+  io.deq.tx.req.flitpend := false.B
+  io.deq.tx.rsp.flitv := async_tx_rsp.flitv
+  io.deq.tx.rsp.flit := async_tx_rsp.flit
+  io.deq.tx.rsp.flitpend := false.B
+  io.deq.tx.dat.flitv := async_tx_dat.flitv
+  io.deq.tx.dat.flit := async_tx_dat.flit
+  io.deq.tx.dat.flitpend := false.B

		io.deq.rx.rsp.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.rsp.lcrdv, params, Some("rxrsp_lcrdv"))
		io.deq.rx.dat.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.dat.lcrdv, params, Some("rxdat_lcrdv"))

-  io.deq.rx.rsp.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.rsp.lcrdv, params, Some("rxrsp_lcrdv"))
-  io.deq.rx.dat.lcrdv <> FromAsyncBundle.bitPulse(io.async.rx.dat.lcrdv, params, Some("rxdat_lcrdv"))
+  // `io.deq.rx.rsp.lcrdv` and `io.deq.rx.dat.lcrdv` are owned by the
+  // downstream LCredit2Decoupled(...) path. Do not also drive them from the
+  // async bridge, or the channels will have conflicting L-Credit sources.

Conversation

Yan-Yiming commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants