From patchwork Wed Nov 30 23:08:59 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 28045 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp1198961wrr; Wed, 30 Nov 2022 15:12:32 -0800 (PST) X-Google-Smtp-Source: AA0mqf4yUNydyy3hYbXLHxDh7EbHmp7umcx9Jh5nqrlB7LUKkwacGRplf+b0HjFxx2rOLWndXq/p X-Received: by 2002:a63:1915:0:b0:477:3803:674f with SMTP id z21-20020a631915000000b004773803674fmr37097301pgl.469.1669849952393; Wed, 30 Nov 2022 15:12:32 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1669849952; cv=none; d=google.com; s=arc-20160816; b=Vjxbwr2DpQKZhL9FYlSw4NQEfKtzq7SplKi8j25dpn111gvZEnoZBgs0SmyMgZDo/g rsywlPcuSgCFo7kjorjnrbrxLdS6wqGAgcRvnHigqewb5VIcsducFIpyIuYJTk/RaUGY CkkcS0YxW5ywFkGZ9cYkYMu7/oAvH2tGtsOlUp2RoSEnkdxoumQ0IVG0d6B48G+ysHxP TreD/OzE7AXy4THvE1IeTQQfVBmCdSmR2nL0LpTUoms5dTPH6+M6y0kHgRAZWvkMU01u LUr0KdssHli17dsc98fo92GlVn844bk9u0gDOprQoHZIXQNhUrkHkHk8NPviP1i/AEg8 9bpw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:reply-to:dkim-signature; bh=DDPlKttPz95bIPigIbsZnvNedazYOn7GaaeDonhS6tI=; b=AjECo9tn2FVHVNyXS9uwok85FDZE0OBviQQAahrsxFeZqx8/4ZB/+QYsaRC3bpF6iq 4ZlL/t8fm2vprUUi2y2d2+kTwSnwQFpsL4Jhwkfa48Tzr1jEdMA/TMWZBVnGk9ApICer Kd1XsVIMr6p3zq/nevnsZyV9Gc87JefLJL5EFL/FE9X09NcHXpeIUhNoUHPUmV0An2nv PgwJYqUlWZl6XVGrElpm155GtegPpqRrUEinU+ZcT2HI6bhhLFC0TdGEsY3arYu/Ywf+ vNG0VhZuhevJ21s+zgxHj9eQJNOCb2COkJEGdjlY3mbmsz1PkF8EwFKUbxfzbfu1Vp9m t9yA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=Z6nLxfTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id x6-20020a170902ea8600b001782bd6c443si2290586plb.621.2022.11.30.15.12.18; Wed, 30 Nov 2022 15:12:32 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=Z6nLxfTz; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229955AbiK3XLY (ORCPT + 99 others); Wed, 30 Nov 2022 18:11:24 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47482 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229908AbiK3XKm (ORCPT ); Wed, 30 Nov 2022 18:10:42 -0500 Received: from mail-pj1-x1049.google.com (mail-pj1-x1049.google.com [IPv6:2607:f8b0:4864:20::1049]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2D950934FE for ; Wed, 30 Nov 2022 15:10:03 -0800 (PST) Received: by mail-pj1-x1049.google.com with SMTP id u10-20020a17090a400a00b00215deac75b4so201823pjc.3 for ; Wed, 30 Nov 2022 15:10:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=DDPlKttPz95bIPigIbsZnvNedazYOn7GaaeDonhS6tI=; b=Z6nLxfTzgTUuiwpAG8cvYQa+dN+u8gdYDmvalMupYvTiyNB08ya32PPN3JMi5MQ+zn CKFON4bHGY9uO+OU5rvKypZLm5hBqJSLnUUDK1HFwnV5UgI7QftrXIwrcqqGD/J7JfqE CFBmidj59C7V6og88vcOwpoym4GNhD2bRgcqaNBUXoe/misowqQibWZd3vr8QsfUhhwX i4uWK4yeRp3DB4lRFiuIJyrUfchRtOmsamHvLHwY72jxQuWnDEt5quvvWruAYj+4E7Dq 3qk+oHrWcenEcx4E4wlD8i3c5crP7WSMTS/kUAiUvOcd3cx88naWVp5yOAiefRxYVySU 2z9A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=DDPlKttPz95bIPigIbsZnvNedazYOn7GaaeDonhS6tI=; b=pPEXlufgO24Ogfpj5yyg9VHN2AWQKVThHt2N1jjFRwskXO6YNT8kMBd4IQIm1+Rm2Z GDAHKj6JJ0VCfD3PID8/eQEVCv14kcedPJz8/OzrzlYja5BJrARojgJ4m9lELj+BZ69B 5xAtyB3O2HFSoosJ4iazatZYW0yxJhbZ1drz4TG/ODw2yJTv1/0+M16l8MTrhmBW/qsK RaFeZwrUTxMiucvPve/5EcCtqIZFgwWf3Hy7i20EV3z97M/PaHYgzP3Q4YiHr+u+vFjl U45PKo2uWHs32p0yV9B9szFxPe17bH4jl3D1yquf94piqfNbnmAsWdCNXva8pLpPAWWE 6uiw== X-Gm-Message-State: ANoB5pljKD+WlDo2emB2li4kCEDdTEe/pnjGAsOI37TXMAjWrci9iEri 5oZs2AWP8mUnHQU4pwyPxk6xhTEAAPw= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a17:902:b691:b0:17e:fb19:63ba with SMTP id c17-20020a170902b69100b0017efb1963bamr47034838pls.160.1669849802860; Wed, 30 Nov 2022 15:10:02 -0800 (PST) Reply-To: Sean Christopherson Date: Wed, 30 Nov 2022 23:08:59 +0000 In-Reply-To: <20221130230934.1014142-1-seanjc@google.com> Mime-Version: 1.0 References: <20221130230934.1014142-1-seanjc@google.com> X-Mailer: git-send-email 2.38.1.584.g0f3c55d4c2-goog Message-ID: <20221130230934.1014142-16-seanjc@google.com> Subject: [PATCH v2 15/50] KVM: x86: Serialize vendor module initialization (hardware setup) From: Sean Christopherson To: Paolo Bonzini , Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Christian Borntraeger , Janosch Frank , Claudio Imbrenda , Matthew Rosato , Eric Farman , Sean Christopherson , Vitaly Kuznetsov , David Woodhouse , Paul Durrant Cc: James Morse , Alexandru Elisei , Suzuki K Poulose , Oliver Upton , Atish Patra , David Hildenbrand , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org, linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org, Yuan Yao , Cornelia Huck , Isaku Yamahata , " =?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= " , Fabiano Rosas , Michael Ellerman , Kai Huang , Chao Gao , Thomas Gleixner X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1750964583585474261?= X-GMAIL-MSGID: =?utf-8?q?1750964583585474261?= Acquire a new mutex, vendor_module_lock, in kvm_x86_vendor_init() while doing hardware setup to ensure that concurrent calls are fully serialized. KVM rejects attempts to load vendor modules if a different module has already been loaded, but doesn't handle the case where multiple vendor modules are loaded at the same time, and module_init() doesn't run under the global module_mutex. Note, in practice, this is likely a benign bug as no platform exists that supports both SVM and VMX, i.e. barring a weird VM setup, one of the vendor modules is guaranteed to fail a support check before modifying common KVM state. Alternatively, KVM could perform an atomic CMPXCHG on .hardware_enable, but that comes with its own ugliness as it would require setting .hardware_enable before success is guaranteed, e.g. attempting to load the "wrong" could result in spurious failure to load the "right" module. Introduce a new mutex as using kvm_lock is extremely deadlock prone due to kvm_lock being taken under cpus_write_lock(), and in the future, under under cpus_read_lock(). Any operation that takes cpus_read_lock() while holding kvm_lock would potentially deadlock, e.g. kvm_timer_init() takes cpus_read_lock() to register a callback. In theory, KVM could avoid such problematic paths, i.e. do less setup under kvm_lock, but avoiding all calls to cpus_read_lock() is subtly difficult and thus fragile. E.g. updating static calls also acquires cpus_read_lock(). Inverting the lock ordering, i.e. always taking kvm_lock outside cpus_read_lock(), is not a viable option as kvm_lock is taken in various callbacks that may be invoked under cpus_read_lock(), e.g. x86's kvmclock_cpufreq_notifier(). The lockdep splat below is dependent on future patches to take cpus_read_lock() in hardware_enable_all(), but as above, deadlock is already is already possible. ====================================================== WARNING: possible circular locking dependency detected 6.0.0-smp--7ec93244f194-init2 #27 Tainted: G O ------------------------------------------------------ stable/251833 is trying to acquire lock: ffffffffc097ea28 (kvm_lock){+.+.}-{3:3}, at: hardware_enable_all+0x1f/0xc0 [kvm] but task is already holding lock: ffffffffa2456828 (cpu_hotplug_lock){++++}-{0:0}, at: hardware_enable_all+0xf/0xc0 [kvm] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (cpu_hotplug_lock){++++}-{0:0}: cpus_read_lock+0x2a/0xa0 __cpuhp_setup_state+0x2b/0x60 __kvm_x86_vendor_init+0x16a/0x1870 [kvm] kvm_x86_vendor_init+0x23/0x40 [kvm] 0xffffffffc0a4d02b do_one_initcall+0x110/0x200 do_init_module+0x4f/0x250 load_module+0x1730/0x18f0 __se_sys_finit_module+0xca/0x100 __x64_sys_finit_module+0x1d/0x20 do_syscall_64+0x3d/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd -> #0 (kvm_lock){+.+.}-{3:3}: __lock_acquire+0x16f4/0x30d0 lock_acquire+0xb2/0x190 __mutex_lock+0x98/0x6f0 mutex_lock_nested+0x1b/0x20 hardware_enable_all+0x1f/0xc0 [kvm] kvm_dev_ioctl+0x45e/0x930 [kvm] __se_sys_ioctl+0x77/0xc0 __x64_sys_ioctl+0x1d/0x20 do_syscall_64+0x3d/0x80 entry_SYSCALL_64_after_hwframe+0x63/0xcd other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(cpu_hotplug_lock); lock(kvm_lock); lock(cpu_hotplug_lock); lock(kvm_lock); *** DEADLOCK *** 1 lock held by stable/251833: #0: ffffffffa2456828 (cpu_hotplug_lock){++++}-{0:0}, at: hardware_enable_all+0xf/0xc0 [kvm] Signed-off-by: Sean Christopherson --- Documentation/virt/kvm/locking.rst | 6 ++++++ arch/x86/kvm/x86.c | 18 ++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Documentation/virt/kvm/locking.rst b/Documentation/virt/kvm/locking.rst index 845a561629f1..132a9e5436e5 100644 --- a/Documentation/virt/kvm/locking.rst +++ b/Documentation/virt/kvm/locking.rst @@ -282,3 +282,9 @@ time it will be set using the Dirty tracking mechanism described above. wakeup notification event since external interrupts from the assigned devices happens, we will find the vCPU on the list to wakeup. + +``vendor_module_lock`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +:Type: mutex +:Arch: x86 +:Protects: loading a vendor module (kvm_amd or kvm_intel) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index b33932fca36e..45184ca89317 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -128,6 +128,7 @@ static int kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu); static int __set_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2); static void __get_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2); +static DEFINE_MUTEX(vendor_module_lock); struct kvm_x86_ops kvm_x86_ops __read_mostly; #define KVM_X86_OP(func) \ @@ -9286,7 +9287,7 @@ void kvm_arch_exit(void) } -int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) +static int __kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) { u64 host_pat; int r; @@ -9419,6 +9420,17 @@ int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) kmem_cache_destroy(x86_emulator_cache); return r; } + +int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) +{ + int r; + + mutex_lock(&vendor_module_lock); + r = __kvm_x86_vendor_init(ops); + mutex_unlock(&vendor_module_lock); + + return r; +} EXPORT_SYMBOL_GPL(kvm_x86_vendor_init); void kvm_x86_vendor_exit(void) @@ -9441,7 +9453,6 @@ void kvm_x86_vendor_exit(void) cancel_work_sync(&pvclock_gtod_work); #endif static_call(kvm_x86_hardware_unsetup)(); - kvm_x86_ops.hardware_enable = NULL; kvm_mmu_vendor_module_exit(); free_percpu(user_return_msrs); kmem_cache_destroy(x86_emulator_cache); @@ -9449,6 +9460,9 @@ void kvm_x86_vendor_exit(void) static_key_deferred_flush(&kvm_xen_enabled); WARN_ON(static_branch_unlikely(&kvm_xen_enabled.key)); #endif + mutex_lock(&vendor_module_lock); + kvm_x86_ops.hardware_enable = NULL; + mutex_unlock(&vendor_module_lock); } EXPORT_SYMBOL_GPL(kvm_x86_vendor_exit);