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

Define ResetCrossingType and use with BlockDuringReset in TilePRCIDomain #2641

Merged
merged 33 commits into from
Nov 18, 2020

Conversation

hcook
Copy link
Member

@hcook hcook commented Sep 17, 2020

This PR combines and merges the effects of #2618 and #2623 while also partially reverting #2611. The overall goals are:

  • Avoid tunneling the "raw" subsystem reset into RocketTile, instead, move all logic requiring raw reset into the TilePRCIWrapper
  • Said logic largely takes the form of registers that block tile outputs from appearing valid while the tile is being held in an extended reset, in which case those registers need to be asynchronously reset for certain ResetSchemes (when a ResetStretcher is injected) and then they need to maintain the blockage for the duration of any stretched reset.
  • TileResetDomain is added to manage diplomatic control of the Tile's reset input. All other clock crossing logic in TilePRCIDomain goes back to being on the raw subsystem reset (but still on the tile clock domain). TileResetDomain is inlined by firrtl to prevent further disruption to the module hierarchy.
  • ResetCrossingType is defined to be analogous to ClockCrossingType, the two defined reset crossing types are currently NoResetCrossing and StretchedResetCrossing(cycles: Int)
  • The diplomatic CrossingHelper functionality for TL and Interrupts are extended to also deal with ResetCrossingType
  • Specifically, for StretchedResetCrossing they will inject BlockDuringReset logic in the scope where the helper is invoked; in this specific case, that scope is TilePRCIDomain, ensuring the logic gets the raw reset.
  • Various other minor parameterization cleanups and helper nodes.

@aswaterman, @ernie-sifive can you double check that my changes to the actual BlockDuringReset logics look kosher?

Type of change: other enhancement

Impact: API modification

Development Phase: implementation

Release Notes

Copy link
Contributor

@ernie-sifive ernie-sifive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the reset treatment matches what we discussed.

/** Node for external consumers to source a legacy instruction trace from the core. */
val traceNode: BundleBridgeOutwardNode[Vec[TracedInstruction]] =
BundleBridgeNameNode("trace") :*= traceNexusNode := traceSourceNode
BundleBridgeNameNode("trace") := traceSourceNode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this moves traceNexusNode to the PRCI layer so it can be blocked during reset. There's some technical debt to apply the same treatment to traceCoreNexusNode (below). This bundle doesn't have a valid signal. Either we will have to add a valid to TraceCoreInterface (bending the definition of this interface from the RISC-V Trace Spec) or add another form of BlockDuringReset specifically for this (which would zero out iretire and itype).

Copy link
Member Author

@hcook hcook Sep 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, I was wondering a bit about whether that bundle needed to be blocked too. There's almost certainly a way to have each Bundle subclass define it's own blockMe function (name TBD) that returns a copy of itself but with certain signals tied low. You still have to make one NexusNode per type of bundle bridge, but it's at least very generic code to ensure that the blocking occurs in a type-specific way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants