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

Swap roles of xtval and xtval2 #379

Merged
merged 11 commits into from
Sep 26, 2024
Merged

Conversation

andresag01
Copy link
Collaborator

@andresag01 andresag01 commented Sep 25, 2024

Swap the roles of xtval and xtval2 such that xtval holds the address on a CHERI fault and xtval2 holds the extra CHERI cause and type information for the exception. The following also had to change along the way:

  • Update old references to xtval that must now point to xtval2
  • Add htval2 because otherwise we do not get any CHERI information in HS-mode
  • Adjust diagrams and descriptions for xtval and xtval2

This PR is a follow up to #373

Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

I agree this is more consistent. I do wonder if we want to allocate more values in *cause instead of adding all the *tval2 registers or if that is a more contentious change.

Maybe allocating bits 24-31 of mcause for a "sub-cause" field could be easier since mcause >= 64 is reserved?

But regardless of whether we want to make that change I think this makes it more consistent with other exceptions so LGTM.

src/hypervisor-integration.adoc Outdated Show resolved Hide resolved
@andresag01
Copy link
Collaborator Author

I do wonder if we want to allocate more values in *cause instead of adding all the *tval2 registers or if that is a more contentious change.

We considered this before, but didn't take that approach because it would consume too much space in *cause, and other places connected to it like medeleg. In future, we will probably need to remove the CHERI fault cause 28 and instead use the new software-check fault cause (see #60).

IMHO, we should keep what we currently have in the spec (and this PR) and await feedback from the ARC.

@tariqkurd-repo
Copy link
Collaborator

I do wonder if we want to allocate more values in *cause instead of adding all the *tval2 registers or if that is a more contentious change.

We considered this before, but didn't take that approach because it would consume too much space in *cause, and other places connected to it like medeleg. In future, we will probably need to remove the CHERI fault cause 28 and instead use the new software-check fault cause (see #60).

IMHO, we should keep what we currently have in the spec (and this PR) and await feedback from the ARC.

I agree with @andresag01 , I think it's better to keep what we have and all this area is subject to change by the ARC

Signed-off-by: Andres Amaya Garcia <[email protected]>
@andresag01 andresag01 merged commit 70ba416 into riscv:main Sep 26, 2024
3 checks passed
tariqkurd-repo added a commit to tariqkurd-repo/riscv-cheri that referenced this pull request Oct 9, 2024
Swap the roles of xtval and xtval2 such that xtval holds the address on
a CHERI fault and xtval2 holds the extra CHERI cause and type
information for the exception. The following also had to change along
the way:

* Update old references to xtval that must now point to xtval2
* Add `htval2` because otherwise we do not get any CHERI information in
HS-mode
* Adjust diagrams and descriptions for xtval and xtval2

This PR is a follow up to riscv#373

---------

Signed-off-by: Tariq Kurd <[email protected]>
Signed-off-by: Andres Amaya Garcia <[email protected]>
Co-authored-by: Tariq Kurd <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants