Re: [PATCH v4.1] libxc: Defer initialization of start_page for HVM guests

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 01/08/2016 04:56 AM, Ian Campbell wrote:
> On Thu, 2016-01-07 at 17:11 -0500, Boris Ostrovsky wrote:
>> With commit 8c45adec18e0 ("libxc: create unmapped initrd in domain
>> builder if supported") location of ramdisk may not be available to
>> HVMlite guests by the time alloc_magic_pages_hvm() is invoked if the
>> guest supports unmapped initrd.
>>
>> So let's move ramdisk info initialization (along with a few other
>> operations that are not directly related to allocating magic/special
>> pages) from alloc_magic_pages_hvm() to bootlate_hvm().
> The "few other operations" are just the hvm_info init for standard HVM by
> this iteration?

For HVM yes, but we also move command string initialization for HVMlite.

>
>> --- a/tools/libxc/include/xc_dom.h
>> +++ b/tools/libxc/include/xc_dom.h
>> @@ -71,6 +71,7 @@ struct xc_dom_image {
>>   
>>       /* arguments and parameters */
>>       char *cmdline;
>> +    size_t cmdline_size;
> Perhaps /* Including NULL and padding/alignment */ ?
>
> (That's the sort of thing I don't mind doing on commit if there is
> agreement and no other changes required)

Yes, please add this (unless you want to see v5, v5.1 and v5.2)

>
>> @@ -1095,8 +1044,8 @@ static int vcpu_hvm(struct xc_dom_image *dom)
>>       /* Set the IP. */
>>       bsp_ctx.cpu.rip = dom->parms.phys_entry;
>>   
>> -    if ( dom->start_info_pfn )
>> -        bsp_ctx.cpu.rbx = dom->start_info_pfn << PAGE_SHIFT;
>> +    if ( dom->start_info_seg.pfn )
>> +        bsp_ctx.cpu.rbx = dom->start_info_seg.pfn << PAGE_SHIFT;
> Just to check: dom->start_info_pfn is unused/nil in the regular-hvm case,
> correct (i.e. this code is dm-list specific both before and after)?

Yes, start_info_pfn is only used by PV(H) and since the structure is 
memset to zero during allocation the 'if' clause is only executed for 
HVMlite.

>
>> +    else
>> +    {
>> +        void *hvm_info_page;
>> +
>> +        if ( (hvm_info_page = xc_map_foreign_range(
>> +                  xch, domid, PAGE_SIZE, PROT_READ | PROT_WRITE,
>> +                  HVM_INFO_PFN)) == NULL )
>> +            return -1;
>> +        build_hvm_info(hvm_info_page, dom);
>> +        munmap(hvm_info_page, PAGE_SIZE);
>> +    }
> Did you test regular HVM guests after this movement?

Yes, I did.

-boris