Re: [PATCH v3 2/2] libxc: Defer initialization of start_page for HVM guests

["Roger Pau Monné" <roger.pau@citrix.com>]

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).

Roger.