DM should ignore some bits while abstract command is executing #1046
Conversation
|
@en-sc, can you take a look at this? |
|
Rather than allowing bits to be ignored depending on dmactive=0, does it make sense to ignore writes to hartsel, haltreq, resumereq, etc. whenever abstractcs.busy=1? That seems cleaner and more robust overall. However, if we require this behavior it's not backwards compatible. We can say the hardware should do that, and debuggers should preserve the values, and add a note that when resetting it's sometimes unavoidable for the debugger to preserve them. Thoughts? |
|
Sorry for taking so long to reply.
This is cleaner. However, AFAIU, this complicates the HW and I'm not sure it is useful for the SW. The thing is, after the External Debugger has connected to the DM, it knows the state of If for whatever reason it looses track of that state ( After the proposed change, the External Debugger would instead be required to read back the written value (in case of I think it will complicate error reporting . E.g.:
Currently the similar situation is processed like so:
|
|
@pdonahue-ventana can you comment here? |
|
What about the dmactive recipe? You write 0 to dmactive and then poll until you see dmactive=0 on a read. What happens in between the write and the read with respect to any outstanding abstract command and the hartsel field? There are two types of implementations:
If busy=1 prevents hartsel from changing then I guess that things will be fine with either implementation and then once dmactive actually goes 0 then hartsel will get reset to 0. If busy=1 does not prevent hartsel from changing then type 1 implementations will break, though type 2 implementations will be fine. So maybe we should at least allow implementations (particularly type 1 implementations) to ignore updates to certain fields when busy=1 (where "certain fields" doesn't include ndmreset). |
|
On the other hand, if we ignore hartsel writes when busy=1 then what if I want to reset hart N (where N is not the hart where we're currently doing an abstract command)? I might want to write hartsel=N, hartreset=1 rather than doing the heavy ndmreset. Or do we have to wait for the command to complete before doing that? And if the command doesn't complete then presumably we want to hartreset the hart that was doing the command rather than some other hart (or we could ndmreset). |
That's a good use case for changing hartsel while an abstract command is executing. Sounds like my "simple" proposal of just ignoring hartsel writes when busy=1 doesn't work. |
As discussed in the meeting, this use case is already disallowed. The spec says:
|
I don't think it's required, given that abstractcs.busy cannot spontaneously go high. It only goes high in response to a debugger request.
That will still work. The problem you describe only happens if the debugger asserts haltreq without first confirming that |
|
Since a debugger is not allowed to change hartsel (or haltreq, resumereq, ackhavereset, setresethaltreq, clrresethaltreq) while a command is busy, if we say that hardware may/should ignore changes to these fields while it's busy then there's no backward compatibility problem and no loss of functionality. We can say "may" along with a non-normative note that it's really a "should" for hartsel on implementations where unexpected hartsel changes will break a currently executing abstract command. Or we could simply say "should" for all of the fields and all implementations. (I can imagine that unexpectedly changing hartsel is a problem on some implementations but I can't imagine how setting/clearing resethaltreq would be a problem. This is just an observation, not a proposal to treat that differently.) |
This is a clean way to let a debugger reset a DM without having to know what hartsel is first. It can simply write 0 then 1 to dmactive. These new rules resolve the issue raised in #1021, that if the DM is executing an abstract command when the debugger wants to reset it, the debugger should not be expected to preserve hartsel.
Co-authored-by: Paul Donahue <[email protected]> Signed-off-by: Tim Newsome <[email protected]>
This is what I want the spec to say. But it's not backwards compatible. Can we get away with this?
This behavior is not backwards compatible. It would fix a corner case where the DM is performing an abstract command and reading dmcontrol returns in an error. That is so unlikely that it's not worth making a backwards incompatible change for.
rtwfroody
changed the title
|
This PR has morphed into recommending some more robust behavior while abstract commands are executing. |
This makes things work better in case the debugger wants to reset but doesn't know hartsel, as mentioned in #1021.
Also the debugger should write dmactive=0 to perform a reset, instead of "may".