From patchwork Tue Jan 10 15:25:24 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 13095293 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 101C3C46467 for ; Tue, 10 Jan 2023 15:26:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238891AbjAJP0X (ORCPT ); Tue, 10 Jan 2023 10:26:23 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238936AbjAJPZ4 (ORCPT ); Tue, 10 Jan 2023 10:25:56 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A95D08F28D for ; Tue, 10 Jan 2023 07:25:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=MIME-Version:Content-Type:Date:Cc:To: From:Subject:Message-ID:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:In-Reply-To:References; bh=T8Obw+7sjFmbKysBeUKTza7vyg9gBvKU2wHfICs3Syc=; b=RsAejYPzPUIkQLba5Ot9s2f1dT 8wu5KzgGfebovUBs9s9arHC5bW6d+V0mj4/R9Oyi8V4bcnj/5xIUBQWgOJd93ixxuHeizy+iu2BfH 0Un4EUi6AQ3ZCW/yl3M0mj1MBrM/YXvxHQYEIOrVWiTrbruOoZjw4Cmd/bMPufVMi7VHOZsDu04Lb GD9wASEDtPytUB8VP/sZ1SvZ0Mm2CIt075zLIH5X6GtnKrOruoD7yfZYXLiUacUxv+B+CFaI5tKfH UE4klWlyIZKEyhX0XxlH9lRyUxhqNU3VdzVndlnx7DGwCd4YUOgSvR9tU8xeo4N3FVya3hAIRjuih vg2pNr6w==; Received: from [2001:8b0:10b:5::bb3] (helo=u3832b3a9db3152.ant.amazon.com) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1pFGVO-003K6I-FX; Tue, 10 Jan 2023 15:25:38 +0000 Message-ID: <99b1da6ca8293b201fe0a89fd973a9b2f70dc450.camel@infradead.org> Subject: [PATCH] KVM: x86/xen: Fix lockdep warning on "recursive" gpc locking From: David Woodhouse To: Paolo Bonzini , paul , Sean Christopherson Cc: kvm , Peter Zijlstra Date: Tue, 10 Jan 2023 15:25:24 +0000 User-Agent: Evolution 3.44.4-0ubuntu1 MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org From: David Woodhouse In commit 5ec3289b31 ("KVM: x86/xen: Compatibility fixes for shared runstate area") we declared it safe to obtain two gfn_to_pfn_cache locks at the same time: /* * The guest's runstate_info is split across two pages and we * need to hold and validate both GPCs simultaneously. We can * declare a lock ordering GPC1 > GPC2 because nothing else * takes them more than one at a time. */ However, we forgot to tell lockdep. Do so, by setting a subclass on the first lock before taking the second. Fixes: 5ec3289b31 ("KVM: x86/xen: Compatibility fixes for shared runstate area") Suggested-by: Peter Zijlstra Signed-off-by: David Woodhouse --- arch/x86/kvm/xen.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index 402d9a34552c..07e61cc9881e 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -305,8 +305,10 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic) * The guest's runstate_info is split across two pages and we * need to hold and validate both GPCs simultaneously. We can * declare a lock ordering GPC1 > GPC2 because nothing else - * takes them more than one at a time. + * takes them more than one at a time. Set a subclass on the + * gpc1 lock to make lockdep shut up about it. */ + lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_); read_lock(&gpc2->lock); if (!kvm_gpc_check(gpc2, user_len2)) { From patchwork Wed Jan 11 09:37:50 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 13096352 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 903B7C6379F for ; Wed, 11 Jan 2023 09:43:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232249AbjAKJmc (ORCPT ); Wed, 11 Jan 2023 04:42:32 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43090 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235627AbjAKJlj (ORCPT ); Wed, 11 Jan 2023 04:41:39 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7B6A6D6B for ; Wed, 11 Jan 2023 01:37:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=MIME-Version:Content-Type:References: In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=asDDLkhhiVe8MatzoaV5vyaHB/p2X1IB9PzQ1tQv9UA=; b=pSM9VHjuduHfkTk520DkWAYvI6 DtX6eCDu87iE5K+My7WzLmFzT3xnmqtMUqae2B+Rh32rNrk7pq2x4zkWWDvvxB/3whfq+5vPV+Hxy x0fjYUjObQzu3xP8+iMPfFk6Uby29j1tRL4aVWddio5KhLCEI2+gPoi/MqyvS44DZVIBqVHICFYk0 QNferzVJemrM9UJZ4xaoDrmVm2ekpRPHAMXDvQfoXA+WklH8FsPkhLEZh3xxAgm0aJjxf2a2gmm0D 5hNcuRHn87v86Qid2eJdk3skLAkEZeLlzp9vrpdyHfnejStrpfc3/H1r4pLmiAaQxDRUnC7WrQFC0 JTzZJgNw==; Received: from [2001:8b0:10b:5::bb3] (helo=u3832b3a9db3152.ant.amazon.com) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1pFXYa-0040AZ-Kw; Wed, 11 Jan 2023 09:38:04 +0000 Message-ID: <03f0a9ddf3db211d969ff4eb4e0aeb8789683776.camel@infradead.org> Subject: [PATCH 2/3] KVM: x86/xen: Fix potential deadlock in kvm_xen_update_runstate_guest() From: David Woodhouse To: Paolo Bonzini , paul , Sean Christopherson Cc: kvm , Peter Zijlstra , Michal Luczaj Date: Wed, 11 Jan 2023 09:37:50 +0000 In-Reply-To: <99b1da6ca8293b201fe0a89fd973a9b2f70dc450.camel@infradead.org> References: <99b1da6ca8293b201fe0a89fd973a9b2f70dc450.camel@infradead.org> User-Agent: Evolution 3.44.4-0ubuntu1 MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org From: David Woodhouse The kvm_xen_update_runstate_guest() function can be called when the vCPU is being scheduled out, from a preempt notifier. It *opportunistically* updates the runstate area in the guest memory, if the gfn_to_pfn_cache which caches the appropriate address is still valid. If there is *contention* when it attempts to obtain gpc->lock, then locking inside the priority inheritance checks may cause a deadlock. Lockdep reports: [13890.148997] Chain exists of:                  &gpc->lock --> &p->pi_lock --> &rq->__lock [13890.149002]  Possible unsafe locking scenario: [13890.149003]        CPU0                    CPU1 [13890.149004]        ----                    ---- [13890.149005]   lock(&rq->__lock); [13890.149007]                                lock(&p->pi_lock); [13890.149009]                                lock(&rq->__lock); [13890.149011]   lock(&gpc->lock); [13890.149013]                 *** DEADLOCK *** In the general case, if there's contention for a read lock on gpc->lock, that's going to be because something else is either invalidating or revalidating the cache. Either way, we've raced with seeing it in an invalid state, in which case we would have aborted the opportunistic update anyway. So in the 'atomic' case when called from the preempt notifier, just switch to using read_trylock() and avoid the PI handling altogether. Signed-off-by: David Woodhouse --- First patch in this series was '[PATCH] KVM: x86/xen: Fix lockdep warning on "recursive" gpc locking' but now there are three.  arch/x86/kvm/xen.c | 17 ++++++++++++++---  1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index 07e61cc9881e..c444948ab1ac 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -272,7 +272,12 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)          * Attempt to obtain the GPC lock on *both* (if there are two)          * gfn_to_pfn caches that cover the region.          */ -       read_lock_irqsave(&gpc1->lock, flags); +       local_irq_save(flags); +       if (!read_trylock(&gpc1->lock)) { +               if (atomic) +                       return; +               read_lock(&gpc1->lock); +       }         while (!kvm_gpc_check(gpc1, user_len1)) {                 read_unlock_irqrestore(&gpc1->lock, flags);   @@ -283,7 +288,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)                 if (kvm_gpc_refresh(gpc1, user_len1))                         return;   -               read_lock_irqsave(&gpc1->lock, flags); +               goto retry;         }           if (likely(!user_len2)) { @@ -309,7 +314,13 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)                  * gpc1 lock to make lockdep shut up about it.                  */                 lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_); -               read_lock(&gpc2->lock); +               if (!read_trylock(&gpc2->lock)) { +                       if (atomic) { +                               read_unlock_irqrestore(&gpc1->lock, flags); +                               return; +                       } +                       read_lock(&gpc2->lock); +               }                   if (!kvm_gpc_check(gpc2, user_len2)) {                         read_unlock(&gpc2->lock); From patchwork Wed Jan 11 09:37:57 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 13096351 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0A152C5479D for ; Wed, 11 Jan 2023 09:42:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231598AbjAKJma (ORCPT ); Wed, 11 Jan 2023 04:42:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43096 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235829AbjAKJlj (ORCPT ); Wed, 11 Jan 2023 04:41:39 -0500 Received: from casper.infradead.org (casper.infradead.org [IPv6:2001:8b0:10b:1236::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 40768D9F for ; Wed, 11 Jan 2023 01:38:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=MIME-Version:Content-Type:References: In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=2fDiKDpHWQxPlgiPnPqPDR3+XIqmos/N+hJoBE0bo5Q=; b=qqhYbpDRxaw+n8ruRSimOqxTQU c9MaukGhM982W6SpZGgstAaVYbI/MXo056puJ3HoDinx0+YGXxuCrrF9tBNIZm2O2naeCfrdhqtSa 0z5KhB3fPpuPCgN+jAhz+K3ttbv0kUYX7shh/iP8QD5wQbCzTV+/ziti48XxtTf2nSsm/Kg9rZYVo uBRSOW1gYpWg4L/vQ3NO/3jko+BxPpg3kfy4sprgh7wSFSFoJG5k+xykQyuIzPwMkdNVm8h6ExdI5 rb3Q43G72xRjJVEKJRQSXY00zpJ/K/VG2synjRgNblbKaaOMfkfrqHsH+ucP0T/A5WItDDDBazFyc O4tM1L4g==; Received: from [2001:8b0:10b:5::bb3] (helo=u3832b3a9db3152.ant.amazon.com) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1pFXYg-0040BG-DI; Wed, 11 Jan 2023 09:38:10 +0000 Message-ID: <83b174d505d47967f3c8762d231b69b6fc48d80a.camel@infradead.org> Subject: [PATCH 3/3] KVM: Ensure lockdep knows about kvm->lock vs. vcpu->mutex ordering rule From: David Woodhouse To: Paolo Bonzini , paul , Sean Christopherson Cc: kvm , Peter Zijlstra , Michal Luczaj Date: Wed, 11 Jan 2023 09:37:57 +0000 In-Reply-To: <99b1da6ca8293b201fe0a89fd973a9b2f70dc450.camel@infradead.org> References: <99b1da6ca8293b201fe0a89fd973a9b2f70dc450.camel@infradead.org> User-Agent: Evolution 3.44.4-0ubuntu1 MIME-Version: 1.0 X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org From: David Woodhouse Documentation/virt/kvm/locking.rst tells us that kvm->lock is taken outside vcpu->mutex. But that doesn't actually happen very often; it's only in some esoteric cases like migration with AMD SEV. This means that lockdep usually doesn't notice, and doesn't do its job of keeping us honest. Ensure that lockdep *always* knows about the ordering of these two locks, by briefly taking vcpu->mutex in kvm_vm_ioctl_create_vcpu() while kvm->lock is held. Signed-off-by: David Woodhouse ---  virt/kvm/kvm_main.c | 7 +++++++  1 file changed, 7 insertions(+) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 07bf29450521..5814037148bd 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -3924,6 +3924,13 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)         }           mutex_lock(&kvm->lock); + +#ifdef CONFIG_LOCKDEP +       /* Ensure that lockdep knows vcpu->mutex is taken *inside* kvm->lock */ +       mutex_lock(&vcpu->mutex); +       mutex_unlock(&vcpu->mutex); +#endif +         if (kvm_get_vcpu_by_id(kvm, id)) {                 r = -EEXIST;                 goto unlock_vcpu_destroy;