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

Sebastian Andrzej Siewior
2019-10-09T06:44:31Z

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.

…
> 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;