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

Fix DMI response when command or SBA are busy #138

Merged
merged 7 commits into from
Oct 11, 2022

Conversation

andreaskurth
Copy link
Contributor

The RISC-V External Debug Support spec (v0.13.2) specifies that certain accesses to debug module registers while an abstract command is running result in a busy value for the cmderr field of the abstractcs register (Section 3.12.6). (This was originally not fully implemented, see #37, which has been fixed in #79.) However, the spec does not define the response value returned on an access while busy, so that value is defined by the implementation. The current implementation of this debug module always returns DTM_SUCCESS as response in dmi_resp_o of dm_csrs, even though the data in dmi_resp_o can be garbage if the debug module was busy during the access. This has been reported in #103.

This PR changes the response in dmi_resp_o of dm_csrs to DTM_BUSY if a command or the SBA was busy when the registers served a new command. This commit also changes the JTAG DMI to take the response into account for reads and return an error code instead of data if the read was not successful.

This PR replaces #104. The problem with gating the DMI request and response handshakes with ~cmdbusy_i (as PR #104 does) is that the handshakes are stalled until the command is no longer busy. If, for whatever reason, the command remains busy, the DMI will become unresponsive. This is problematic in many cases. Returning a busy response instead gives the DMI (and the software interfacing it) the chance to retry later or execute a different command.

@bluewww
Copy link
Collaborator

bluewww commented Oct 4, 2022

I agree this is a good change but I think additionally the jtag dtm should set op field to 3 according to 6.1.5 Debug Module Interface Access

@andreaskurth
Copy link
Contributor Author

Thanks for your feedback, @bluewww! I agree and have added three commits to implement this.

Copy link
Collaborator

@bluewww bluewww left a comment

Choose a reason for hiding this comment

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

lgtm. Passes tests.

@a-will
Copy link

a-will commented Aug 11, 2023

Just rudely dropping in late to mention that I don't think this is the correct reading of the spec. DTM_BUSY is not supposed to reflect whether the DM is busy with an operation that triggers from a DM CSR transaction. It purely marks that a DM CSR transaction is pending over the DMI interconnect. The spec is specific about what DTM_BUSY is:

An operation was attempted while a DMI request is still in progress

It mentions nothing about the DM there because the DM's operations don't contribute. It only asks whether the DMI request completes.

You'll find that both Rocket and openocd disagree with the interpretation here. Rocket's TL-UL based DMI only reports error or success from the DM; the busy response is only based on whether there is a pending CSR read (TL-UL op was read, and d_valid is not high by the time the TAP reaches the Capture-DR state).

openocd maintains a separate delay values for DMI base delays and SBA operation delays. If the DM causes the DTM to report busy for SBA, every JTAG transaction becomes as slow as the SBA.

We probably should find DTM_BUSY in only the TAP's RTL files.

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.

3 participants