From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH v3 6/9] libxc: create unmapped initrd in domain builder if supported Date: Wed, 28 Oct 2015 18:07:30 +0100 Message-ID: <563100D2.5040202@suse.com> References: <1444741878-16610-1-git-send-email-jgross@suse.com> <1444741878-16610-7-git-send-email-jgross@suse.com> <20151028161142.GK18674@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20151028161142.GK18674@zion.uk.xensource.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: Wei Liu Cc: roger.pau@citrix.com, stefano.stabellini@eu.citrix.com, ian.jackson@eu.citrix.com, Ian.Campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 10/28/2015 05:11 PM, Wei Liu wrote: > On Tue, Oct 13, 2015 at 03:11:15PM +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/include/xc_dom.h | 5 +++++ >> tools/libxc/xc_dom_core.c | 19 +++++++++++++++++-- >> tools/libxc/xc_dom_x86.c | 8 ++++---- >> 3 files changed, 26 insertions(+), 6 deletions(-) >> >> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h >> index b0120a6..fa772a9 100644 >> --- a/tools/libxc/include/xc_dom.h >> +++ b/tools/libxc/include/xc_dom.h >> @@ -94,6 +94,11 @@ struct xc_dom_image { >> xen_pfn_t pfn_alloc_end; >> xen_vaddr_t virt_alloc_end; >> xen_vaddr_t bsd_symtab_start; >> + >> + /* initrd parameters as specified in start_info page */ >> + unsigned long initrd_start; >> + unsigned long initrd_len; >> + >> unsigned int alloc_bootstack; >> xen_vaddr_t virt_pgtab_end; >> >> diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c >> index 8e1e17f..15e9fa3 100644 >> --- a/tools/libxc/xc_dom_core.c >> +++ b/tools/libxc/xc_dom_core.c >> @@ -1041,6 +1041,7 @@ static int xc_dom_build_ramdisk(struct xc_dom_image *dom) >> int xc_dom_build_image(struct xc_dom_image *dom) >> { >> unsigned int page_size; >> + bool unmapped_initrd; >> >> DOMPRINTF_CALLED(dom->xch); >> >> @@ -1064,11 +1065,15 @@ 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. */ >> + unmapped_initrd = dom->parms.unmapped_initrd && !dom->ramdisk_seg.vstart; > > A minor suggestion, the comment is describing the reverse logic of the > statement that immediately follows it, can you make the comment and > statement match? Sure. > I think I manage to work this out: if ramdisk_seg.vstart is set that > means upper layer wants the ramdisk to be mapped at that address (ARM is > doing that), so in that case even if kernel supports unmapped initrd we > should still map the ramdisk. Correct me if I'm wrong. That's how it was meant to be. > The rest of code looks correct. With that understand (and whether you > follow my minor suggestion or not): > > Acked-by: Wei Liu > Thanks, Juergen