Add support for prefilling memory regions from data files at compile time#434
Add support for prefilling memory regions from data files at compile time#434dreamliner787-9 wants to merge 3 commits intoseL4:mainfrom
Conversation
dreamliner787-9
force-pushed
the
issue_248
branch
from
af1ca0d to
b99e2f3
Compare
| if allow_setvar { | ||
| attrs.push("setvar_vaddr"); | ||
| attrs.push("setvar_size"); | ||
| attrs.push("setvar_prefill_size"); |
There was a problem hiding this comment.
what's differnce between prefill size and size?
There was a problem hiding this comment.
setvar_size is the size of the entire memory region, whereas setvar_prefill_size is the size of the data blob that was prefilled.
There was a problem hiding this comment.
why would you ever need the size of the memory region? (presumably by that you mean a page aligned size?) is not the size of the data good enough?
unless you're saying the size of the memory region has to be specified separately to the size of the data, in which case (I don't know why you would want this?) you should probably add more checks for handling the overflow case nicely. but again I don't know why you wouldn't ever just want the actual size.
There was a problem hiding this comment.
Yes the size of the MR must be specified separately to the size of the data, since they may be different. setvar_size was an existing feature so I didn't change it. The overflow condition is checked in https://github.com/seL4/microkit/pull/434/changes#diff-e9fc0f34d513cf693bc173e1ee69c3b08a3f7be618e5a70d4d6c08faf8aa3955R1296
There was a problem hiding this comment.
But why? Why would this ever matter? This is only ever going to add more work for the user who has to match the size to the prefill.
There was a problem hiding this comment.
yes. I don't see a case where that wouldn't be the desired behaviour.
There was a problem hiding this comment.
I see, one case I thought of when doing this was that the user might use the prefill data for something, then "reclaim" the MR for other things. Which is more useful if the MR is large to begin with. Such as having a large guest RAM MR, prefilling it with the kernel and initrd at the start. Then at init(), memmove() it to the right place. Plus, if the user is responsible for sizing the MR, they have control over the page size and whatnot.
But now that I think about it, this might not be useful for PD restart and whatnot. I can change it so that prefilled MRs are automatically sized. What do you think?
There was a problem hiding this comment.
It's possible that that is useful, yes.
Maybe it makes sense for it to be an optional thing?
There was a problem hiding this comment.
Yes I can make it an optional thing
There was a problem hiding this comment.
I've made specifying size of a prefilled MR optional.
dreamliner787-9
force-pushed
the
issue_248
branch
2 times, most recently
from
c11e48d to
d29824c
Compare
Signed-off-by: Bill Nguyen <bill.nguyen@unsw.edu.au>
Signed-off-by: Bill Nguyen <bill.nguyen@unsw.edu.au>
Signed-off-by: Bill Nguyen <bill.nguyen@unsw.edu.au>
dreamliner787-9
force-pushed
the
issue_248
branch
from
d29824c to
40a7f15
Compare
2 participants
The Memory Region (MR) prefill data file path is specified in a
prefill_pathattribute of amemory_regionelement. Then any PDs that maps the MR can learn of the prefilled data size viasetvar_prefill_size. For prefilled MRs, it is now optional to specify the size.The tool will always prefill data from the start of the MR. This is useful in cases such as packing Linux kernel and initial ramdisk images into a VMM PD.
Additional thoughts adjacent to #422 and au-ts/sddf#622 (comment): perhaps it would be better if we get rid of all the "setvar" and just have a "bootinfo" structure that we pass to
init().Closes #248