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 A4333D2E027 for ; Wed, 23 Oct 2024 08:58:41 +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=wVMyp9m7uiqhk1pHWmqihIDtTHvYIv0I12fEMcnulzQ=; b=bNtpOT/UzSe+AMz9AmfVDQygU8 ATtLo5BC3uMLM5lD9Z+7rFZRxL9jYbg6zqCwPSoNwQVM09O+U0FUdCSN0Uujrk0bZ0cO/q84k/Yp0 +2I93rq70LBXpObkmhGs0bjycMHPKmdRYzeFknKMXFqX3DywcvjvCt2IHYzxOiEF7m0ieLkfwE2wE 9gvt2RWhW9lWjVmrmK4jABaGuXATWxjz3vI3CksZtT//PBQI39ICAWOT5f9exToTqMc80E1iNN5/o eSmZ826KsKD9jX4IOUMOv5fLKDs6cK1JUMvVnYr14BaMMAKmC8Jy0rXrJGh47kHpEvWjuXY0N4IHO LeOhXu/Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1t3XCK-0000000Dfc5-1tnV; Wed, 23 Oct 2024 08:58:32 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1t3X3M-0000000DdI7-1RkG for linux-arm-kernel@lists.infradead.org; Wed, 23 Oct 2024 08:49:17 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id EAF6C5C5EA0; Wed, 23 Oct 2024 08:49:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 30902C4CEC7; Wed, 23 Oct 2024 08:49:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1729673355; bh=aR0cBNJNvx0pC7rUSs2+VJJ+53XLheSlUDQRnOTIgzk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ObHgILwXNR82AnJ4xg1/nF6/fZ6ryTPGPghZMb3V3D3ksSOm8f1OAgnTWy4MJP8oF nFKKhaRKXCDTsQMGYkkC0ee0tHoKHj+Xnsa43lxJaAD2Hv4eAatfZ3Oe49qKFhySAU ENMcHIxQRvP+KEBY41KUAMlMWyyIxEBU49yX52E5sfxb+C3vy/w3+/Ak8i7b85UWFU 1wT1FX5TovDn+aBfBQnAkacfyMgKvkG0uP60t8TDqKnW2FVbYcb74iWj20AcYtimfM 4z9WYNjyzjxZaOf3svI5iNdxyKru//+ZxQwDvAGGSTpmbvRhYgEF4pgVqAUv+oaqtc 2V5sqbYD7mmZw== Received: from [104.132.45.109] (helo=wait-a-minute.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 1t3X3I-0062U1-Rh; Wed, 23 Oct 2024 09:49:12 +0100 Date: Wed, 23 Oct 2024 09:49:12 +0100 Message-ID: <87wmhztd9z.wl-maz@kernel.org> From: Marc Zyngier To: Zenghui Yu Cc: , , Thomas Gleixner , Kunkun Jiang Subject: Re: [PATCH] irqchip/gic-v4: Don't allow a VMOVP on a dying VPE In-Reply-To: References: <20241002204959.2051709-1-maz@kernel.org> 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/29.4 (x86_64-pc-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: 104.132.45.109 X-SA-Exim-Rcpt-To: yuzenghui@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, tglx@linutronix.de, jiangkunkun@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-20241023_014916_499629_9C952224 X-CRM114-Status: GOOD ( 34.78 ) 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 Hi Zenghui, On Tue, 22 Oct 2024 08:45:17 +0100, Zenghui Yu wrote: > > Hi Marc, > > On 2024/10/3 4:49, Marc Zyngier wrote: > > Kunkun Jiang reports that there is a small window of opportunity for > > userspace to force a change of affinity for a VPE while the VPE has > > already been unmapped, but the corresponding doorbell interrupt still > > visible in /proc/irq/. > > > > Plug the race by checking the value of vmapp_count, which tracks whether > > the VPE is mapped ot not, and returning an error in this case. > > > > This involves making vmapp_count common to both GICv4.1 and its v4.0 > > ancestor. > > > > Reported-by: Kunkun Jiang > > Signed-off-by: Marc Zyngier > > Link: https://lore.kernel.org/r/c182ece6-2ba0-ce4f-3404-dba7a3ab6c52@huawei.com > > --- > > drivers/irqchip/irq-gic-v3-its.c | 18 ++++++++++++------ > > include/linux/irqchip/arm-gic-v4.h | 4 +++- > > 2 files changed, 15 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > > index fdec478ba5e7..ab597e74ba08 100644 > > --- a/drivers/irqchip/irq-gic-v3-its.c > > +++ b/drivers/irqchip/irq-gic-v3-its.c > > @@ -797,8 +797,8 @@ static struct its_vpe *its_build_vmapp_cmd(struct its_node *its, > > its_encode_valid(cmd, desc->its_vmapp_cmd.valid); > > > > if (!desc->its_vmapp_cmd.valid) { > > + alloc = !atomic_dec_return(&desc->its_vmapp_cmd.vpe->vmapp_count); > > if (is_v4_1(its)) { > > - alloc = !atomic_dec_return(&desc->its_vmapp_cmd.vpe->vmapp_count); > > its_encode_alloc(cmd, alloc); > > /* > > * Unmapping a VPE is self-synchronizing on GICv4.1, > > @@ -817,13 +817,13 @@ static struct its_vpe *its_build_vmapp_cmd(struct its_node *its, > > its_encode_vpt_addr(cmd, vpt_addr); > > its_encode_vpt_size(cmd, LPI_NRBITS - 1); > > > > + alloc = !atomic_fetch_inc(&desc->its_vmapp_cmd.vpe->vmapp_count); > > + > > if (!is_v4_1(its)) > > goto out; > > > > vconf_addr = virt_to_phys(page_address(desc->its_vmapp_cmd.vpe->its_vm->vprop_page)); > > > > - alloc = !atomic_fetch_inc(&desc->its_vmapp_cmd.vpe->vmapp_count); > > - > > its_encode_alloc(cmd, alloc); > > > > /* > > @@ -3806,6 +3806,13 @@ static int its_vpe_set_affinity(struct irq_data *d, > > struct cpumask *table_mask; > > unsigned long flags; > > > > + /* > > + * Check if we're racing against a VPE being destroyed, for > > + * which we don't want to allow a VMOVP. > > + */ > > + if (!atomic_read(&vpe->vmapp_count)) > > + return -EINVAL; > > We lazily map the vPE so that vmapp_count is likely to be 0 on GICv4.0 > implementations with the ITSList feature. Seems that that implementation > is not affected by the reported race and we don't need to check > vmapp_count for that. Indeed, the ITSList guards the sending of VMOVP in that case, and we avoid the original issue in that case. However, this still translates in the doorbell being moved for no reason (see its_vpe_db_proxy_move). How about something like this? Thanks, M. diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index ab597e74ba08..ac8ed56f1e48 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -3810,8 +3810,17 @@ static int its_vpe_set_affinity(struct irq_data *d, * Check if we're racing against a VPE being destroyed, for * which we don't want to allow a VMOVP. */ - if (!atomic_read(&vpe->vmapp_count)) - return -EINVAL; + if (!atomic_read(&vpe->vmapp_count)) { + if (gic_requires_eager_mapping()) + return -EINVAL; + + /* + * If we lazily map the VPEs, this isn't an error, and + * we exit cleanly. + */ + irq_data_update_effective_affinity(d, cpumask_of(cpu)); + return IRQ_SET_MASK_OK_DONE; + } /* * Changing affinity is mega expensive, so let's be as lazy as -- Without deviation from the norm, progress is not possible.