fix(DataStorage): Fix req_valid and Add MCP2 Check Assertions (#254)

* TMP: add HoldChecker to ensure MCP2 Hold condition

* misc: an official version of MCP2 hold check

* misc: add check right at predecessor regs' output

* misc: remove checks that are no longer needed

* DataStorage: use LSB instead of orR for req_valid
Since task_s3_valid_hold2(1) will change during the two cycles,
      we just use (0) as req_valid

* misc: add check for task_valid_s3_hold2(0)

* misc: fix wrong assertion

src/main/scala/coupledL2/CoupledL2.scala

@@ -79,6 +79,7 @@ trait HasCoupledL2Parameters {
   def hasPrefetchSrc = prefetchers.exists(_.hasPrefetchSrc)
   def hasCMO = cacheParams.hasCMO
   def topDownOpt = if(cacheParams.elaboratedTopDown) Some(true) else None
+  def hasMCP2Check = cacheParams.MCP2Check
 
   def enableHintGuidedGrant = true
 

src/main/scala/coupledL2/Directory.scala

@@ -113,6 +113,7 @@ class Directory(implicit p: Parameters) extends L2Module {
     val replResp = ValidIO(new ReplacerResult)
     // used to count occWays for Grant to retry
     val msInfo = Vec(mshrsAll, Flipped(ValidIO(new MSHRInfo)))
+    val mcp2Check = if(hasMCP2Check) Some(Input(new MCP2CheckEn)) else None
   })
 
   def invalid_way_sel(metaVec: Seq[MetaEntry], repl: UInt) = {
@@ -373,4 +374,20 @@ class Directory(implicit p: Parameters) extends L2Module {
 
   XSPerfAccumulate("dirRead_cnt", io.read.fire)
   XSPerfAccumulate("choose_busy_way", reqValid_s3 && !req_s3.wayMask(chosenWay))
+
+  // ===== for MCP2 hold check =====
+  if (hasMCP2Check) {
+    // TODO: it now relies on physical design analysis to tell us which regs to check hold
+    // We may use chisel methods to distinguish all predecessor registers
+    // of dataStorage inputs in the future
+    val en = io.mcp2Check.get.en
+    HoldChecker.check2(io.resp.bits, en, "dirResp_s3")
+    HoldChecker.check2(io.replResp.bits, en, "replResp_s3")
+
+    HoldChecker.check2(freeWayMask_s3, en, "freeWayMask_s3")
+    HoldChecker.check2(metaAll_s3, en, "metaAll_s3")
+    HoldChecker.check2(repl_state_s3, en, "repl_state_s3")
+    HoldChecker.check2(req_s3, en, "req_s3")
+    HoldChecker.check2(tagAll_s3, en, "tagAll_s3")
+  }
 }

src/main/scala/coupledL2/L2Param.scala

@@ -106,6 +106,8 @@ case class L2Param(
   FPGAPlatform: Boolean = false,
   // CMO
   hasCMO: Boolean = false,
+  // has DataStorage MCP2 Check
+  MCP2Check: Boolean = true,
 
   // Network layer SAM
   sam: Seq[(AddressSet, Int)] = Seq(AddressSet.everything -> 0)

src/main/scala/coupledL2/MSHRBuffer.scala

@@ -42,6 +42,7 @@ class MSHRBuffer(wPorts: Int = 1)(implicit p: Parameters) extends L2Module {
     val r = Flipped(ValidIO(new MSHRBufRead))
     val resp = new MSHRBufResp
     val w = Vec(wPorts, Flipped(ValidIO(new MSHRBufWrite)))
+    val mcp2Check = if(hasMCP2Check) Some(Input(new MCP2CheckEn)) else None
   })
 
   val buffer = Reg(Vec(mshrsAll, Vec(beatSize, UInt((beatBytes * 8).W))))
@@ -63,6 +64,11 @@ class MSHRBuffer(wPorts: Int = 1)(implicit p: Parameters) extends L2Module {
 
   val rdata = buffer(io.r.bits.id).asUInt
   io.resp.data.data := RegEnable(rdata, 0.U.asTypeOf(rdata), io.r.valid)
+
+  if (hasMCP2Check) {
+    assert(!io.r.valid || !RegNext(io.r.valid), "No continuous read")
+    HoldChecker.check2(io.resp.data.data, io.mcp2Check.get.wen, "mshrBuf_wdata")
+  }
 }
 
 // may consider just choose an empty entry to insert

src/main/scala/coupledL2/SinkC.scala

@@ -23,6 +23,7 @@ import freechips.rocketchip.tilelink._
 import freechips.rocketchip.tilelink.TLMessages._
 import org.chipsalliance.cde.config.Parameters
 import utility.{MemReqSource, XSPerfAccumulate, RRArbiterInit}
+import coupledL2.utils._
 
 class PipeBufferResp(implicit p: Parameters) extends L2Bundle {
   val data = Vec(beatSize, UInt((beatBytes * 8).W))
@@ -41,6 +42,7 @@ class SinkC(implicit p: Parameters) extends L2Module {
     val bufResp = Output(new PipeBufferResp)
     val refillBufWrite = ValidIO(new MSHRBufWrite)
     val msInfo = Vec(mshrsAll, Flipped(ValidIO(new MSHRInfo)))
+    val mcp2Check = if(hasMCP2Check) Some(Input(new MCP2CheckEn)) else None
   })
 
   val (first, last, _, beat) = edgeIn.count(io.c)
@@ -196,4 +198,9 @@ class SinkC(implicit p: Parameters) extends L2Module {
   XSPerfAccumulate("NewDataNestC", io.refillBufWrite.valid)
   //!!WARNING: TODO: if this is zero, that means fucntion [Release-new-data written into refillBuf]
   // is never tested, and may have flaws
+  // ===== for MCP2 hold check =====
+  if (hasMCP2Check) {
+    assert(!io.task.fire || !RegNext(io.task.fire), "No continuous write @SinkC")
+    HoldChecker.check2(io.bufResp.data, io.mcp2Check.get.wen, "sinkC_wdata")
+  }
 }

src/main/scala/coupledL2/tl2chi/MainPipe.scala

@@ -25,6 +25,7 @@ import freechips.rocketchip.tilelink.TLMessages._
 import freechips.rocketchip.tilelink.TLPermissions._
 import org.chipsalliance.cde.config.Parameters
 import coupledL2._
+import coupledL2.utils.HoldChecker
 import coupledL2.prefetch.{PrefetchTrain, PfSource}
 import coupledL2.tl2chi.CHICohStates._
 import coupledL2.MetaData._
@@ -396,7 +397,7 @@ class MainPipe(implicit p: Parameters) extends TL2CHIL2Module with HasCHIOpcodes
   }
 
   io.toDS.en_s3 := task_s3.valid && (ren || wen)
-  io.toDS.req_s3.valid := task_s3_valid_hold2.orR && (ren || wen)
+  io.toDS.req_s3.valid := task_s3_valid_hold2(0) && (ren || wen)
   io.toDS.req_s3.bits.way := Mux(
     mshr_refill_s3 && req_s3.replTask,
     io.replResp.bits.way,
@@ -414,6 +415,11 @@ class MainPipe(implicit p: Parameters) extends TL2CHIL2Module with HasCHIOpcodes
     )
   )
 
+  if (hasMCP2Check) {
+    HoldChecker.check2(task_s3.bits, io.toDS.en_s3, "task_s3_bits")
+    HoldChecker.check2(task_s3_valid_hold2(0), io.toDS.en_s3, "task_s3_valid_hold2_0")
+  }
+
   /* ======== Read DS and store data in Buffer ======== */
   // A: need_write_releaseBuf indicates that DS should be read and the data will be written into ReleaseBuffer
   //    need_write_releaseBuf is assigned true when:

src/main/scala/coupledL2/tl2chi/Slice.scala

@@ -22,6 +22,7 @@ import chisel3.util._
 import freechips.rocketchip.tilelink._
 import org.chipsalliance.cde.config.Parameters
 import coupledL2._
+import coupledL2.utils._
 import coupledL2.prefetch.PrefetchIO
 
 class OuterBundle(implicit p: Parameters) extends DecoupledPortIO with BaseOuterBundle
@@ -209,4 +210,15 @@ class Slice()(implicit p: Parameters) extends BaseSlice[OuterBundle]
   rxrsp.io.out <> io.out.rx.rsp
 
   io_pCrd <> mshrCtl.io.pCrd
+
+  if (hasMCP2Check) {
+    val dsEnable = WireInit(0.U.asTypeOf(new MCP2CheckEn))
+    dsEnable.en := dataStorage.io.en
+    dsEnable.wen := dataStorage.io.en && dataStorage.io.req.bits.wen
+
+    directory.io.mcp2Check.get := dsEnable
+    releaseBuf.io.mcp2Check.get := dsEnable
+    refillBuf.io.mcp2Check.get := dsEnable
+    sinkC.io.mcp2Check.get := dsEnable
+  }
 }

src/main/scala/coupledL2/tl2tl/MainPipe.scala

@@ -299,7 +299,7 @@ class MainPipe(implicit p: Parameters) extends L2Module {
   }
 
   io.toDS.en_s3           := task_s3.valid && (ren || wen)
-  io.toDS.req_s3.valid    := task_s3_valid_hold2.orR && (ren || wen)
+  io.toDS.req_s3.valid    := task_s3_valid_hold2(0) && (ren || wen)
   io.toDS.req_s3.bits.way := Mux(mshr_refill_s3 && req_s3.replTask, io.replResp.bits.way,
     Mux(mshr_req_s3, req_s3.way, dirResult_s3.way))
   io.toDS.req_s3.bits.set := Mux(mshr_req_s3, req_s3.set, dirResult_s3.set)
@@ -314,6 +314,11 @@ class MainPipe(implicit p: Parameters) extends L2Module {
     )
   )
 
+  if (hasMCP2Check) {
+    HoldChecker.check2(task_s3.bits, io.toDS.en_s3, "task_s3_bits")
+    HoldChecker.check2(task_s3_valid_hold2(0), io.toDS.en_s3, "task_s3_valid_hold2_0")
+  }
+
   /* ======== Read DS and store data in Buffer ======== */
   // A: need_write_releaseBuf indicates that DS should be read and the data will be written into ReleaseBuffer
   //    need_write_releaseBuf is assigned true when:

src/main/scala/coupledL2/tl2tl/Slice.scala

@@ -205,4 +205,15 @@ class Slice()(implicit p: Parameters) extends BaseSlice[OuterBundle] {
 
   val monitor = Module(new Monitor())
   monitor.io.fromMainPipe <> mainPipe.io.toMonitor
+
+  if (hasMCP2Check) {
+    val dsEnable = WireInit(0.U.asTypeOf(new MCP2CheckEn))
+    dsEnable.en := dataStorage.io.en
+    dsEnable.wen := dataStorage.io.en && dataStorage.io.req.bits.wen
+
+    directory.io.mcp2Check.get := dsEnable
+    releaseBuf.io.mcp2Check.get := dsEnable
+    refillBuf.io.mcp2Check.get := dsEnable
+    sinkC.io.mcp2Check.get := dsEnable
+  }
 }

src/main/scala/coupledL2/utils/HoldChecker.scala

@@ -0,0 +1,59 @@
+/** *************************************************************************************
+ * Copyright (c) 2020-2021 Institute of Computing Technology, Chinese Academy of Sciences
+ * Copyright (c) 2020-2021 Peng Cheng Laboratory
+ *
+ * XiangShan is licensed under Mulan PSL v2.
+ * You can use this software according to the terms and conditions of the Mulan PSL v2.
+ * You may obtain a copy of Mulan PSL v2 at:
+ * http://license.coscl.org.cn/MulanPSL2
+ *
+ * THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND,
+ * EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT,
+ * MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE.
+ *
+ * See the Mulan PSL v2 for more details.
+ * *************************************************************************************
+ */
+
+package coupledL2.utils
+
+import chisel3._
+import chisel3.util._
+
+// enable signals, only used to check mcp2 hold condition of predecessor regs
+class MCP2CheckEn extends Bundle {
+  val en = Bool()
+  val wen = Bool()
+}
+
+/**
+ * Assert the signal must hold for certain cycles when enable is high
+ */
+object HoldChecker {
+  /**
+   * signal holds at en and the next cycle
+   */
+  def check2(signal: Data, en: Bool, name: String): Unit = {
+    // at the 2nd cycle, signal changes
+    assert(!(RegNext(en) && (signal.asUInt =/= RegNext(signal).asUInt)),
+      s"signal changed at $name, fails to hold for 2 cycles")
+  }
+
+  /**
+   * signal holds for N cycles
+   */
+  def apply(signal: Data, en: Bool, cycles: Int, name: String): Unit = {
+    val counter = RegInit(0.U(log2Ceil(cycles).W))
+    val data = RegEnable(signal, 0.U.asTypeOf(signal), en)
+
+    when(en) {
+      counter := cycles.U - 1.U
+    }.otherwise {
+      counter := Mux(counter === 0.U, 0.U, counter - 1.U)
+    }
+
+    assert((counter =/= 0.U) && (signal.asUInt =/= data.asUInt),
+      s"Signal should hold for $cycles cycles, but it fails")
+  }
+}
+