Conversation
This is not forwards compatible. Debuggers that rely on haltsum0 would break with this change. But it makes the debug interface on tiny 1-hart systems smaller, especially because it allows the number of address bits in the DMI to be 1 less. (Haltsum0 at 0x40 due to other efforts to maintain backwards compatibility.) Fixes #389.
|
@markuslauterbach, can you comment? |
| is halted or not. Unavailable/nonexistent harts are not considered to | ||
| be halted. | ||
|
|
||
| This register may not be present in systems with fewer than 2 harts. |
There was a problem hiding this comment.
"May not" is ambiguous, possibly meaning "shall not" and possibly meaning "may." How about something like:
This register must be present in systems with 2 or more harts and may be in systems with 1 hart.
Also, is this statement related to the number of harts in the system or the number of harts attached to this DM?
There was a problem hiding this comment.
Does "might not" instead of "may not" work?
There was a problem hiding this comment.
That sounds good.
My other question still applies. Should it be something like "This register might not be present if fewer than 2 harts are attached to the DM." or do you think it's correct as written?
There was a problem hiding this comment.
You are right on that point. I've pushed a change to reflect all this.
|
I can't really comment on whether it's justified to make a backwards-incompatible change here, but I agree that the change itself would be reasonable. So far my debug driver does not use haltsum at all. |
|
Thank you, @markuslauterbach. Knowing it won't be a problem for you either makes me more confident that this change is OK. |
ernie-sifive
left a comment
There was a problem hiding this comment.
That wording sounds good to me.
This is not forwards compatible. Debuggers that rely on haltsum0 would
break with this change. But it makes the debug interface on tiny 1-hart
systems smaller, especially because it allows the number of address bits
in the DMI to be 1 less. (Haltsum0 at 0x40 due to other efforts to
maintain backwards compatibility.)
Fixes #389.