<gregkh@xxxxxxxxxxxxxxxxxxx> writes: > The patch below does not apply to the 5.16-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to <stable@xxxxxxxxxxxxxxx>. I'll do the backporting. Side note: there's (at least) one more fix needed in the area for SGX enabled CPUs: https://lore.kernel.org/kvm/20220121132852.2482355-3-vkuznets@xxxxxxxxxx/ ("No functional change intended" doesn't apply anymore as the order has changed). Hope it'll make upstream (Cc: stable) in the next rc. > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From c6617c61e8fe44b9e9fdfede921f61cac6b5149d Mon Sep 17 00:00:00 2001 > From: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > Date: Mon, 17 Jan 2022 16:05:40 +0100 > Subject: [PATCH] KVM: x86: Partially allow KVM_SET_CPUID{,2} after KVM_RUN > > Commit feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN") > forbade changing CPUID altogether but unfortunately this is not fully > compatible with existing VMMs. In particular, QEMU reuses vCPU fds for > CPU hotplug after unplug and it calls KVM_SET_CPUID2. Instead of full ban, > check whether the supplied CPUID data is equal to what was previously set. > > Reported-by: Igor Mammedov <imammedo@xxxxxxxxxx> > Fixes: feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN") > Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > Message-Id: <20220117150542.2176196-3-vkuznets@xxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > [Do not call kvm_find_cpuid_entry repeatedly. - Paolo] > Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 812190a707f6..7eb046d907c6 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -119,6 +119,28 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu, > return fpu_enable_guest_xfd_features(&vcpu->arch.guest_fpu, xfeatures); > } > > +/* Check whether the supplied CPUID data is equal to what is already set for the vCPU. */ > +static int kvm_cpuid_check_equal(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2, > + int nent) > +{ > + struct kvm_cpuid_entry2 *orig; > + int i; > + > + if (nent != vcpu->arch.cpuid_nent) > + return -EINVAL; > + > + for (i = 0; i < nent; i++) { > + orig = &vcpu->arch.cpuid_entries[i]; > + if (e2[i].function != orig->function || > + e2[i].index != orig->index || > + e2[i].eax != orig->eax || e2[i].ebx != orig->ebx || > + e2[i].ecx != orig->ecx || e2[i].edx != orig->edx) > + return -EINVAL; > + } > + > + return 0; > +} > + > static void kvm_update_kvm_cpuid_base(struct kvm_vcpu *vcpu) > { > u32 function; > @@ -313,6 +335,20 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2, > > __kvm_update_cpuid_runtime(vcpu, e2, nent); > > + /* > + * KVM does not correctly handle changing guest CPUID after KVM_RUN, as > + * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't > + * tracked in kvm_mmu_page_role. As a result, KVM may miss guest page > + * faults due to reusing SPs/SPTEs. In practice no sane VMM mucks with > + * the core vCPU model on the fly. It would've been better to forbid any > + * KVM_SET_CPUID{,2} calls after KVM_RUN altogether but unfortunately > + * some VMMs (e.g. QEMU) reuse vCPU fds for CPU hotplug/unplug and do > + * KVM_SET_CPUID{,2} again. To support this legacy behavior, check > + * whether the supplied CPUID data is equal to what's already set. > + */ > + if (vcpu->arch.last_vmentry_cpu != -1) > + return kvm_cpuid_check_equal(vcpu, e2, nent); > + > r = kvm_check_cpuid(vcpu, e2, nent); > if (r) > return r; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 60da2331ec32..a0bc637348b2 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5230,17 +5230,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > struct kvm_cpuid __user *cpuid_arg = argp; > struct kvm_cpuid cpuid; > > - /* > - * KVM does not correctly handle changing guest CPUID after KVM_RUN, as > - * MAXPHYADDR, GBPAGES support, AMD reserved bit behavior, etc.. aren't > - * tracked in kvm_mmu_page_role. As a result, KVM may miss guest page > - * faults due to reusing SPs/SPTEs. In practice no sane VMM mucks with > - * the core vCPU model on the fly, so fail. > - */ > - r = -EINVAL; > - if (vcpu->arch.last_vmentry_cpu != -1) > - goto out; > - > r = -EFAULT; > if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid))) > goto out; > @@ -5251,14 +5240,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp, > struct kvm_cpuid2 __user *cpuid_arg = argp; > struct kvm_cpuid2 cpuid; > > - /* > - * KVM_SET_CPUID{,2} after KVM_RUN is forbidded, see the comment in > - * KVM_SET_CPUID case above. > - */ > - r = -EINVAL; > - if (vcpu->arch.last_vmentry_cpu != -1) > - goto out; > - > r = -EFAULT; > if (copy_from_user(&cpuid, cpuid_arg, sizeof(cpuid))) > goto out; > -- Vitaly
