Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update resume from Debug Mode behavior with Smdbltrp #1051

Merged
merged 2 commits into from
Jul 10, 2024

Conversation

ved-rivos
Copy link
Contributor

No description provided.

Copy link
Collaborator

@pdonahue-ventana pdonahue-ventana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. MRET/SRET clear MDT so that you won't get a critical error on the next trap from U mode (or whatever). Resuming should clear it for the same reason.

This is very slightly different than the MRET/SRET requirements in the priv spec because you are able to go back to M mode with MDT=1 (whereas MRET to M would clear MDT). But we don't want to clear MDT in that case because the debugger needs to be allowed to resume to the critical part of the M mode handler where another trap should be a critical error. So this all makes sense.

@ved-rivos
Copy link
Contributor Author

Additionally, debug mode can resume to S with sstatus.SDT=1 and to VS with vsstatus.SDT=1 to allow debugging M, S, and VS mode without trashing their state.

@ved-rivos
Copy link
Contributor Author

@pdonahue-ventana - I had missed qualifying the SDT bits clearing by Ssdbltrp being implemented. I made an update to add that qualification.

@pdonahue-ventana
Copy link
Collaborator

It's not entirely clear that the third sentence has the same Ssdbltrp condition as the second sentence. It really depends on both H and Ssdbltrp because (for obvious reasons) Ssdbltrp only adds the vsstatus bit if H is implemented.

@ved-rivos
Copy link
Contributor Author

I placed the second and third sentence in a separate list item to avoid confusion that sentence 3 is not related to Ssdbltrp. I dont say "if H is implemented" as it is implied by VU mode which is provided only when H is implemented.

@pdonahue-ventana
Copy link
Collaborator

A separate bullet is perfect.

Sdext.adoc Outdated Show resolved Hide resolved
Sdext.adoc Outdated
. If the Smdbltrp extension is implemented and the new privilege mode is not M,
then the `MDT` bit is set to 0.
. If the Ssdbltrp extension is implemented and the new privilege mode is U, VS,
or VU, then `sstatus.SDT` is also set to 0. Additionally, if it is VU, then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
or VU, then `sstatus.SDT` is also set to 0. Additionally, if it is VU, then
or VU, then `sstatus.SDT` is set to 0. Additionally, if it is VU, then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@ved-rivos
Copy link
Contributor Author

@pdonahue-ventana @rtwfroody
I noticed that cetrig was marked R/W but should have been WARL. Please take a look.

@pdonahue-ventana
Copy link
Collaborator

The cetrig change looks good.

@rtwfroody rtwfroody merged commit d0c692a into riscv:main Jul 10, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants