Skip to content

arch-split Init_R and InterruptAcc_R#985

Merged
Xaphiosis merged 2 commits intomasterfrom
arch-split
Apr 1, 2026
Merged

arch-split Init_R and InterruptAcc_R#985
Xaphiosis merged 2 commits intomasterfrom
arch-split

Conversation

@Xaphiosis
Copy link
Copy Markdown
Member

@Xaphiosis Xaphiosis commented Mar 31, 2026

These are small files, was able to get through them relatively quickly.

🦆🦆🦆 Init_R is mainly taking out the arch states into their own file with two wrapped lemmas, so I didn't bother with the whole PR diff-minimisation setup for it, but did the setup for InterruptAcc_R.

Instructions:

In real life, I did the usual addition of InterruptAcc_R, hierarchy update, arch-split and then update. However, since the arch-split part first splits AARCH64, then takes the AARCH64 version of InterruptAcc_R and only fixes what broke, we ended up with a small(er) diff there (again).

Please review commit-by commit with the following notes:

  • single-commit split of Init_R due to its small size
  • [squash] PR: copy AARCH64 version into ..._R - same as last time: stop ..._R looking like a wall of green (no need to review)
  • aarch64 refine: arch-split ..._R` - this is the main bulk of the arch-split, dealing with AARCH64, as well as updates
  • [squash] copy AARCH64 Arch..._R to other arches - wipes the body (not the header!) of other arches' Arch..._R with a copy of the AARCH64 version with AARCH64 appropriately substituted - (mostly pointless to review)
  • `[squash][fake] refine: arch-split ..._R (other arches) - result of fixing up the ArchSchedule_R of other architectures - shows diff to AARCH64

Reminder: DO NOT MERGE squashed version if it still says aarch64 refine: arch-split or AARCH64

Stats: 31 files changed, 386 insertions(+), 1243 deletions(-)
Total: 857 lines removed

@Xaphiosis Xaphiosis added the arch-split splitting proofs into generic and architecture dependent label Mar 31, 2026
@Xaphiosis
Copy link
Copy Markdown
Member Author

Still checking the builds on my side, but not expecting CRefine to be affected.

Comment on lines +155 to +156
assumes preemptionPoint_corres:
"corres (dc \<oplus> dc) \<top> \<top> preemption_point preemptionPoint"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might not be worth it since the current proof just unfolds everything, but it feels like this should be able to be a generic lemma. I think the only arch-specific part of preemptionPoint is the doMachineOp (getActiveIRQ True), so it should be possible to have an interface lemma just for that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done now, see new commit and comment below.

Copy link
Copy Markdown
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.

It's a bit confusing that AARCH64 had so much more in InterruptAcc_R that could be removed, but this looks very good!

@Xaphiosis
Copy link
Copy Markdown
Member Author

Followed @corlewis's advice and tried to make preemptionPoint_corres generic. There was a lot of friction fighting OR_choice_E and get/select_f, and the proof is more hand-holdy. It does clear out the locale though, since there are no longer any arch assumptions.

Please look over the extra commit, also @lsf37

@Xaphiosis
Copy link
Copy Markdown
Member Author

It's a bit confusing that AARCH64 had so much more in InterruptAcc_R that could be removed, but this looks very good!

Thinking more about this: if we were to move those somewhere else then there wouldn't be a need for ArchInterruptAcc_R since for non-AARCH64 they are empty now.

Comment on lines +19 to +20
lemma exec_liftE_get:
"(liftE get >>=E f) x = f x x"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is worth a FIXME move

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will add. This is worth a moment of discussion of where to while we're here. exec_get is in NonDet_Lemmas.thy which don't see monad_eq yet. The earliest I think that does see monad_eq is ... uh... NonDetMonadLemmaBucket?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nondet_Monad_Equations is a good place, I think

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK, added those FIXMEs

@lsf37
Copy link
Copy Markdown
Member

lsf37 commented Apr 1, 2026

It's a bit confusing that AARCH64 had so much more in InterruptAcc_R that could be removed, but this looks very good!

Thinking more about this: if we were to move those somewhere else then there wouldn't be a need for ArchInterruptAcc_R since for non-AARCH64 they are empty now.

That would be quite nice. What is left in AArch64 ArchInterruptAcc_R at this point? (hard to see in the diff)

@lsf37
Copy link
Copy Markdown
Member

lsf37 commented Apr 1, 2026

Followed @corlewis's advice and tried to make preemptionPoint_corres generic. There was a lot of friction fighting OR_choice_E and get/select_f, and the proof is more hand-holdy.

I like the new generic proof better than the old unfold+hammer proofs. At least there is now chance of figuring out what is going on.

Copy link
Copy Markdown
Member

@lsf37 lsf37 left a comment

Choose a reason for hiding this comment

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

Nice, I don't think we split the init proofs in AInvs, and there is really no reason not to. Also great that preemption point is generic now, I think that was worth it.

If we can eliminate ArchInterruptAcc entirely by moving AARCH64 lemmas I'm all for it (unless it's completely messy, but maybe we're in luck of a change).

@Xaphiosis
Copy link
Copy Markdown
Member Author

If we can eliminate ArchInterruptAcc entirely by moving AARCH64 lemmas I'm all for it (unless it's completely messy, but maybe we're in luck of a change).

This is done in latest commit, please check if it's acceptable. It undoes some of the earlier work (rename and update hierarch), which means when I squash it'll also end up as a single commit.

@lsf37
Copy link
Copy Markdown
Member

lsf37 commented Apr 1, 2026

This is done in latest commit, please check if it's acceptable. It undoes some of the earlier work (rename and update hierarch), which means when I squash it'll also end up as a single commit.

Looks all good, ready to go from my side!

@corlewis
Copy link
Copy Markdown
Member

corlewis commented Apr 1, 2026

This is done in latest commit, please check if it's acceptable. It undoes some of the earlier work (rename and update hierarch), which means when I squash it'll also end up as a single commit.

Looks good to me as well!

Mainly to split off the zeroed arch state definitions.

Signed-off-by: Rafal Kolanski <rafal.kolanski@proofcraft.systems>
Make InterruptAcc_R generic-only, including:
* make preemptionPoint_corres generic at the cost of being a bit longer
  than the unfold-everything approach
* migrate AARCH64 VCPU lemmas to Interrupt_R and Arch_R as needed and,
  since these theories don't depend on each other, move the common
  dependency virqType_eq to ArchStateRelationLemmas.

Signed-off-by: Rafal Kolanski <rafal.kolanski@proofcraft.systems>
@Xaphiosis
Copy link
Copy Markdown
Member Author

Squashed and rebased. Ready to merge once tests pass.

@Xaphiosis Xaphiosis merged commit 88b6421 into master Apr 1, 2026
14 checks passed
@Xaphiosis Xaphiosis deleted the arch-split branch April 1, 2026 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-split splitting proofs into generic and architecture dependent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants