Re: [PATCH 03/10] x86/hvm: pkeys, add the flag to enable Memory Protection Keys

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

On 16/11/15 10:31, Huaitong Han wrote:
> This patch adds the flag to enable Memory Protection Keys.
>
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index a565c1b..0ded4bf 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1303,6 +1303,13 @@ Flag to enable Supervisor Mode Execution Protection
>  
>  Flag to enable Supervisor Mode Access Prevention
>  
> +### pku

Options should be in alphabetical order please.

> +> `= <boolean>>`

Extra closing arrow.

> +
> +> Default: `true`
> +
> +Flag to enable Memory Protection Keys

I know there are a number of bad examples in this file, but please avoid
adding to the problem.

It would be useful to have a very brief description of what PKU is, and
which hardware it is available on.  See the 'psr' option as an example.

> +
>  ### snb\_igd\_quirk
>  > `= <boolean> | cap | <integer>`
>  
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 3946e4c..c1f924e 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -67,6 +67,10 @@ invbool_param("smep", disable_smep);
>  static bool_t __initdata disable_smap;
>  invbool_param("smap", disable_smap);
>  
> +/* pku: Enable/disable Memory Protection Keys (default on). */
> +static bool_t __initdata disable_pku;
> +invbool_param("pku", disable_pku);

I am going to submit a patch removing invbool_param(), as it is barely
used and adds unnecessary cognitive overhead

Please us:

static bool_t __initdata opt_pku = 1;
boolean_param("pku", opt_pku);

instead.

~Andrew

> +
>  /* Boot dom0 in pvh mode */
>  static bool_t __initdata opt_dom0pvh;
>  boolean_param("dom0pvh", opt_dom0pvh);
> @@ -1304,6 +1308,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>      if ( cpu_has_smap )
>          set_in_cr4(X86_CR4_SMAP);
>  
> +    if ( disable_pku )
> +        setup_clear_cpu_cap(X86_FEATURE_PKU);
> +
>      if ( cpu_has_fsgsbase )
>          set_in_cr4(X86_CR4_FSGSBASE);
>