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

Update proofs for AArch64 seL4 PR #663

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Update proofs for AArch64 seL4 PR #663

merged 5 commits into from
Aug 14, 2023

Conversation

lsf37
Copy link
Member

@lsf37 lsf37 commented Aug 11, 2023

Update proofs for the changes in seL4/seL4#801, in particular commit seL4/seL4@b3cc852

@lsf37 lsf37 self-assigned this Aug 11, 2023
@lsf37 lsf37 added seL4-PR requires merging a corresponding seL4 pull request Aarch64 AArch64-specific proofs, specs, etc labels Aug 11, 2023
@@ -2332,6 +2332,9 @@ definition
| ARM_HYP_H.PageDirectoryObject \<Rightarrow> scast seL4_ARM_PageDirectoryObject
| ARM_HYP_H.VCPUObject \<Rightarrow> scast seL4_ARM_VCPUObject"

(* always unfold StrictC'_mode_object_defs together with api_object_defs *)
lemmas api_object_defs = api_object_defs StrictC'_mode_object_defs
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% on whether having this kind of def override so far into CRefine is a risky approach. Should be ok here, but made me stop and think.

Copy link
Member Author

Choose a reason for hiding this comment

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

In this case I think it's fine for ARM/ARM_HYP, because it recreates the old situation before it matters the first time. But generally I agree, it's a bit risky.

Copy link
Member

@corlewis corlewis left a comment

Choose a reason for hiding this comment

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

The changes themselves look fine to me but I'm a bit confused. Isn't the C change generally getting rid of the PageDirectoryObj and using PageTableObj instead?

Otherwise, a very minor comment is that I think the more standard commit tag would be arm abstract+design instead of arm abstract/design.

@lsf37
Copy link
Member Author

lsf37 commented Aug 14, 2023

The changes themselves look fine to me but I'm a bit confused. Isn't the C change generally getting rid of the PageDirectoryObj and using PageTableObj instead?

It doesn't actually get rid of PageDirectoryObj, it just moves it to the mode file which defines the top-level vspace object. For AArch64 that's going to be called VSpaceObj, and for AArch32 it remains PageDirectoryObj, but now has the same enum position as VSpaceObj on AArch64.

Otherwise, a very minor comment is that I think the more standard commit tag would be arm abstract+design instead of arm abstract/design.

👍

AArch64 C code changes now designate PageDirectoryObj as the VSpace
object type, which comes first in the enum.

Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
@lsf37 lsf37 merged commit 4d97b26 into master Aug 14, 2023
14 checks passed
@lsf37 lsf37 deleted the enum-reorder branch August 14, 2023 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Aarch64 AArch64-specific proofs, specs, etc seL4-PR requires merging a corresponding seL4 pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants