Re: [PATCH SpectreV1+L1TF v5 4/9] spec: add l1tf-barrier

["Jan Beulich" <JBeulich@suse.com>]

>>> On 29.01.19 at 15:43, <nmanthey@amazon.de> wrote:
> @@ -1942,6 +1942,12 @@ Irrespective of Xen's setting, the feature is virtualised for HVM guests to
>  use.  By default, Xen will enable this mitigation on hardware believed to 
> be
>  vulnerable to L1TF.
>  
> +On hardware vulnerable to L1TF, the `l1tf-barrier=` option can be used to force
> +or prevent Xen from protecting evaluations inside the hypervisor with a barrier
> +instruction to not load potentially secret information into L1 cache.  By
> +default, Xen will enable this mitigation on hardware believed to be vulnerable
> +to L1TF.

... and having SMT enabled, since aiui this is a non-issue without.

> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -21,6 +21,7 @@
>  #include <xen/lib.h>
>  #include <xen/warning.h>
>  
> +#include <asm-x86/cpuid.h>

asm/cpuid.h please

> @@ -100,6 +102,7 @@ static int __init parse_spec_ctrl(const char *s)
>              opt_ibpb = false;
>              opt_ssbd = false;
>              opt_l1d_flush = 0;
> +            opt_l1tf_barrier = 0;
>          }
>          else if ( val > 0 )
>              rc = -EINVAL;

Is this really something we want "spec-ctrl=no-xen" to disable?
It would seem to me that this should be restricted to "spec-ctrl=no".

> @@ -843,6 +849,14 @@ void __init init_speculation_mitigations(void)
>          opt_l1d_flush = cpu_has_bug_l1tf && !(caps & ARCH_CAPS_SKIP_L1DFL);
>  
>      /*
> +     * By default, enable L1TF_VULN on L1TF-vulnerable hardware
> +     */

This ought to be a single line comment.

> +    if ( opt_l1tf_barrier == -1 )
> +        opt_l1tf_barrier = cpu_has_bug_l1tf;

At the very least opt_smt should be taken into account here. But
I guess this setting of the default may need to be deferred
further, until the topology of the system is known (there may
not be any hyperthreads after all).

> +    if ( cpu_has_bug_l1tf && opt_l1tf_barrier > 0)
> +        setup_force_cpu_cap(X86_FEATURE_SC_L1TF_VULN);

Why the left side of the &&?

> +    /*
>       * We do not disable HT by default on affected hardware.
>       *
>       * Firstly, if the user intends to use exclusively PV, or HVM shadow

Furthermore, as per the comment and logic here and below a
!HVM configuration ought to be safe too, unless "pv-l1tf=" was
used (in which case we defer to the admin anyway), so it's
questionable whether the whole logic should be there in the
first place in this case. This would then in particular keep all
of this out for the PV shim.

> --- a/xen/include/asm-x86/cpufeatures.h
> +++ b/xen/include/asm-x86/cpufeatures.h
> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_RSB_PV,       (FSCAPINTS+0)*32+18) /* RSB overwrite needed for
>  XEN_CPUFEATURE(SC_RSB_HVM,      (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
>  XEN_CPUFEATURE(SC_MSR_IDLE,     (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
>  XEN_CPUFEATURE(XEN_LBR,         (FSCAPINTS+0)*32+22) /* Xen uses MSR_DEBUGCTL.LBR */
> +XEN_CPUFEATURE(SC_L1TF_VULN,    (FSCAPINTS+0)*32+23) /* L1TF protection required */

Would you mind using one of the unused slots above first?

Jan



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