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 ARM Access for det_ext changes #840

Open
wants to merge 4 commits into
base: det_ext_state
Choose a base branch
from

Conversation

corlewis
Copy link
Member

@corlewis corlewis commented Dec 12, 2024

Following on from #824 and #844, this PR additionally updates the Access Control proofs for the changes to det_ext and the scheduler state.

@corlewis corlewis requested review from Xaphiosis and lsf37 December 12, 2024 05:46
@corlewis corlewis self-assigned this Dec 12, 2024
@corlewis corlewis requested a review from ryybrr December 12, 2024 05:47
@corlewis
Copy link
Member Author

corlewis commented Dec 12, 2024

Ignore the first commit for reviewing purposes, it contains the limited changes that could be copied directly across from AArch64 AInvs.


lemma thread_set_time_slice_pas_refined[wp]:
"thread_set_time_slice tptr time \<lbrace>pas_refined aag\<rbrace>"
by (simp add: thread_set_time_slice_def | wp)+
"\<lbrace>pas_refined aag and pspace_aligned and valid_vspace_objs and valid_arch_state\<rbrace>
Copy link
Member Author

Choose a reason for hiding this comment

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

This precondition (and those of several similar lemmas) could be weakened using the changes from 357ccc9. I'll do so once that commit is either in master or is able to be cherry-picked into this branch.

Copy link
Member

Choose a reason for hiding this comment

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

does this also apply to lemma pas_refined?

Copy link
Member Author

Choose a reason for hiding this comment

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

This precondition (and those of several similar lemmas) could be weakened using the changes from 357ccc9. I'll do so once that commit is either in master or is able to be cherry-picked into this branch.

This has now been done, see e3797a8 for the preconditions that were relaxed.

Copy link
Member Author

Choose a reason for hiding this comment

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

does this also apply to lemma pas_refined?

If I'm looking at the right lemma then no. This was specifically for preconditions where I'd had to add pspace_aligned and valid_vspace_objs and valid_arch_state

@lsf37
Copy link
Member

lsf37 commented Dec 19, 2024

Is the plan to rebase this over #844?

@Xaphiosis
Copy link
Member

Is the plan to rebase this over #844?

I think it already is. This PR has 3 commits, but only one is new, and the second one is the same as in 844 (currently)

apply (clarsimp simp: etcbs_of'_def split: if_split_asm kernel_object.splits)
apply (clarsimp simp: default_object_def default_tcb_def tyunt split: apiobject_type.splits)
defer
apply (force intro: domtcbs)
Copy link
Member

Choose a reason for hiding this comment

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

is this copied from elsewhere? post-defer indent is wrong, and probably there's no need to split obj and aobj separately anyway, and on top of that force can probably finish it after the erule, or maybe even do the whole lemma if we're lucky

Copy link
Member Author

Choose a reason for hiding this comment

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

I have to admit that I don't remember but it doesn't look like my style so I assume that I copied it from somewhere.

It's actually even weirder than you thought, the post-defer indent is correct, because the defer is done when there is only one subgoal. I've changed it like you suggested and moved everything into the force.

Copy link
Member

@Xaphiosis Xaphiosis left a comment

Choose a reason for hiding this comment

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

One minor comment, else good. Nice to see more red than expected.

@corlewis corlewis changed the title Update ARM AInvs and Access for det_ext changes Update ARM Access for det_ext changes Dec 19, 2024
@corlewis
Copy link
Member Author

Is the plan to rebase this over #844?

I think it already is. This PR has 3 commits, but only one is new, and the second one is the same as in 844 (currently)

The exact history is a bit more roundabout but yes, I've rebased this PR over the squashed #844 so that this one is just the Access commit.

ryybrr and others added 4 commits February 4, 2025 13:26
-Remove various instances of pspace_aligned, valid_vspace_objs, and
valid_asid_table/valid_arch_state from preconditions
-Relax the is_subject predicate in various Syscall_AC lemmas

Signed-off-by: Ryan Barry <ryan.barry@proofcraft.systems>
Signed-off-by: Corey Lewis <corey.lewis@proofcraft.systems>
Signed-off-by: Corey Lewis <corey.lewis@proofcraft.systems>
Signed-off-by: Corey Lewis <corey.lewis@proofcraft.systems>
Copy link
Contributor

@ryybrr ryybrr left a comment

Choose a reason for hiding this comment

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

All looks good to me, hope my commit helped!

@@ -138,6 +143,7 @@ abbreviation integrity_asids_aux ::
pp_opt = pp_opt' \<or> (\<forall>asid'. asid' \<noteq> 0 \<and> asid_high_bits_of asid' = asid_high_bits_of asid
\<longrightarrow> pasASIDAbs aag asid' \<in> subjects)"

\<comment> \<open>FIXME: could we define integrity_asids to operate on arch_state only?\<close>
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately not, we use asid_pools_of in RISCV64

@@ -33,12 +33,25 @@ end

context retype_region_proofs' begin interpretation Arch .

\<comment> \<open>FIXME: move to pas_refined_state_objs_to_policy_subset?\<close>
Copy link
Contributor

Choose a reason for hiding this comment

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

If the proof impact is minimal, that would be ideal. Curious what case you ran into where the subset relation was too strong.

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.

4 participants