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 4B375D65C49 for ; Wed, 17 Dec 2025 14:29:28 +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-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Subject:Cc:To:From: Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=GTSFtdG7RilX07KRE9mudKZaTftBSUpECb14yIxCKJw=; b=0iVirtg4onnRYv37J4eq0ywHZe NBXnHAr4Ogil81BfR7HhII3t2SX8ifPDJiazn2FdGOG6s5ZBZQd44O73sMhCLm0u7TG/rUomVQBXw TMBT1EGl9TZQe/R6n16tb0jMo55MQs8owJrM/V0hh5Nm/+xsdAPVh64BVohx/mwBidNjwiRAAf9Xn 4nXdEiTVlwkjsH7RSYZxl9r5Y2WRtIRF+ok9r6SgY8cc8nNb0ZHdaT4hct6iF8ufJOybjir1WQ0TJ VvL7W98xaH4o9gA+vbOOuGtNwlvmp8t0G0eQ5zFshlr2iHsi8wa8Ei1e4T4qN4ZgZpRcYTSGjG6Nb kwUJKONw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1vVsWq-00000006whP-24pr; Wed, 17 Dec 2025 14:29:25 +0000 Received: from tor.source.kernel.org ([172.105.4.254]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1vVsWp-00000006wgz-0yEu for linux-arm-kernel@lists.infradead.org; Wed, 17 Dec 2025 14:29:23 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 394A860567; Wed, 17 Dec 2025 14:29:22 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BD8F3C113D0; Wed, 17 Dec 2025 14:29:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1765981761; bh=wFfaliFE9aT542xijNvuDtQEWHPNLaDffA9AasSc3Ow=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=lqM8XNqzqosbQvO45T5UX8yqwpgHKJzJNuHlzxIyUa1fSnbiZjqSzJ5/vXHc9uzdg PPriKgMOgNX6fPqoc1zKOZ9u7X3F3i00RtILr4Dhd3cBCZnw7O0SEVaviphDDAg9xH qcz8yLMPr43BDJ9XBnEkDhXNaKNdkepgrUKcMDwy0tU+HAke0NXBcBKLv+Zh1gApLs BGpJuo0p+hmzzPb6CH7iCTMRNIs3wvG5j7nwBzzfGzSSCYrsOescJD222rZaflKYrk 7O0hlDx80HwCIJGu7ZZULNbqT4zmfEOp8Jiz/w+wjHcz85CmQS/EAfs9NLLZt9Lj3u imniPCo24QZlg== 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 1vVsWl-0000000DOoB-0MFe; Wed, 17 Dec 2025 14:29:19 +0000 Date: Wed, 17 Dec 2025 14:29:18 +0000 Message-ID: <86zf7hmbb5.wl-maz@kernel.org> From: Marc Zyngier To: Sascha Bischoff Cc: "linux-arm-kernel@lists.infradead.org" , "kvmarm@lists.linux.dev" , "kvm@vger.kernel.org" , nd , "oliver.upton@linux.dev" , Joey Gouly , Suzuki Poulose , "yuzenghui@huawei.com" , "peter.maydell@linaro.org" , "lpieralisi@kernel.org" , Timothy Hayes Subject: Re: [PATCH 18/32] KVM: arm64: gic-v5: Check for pending PPIs In-Reply-To: <20251212152215.675767-19-sascha.bischoff@arm.com> References: <20251212152215.675767-1-sascha.bischoff@arm.com> <20251212152215.675767-19-sascha.bischoff@arm.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 Content-Transfer-Encoding: quoted-printable X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: Sascha.Bischoff@arm.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, kvm@vger.kernel.org, nd@arm.com, oliver.upton@linux.dev, Joey.Gouly@arm.com, Suzuki.Poulose@arm.com, yuzenghui@huawei.com, peter.maydell@linaro.org, lpieralisi@kernel.org, Timothy.Hayes@arm.com 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 Fri, 12 Dec 2025 15:22:41 +0000, Sascha Bischoff wrote: >=20 > This change allows KVM to check for pending PPI interrupts. This has > two main components: >=20 > First of all, the effective priority mask is calculated. This is a > combination of the priority mask in the VPEs ICC_PCR_EL1.PRIORITY and > the currently running priority as determined from the VPE's > ICH_APR_EL1. If an interrupt's prioirity is greater than or equal to > the effective priority mask, it can be signalled. Otherwise, it > cannot. >=20 > Secondly, any Enabled and Pending PPIs must be checked against this > compound priority mask. The reqires the PPI priorities to by synced > back to the KVM shadow state - this is skipped in general operation as > it isn't required and is rather expensive. If any Enabled and Pending > PPIs are of sufficient priority to be signalled, then there are > pending PPIs. Else, there are not. This ensures that a VPE is not > woken when it cannot actually process the pending interrupts. >=20 > Signed-off-by: Sascha Bischoff > --- > arch/arm64/kvm/vgic/vgic-v5.c | 123 ++++++++++++++++++++++++++++++++++ > arch/arm64/kvm/vgic/vgic.c | 10 ++- > arch/arm64/kvm/vgic/vgic.h | 1 + > 3 files changed, 131 insertions(+), 3 deletions(-) >=20 > diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c > index d54595fbf4586..35740e88b3591 100644 > --- a/arch/arm64/kvm/vgic/vgic-v5.c > +++ b/arch/arm64/kvm/vgic/vgic-v5.c > @@ -54,6 +54,31 @@ int vgic_v5_probe(const struct gic_kvm_info *info) > return 0; > } > =20 > +static u32 vgic_v5_get_effective_priority_mask(struct kvm_vcpu *vcpu) > +{ > + struct vgic_v5_cpu_if *cpu_if =3D &vcpu->arch.vgic_cpu.vgic_v5; > + unsigned highest_ap, priority_mask; Please use explicit types that match their assignment. > + > + /* > + * Counting the number of trailing zeros gives the current > + * active priority. Explicitly use the 32-bit version here as > + * we have 32 priorities. 0x20 then means that there are no > + * active priorities. > + */ > + highest_ap =3D __builtin_ctz(cpu_if->vgic_apr); =46rom https://gcc.gnu.org/onlinedocs/gcc/Bit-Operation-Builtins.html Built-in Function: int __builtin_ctz (unsigned int x) Returns the number of trailing 0-bits in x, starting at the least significant bit position. If x is 0, the result is undefined. We really don't like undefined results. > + > + /* > + * An interrupt is of sufficient priority if it is equal to or > + * greater than the priority mask. Add 1 to the priority mask > + * (i.e., lower priority) to match the APR logic before taking > + * the min. This gives us the lowest priority that is masked. > + */ > + priority_mask =3D FIELD_GET(FEAT_GCIE_ICH_VMCR_EL2_VPMR, cpu_if->vgic_v= mcr); > + priority_mask =3D min(highest_ap, priority_mask + 1); > + > + return priority_mask; > +} > + > static bool vgic_v5_ppi_set_pending_state(struct kvm_vcpu *vcpu, > struct vgic_irq *irq) > { > @@ -121,6 +146,104 @@ void vgic_v5_set_ppi_ops(struct vgic_irq *irq) > irq->ops =3D &vgic_v5_ppi_irq_ops; > } > =20 > + > +/* > + * Sync back the PPI priorities to the vgic_irq shadow state > + */ > +static void vgic_v5_sync_ppi_priorities(struct kvm_vcpu *vcpu) > +{ > + struct vgic_v5_cpu_if *cpu_if =3D &vcpu->arch.vgic_cpu.vgic_v5; > + unsigned long flags; > + int i, reg; > + > + /* We have 16 PPI Priority regs */ > + for (reg =3D 0; reg < 16; reg++) { > + const unsigned long priorityr =3D cpu_if->vgic_ppi_priorityr[reg]; > + > + for (i =3D 0; i < 8; ++i) { Urgh... 128 locks being taken is no good. We need something better. > + struct vgic_irq *irq; > + u32 intid; > + u8 priority; > + > + priority =3D (priorityr >> (i * 8)) & 0x1f; > + > + intid =3D FIELD_PREP(GICV5_HWIRQ_TYPE, GICV5_HWIRQ_TYPE_PPI); > + intid |=3D FIELD_PREP(GICV5_HWIRQ_ID, reg * 8 + i); > + > + irq =3D vgic_get_vcpu_irq(vcpu, intid); > + raw_spin_lock_irqsave(&irq->irq_lock, flags); > + > + irq->priority =3D priority; > + > + raw_spin_unlock_irqrestore(&irq->irq_lock, flags); scoped_guard() > + vgic_put_irq(vcpu->kvm, irq); > + } > + } > +} > + > +bool vgic_v5_has_pending_ppi(struct kvm_vcpu *vcpu) > +{ > + struct vgic_v5_cpu_if *cpu_if =3D &vcpu->arch.vgic_cpu.vgic_v5; > + unsigned long flags; > + int i, reg; > + unsigned int priority_mask; > + > + /* If no pending bits are set, exit early */ > + if (likely(!cpu_if->vgic_ppi_pendr[0] && !cpu_if->vgic_ppi_pendr[1])) > + return false; > + > + priority_mask =3D vgic_v5_get_effective_priority_mask(vcpu); > + > + /* If the combined priority mask is 0, nothing can be signalled! */ > + if (!priority_mask) > + return false; > + > + /* The shadow priority is only updated on demand, sync it across first = */ > + vgic_v5_sync_ppi_priorities(vcpu); > + > + for (reg =3D 0; reg < 2; reg++) { > + unsigned long possible_bits; > + const unsigned long enabler =3D cpu_if->vgic_ich_ppi_enabler_exit[reg]; > + const unsigned long pendr =3D cpu_if->vgic_ppi_pendr_exit[reg]; > + bool has_pending =3D false; > + > + /* Check all interrupts that are enabled and pending */ > + possible_bits =3D enabler & pendr; > + > + /* > + * Optimisation: pending and enabled with no active priorities > + */ > + if (possible_bits && priority_mask > 0x1f) > + return true; > + > + for_each_set_bit(i, &possible_bits, 64) { > + struct vgic_irq *irq; > + u32 intid; > + > + intid =3D FIELD_PREP(GICV5_HWIRQ_TYPE, GICV5_HWIRQ_TYPE_PPI); > + intid |=3D FIELD_PREP(GICV5_HWIRQ_ID, reg * 64 + i); > + > + irq =3D vgic_get_vcpu_irq(vcpu, intid); > + raw_spin_lock_irqsave(&irq->irq_lock, flags); > + > + /* > + * We know that the interrupt is enabled and pending, so > + * only check the priority. > + */ > + if (irq->priority <=3D priority_mask) > + has_pending =3D true; > + > + raw_spin_unlock_irqrestore(&irq->irq_lock, flags); > + vgic_put_irq(vcpu->kvm, irq); > + > + if (has_pending) > + return true; > + } > + } So we do this stuff *twice*. Doesn't strike me as being optimal. It is also not clear that we need to resync it all when calling kvm_vgic_vcpu_pending_irq(), which can happen for any odd reason (spurious wake-up from kvm_vcpu_check_block()). > + > + return false; > +} > + > /* > * Detect any PPIs state changes, and propagate the state with KVM's > * shadow structures. > diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c > index e534876656ca7..5d18a03cc11d5 100644 > --- a/arch/arm64/kvm/vgic/vgic.c > +++ b/arch/arm64/kvm/vgic/vgic.c > @@ -1174,11 +1174,15 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vc= pu) > unsigned long flags; > struct vgic_vmcr vmcr; > =20 > - if (!vcpu->kvm->arch.vgic.enabled) > + if (!vcpu->kvm->arch.vgic.enabled && !vgic_is_v5(vcpu->kvm)) > return false; > =20 > - if (vcpu->arch.vgic_cpu.vgic_v3.its_vpe.pending_last) > - return true; > + if (vcpu->kvm->arch.vgic.vgic_model =3D=3D KVM_DEV_TYPE_ARM_VGIC_V5) { > + return vgic_v5_has_pending_ppi(vcpu); > + } else { Drop the 'else'. > + if (vcpu->arch.vgic_cpu.vgic_v3.its_vpe.pending_last) > + return true; > + } > =20 > vgic_get_vmcr(vcpu, &vmcr); > =20 > diff --git a/arch/arm64/kvm/vgic/vgic.h b/arch/arm64/kvm/vgic/vgic.h > index 5a77318ddb87a..4b3a1e7ca3fb4 100644 > --- a/arch/arm64/kvm/vgic/vgic.h > +++ b/arch/arm64/kvm/vgic/vgic.h > @@ -387,6 +387,7 @@ void vgic_debug_destroy(struct kvm *kvm); > int vgic_v5_probe(const struct gic_kvm_info *info); > void vgic_v5_set_ppi_ops(struct vgic_irq *irq); > int vgic_v5_set_ppi_dvi(struct kvm_vcpu *vcpu, u32 irq, bool dvi); > +bool vgic_v5_has_pending_ppi(struct kvm_vcpu *vcpu); > void vgic_v5_flush_ppi_state(struct kvm_vcpu *vcpu); > void vgic_v5_fold_irq_state(struct kvm_vcpu *vcpu); > void vgic_v5_load(struct kvm_vcpu *vcpu); Thanks, M. --=20 Without deviation from the norm, progress is not possible.