Re: [PATCH RFC 04/12] xen/x86: split Dom0 build into PV and PVHv2

[Roger Pau Monne <roger.pau@citrix.com>]

On Fri, Jul 29, 2016 at 06:57:08PM +0100, Andrew Cooper wrote:
> On 29/07/16 17:28, Roger Pau Monne wrote:
> > Split the Dom0 builder into two different functions, one for PV (and classic
> > PVH), and another one for PVHv2. Introduce a new command line parameter,
> > dom0hvm in order to request the creation of a PVHv2 Dom0.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > ---
> >  xen/arch/x86/domain_build.c | 27 ++++++++++++++++++++++++++-
> >  xen/arch/x86/setup.c        |  9 +++++++++
> 
> A patch to docs/misc/xen-command-line.markdown please.

OK, I wasn't really sure if we want to introduce a new command line option, 
or just use dom0pvh. In any case I will add it, we can always alias dom0pvh 
to dom0pvh (or the other way around) when classic PVH support is 
removed.
 
> >  2 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> > index 09d79be..c0ef40f 100644
> > --- a/xen/arch/x86/domain_build.c
> > +++ b/xen/arch/x86/domain_build.c
> > @@ -952,7 +952,7 @@ static int __init setup_permissions(struct domain *d)
> >      return rc;
> >  }
> >  
> > -int __init construct_dom0(
> > +static int __init construct_dom0_pv(
> >      struct domain *d,
> >      const module_t *image, unsigned long image_headroom,
> >      module_t *initrd,
> > @@ -1647,6 +1647,31 @@ out:
> >      return rc;
> >  }
> >  
> > +static int __init construct_dom0_hvm(struct domain *d, const module_t *image,
> > +                                     unsigned long image_headroom,
> > +                                     module_t *initrd,
> > +                                     void *(*bootstrap_map)(const module_t *),
> > +                                     char *cmdline)
> > +{
> > +
> > +    printk("** Building a PVH Dom0 **\n");
> 
> Some naming curiosities here, especially given the parameter name.

Hm, AFAIK we agreed on keeping the 'PVH' naming, but since internally Xen 
has no concept of 'PVH' I think the constructor is better named as HVM (and 
in fact if PVH wasn't there before I would just consider this a HVM Dom0).

If people prefer HVM I can certainly change it, but I think it's going to 
get messy.

> > +
> > +    return 0;
> > +}
> > +
> > +int __init construct_dom0(struct domain *d, const module_t *image,
> > +                          unsigned long image_headroom, module_t *initrd,
> > +                          void *(*bootstrap_map)(const module_t *),
> > +                          char *cmdline)
> > +{
> > +
> > +    return is_hvm_domain(d) ?
> > +            construct_dom0_hvm(d, image, image_headroom, initrd,
> > +                               bootstrap_map, cmdline) :
> > +            construct_dom0_pv(d, image, image_headroom, initrd, bootstrap_map,
> > +                              cmdline);
> 
> This could be slightly less awkwardly split as:
> 
> (is_hvm_domain(d) ? construct_dom0_hvm : construct_dom0_pv)
> (d, image, image_headroom, initrd, bootstrap_map, cmdline);
> 
> with some appropriate indentation/alignment.

Done, thanks!

Roger.

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