Fix sbdata0 read order of operations.#392
Conversation
|
Can I get a review on this? |
|
Presumably this is blocked by #391 ? |
|
Hey Tim Newsome (@timsifive), LGTM, but I think the spec would be more clear if there were an explicit note that the increment doesn't happen until the system bus memory transaction completes |
| Debug Specification. | ||
|
|
||
| The following pre-built PDFs may be of interest: | ||
| * [0.13-DRAFT, which will likely be ratified](https://github.com/riscv/riscv-debug-spec/blob/task_group_vote/riscv-debug-draft.pdf) |
There was a problem hiding this comment.
"likely be ratified" -> "has been submitted to the TC for ratification"?
There was a problem hiding this comment.
I think the former is more clear to people not familiar with the ratification process. Do you prefer to hedge a bit?
There was a problem hiding this comment.
Yes, but I wanted to mention the errata doc in the README, which isn't added until this change. I could have made changes in both and then resolve conflicts later, but this seemed much simpler.
|
Megan Wachs (@mwachs5) Yes, #391 must merge first (and once this merges, I'll work on #393). Ryan Macdonald (@rmac-sifive) I've clarified the language in the draft spec, but not in the errata. I feel the errata is only for fixing real bugs, in as simple a way as possible to avoid introducing new bugs. |
| The following pre-built PDFs may be of interest: | ||
| * [0.13-DRAFT, which will likely be ratified](https://github.com/riscv/riscv-debug-spec/blob/task_group_vote/riscv-debug-draft.pdf) | ||
| * [Errata Document](https://github.com/riscv/riscv-debug-spec/blob/master/riscv-debug-errata.pdf) | ||
| * [Latest draft](https://github.com/riscv/riscv-debug-spec/blob/master/riscv-debug-draft.pdf) |
There was a problem hiding this comment.
Should we say something in the README about the rationale of 0.13.1?
I was assuming it was as described on: https://semver.org/
Given a version number MAJOR.MINOR.PATCH, increment the:
MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards-compatible manner, and
PATCH version when you make backwards-compatible bug fixes.
Additional labels for pre-release and build metadata are available as extensions to the MAJOR.MINOR.PATCH format.
There was a problem hiding this comment.
The spec itself should mention all previous ratified versions, and their compatibility, but not until there actually is a ratified spec.
Also created an errata document, since this is a real bug in the spec. I've implemented the new semantics in spike and confirmed OpenOCD still "just works." Rocket had implemented the right thing from the start.
Tim Newsome (timsifive)
force-pushed
the
sbdata0
branch
from
84c4b45 to
41a6951
Compare
|
This change now only fixes the actual spec bug. |
3 participants
Also created an errata document, since this is a real bug in the spec.
I've implemented the new semantics in spike and confirmed OpenOCD still
"just works." Rocket had implemented the right thing from the start.