On 11/27/2018 02:42 PM, Thomas Gleixner wrote: > On Tue, 27 Nov 2018, Thomas Gleixner wrote: >> On Tue, 27 Nov 2018, Lendacky, Thomas wrote: >>> On 11/25/2018 12:33 PM, Thomas Gleixner wrote: >>>> --- a/arch/x86/kernel/process.c >>>> +++ b/arch/x86/kernel/process.c >>>> @@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_up >>>> if (static_cpu_has(X86_FEATURE_SSBD)) >>>> msr |= ssbd_tif_to_spec_ctrl(tifn); >>> >>> I did some quick testing and found my original logic was flawed. Since >>> spec_ctrl_update_msr() can now be called for STIBP, an additional check >>> is needed to set the SSBD MSR bit. >>> >>> Both X86_FEATURE_VIRT_SSBD and X86_FEATURE_LS_CFG_SSBD cause >>> X86_FEATURE_SSBD to be set. Before this patch, spec_ctrl_update_msr() was >>> only called if X86_FEATURE_SSBD was set and one of the other SSBD features >>> wasn't set. But now, STIBP can cause spec_ctrl_update_msr() to get called >>> and cause the SSBD MSR bit to be set when it shouldn't (could result in >>> a GP fault). >> >> The below should fix that. We have the same logic in x86_virt_spec_ctrl() > > Actually it's incomplete. Full version below.
Just one little nit on the comment below, otherwise works nicely. Thanks, Tom > > Thanks, > > tglx > > 8<----------------- > --- a/arch/x86/kernel/process.c > +++ b/arch/x86/kernel/process.c > @@ -403,10 +403,11 @@ static __always_inline void spec_ctrl_up > u64 msr = x86_spec_ctrl_base; > > /* > - * If X86_FEATURE_SSBD is not set, the SSBD bit is not to be > - * touched. > + * If SSBD is not controlled in MSR_SPEC_CTRL, the SSBD bit has not s/has not/is not/ > + * to be touched. > */ > - if (static_cpu_has(X86_FEATURE_SSBD)) > + if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) || > + static_cpu_has(X86_FEATURE_AMD_SSBD)) > msr |= ssbd_tif_to_spec_ctrl(tifn); > > /* Only evaluate if conditional STIBP is enabled */ > @@ -440,7 +441,8 @@ static __always_inline void __speculatio > amd_set_ssb_virt_state(tifn); > else if (static_cpu_has(X86_FEATURE_LS_CFG_SSBD)) > amd_set_core_ssb_state(tifn); > - else if (static_cpu_has(X86_FEATURE_SSBD)) > + else if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) || > + static_cpu_has(X86_FEATURE_AMD_SSBD)) > updmsr = true; > } > >
