Re: [Xen-devel] [PATCH] x86/CPUID: don't override tool stack decision to hide STIBP

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.05.18 at 18:44, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 22/05/18 11:33, Jan Beulich wrote:
>> Other than in the feature sets, where we indeed want to offer the
>> feature even if not enumerated on hardware, we shouldn't dictate the
>> feature being available if tool stack or host admin have decided not
>> to expose it (for whatever [questionable?] reason).
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> This is effectively accompanying the discussion rooted at the 4.8/4.7
>> patch at
>> https://lists.xenproject.org/archives/html/xen-devel/2018-05/msg01028.html 
>> dealing with a feature leveling issue.
>>
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -642,14 +642,6 @@ void recalculate_cpuid_policy(struct dom
>>      recalculate_xstate(p);
>>      recalculate_misc(p);
>>  
>> -    /*
>> -     * Override STIBP to match IBRS.  Guests can safely use STIBP
>> -     * functionality on non-HT hardware, but can't necesserily protect
>> -     * themselves from SP2/Spectre/Branch Target Injection if STIBP is 
>> hidden
>> -     * on HT-capable hardware.
>> -     */
>> -    p->feat.stibp = p->feat.ibrsb;
> 
> You've deleted a comment explaining why this is needed for safety
> reasons, without addressing the safety argument.  Simply "because we
> shouldn't override the toolstack" isn't a reasonable argument.

It very much is: Policy decisions belong, as far as possible, in the tool
stack rather than the hypervisor, and in the admin's hands rather than
pre-programmed tool stack behavior.

> With the SP2 microcode, we have the following situations which can occur:
>  * No mitigations
>  * IBRSB visible
>  * IBRSB and STIBP visible
> 
> IBSRB enumerates MSR_SPEC_CTRL, MSR_SPEC_CTRL.IBRS, and MSR_PRED_CMD.
> STIBP enumerates MSR_SPEC_CTRL.STIBP
> 
> SPEC_CTRL.STIBP is specified as usable (albeit, as a nop) even if STIBP
> isn't enumerated.  This is deliberately and explicitly for heterogeneous
> migration scenarios, as it won't be a nop on other processors.  In
> practice, this is so hypervisors can offer the feature unilaterally, and
> have it usable on non-HT hardware.
> 
> However, to safely level it, dom0 needs to see it set in the information
> used to construct the guest policies.  In principle, this should just be
> in the guest policy which needs adjusting.
> 
> However, due to still not having got the CPUID policy improvements
> finished, dom0 is still excluded from cpuid faulting for the exclusive
> benefit of the CPUID logic in the domain builder, because it uses native
> CPUID to construct the guests CPUID policy.  Therefore, dom0 is unable
> to create a safe CPUID policy for the guest, and Xen must override the
> setting.

I don't understand this. Both xc_cpuid_{hvm,pv}_policy() have

    case 0x00000007: /* Intel-defined CPU features */
        if ( input[1] == 0 )
        {
            regs[1] = info->featureset[featureword_of(X86_FEATURE_FSGSBASE)];
            regs[2] = info->featureset[featureword_of(X86_FEATURE_PREFETCHWT1)];
            regs[3] = 
info->featureset[featureword_of(X86_FEATURE_AVX512_4VNNIW)];
        }

Where is the use of native CPUID here?

Jan



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