From 0c013a56c03e25871f48e0b97a5d35082c9828b7 Mon Sep 17 00:00:00 2001
From: Megan Wachs <megan@sifive.com>
Date: Wed, 19 Apr 2017 13:17:22 -0700
Subject: [PATCH] debug: Make DMI NOPs really NOPs.

This simplifies SW design and CDC issues.
---
 .../scala/rocketchip/DebugTransport.scala     | 41 ++++++++++++-------
 .../scala/uncore/devices/debug/Debug.scala    | 16 +++-----
 2 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/src/main/scala/rocketchip/DebugTransport.scala b/src/main/scala/rocketchip/DebugTransport.scala
index 349336f3..3dc81e5b 100644
--- a/src/main/scala/rocketchip/DebugTransport.scala
+++ b/src/main/scala/rocketchip/DebugTransport.scala
@@ -83,8 +83,11 @@ class DebugTransportModuleJTAG(debugAddrBits: Int, c: JtagDTMConfig)
   val busy = Wire(Bool())
   val nonzeroResp = Wire(Bool())
 
-  val busyResp = Wire(new DMIAccessCapture(debugAddrBits))
+  val busyResp    = Wire(new DMIAccessCapture(debugAddrBits))
   val nonbusyResp = Wire(new DMIAccessCapture(debugAddrBits))
+  val dmiResp     = Wire(new DMIAccessCapture(debugAddrBits))
+  val nopResp     = Wire(new DMIAccessCapture(debugAddrBits))
+
 
   val dmiReqReg  = Reg(new DMIReq(debugAddrBits))
   val dmiReqValidReg = Reg(init = Bool(false));
@@ -156,20 +159,29 @@ class DebugTransportModuleJTAG(debugAddrBits: Int, c: JtagDTMConfig)
   // Especially for the first request, we must consider dtmResp.valid,
   // so that we don't consider junk in the FIFO to be an error response.
   // The current specification says that any non-zero response is an error.
+  // But there is actually no case in the current design where you SHOULD get an error,
+  // as we haven't implemented Bus Masters or Serial Ports, which are the only cases errors
+  // can occur.
   nonzeroResp := stickyNonzeroRespReg | (io.dmi.resp.valid & (io.dmi.resp.bits.resp != UInt(0)))
+  assert(!nonzeroResp, "There is no reason to get a non zero response in the current system.");
+  assert(!stickyNonzeroRespReg, "There is no reason to have a sticky non zero response in the current system.");
 
   busyResp.addr  := UInt(0)
-  busyResp.resp  := -1.S(DMIConsts.dmiRespSize.W).asUInt // Generalizing busy to 'all-F'
+  busyResp.resp  := Fill(DMIConsts.dmiRespSize, 1.U) // Generalizing busy to 'all-F'
   busyResp.data  := UInt(0)
 
-  nonbusyResp.addr := dmiReqReg.addr
-  nonbusyResp.resp := io.dmi.resp.bits.resp
-  nonbusyResp.data := io.dmi.resp.bits.data
+  dmiResp.addr := dmiReqReg.addr
+  dmiResp.resp := io.dmi.resp.bits.resp
+  dmiResp.data := io.dmi.resp.bits.data
+
+  nopResp.addr := UInt(0)
+  nopResp.resp := UInt(0)
+  nopResp.data := UInt(0)
 
   //--------------------------------------------------------
   // Debug Access Chain Implementation
 
-  dmiAccessChain.io.capture.bits := Mux(busy, busyResp, nonbusyResp)
+  dmiAccessChain.io.capture.bits := Mux(busy, busyResp, Mux(io.dmi.resp.valid, dmiResp, nopResp))
   when (dmiAccessChain.io.update.valid) {
     skipOpReg := Bool(false)
     downgradeOpReg := Bool(false)
@@ -177,24 +189,25 @@ class DebugTransportModuleJTAG(debugAddrBits: Int, c: JtagDTMConfig)
   when (dmiAccessChain.io.capture.capture) {
     skipOpReg := busy
     downgradeOpReg := (!busy & nonzeroResp)
-      stickyBusyReg := busy
+    stickyBusyReg := busy
     stickyNonzeroRespReg := nonzeroResp
   }
 
   //--------------------------------------------------------
   // Drive Ready Valid Interface
 
+  assert(!(dmiAccessChain.io.update.valid && io.dmi.req.ready), "Conflicting updates for dmiReqValid, should not happen.");
+
   when (dmiAccessChain.io.update.valid) {
     when (skipOpReg) {
       // Do Nothing
+    }.elsewhen(downgradeOpReg || (dmiAccessChain.io.update.bits.op === DMIConsts.dmi_OP_NONE)) {
+      // Do Nothing
+      dmiReqReg.addr := UInt(0)
+      dmiReqReg.data := UInt(0)
+      dmiReqReg.op   := UInt(0)
     }.otherwise {
-      when (downgradeOpReg) {
-        dmiReqReg.addr := UInt(0)
-        dmiReqReg.data := UInt(0)
-        dmiReqReg.op   := UInt(0)
-      }.otherwise {
-        dmiReqReg := dmiAccessChain.io.update.bits
-      }
+      dmiReqReg := dmiAccessChain.io.update.bits
       dmiReqValidReg := Bool(true)
     }
   }.otherwise {
diff --git a/src/main/scala/uncore/devices/debug/Debug.scala b/src/main/scala/uncore/devices/debug/Debug.scala
index 100649f4..c7b9767b 100644
--- a/src/main/scala/uncore/devices/debug/Debug.scala
+++ b/src/main/scala/uncore/devices/debug/Debug.scala
@@ -1097,19 +1097,13 @@ class DMIToTL(implicit p: Parameters) extends LazyModule {
     val (_,  gbits) = edge.Get(src, addr, size)
     val (_, pfbits) = edge.Put(src, addr, size, io.dmi.req.bits.data)
 
-    // Note we force DMI NOPs to go to CONTROL register because
-    // Inner  may be in reset / not have a clock,
-    // so we force address to be the one that goes to Outer.
-    // Besides, for a NOP we don't really need to pay the penalty to go
-    // across the CDC.
-
-    val (_, nbits)  = edge.Put(src, toAddress = (DMI_RegAddrs.DMI_DMCONTROL << 2).U, size, data=0.U, mask = 0.U)
-
-    when (io.dmi.req.bits.op === DMIConsts.dmi_OP_WRITE)       { tl.a.bits := pfbits
-    }.elsewhen  (io.dmi.req.bits.op === DMIConsts.dmi_OP_READ) { tl.a.bits := gbits
-    }.otherwise {                                                tl.a.bits := nbits
+    when (io.dmi.req.fire()) {
+      assert (io.dmi.req.bits.op === DMIConsts.dmi_OP_WRITE || io.dmi.req.bits.op === DMIConsts.dmi_OP_READ,
+        "Invalid DMI Request Type.");
     }
 
+    tl.a.bits := Mux(io.dmi.req.bits.op === DMIConsts.dmi_OP_WRITE, pfbits, gbits);
+
     tl.a.valid       := io.dmi.req.valid
     io.dmi.req.ready := tl.a.ready