lkml.org 
[lkml]   [2021]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 09/22] kvm: x86: Enable dynamic XSAVE features at KVM_SET_CPUID2
On Wed, Dec 22, 2021, Jing Liu wrote:
> Statically enable all xfeatures allowed by guest perm in

Statically isn't the right word. It's not dymanic with respect to running the
vCPU, but it's certainly not static. I think you can just omit "Statically"
entirely.

> KVM_SET_CPUID2, with fpstate buffer sized accordingly. This avoids
> run-time expansion in the emulation and restore path of XCR0 and
> XFD MSR [1].
>
> Change kvm_vcpu_after_set_cpuid() to return error given fpstate
> reallocation may fail.
>
> [1] https://lore.kernel.org/all/20211214024948.048572883@linutronix.de/
>
> Suggested-by: Thomas Gleixner <tglx@linutronix.de>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jing Liu <jing2.liu@intel.com>
> ---
> arch/x86/kvm/cpuid.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index a068373a7fbd..eb5a5070accb 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -204,10 +204,12 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu)
> }
> EXPORT_SYMBOL_GPL(kvm_update_cpuid_runtime);
>
> -static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> +static int kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> {
> struct kvm_lapic *apic = vcpu->arch.apic;
> struct kvm_cpuid_entry2 *best;
> + u64 xfeatures;
> + int r;
>
> best = kvm_find_cpuid_entry(vcpu, 1, 0);
> if (best && apic) {
> @@ -222,9 +224,17 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> best = kvm_find_cpuid_entry(vcpu, 0xD, 0);
> if (!best)
> vcpu->arch.guest_supported_xcr0 = 0;
> - else
> - vcpu->arch.guest_supported_xcr0 =
> - (best->eax | ((u64)best->edx << 32)) & supported_xcr0;
> + else {
> + xfeatures = best->eax | ((u64)best->edx << 32);
> +
> + vcpu->arch.guest_supported_xcr0 = xfeatures & supported_xcr0;
> +
> + if (xfeatures != vcpu->arch.guest_fpu.xfeatures) {
> + r = fpu_update_guest_perm_features(&vcpu->arch.guest_fpu);
> + if (r)
> + return r;

IMO, this should be done and check before "committing" state, otherwise KVM will
set the vCPU's CPUID info and update a variety of state, but then tell userspace
that it failed. The -EPERM case in particular falls squarely into the "check"
category.

\
 
 \ /
  Last update: 2021-12-29 00:54    [W:0.211 / U:2.676 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site