diff mbox

[v4,09/14] xen/x86: split Dom0 build into PV and PVHv2

Message ID 20161130164950.43543-10-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monné Nov. 30, 2016, 4:49 p.m. UTC
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
called 'dom0' that can be used to request the creation of a PVHv2 Dom0 by
setting the 'hvm' sub-option. A panic has also been added if a user tries
to use dom0=hvm until all the code is in place, then the panic will be
removed.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v3:
 - Correctly declare the parameter list.
 - Add a panic if dom0=hvm is used. This will be removed once all the code is in
   place.

Changes since v2:
 - Fix coding style.
 - Introduce a new dom0 option that allows passing several parameters.
   Currently supported ones are hvm and shadow.

Changes since RFC:
 - Add documentation for the new command line option.
 - Simplify the logic in construct_dom0.
---
 docs/misc/xen-command-line.markdown | 17 +++++++++++++++++
 xen/arch/x86/domain_build.c         | 28 +++++++++++++++++++++++----
 xen/arch/x86/setup.c                | 38 +++++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/setup.h         |  6 ++++++
 4 files changed, 85 insertions(+), 4 deletions(-)

Comments

Jan Beulich Dec. 9, 2016, 4:07 p.m. UTC | #1
>>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -656,6 +656,23 @@ affinities to prefer but be not limited to the specified node(s).
>  
>  Pin dom0 vcpus to their respective pcpus
>  
> +### dom0
> +> `= List of [ hvm | shadow ]`
> +
> +> Sub-options:
> +
> +> `hvm`
> +
> +> Default: `false`
> +
> +Flag that makes a dom0 boot in PVHv2 mode.
> +
> +> `shadow`
> +
> +> Default: `false`
> +
> +Flag that makes a dom0 use shadow paging.

Would you mind marking dom0_shadow deprecated at once? In fact
I wouldn't mind if it was removed from the documentation altogether,
the more that it still has no description at all.

> @@ -1655,6 +1653,28 @@ 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");

Why again is it that you call the function "hvm" but mean "pvh"?

> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -187,6 +187,35 @@ static void __init parse_acpi_param(char *s)
>      }
>  }
>  
> +/*
> + * List of parameters that affect Dom0 creation:
> + *
> + *  - hvm               Create a PVHv2 Dom0.
> + *  - shadow            Use shadow paging for Dom0.
> + */
> +static bool __initdata dom0_hvm;
> +static void __init parse_dom0_param(char *s)
> +{
> +    char *ss;
> +
> +    do {
> +
> +        ss = strchr(s, ',');
> +        if ( ss )
> +            *ss = '\0';
> +
> +        if ( !strcmp(s, "hvm") )
> +            dom0_hvm = true;
> +#ifdef CONFIG_SHADOW_PAGING
> +        else if ( !strcmp(s, "shadow") )
> +            opt_dom0_shadow = true;
> +#endif
> +
> +        s = ss + 1;
> +    } while ( ss );
> +}
> +custom_param("dom0", parse_dom0_param);

I continue to think that this should live in domain_build.c, and
dom0_hvm be the one off variable which needs to be global. After
all we intend to extend the "dom0=" quite a bit (presumably to
subsume everything which the various "dom0..." options now do),
and all that stuff lives there anyway.

> --- a/xen/include/asm-x86/setup.h
> +++ b/xen/include/asm-x86/setup.h
> @@ -57,4 +57,10 @@ extern uint8_t kbd_shift_flags;
>  extern unsigned long highmem_start;
>  #endif
>  
> +#ifdef CONFIG_SHADOW_PAGING
> +extern bool opt_dom0_shadow;
> +#else
> +#define opt_dom0_shadow 0

"false" please, to match up with "bool".

Jan
Roger Pau Monné Dec. 16, 2016, 2:28 p.m. UTC | #2
On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
> >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> > --- a/docs/misc/xen-command-line.markdown
> > +++ b/docs/misc/xen-command-line.markdown
> > @@ -656,6 +656,23 @@ affinities to prefer but be not limited to the specified node(s).
> >  
> >  Pin dom0 vcpus to their respective pcpus
> >  
> > +### dom0
> > +> `= List of [ hvm | shadow ]`
> > +
> > +> Sub-options:
> > +
> > +> `hvm`
> > +
> > +> Default: `false`
> > +
> > +Flag that makes a dom0 boot in PVHv2 mode.
> > +
> > +> `shadow`
> > +
> > +> Default: `false`
> > +
> > +Flag that makes a dom0 use shadow paging.
> 
> Would you mind marking dom0_shadow deprecated at once? In fact
> I wouldn't mind if it was removed from the documentation altogether,
> the more that it still has no description at all.

Sure, AFAICT it's just removing it from the documentation and a single usage
in construct_dom0_pv (the one in compute_dom0_nr_pages needs to stay for PVHv2
Dom0).

> > @@ -1655,6 +1653,28 @@ 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");
> 
> Why again is it that you call the function "hvm" but mean "pvh"?

This was to differentiate between the current "pvh" functions in this file that
refer to PVHv1. I could name them pvhv2, but IMHO hvm seems clearer and
shorter.

> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -187,6 +187,35 @@ static void __init parse_acpi_param(char *s)
> >      }
> >  }
> >  
> > +/*
> > + * List of parameters that affect Dom0 creation:
> > + *
> > + *  - hvm               Create a PVHv2 Dom0.
> > + *  - shadow            Use shadow paging for Dom0.
> > + */
> > +static bool __initdata dom0_hvm;
> > +static void __init parse_dom0_param(char *s)
> > +{
> > +    char *ss;
> > +
> > +    do {
> > +
> > +        ss = strchr(s, ',');
> > +        if ( ss )
> > +            *ss = '\0';
> > +
> > +        if ( !strcmp(s, "hvm") )
> > +            dom0_hvm = true;
> > +#ifdef CONFIG_SHADOW_PAGING
> > +        else if ( !strcmp(s, "shadow") )
> > +            opt_dom0_shadow = true;
> > +#endif
> > +
> > +        s = ss + 1;
> > +    } while ( ss );
> > +}
> > +custom_param("dom0", parse_dom0_param);
> 
> I continue to think that this should live in domain_build.c, and
> dom0_hvm be the one off variable which needs to be global. After
> all we intend to extend the "dom0=" quite a bit (presumably to
> subsume everything which the various "dom0..." options now do),
> and all that stuff lives there anyway.

opt_dom0_shadow is also needed by setup.c in order to pass the right DOMCRF
flags. I don't really mind putting it here or in setup.c, but ATM both options
will need to be exported one way or another.

> > --- a/xen/include/asm-x86/setup.h
> > +++ b/xen/include/asm-x86/setup.h
> > @@ -57,4 +57,10 @@ extern uint8_t kbd_shift_flags;
> >  extern unsigned long highmem_start;
> >  #endif
> >  
> > +#ifdef CONFIG_SHADOW_PAGING
> > +extern bool opt_dom0_shadow;
> > +#else
> > +#define opt_dom0_shadow 0
> 
> "false" please, to match up with "bool".

Fixed, thanks.

Roger.
Roger Pau Monné Dec. 16, 2016, 2:45 p.m. UTC | #3
On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> > > @@ -1655,6 +1653,28 @@ 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");
> > 
> > Why again is it that you call the function "hvm" but mean "pvh"?
> 
> This was to differentiate between the current "pvh" functions in this file that
> refer to PVHv1. I could name them pvhv2, but IMHO hvm seems clearer and
> shorter.

Oh, and the other reason was that Xen doesn't really know the difference
between a HVM guest and a PVHv2 guest, hence hvm felt more natural.

Roger.
Roger Pau Monné Dec. 16, 2016, 3:28 p.m. UTC | #4
On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> > > --- a/xen/arch/x86/setup.c
> > > +++ b/xen/arch/x86/setup.c
> > > @@ -187,6 +187,35 @@ static void __init parse_acpi_param(char *s)
> > >      }
> > >  }
> > >  
> > > +/*
> > > + * List of parameters that affect Dom0 creation:
> > > + *
> > > + *  - hvm               Create a PVHv2 Dom0.
> > > + *  - shadow            Use shadow paging for Dom0.
> > > + */
> > > +static bool __initdata dom0_hvm;
> > > +static void __init parse_dom0_param(char *s)
> > > +{
> > > +    char *ss;
> > > +
> > > +    do {
> > > +
> > > +        ss = strchr(s, ',');
> > > +        if ( ss )
> > > +            *ss = '\0';
> > > +
> > > +        if ( !strcmp(s, "hvm") )
> > > +            dom0_hvm = true;
> > > +#ifdef CONFIG_SHADOW_PAGING
> > > +        else if ( !strcmp(s, "shadow") )
> > > +            opt_dom0_shadow = true;
> > > +#endif
> > > +
> > > +        s = ss + 1;
> > > +    } while ( ss );
> > > +}
> > > +custom_param("dom0", parse_dom0_param);
> > 
> > I continue to think that this should live in domain_build.c, and
> > dom0_hvm be the one off variable which needs to be global. After
> > all we intend to extend the "dom0=" quite a bit (presumably to
> > subsume everything which the various "dom0..." options now do),
> > and all that stuff lives there anyway.

In fact opt_dom0_shadow is only going to be needed by setup.c after the removal
of it's usage by PV Dom0, so I think it's better to keep parse_dom0_param in
setup.c.

Roger.
Jan Beulich Dec. 16, 2016, 4:15 p.m. UTC | #5
>>> On 16.12.16 at 16:28, <roger.pau@citrix.com> wrote:
> On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
>> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
>> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
>> > > --- a/xen/arch/x86/setup.c
>> > > +++ b/xen/arch/x86/setup.c
>> > > @@ -187,6 +187,35 @@ static void __init parse_acpi_param(char *s)
>> > >      }
>> > >  }
>> > >  
>> > > +/*
>> > > + * List of parameters that affect Dom0 creation:
>> > > + *
>> > > + *  - hvm               Create a PVHv2 Dom0.
>> > > + *  - shadow            Use shadow paging for Dom0.
>> > > + */
>> > > +static bool __initdata dom0_hvm;
>> > > +static void __init parse_dom0_param(char *s)
>> > > +{
>> > > +    char *ss;
>> > > +
>> > > +    do {
>> > > +
>> > > +        ss = strchr(s, ',');
>> > > +        if ( ss )
>> > > +            *ss = '\0';
>> > > +
>> > > +        if ( !strcmp(s, "hvm") )
>> > > +            dom0_hvm = true;
>> > > +#ifdef CONFIG_SHADOW_PAGING
>> > > +        else if ( !strcmp(s, "shadow") )
>> > > +            opt_dom0_shadow = true;
>> > > +#endif
>> > > +
>> > > +        s = ss + 1;
>> > > +    } while ( ss );
>> > > +}
>> > > +custom_param("dom0", parse_dom0_param);
>> > 
>> > I continue to think that this should live in domain_build.c, and
>> > dom0_hvm be the one off variable which needs to be global. After
>> > all we intend to extend the "dom0=" quite a bit (presumably to
>> > subsume everything which the various "dom0..." options now do),
>> > and all that stuff lives there anyway.
> 
> In fact opt_dom0_shadow is only going to be needed by setup.c after the removal
> of it's usage by PV Dom0, so I think it's better to keep parse_dom0_param in
> setup.c.

No. I thought I had made this clear by a comment to a later patch
(where dom0_hvm gets made non-static).

Jan
Jan Beulich Dec. 16, 2016, 4:17 p.m. UTC | #6
>>> On 16.12.16 at 15:45, <roger.pau@citrix.com> wrote:
> On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
>> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
>> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
>> > > @@ -1655,6 +1653,28 @@ 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");
>> > 
>> > Why again is it that you call the function "hvm" but mean "pvh"?
>> 
>> This was to differentiate between the current "pvh" functions in this file that
>> refer to PVHv1. I could name them pvhv2, but IMHO hvm seems clearer and
>> shorter.
> 
> Oh, and the other reason was that Xen doesn't really know the difference
> between a HVM guest and a PVHv2 guest, hence hvm felt more natural.

Xen certainly can tell the difference for Dom0, since a true HVM
Dom0 can't exist without a lot of work towards getting a device
model run somewhere to service it. I continue to think that "hvm"
in any of the names involved in this series is misleading.

Jan
Jan Beulich Dec. 16, 2016, 4:18 p.m. UTC | #7
>>> On 16.12.16 at 15:28, <roger.pau@citrix.com> wrote:
> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
>> >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
>> > --- a/docs/misc/xen-command-line.markdown
>> > +++ b/docs/misc/xen-command-line.markdown
>> > @@ -656,6 +656,23 @@ affinities to prefer but be not limited to the specified node(s).
>> >  
>> >  Pin dom0 vcpus to their respective pcpus
>> >  
>> > +### dom0
>> > +> `= List of [ hvm | shadow ]`
>> > +
>> > +> Sub-options:
>> > +
>> > +> `hvm`
>> > +
>> > +> Default: `false`
>> > +
>> > +Flag that makes a dom0 boot in PVHv2 mode.
>> > +
>> > +> `shadow`
>> > +
>> > +> Default: `false`
>> > +
>> > +Flag that makes a dom0 use shadow paging.
>> 
>> Would you mind marking dom0_shadow deprecated at once? In fact
>> I wouldn't mind if it was removed from the documentation altogether,
>> the more that it still has no description at all.
> 
> Sure, AFAICT it's just removing it from the documentation and a single usage
> in construct_dom0_pv (the one in compute_dom0_nr_pages needs to stay for PVHv2
> Dom0).

Just to avoid any misunderstanding: I talked about documentation
only, not about removing anything from code.

Jan
Roger Pau Monné Dec. 16, 2016, 5:42 p.m. UTC | #8
On Fri, Dec 16, 2016 at 09:18:13AM -0700, Jan Beulich wrote:
> >>> On 16.12.16 at 15:28, <roger.pau@citrix.com> wrote:
> > On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
> >> >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> >> > --- a/docs/misc/xen-command-line.markdown
> >> > +++ b/docs/misc/xen-command-line.markdown
> >> > @@ -656,6 +656,23 @@ affinities to prefer but be not limited to the specified node(s).
> >> >  
> >> >  Pin dom0 vcpus to their respective pcpus
> >> >  
> >> > +### dom0
> >> > +> `= List of [ hvm | shadow ]`
> >> > +
> >> > +> Sub-options:
> >> > +
> >> > +> `hvm`
> >> > +
> >> > +> Default: `false`
> >> > +
> >> > +Flag that makes a dom0 boot in PVHv2 mode.
> >> > +
> >> > +> `shadow`
> >> > +
> >> > +> Default: `false`
> >> > +
> >> > +Flag that makes a dom0 use shadow paging.
> >> 
> >> Would you mind marking dom0_shadow deprecated at once? In fact
> >> I wouldn't mind if it was removed from the documentation altogether,
> >> the more that it still has no description at all.
> > 
> > Sure, AFAICT it's just removing it from the documentation and a single usage
> > in construct_dom0_pv (the one in compute_dom0_nr_pages needs to stay for PVHv2
> > Dom0).
> 
> Just to avoid any misunderstanding: I talked about documentation
> only, not about removing anything from code.

So, you just want to remove the documentation line about dom0_shadow and leave
the option there?

It seems kind of pointless to me, the more that a) it's not going to be
documented and b) AFAIK it's not working.

Roger.
Roger Pau Monné Dec. 16, 2016, 5:57 p.m. UTC | #9
On Fri, Dec 16, 2016 at 09:17:01AM -0700, Jan Beulich wrote:
> >>> On 16.12.16 at 15:45, <roger.pau@citrix.com> wrote:
> > On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
> >> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
> >> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
> >> > > @@ -1655,6 +1653,28 @@ 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");
> >> > 
> >> > Why again is it that you call the function "hvm" but mean "pvh"?
> >> 
> >> This was to differentiate between the current "pvh" functions in this file that
> >> refer to PVHv1. I could name them pvhv2, but IMHO hvm seems clearer and
> >> shorter.
> > 
> > Oh, and the other reason was that Xen doesn't really know the difference
> > between a HVM guest and a PVHv2 guest, hence hvm felt more natural.
> 
> Xen certainly can tell the difference for Dom0, since a true HVM
> Dom0 can't exist without a lot of work towards getting a device
> model run somewhere to service it. I continue to think that "hvm"
> in any of the names involved in this series is misleading.

Yes, but that device model isn't a Xen-kernel component, and would be attached
using the ioreq server machinery anyway, so IMHO I still think this is more
like HVM rather than anything else from a Xen PoV. And if we ever got something
like a complete HVM Dom0 with a device model it would certainly use this
machinery.

In any case, I don't want to start a bikeshed over it, so please tell me your
preference for either pvh_ or pvh2 or pvhv2_ prefix for these new functions.

Thanks, Roger.
Jan Beulich Dec. 19, 2016, 7:41 a.m. UTC | #10
>>> On 16.12.16 at 18:42, <roger.pau@citrix.com> wrote:
> On Fri, Dec 16, 2016 at 09:18:13AM -0700, Jan Beulich wrote:
>> >>> On 16.12.16 at 15:28, <roger.pau@citrix.com> wrote:
>> > On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
>> >> >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
>> >> > --- a/docs/misc/xen-command-line.markdown
>> >> > +++ b/docs/misc/xen-command-line.markdown
>> >> > @@ -656,6 +656,23 @@ affinities to prefer but be not limited to the 
> specified node(s).
>> >> >  
>> >> >  Pin dom0 vcpus to their respective pcpus
>> >> >  
>> >> > +### dom0
>> >> > +> `= List of [ hvm | shadow ]`
>> >> > +
>> >> > +> Sub-options:
>> >> > +
>> >> > +> `hvm`
>> >> > +
>> >> > +> Default: `false`
>> >> > +
>> >> > +Flag that makes a dom0 boot in PVHv2 mode.
>> >> > +
>> >> > +> `shadow`
>> >> > +
>> >> > +> Default: `false`
>> >> > +
>> >> > +Flag that makes a dom0 use shadow paging.
>> >> 
>> >> Would you mind marking dom0_shadow deprecated at once? In fact
>> >> I wouldn't mind if it was removed from the documentation altogether,
>> >> the more that it still has no description at all.
>> > 
>> > Sure, AFAICT it's just removing it from the documentation and a single usage
>> > in construct_dom0_pv (the one in compute_dom0_nr_pages needs to stay for PVHv2
>> > Dom0).
>> 
>> Just to avoid any misunderstanding: I talked about documentation
>> only, not about removing anything from code.
> 
> So, you just want to remove the documentation line about dom0_shadow and leave
> the option there?
> 
> It seems kind of pointless to me, the more that a) it's not going to be
> documented and b) AFAIK it's not working.

Well, if it's _provably_ not working, then of course it could be
removed altogether.

Jan
Jan Beulich Dec. 19, 2016, 7:42 a.m. UTC | #11
>>> On 16.12.16 at 18:57, <roger.pau@citrix.com> wrote:
> On Fri, Dec 16, 2016 at 09:17:01AM -0700, Jan Beulich wrote:
>> >>> On 16.12.16 at 15:45, <roger.pau@citrix.com> wrote:
>> > On Fri, Dec 16, 2016 at 02:28:54PM +0000, Roger Pau Monne wrote:
>> >> On Fri, Dec 09, 2016 at 09:07:16AM -0700, Jan Beulich wrote:
>> >> > >>> On 30.11.16 at 17:49, <roger.pau@citrix.com> wrote:
>> >> > > @@ -1655,6 +1653,28 @@ 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");
>> >> > 
>> >> > Why again is it that you call the function "hvm" but mean "pvh"?
>> >> 
>> >> This was to differentiate between the current "pvh" functions in this file that
>> >> refer to PVHv1. I could name them pvhv2, but IMHO hvm seems clearer and
>> >> shorter.
>> > 
>> > Oh, and the other reason was that Xen doesn't really know the difference
>> > between a HVM guest and a PVHv2 guest, hence hvm felt more natural.
>> 
>> Xen certainly can tell the difference for Dom0, since a true HVM
>> Dom0 can't exist without a lot of work towards getting a device
>> model run somewhere to service it. I continue to think that "hvm"
>> in any of the names involved in this series is misleading.
> 
> Yes, but that device model isn't a Xen-kernel component, and would be attached
> using the ioreq server machinery anyway, so IMHO I still think this is more
> like HVM rather than anything else from a Xen PoV. And if we ever got something
> like a complete HVM Dom0 with a device model it would certainly use this
> machinery.
> 
> In any case, I don't want to start a bikeshed over it, so please tell me your
> preference for either pvh_ or pvh2 or pvhv2_ prefix for these new functions.

pvh please.

Jan
diff mbox

Patch

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 0138978..c9729be 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -656,6 +656,23 @@  affinities to prefer but be not limited to the specified node(s).
 
 Pin dom0 vcpus to their respective pcpus
 
+### dom0
+> `= List of [ hvm | shadow ]`
+
+> Sub-options:
+
+> `hvm`
+
+> Default: `false`
+
+Flag that makes a dom0 boot in PVHv2 mode.
+
+> `shadow`
+
+> Default: `false`
+
+Flag that makes a dom0 use shadow paging.
+
 ### dom0pvh
 > `= <boolean>`
 
diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 1e557b9..2c9ebf2 100644
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -191,10 +191,8 @@  struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
 }
 
 #ifdef CONFIG_SHADOW_PAGING
-static bool_t __initdata opt_dom0_shadow;
+bool __initdata opt_dom0_shadow;
 boolean_param("dom0_shadow", opt_dom0_shadow);
-#else
-#define opt_dom0_shadow 0
 #endif
 
 static char __initdata opt_dom0_ioports_disable[200] = "";
@@ -951,7 +949,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,
@@ -1655,6 +1653,28 @@  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");
+
+    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 : construct_dom0_pv)
+           (d, image, image_headroom, initrd,bootstrap_map, cmdline);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b130671..255e20c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -187,6 +187,35 @@  static void __init parse_acpi_param(char *s)
     }
 }
 
+/*
+ * List of parameters that affect Dom0 creation:
+ *
+ *  - hvm               Create a PVHv2 Dom0.
+ *  - shadow            Use shadow paging for Dom0.
+ */
+static bool __initdata dom0_hvm;
+static void __init parse_dom0_param(char *s)
+{
+    char *ss;
+
+    do {
+
+        ss = strchr(s, ',');
+        if ( ss )
+            *ss = '\0';
+
+        if ( !strcmp(s, "hvm") )
+            dom0_hvm = true;
+#ifdef CONFIG_SHADOW_PAGING
+        else if ( !strcmp(s, "shadow") )
+            opt_dom0_shadow = true;
+#endif
+
+        s = ss + 1;
+    } while ( ss );
+}
+custom_param("dom0", parse_dom0_param);
+
 static const module_t *__initdata initial_images;
 static unsigned int __initdata nr_initial_images;
 
@@ -1541,6 +1570,15 @@  void __init noreturn __start_xen(unsigned long mbi_p)
     if ( opt_dom0pvh )
         domcr_flags |= DOMCRF_pvh | DOMCRF_hap;
 
+    if ( dom0_hvm )
+    {
+        panic("Building a PVHv2 Dom0 is not yet supported.");
+        domcr_flags |= DOMCRF_hvm |
+                       ((hvm_funcs.hap_supported && !opt_dom0_shadow) ?
+                         DOMCRF_hap : 0);
+        config.emulation_flags = XEN_X86_EMU_LAPIC|XEN_X86_EMU_IOAPIC;
+    }
+
     /* Create initial domain 0. */
     dom0 = domain_create(0, domcr_flags, 0, &config);
     if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) )
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index c65b79c..c4179d1 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -57,4 +57,10 @@  extern uint8_t kbd_shift_flags;
 extern unsigned long highmem_start;
 #endif
 
+#ifdef CONFIG_SHADOW_PAGING
+extern bool opt_dom0_shadow;
+#else
+#define opt_dom0_shadow 0
+#endif
+
 #endif