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 72D25C7115C for ; Fri, 20 Jun 2025 18:51:48 +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=SCWfCfaZ3u6KNLTf3q+pHDxfcJpIR3G2GwUv6btP+Lc=; b=1BvW2/qg+yw7XkOKR8bW2fDOJO ZMh3PbhEgdDnhVesaR9Bibb8//k6SiJhqixgDwtr6resg3bQIBK30p+MPAfUehsgqmY1I9MjUO9nV 6uEoh7M3fp2P4rc98EeWgccWxs4zb1cYPdadyoD+2wOIMBDaBDGoBBDz1Lq+0dhykKHtUoS8e7Xij kI4G4AJ9Gtf2dKhq+NNSnd8Dn4Hxo9i0YYEeZrspp7bpdquJ9AGbcqX9uZHtC/6+j3GveCg3YEWtY CJ6nxCO8PpOjf2dRA+/YQ1AXttKiTsuProv6XrIKHw8IMXVzEWstgd+JosK8V0TwlSdxBxiLsdnJ9 0j2w/CZg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uSgpy-0000000GPLd-0zcn; Fri, 20 Jun 2025 18:51:42 +0000 Received: from out-186.mta1.migadu.com ([95.215.58.186]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uSgna-0000000GP4r-2U1S for linux-arm-kernel@lists.infradead.org; Fri, 20 Jun 2025 18:49:16 +0000 Date: Fri, 20 Jun 2025 11:48:39 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1750445349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=SCWfCfaZ3u6KNLTf3q+pHDxfcJpIR3G2GwUv6btP+Lc=; b=Uv+9F3kf+uawsW/hRSt0qY1C/WI5g9xdumI1n+LnBI60VZNdEmkQYEaHL5g5ZExXcc+as6 QUhbhNdcxCDNrcOlA8K+d1EC2hpMHuNMfm0G1BQgs59RGSbjCYzL7dpN5580FwA4iPVAYB rU+Gk24NczndMIpU5aNVPd4GUJSYLx0= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Oliver Upton To: Sean Christopherson Cc: Marc Zyngier , Paolo Bonzini , Joerg Roedel , David Woodhouse , Lu Baolu , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, iommu@lists.linux.dev, linux-kernel@vger.kernel.org, Sairaj Kodilkar , Vasant Hegde , Maxim Levitsky , Joao Martins , Francesco Lavra , David Matlack Subject: Re: [PATCH v3 02/62] KVM: arm64: WARN if unmapping vLPI fails Message-ID: References: <20250611224604.313496-2-seanjc@google.com> <20250611224604.313496-4-seanjc@google.com> <86tt4lcgs3.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Migadu-Flow: FLOW_OUT X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250620_114915_039828_B24DB7D0 X-CRM114-Status: GOOD ( 39.59 ) 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 Fri, Jun 20, 2025 at 10:22:32AM -0700, Sean Christopherson wrote: > On Fri, Jun 13, 2025, Oliver Upton wrote: > > On Thu, Jun 12, 2025 at 07:34:35AM -0700, Sean Christopherson wrote: > > > On Thu, Jun 12, 2025, Marc Zyngier wrote: > > > > But not having an VLPI mapping for an interrupt at the point where we're > > > > tearing down the forwarding is pretty benign. IRQs *still* go where they > > > > should, and we don't lose anything. > > > > The VM may not actually be getting torn down, though. The series of > > fixes [*] we took for 6.16 addressed games that VMMs might be playing on > > irqbypass for a live VM. > > > > [*] https://lore.kernel.org/kvmarm/20250523194722.4066715-1-oliver.upton@linux.dev/ > > > > > All of those failure scenario seem like warnable offences when KVM thinks it has > > > configured the IRQ to be forwarded to a vCPU. > > > > I tend to agree here, especially considering how horribly fragile GICv4 > > has been in some systems. I know of a couple implementations where ITS > > command failures and/or unmapped MSIs are fatal for the entire machine. > > Debugging them has been a genuine pain in the ass. > > > > WARN'ing when state tracking for vLPIs is out of whack would've made it > > a little easier. > > Marc, does this look and read better? > > I'd really, really like to get this sorted out asap, as it's the only thing > blocking the series, and I want to get the series into linux-next early next > week, before I go OOO for ~10 days. Can you just send it out as a standalone patch? It's only tangientally related to the truckload of x86 stuff that I'd rather not pull in the event of conflict resolution. Thanks, Oliver > -- > From: Sean Christopherson > Date: Thu, 12 Jun 2025 16:51:47 -0700 > Subject: [PATCH] KVM: arm64: WARN if unmapping a vLPI fails in any path > > When unmapping a vLPI, WARN if nullifying vCPU affinity fails, not just if > failure occurs when freeing an ITE. If undoing vCPU affinity fails, then > odds are very good that vLPI state tracking has has gotten out of whack, > i.e. that KVM and the GIC disagree on the state of an IRQ/vLPI. At best, > inconsistent state means there is a lurking bug/flaw somewhere. At worst, > the inconsistency could eventually be fatal to the host, e.g. if an ITS > command fails because KVM's view of things doesn't match reality/hardware. > > Note, only the call from kvm_arch_irq_bypass_del_producer() by way of > kvm_vgic_v4_unset_forwarding() doesn't already WARN. Common KVM's > kvm_irq_routing_update() WARNs if kvm_arch_update_irqfd_routing() fails. > For that path, if its_unmap_vlpi() fails in kvm_vgic_v4_unset_forwarding(), > the only possible causes are that the GIC doesn't have a v4 ITS (from > its_irq_set_vcpu_affinity()): > > /* Need a v4 ITS */ > if (!is_v4(its_dev->its)) > return -EINVAL; > > guard(raw_spinlock)(&its_dev->event_map.vlpi_lock); > > /* Unmap request? */ > if (!info) > return its_vlpi_unmap(d); > > or that KVM has gotten out of sync with the GIC/ITS (from its_vlpi_unmap()): > > if (!its_dev->event_map.vm || !irqd_is_forwarded_to_vcpu(d)) > return -EINVAL; > > All of the above failure scenarios are warnable offences, as they should > never occur absent a kernel/KVM bug. > > Signed-off-by: Sean Christopherson > --- > arch/arm64/kvm/vgic/vgic-its.c | 2 +- > arch/arm64/kvm/vgic/vgic-v4.c | 4 ++-- > drivers/irqchip/irq-gic-v4.c | 4 ++-- > include/linux/irqchip/arm-gic-v4.h | 2 +- > 4 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c > index 534049c7c94b..98630dae910d 100644 > --- a/arch/arm64/kvm/vgic/vgic-its.c > +++ b/arch/arm64/kvm/vgic/vgic-its.c > @@ -758,7 +758,7 @@ static void its_free_ite(struct kvm *kvm, struct its_ite *ite) > if (irq) { > scoped_guard(raw_spinlock_irqsave, &irq->irq_lock) { > if (irq->hw) > - WARN_ON(its_unmap_vlpi(ite->irq->host_irq)); > + its_unmap_vlpi(ite->irq->host_irq); > > irq->hw = false; > } > diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c > index 193946108192..911170d4a9c8 100644 > --- a/arch/arm64/kvm/vgic/vgic-v4.c > +++ b/arch/arm64/kvm/vgic/vgic-v4.c > @@ -545,10 +545,10 @@ int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int host_irq) > if (irq->hw) { > atomic_dec(&irq->target_vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count); > irq->hw = false; > - ret = its_unmap_vlpi(host_irq); > + its_unmap_vlpi(host_irq); > } > > raw_spin_unlock_irqrestore(&irq->irq_lock, flags); > vgic_put_irq(kvm, irq); > - return ret; > + return 0; > } > diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c > index 58c28895f8c4..8455b4a5fbb0 100644 > --- a/drivers/irqchip/irq-gic-v4.c > +++ b/drivers/irqchip/irq-gic-v4.c > @@ -342,10 +342,10 @@ int its_get_vlpi(int irq, struct its_vlpi_map *map) > return irq_set_vcpu_affinity(irq, &info); > } > > -int its_unmap_vlpi(int irq) > +void its_unmap_vlpi(int irq) > { > irq_clear_status_flags(irq, IRQ_DISABLE_UNLAZY); > - return irq_set_vcpu_affinity(irq, NULL); > + WARN_ON_ONCE(irq_set_vcpu_affinity(irq, NULL)); > } > > int its_prop_update_vlpi(int irq, u8 config, bool inv) > diff --git a/include/linux/irqchip/arm-gic-v4.h b/include/linux/irqchip/arm-gic-v4.h > index 7f1f11a5e4e4..0b0887099fd7 100644 > --- a/include/linux/irqchip/arm-gic-v4.h > +++ b/include/linux/irqchip/arm-gic-v4.h > @@ -146,7 +146,7 @@ int its_commit_vpe(struct its_vpe *vpe); > int its_invall_vpe(struct its_vpe *vpe); > int its_map_vlpi(int irq, struct its_vlpi_map *map); > int its_get_vlpi(int irq, struct its_vlpi_map *map); > -int its_unmap_vlpi(int irq); > +void its_unmap_vlpi(int irq); > int its_prop_update_vlpi(int irq, u8 config, bool inv); > int its_prop_update_vsgi(int irq, u8 priority, bool group); > > > base-commit: 4fc39a165c70a49991b7cc29be3a19eddcd9e5b9 > -- >