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
