From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4.1] libxc: Defer initialization of start_page for HVM guests Date: Fri, 8 Jan 2016 09:25:09 -0500 Message-ID: <568FC6C5.4020403@oracle.com> References: <1452204704-6230-1-git-send-email-boris.ostrovsky@oracle.com> <1452247006.21055.294.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1452247006.21055.294.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, wei.liu2@citrix.com Cc: jgross@suse.com, andrew.cooper3@citrix.com, xen-devel@lists.xen.org, roger.pau@citrix.com List-Id: xen-devel@lists.xenproject.org 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