Merge pull request #1238 from freechipsproject/error-bifurcate

Error: don't be an exception wrt. caching

src/main/scala/devices/tilelink/Error.scala

@@ -10,7 +10,7 @@ import freechips.rocketchip.tilelink._
 import freechips.rocketchip.util._
 import scala.math.min
 
-case class ErrorParams(address: Seq[AddressSet], maxAtomic: Int, maxTransfer: Int)
+case class ErrorParams(address: Seq[AddressSet], maxAtomic: Int, maxTransfer: Int, acquire: Boolean = false)
 {
   require (1 <= maxAtomic && maxAtomic <= maxTransfer && maxTransfer <= 4096)
 }
@@ -26,10 +26,10 @@ abstract class DevNullDevice(params: ErrorParams, beatBytes: Int = 4)
     Seq(TLManagerParameters(
       address            = params.address,
       resources          = device.reg("mem"),
-      regionType         = RegionType.UNCACHEABLE,
+      regionType         = if (params.acquire) RegionType.TRACKED else RegionType.UNCACHEABLE,
       executable         = true,
-      supportsAcquireT   = xfer,
-      supportsAcquireB   = xfer,
+      supportsAcquireT   = if (params.acquire) xfer else TransferSizes.none,
+      supportsAcquireB   = if (params.acquire) xfer else TransferSizes.none,
       supportsGet        = xfer,
       supportsPutPartial = xfer,
       supportsPutFull    = xfer,
@@ -52,19 +52,15 @@ class TLError(params: ErrorParams, beatBytes: Int = 4)(implicit p: Parameters)
 
     val (in, edge) = node.in(0)
     val a = Queue(in.a, 1)
-    val c = Queue(in.c, 1)
     val da = Wire(in.d)
-    val dc = Wire(in.d)
 
     val a_last = edge.last(a)
-    val c_last = edge.last(c)
     val da_last = edge.last(da)
-    val dc_last = edge.last(dc)
 
     a.ready := (da.ready && da_last) || !a_last
     da.valid := a.valid && a_last
 
-    val a_opcodes = Vec(AccessAck, AccessAck, AccessAckData, AccessAckData, AccessAckData, HintAck, Grant)
+    val a_opcodes = Vec(AccessAck, AccessAck, AccessAckData, AccessAckData, AccessAckData, HintAck, Grant, Grant)
     da.bits.opcode  := a_opcodes(a.bits.opcode)
     da.bits.param   := UInt(0) // toT, but error grants must be handled transiently (ie: you don't keep permissions)
     da.bits.size    := a.bits.size
@@ -73,6 +69,13 @@ class TLError(params: ErrorParams, beatBytes: Int = 4)(implicit p: Parameters)
     da.bits.data    := UInt(0)
     da.bits.error   := Bool(true)
 
+    if (params.acquire) {
+      val c = Queue(in.c, 1)
+      val dc = Wire(in.d)
+
+      val c_last = edge.last(c)
+      val dc_last = edge.last(dc)
+
       c.ready := (dc.ready && dc_last) || !c_last
       dc.valid := c.valid && c_last
 
@@ -86,8 +89,11 @@ class TLError(params: ErrorParams, beatBytes: Int = 4)(implicit p: Parameters)
 
       // Combine response channels
       TLArbiter.lowest(edge, in.d, dc, da)
+    } else {
+      in.d <> da
+    }
 
-    // We never probe or issue B requests; we are UNCACHED
+    // We never probe or issue B requests
     in.b.valid := Bool(false)
 
     // Sink GrantAcks

src/main/scala/rocket/HellaCache.scala

@@ -163,7 +163,7 @@ abstract class HellaCache(hartid: Int)(implicit p: Parameters) extends LazyModul
       TLClientParameters(
         name          = s"Core ${hartid} DCache",
          sourceId      = IdRange(0, firstMMIO),
-         supportsProbe = TransferSizes(1, cfg.blockBytes)),
+         supportsProbe = TransferSizes(cfg.blockBytes, cfg.blockBytes)),
       TLClientParameters(
         name          = s"Core ${hartid} DCache MMIO",
         sourceId      = IdRange(firstMMIO, firstMMIO+cfg.nMMIOs),

src/main/scala/tilelink/CacheCork.scala

@@ -26,10 +26,10 @@ class TLCacheCork(unsafe: Boolean = false)(implicit p: Parameters) extends LazyM
     (node.in zip node.out) foreach { case ((in, edgeIn), (out, edgeOut)) =>
       val clients = edgeIn.client.clients
       val caches = clients.filter(_.supportsProbe)
-      require (clients.size == 1 || caches.size == 0 || unsafe, "Only one client can safely use a TLCacheCork")
-      require (caches.size <= 1 || unsafe, "Only one caching client allowed")
+      require (clients.size == 1 || caches.size == 0 || unsafe, s"Only one client can safely use a TLCacheCork; ${clients.map(_.name)}")
+      require (caches.size <= 1 || unsafe, s"Only one caching client allowed; ${clients.map(_.name)}")
       edgeOut.manager.managers.foreach { case m =>
-        require (!m.supportsAcquireB || unsafe, "Cannot support caches beyond the Cork")
+        require (!m.supportsAcquireB || unsafe, s"Cannot support caches beyond the Cork; ${m.name}")
         require (m.regionType <= RegionType.UNCACHED)
       }
 
@@ -39,8 +39,15 @@ class TLCacheCork(unsafe: Boolean = false)(implicit p: Parameters) extends LazyM
       // A caveat is that we get Acquire+Release with the same source and must keep the
       // source unique after transformation onto the A channel.
       // The coding scheme is:
-      //   Put: 1, Release: 0 => AccessAck
-      //   *: 0, Acquire: 1 => AccessAckData
+      //   Release, AcquireBlock.BtoT, AcquirePerm => instant response
+      //   Put{Full,Partial}Data: 1, ReleaseData: 0 => AccessAck
+      //   {Arithmetic,Logical}Data,Get: 0, Acquire: 1 => AccessAckData
+      //   Hint:0 => HintAck
+
+      // The CacheCork can potentially send the same source twice if a client sends
+      // simultaneous Release and AMO/Get with the same source. It will still correctly
+      // decode the messages based on the D.opcode, but the double use violates the spec.
+      // Fortunately, no masters we know of behave this way!
 
       // Take requests from A to A or D (if BtoT Acquire)
       val a_a = Wire(out.a)

src/main/scala/tilelink/Parameters.scala

@@ -40,7 +40,7 @@ case class TLManagerParameters(
   require (supportsAcquireB.contains(supportsAcquireT),  s"AcquireB($supportsAcquireB) < AcquireT($supportsAcquireT)")
 
   // Make sure that the regionType agrees with the capabilities
-  require (!supportsAcquireB || regionType >= RegionType.UNCACHEABLE) // acquire -> uncached, tracked, cached
+  require (!supportsAcquireB || regionType >= RegionType.UNCACHED) // acquire -> uncached, tracked, cached
   require (regionType <= RegionType.UNCACHED || supportsAcquireB)  // tracked, cached -> acquire
   require (regionType != RegionType.UNCACHED || supportsGet) // uncached -> supportsGet
 
@@ -186,7 +186,7 @@ case class TLClientParameters(
   name:                String,
   sourceId:            IdRange       = IdRange(0,1),
   nodePath:            Seq[BaseNode] = Seq(),
-  requestFifo:         Boolean       = false, // only a request, not a requirement
+  requestFifo:         Boolean       = false, // only a request, not a requirement. applies to A, not C.
   // Supports both Probe+Grant of these sizes
   supportsProbe:       TransferSizes = TransferSizes.none,
   supportsArithmetic:  TransferSizes = TransferSizes.none,
@@ -204,8 +204,6 @@ case class TLClientParameters(
   require (supportsProbe.contains(supportsPutFull))
   require (supportsProbe.contains(supportsPutPartial))
   require (supportsProbe.contains(supportsHint))
-  // If you need FIFO, you better not be TL-C (due to independent A vs. C order)
-  require (!requestFifo || !supportsProbe)
 
   val maxTransfer = List(
     supportsProbe.max,