Re: [Patch 3/6] kvm: svm: Add support for XSAVES on AMD

Aaron Lewis
2019-10-10T14:42:44Z

On Tue, Oct 8, 2019 at 11:44 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2019-10-08 17:41:39 [-0700], Aaron Lewis wrote:
> > Hoist support for IA32_XSS so it can be used for both AMD and Intel,
>
> Hoist
>
> > instead of for just Intel.
> >
> > AMD has no equivalent of Intel's "Enable XSAVES/XRSTORS" VM-execution
> > control. Instead, XSAVES is always available to the guest when supported
> > on the host.
>
> You could add that implement the XSAVES check based on host's features
> and move the MSR_IA32_XSS msr R/W from Intel only code to the common
> code.

Isn't this covered by my comments? I mention that we are hoisting
IA32_XSS to common code in the first comment, then in the second
comment I say that XSAVES is available in the guest when supported on
the host.

>
> …
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index e90e658fd8a9..77f2e8c05047 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2702,6 +2702,15 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > case MSR_IA32_TSC:
> > kvm_write_tsc(vcpu, msr_info);
> > break;
> > + case MSR_IA32_XSS:
> > + if (!kvm_x86_ops->xsaves_supported() ||
> > + (!msr_info->host_initiated &&
> > + !guest_cpuid_has(vcpu, X86_FEATURE_XSAVES)))
> > + return 1;
>
> I wouldn't ditch the comment. You could explain why only zero is allowed
> to be written. The Skylake is not true for both but this probably
> requires an explanation.
>
> > + if (data != 0)
> > + return 1;
> > + vcpu->arch.ia32_xss = data;
> > + break;
> > case MSR_SMI_COUNT:
> > if (!msr_info->host_initiated)
> > return 1;

Resending as plain text!