regmapper: eliminate race condition in RegisterCrossing bypass

src/main/scala/regmapper/RegisterCrossing.scala

@@ -7,20 +7,32 @@ import chisel3.util.{Irrevocable}
 import util.{AsyncQueue,AsyncResetRegVec}
 
 // A very simple flow control state machine, run in the specified clock domain
-class BusyRegisterCrossing(clock: Clock, reset: Bool)
+class BusyRegisterCrossing extends Module {
-    extends Module(_clock = clock, _reset = reset) {
   val io = new Bundle {
-    val progress       = Bool(INPUT)
+    val bypass                  = Bool(INPUT)
-    val request_valid  = Bool(INPUT)
+    val master_request_valid    = Bool(INPUT)
-    val response_ready = Bool(INPUT)
+    val master_request_ready    = Bool(OUTPUT)
-    val busy           = Bool(OUTPUT)
+    val master_response_valid   = Bool(OUTPUT)
+    val master_response_ready   = Bool(INPUT)
+    val crossing_request_valid  = Bool(OUTPUT)
+    val crossing_request_ready  = Bool(INPUT)
+    // ... no crossing_response_ready; we are always ready
   }
 
   val busy   = RegInit(Bool(false))
-  when (io.progress) {
+  val bypass = Reg(Bool())
-    busy := Mux(busy, !io.response_ready, io.request_valid)
+
+  when (io.crossing_request_ready || Mux(busy, bypass, io.bypass)) {
+    busy := Mux(busy, !io.master_response_ready, io.master_request_valid)
   }
-  io.busy := busy
+
+  when (io.master_request_valid && io.master_request_ready) {
+    bypass := io.bypass
+  }
+
+  io.crossing_request_valid := io.master_request_valid && !io.bypass && !busy
+  io.master_request_ready   := (io.bypass || io.crossing_request_ready) && !busy
+  io.master_response_valid  := (bypass    || io.crossing_request_ready) &&  busy
 }
 
 // RegField should support connecting to one of these
@@ -29,25 +41,36 @@ class RegisterWriteIO[T <: Data](gen: T) extends Bundle {
   val response = Irrevocable(Bool()) // ignore .bits
 }
 
-// To turn on/off a domain:
+// To turn off=>on a domain:
-//  1. lower allow on the other side
+//  A. To turn disable the master domain
+//    1. wait for all inflight traffic to resolve
+//    2. assert master reset
+//    3. (optional) stop the master clock
+//    --- YOU MAY NOT TURN OFF POWER ---
+//    4. re-enable the clock
+//    5. deassert reset
+//  B. To turn off the slave domain
+//    1. assert bypass
 //    2. wait for inflight traffic to resolve
-//  3. assert reset in the domain
+//    3. assert slave reset
-//  4. turn off the domain
+//    4. (optional) stop the slave clock
-//  5. turn on the domain
+//    --- YOU MAY NOT TURN OFF POWER ---
-//  6. deassert reset in the domain
+//    5. re-enable the clock
-//  7. raise allow on the other side
+//    6. deassert reset
+//    7. deassert bypass
+//
+// If you need to cut power, use something that support isolation gates.
 
 class RegisterWriteCrossingIO[T <: Data](gen: T) extends Bundle {
   // Master clock domain
   val master_clock   = Clock(INPUT)
   val master_reset   = Bool(INPUT)
-  val master_allow   = Bool(INPUT) // actually wait for the slave
   val master_port    = new RegisterWriteIO(gen)
+  // Bypass requests from the master to be noops
+  val master_bypass  = Bool(INPUT)
   // Slave clock domain
   val slave_clock    = Clock(INPUT)
   val slave_reset    = Bool(INPUT)
-  val slave_allow    = Bool(INPUT) // honour requests from the master
   val slave_register = gen.asOutput
   val slave_valid    = Bool(OUTPUT) // is high on 1st cycle slave_register has a new value
 }
@@ -55,32 +78,29 @@ class RegisterWriteCrossingIO[T <: Data](gen: T) extends Bundle {
 class RegisterWriteCrossing[T <: Data](gen: T, sync: Int = 3) extends Module {
   val io = new RegisterWriteCrossingIO(gen)
   // The crossing must only allow one item inflight at a time
+  val control = Module(new BusyRegisterCrossing)
   val crossing = Module(new AsyncQueue(gen, 1, sync))
 
-  // We can just randomly reset one-side of a single entry AsyncQueue.
+  control.clock := io.master_clock
-  // If the enq side is reset, at worst deq.bits is reassigned from mem(0), which stays fixed.
+  control.reset := io.master_reset
-  // If the deq side is reset, at worst the master rewrites mem(0) once, deq.bits stays fixed.
   crossing.io.enq_clock := io.master_clock
   crossing.io.enq_reset := io.master_reset
   crossing.io.deq_clock := io.slave_clock
   crossing.io.deq_reset := io.slave_reset
 
+  control.io.bypass := io.master_bypass
+  control.io.master_request_valid  := io.master_port.request.valid
+  control.io.master_response_ready := io.master_port.response.ready
+  io.master_port.request.ready  := control.io.master_request_ready
+  io.master_port.response.valid := control.io.master_response_valid
+
+  control.io.crossing_request_ready := crossing.io.enq.ready
+  crossing.io.enq.valid := control.io.crossing_request_valid
   crossing.io.enq.bits := io.master_port.request.bits
-  io.slave_register := crossing.io.deq.bits
-  io.slave_valid    := crossing.io.deq.valid
-
-  // If the slave is not operational, just drop the write.
-  val progress = crossing.io.enq.ready || !io.master_allow
-
-  val reg = Module(new BusyRegisterCrossing(io.master_clock, io.master_reset))
-  reg.io.progress := progress
-  reg.io.request_valid  := io.master_port.request.valid
-  reg.io.response_ready := io.master_port.response.ready
 
   crossing.io.deq.ready := Bool(true)
-  crossing.io.enq.valid := io.master_port.request.valid && !reg.io.busy
+  io.slave_valid := crossing.io.deq.valid
-  io.master_port.request.ready  := progress && !reg.io.busy
+  io.slave_register := crossing.io.deq.bits
-  io.master_port.response.valid := progress && reg.io.busy
 }
 
 // RegField should support connecting to one of these
@@ -93,43 +113,40 @@ class RegisterReadCrossingIO[T <: Data](gen: T) extends Bundle {
   // Master clock domain
   val master_clock   = Clock(INPUT)
   val master_reset   = Bool(INPUT)
-  val master_allow   = Bool(INPUT) // actually wait for the slave
   val master_port    = new RegisterReadIO(gen)
+  // Bypass requests from the master to be noops
+  val master_bypass  = Bool(INPUT)
   // Slave clock domain
   val slave_clock    = Clock(INPUT)
   val slave_reset    = Bool(INPUT)
-  val slave_allow    = Bool(INPUT) // honour requests from the master
   val slave_register = gen.asInput
 }
 
 class RegisterReadCrossing[T <: Data](gen: T, sync: Int = 3) extends Module {
   val io = new RegisterReadCrossingIO(gen)
   // The crossing must only allow one item inflight at a time
+  val control = Module(new BusyRegisterCrossing)
   val crossing = Module(new AsyncQueue(gen, 1, sync))
 
-  // We can just randomly reset one-side of a single entry AsyncQueue.
+  control.clock := io.master_clock
-  // If the enq side is reset, at worst deq.bits is reassigned from mem(0), which stays fixed.
+  control.reset := io.master_reset
-  // If the deq side is reset, at worst the slave rewrites mem(0) once, deq.bits stays fixed.
-  crossing.io.enq_clock := io.slave_clock
-  crossing.io.enq_reset := io.slave_reset
   crossing.io.deq_clock := io.master_clock
   crossing.io.deq_reset := io.master_reset
+  crossing.io.enq_clock := io.slave_clock
+  crossing.io.enq_reset := io.slave_reset
 
-  crossing.io.enq.bits := io.slave_register
+  control.io.bypass := io.master_bypass
+  control.io.master_request_valid  := io.master_port.request.valid
+  control.io.master_response_ready := io.master_port.response.ready
+  io.master_port.request.ready  := control.io.master_request_ready
+  io.master_port.response.valid := control.io.master_response_valid
+
+  control.io.crossing_request_ready := crossing.io.deq.valid
+  crossing.io.deq.ready := control.io.crossing_request_valid
   io.master_port.response.bits := crossing.io.deq.bits
 
-  // If the slave is not operational, just repeat the last value we saw.
-  val progress = crossing.io.deq.valid || !io.master_allow
-
-  val reg = Module(new BusyRegisterCrossing(io.master_clock, io.master_reset))
-  reg.io.progress := progress
-  reg.io.request_valid  := io.master_port.request.valid
-  reg.io.response_ready := io.master_port.response.ready
-
-  io.master_port.response.valid := progress && reg.io.busy
-  io.master_port.request.ready  := progress && !reg.io.busy
-  crossing.io.deq.ready := io.master_port.request.valid && !reg.io.busy
   crossing.io.enq.valid := Bool(true)
+  crossing.io.enq.bits := io.slave_register
 }
 
 /** Wrapper to create an
@@ -151,8 +168,7 @@ object AsyncRWSlaveRegField {
     width:  Int,
     init: Int,
     name: Option[String] = None,
-    master_allow: Bool = Bool(true),
+    master_bypass: Bool = Bool(true)
-    slave_allow: Bool = Bool(true)
   ): (UInt, RegField) = {
 
     val async_slave_reg = Module(new AsyncResetRegVec(width, init))
@@ -165,10 +181,9 @@ object AsyncRWSlaveRegField {
 
     wr_crossing.io.master_clock  := master_clock
     wr_crossing.io.master_reset  := master_reset
-    wr_crossing.io.master_allow := master_allow
+    wr_crossing.io.master_bypass := master_bypass
     wr_crossing.io.slave_clock   := slave_clock
     wr_crossing.io.slave_reset   := slave_reset
-    wr_crossing.io.slave_allow  := slave_allow
 
     async_slave_reg.io.en := wr_crossing.io.slave_valid
     async_slave_reg.io.d  := wr_crossing.io.slave_register
@@ -178,10 +193,9 @@ object AsyncRWSlaveRegField {
 
     rd_crossing.io.master_clock  := master_clock
     rd_crossing.io.master_reset  := master_reset
-    rd_crossing.io.master_allow := master_allow
+    rd_crossing.io.master_bypass := master_bypass
     rd_crossing.io.slave_clock   := slave_clock
     rd_crossing.io.slave_reset   := slave_reset
-    rd_crossing.io.slave_allow  := slave_allow
 
     rd_crossing.io.slave_register := async_slave_reg.io.q
 

src/main/scala/uncore/tilelink2/RegisterRouterTest.scala

@@ -231,18 +231,16 @@ trait RRTest1Module extends Module with HasRegMap
     val readCross = Module(new RegisterReadCrossing(field))
     readCross.io.master_clock  := clock
     readCross.io.master_reset  := reset
-    readCross.io.master_allow := Bool(true)
+    readCross.io.master_bypass := Bool(false)
     readCross.io.slave_clock   := clocks.io.clock_out
     readCross.io.slave_reset   := reset
-    readCross.io.slave_allow := Bool(true)
 
     val writeCross = Module(new RegisterWriteCrossing(field))
     writeCross.io.master_clock  := clock
     writeCross.io.master_reset  := reset
-    writeCross.io.master_allow := Bool(true)
+    writeCross.io.master_bypass := Bool(false)
     writeCross.io.slave_clock   := clocks.io.clock_out
     writeCross.io.slave_reset   := reset
-    writeCross.io.slave_allow := Bool(true)
 
     readCross.io.slave_register := writeCross.io.slave_register
     RegField(bits, readCross.io.master_port, writeCross.io.master_port)