Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.01.19 at 12:51, <nmanthey@xxxxxxxxx> wrote:
> This commit introduces the configuration option L1TF_LFENCE that allows
> to control the implementation of the protection of privilege checks via
> lfence instructions. The following four alternatives are provided:
> 
>  - not injecting lfence instructions
>  - inject an lfence instruction for both outcomes of the conditional
>  - inject an lfence instruction only if the conditional would evaluate
>    to true, so that this case cannot be entered under speculation

I'd really like to see justification for this variant, as the asymmetric
handling doesn't look overly helpful. It's also not clear to me how
someone configuring Xen should tell whether this would be a safe
selection to make. I'm tempted to request that this option become
EXPERT dependent.

>  - evaluating the condition and store the result into a local variable.
>    before using this value, inject an lfence instruction.
> 
> The different options allow to control the level of protection vs the
> slowdown the addtional lfence instructions would introduce. The default
> value is set to protecting both branches.
> 
> For non-x86 platforms, the protection is disabled by default.

At least the "by default" is wrong here.

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -176,6 +176,30 @@ config PV_SHIM_EXCLUSIVE
>         firmware, and will not function correctly in other scenarios.
>  
>         If unsure, say N.
> +
> +choice
> +     prompt "Default L1TF Branch Protection?"
> +
> +     config L1TF_LFENCE_BOTH
> +             bool "Protect both branches of certain conditionals" if HVM
> +             ---help---
> +               Inject an lfence instruction after the condition to be
> +               evaluated for both outcomes of the condition
> +     config L1TF_LFENCE_TRUE
> +             bool "Protect true branch of certain conditionals" if HVM
> +             ---help---
> +               Protect only the path where the condition is evaluated to true
> +     config L1TF_LFENCE_INTERMEDIATE
> +             bool "Protect before using certain conditionals value" if HVM
> +             ---help---
> +               Inject an lfence instruction after evaluating the condition
> +               but before forwarding this value from a local variable
> +     config L1TF_LFENCE_NONE
> +             bool "No conditional protection"
> +             ---help---
> +               Do not inject lfences for conditional evaluations
> +endchoice

I guess we should settle on some default for this choice. The
description talks about a default, but I don't see it set here (and
I don't think relying merely on the order is a good idea).

> --- a/xen/include/xen/nospec.h
> +++ b/xen/include/xen/nospec.h
> @@ -68,10 +68,18 @@ static inline bool lfence_true(void) { return true; }
>  #endif
>  
>  /*
> - * protect evaluation of conditional with respect to speculation
> + * allow to protect evaluation of conditional with respect to speculation on 
> x86
>   */
> -#define evaluate_nospec(condition)                                      \
> +#if defined(CONFIG_L1TF_LFENCE_NONE) || !defined(CONFIG_X86)
> +#define evaluate_nospec(condition) (condition)
> +#elif defined(CONFIG_L1TF_LFENCE_BOTH)
> +#define evaluate_nospec(condition)                                         \

I'm puzzled by this line changing - do you really need to move the
backslash here?

Jan



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