lkml.org 
[lkml]   [2022]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.16 1033/1039] KVM: x86: Partially allow KVM_SET_CPUID{,2} after KVM_RUN
    Date
    From: Vitaly Kuznetsov <vkuznets@redhat.com>

    commit c6617c61e8fe44b9e9fdfede921f61cac6b5149d upstream.

    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@redhat.com>
    Fixes: feb627e8d6f6 ("KVM: x86: Forbid KVM_SET_CPUID{,2} after KVM_RUN")
    Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
    Message-Id: <20220117150542.2176196-3-vkuznets@redhat.com>
    Cc: stable@vger.kernel.org
    [Do not call kvm_find_cpuid_entry repeatedly. - Paolo]
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
    Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    arch/x86/kvm/cpuid.c | 36 ++++++++++++++++++++++++++++++++++++
    arch/x86/kvm/x86.c | 19 -------------------
    2 files changed, 36 insertions(+), 19 deletions(-)

    --- a/arch/x86/kvm/cpuid.c
    +++ b/arch/x86/kvm/cpuid.c
    @@ -99,6 +99,28 @@ static int kvm_check_cpuid(struct kvm_cp
    return 0;
    }

    +/* 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;
    @@ -293,6 +315,20 @@ static int kvm_set_cpuid(struct kvm_vcpu

    __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(e2, nent);
    if (r)
    return r;
    --- a/arch/x86/kvm/x86.c
    +++ b/arch/x86/kvm/x86.c
    @@ -5149,17 +5149,6 @@ long kvm_arch_vcpu_ioctl(struct file *fi
    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;
    @@ -5170,14 +5159,6 @@ long kvm_arch_vcpu_ioctl(struct file *fi
    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;

    \
     
     \ /
      Last update: 2022-01-25 00:11    [W:4.024 / U:0.764 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site