Support for Multiple Reset Schemes (for some parts of the codebase) #2375

mwachs5 · 2020-03-28T07:15:13Z

~~Currently this PR includes parts of #2205, which I would like to be merged first and then I will rebase this on top of that~~ (note, this has been done). ~~This PR will be rebased often before merging if necessary and commit history will be cleaned up.~~ (this has also been done)

This PR includes/replaces #2237 and #2217

Related issue:

Type of change: feature request

Impact: API modification

Development Phase: implementation

Release Notes

Adds support for multiple reset schemes for much of the code base (some parts of the code base would still need to be converted to chisel3 or use the DefaultCompileOptions.NonStrictInferReset capability to infer resets, as previously this code base only allowed Bool resets. Notably, async reset of the Rocket Core/Tile is not supported.

Important changes to note:
Adds parameterization scheme for reset types (SubsystemResetSchemeKey)
Adds LazyModule withClock, withClockAndReset, withReset
Changes significantly the exported I/Os for debug related logic
Removes internal synchronization logic for debug reset-- this must be done externally now

More detailed release notes from #2237:

This PR converts the debug module to the new Configurable Reset Scheme using chisel3 abstract reset.

There is now a 1:1 relationship required between resets and clocks (except for rational crossing clock/reset pairs). dmOuter uses dmi_clock and dmi_reset and dmInner uses new inputs debug_clock and debug_reset. A portion of SBA is in the TileLink clock / reset domain.
debug_clock must be synchronous to clock. The clock gate formerly in dmInnerAsync now resides outside the debug module in customer logic. User logic can call connectDebugClockAndReset to achieve the same functionality as before, in a different level of the hierarchy.
A new input dmactiveAck is returned from customer logic and is used to indicate when dmInner is able to accept DMI transactions (i.e. debug_clock is running and debug_reset is negated). When dmactiveAck is negated, a bus blocker returns denied for transactions to dmInner. The current version of OpenOCD is tolerant of this blocker, but other software may need to be updated.
dmi_reset must be asynchronous. debug_reset may be either synchronous or asynchronous but its deassertion must always occur either when debug_clock is stopped or synchronously to debug_clock.
The APB and JTAG interface conforms to the above and now require reset synchronization.
hart-reset and halt-on-reset functions have been changed to simple I/O and require customer logic involvement. The two signals are grouped in a new ResetCtrlIO bundle. hartIsInReset is a logical equivalent of core_reset and is synchronous to core_clock. For debug module halt-on-reset function, this signal must remain asserted for at least 4 debug_clock cycles for the DM to assert debug interrupt plus 3 core_clock cycles for the debug interrupt to reach the core prior to the core_reset signal being deasserted. Halt-on-reset will not work if either clock or core_clock is not running.
The clock and reset signals formerly in TracedInstruction have been removed since a trace module attached would not be allowed to use them because of the 1:1 rule.

src/main/scala/tilelink/Arbiter.scala

ernie-sifive

This looks good to me.

src/main/scala/util/AsyncQueue.scala

src/main/scala/tilelink/Arbiter.scala

mwachs5 · 2020-03-29T12:23:34Z

This is failing because fesvr writes dmactive =1, then immediately tries to do another operation and gets an error response. This is due to the synchronization on dmActiveAck which adds a delay before the blocker is released. Another rocket-tools bump to fesvr 😭 ? or other ideas @ernie-sifive ?

mwachs5 · 2020-03-29T13:49:40Z

@ernie-sifive I filed FESVR issue (riscv-software-src/riscv-isa-sim#435) and added a workaround. But I am confused why the dmactiveAck needs to be synchronized in the testbench anyway. Isn't it already synchronized here:

https://github.com/chipsalliance/rocket-chip/pull/2375/files#diff-579e415dc2dfe635c9d572021aeeccf8R657

ernie-sifive · 2020-03-29T13:57:42Z

@mwachs5 dmactive is generated in dmi_clock domain, get synced to clock in the testbench (where dmactiveAck is used to control the clock gate), then dmactiveAck returns to dmOuter where it is synced back to dmi_clock. If you remove the sync to clock then won't the clock gate misbehave? Ah, I see this workaround is only for simulation...

mwachs5 · 2020-03-29T14:15:21Z

src/main/scala/diplomacy/LazyModule.scala

@@ -198,7 +199,7 @@ class LazyRawModuleImp(val wrapper: LazyModule) extends RawModule with LazyModul
  // It is recommended to drive these even if you manually shove most of your children
  // Otherwise, anonymous children (Monitors for example) will not be clocked
  val childClock = Wire(Clock())
-  val childReset = Wire(Bool())
+  val childReset = Wire(Reset())


src/main/scala/devices/debug/Periphery.scala

mwachs5 · 2020-03-30T12:29:03Z

Am going to do a cleanup rebase now

Add IO() required by chisel3 Revert chisel3 to Chisel for now

… chisel3 reset type

…g assigned to Bool

Review comments

…th no clock edges to avoid violating tile link protocol. Also use chiselName to give predictable naming within withClockAndReset blocks

Arbiter: fix parenthesis Arbiter more typo fixes arbiter. srsly don't try to write actual code throug github GUI interface

bumping to have proper fesvr handling

mwachs5 · 2020-04-03T02:29:46Z

closing to reopen because travis seems borked

mwachs5 requested review from hcook, aswaterman, jackkoenig and ernie-sifive March 28, 2020 07:15

mwachs5 commented Mar 28, 2020

View reviewed changes

src/main/scala/tilelink/Arbiter.scala Outdated Show resolved Hide resolved

ernie-sifive approved these changes Mar 28, 2020

View reviewed changes

mwachs5 commented Mar 28, 2020

View reviewed changes

src/main/scala/util/AsyncQueue.scala Show resolved Hide resolved

mwachs5 commented Mar 28, 2020

View reviewed changes

src/main/scala/util/AsyncQueue.scala Show resolved Hide resolved

mwachs5 commented Mar 28, 2020

View reviewed changes

src/main/scala/util/AsyncQueue.scala Show resolved Hide resolved

mwachs5 mentioned this pull request Mar 28, 2020

Convert Debug Module to abstract reset #2237

Closed

mwachs5 force-pushed the multiple-reset-schemes-rebased branch from 13a73f7 to fd171ba Compare March 28, 2020 21:45

mwachs5 commented Mar 29, 2020

View reviewed changes

src/main/scala/tilelink/Arbiter.scala Outdated Show resolved Hide resolved

mwachs5 commented Mar 29, 2020

View reviewed changes

src/main/scala/tilelink/Arbiter.scala Outdated Show resolved Hide resolved

mwachs5 commented Mar 29, 2020

View reviewed changes

mwachs5 commented Mar 30, 2020

View reviewed changes

src/main/scala/devices/debug/Periphery.scala Outdated Show resolved Hide resolved

mwachs5 force-pushed the multiple-reset-schemes-rebased branch 2 times, most recently from c27108b to 773d4fb Compare April 2, 2020 00:23

mwachs5 and others added 9 commits April 2, 2020 11:40

Debug: reduce hardware added to TLError on APB interface

ba8cfed

Chisel3: cast reset

da2b9aa

Add IO() required by chisel3 Revert chisel3 to Chisel for now

Convert DM to new reset methodology

93313d9

Remove clock and reset from TracedInstruction bundle

7b4efd4

Decouple debug_reset from dmactive

bd927ac

SubsystemResetScheme config, changes for asyncReset compatibility

9c8ae5c

Added rf_reset signals needed by RF transform

a7ad791

Add missing signals to Debug.tieoff

f104cf4

Expand CoreComplexResetType OM to include new reset schemes

a32bda4

mwachs5 and others added 21 commits April 2, 2020 11:40

Multiple Resets: handle diplomatic clocking

af9fda6

LazyModule: don't assign Reset to Bool type automatically, keep it as…

44fa762

… chisel3 reset type

Async Reset: make some more things chisel3 wires to prevent them bein…

8b91b54

…g assigned to Bool

Use Reset() and DontCare for childReset and use fixed chisel3 and firrtl

bd3e408

Give resetCtrl argument to tieoffDebug a default value

dec0620

Debug: revert unncessary whitespace change

000c07c

UserYanker: make Queues inside get chisel3 reset treatment

5c0c9cb

Bus Bypass: remove reliance on synchronous reset

431881d

BusBypass: correct removal of sync reset reliance

58f7fa9

Debug: turn TLDebugModule into RawModule to allow different reset types

03f170f

Use inferred resets for JTAG and DebugTransport modules

c44c536

Review comments

JtagTap output reset changed to tapIsInTestLogicReset for clarity

6f4f3e7

Debug Transport: partially revert conversion of rests to AsyncReset

bfb438f

JTAG Debug Transport: asynchronously reset flops that need to work wi…

d059653

…th no clock edges to avoid violating tile link protocol. Also use chiselName to give predictable naming within withClockAndReset blocks

Rocket Core: add requirement that reset scheme is Synchronous, for now

fcfe849

Improve verilog by naming registers

7cde1fa

PLIC: convert to chisel3 inferred reset

bf7527c

Avoid unnecessary code changes in Arbieter

569bf36

Arbiter: fix parenthesis Arbiter more typo fixes arbiter. srsly don't try to write actual code throug github GUI interface

Change dmactive read value to dmactive & dmactiveAck

09b91ef

Update riscv-tools.hash

235f9c6

bumping to have proper fesvr handling

bump firrtl along 1.2.x with several bugfixes

7f31ae4

mwachs5 force-pushed the multiple-reset-schemes-rebased branch from 773d4fb to 7f31ae4 Compare April 2, 2020 18:40

mwachs5 closed this Apr 3, 2020

mwachs5 reopened this Apr 3, 2020

mwachs5 merged commit 3933197 into master Apr 3, 2020

jackkoenig deleted the multiple-reset-schemes-rebased branch May 26, 2020 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for Multiple Reset Schemes (for some parts of the codebase) #2375

Support for Multiple Reset Schemes (for some parts of the codebase) #2375

mwachs5 commented Mar 28, 2020 •

edited

Loading

ernie-sifive left a comment

mwachs5 commented Mar 29, 2020

mwachs5 commented Mar 29, 2020

ernie-sifive commented Mar 29, 2020 •

edited

Loading

mwachs5 Mar 29, 2020

mwachs5 commented Mar 30, 2020

mwachs5 commented Apr 3, 2020

Support for Multiple Reset Schemes (for some parts of the codebase) #2375

Support for Multiple Reset Schemes (for some parts of the codebase) #2375

Conversation

mwachs5 commented Mar 28, 2020 • edited Loading

ernie-sifive left a comment

Choose a reason for hiding this comment

mwachs5 commented Mar 29, 2020

mwachs5 commented Mar 29, 2020

ernie-sifive commented Mar 29, 2020 • edited Loading

mwachs5 Mar 29, 2020

Choose a reason for hiding this comment

mwachs5 commented Mar 30, 2020

mwachs5 commented Apr 3, 2020

mwachs5 commented Mar 28, 2020 •

edited

Loading

ernie-sifive commented Mar 29, 2020 •

edited

Loading