Re: [PATCH 02/10] VMX: New parameter to control PML enabling

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/03/15 02:35, Kai Huang wrote:
> A top level EPT parameter "ept=<options>" and a sub boolean "pml_enable" are
> added to control PML. Other booleans can be further added for any other EPT
> related features.
>
> Signed-off-by: Kai Huang <kai.huang@linux.intel.com>

Please patch docs/misc/xen-command-line.markdown as well.  See the
existing "psr" option as a similar example.

Also, as indicated in patch 1, I think patches 1 and 2 need swapping in
the series.

> ---
>  xen/arch/x86/hvm/vmx/vmcs.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 2f645fe..9b20a4b 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -50,6 +50,16 @@ boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
>  static bool_t __read_mostly opt_apicv_enabled = 1;
>  boolean_param("apicv", opt_apicv_enabled);
>  
> +static void parse_ept_param(char *s);
> +/*
> + * The 'ept' parameter controls functionalities that depend on, or impact the
> + * EPT mechanism. Optional comma separated value may contain:
> + *
> + *  pml                 Enable PML
> + */
> +custom_param("ept", parse_ept_param);

It is common to put the custom_param() call below parse_ept_param() so
you don't need to forward-declare the function.  The comment can happily
live at the top of parse_ept_param().

> +static bool_t __read_mostly pml_enable = 0;
> +
>  /*
>   * These two parameters are used to config the controls for Pause-Loop Exiting:
>   * ple_gap:    upper bound on the amount of time between two successive
> @@ -92,6 +102,28 @@ DEFINE_PER_CPU(bool_t, vmxon);
>  static u32 vmcs_revision_id __read_mostly;
>  u64 __read_mostly vmx_basic_msr;
>  
> +/* Copied from parse_iommu_param */

Not a useful comment, as it is likely to diverge in the future.

> +static void parse_ept_param(char *s)

__init

~Andrew

> +{
> +    char *ss;
> +    int val;
> +
> +    do {
> +        val = !!strncmp(s, "no-", 3);
> +        if ( !val )
> +            s += 3;
> +
> +        ss = strchr(s, ',');
> +        if ( ss )
> +            *ss = '\0';
> +
> +        if ( !strcmp(s, "pml") )
> +            pml_enable = val;
> +
> +        s = ss + 1;
> +    } while ( ss );
> +}
> +
>  static void __init vmx_display_features(void)
>  {
>      int printed = 0;