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 95BC9C3ABC3 for ; Mon, 12 May 2025 16:23:05 +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=td/cmAwJ5vdOc4Vpk7VrZsMxPNggJcX6NMwL29L+8Bc=; b=uqXq+2H+6hjqiJZSvttubyBN0o E7JCc4sU8/wtakkrtfJLURIB8WeN3PzSprOZILfrSYxZW9eYvE/yjwwv52jJO3xhM6G70/cCCbKsZ jxeNcxdg7CjYbe7gFrBaR1yns8h+ASXukvnZKt8fgpAv9hg8PHpEkIY49UHCZDm1c3H4boFPKot9l PEmn6Z0bBSRalO2Zivj+TsnWueIMZyVgsM1lJeZKOSI+7PxeW1wPHnMySyFy1GA9ZKv1R6XatKt5T dF3pRtGWgM8rQckg/zLEowEoV4ldpON1Jmht2NI8j56yFvhvaqAxNlXn87cmaRToi/cwB37b57L1Y ZQdeAnXQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEVvd-0000000A1P5-1WGq; Mon, 12 May 2025 16:22:57 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEVV9-00000009xA8-1OkP for linux-arm-kernel@lists.infradead.org; Mon, 12 May 2025 15:55:36 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id DEF9E4AA8C; Mon, 12 May 2025 15:55:34 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id B7DEAC4CEF1; Mon, 12 May 2025 15:55:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1747065334; bh=9oH64TA/cPDc+bTwTS/UrRp0io3FdvKt4QRGJTRhPtY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=D/WYmqdulX8dzExpOzgURZMbBxBmPx9D3guBxSirAv77HZmH0H5KoD4qrTaIL89XA 74Ntwz8YWVUEg5beI9p5zPUmfb/5nyLEIo9V10SJ1Bzaf58kpy46fZwwVZ/UGYWNAj UJJGcugWliGu7Wh0cfkx4yuXuZGerZk2M/+8PQCYmartSqxGOdPydM1SFqX5NCCD0u VGMJWe+WVcVk1LSRcbqh/cFoy37KYujwH2HofWu5+CqIcvuXTBXq9SxKswOU6FduYm etHb06T4uvTvNkqORaDtjKM6OvoulLHYKaQFpc687n2BWpvJOZUEi1e011yDpWKa9N C7vD0kxcnemwg== 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 1uEVV6-00EBVM-Gg; Mon, 12 May 2025 16:55:32 +0100 Date: Mon, 12 May 2025 16:55:31 +0100 Message-ID: <86v7q5g6x8.wl-maz@kernel.org> From: Marc Zyngier To: Thomas Gleixner Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Lorenzo Pieralisi , Sascha Bischoff , Timothy Hayes Subject: Re: [PATCH 3/4] genirq/msi: Move prepare() call to per-device allocation In-Reply-To: <87jz6llxeg.ffs@tglx> References: <20250511163520.1307654-1-maz@kernel.org> <20250511163520.1307654-4-maz@kernel.org> <87jz6llxeg.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/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: tglx@linutronix.de, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, lpieralisi@kernel.org, sascha.bischoff@arm.com, 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-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250512_085535_422426_15897A64 X-CRM114-Status: GOOD ( 36.21 ) 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, 12 May 2025 15:24:39 +0100, Thomas Gleixner wrote: > > On Sun, May 11 2025 at 17:35, Marc Zyngier wrote: > > The current device MSI infrastructure is subtly broken, as it > > will issue an .msi_prepare() callback into the MSI controller > > driver every time it needs to allocate an MSI. That's pretty wrong, > > as the contract between the MSI controller and the core code is that > > .msi_prepare() is called exactly once per device. > > That contract is nowhere written in stone. It was *definitely* there the first place, and a baked in assumption since the ITS code was merged. You're welcome to come up with a new scheme, but the way the HW works requires this prepare phase to take place once per device. If we can't have that, maybe we should consider reverting the GICv3/v4 code back to the pre-6.10 scheme that doesn't suffer from this issue. > There are some MSI controller which get confused about that, but that's > a problem of said controllers No. It's an infrastructure problem. This model worked before for a whole class of HW, until it was mutated into something else. > > > diff --git a/include/linux/msi.h b/include/linux/msi.h > > index 0a44a2cba3105..68a8b2d03eba9 100644 > > --- a/include/linux/msi.h > > +++ b/include/linux/msi.h > > @@ -513,12 +513,14 @@ struct msi_domain_info { > > * @chip: Interrupt chip for this domain > > * @ops: MSI domain ops > > * @info: MSI domain info data > > + * @arg: MSI domain allocation data (arch specific) > > arg is a horrible name. Can this please be alloc_info or such? Because that's the name every single function that takes it as a parameter uses? But sure, whatever name you want. > > > @@ -1025,6 +1026,7 @@ bool msi_create_device_irq_domain(struct device *dev, unsigned int domid, > > bundle->info.ops = &bundle->ops; > > bundle->info.data = domain_data; > > bundle->info.chip_data = chip_data; > > + bundle->info.alloc_data = &bundle->arg; > > > > pops = parent->msi_parent_ops; > > snprintf(bundle->name, sizeof(bundle->name), "%s%s-%s", > > @@ -1053,21 +1055,28 @@ bool msi_create_device_irq_domain(struct device *dev, unsigned int domid, > > msi_lock_descs(dev); > > Please work against tip irq/msi which carries the guard() replacement > for msi_lock_descs(). This patch heavily conflicts with the queued > changes. > > > +static int __populate_alloc_info(struct irq_domain *domain, struct device *dev, > > + unsigned int nirqs, msi_alloc_info_t *arg) > > +{ > > Why does this need double underscores? Because it doesn't look that out of place in this file? > > > + struct msi_domain_info *info = domain->host_data; > > + int ret = 0; > > + > > + /* > > + * If the caller has provided a template alloc info, use that. Once > > + * all users of msi_create_irq_domain() have been eliminated, this > > + * should be the only source of allocation information, and the > > + * prepare call below should be finally removed. > > That's only a matter of decades :) > > > + */ > > + if (info->alloc_data) > > + *arg = *info->alloc_data; > > + else > > + ret = msi_domain_prepare_irqs(domain, dev, nirqs, arg); > > + > > + return ret; > > if (!info->alloc_data) > return msi_domain_prepare_irqs(domain, dev, nirqs, arg); > > *arg = *info->alloc_data; > return 0; > > perhaps? Sure. M. -- Without deviation from the norm, progress is not possible.