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

capDL-tool, capdl-loader-app: Add support for binding notifications for TCBs #62

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

nspin
Copy link
Member

@nspin nspin commented Feb 14, 2024

This will be required by Microkit.

@lsf37 lsf37 requested a review from corlewis February 14, 2024 21:54
@lsf37
Copy link
Member

lsf37 commented Feb 14, 2024

This might have verification impact for the system initialiser. @corlewis do we need to do anything special in the tools or is this just excluded in spec wellformedness for sys-init?

Do you think it would be hard to add to the sys-init proofs?

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

The code changes as such look good to me.

@corlewis
Copy link
Member

This might have verification impact for the system initialiser. @corlewis do we need to do anything special in the tools or is this just excluded in spec wellformedness for sys-init?

Do you think it would be hard to add to the sys-init proofs?

Bound notifications are currently excluded by wellformedness, so there won't be any immediate issues with sys-init.

There's two parts to adding it to the verified system initialiser, creating a wp rule for seL4_TCB_BindNotification, and updating the specification and correctness proof. The second part shouldn't be too hard, but while I don't have any experience with the capDL-api proofs, my guess is that the first could be difficult.

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 capDL-tool will need some changes in PrintIsabelle.hs so that it produces output that is consistent with the verification capDL specification. In particular, the verification specification doesn't know about the MCS caps that can be in a TCBs CNode, and so its equivalent to tcbBoundNotificationSlot is set to 6 instead of 8. We'll need to do this translation somewhere in PrintIsabelle.hs.

@corlewis
Copy link
Member

The capDL-tool will need some changes in PrintIsabelle.hs so that it produces output that is consistent with the verification capDL specification. In particular, the verification specification doesn't know about the MCS caps that can be in a TCBs CNode, and so its equivalent to tcbBoundNotificationSlot is set to 6 instead of 8. We'll need to do this translation somewhere in PrintIsabelle.hs.

Hmm, I guess that an easier alternative would be to update the verification specification's TCB slots instead. I don't think that that would affect any of the proofs and we could leave a comment there explaining the unexpected ordering.

That would probably be more maintainable as well. I might give that a go to make sure that there aren't any proof impacts that I'm missing. I'll report back once I've done an initial pass.

@corlewis
Copy link
Member

Hmm, I guess that an easier alternative would be to update the verification specification's TCB slots instead. I don't think that that would affect any of the proofs and we could leave a comment there explaining the unexpected ordering.

That would probably be more maintainable as well. I might give that a go to make sure that there aren't any proof impacts that I'm missing. I'll report back once I've done an initial pass.

The proof update was a little bit more involved than I was first guessing, but it wasn't too bad. See seL4/l4v#716.

However, while this is still the easiest way forwards at the moment, I've changed my mind and now think the best maintainability would be to do the translation in capDL-tool. That would keep all of the changes in one place and not lead to needing to know to make a change to l4v after making a change to the TCB cap slots here in capdl.

I'm happy to go with what we have here though, and leave that for the future if/when we make further changes to the cap slots.

@lsf37
Copy link
Member

lsf37 commented Jun 26, 2024

@corlewis I'd be fine with merging seL4/l4v#716 and this PR here. It's relatively rare that we update these slots, so it shouldn't be too much of maintenance burden.

@lsf37 lsf37 self-assigned this Jun 26, 2024
Add support for binding notifications to TCBs.

Signed-off-by: Nick Spinale <[email protected]>
Signed-off-by: Gerwin Klein <[email protected]>
@lsf37 lsf37 force-pushed the pr/tcb-bound-notification branch from fc0755c to 553f366 Compare July 1, 2024 05:35
@lsf37 lsf37 merged commit 60d1c3c into seL4:master Jul 1, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants