From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5924B241689 for ; Mon, 12 May 2025 15:55:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747065335; cv=none; b=tcxrIXMtpYXhlMjgYlKIVByLj9tw/tQEACWDWG6SXlgc574esU0i2c01LyNz4qbiHdUO1lgZtFiYQRmyASUgn8D4lXU4tOFpj9JEvf/J0Z2ExtgCAn86t06H1lhjE57VskjmWHcJy7dNicvTnEVsSvPaiEJKeQDvQV1mflBZX5I= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1747065335; c=relaxed/simple; bh=9oH64TA/cPDc+bTwTS/UrRp0io3FdvKt4QRGJTRhPtY=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=N16PrK7wG5jYHa13eZKLCJN9xiTyGQAPmpyXPN/5ogQzDxpl7i9UFaHF/NEdBxmLLQafF0UZef2MfEALnrM89WJBTCQbRVyZiW35dVRnKkZ5SlvMnsbRrehH6/z63peYtrgarMiaE9GH1arRuO5Ocs6YflCsmiWnHcmmy0F6UXU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=D/WYmqdu; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="D/WYmqdu" 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) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: 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 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.