Skip to content
This repository was archived by the owner on May 16, 2025. It is now read-only.

Fully apply implicit acperm, including tag clear - #9

Open
tomaird wants to merge 1 commit into
CHERI-Alliance:codasipfrom
tomaird:implicit_acperm
Open

Fully apply implicit acperm, including tag clear#9
tomaird wants to merge 1 commit into
CHERI-Alliance:codasipfrom
tomaird:implicit_acperm

Conversation

@tomaird

@tomaird tomaird commented Apr 16, 2025

Copy link
Copy Markdown

Implicit ACPERMs, done for permission-clearing on loads, should apply the full instruction semantics, including clearing the tag for certain cases.

Raised from this issue: riscv/riscv-cheri#577

While the spec may change on this, the issue clarified the intention was always that the full ACPERM semantics should be applied, so the model was always wrong on this.

@Timmmm
Timmmm requested a review from PRugg-Cap April 16, 2025 11:31
let restrictCL = loaded_cap.tag & not(canEL(auth_cap));
if restrictCL then {loaded_cap with cl = auth_cap.cl & loaded_cap.cl} else loaded_cap
if restrictCL | clearWLM | clearEL then {
clearTagAcperm(loaded_cap)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This seems wrong, we should not zero the tag on sealed if all we did was restrict CL.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The spec says that restricting CL is done with an implicit ACPERM, even if you don't do clear EL too.

"implicit ACPERM" means do the whole instruction, as discussed in the linked ticket.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah my view is that this is incorrect and should be fixed in the spec.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@arichardson do you know if this was fixed in the spec?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm archiving this repo but we can continue the discussion here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants