Re: [Xen-devel] [PATCH] x86/sm{e, a}p: do not enable SMEP/SMAP in PV shim by default on AMD

From: Jan Beulich <jbeulich@suse.com>
To: Igor Druzhinin <igor.druzhinin@citrix.com>
Cc: sstabellini@kernel.org, julien@xen.org, wl@xen.org,
	konrad.wilk@oracle.com, George.Dunlap@eu.citrix.com,
	andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com,
	xen-devel@lists.xenproject.org, roger.pau@citrix.com
Subject: Re: [Xen-devel] [PATCH] x86/sm{e, a}p: do not enable SMEP/SMAP in PV shim by default on AMD
Date: Fri, 17 Jan 2020 12:10:55 +0100	[thread overview]
Message-ID: <40cafb96-8a5b-025a-c2e5-281e25b1ce97@suse.com> (raw)
In-Reply-To: <1579190403-23720-1-git-send-email-igor.druzhinin@citrix.com>

On 16.01.2020 17:00, Igor Druzhinin wrote:
> Due to AMD and Hygon being unable to selectively trap CR4 bit modifications
> running 32-bit PV guest inside PV shim comes with significant performance
> hit. Moreover, for SMEP in particular every time CR4.SMEP changes on context
> switch to/from 32-bit PV guest, it gets trapped by L0 Xen which then
> tries to perform global TLB invalidation for PV shim domain. This usually
> results in eventual hang of a PV shim with at least several vCPUs.
> 
> Since the overall security risk is generally lower for shim Xen as it being
> there more of a defense-in-depth mechanism, choose to disable SMEP/SMAP in
> it by default on AMD and Hygon unless a user chose otherwise.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with two minor adjustments (two instances each):

> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1936,19 +1936,25 @@ is 1MB.
>  ### smap (x86)
>  > `= <boolean> | hvm`
>  
> -> Default: `true`
> +> Default: `true` unless running in pv-shim mode on AMD or Hygon hardware
>  
>  Flag to enable Supervisor Mode Access Prevention
>  Use `smap=hvm` to allow SMAP use by HVM guests only.
>  
> +In PV shim mode on AMD or Hygon hardware due to significant perfomance impact

Missing 'r' in performance.

> @@ -1616,6 +1616,14 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>  
>      set_in_cr4(X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT);
>  
> +    /* Do not enable SMEP/SMAP in PV shim on AMD and Hygon by default */
> +    if ( opt_smep == -1 )
> +        opt_smep = !pv_shim || !(boot_cpu_data.x86_vendor &
> +                                (X86_VENDOR_AMD | X86_VENDOR_HYGON));

One space missing on this line to cover for the still open parenthesis
from the previous one.

Both can be easily taken care of while committing.

Jan

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