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

[Norbert Manthey <nmanthey@xxxxxxxxx>]

On 2/12/19 14:44, Jan Beulich wrote:
>>>> On 08.02.19 at 14:44, <nmanthey@xxxxxxxxx> wrote:
>> To control the runtime behavior on L1TF vulnerable platforms better, the
>> command line option l1tf-barrier is introduced. This option controls
>> whether on vulnerable x86 platforms the lfence instruction is used to
>> prevent speculative execution from bypassing the evaluation of
>> conditionals that are protected with the evaluate_nospec macro.
>>
>> By now, Xen is capable of identifying L1TF vulnerable hardware. However,
>> this information cannot be used for alternative patching, as a CPU feature
>> is required. To control alternative patching with the command line option,
>> a new x86 feature "X86_FEATURE_SC_L1TF_VULN" is introduced. This feature
>> is used to patch the lfence instruction into the arch_barrier_nospec_true
>> function. The feature is enabled only if L1TF vulnerable hardware is
>> detected and the command line option does not prevent using this feature.
>>
>> The status of hyperthreading is not considered when automatically enabling
>> adding the lfence instruction, because platforms without hyperthreading
>> can still be vulnerable to L1TF in case the L1 cache is not flushed
>> properly.
>>
>> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
>>
>> ---
>>
>> Notes:
>>   v6: Move disabling l1tf-barrier into spec-ctrl=no
>>       Use gap in existing flags
>>       Force barrier based on commandline, independently of L1TF detection
>>
>>  docs/misc/xen-command-line.pandoc | 14 ++++++++++----
>>  xen/arch/x86/spec_ctrl.c          | 17 +++++++++++++++--
>>  xen/include/asm-x86/cpufeatures.h |  1 +
>>  xen/include/asm-x86/spec_ctrl.h   |  1 +
>>  4 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/docs/misc/xen-command-line.pandoc 
>> b/docs/misc/xen-command-line.pandoc
>> --- a/docs/misc/xen-command-line.pandoc
>> +++ b/docs/misc/xen-command-line.pandoc
>> @@ -483,9 +483,9 @@ accounting for hardware capabilities as enumerated via 
>> CPUID.
>>  
>>  Currently accepted:
>>  
>> -The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb`,
>> -`l1d-flush` and `ssbd` are used by default if available and applicable.  
>> They 
>> can
>> -be ignored, e.g. `no-ibrsb`, at which point Xen won't use them itself, and
>> +The Speculation Control hardware features `ibrsb`, `stibp`, `ibpb`, 
>> `l1d-flush`,
>> +`l1tf-barrier` and `ssbd` are used by default if available and applicable.  
>> They
>> +can be ignored, e.g. `no-ibrsb`, at which point Xen won't use them itself, 
>> and
>>  won't offer them to guests.
>>  
>>  ### cpuid_mask_cpu
>> @@ -1896,7 +1896,7 @@ By default SSBD will be mitigated at runtime (i.e 
>> `ssbd=runtime`).
>>  ### spec-ctrl (x86)
>>  > `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
>>  >              bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd,eager-fpu,
>> ->              l1d-flush}=<bool> ]`
>> +>              l1d-flush,l1tf-barrier}=<bool> ]`
>>  
>>  Controls for speculative execution sidechannel mitigations.  By default, 
>> Xen
>>  will pick the most appropriate mitigations based on compiled in support,
>> @@ -1962,6 +1962,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.
>> +
>>  ### sync_console
>>  > `= <boolean>`
>>  
>> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
>> --- 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/cpuid.h>
>>  #include <asm/microcode.h>
>>  #include <asm/msr.h>
>>  #include <asm/processor.h>
>> @@ -50,6 +51,7 @@ bool __read_mostly opt_ibpb = true;
>>  bool __read_mostly opt_ssbd = false;
>>  int8_t __read_mostly opt_eager_fpu = -1;
>>  int8_t __read_mostly opt_l1d_flush = -1;
>> +int8_t __read_mostly opt_l1tf_barrier = -1;
>>  
>>  bool __initdata bsp_delay_spec_ctrl;
>>  uint8_t __read_mostly default_xen_spec_ctrl;
>> @@ -91,6 +93,8 @@ static int __init parse_spec_ctrl(const char *s)
>>              if ( opt_pv_l1tf_domu < 0 )
>>                  opt_pv_l1tf_domu = 0;
>>  
>> +            opt_l1tf_barrier = 0;
>> +
>>          disable_common:
>>              opt_rsb_pv = false;
>>              opt_rsb_hvm = false;
>> @@ -157,6 +161,8 @@ static int __init parse_spec_ctrl(const char *s)
>>              opt_eager_fpu = val;
>>          else if ( (val = parse_boolean("l1d-flush", s, ss)) >= 0 )
>>              opt_l1d_flush = val;
>> +        else if ( (val = parse_boolean("l1tf-barrier", s, ss)) >= 0 )
>> +            opt_l1tf_barrier = val;
>>          else
>>              rc = -EINVAL;
>>  
>> @@ -248,7 +254,7 @@ static void __init print_details(enum ind_thunk thunk, 
>> uint64_t caps)
>>                 "\n");
>>  
>>      /* Settings for Xen's protection, irrespective of guests. */
>> -    printk("  Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s\n",
>> +    printk("  Xen settings: BTI-Thunk %s, SPEC_CTRL: %s%s, Other:%s%s%s\n",
>>             thunk == THUNK_NONE      ? "N/A" :
>>             thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>             thunk == THUNK_LFENCE    ? "LFENCE" :
>> @@ -258,7 +264,8 @@ static void __init print_details(enum ind_thunk thunk, 
>> uint64_t caps)
>>             !boot_cpu_has(X86_FEATURE_SSBD)           ? "" :
>>             (default_xen_spec_ctrl & SPEC_CTRL_SSBD)  ? " SSBD+" : " SSBD-",
>>             opt_ibpb                                  ? " IBPB"  : "",
>> -           opt_l1d_flush                             ? " L1D_FLUSH" : "");
>> +           opt_l1d_flush                             ? " L1D_FLUSH" : "",
>> +           opt_l1tf_barrier                          ? " L1TF_BARRIER" : 
>> "");
>>  
>>      /* L1TF diagnostics, printed if vulnerable or PV shadowing is in use. 
>> */
>>      if ( cpu_has_bug_l1tf || opt_pv_l1tf_hwdom || opt_pv_l1tf_domu )
>> @@ -842,6 +849,12 @@ void __init init_speculation_mitigations(void)
>>      else if ( opt_l1d_flush == -1 )
>>          opt_l1d_flush = cpu_has_bug_l1tf && !(caps & ARCH_CAPS_SKIP_L1DFL);
>>  
>> +    /* By default, enable L1TF_VULN on L1TF-vulnerable hardware */
>> +    if ( opt_l1tf_barrier == -1 )
>> +        opt_l1tf_barrier = cpu_has_bug_l1tf;
>> +    if ( opt_l1tf_barrier > 0)
>> +        setup_force_cpu_cap(X86_FEATURE_SC_L1TF_VULN);
> Did we end with a misunderstanding in the v5 discussion? I didn't
> answer your question regarding whether to also consider L1D
> flush availability (on top of my request to consider SMT) with a
> straight "yes", but I think it was still clear that my more extensive
> response boiled down to a "yes". Oh, I see now - the same topic
> was discussed in two places, and for the second I then said that
> with the "for now" aspect properly stated (which you now do)
> I'd be fine.
>
> So let me put it this way: Is taking into consideration at least
> opt_smt and opt_l1d_flush (but putting on the side the "too
> early" aspect of the determination here) very difficult to do,
> or would that leave un-addressed concerns of yours? If not,
> may I ask that you go at least that little step further? As said
> before - we'd like to avoid penalizing configurations as well as
> setups which aren't affected. In particular it would seem
> pretty bad of us to further penalize people who have set
> "smt=0" and who use up-to-date microcode.
I understand that smt=0 should not be penalized. However, only if
flushing is used as well, smt=0 is actually safe. I will extend the
logic to set the CPU flag automatically in case L1TF hardware has been
detected, and smt!=0 or !L1D-flush, i.e. opt_l1tf_barrier =
cpu_has_bug_l1tf && (opt_smt != 0 || opt_l1d_flush == 0);
>
> Also in the second if() there's yet again a missing blank.

Will fix.

Best,
Norbert

>
> Jan
>




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

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