From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v2 3/5] libxc: create unmapped initrd in domain builder if supported Date: Fri, 2 Oct 2015 16:46:52 +0200 Message-ID: <560E98DC.4030805@suse.com> References: <1443764987-23639-1-git-send-email-jgross@suse.com> <1443764987-23639-4-git-send-email-jgross@suse.com> <1443790779.11707.95.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: <1443790779.11707.95.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 , xen-devel@lists.xen.org, ian.jackson@eu.citrix.com, stefano.stabellini@eu.citrix.com, wei.liu2@citrix.com List-Id: xen-devel@lists.xenproject.org On 10/02/2015 02:59 PM, Ian Campbell wrote: > On Fri, 2015-10-02 at 07:49 +0200, Juergen Gross wrote: >> In case the kernel of a new pv-domU indicates it is supporting an >> unmapped initrd, don't waste precious virtual space for the initrd, >> but allocate only guest physical memory for it. >> >> Signed-off-by: Juergen Gross >> --- >> tools/libxc/xc_dom_core.c | 22 ++++++++++++++++++++-- >> 1 file changed, 20 insertions(+), 2 deletions(-) >> >> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c >> index b510bbd..85b531a 100644 >> --- a/tools/libxc/xc_dom_core.c >> +++ b/tools/libxc/xc_dom_core.c >> @@ -1019,8 +1019,9 @@ int xc_dom_build_image(struct xc_dom_image *dom) >> if ( dom->kernel_loader->loader(dom) != 0 ) >> goto err; >> >> - /* load ramdisk */ >> - if ( dom->ramdisk_blob ) >> + /* Load ramdisk if initial mapping required. */ >> + if ( dom->ramdisk_blob && >> + (!dom->parms.mod_start_pfn || dom->ramdisk_seg.vstart) ) >> { >> if ( xc_dom_build_ramdisk(dom) != 0 ) >> goto err; >> @@ -1063,6 +1064,23 @@ int xc_dom_build_image(struct xc_dom_image *dom) >> __FUNCTION__, dom->virt_alloc_end); >> DOMPRINTF("%-20s: virt_pgtab_end : 0x%" PRIx64 "", >> __FUNCTION__, dom->virt_pgtab_end); >> + >> + /* Prepare allocating unmapped memory. */ >> + if ( dom->virt_pgtab_end ) >> + dom->virt_alloc_end = dom->virt_pgtab_end; >> + >> + /* Load ramdisk if no initial mapping required. */ >> + if ( dom->ramdisk_blob && !dom->ramdisk_seg.vstart && >> + dom->parms.mod_start_pfn ) >> + { >> + if ( xc_dom_build_ramdisk(dom) != 0 ) >> + goto err; >> + dom->flags |= SIF_MOD_START_PFN; >> + dom->ramdisk_seg.vend = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; >> + dom->ramdisk_seg.vstart = dom->ramdisk_seg.pfn; >> + dom->ramdisk_seg.vend += dom->ramdisk_seg.vstart; > > This seems like it is trying to do something clever, like partially > reversing something which the xc_dom_alloc_segment call in > xc_dom_build_ramdisk has done. It's just changing the boundaries of the initrd to fit the interface for it being not mapped (indicated by the SIF_MOD_START_PFN flag). > It looks like the vend handling in particular is just a complicated way of > subtracting vstart and adding pfn, with the aim of rebasing from virt to > phys world. Hmm, not more complicated as: len = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; dom->ramdisk_seg.vstart = dom->ramdisk_seg.pfn; dom->ramdisk_seg.vend = dom->ramdisk_seg.pfn + len; I have to admit that above variant might be easier to understand. :-) > Plus vstart/end are addresses, while presumably pfn is a page number, so > I'm confused about that as well. The naming is irritating, yes. I think I'll change this when I'm modifying the allocation interface (see my answer to patch 5). > I'm also not clear how/where the virtual mapping is avoided, given that > this code here does strictly more than the original code above which is now > made conditional does. The page tables are built before. So they don't cover the initrd memory. Juergen