search

pulp-platform / riscv-dbg

RISC-V Debug Support for our PULP RISC-V Cores
Other
218 stars 72 forks source link

Fix DMI response when command or SBA are busy #138

Closed andreaskurth closed 1 year ago

andreaskurth commented 2 years ago

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 commented 2 years ago

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 commented 1 year ago

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

a-will commented 1 year ago

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.