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 3422EC982FA for ; Tue, 22 Sep 2026 16:02:08 +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:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From: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=Fj8qv/s1QXBda8PJDTUNriPLEJtaxLhFWQtl59nKm2g=; b=vwtz/225CwY65YJp/QYEeXHcPl fkfHedUTQmahKm2kTf0q+Jav7dHWlpSQfx4DcReVIzNmTuDNv/Y95VxO7AsOCZBMN6wqIW+s/kAOs JVzcxr9zYBTLVtlLpqYFBJw7bunq2OZMnZQ4uH4FSj6aj7rNv89cjQ6siXJpaywr6owlPvky1VbMJ B+XowZ+nhEl6zIBBGgGQrHJzVMUpmoEHOR8tAOEfPwFFdvB2/FZ+TvQrPEc4KbqcRzJ/80BhJCiO2 gH89zMUh4ottvDEajellA+D3QaXlGRJvOPFdZcz/rOl+zqcW7CPQBEx1KU9E7exr5Z0jm1fRWcyDx gZHqGTGg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1x92wP-00000005yjr-2Zws; Tue, 22 Sep 2026 16:01:57 +0000 Received: from tor.source.kernel.org ([2600:3c04:e001:324:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1x92wF-00000005ycD-44t7 for linux-arm-kernel@lists.infradead.org; Tue, 22 Sep 2026 16:01:48 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by tor.source.kernel.org (Postfix) with ESMTP id 1DB0960E1D; Tue, 22 Sep 2026 16:01:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 61E421F0089B; Tue, 22 Sep 2026 16:01:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790092906; bh=Fj8qv/s1QXBda8PJDTUNriPLEJtaxLhFWQtl59nKm2g=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=TgHOkTxpKTkCp2koCURoESESmBJBN0clKbStRr6qZEszbQCwdNdvQlCngTcc4FXyT X3ciCuZGg4tsjBvDauLeGD99T9LKFTC38AG7YYn2SE8tKI0MP9RYZpjh6VPGp1mrE6 QnB8WRl9Ahwhvj8TkU40GxcT1CyqaqHBay5DfVSXfFnwlbl5Y63YYFGLiaRO3yPeA5 ghBCXsnSifEuUsagpHdAwCvn/VOzPBB26gUJnEDMOaPnt4BCcA+/70UlAGxb8A2v0G Kj8Y7aeLSig6sJTwRB2DHqMDIhfmnxtkMdTQPa+M9BMRFJZIorJ4+Ep0yymWA2X/4+ v62pUHjkSJF0g== Date: Tue, 22 Sep 2026 17:01:40 +0100 From: "Lorenzo Stoakes (ARM)" To: Fuad Tabba Cc: Marc Zyngier , Oliver Upton , Joey Gouly , Steffen Eiden , Suzuki K Poulose , Zenghui Yu , Will Deacon , Jack Thomson , kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Fuad Tabba Subject: Re: [PATCH v3] KVM: arm64: Clear the VM's feature bitmap when kvm_setup_vcpu() fails Message-ID: References: <20260921190843.107881-1-fuad.tabba@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260921190843.107881-1-fuad.tabba@linux.dev> 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 On Mon, Sep 21, 2026 at 08:08:43PM +0100, Fuad Tabba wrote: > __kvm_vcpu_set_target() copies the requested features into the VM-wide > bitmap before kvm_setup_vcpu() runs and doesn't undo it when setup > fails, so a rejected KVM_ARM_VCPU_INIT leaves the VM recording features > that were never set up. With HAS_EL2 | HAS_EL2_E2H0 on a host without > FEAT_NV1, kvm_vcpu_init_nested() returns -EINVAL before it allocates any > nested stage-2 MMU, and vcpu_has_nv() is then true with > nested_mmus_size == 0; its -ENOMEM paths do the same on a VM's first > INIT. > > Until one INIT has succeeded, every INIT that passes the feature check > rewrites the bitmap and a failure leaves it there; once > KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED is set, kvm_vcpu_init_changed() > has already required the requested features to equal the bitmap, so > the copy is a no-op. Clear the bitmap on failure while the flag is > clear. > > This showed up with the series that enables KVM_PRE_FAULT_MEMORY for > arm64, since the generic kvm_vcpu_pre_fault_memory() calls vcpu_load() > without checking that the vCPU has been initialised. After the rejected > INIT, the first call's vcpu_load() reads hw_mmu still set to the > canonical MMU and leaves it alone, but its vcpu_put() takes the > vcpu_has_nv() branch into kvm_vcpu_put_hw_mmu(), which clears hw_mmu. > The second call's vcpu_load() reads hw_mmu as NULL and takes the > nested branch into get_s2_mmu_nested(), whose search over > nested_mmus_size == 0 leaves s2_mmu NULL for the > BUG_ON(atomic_read(&s2_mmu->refcnt)), under mmu_lock. On kvmarm/next > with the series applied: This is no longer accurate at the latest revision of my series ([0])but maybe worth keeping for historic detail? [0]: https://lore.kernel.org/kvmarm/20260922-kvm-arm-prefault-v3-0-787bd3bc7e3f@kernel.org/ > > Unable to handle kernel NULL pointer dereference at virtual address 0000000000000074 > Call trace: > kvm_vcpu_load_hw_mmu (arch/arm64/kvm/nested.c:891) (P) > kvm_arch_vcpu_load (arch/arm64/kvm/arm.c:662) > kvm_vcpu_pre_fault_memory (virt/kvm/kvm_main.c:170 virt/kvm/kvm_main.c:4349) > kvm_vcpu_ioctl (virt/kvm/kvm_main.c:4639) > > Fixes: 427733579744e ("KVM: arm64: Select default PMU in KVM_ARM_VCPU_INIT handler") > Suggested-by: Oliver Upton > Link: https://lore.kernel.org/r/20260825-kvm-arm-prefault-v1-0-befe8947702e@kernel.org/ > Signed-off-by: Fuad Tabba Seems reasonable to me so: Reviewed-by: Lorenzo Stoakes (ARM) > --- > v3: > - Clear the bitmap on failure instead of saving and restoring it, only > while KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED is clear, since with the > flag set the copy was a no-op (Oliver). > - Commit message: drop the "latent, exposed by the series" framing and > state the flag invariant behind the clear; the pre-fault crash is > how the bug showed up. > - Dropped Lorenzo's Reviewed-by since the code changed. > > v2: > - Commit message: say what the prefault series enables, bring the > two-ioctl walk and the trace up from below the fold, the trace > decoded, and drop the Fixes: on 1de10b7d13a97, which had nothing > fallible after the copy (Lorenzo). > - Fold Lorenzo's Reviewed-by. > > Reproduced on kvmarm/next plus the series under QEMU (-cpu max with an > Apple M2 MIDR, which has_nv1() denies; kvm-arm.mode=nested): > KVM_ARM_VCPU_INIT with HAS_EL2 | HAS_EL2_E2H0 returns -EINVAL, then > KVM_PRE_FAULT_MEMORY twice. With the fix both calls return -ENOENT and > the host is unaffected. Based on kvmarm/fixes; applies cleanly to > v7.3-rc4 and to kvmarm/next. > > v2: https://lore.kernel.org/r/20260921063718.1604533-1-fuad.tabba@linux.dev/ > v1: https://lore.kernel.org/r/20260918120553.163139-1-fuad.tabba@linux.dev/ > > arch/arm64/kvm/arm.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index eaf583b771931..bd19f64ae82ef 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -1698,8 +1698,16 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu, > bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES); > > ret = kvm_setup_vcpu(vcpu); > - if (ret) > + if (ret) { > + /* > + * Clear the bitmap if setup fails on the first vCPU to be > + * initialized. > + */ > + if (!test_bit(KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED, &kvm->arch.flags)) > + bitmap_zero(kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES); > + > goto out_unlock; > + } > > /* Now we know what it is, we can reset it. */ > kvm_reset_vcpu(vcpu); > > base-commit: 6b1bca1b1ab77f60a62087337bfe6e2f0efb9e6d > -- > 2.39.5 > -- Cheers, Lorenzo