Re: [PATCH v2 9/9] x86/cpuid: Enable MSR_SPEC_CTRL in SVM guests by default

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 31/01/2022 10:39, Jan Beulich wrote:
> On 28.01.2022 14:29, Andrew Cooper wrote:
>> With all other pieces in place, MSR_SPEC_CTRL is fully working for HVM 
>> guests.
>>
>> Update the CPUID derivation logic (both PV and HVM to avoid losing subtle
>> changes), drop the MSR intercept, and explicitly enable the CPUID bits for 
>> HVM
>> guests.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

>
> Oneremark:
>
>> --- a/xen/arch/x86/hvm/svm/svm.c
>> +++ b/xen/arch/x86/hvm/svm/svm.c
>> @@ -606,6 +606,10 @@ static void svm_cpuid_policy_changed(struct vcpu *v)
>>  
>>      vmcb_set_exception_intercepts(vmcb, bitmap);
>>  
>> +    /* Give access to MSR_SPEC_CTRL if the guest has been told about it. */
>> +    svm_intercept_msr(v, MSR_SPEC_CTRL,
>> +                      cp->extd.ibrs ? MSR_INTERCEPT_NONE : 
>> MSR_INTERCEPT_RW);
> Technically I suppose the intercept would also be unneeded if the MSR
> doesn't exist at all, as then the CPU would raise #GP(0) for any guest
> attempt to access it.

Yes, but that is very dangerous.  There are known examples of real model
specific registers in the place where architectural ones also exist. 
The Haswell uarch has two non-faulting MSRs in the x2APIC range.

A guest poking MSR_SPEC_CTRL when it isn't enumerated is not a path
which needs optimising, and taking a vmexit is more robust against
model-specific behaviour.

~Andrew