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

Specify behaviour for reserved permission encoding #53

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

Timmmm
Copy link
Contributor

@Timmmm Timmmm commented Jan 25, 2024

Fixes #47

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 think this makes a lot of sense especially if we consider forward compatibility.

Copy link
Collaborator

@andresag01 andresag01 left a comment

Choose a reason for hiding this comment

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

@Timmmm @arichardson @tariqkurd-repo : What do you think would happen if executing CBuildCap to tag a cap that has permissions set to a reserved value? Options include:

  1. Set the permissions of the output cap to 0 (i.e. no permissions)
  2. Set tag 0 for the output tag

I would prefer (2)!

@jrtc27
Copy link
Collaborator

jrtc27 commented Jan 26, 2024

Any kind of reserved input should always be untagged in the output. Anything else sounds like trouble.

xref:cap_perms_encoding32[xrefstyle=short]). It is not possible for a tagged
capability to have this value since <<CANDPERM>> will never create it. It is
possible for untagged capabilities to have it. <<CGETPERM>> will interpret it
as if it were 0b0000 (no permissions).
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way this is worded forbids an extension later assigning meaning to it (yes, this is non-normative text, but you still don't want it to be contradictory)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Would it be ok if I change it to It is currently not possible... <<CGETPERM>> will currently interpret it... This may change in future..

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this still allows for future extensibility:

If the permission encoding has reserved bit patterns (e.g. the format for XLENMAX=32 with {cheri_base_ext_name}
xref:cap_perms_encoding32[xrefstyle=short]) then it is not possible for a tagged
capability to have this value since <<CANDPERM>> will never create it. However, it is
possible for untagged capabilities to have it in which case <<CGETPERM>> will interpret it
as if it were 0b0000 (no permissions). Future extensions may assign meaning to these bit patterns for which <<CANDPERM>> is allowed to report a non-zero value.

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 based on @arichardson's suggestion.

@Timmmm Timmmm force-pushed the user/timh/specify_cgetperm_reserved branch from d5b117c to fb474bf Compare January 30, 2024 12:45
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.

Lgtm, @jrtc27 are you happy with this wording?

Copy link
Collaborator

@andresag01 andresag01 left a comment

Choose a reason for hiding this comment

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

It looks good to me, but we still need a comment about CBuildCap. Perhaps its best to do that as a separate PR.

@andresag01 andresag01 merged commit a00875d into riscv:main Feb 2, 2024
3 checks passed
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.

Define behaviour for reserved RV32 permission encoding
4 participants