target/riscv: reset delays during batch scans

[en-sc]

Prior to the commit, it was impossible to get a dmi.busy responce on a scan in a batch that was not the first scan in the batch.

Change-Id: Ib0714ecaf7d2e11878140d16d9aa6152ff20f1e9

[JanMatCodasip]

@en-sc Please, could you explain in bit more detail what issue this PR is trying to address (e.g. in the commit message). After taking an initial quick look at the code, I was not able to grasp what the issue actually is.

My understanding of the batch code is that:

• after each scan, the corresponding amount of idle cycles is really inserted: batch.c, line 110
• if the DMI busy situation occurs at any point in the batch (regardless whether it is the first scan or any subsequent), the busy status is sticky, so the rest of the batch will also be marked as busy, therefore it is enough to just check the status of the last operation in the batch - batch.c, line 248

What am I missing? :)

[en-sc]

@JanMatCodasip, you are right, the description is not sufficient.

This commit is related to testing how OpenOCD responds to dmi.busy.

Consider testing on Spike (e.g. riscv-tests/debug testsuite). Spike returns dmi.busy if there were less then a given number of RTI cycles (required_rti_cycles) between DR_UPDATE and DR_CAPTURE:
https://github.com/riscv-software-src/riscv-isa-sim/blob/master/riscv/jtag_dtm.cc#L145
https://github.com/riscv-software-src/riscv-isa-sim/blob/master/riscv/jtag_dtm.cc#L202
required_rti_cycles gets it's value from --dmi-rti CLI argument and is constant throughout the run.

OpenOCD learns this required number of RTI cycles by starting with zero and increasing it if dmi.busy is encountered. So the required number of RTI cycles is learned during the first DMI access in the examine().

To induce dmi.busy on demand riscv reset_delays <x> command is provided. This command initializes riscv_info::reset_delays_wait counter to the provided <x> value. The counter is decreased before a DMI access and when it reaches zero the learned value of RTI cycles required is reset, so the DMI access results in dmi.busy.

E.g. riscv-tests/debug testsuite issues riscv reset_delays <x> before most operations:
https://github.com/riscv-software-src/riscv-tests/blob/fd01e46314636666b5bed1f08465f734751a4cda/debug/testlib.py#L837

Now consider running a batch of accesses.
Before the change all the accesses in the batch had the same number of RIT cycles in between them.
So either:

• Number of accesses in the batch was greater then the value of riscv_info::reset_delays_wait counter and there was no dmi.busy throughout the batch.
• Number of accesses in the batch was less or equal then the value of riscv_info::reset_delays_wait counter and the first access of the batch resulted in dmi.busy.

Therefore it was impossible to encounter dmi.busy on any scan of the batch except the first one.

[en-sc]

One can argue that the better solution would be to change Spike's required_rti_cycles value by exposing it through some other custom DM/DMI register. I have avoided this approach because:

1. This will take more time (changing both Spike and riscv-tests/debug testsuite).
2. In this commit I have implemented the mechanism that allows to change the number of RTI cycles used after scans in a batch. I will need this mechanism to implement more efficient register read/writes via abstract commands:

e.g. 32-bit read now involves executing the following DMI operations:

• write command
• read abstractcs
• nop
• read data0
• nop

We don't need all these NOPs (apart from the last one), since error statuses are sticky. The easiest way to eliminate them is to use a batch. However, after writing abstract command we need more RTI cycles, so that command finishes and abstractcs.busy is low by the time it is read.
(I have started implementing this in #1033)

[en-sc]

... could you explain in bit more detail what issue this PR is trying to address (e.g. in the commit message).

Commit message is changed.

[JanMatCodasip]

@en-sc, thank you for explaining the reasons for this change in detail. Now it is very clear to me.

As for using Spike ISA sim for testing of OpenOCD, I agree that Spike will not help us cover all the various edge cases of RISC-V debug protocol (like DMI busy, abstract command busy or system bus busy). As an example, you can take a look at this merge request in which configurable system bus delay was proposed to Spike but only accepted in a reduced form: riscv-software-src/riscv-isa-sim#1513

(Side note: My vision would be to implement a simple simulator of RISC-V debug module + core, much simpler than spike, just a bare minimum, which would however allow to trigger the debug edge cases that are impossible to trigger via a general-purpose simulator like Spike. It is of course unclear whether there will be ever a capacity to do it :/)

For the current pull request, I have posted suggestions how to implement the same thing this using simpler code. Please if you can look at it.

[JanMatCodasip]

1. For my overall comment, please see above: target/riscv: reset delays during batch scans #1028 (comment)

2. In the individual review suggestions, I have proposed how to implement the same capability using simpler code. Please let me know if you like it.

[JanMatCodasip]

I am sending few smaller coding suggestions, and don't expect to have any more comments.

Overall the code looks good, thank you.

[JanMatCodasip]

LGTM, thank you!