Re: [PATCH] x86/spec-ctrl: Use IST RSB protection for !SVM systems

[Jan Beulich <jbeulich@suse.com>]

On 15.08.2022 11:33, Andrew Cooper wrote:
> On 15/08/2022 09:26, Jan Beulich wrote:
>> On 05.08.2022 12:38, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/spec_ctrl.c
>>> +++ b/xen/arch/x86/spec_ctrl.c
>>> @@ -1327,8 +1327,24 @@ void __init init_speculation_mitigations(void)
>>>       * mappings.
>>>       */
>>>      if ( opt_rsb_hvm )
>>> +    {
>>>          setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM);
>>>  
>>> +        /*
>>> +         * For SVM, Xen's RSB safety actions are performed before STGI, so
>>> +         * behave atomically with respect to IST sources.
>>> +         *
>>> +         * For VT-x, NMIs are atomic with VMExit (the NMI gets queued but not
>>> +         * delivered) whereas other IST sources are not atomic.  Specifically,
>>> +         * #MC can hit ahead the RSB safety action in the vmexit path.
>>> +         *
>>> +         * Therefore, it is necessary for the IST logic to protect Xen against
>>> +         * possible rogue RSB speculation.
>>> +         */
>>> +        if ( !cpu_has_svm )
>>> +            default_spec_ctrl_flags |= SCF_ist_rsb;
>> Only now, when I'm about to backport this, it occurs to me to ask: Why
>> is this !cpu_has_svm rather than cpu_has_vmx?
> 
> Because it is only SVM known to be safe.

Yes. Which amounts to only VT-x being unsafe. And in particular PV alone
(e.g. shim, from the perspective of the shim itself) is safe as well, no
matter what CPU we're on.

>> Plus shouldn't this further
>> be gated upon HVM actually being in use (i.e. in particular CONFIG_HVM=y
>> in the first place)?
> 
> Perhaps, but not locally here.  All of init_speculation_mitigations()
> wants reconsidering if you're going down that route.

Not sure - many of the settings (like X86_FEATURE_SC_RSB_HVM also being
set in the enclosing if()) only affect HVM-specific code paths, so which
way they are set wouldn't matter when !CONFIG_HVM. But the one here
clearly affects a common code path, for no gains at all. It's not an
overly hot code path, sure, but it still strikes me as odd.

Jan