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 47D79C3DA4A for ; Mon, 29 Jul 2024 07:25:44 +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=ASkMX4huctK1O2jUrEProOBFaOM4y7G/N/p4ME2KhFA=; b=zNdx6drX/+nS025e7uKKal+B+2 iafuFph+CYKuHeIENpWaahFFyjyLdrZ5R8c0aCyZUN9+0IVSJ/n8iC7l0hZG2uv7DHsg9qqTkRc1H A/Zgrl16UzbH8rcOpSsSkRetcIBkys3stoITG0Bz2iso1kIzLvW/sgyoYRgtqM3SPtQDCU5ebQXp0 j7tMTcY1DJBZ1GQvRzk7wDhZBJmErO/pQ90A5XAlCuCqva/sub1H3UmDpjEvXD+NcM+/HgQsWf3o9 6KUREIciUmpnXAH5JMYbLFQ+dNqStzsq+cW6ddEyUoiKqBd7cu5onaXAELB6JC0bbe+XsZmAFPpXb uNbPMtbA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYKlD-0000000AKpu-0YvM; Mon, 29 Jul 2024 07:25:35 +0000 Received: from dfw.source.kernel.org ([2604:1380:4641:c500::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYKkn-0000000AKld-21H6 for linux-arm-kernel@lists.infradead.org; Mon, 29 Jul 2024 07:25:10 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 8A79B61460; Mon, 29 Jul 2024 07:25:08 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3EB54C32786; Mon, 29 Jul 2024 07:25:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1722237908; bh=PxY9OfyCxQQ8fcubazRFKXRnv9iR3nUeGTLtBKNYxsM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ku9FskPdEYt1ftVEY0gpdB3QZNTtf7WllULvSqLsL0Ox4MWN21lgzsRS3la1B4/QK z2KGsGL0KfTJeHY/v5G5FE0ubcc00cYwEyTBHrKRN2qvw83G5jBRcoEX4Q4hTmNJg0 3OrUI+M/I4u+4AP87108kNp8DqzyQ4KvI5/tH6E/zqbqVJLomiGHKlGz/w7tIKDOWY Q2XsbHOzBDTmMnd3ixZOFh4umZsDw6c9+nBUQx70dH7Z1KUMDnKCTtBB84AWU6zUjw FtvckMz3pDGyOWtrMi4gvTl3qhOBZEj/N27ql8u4VFcgAfw6x0DYgmHb5Mof06ye+Q zkqKM91vsti9Q== 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.95) (envelope-from ) id 1sYKkj-00GDrF-Ry; Mon, 29 Jul 2024 08:25:06 +0100 Date: Mon, 29 Jul 2024 08:25:04 +0100 Message-ID: <86y15k1wz3.wl-maz@kernel.org> From: Marc Zyngier To: Thomas Gleixner Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Zhou Wang Subject: Re: [PATCH] irqchip/gic-v4: Fix ordering between vmapp and vpe locks In-Reply-To: <875xsrvpt3.ffs@tglx> References: <20240723175203.3193882-1-maz@kernel.org> <875xsrvpt3.ffs@tglx> 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.3 (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: tglx@linutronix.de, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, wangzhou1@hisilicon.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-20240729_002509_646609_68BEBA27 X-CRM114-Status: GOOD ( 25.23 ) 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, 26 Jul 2024 21:52:40 +0100, Thomas Gleixner wrote: > > On Tue, Jul 23 2024 at 18:52, Marc Zyngier wrote: > > @@ -3808,7 +3802,7 @@ static int its_vpe_set_affinity(struct irq_data *d, > > struct its_vpe *vpe = irq_data_get_irq_chip_data(d); > > unsigned int from, cpu = nr_cpu_ids; > > struct cpumask *table_mask; > > - unsigned long flags; > > + unsigned long flags, vmapp_flags; > > What's this flags business for? its_vpe_set_affinity() is called with > interrupts disabled, no? > > > /* > > * Changing affinity is mega expensive, so let's be as lazy as > > @@ -3822,7 +3816,14 @@ static int its_vpe_set_affinity(struct irq_data *d, > > * protect us, and that we must ensure nobody samples vpe->col_idx > > * during the update, hence the lock below which must also be > > * taken on any vLPI handling path that evaluates vpe->col_idx. > > + * > > + * Finally, we must protect ourselves against concurrent > > + * updates of the mapping state on this VM should the ITS list > > + * be in use. > > */ > > + if (its_list_map) > > + raw_spin_lock_irqsave(&vpe->its_vm->vmapp_lock, vmapp_flags); > > Confused. This changes the locking from unconditional to > conditional. What's the rationale here? Haven't managed to sleep much, but came to the conclusion that I wasn't that stupid in my initial patch. Let's look at the full picture, starting with its_send_vmovp(): if (!its_list_map) { its = list_first_entry(&its_nodes, struct its_node, entry); desc.its_vmovp_cmd.col = &its->collections[col_id]; its_send_single_vcommand(its, its_build_vmovp_cmd, &desc); return; } /* * Protect against concurrent updates of the mapping state on * individual VMs. */ guard(raw_spinlock_irqsave)(&vpe->its_vm->vmapp_lock); The vmapp locking *is* conditional. Which makes a lot of sense as the presence of ITS list is the only thing that prevents the VPEs from being mapped eagerly at VM startup time (although this is a performance reason, and not a correctness issue). So there is no point in taking that lock if there is no ITS list, given that the VPEs are mapped before we can do anything else. This has the massive benefit of allowing concurrent VPE affinity changes on modern HW. This means that on GICv4.0 without ITSList or GICv4.1, the only lock we need to acquire is the VPE lock itself on VPE affinity change. I'll respin the patch shortly. Thanks, M. -- Without deviation from the norm, progress is not possible.