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

Clarify permissions when the level extension is not implemented #501

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arichardson
Copy link
Collaborator

@arichardson arichardson commented Jan 7, 2025

Update the infinite cap bit pattern and add a note to GCPERM.

@arichardson
Copy link
Collaborator Author

We could also report a hardcoded 1 here since not implementing the levels extension ensures that all loads/stores behave as if the permissions were always granted. But I think being able to easily detect whether the levels ext is implemented probably makes more sense?

src/insns/gcperm_32bit.adoc Outdated Show resolved Hide resolved
@@ -33,6 +33,9 @@ permission bits in `rd` are set to 0.
[#gcperm_bit_field]
include::../img/acperm_bit_field.edn[]

NOTE: When {cheri_levels_ext_name} is not implemented, the `CL`, `SL`, `EL` and EL fields always report 0.
Therefore, the presence of the {cheri_levels_ext_name} can be detected by checking the <<GCPERM>> result on the <<infinite-cap>>.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, the RISC-V way is to get that information externally, for better or (generally) worse.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's true, happy to use hardwired to 1 instead if that sounds better to you?

I noticed I also need to update the infinite cap format to set those bits to zero/one.

This change is motivated by CHERI-Alliance/sail-cheri-riscv#3 since I noticed the M bit location was wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Reserved bits should be 0, and without levels the bits are reserved. When an extension adds meaning to those bits, the existing bit pattern should be unchanged in its meaning. I'm concerned the latter isn't true if the natural default is 1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that reserved bits should be zero. The problem is that if we add new permissions, those will generally be '1' for permissive behaviour and '0' for restricted, so the natural default is 1 even though cores that don't implement this extension will be reporting zero and doing the permissive behaviour. I am not sure if this can really be fixed.

Copy link
Collaborator

@jrtc27 jrtc27 Jan 7, 2025

Choose a reason for hiding this comment

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

You invert them, you have RES1 bits or you have separate feature enable bits. #502 is the short dump of thoughts I have on the matter, and that I've brought up many times over the years wanting people to think about this carefully with worked examples, to no avail.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok - so RES1 seems like the way to go

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the in-memory format it seems like we would have to use inverted bit meaning? Otherwise if we use RES1 we end up having those permissions set to 0 for the null cap which sounds like it would be a problem?

@arichardson arichardson force-pushed the levels-clarification branch from bc03fa7 to 2b4bbb4 Compare January 7, 2025 19:58
Update the infinite cap bit pattern and add a note to GCPERM.
@arichardson arichardson force-pushed the levels-clarification branch from 2b4bbb4 to 4d8e589 Compare January 7, 2025 19:59
@arichardson arichardson changed the title Clarify GCPERM result when the level extension is not implemented Clarify permissions when the level extension is not implemented Jan 7, 2025
@arichardson
Copy link
Collaborator Author

If we go for RES1 on GCPERM/ACPERM, does that mean we need to have all unused bits hardwired to one?

That would break patterns like gcperm(acperm(cap, REQUESTED_PERMS)) == REQUESTED_PERMS to check if you got exactly what you asked for.

@jrtc27
Copy link
Collaborator

jrtc27 commented Jan 10, 2025

These are all good questions that need to be thought through carefully, hence my repeated invitations to think about the problem (e.g. I mentioned this back in October 2023 on our internal Slack)...

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