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 15720C3ABC3 for ; Mon, 12 May 2025 16:42:35 +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=BBVk2oo5ady6BFVtBDvrg6Jt6Hsz+CdB2psG6bRhYqQ=; b=B6+iN1MlSIgaCvTdMCY+5BmQS3 k/u1YNqhpquj2iVU1NzfotRhBrZ7gIPdNjFnmawSDzz0kVnVr2lnbjRQvOqEmikphMSypToeeT8wF AYojJtI+BaaM9ZKhVItF/eN5Au+XctSCart9S5QLFaZv+v7NyXNPbCgMddSspcG8ZcMIzKBMxzR9S Ns9hA88jF70gqwRFfhdbt0ZToJ27nIYcuYSh5Z+yDXlFXv1y2C/0Mswpomt9ESBD1WE+IZ/8qR9EV ZDZmeWufLVEPIJnotRhKZ6QalbGIBByqwWs2odyRScNypb3MsszGx9jg0qZ/wGKFy9f4/48zgQXoy ebwiXGOw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEWEV-0000000A5SB-1Ama; Mon, 12 May 2025 16:42:27 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uEVjA-00000009ykm-1UJT for linux-arm-kernel@lists.infradead.org; Mon, 12 May 2025 16:10:05 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 35FFB5C4AA4; Mon, 12 May 2025 16:07:46 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2CDA2C4CEE7; Mon, 12 May 2025 16:10:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1747066203; bh=iL2V/bvrYGW9bbo4sD+uM9MvWXY78cNQRMuPYip5Pf4=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=o0uxskyoOjACJ12rcKa0ajOh1RM/FYNmekNIeB1lXBSkFcMTmF7izyYVHP8mgKsxE BenwT/plXzUpPoq4okelpfKHVfMunDBorTUMD0Ki4UcGhcyf4oPvRkuqrzkAQWdis8 4c1VxVVRAtY6KGuwigB0YMH/1EN74jR9mkTMCfn54HbHxBjDh6dgUaEMk5dD+M02+H ayuM7ICDbqxuViEjLFabEhoSy5bpqiwgrfMlfAcdBj5NurZr+/1R2cdgKtL3r6EN5R MltFcTky9GZOrPZ6/Zgj/dWhISEDp68P/uqFtns4+yIhEY1ZW4gRXTYKHduyz96to5 gR/zfCFzwG/OQ== 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 1uEVj6-00EBmE-W1; Mon, 12 May 2025 17:10:01 +0100 Date: Mon, 12 May 2025 17:09:59 +0100 Message-ID: <86sel9g694.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 2/4] irqchip/gic-v3-its: Implement .msi_teardown() callback In-Reply-To: <87ecwtlwx8.ffs@tglx> References: <20250511163520.1307654-1-maz@kernel.org> <20250511163520.1307654-3-maz@kernel.org> <87ecwtlwx8.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_091004_480442_E3F84671 X-CRM114-Status: GOOD ( 30.61 ) 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:34:59 +0100, Thomas Gleixner wrote: > > On Sun, May 11 2025 at 17:35, Marc Zyngier wrote: > > We currently nuke the structure representing an endpoint device > > How is we? We means nothing as you know :) We means a lot to *me*. > > > translating via an ITS on freeing the last LPI allocated for it. > > > > That's an unfortunate state of affair, as it is pretty common for > > a driver to allocate a single MSI, do something clever, teardown > > this MSI, and reallocate a whole bunch of them. The nvme driver > > does exactly that, amongst others. > > > > What happens in that case is that the core code is buggy enough > > to issue another .msi_prepare() call, even if it shouldn't. > > This luckily cancels the above behaviour and hides the problem. > > > > In order to fix the core code, let's start by implementing the new > > s/let's// > > I really have to understand why everyone is so fond of "let's". It only > makes limited sense when the patch is proposed, but in a change log it > does not make sense at all. > > > .msi_teardown() callback. Nothing calls it yet, so a side effect > > is that the its_dev structure will not be freed and that the DID > > will stay mapped. Not a big deal, and this will be solved in the > > following patch. > > Now I see why you added this incomprehensible condition into the core > code. Bah. > > Why don't you add this callback once you changed the prepare muck, which > introduces info::alloc_data? Changing prepare first breaks nvme and igbxe, amongst other things, for the reasons explained just above, so you need to implement teardown first, plug the MSI controller into it, and only then you can switch prepare. What I can do is: (1) introduce teardown without the call site in msi_remove_device_irq_domain() (2) add its implementation in the ITS driver (3) move the prepare crap (4) add the teardown call to msi_remove_device_irq_domain(), without the check on info->alloc_data With that order, everything will keep working, with a temporary leak of ITTs in the ITS driver between patches (2) and (4). Is that something you can live with? M. -- Without deviation from the norm, progress is not possible.