Re: [Xen-devel] [PATCH v3 6/9] libxc: create unmapped initrd in domain builder if supported

[Juergen Gross <jgross@xxxxxxxx>]

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 <jgross@xxxxxxxx>
> > ---
> >   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 <wei.liu2@xxxxxxxxxx>
Thanks,

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel