lazy Scheduling Context rebind + improve tool <-> monitor ABI#439
Open
dreamliner787-9 wants to merge 1 commit intoseL4:mainfrom
Open
lazy Scheduling Context rebind + improve tool <-> monitor ABI#439dreamliner787-9 wants to merge 1 commit intoseL4:mainfrom
dreamliner787-9 wants to merge 1 commit intoseL4:mainfrom
Conversation
d10d766 to
417fbb1
Compare
Signed-off-by: Bill Nguyen <bill.nguyen@unsw.edu.au>
417fbb1 to
d500f22
Compare
Indanz
reviewed
Mar 16, 2026
There was a problem hiding this comment.
First, this really should be two separate commits, not lumped together like this.
Secondly, I'm missing creating a notifications object for passive servers that don't otherwise have one already, and binding the SC to the notification object if the PD is passive.
That last thing is really the only change you need compared to active PDs for setup.
Only other change needed is that you bind the SC when restarting the task, but I don't think Microkit does that yet, but rust-sel4 does I think.
Comment on lines
+896
to
+918
| /* If there are passive PDs in the system, lazily rebind it's Scheduling Context and resume TCB. | ||
| * To workaround https://github.com/seL4/seL4/issues/1617 by applying https://github.com/seL4/seL4/pull/523 | ||
| */ | ||
| for (unsigned idx = 0; idx < pd_metadata_len; idx++) { | ||
| if (pd_metadata[idx].passive) { | ||
| seL4_Error err = seL4_SchedContext_Bind(BASE_SCHED_CONTEXT_CAP + idx, BASE_NOTIFICATION_CAP + idx); | ||
| if (err != seL4_NoError) { | ||
| puts("MON|ERROR: could not bind scheduling context to notification object\n"); | ||
| continue; | ||
| } | ||
|
|
||
| err = seL4_TCB_Resume(BASE_PD_TCB_CAP + idx); | ||
| if (err != seL4_NoError) { | ||
| puts("MON|ERROR: could not bind resume passive PD TCB\n"); | ||
| continue; | ||
| } | ||
|
|
||
| puts("MON|INFO: PD '"); | ||
| puts(pd_metadata[idx].name); | ||
| puts("' is now passive!\n"); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
There is no need to do this resume and you can bind the SC to the notification when you create the notification for a passive thread. This feels like the wrong place for that.
Comment on lines
+982
to
+983
| // Passive PDs are resumed by the Monitor | ||
| pd_tcb.extra.resume = !pd.passive; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Applied seL4/seL4#523 to workaround seL4/seL4#1617, and improved tool <-> monitor ABI by creating a metadata struct per PD, rather than adding a new symbol for every type of metadata.