Address PR comments

generators/chipyard/src/main/scala/ChipTop.scala

@@ -19,7 +19,7 @@ case object BuildSystem extends Field[Parameters => LazyModule]((p: Parameters)
 /**
  * The base class used for building chips. This constructor instantiates a module specified by the BuildSystem parameter,
  * named "system", which is an instance of DigitalTop by default. The diplomatic clocks of System, as well as its implicit clock,
- * is aggregated into the clockGroupNode. The parameterized functions controlled by ChipyardClockKey and GlobalResetSchemeKey
+ * is aggregated into the clockGroupNode. The parameterized functions controlled by ClockingSchemeKey and GlobalResetSchemeKey
  * drive clock and reset generation
  */
 
@@ -36,7 +36,7 @@ class ChipTop(implicit p: Parameters) extends LazyModule with HasTestHarnessFunc
   val implicitClockSinkNode = ClockSinkNode(Seq(ClockSinkParameters()))
 
   // Generate Clocks and Reset
-  p(ChipyardClockKey)(this)
+  p(ClockingSchemeKey)(this)
 
   // NOTE: Making this a LazyRawModule is moderately dangerous, as anonymous children
   // of ChipTop (ex: ClockGroup) do not receive clock or reset.

generators/chipyard/src/main/scala/Clocks.scala

@@ -69,7 +69,7 @@ object GenerateReset {
     }
     reset_io.suggestName("reset")
     chiptop.iocells ++= resetIOCell
-    chiptop.harnessFunctions += ((th: HasHarnessUtils) => {
+    chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
       reset_io := th.dutReset
       Nil
     })
@@ -78,11 +78,11 @@ object GenerateReset {
 }
 
 
-case object ChipyardClockKey extends Field[ChipTop => Unit](ClockDrivers.harnessClock)
+case object ClockingSchemeKey extends Field[ChipTop => Unit](ClockingSchemeGenerators.harnessClock)
 
 
 
-object ClockDrivers {
+object ClockingSchemeGenerators {
   // A simple clock provider, for testing
   val harnessClock: ChipTop => Unit = { chiptop =>
     implicit val p = chiptop.p
@@ -120,7 +120,7 @@ object ClockDrivers {
         }
       }}
 
-      chiptop.harnessFunctions += ((th: HasHarnessUtils) => {
+      chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
         clock_io := th.harnessClock
         Nil
       })
@@ -132,6 +132,8 @@ object ClockDrivers {
   val harnessDividedClock: ChipTop => Unit = { chiptop =>
     implicit val p = chiptop.p
 
+    require(false, "Divided clock is broken until we fix passing onchip clocks to TestHarness objects")
+
     val implicitClockSourceNode = ClockSourceNode(Seq(ClockSourceParameters()))
     chiptop.implicitClockSinkNode := implicitClockSourceNode
 
@@ -166,7 +168,7 @@ object ClockDrivers {
         }
       }}
 
-      chiptop.harnessFunctions += ((th: HasHarnessUtils) => {
+      chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
         clock_io := th.harnessClock
         Nil
       })

generators/chipyard/src/main/scala/ConfigFragments.scala

@@ -26,7 +26,7 @@ import sifive.blocks.devices.gpio._
 import sifive.blocks.devices.uart._
 import sifive.blocks.devices.spi._
 
-import chipyard.{BuildTop, BuildSystem, ClockDrivers, ChipyardClockKey, TestSuitesKey, TestSuiteHelper}
+import chipyard.{BuildTop, BuildSystem, ClockingSchemeGenerators, ClockingSchemeKey, TestSuitesKey, TestSuiteHelper}
 
 
 // -----------------------
@@ -98,7 +98,7 @@ class WithMultiRoCCHwacha(harts: Int*) extends Config(
     case MultiRoCCKey => {
       up(MultiRoCCKey, site) ++ harts.distinct.map{ i =>
         (i -> Seq((p: Parameters) => {
-          val hwacha = LazyModule(new Hwacha()(p)).suggestName("hwacha")
+          val hwacha = LazyModule(new Hwacha()(p))
           hwacha
         }))
       }
@@ -160,5 +160,6 @@ class WithNoSubsystemDrivenClocks extends Config((site, here, up) => {
 })
 
 class WithTileDividedClock extends Config((site, here, up) => {
-  case ChipyardClockKey => ClockDrivers.harnessDividedClock
+  case ClockingSchemeKey => ClockingSchemeGenerators.harnessDividedClock
 })
+

generators/chipyard/src/main/scala/IOBinders.scala

@@ -41,7 +41,7 @@ import scala.reflect.{ClassTag}
 
 // DOC include start: IOBinders
 // This type describes a function callable on the TestHarness instance. Its return type is unused.
-type TestHarnessFunction = (chipyard.HasHarnessUtils) => Seq[Any]
+type TestHarnessFunction = (chipyard.HasHarnessSignalReferences) => Seq[Any]
 // IOBinders will return a Seq of this tuple, which contains three fields:
 //  1. A Seq containing all IO ports created by the IOBinder function
 //  2. A Seq containing all IO cell modules created by the IOBinder function
@@ -228,7 +228,7 @@ object AddIOCells {
 class WithGPIOTiedOff extends OverrideIOBinder({
   (system: HasPeripheryGPIOModuleImp) => {
     val (ports2d, ioCells2d) = AddIOCells.gpio(system.gpio)
-    val harnessFn = (th: HasHarnessUtils) => { ports2d.flatten.foreach(_ <> AnalogConst(0)); Nil }
+    val harnessFn = (th: HasHarnessSignalReferences) => { ports2d.flatten.foreach(_ <> AnalogConst(0)); Nil }
     Seq((ports2d.flatten, ioCells2d.flatten, Some(harnessFn)))
   }
 })
@@ -237,7 +237,7 @@ class WithGPIOTiedOff extends OverrideIOBinder({
 class WithUARTAdapter extends OverrideIOBinder({
   (system: HasPeripheryUARTModuleImp) => {
     val (ports, ioCells2d) = AddIOCells.uart(system.uart)
-    val harnessFn = (th: HasHarnessUtils) => { UARTAdapter.connect(ports)(system.p); Nil }
+    val harnessFn = (th: HasHarnessSignalReferences) => { UARTAdapter.connect(ports)(system.p); Nil }
     Seq((ports, ioCells2d.flatten, Some(harnessFn)))
   }
 })
@@ -245,7 +245,7 @@ class WithUARTAdapter extends OverrideIOBinder({
 class WithSimSPIFlashModel(rdOnly: Boolean = true) extends OverrideIOBinder({
   (system: HasPeripherySPIFlashModuleImp) => {
     val (ports, ioCells2d) = AddIOCells.spi(system.qspi, "qspi")
-    val harnessFn = (th: HasHarnessUtils) => { SimSPIFlashModel.connect(ports, th.harnessReset, rdOnly)(system.p); Nil }
+    val harnessFn = (th: HasHarnessSignalReferences) => { SimSPIFlashModel.connect(ports, th.harnessReset, rdOnly)(system.p); Nil }
     Seq((ports, ioCells2d.flatten, Some(harnessFn)))
   }
 })
@@ -253,7 +253,7 @@ class WithSimSPIFlashModel(rdOnly: Boolean = true) extends OverrideIOBinder({
 class WithSimBlockDevice extends OverrideIOBinder({
   (system: CanHavePeripheryBlockDeviceModuleImp) => system.bdev.map { bdev =>
     val (port, ios) = AddIOCells.blockDev(bdev)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       // TODO: Using harness clock/reset will be incorrect when systemClock =/= harnessClock
       SimBlockDevice.connect(th.harnessClock, th.harnessReset.asBool, Some(port))(system.p)
       Nil
@@ -265,7 +265,7 @@ class WithSimBlockDevice extends OverrideIOBinder({
 class WithBlockDeviceModel extends OverrideIOBinder({
   (system: CanHavePeripheryBlockDeviceModuleImp) => system.bdev.map { bdev =>
     val (port, ios) = AddIOCells.blockDev(bdev)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       BlockDeviceModel.connect(Some(port))(system.p)
       Nil
     }
@@ -288,7 +288,7 @@ class WithSimAXIMem extends OverrideIOBinder({
     val peiTuples = AddIOCells.axi4(system.mem_axi4, system.memAXI4Node, "mem")
     // TODO: we are inlining the connectMem method of SimAXIMem because
     //   it takes in a dut rather than seq of axi4 ports
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       peiTuples.map { case (port, edge, ios) =>
         val mem = LazyModule(new SimAXIMem(edge, size = p(ExtMem).get.master.size))
         Module(mem.module).suggestName("mem")
@@ -305,7 +305,7 @@ class WithBlackBoxSimMem extends OverrideIOBinder({
   (system: CanHaveMasterAXI4MemPort) => {
     implicit val p: Parameters = GetSystemParameters(system)
     val peiTuples = AddIOCells.axi4(system.mem_axi4, system.memAXI4Node, "mem")
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       peiTuples.map { case (port, edge, ios) =>
         val memSize = p(ExtMem).get.master.size
         val lineSize = p(CacheBlockBytes)
@@ -325,7 +325,7 @@ class WithSimAXIMMIO extends OverrideIOBinder({
   (system: CanHaveMasterAXI4MMIOPort) => {
     implicit val p: Parameters = GetSystemParameters(system)
     val peiTuples = AddIOCells.axi4(system.mmio_axi4, system.mmioAXI4Node, "mmio_mem")
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       peiTuples.zipWithIndex.map { case ((port, edge, ios), i) =>
         val mmio_mem = LazyModule(new SimAXIMem(edge, size = 4096))
         Module(mmio_mem.module).suggestName(s"mmio_mem_${i}")
@@ -345,7 +345,7 @@ class WithTieOffInterrupts extends OverrideIOBinder({
   (system: HasExtInterruptsModuleImp) => {
     val (port, ioCells) = IOCell.generateIOFromSignal(system.interrupts, Some("iocell_interrupts"))
     port.suggestName("interrupts")
-    val harnessFn = (th: HasHarnessUtils) => { port := 0.U; Nil }
+    val harnessFn = (th: HasHarnessSignalReferences) => { port := 0.U; Nil }
     Seq((Seq(port), ioCells, Some(harnessFn)))
   }
 })
@@ -353,7 +353,7 @@ class WithTieOffInterrupts extends OverrideIOBinder({
 class WithTieOffL2FBusAXI extends OverrideIOBinder({
   (system: CanHaveSlaveAXI4Port) => {
     val peiTuples = AddIOCells.axi4(system.l2_frontend_bus_axi4, system.l2FrontendAXI4Node, "l2_fbus")
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       peiTuples.zipWithIndex.map { case ((port, edge, ios), i) =>
         port := DontCare // tieoff doesn't completely tie-off, for some reason
         port.tieoff()
@@ -364,18 +364,19 @@ class WithTieOffL2FBusAXI extends OverrideIOBinder({
   }
 })
 
+// TODO we need to rethink what "Tie-off-debug" means. The current system punches out
+// excessive IOs.
 class WithTiedOffDebug extends OverrideIOBinder({
   (system: HasPeripheryDebugModuleImp) => {
     val (psdPort, resetctrlOpt, debugPortOpt, ioCells) =
       AddIOCells.debug(system.psd, system.resetctrl, system.debug)(system.p)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       Debug.tieoffDebug(debugPortOpt, resetctrlOpt, Some(psdPort))(system.p)
       // tieoffDebug doesn't actually tie everything off :/
       debugPortOpt.foreach { d =>
-        // TODO: Using harness clock/reset will be incorrect when systemClock =/= harnessClock
         d.clockeddmi.foreach({ cdmi => cdmi.dmi.req.bits := DontCare; cdmi.dmiClock := th.harnessClock })
         d.dmactiveAck := DontCare
-        d.clock := th.harnessClock
+        d.clock := th.harnessClock // TODO fix: This should be driven from within the chip
       }
       Nil
     }
@@ -383,11 +384,13 @@ class WithTiedOffDebug extends OverrideIOBinder({
   }
 })
 
+// TODO we need to rethink what this does. The current system punches out excessive IOs.
+// Some of the debug clock/reset should be driven from on-chip
 class WithSimDebug extends OverrideIOBinder({
   (system: HasPeripheryDebugModuleImp) => {
     val (psdPort, resetctrlPortOpt, debugPortOpt, ioCells) =
       AddIOCells.debug(system.psd, system.resetctrl, system.debug)(system.p)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       val dtm_success = Wire(Bool())
       Debug.connectDebug(debugPortOpt, resetctrlPortOpt, psdPort, th.harnessClock, th.harnessReset.asBool, dtm_success)(system.p)
       when (dtm_success) { th.success := true.B }
@@ -401,7 +404,7 @@ class WithSimDebug extends OverrideIOBinder({
 class WithTiedOffSerial extends OverrideIOBinder({
   (system: CanHavePeripherySerialModuleImp) => system.serial.map({ serial =>
     val (port, ioCells) = AddIOCells.serial(serial)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       SerialAdapter.tieoff(port)
       Nil
     }
@@ -412,7 +415,7 @@ class WithTiedOffSerial extends OverrideIOBinder({
 class WithSimSerial extends OverrideIOBinder({
   (system: CanHavePeripherySerialModuleImp) => system.serial.map({ serial =>
     val (port, ioCells) = AddIOCells.serial(serial)
-    val harnessFn = (th: HasHarnessUtils) => {
+    val harnessFn = (th: HasHarnessSignalReferences) => {
       val ser_success = SerialAdapter.connectSimSerial(port, th.harnessClock, th.harnessReset)
       when (ser_success) { th.success := true.B }
       Nil
@@ -425,7 +428,7 @@ class WithTraceGenSuccessBinder extends OverrideIOBinder({
   (system: TraceGenSystemModuleImp) => {
     val (successPort, ioCells) = IOCell.generateIOFromSignal(system.success, Some("iocell_success"))
     successPort.suggestName("success")
-    val harnessFn = (th: HasHarnessUtils) => { when (successPort) { th.success := true.B }; Nil }
+    val harnessFn = (th: HasHarnessSignalReferences) => { when (successPort) { th.success := true.B }; Nil }
     Seq((Seq(successPort), ioCells, Some(harnessFn)))
   }
 })

generators/chipyard/src/main/scala/TestHarness.scala

@@ -16,14 +16,14 @@ trait HasTestHarnessFunctions {
   val harnessFunctions: Seq[TestHarnessFunction]
 }
 
-trait HasHarnessUtils {
-  val harnessClock: Clock
-  val harnessReset: Reset
-  val dutReset: Reset
-  val success: Bool
+trait HasHarnessSignalReferences {
+  def harnessClock: Clock
+  def harnessReset: Reset
+  def dutReset: Reset
+  def success: Bool
 }
 
-class TestHarness(implicit val p: Parameters) extends Module with HasHarnessUtils {
+class TestHarness(implicit val p: Parameters) extends Module with HasHarnessSignalReferences {
   val io = IO(new Bundle {
     val success = Output(Bool())
   })

generators/chipyard/src/main/scala/config/RocketConfigs.scala

@@ -194,8 +194,3 @@ class DividedClockRocketConfig extends Config(
   new chipyard.config.AbstractConfig)
 
 
-class TestClockRocketConfig extends Config(
-  //new chipyard.config.WithTileMultiClock ++                       // Put the Tile on its own clock domain
-  new freechips.rocketchip.subsystem.WithAsynchronousRocketTiles(8, 3) ++   // Add rational crossings between RocketTile and uncore
-  new freechips.rocketchip.subsystem.WithNBigCores(1) ++
-  new chipyard.config.AbstractConfig)

generators/firechip/src/main/scala/FireSim.scala

@@ -13,7 +13,7 @@ import freechips.rocketchip.util.{ResetCatchAndSync}
 
 import midas.widgets.{Bridge, PeekPokeBridge, RationalClockBridge, RationalClock}
 
-import chipyard.{BuildSystem, BuildTop, HasHarnessUtils, ChipyardSubsystem, ChipyardClockKey, ChipTop}
+import chipyard.{BuildSystem, BuildTop, HasHarnessSignalReferences, ChipyardSubsystem, ClockingSchemeKey, ChipTop}
 import chipyard.iobinders.{IOBinders}
 
 // Determines the number of times to instantiate the DUT in the harness.
@@ -43,7 +43,7 @@ object NodeIdx {
 }
 
 class WithFireSimSimpleClocks extends Config((site, here, up) => {
-  case ChipyardClockKey => { chiptop: ChipTop =>
+  case ClockingSchemeKey => { chiptop: ChipTop =>
     implicit val p = chiptop.p
 
     val implicitClockSourceNode = ClockSourceNode(Seq(ClockSourceParameters()))
@@ -75,7 +75,7 @@ class WithFireSimSimpleClocks extends Config((site, here, up) => {
         }
       }}
 
-      chiptop.harnessFunctions += ((th: HasHarnessUtils) => {
+      chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
         clock := th.harnessClock
         reset := th.harnessReset
         Nil
@@ -86,7 +86,7 @@ class WithFireSimSimpleClocks extends Config((site, here, up) => {
 
 class WithFireSimRationalTileDomain(multiplier: Int, divisor: Int) extends Config((site, here, up) => {
   case FireSimClockKey => FireSimClockParameters(Seq(RationalClock("TileDomain", multiplier, divisor)))
-  case ChipyardClockKey => { chiptop: ChipTop =>
+  case ClockingSchemeKey => { chiptop: ChipTop =>
     implicit val p = chiptop.p
 
     val implicitClockSourceNode = ClockSourceNode(Seq(ClockSourceParameters()))
@@ -125,7 +125,7 @@ class WithFireSimRationalTileDomain(multiplier: Int, divisor: Int) extends Confi
         }
       }}
 
-      chiptop.harnessFunctions += ((th: HasHarnessUtils) => {
+      chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
         uncore_clock := th.harnessClock
         reset        := th.harnessReset
         th match {
@@ -138,15 +138,15 @@ class WithFireSimRationalTileDomain(multiplier: Int, divisor: Int) extends Confi
   }
 })
 
-class FireSim(implicit val p: Parameters) extends RawModule with HasHarnessUtils {
+class FireSim(implicit val p: Parameters) extends RawModule with HasHarnessSignalReferences {
   freechips.rocketchip.util.property.cover.setPropLib(new midas.passes.FireSimPropertyLibrary())
   val clockBridge = Module(new RationalClockBridge(p(FireSimClockKey).additionalClocks:_*))
   val harnessClock = clockBridge.io.clocks.head // This is the reference clock
   val additionalClocks = clockBridge.io.clocks.tail
   val harnessReset = WireInit(false.B)
   val peekPokeBridge = PeekPokeBridge(harnessClock, harnessReset)
-  val dutReset = false.B // unused (if used, its a bug)
-  val success = false.B // unused (if used, its a bug)
+  def dutReset = { require(false, "dutReset should not be used in Firesim"); false.B }
+  def success = { require(false, "success should not be used in Firesim"); false.B }
 
   // Instantiate multiple instances of the DUT to implement supernode
   for (i <- 0 until p(NumNodes)) {

vlsi/Makefile

@@ -34,7 +34,7 @@ INPUT_CONFS        ?= $(if $(filter $(tech_name),nangate45),\
                         example-asap7.yml)
 HAMMER_EXEC        ?= example-vlsi
 VLSI_TOP           ?= $(TOP)
-VLSI_HARNESS_DUT_NAME ?= dut
+VLSI_HARNESS_DUT_NAME ?= chiptop
 VLSI_OBJ_DIR       ?= $(vlsi_dir)/build
 ifneq ($(CUSTOM_VLOG),)
 	OBJ_DIR          ?= $(VLSI_OBJ_DIR)/custom-$(VLSI_TOP)