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 46547C5AD55 for ; Mon, 10 Aug 2026 14:04:04 +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=Em+BQBracOSXwokRIimW6WdMSO/3+8vopQcKAyQpy+Y=; b=u0M8ZlHvtSyMA8asEkOZ8Efa+e 6lczrPxRFFKJRubSP5J3+XuykiYU/Oa800DY5mBxN3/Wh1wbn2O+UZnOWfUjPYIKq8UNw8WUsVy8p glBtv60YRAVgxu7ptkbFtYcbYDJgy7Kd/21+pTJQJYJgvh/vwuXiK7j6lqTTRoJmeBq5OIxPWE1lF 4I4mcAwtWvUFE5c0lLgUTw5BHGi6GbhzdPSdbz0GqjeXWFFjmM7WOMs1qGqpJXyGQ6x+sTYCQr4PO wG+kpXOibAfZacAAE+nLunMpXI2prXMUWlPyj7GMC+iy1rBVCRonixZzWXiGeeP4V9IcZhg8LU5yh 5XqVCjjA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtQbZ-0000000BwsZ-0c3q; Mon, 10 Aug 2026 14:03:53 +0000 Received: from sea.source.kernel.org ([2600:3c0a:e001:78e:0:1991:8:25]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtQbX-0000000BwsH-3HyH for linux-arm-kernel@lists.infradead.org; Mon, 10 Aug 2026 14:03:51 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id CAE5240BC0; Mon, 10 Aug 2026 14:03:47 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AB20F1F000E9; Mon, 10 Aug 2026 14:03:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1786370627; bh=Em+BQBracOSXwokRIimW6WdMSO/3+8vopQcKAyQpy+Y=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=SWQRkjKPpy6Z7XWCAcAQi4AW/srTp0eWQMuAibTB4MPtBtgKWR4V3WXbolzXHvHHY QtzWvJ8JO7R1vg2lzPAN8biSEP+74sl7wOqPCccf47NCJfqwDbsfv0dB/56OBNe5vX eu1ja8fMK5ScNpW/ARrrLswNrpJcvolKzVq2UHmXZmtiiCI3HmoyLh5v5aArKDYdbA Tqvp5deUdQLGmhcizGzCp3y5laEpx9ydGOkrgzcE1L/NPbbZXJR7PfygMJUlTcXO1O kg6k/ut045DscaciSP6kwFJYwm8vVSQln4AKTXDHZ9ulo4a4NZL5mkJUT/L4UJ1Zw9 QRYfhYK5cOEOg== 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.98.2) (envelope-from ) id 1wtQbR-0000000E8L5-19oa; Mon, 10 Aug 2026 14:03:45 +0000 Date: Mon, 10 Aug 2026 15:03:44 +0100 Message-ID: <86tsp2139b.wl-maz@kernel.org> From: Marc Zyngier To: Karl Mehltretter Cc: Oliver Upton , kvmarm@lists.linux.dev, Fuad Tabba , Joey Gouly , Steffen Eiden , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Paolo Bonzini , Shuah Khan , Eric Auger , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [RFC PATCH 1/2] KVM: arm64: vgic-v3: Roll back failed redistributor region setup In-Reply-To: <4e00fc25aa61ec52e6ef033a53588ce3f5550982.1786344511.git.kmehltretter@gmail.com> References: <4e00fc25aa61ec52e6ef033a53588ce3f5550982.1786344511.git.kmehltretter@gmail.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: kmehltretter@gmail.com, oupton@kernel.org, kvmarm@lists.linux.dev, fuad.tabba@linux.dev, joey.gouly@arm.com, seiden@linux.ibm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, pbonzini@redhat.com, shuah@kernel.org, eric.auger@redhat.com, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false 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, 10 Aug 2026 07:52:16 +0100, Karl Mehltretter wrote: I'm sorry, but I find it difficult understand what you are trying to explain here: > A later REDIST_REGION attribute can be inserted successfully and then Later than what? > fail while registering redistributor iodevs. For example, a region that > overlaps the distributor is caught by vgic_v3_check_base() when the first > unassigned vCPU is processed. > > The existing rollback unregisters all previously registered > redistributor iodevs. It leaves their rd_iodev.base_addr values set, so a > corrected retry skips re-registering those stale vCPUs and succeeds with > holes in the MMIO bus. Holes in the MMIO space are the norm. The IPA space can multi-TB large, and there is no reason why it'd cover everything (where would you place the RAM otherwise?). Is the problem here that you are left with vcpus that seem to have been matched to an RD (base_addr being set), but that really are left unconnected? > > A failure from kvm_io_bus_register_dev() can leave another hole. The MMIO hole? Coverage hole? > failing vCPU already has its base address and region assigned, but the > old i < c rollback does not include it. What is "it"? > > Preserve devices assigned by earlier successful setters. On failure, > unregister only vCPUs associated with the newly inserted region, clear > their cached base addresses, and free that region. This also includes the > current vCPU when iodev registration itself fails. What I don't see here is an argument explaining that doing this doesn't change the guest-visible assignment of RDs, which would be a regression. > > Fixes: c011f4ea106b ("KVM: arm/arm64: Check vcpu redist base before registering an iodev") > Assisted-by: Codex:gpt-5.6-sol > Signed-off-by: Karl Mehltretter > --- > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 42 ++++++++++++++++++------------ > 1 file changed, 26 insertions(+), 16 deletions(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > index 5913a20d83019..832a7fe65984c 100644 > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c > @@ -841,6 +841,31 @@ void vgic_unregister_redist_iodev(struct kvm_vcpu *vcpu) > kvm_io_bus_unregister_dev(vcpu->kvm, KVM_MMIO_BUS, &rd_dev->dev); > } > > +static void vgic_v3_rollback_redist_region(struct kvm *kvm, u32 index) > +{ > + struct vgic_redist_region *rdreg; > + struct kvm_vcpu *vcpu; > + unsigned long c; > + > + lockdep_assert_held(&kvm->slots_lock); > + > + rdreg = vgic_v3_rdist_region_from_index(kvm, index); > + > + kvm_for_each_vcpu(c, vcpu, kvm) { > + if (vcpu->arch.vgic_cpu.rdreg == rdreg) > + vgic_unregister_redist_iodev(vcpu); > + } > + > + guard(mutex)(&kvm->arch.config_lock); > + > + kvm_for_each_vcpu(c, vcpu, kvm) { > + if (vcpu->arch.vgic_cpu.rdreg == rdreg) > + vcpu->arch.vgic_cpu.rd_iodev.base_addr = VGIC_ADDR_UNDEF; > + } > + > + vgic_v3_free_redist_region(kvm, rdreg); > +} > + > static int vgic_register_all_redist_iodevs(struct kvm *kvm) > { > struct kvm_vcpu *vcpu; > @@ -855,16 +880,6 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm) > break; > } > > - if (ret) { > - /* The current c failed, so iterate over the previous ones. */ > - int i; > - > - for (i = 0; i < c; i++) { > - vcpu = kvm_get_vcpu(kvm, i); > - vgic_unregister_redist_iodev(vcpu); > - } > - } > - > return ret; > } > > @@ -984,12 +999,7 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u32 index, u64 addr, u32 count) > */ > ret = vgic_register_all_redist_iodevs(kvm); > if (ret) { > - struct vgic_redist_region *rdreg; > - > - mutex_lock(&kvm->arch.config_lock); > - rdreg = vgic_v3_rdist_region_from_index(kvm, index); > - vgic_v3_free_redist_region(kvm, rdreg); > - mutex_unlock(&kvm->arch.config_lock); > + vgic_v3_rollback_redist_region(kvm, index); > return ret; > } > Based on what I understand of your earlier description, why isn't this as simple as this untested hack: diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c index 5913a20d83019..804fcc69cbd48 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c @@ -859,7 +859,7 @@ static int vgic_register_all_redist_iodevs(struct kvm *kvm) /* The current c failed, so iterate over the previous ones. */ int i; - for (i = 0; i < c; i++) { + for (i = 0; i <= c; i++) { vcpu = kvm_get_vcpu(kvm, i); vgic_unregister_redist_iodev(vcpu); } @@ -960,8 +960,10 @@ void vgic_v3_free_redist_region(struct kvm *kvm, struct vgic_redist_region *rdre /* Garbage collect the region */ kvm_for_each_vcpu(c, vcpu, kvm) { - if (vcpu->arch.vgic_cpu.rdreg == rdreg) + if (vcpu->arch.vgic_cpu.rdreg == rdreg) { vcpu->arch.vgic_cpu.rdreg = NULL; + vcpu->arch.vgic_cpu.rd_iodev.base_addr = VGIC_ADDR_UNDEF; + } } list_del(&rdreg->list); I don't mind the cleaning up, but not as part of fixing the issue, which has to be as small as possible (think of the backports). Thanks, M. -- Without deviation from the norm, progress is not possible.