Re: [PATCH v5 6/9] libxc: create unmapped initrd in domain builder if supported

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Nov 30, 2015 at 11:23:34AM +0100, Juergen Gross wrote:
> On 30/11/15 11:20, Wei Liu wrote:
> > On Thu, Nov 26, 2015 at 08:35:02AM +0100, Juergen Gross wrote:
> >> On 25/11/15 17:12, Boris Ostrovsky wrote:
> >>> On 11/12/2015 08:43 AM, 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.
> >>>
> >>> This patch breaks 32-bit pygrub.
> >>>
> >>> I am not 100% sure yet but it may be that only 64-bit guests are affected.
> >>>
> >>> With RHEL5 I get
> >>>     initrd extends beyond end of memory (0x780080eda000 > 0x40000000)
> >>
> >> I think I have found the problem. Can you verify the attached patch is
> >> working?
> >>
> >>
> >> Juergen
> >>
> > 
> >> From 11eaee2aa2291a1d56556d538ac23b8156cf3388 Mon Sep 17 00:00:00 2001
> >> From: Juergen Gross <jgross@suse.com>
> >> Date: Thu, 26 Nov 2015 08:32:26 +0100
> >> Subject: [PATCH] libxc: correct domain builder for 64 bit guest with 32 bit
> >>  tools
> >>
> >> Commit 8c45adec18e0512c3d34dcafb13414ecba21be6a ("create unmapped
> >> initrd in domain builder if supported") introduced an error for
> >> building a 64 bit guest with a 32 bit toolset.
> >>
> >> The initrd start address and size where stored in an unsigned long
> >> instead of using a 64 bit type.
> >>
> >> Signed-off-by: Juergen Gross <jgross@suse.com>
> >> ---
> >>  tools/libxc/include/xc_dom.h | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> >> index 2176216..370dddd 100644
> >> --- a/tools/libxc/include/xc_dom.h
> >> +++ b/tools/libxc/include/xc_dom.h
> >> @@ -99,8 +99,8 @@ struct xc_dom_image {
> >>      xen_vaddr_t bsd_symtab_start;
> >>  
> >>      /* initrd parameters as specified in start_info page */
> >> -    unsigned long initrd_start;
> >> -    unsigned long initrd_len;
> >> +    uint64_t initrd_start;
> >> +    uint64_t initrd_len;
> >>  
> > 
> > I think these should be of type xen_vaddr_t. Doesn't make a difference
> > in the end though.
> 
> xen_vaddr_t seems not to be appropriate. It can be either a virtual
> address or a pfn. So the type should be agnostic to any special
> semantics and just needs to be big enough for all cases.
> 

Fair enough.  I forgot that it can also be a pfn.

Wei.

> 
> Juergen
>