Re: [PATCH 4/5] libxc: split p2m allocation in domain builder from other magic pages

[Ian Campbell <ian.campbell@citrix.com>]

On Fri, 2015-10-02 at 05:55 +0200, Juergen Gross wrote:
> On 10/01/2015 02:47 PM, Ian Campbell wrote:
> > On Fri, 2015-09-11 at 14:32 +0200, Juergen Gross wrote:
> > > Add an own function to allocate the p2m list in the domain builder in
> > 
> > I'm not sure what you mean by "own" here. Perhaps "hook" or "callback"?
> 
> Before the p2m list allocation was done in the alloc_magic_pages hook.
> I'm just splitting this function.

I get that, but "own" is not the right word for this.

Perhaps you meant "separate (hook|function)" or "specific (hook|function)"
or something like that?

Perhaps something like:

    Refactor allocation of the p2m list out of alloc_magic_pages into a new
    alloc_p2m_list hook.

?

Ian.

> 
> 
> Juergen
> 
> > 
> > > order to prepare allocating the p2m list outside of the initial
> > > kernel
> > > mapping. This will be needed to support loading domains with huge
> > > memory (>512 GB).
> > > 
> > > Signed-off-by: Juergen Gross <jgross@suse.com>
> > 
> > Other than the above it otherwise LGTM, so with that clarification:
> > 
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > 
> > > ---
> > >   tools/libxc/include/xc_dom.h |  1 +
> > >   tools/libxc/xc_dom_core.c    |  3 +++
> > >   tools/libxc/xc_dom_x86.c     | 11 ++++++++++-
> > >   3 files changed, 14 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tools/libxc/include/xc_dom.h
> > > b/tools/libxc/include/xc_dom.h
> > > index a4ca8a4..43b1eab 100644
> > > --- a/tools/libxc/include/xc_dom.h
> > > +++ b/tools/libxc/include/xc_dom.h
> > > @@ -199,6 +199,7 @@ void xc_dom_register_loader(struct xc_dom_loader
> > > *loader);
> > >   struct xc_dom_arch {
> > >       /* pagetable setup */
> > >       int (*alloc_magic_pages) (struct xc_dom_image * dom);
> > > +    int (*alloc_p2m_list) (struct xc_dom_image * dom);
> > >       int (*count_pgtables) (struct xc_dom_image * dom);
> > >       int (*setup_pgtables) (struct xc_dom_image * dom);
> > > 
> > > diff --git a/tools/libxc/xc_dom_core.c b/tools/libxc/xc_dom_core.c
> > > index bb668b1..81b642e 100644
> > > --- a/tools/libxc/xc_dom_core.c
> > > +++ b/tools/libxc/xc_dom_core.c
> > > @@ -1048,6 +1048,9 @@ int xc_dom_build_image(struct xc_dom_image
> > > *dom)
> > >       }
> > > 
> > >       /* allocate other pages */
> > > +    if ( dom->arch_hooks->alloc_p2m_list &&
> > > +         dom->arch_hooks->alloc_p2m_list(dom) != 0 )
> > > +        goto err;
> > >       if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 )
> > >           goto err;
> > >       if ( dom->arch_hooks->count_pgtables )
> > > diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> > > index 3d40fa4..91d4e49 100644
> > > --- a/tools/libxc/xc_dom_x86.c
> > > +++ b/tools/libxc/xc_dom_x86.c
> > > @@ -438,7 +438,7 @@ pfn_error:
> > > 
> > >   /* ----------------------------------------------------------------
> > > -----
> > > --- */
> > > 
> > > -static int alloc_magic_pages(struct xc_dom_image *dom)
> > > +static int alloc_p2m_list(struct xc_dom_image *dom)
> > >   {
> > >       size_t p2m_alloc_size = dom->p2m_size * dom->arch_hooks
> > > ->sizeof_pfn;
> > > 
> > > @@ -450,6 +450,13 @@ static int alloc_magic_pages(struct xc_dom_image
> > > *dom)
> > >       if ( dom->p2m_guest == NULL )
> > >           return -1;
> > > 
> > > +    return 0;
> > > +}
> > > +
> > > +/* -----------------------------------------------------------------
> > > ----
> > > --- */
> > > +
> > > +static int alloc_magic_pages(struct xc_dom_image *dom)
> > > +{
> > >       /* allocate special pages */
> > >       dom->start_info_pfn = xc_dom_alloc_page(dom, "start info");
> > >       dom->xenstore_pfn = xc_dom_alloc_page(dom, "xenstore");
> > > @@ -676,6 +683,7 @@ static struct xc_dom_arch xc_dom_32_pae = {
> > >       .page_shift = PAGE_SHIFT_X86,
> > >       .sizeof_pfn = 4,
> > >       .alloc_magic_pages = alloc_magic_pages,
> > > +    .alloc_p2m_list = alloc_p2m_list,
> > >       .count_pgtables = count_pgtables_x86_32_pae,
> > >       .setup_pgtables = setup_pgtables_x86_32_pae,
> > >       .start_info = start_info_x86_32,
> > > @@ -689,6 +697,7 @@ static struct xc_dom_arch xc_dom_64 = {
> > >       .page_shift = PAGE_SHIFT_X86,
> > >       .sizeof_pfn = 8,
> > >       .alloc_magic_pages = alloc_magic_pages,
> > > +    .alloc_p2m_list = alloc_p2m_list,
> > >       .count_pgtables = count_pgtables_x86_64,
> > >       .setup_pgtables = setup_pgtables_x86_64,
> > >       .start_info = start_info_x86_64,
> > 
>