Re: [Patch v5 11/16] x86/speculation: Add Spectre v2 app to app protection modes

[Tim Chen]

On 11/19/2018 04:00 PM, Thomas Gleixner wrote:
> On Tue, 20 Nov 2018, Jiri Kosina wrote:
> 
>> On Tue, 20 Nov 2018, Thomas Gleixner wrote:
>>
>>> What? IBPB makes tons of sense even without STIBP.
>>
>> On non-SMT, yes. But this patchset ties those two the other (sensible) way 
>> around AFAICS ("STIBP iff (IBPB && SMT)").
> 
> Errm. No.
> 
> The patches disable IBPB if STIBP is not available and that has absolutely
> nothing to do with SMT simply because the static key controlling IBPB is
> only flipped on when both IBPB and STIBP are available.

Ok, I assume you will like this change

@@ -560,7 +559,7 @@ static void __init spectre_v2_select_mitigation(void)
        }
 
        app2app_mode = SPECTRE_V2_APP2APP_NONE;
-       if (!boot_cpu_has(X86_FEATURE_IBPB) ||
+       if (!boot_cpu_has(X86_FEATURE_IBPB) &&
            !boot_cpu_has(X86_FEATURE_STIBP))
                goto set_app2app_mode;
 
before setting the IBPB feature.  So we will still do the
IBPB feature set even if STIBP feature is unavailable.

        /*
         * Initialize Indirect Branch Prediction Barrier if supported
         * and not disabled explicitly
         */
        if (app2app_mode != SPECTRE_V2_APP2APP_NONE) {
                setup_force_cpu_cap(X86_FEATURE_USE_IBPB);
                pr_info("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n");
        }

In that case I'll need to add a static_cpu_has feature flag check when
we are using STIBP and can't assume that when lite mode is on, STIBP is there.
We already check the cpu feature flag in IBPB path so we're good there.

When I first wrote the code, I had in mind that CPU that has one will have
the other.  But that assumption is best avoided.

Tim