Re: [Patch] KVM: SVM: Fix svm_xsaves_supported

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Aaron Lewis <aaronlewis@google.com> writes:

> On Wed, Sep 4, 2019 at 9:51 AM Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Aaron Lewis <aaronlewis@google.com> writes:
>>
>> > AMD allows guests to execute XSAVES/XRSTORS if supported by the host.  This is different than Intel as they have an additional control bit that determines if XSAVES/XRSTORS can be used by the guest. Intel also has intercept bits that might prevent the guest from intercepting the instruction as well. AMD has none of that, not even an Intercept mechanism.  AMD simply allows XSAVES/XRSTORS to be executed by the guest if also supported by the host.
>> >
>>
>> WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
>>
>> > Signed-off-by: Aaron Lewis <aaronlewis@google.com>
>> > ---
>> >  arch/x86/kvm/svm.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>> > index 1f220a85514f..b681a89f4f7e 100644
>> > --- a/arch/x86/kvm/svm.c
>> > +++ b/arch/x86/kvm/svm.c
>> > @@ -5985,7 +5985,7 @@ static bool svm_mpx_supported(void)
>> >
>> >  static bool svm_xsaves_supported(void)
>> >  {
>> > -     return false;
>> > +     return boot_cpu_has(X86_FEATURE_XSAVES);
>> >  }
>> >
>> >  static bool svm_umip_emulated(void)
>>
>> I had a similar patch in my stash when I tried to debug Hyper-V 2016
>> not being able to boot on KVM. I may have forgotten some important
>> details, but if I'm not mistaken XSAVES comes paired with MSR_IA32_XSS
>> and some OSes may actually try to write there, in particular I've
>> observed Hyper-V 2016 trying to write '0'. Currently, we only support
>> MSR_IA32_XSS in vmx code, this will need to be extended to SVM.
>>
>> Currently, VMX code only supports writing '0' to MSR_IA32_XSS:
>>
>>         case MSR_IA32_XSS:
>>                 if (!vmx_xsaves_supported() ||
>>                     (!msr_info->host_initiated &&
>>                      !(guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
>>                        guest_cpuid_has(vcpu, X86_FEATURE_XSAVES))))
>>                         return 1;
>>                 /*
>>                  * The only supported bit as of Skylake is bit 8, but
>>                  * it is not supported on KVM.
>>                  */
>>                 if (data != 0)
>>                         return 1;
>>
>>
>> we will probably need the same limitation for SVM, however, I'd vote for
>> creating separate kvm_x86_ops->set_xss() implementations.
>>
>> --
>> Vitaly
>
> Fixed the unwrapped description in v2.
>
> As for extending VMX behavior to SVM for MSR_IA_32_XSS; I will do this
> in a follow up patch.  Thanks for calling this out.

Doing this in a separate patch is fine, however, I think this patch
should come before we start announcing XSAVES support on AMD: both
MSR_IA_32_XSS and XSAVES/XRSTORS instructions are enumerated by
CPUID.(EAX=0DH, ECX=1).EAX[bit 3] so an unprepared guest may try
accessing MSR_IA_32_XSS and get #GP.

-- 
Vitaly