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

Extended Zcherihybrid to reflect that access is not allowed to CHERI instructions #464

Closed
wants to merge 1 commit into from

Conversation

francislaus
Copy link
Contributor

The text in most cases only considered CHERI registers, but CRE also prevents executing CHERI instructions.

@@ -324,7 +324,7 @@ it is not possible to disable CHERI checks completely.
endif::[]

{cheri_default_ext_name} includes functions to disable explicit access to CHERI
registers. The following occurs when executing code in a privilege mode that
registers and instructions. The following occurs when executing code in a privilege mode that
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought this was implicit because any CHERI instruction that uses a CHERI register is not possible, but happy to add this clarification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arichardson I agree, it is implicit. For some CSRs, the text explicitly says "registers and instructions" (https://github.com/riscv/riscv-cheri/blob/main/src/riscv-hybrid-integration.adoc?plain=1#L446-L448) whereas for others the text only refers to registers. We might want to have that consistent. I am fine with either way.

If we go with only referring to registers, we might want to make it clear somewhere that this implies that code cannot execute CHERI instructions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The key question here is whether CRAM needs CRE, which it does, and that does not access CHERI registers.
Reading through the doc it's inconsistent about whether it's only registers or registers and instructions.
Maybe we need a better term, although I don't want to start renaming things now.
I'll have a go at improving the text....

Copy link
Collaborator

Choose a reason for hiding this comment

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

pushing to this branch resulted in a new PR (no idea why) but here it is: #474

@tariqkurd-repo tariqkurd-repo self-requested a review December 9, 2024 09:30
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