From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4.2] libxc: Defer initialization of start_page for HVM guests Date: Fri, 8 Jan 2016 09:53:30 -0500 Message-ID: <568FCD6A.4080704@oracle.com> References: <1452205147-6320-1-git-send-email-boris.ostrovsky@oracle.com> <568F41B1.9070205@suse.com> <568FC56F.3050209@oracle.com> <568FC80D.5020200@suse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <568FC80D.5020200@suse.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: Juergen Gross , ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, ian.campbell@citrix.com, wei.liu2@citrix.com Cc: andrew.cooper3@citrix.com, roger.pau@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 01/08/2016 09:30 AM, Juergen Gross wrote: > On 08/01/16 15:19, Boris Ostrovsky wrote: >> On 01/07/2016 11:57 PM, Juergen Gross wrote: >>> On 07/01/16 23:19, 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(). >>>> >>>> Since we now split allocation and mapping of the start_info segment >>>> let's stash it, along with cmdline length, in xc_dom_image so that we >>>> can check whether we are mapping correctly-sized range. >>>> >>>> We can also stop using xc_dom_image.start_info_pfn and leave it for >>>> PV(H) guests only. >>>> >>>> Signed-off-by: Boris Ostrovsky >>>> --- >>>> v4: >>>> * See the last two paragraphs from commit message above >>>> >>>> v4.1: >>>> * Inverted testing of start_info_size in bootlate_hvm(). >>>> >>>> v4.2 >>>> * Actually do what I said I'd do in 4.1 >>>> >>>> tools/libxc/include/xc_dom.h | 2 + >>>> tools/libxc/xc_dom_x86.c | 140 >>>> +++++++++++++++++++++++------------------ >>>> 2 files changed, 80 insertions(+), 62 deletions(-) >>>> >>>> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h >>>> index 2460818..cac4698 100644 >>>> --- 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; >>>> uint32_t f_requested[XENFEAT_NR_SUBMAPS]; >>>> /* info from (elf) kernel image */ >>>> @@ -91,6 +92,7 @@ struct xc_dom_image { >>>> struct xc_dom_seg p2m_seg; >>>> struct xc_dom_seg pgtables_seg; >>>> struct xc_dom_seg devicetree_seg; >>>> + struct xc_dom_seg start_info_seg; /* HVMlite only */ >>> Instead of adding HVM specific members here, you could make use of >>> dom.arch_private and use just a local structure defined in xc_dom_x86.c. >> I did consider this but since we already keep type-specific segments in >> this structure (e.g. p2m_seg) decided to add an explicit segment for >> HVMlite. > But p2m_seg is accessed from multiple sources, while cmdline_size and > start_info_seg would be local to xc_dom_x86.c > > BTW: thanks for the hint - I'll have a look whether p2m_seg can't be > moved to arch_private... > >> Besides, I think to properly use it we'd need to add an arch hook and >> IMHO it's not worth the trouble in this case. > Why would you need another arch hook? Just add the arch_private_size > member to struct xc_dom_arch and everything is set up for you. Look > how it is handled for the pv case in xc_dom_x86.c So it is already hooked up, I didn't notice that we do register xc_hvm_32, even though arch_private_size is 0. This would be a type-specific area though, not arch-specific as the name implies. So perhaps xc_dom_image_x86 should be modified to include type-specific structures (via a union). -boris