From 87bbc63083a884fdc8e7b20469dbe79855e4097d Mon Sep 17 00:00:00 2001 From: Jerry Zhao Date: Mon, 24 Aug 2020 17:51:48 -0700 Subject: [PATCH] Address PR comments --- .../chipyard/src/main/scala/ChipTop.scala | 4 +- .../chipyard/src/main/scala/Clocks.scala | 10 ++--- .../src/main/scala/ConfigFragments.scala | 7 ++-- .../chipyard/src/main/scala/IOBinders.scala | 38 ++++++++++--------- .../chipyard/src/main/scala/TestHarness.scala | 12 +++--- .../src/main/scala/config/RocketConfigs.scala | 5 --- .../firechip/src/main/scala/FireSim.scala | 16 ++++---- vlsi/Makefile | 2 +- 8 files changed, 46 insertions(+), 48 deletions(-) diff --git a/generators/chipyard/src/main/scala/ChipTop.scala b/generators/chipyard/src/main/scala/ChipTop.scala index f7b94d2b53..cf71987bba 100644 --- a/generators/chipyard/src/main/scala/ChipTop.scala +++ b/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. diff --git a/generators/chipyard/src/main/scala/Clocks.scala b/generators/chipyard/src/main/scala/Clocks.scala index 7f181bb5e1..ef038d7d81 100644 --- a/generators/chipyard/src/main/scala/Clocks.scala +++ b/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 }) @@ -166,7 +166,7 @@ object ClockDrivers { } }} - chiptop.harnessFunctions += ((th: HasHarnessUtils) => { + chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => { clock_io := th.harnessClock Nil }) diff --git a/generators/chipyard/src/main/scala/ConfigFragments.scala b/generators/chipyard/src/main/scala/ConfigFragments.scala index 704b849a2b..6336c05aae 100644 --- a/generators/chipyard/src/main/scala/ConfigFragments.scala +++ b/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 }) + diff --git a/generators/chipyard/src/main/scala/IOBinders.scala b/generators/chipyard/src/main/scala/IOBinders.scala index 84ef5269e2..7fce6abaaf 100644 --- a/generators/chipyard/src/main/scala/IOBinders.scala +++ b/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,18 @@ 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 } Nil } @@ -383,11 +383,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 +403,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 +414,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 +427,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))) } }) diff --git a/generators/chipyard/src/main/scala/TestHarness.scala b/generators/chipyard/src/main/scala/TestHarness.scala index 7080278369..b296e328d8 100644 --- a/generators/chipyard/src/main/scala/TestHarness.scala +++ b/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()) }) diff --git a/generators/chipyard/src/main/scala/config/RocketConfigs.scala b/generators/chipyard/src/main/scala/config/RocketConfigs.scala index e05874547d..070336093f 100644 --- a/generators/chipyard/src/main/scala/config/RocketConfigs.scala +++ b/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) diff --git a/generators/firechip/src/main/scala/FireSim.scala b/generators/firechip/src/main/scala/FireSim.scala index e807e840f5..158674a0da 100644 --- a/generators/firechip/src/main/scala/FireSim.scala +++ b/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)) { diff --git a/vlsi/Makefile b/vlsi/Makefile index a9e3d3a581..da9f219359 100644 --- a/vlsi/Makefile +++ b/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)