Make proof generic over current seL4 RISC-V platforms #1024
Conversation
- add machine declarations for maxIRQ - remove old manual maxIRQ definition - add interface lemmas for maxIRQ Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Fix the definition of getActiveIRQ in RSICV64 to not suppress 0xFF, which was an invalid IRQ of some previous platform. Instead check for symbolic irqInvalid. From the proof perspective, even this check is not necessary, but it models actual hardware behaviour where irqInvalid (= irq 0) is reserved to mean "no IRQ". Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
- make sure we are referring to Kernel_Config.maxIRQ - increase alignment of IRQ node in example init state and add alignment check lemma (also used later in AInvs). We need to increase alignment, because some platforms have a much larger number of IRQs (>512). Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Use properties from Arch_Kernel_Config_Lemmas instead of unfolding, which also simplifies most proofs that are involved. Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
| unfolding kernelELFBase_def kernelELFPAddrBase_def pptrTop_def | ||
| by (simp add: mask_def Kernel_Config.physBase_def) | ||
|
|
||
| (* maxIRQ lemmas *) |
There was a problem hiding this comment.
when we use (* *) comments for a grouping, typically we have an empty line after... if there's no empty line, we assume the comment is only for the thing it's attached to
| "x \<le> Kernel_Config.maxIRQ \<Longrightarrow> toEnum (unat x) = x" for x::machine_word | ||
| by (simp add: word_le_nat_alt Kernel_Config.maxIRQ_def) | ||
|
|
||
| (* The following are derived and no longer need to unfold, but are small enough to include here *) |
There was a problem hiding this comment.
Same comment as above on needing an extra line after if this applies to more than just the next lemma.
Also I don't really understand what this comment says. The following [...] no longer need to unfold?
There was a problem hiding this comment.
None of the lemmas that follow unfold any Kernel_Config.*dev. This means they don't really have to be in this theory. (But they are similar enough to the other ones and small enough that we want to keep them together).
There was a problem hiding this comment.
Ah, then you might also want to add something to the comment, at least something after the "unfold", otherwise it's a complete mystery when reading it.
|
|
||
| lemma ucast_irq_bounded_machine_word: | ||
| "(ucast irq :: machine_word) < 2 ^ irq_len" for irq::irq | ||
| by (simp add: word_less_nat_alt unat_ucast_upcast is_up unat_2p_irq_len_machine unat_irq_bounded) |
There was a problem hiding this comment.
aside: how close are these Kernel_Config lemmas to the other Arms'?
There was a problem hiding this comment.
many are just copies of the Arm ones
| if is_masked active_irq \<or> active_irq = 0xFF \<or> (in_kernel \<and> active_irq \<in> non_kernel_IRQs) | ||
| if is_masked active_irq \<or> active_irq = irqInvalid \<or> (in_kernel \<and> active_irq \<in> non_kernel_IRQs) | ||
| then return None | ||
| else return ((Some active_irq) :: irq option) |
There was a problem hiding this comment.
For this commit (riscv machine: fix getActiveIRQ) I have an irrational bias towards a non-fix title. Something like riscv machine: use irqInvalid in getActiveIRQ maybe? I have traumatic experiences where saying something was "fixed" was like taunting the gods. Saying it was updated to use irqInvalid instead of the incorrect 0xFF feels less drastic.
Also typo in commit message: RSICV64
|
|
||
| (* Sufficiently aligned for irq type + cte_level_bits *) | ||
| (* Sufficiently aligned for irq type + cte_level_bits. irq_len can be up to 10, depending on | ||
| platform, so we need alignment to at least 15 bits. *) |
There was a problem hiding this comment.
alignment to at least 15 bits -> an alignment of at least 15 bits?
If you want "to at least", then I think it would fit in a "we need X to be aligned to at least ..." style sentence
| apply (clarsimp simp: up_ucast_inj_eq c_valid_cap_def ucast_eq_mask | ||
| cl_valid_cap_def mask_twice from_bool_0 | ||
| simp flip: LENGTH_irq_len_irqBits | ||
| split: if_split bool.split |
There was a problem hiding this comment.
indentation edge case: alignment is now to argument start for first two args (simp, simp flip), but : for third (split)
| lemma array_assertion_abs_irq: | ||
| "\<forall>s s'. (s, s') \<in> rf_sr \<and> True | ||
| \<and> (n s' \<le> 64 \<and> (x s' \<noteq> 0 \<longrightarrow> n s' \<noteq> 0)) | ||
| \<and> (n s' \<le> 2^irqBits \<and> (x s' \<noteq> 0 \<longrightarrow> n s' \<noteq> 0)) |
There was a problem hiding this comment.
did we ever decide if we prefer 2 ^ n over 2^n? I feel like we talked about this at some point, but completely forgot what the outcome was
There was a problem hiding this comment.
yeah we did, and I think you won :)
There was a problem hiding this comment.
Ahh, if I won then 2 ^ n please (also elsewhere in PR)
| apply (rule mask_eq_ucast_eq[where 'a="irq_len" and 'b="machine_word_len" and 'c="machine_word_len", | ||
| symmetric, simplified]) | ||
| apply (simp flip: LENGTH_irq_len_irqBits) | ||
| done |
There was a problem hiding this comment.
waaay nicer now; do the Arms use this nicer tech too?
There was a problem hiding this comment.
yes, most of that is from Arm :-)
| apply (cinit lift: irq_') | ||
| apply (simp add: getIRQState_def getInterruptState_def) | ||
| apply (rule_tac P="irq \<le> ucast Kernel_C.maxIRQ \<and> unat irq < (126::nat)" in ccorres_gen_asm) | ||
| apply (rule_tac P="irq \<le> ucast Kernel_C.maxIRQ \<and> unat irq < Kernel_Config.maxIRQ+1" in ccorres_gen_asm) |
| for x::machine_word | ||
| by (simp add: word_le_nat_alt maxIRQ_def Kernel_C.maxIRQ_def ucast_nat_def unat_ucast) | ||
| unfolding maxIRQ_def | ||
| by (simp add: Kernel_C_maxIRQ maxIRQ_ucast_toEnum_eq_irq irq_machine_le_maxIRQ_irq) |
There was a problem hiding this comment.
I'm a little confused by this block. unfolding maxIRQ_def will take it from the Platform one to the Kernel_Config one, and then maxIRQ_ucast_toEnum_eq_irq and irq_machine_le_maxIRQ_irq are Kernel_Config lemmas? If so, why don't they need the Kernel_Config. prefix here?
There was a problem hiding this comment.
The lemmas are fine to use, that's why we make them. They never need the Kernel_Config prefix (unless the name is ambiguous somehow)
There was a problem hiding this comment.
Right, so it's a case of "we don't have all the same theorems on Platform.maxIRQ because that would be duplication, and we can't just have everyone use the Kernel_Config ones uniformly because x86"
There was a problem hiding this comment.
Correct, yes. This is CRefine, so everything is fully arch specific, and going via Platform.maxIRQ wouldn't really help anything.
| value_type irq_array_size = "Suc (unat Kernel_C.maxIRQ)" | ||
| lemma Kernel_C_maxIRQ: | ||
| "Kernel_C.maxIRQ = Kernel_Config.maxIRQ" | ||
| by (simp add: Kernel_C.maxIRQ_def Kernel_Config.maxIRQ_def) |
There was a problem hiding this comment.
I'm wondering... this equation bypasses the Platform maxIRQ, which means it can only be applied after unfolding maxIRQ. Is this related to only having some properties on Platform maxIRQ, and the rest on Kernel_Config.maxIRQ, and so we end up in this strange land where we have to unfold to use the Kernel_Config lemmas, but if we unfolded we now need to relate the Kernel_C maxIRQ to Kernel_Config.maxIRQ?
| apply (rule equiv_valid_guard_imp, rule invoke_untyped_reads_respects_g_wcap) | ||
| apply (cases ui, clarsimp simp: cte_wp_at_caps_of_state valid_untyped_inv_wcap) | ||
| apply auto[1] | ||
| (* resolve schematics first, auto and fastforce only take forever *) |
There was a problem hiding this comment.
I first read this as (only take forever), which seemed very deep and poetic... on second read, I realised it's auto and (fastforce only) :P
| pt_offs_range_def cnode_offs_range_def | ||
| page_offs_range_def irq_len_val | ||
| s0_ptr_defs mask_def is_aligned_def, | ||
| unat_arith?\<close>\<close>) |
There was a problem hiding this comment.
While this is bravery award territory, there's also a lot of repetition, and the differences appear to only be 16 vs 17. Would it make sense here to deploy supply_local_method technology here?
There was a problem hiding this comment.
We could, and maybe I'm wrong, but I somewhat wanted to keep this ugly so that it doesn't look like this needs no cleanup. The current approach in these lemmas is "unfold everything to raw numbers, then use unat_arith to solve". Previously it was "unfold most of everything to raw numbers, do some word rules on magic numbers, then try to solve with big hammers". Neither of them is good. Ideally this would be mostly symbolic, which would make the whole theory a lot faster and hopefully easier to understand. It's a bit of a project, though.
|
Commits:
|
irq_cnode is unused, the real IRQ node is at init_irq_node_ptr. Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
Replace quadratic number of long-running arithmetic goals (with unat_arith) for pspace_distinct' with linear check on sorted list of addresses. This should generalise to abstract-level pspace_distinct as well in the future. Signed-off-by: Gerwin Klein <gerwin.klein@proofcraft.systems>
This PR mostly makes the treatment of
maxIRQgeneric in the proofs, which is the only not-yet-generic parameter that still varies between RISC-V platforms. This means, after this PR is merged, all seL4 RISC-V platforms are supported by the full RISC-V proof stack.Part of this update is fixing the machine interface definition
getActiveIRQfor RISC-V. It was suppressing IRQ 0xFF, which was an invalid IRQ for a previous architecture, not for RISC-V. There is no effect on the proof, since this is an assumption that was not made use of anywhere (invalidIRQ = 0 on RISC-V).PR for verified configs here: seL4/seL4#1698. I'm planning to make a separate PR for adding some of these to CI one both PRs are merged.