Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Debug Module to abstract reset #2237

Closed
wants to merge 48 commits into from
Closed

Conversation

ernie-sifive
Copy link
Contributor

@ernie-sifive ernie-sifive commented Dec 30, 2019

Type of change: feature request

Impact: API modification

Development Phase: implementation

Release Notes
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. 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. Customer logic can call connectDebugClockAndReset to achieve the same functionality as before.
  • 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 interface conforms to the above, but JTAG still has internally-generated resets for now and currently does not 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.
  • Synchronizers have been updated and AsyncResetReg instantiations have been changed to abstract reset.

This PR includes cherry-picks from these other branches:

  • synchronizer-updates: SynchronizerShiftReg replaced by versions which specify the type of reset to be applied to the internal registers.
  • add-dmactive-bus-blocker: Adds the blocker controlled by dmactive. This PR modifies the control to include both dmactive and a synchronized version of dmactiveAck.
  • lazymoduleimp-abstract-reset: Changes to make implicit reset a Reset() rather than a Bool().
  • bump-chisel-3.2.x-and-firrtl-1.2.x: Fixes bug with AsyncReset registers when the initializer is a literal cast to a Bundle.
  • bump-firrtl: Updates firrtl with last-connect semantics to allow changing reset type.

There are still a few issues to resolve:

  1. @jackkoenig LazyModule assigns the type of reset to the parent's reset type. This cannot then be changed by assigning reset to another type within LazyModuleImp. This currently makes it impossible to have two LazyModules invoked with different reset types (AsyncReset and Bool). We need this function to properly use reset type inference in the debug module hierarchy. --> Edit: this has been done in the bump-firrtl branch.
  2. @jackkoenig Currently dmOuter casts reset internally to AsyncReset. However, registers in the APB interface are not currently async reset. The fix for issue 1 should fix this and then the cast can be removed. We would still like to have a way to check that the reset type is asynchronous (perhaps some kind of require() statement).
  3. @mwachs5 The debug interrupt still passes through a synchronizer which requires the core_clock to be running prior to deasserting core_reset. Without this, the core will not halt prior to executing the first instruction.

ernie-sifive and others added 30 commits November 25, 2019 13:12
Reduce size of Bypass logic in debug modoule

Parameterize atomics and transferSize in TLBusBypassBase, set in dmOuterAsync

Bus Blocker: BriskBusBlocker is very similar to TLBusBlocker
…t that async crossings are truly async and marked with SynchronizerPrimitiveShiftReg
…gSink API for asynchronous interrupt crossings
Correct the minLatency for TLZero back to 1
Adds LazyModule.withClock, LazyModule.withReset, and
LazyModule.withClockAndReset to support setting custom clocks and resets
for LazyModuleImps
@ernie-sifive
Copy link
Contributor Author

@mwachs5 There are 2 issues related to riscv-tools causing the CI failures - see comment in #2205. Turns out my attempt at bumping rocket-tools for OpenOCD didn't work because riscv-unknown-elf-objcopy is missing. Need someone to get a working set of tools set up in rocket-tools.

@mwachs5
Copy link
Contributor

mwachs5 commented Mar 28, 2020

Included in #2375

@mwachs5 mwachs5 closed this Mar 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants