Re: [Xen-devel] [PATCH v4 09/14] xen/x86: split Dom0 build into PV and PVHv2

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.11.16 at 17:49, <roger.pau@xxxxxxxxxx> 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


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