On Tue, Oct 8, 2019 at 11:44 PM Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> 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!
