Re: [Xen-devel] [PATCH v8 11/17] x86: Protect unaware domains from meddling hyperthreads

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/18 11:26, Jan Beulich wrote:
>>>> On 12.01.18 at 19:01, <andrew.cooper3@xxxxxxxxxx> wrote:
>> --- a/xen/include/asm-x86/spec_ctrl.h
>> +++ b/xen/include/asm-x86/spec_ctrl.h
>> @@ -20,8 +20,29 @@
>>  #ifndef __X86_SPEC_CTRL_H__
>>  #define __X86_SPEC_CTRL_H__
>>  
>> +#include <xen/sched.h>
>> +
>>  void init_speculation_mitigations(void);
>>  
>> +/*
>> + * For guests which know about IBRS but are not told about STIBP running on
>> + * hardware supporting hyperthreading, the guest doesn't know to protect
>> + * itself fully.  (Such a guest won't be permitted direct access to the 
>> MSR.)
>> + * Have Xen fill in the gaps, so an unaware guest can't be interfered with 
>> by
>> + * a meddling guest on an adjacent hyperthread.
>> + */
>> +static inline unsigned int spec_ctrl_host_val(const struct domain *d,
>> +                                              unsigned int guest_val)
>> +{
>> +    const struct cpuid_policy *cp = d->arch.cpuid;
>> +
>> +    if ( !cp->feat.stibp && cpu_has_stibp &&
>> +         !(guest_val & (SPEC_CTRL_IBRS | SPEC_CTRL_STIBP)) )
>> +        return SPEC_CTRL_STIBP;
>> +    else
>> +        return guest_val;
>> +}
> The comment's "won't be permitted direct access" can be verified
> by looking at patch 10, but where's the HT dependency here (or
> in another patch)? For now it looks to me as if you enabled this
> behind-the-back protection even in the non-HT case.

The problem is that this is all speculative programming (pardon the
pun), without answers to several questions, and without microcode to
actually try it out on.  I guess I will have to unwind the changes, and
hope there isn't some leaky record of this train of thought somewhere...
(Sorry - I couldn't resist!)

I've guessed since the very first spec I saw that STIBP was expected not
always to be advertised, and the fact that there are two CPUID bits
means that, whether the situation exists in practice, it is possible to
level a VM into such a state.

Therefore, Xen should provide a defence against such a levelling so an
unassuming guest doesn't get caught out.

The latest intel spec (published since I posted v8) says:

"These include processors on which Intel Hyper-Threading Technology is
not enabled and those that do not share indirect branch predictors
between logical processors. To simplify software enabling and enhance
workload migration, STIBP may be enumerated (and setting
IA32_SPEC_CTRL.STIBP allowed) on such processors."

which I take to mean "for microcode simplicity, we will always advertise
it on HT-capable hardware", and

"A processor may enumerate support for the IA32_SPEC_CTRL MSR (e.g., by
enumerating CPUID.(EAX=7H,ECX=0):EDX[26] as 1) but not for STIBP
(CPUID.(EAX=7H,ECX=0):EDX[27] is enumerated as 0). On such processors,
execution of WRMSR to IA32_SPEC_CTRL ignores the value of bit 1 (STIBP)
and does not cause a general-protection exception (#GP) if bit 1 of the
source operand is set. It is expected that this fact will simplify
virtualization in some cases."

which I take to mean "non-HT hardware won't enumerate STIBP, but will
cope with an OS trying to use it".

An alternative to the current levelling logic would be to treat STIBP as
a Special Bit (in CPUID terms, like OSXSAVE/etc) and unconditionally set
it equal to IBRS, irrespective of the toolstack setting.  That way,
migration between HT and non-HT hardware is safe and a VM which chooses
to use STIBP will work even on non-HT hardware which simply ignores the
request.

~Andrew

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