Re: [Xen-devel] [PATCH v2 2/5] xen: add generic flag to elf_dom_parms indicating support of unmapped initrd

[Juergen Gross <jgross@xxxxxxxx>]

On 10/02/2015 11:37 AM, Andrew Cooper wrote:
> On 02/10/15 06:49, Juergen Gross wrote:
> > Support of an unmapped initrd is indicated by the kernel of the domain
> > via elf notes. In order not to have to use raw elf data in the tools
> > for support of an unmapped initrd add a flag to the parsed data area
> > to indicate the kernel supporting this feature.
> >
> > Switch using this flag in the hypervisor domain builder.
> >
> > Cc: Keir Fraser <keir@xxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > Suggested-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   xen/arch/x86/domain_build.c        | 4 ++--
> >   xen/common/libelf/libelf-dominfo.c | 3 +++
> >   xen/include/xen/libelf.h           | 1 +
> >   3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> > index c2ef87a..a02c9fb 100644
> > --- a/xen/arch/x86/domain_build.c
> > +++ b/xen/arch/x86/domain_build.c
> > @@ -353,7 +353,7 @@ static unsigned long __init compute_dom0_nr_pages(
> >
> >           vstart = parms->virt_base;
> >           vend = round_pgup(parms->virt_kend);
> > -        if ( !parms->elf_notes[XEN_ELFNOTE_MOD_START_PFN].data.num )
> > +        if ( !parms->mod_start_pfn )
> >               vend += round_pgup(initrd_len);
> >           end = vend + nr_pages * sizeof_long;
> >
> > @@ -1037,7 +1037,7 @@ int __init construct_dom0(
> >       v_start          = parms.virt_base;
> >       vkern_start      = parms.virt_kstart;
> >       vkern_end        = parms.virt_kend;
> > -    if ( parms.elf_notes[XEN_ELFNOTE_MOD_START_PFN].data.num )
> > +    if ( parms.mod_start_pfn )
> >       {
> >           vinitrd_start  = vinitrd_end = 0;
> >           vphysmap_start = round_pgup(vkern_end);
> > diff --git a/xen/common/libelf/libelf-dominfo.c 
> > b/xen/common/libelf/libelf-dominfo.c
> > index 3de1c23..31d8436 100644
> > --- a/xen/common/libelf/libelf-dominfo.c
> > +++ b/xen/common/libelf/libelf-dominfo.c
> > @@ -190,6 +190,9 @@ elf_errorstatus elf_xen_parse_note(struct elf_binary *elf,
> >       case XEN_ELFNOTE_INIT_P2M:
> >           parms->p2m_base = val;
> >           break;
> > +    case XEN_ELFNOTE_MOD_START_PFN:
> > +        parms->mod_start_pfn = !!val;
> > +        break;
> >       case XEN_ELFNOTE_PADDR_OFFSET:
> >           parms->elf_paddr_offset = val;
> >           break;
> > diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
> > index d7045f6..07b96a3 100644
> > --- a/xen/include/xen/libelf.h
> > +++ b/xen/include/xen/libelf.h
> > @@ -423,6 +423,7 @@ struct elf_dom_parms {
> >       char loader[16];
> >       enum xen_pae_type pae;
> >       bool bsd_symtab;
> > +    bool mod_start_pfn;
> The _pfn suffix here is confusing given the type of bool.
>
> Perhaps "has_initrd" is a better choice of name?  The rest of the patch
> looks fine.
Hmm, I just followed the naming of the note index in the raw data:
XEN_ELFNOTE_MOD_START_PFN. "has_initrd" would be completely misleading:
the flag doesn't indicate the support of an initrd, but the support of
an initrd (or more general: module as understood by grub) not covered by
the initial kernel mapping and due to this specified by it's starting
pfn instead it's virtual address.


Juergen

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