From mboxrd@z Thu Jan 1 00:00:00 1970 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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0EFB5C3ABAA for ; Thu, 1 May 2025 08:26:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:In-Reply-To:Subject:Cc:To:From:Message-ID:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=G7qS+gI0XZnjz79ADOoqcYjjhtageVKnZ8CT2Fcuyw4=; b=cdYXFa/DyuwdT35ztm380ULeA8 4gDKhbaiZnlK5fq7NLDECqaTgEjfi4ajK3z6maTCnhHrIzQV0d6hMs6xp4ghj6hs1UMwaCdWgnUTy LQJmPoRszhwWesNwxYXO479ZAwzUiyz26n2+1PcNKgt3ZIDWxiyCO+zRA2yZCDzb3HyFSRGWl9F7j ePfhcX49VSSbrdiJmsWhnhZYIG3OUOwCC6Y+DTp7J/GU5z17q6uOGgNDuhof3rAeBdP0gIsvyXf1u jqt/H98V6BK3AHdgDe4oGde0yZphn2dtIfWegU1Lk89GhZx/VF/XB3dNZRGqCMry1CJd8qZQT/VJ3 0DE4X4yw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uAPFG-0000000F2T5-3Vl7; Thu, 01 May 2025 08:26:14 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uAPDM-0000000F2Bq-2Wbn; Thu, 01 May 2025 08:24:17 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 973215C1364; Thu, 1 May 2025 08:21:58 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 60251C4CEE3; Thu, 1 May 2025 08:24:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1746087855; bh=FUtw7sDZdVXOm9Nv2svOTgxrCByJlvpP+UTjvuTtS1I=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=O97L3cnMvunZjO/0b3AD7mX6YsWjOA1AairfrKios6nfa+PekJx8V9jm72WccaYLD MFg2U9+xORpdBstItLdTn2jDJ/KEsIRT04HWQvHyQCbyTtG0ClRI3D/X2CD6XM+FOj /tz1rvNkqLQslwyHEIsvRxeQEF7oPqOkIM7OE5yCyUmforniQ2Wdg+RNlnV8cmfd9X 16HjICyqst700kZfBgn5lDQrvldBI/haW5J2/qj7rZP5/M2+H5tq4/MkVCxXr+bLok BPs+unjgpwBK9BDZKgsuXLVUjyyxhBLmEvRC18HDlHZbiqF5SS0uUSHzhHrhH/LVHG 92LXRRsmolgow== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1uAPDI-00AWZy-KR; Thu, 01 May 2025 09:24:12 +0100 Date: Thu, 01 May 2025 09:24:11 +0100 Message-ID: <864iy4ivro.wl-maz@kernel.org> From: Marc Zyngier To: Maxim Levitsky Cc: kvm@vger.kernel.org, linux-riscv@lists.infradead.org, Kunkun Jiang , Waiman Long , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Catalin Marinas , Bjorn Helgaas , Boqun Feng , Borislav Petkov , Albert Ou , Anup Patel , Paul Walmsley , Suzuki K Poulose , Palmer Dabbelt , Alexandre Ghiti , Alexander Potapenko , Oliver Upton , Andre Przywara , x86@kernel.org, Joey Gouly , Thomas Gleixner , kvm-riscv@lists.infradead.org, Atish Patra , Ingo Molnar , Jing Zhang , "H. Peter Anvin" , Dave Hansen , kvmarm@lists.linux.dev, Will Deacon , Keisuke Nishimura , Sebastian Ott , Peter Zijlstra , Shusen Li , Paolo Bonzini , Randy Dunlap , Sean Christopherson , Zenghui Yu Subject: Re: [PATCH v4 2/5] arm64: KVM: use mutex_trylock_nest_lock when locking all vCPUs In-Reply-To: <20250430203013.366479-3-mlevitsk@redhat.com> References: <20250430203013.366479-1-mlevitsk@redhat.com> <20250430203013.366479-3-mlevitsk@redhat.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: mlevitsk@redhat.com, kvm@vger.kernel.org, linux-riscv@lists.infradead.org, jiangkunkun@huawei.com, longman@redhat.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com, bhelgaas@google.com, boqun.feng@gmail.com, bp@alien8.de, aou@eecs.berkeley.edu, anup@brainfault.org, paul.walmsley@sifive.com, suzuki.poulose@arm.com, palmer@dabbelt.com, alex@ghiti.fr, glider@google.com, oliver.upton@linux.dev, andre.przywara@arm.com, x86@kernel.org, joey.gouly@arm.com, tglx@linutronix.de, kvm-riscv@lists.infradead.org, atishp@atishpatra.org, mingo@redhat.com, jingzhangos@google.com, hpa@zytor.com, dave.hansen@linux.intel.com, kvmarm@lists.linux.dev, will@kernel.org, keisuke.nishimura@inria.fr, sebott@redhat.com, peterz@infradead.org, lishusen2@huawei.com, pbonzini@redhat.com, rdunlap@infradead.org, seanjc@google.com, yuzenghui@huawei.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250501_012416_730089_EA7DABC5 X-CRM114-Status: GOOD ( 28.81 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org nit: in keeping with the existing arm64 patches, please write the subject as "KVM: arm64: Use ..." On Wed, 30 Apr 2025 21:30:10 +0100, Maxim Levitsky wrote: [...] > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 68fec8c95fee..d31f42a71bdc 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1914,49 +1914,6 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) > } > } > > -/* unlocks vcpus from @vcpu_lock_idx and smaller */ > -static void unlock_vcpus(struct kvm *kvm, int vcpu_lock_idx) > -{ > - struct kvm_vcpu *tmp_vcpu; > - > - for (; vcpu_lock_idx >= 0; vcpu_lock_idx--) { > - tmp_vcpu = kvm_get_vcpu(kvm, vcpu_lock_idx); > - mutex_unlock(&tmp_vcpu->mutex); > - } > -} > - > -void unlock_all_vcpus(struct kvm *kvm) > -{ > - lockdep_assert_held(&kvm->lock); Note this assertion... > - > - unlock_vcpus(kvm, atomic_read(&kvm->online_vcpus) - 1); > -} > - > -/* Returns true if all vcpus were locked, false otherwise */ > -bool lock_all_vcpus(struct kvm *kvm) > -{ > - struct kvm_vcpu *tmp_vcpu; > - unsigned long c; > - > - lockdep_assert_held(&kvm->lock); and this one... > - > - /* > - * Any time a vcpu is in an ioctl (including running), the > - * core KVM code tries to grab the vcpu->mutex. > - * > - * By grabbing the vcpu->mutex of all VCPUs we ensure that no > - * other VCPUs can fiddle with the state while we access it. > - */ > - kvm_for_each_vcpu(c, tmp_vcpu, kvm) { > - if (!mutex_trylock(&tmp_vcpu->mutex)) { > - unlock_vcpus(kvm, c - 1); > - return false; > - } > - } > - > - return true; > -} > - > static unsigned long nvhe_percpu_size(void) > { > return (unsigned long)CHOOSE_NVHE_SYM(__per_cpu_end) - [...] > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 69782df3617f..834f08dfa24c 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -1368,6 +1368,40 @@ static int kvm_vm_release(struct inode *inode, struct file *filp) > return 0; > } > > +/* > + * Try to lock all of the VM's vCPUs. > + * Assumes that the kvm->lock is held. Assuming is not enough. These assertions have caught a number of bugs, and I'm not prepared to drop them. > + */ > +int kvm_trylock_all_vcpus(struct kvm *kvm) > +{ > + struct kvm_vcpu *vcpu; > + unsigned long i, j; > + > + kvm_for_each_vcpu(i, vcpu, kvm) > + if (!mutex_trylock_nest_lock(&vcpu->mutex, &kvm->lock)) > + goto out_unlock; > + return 0; > + > +out_unlock: > + kvm_for_each_vcpu(j, vcpu, kvm) { > + if (i == j) > + break; > + mutex_unlock(&vcpu->mutex); > + } > + return -EINTR; > +} > +EXPORT_SYMBOL_GPL(kvm_trylock_all_vcpus); > + > +void kvm_unlock_all_vcpus(struct kvm *kvm) > +{ > + struct kvm_vcpu *vcpu; > + unsigned long i; > + > + kvm_for_each_vcpu(i, vcpu, kvm) > + mutex_unlock(&vcpu->mutex); > +} > +EXPORT_SYMBOL_GPL(kvm_unlock_all_vcpus); I don't mind you not including the assertions in these helpers, but then the existing primitives have to stay and call into the new stuff. Which, from a simple patch volume, would be far preferable and help with managing backports. I'd also expect the introduction of these new helpers to be done in its own patch, so that we don't get cross architecture dependencies if something needs to be backported for a reason or another. Thanks, M. -- Without deviation from the norm, progress is not possible.