[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v3 2/2] libxc: Defer initialization of start_page for HVM guests
On 01/07/2016 12:06 PM, Ian Campbell wrote:
On Thu, 2016-01-07 at 17:54 +0100, Roger Pau Monnà wrote:
El 07/01/16 a les 15.47, Boris Ostrovsky ha escrit:
On 01/07/2016 06:43 AM, Roger Pau Monnà wrote:
El 06/01/16 a les 21.03, Boris Ostrovsky ha escrit:
static int bootlate_hvm(struct xc_dom_image *dom)
{
- DOMPRINTF("%s: doing nothing", __func__);
+ uint32_t domid = dom->guest_domid;
+ xc_interface *xch = dom->xch;
+
+ if ( !dom->device_model )
+ {
+ struct hvm_start_info *start_info;
+ size_t start_info_size = sizeof(*start_info);
+ void *start_page;
+ struct hvm_modlist_entry *modlist;
+ size_t cmdline_size = 0;
+
+ if ( dom->cmdline )
+ {
+ cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 8);
+ start_info_size += cmdline_size;
+ }
+ if ( dom->ramdisk_blob )
+ start_info_size += sizeof(*modlist); /* Limited to one
module. */
The size calculations are duplicated, could you either stash
start_info_size into xc_dom_image, or simply do the memory allocation
(xc_dom_alloc_segment) inside of bootlate_hvm? (I think the latter
would
be better if possible).
I didn't want to do the first because we'd use that information (two
pieces --- we need to to know both the size of the extra chunk and
where
modlist starts) only once and it's not on a critical path. You can, of
course, argue that we increase text size.
It's just that I don't want to get them out of synch, and that's what
usually happens when you perform the same calculations in two different
places, one might get out of synch with the other.
The problem with the second approach is that while it does seem to work
I don't know whether we can delay allocations until bootlate(): notice
how we print dom->virt_alloc_end in xc_dom_build_image() which to me
indicates some "finality" as far as allocations for domain are
concerned.
For PV guests it probably matters, because we have to build the page
tables and the p2m, for HVMlite guests I don't think it matters at all
(or at least I don't see any obvious reason).
Another third option is to place the code that performs the size
calculations inside of a function that's called by both (bootlate_hvm
and alloc_magic_pages_hvm).
The call to xc_dom_alloc_segment initialises a xc_dom_seg, which in this
case alloc_magic just throws away. If the location/size of that segment is
required elsewhere then the normal approach would be to add it to the
handle struct (cf dom->{kernel,ramdisk}_seg et al) and to consume it in the
other places.
xc_dom_alloc_segment() also updates domain's pfn_alloc_end and virt_alloc_end,
which is what I was thinking about (i.e. that updating those values bootlate()
time may be too late).
My concerns would be the size calculation and the pointer figuring out
getting out of sync or the size of one of the inputs changing between the
size calculation and the use (i.e. by a future patch moving something
around). So the place which figures out the pointers would also need to
bounds check against the mapped region's size.
So then I think the safest is to stash the value of cmdline length in
xc_dom_image (and use strncpy).
For modlist we only have a single module (ramdisk) and I just realized
that we can actually use ramdisk_seg. In fact, it looks like we are
setting it up and then never use it for HVMlite.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
