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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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?

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.

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>

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