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 6AA97C2BD09 for ; Fri, 12 Jul 2024 20:41:46 +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:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID:Subject:Cc:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:References:List-Owner; bh=/kwvllVwKcshng2U6cheF0a928arvUqme8xLGAJbpTg=; b=q+c1JfWdMvbwvvcLVJP4xRP3n2 O7JTwMlujb6D9eCBsRfg1ZS+fRKgXw/n2YM1NsbvFesCoysyiIISZMJ4McYz1OmTvZCF/ui0AUA2Q 87lH/Z2fN/JbA+dc0glcPeSLapKaMayH88lOjtn5DJnypaC5CeLGxyVKM6I46RYPBW+o0KqD1Is+N vgc/vqY8wOWE/7bPN2yGSMoj4N2pUqSv74OD1irpwwGtgjlL5Ffkv3A977vgwAL8wVvkYRvUGAQpq ClL/XYJzidsSW8VIZdJ754NZ8yWTOAZY5y0B9rVnvldi572iW1dUc7B5sYu1St4t3PiQXBirVWiU4 1S3CdlOA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sSN5A-00000001D42-2wTL; Fri, 12 Jul 2024 20:41:32 +0000 Received: from sin.source.kernel.org ([2604:1380:40e1:4800::1]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sSN4r-00000001D1F-2CBr for linux-arm-kernel@lists.infradead.org; Fri, 12 Jul 2024 20:41:15 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 21C9DCE0AB9; Fri, 12 Jul 2024 20:41:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 33009C32782; Fri, 12 Jul 2024 20:41:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1720816869; bh=dTCn1JYLyphG0HhWrMCWZ3fJpATH0SVWmU5AKhmUuB4=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=sJslttH73a0cpLDLwR87f0mObTZUfDNuCKXbAOGIY6SF6g/OkijCAmpQfJcov7YR3 9ctuwnfbtDYGuPVUtwBtCJTq0nHS5ThC0Ov0u9hT7yb9GrXWPHqQtsGNr2i4vS86OR jIK9HXN6CxvCSVwDheGTjzRjGqWuScQ3FQ2NzRuvdKqnwfIKuBgcVs/zzzYGNMAiXu Rmy6djQQaNlP0DlSat5r3z7DcfR6tvazV/KvEMaTgvWhknzT6XLBX4r5lscKz/i9KZ s8dpQR0Oe1T/0R6IGgTm/ln4wOInXCVlb2YLR35zIbwuZCTdf2vj9Scig/9FIQLyGi EpuZRX6W9zqXg== Date: Fri, 12 Jul 2024 15:41:06 -0500 From: Bjorn Helgaas To: Marek =?utf-8?B?QmVow7pu?= Cc: Lorenzo Pieralisi , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Bjorn Helgaas , Andrew Lunn , Gregory Clement , Thomas Petazzoni , Rob Herring , linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Manivannan Sadhasivam , Pali =?utf-8?B?Um9ow6Fy?= , Thomas Gleixner , Marc Zyngier Subject: Re: [PATCH v2] PCI: mvebu: Dispose INTx IRQs before to removing INTx domain Message-ID: <20240712204106.GA336836@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20240711132544.9048-1-kabel@kernel.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240712_134113_928653_CE643CC0 X-CRM114-Status: GOOD ( 28.82 ) 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 [+cc Pali, seems like the author should be included, Thomas, Marc since they actually know about IRQs, unlike me] On Thu, Jul 11, 2024 at 03:25:44PM +0200, Marek Behún wrote: > From: Pali Rohár > > The documentation for the irq_domain_remove() function says that all > mappings within the IRQ domain must be disposed before the domain is > removed. > > Currently, the INTx IRQs are not disposed in pci-mvebu driver .remove() > method, which causes the kernel to crash when unloading the driver and > then reading /sys/kernel/debug/irq/irqs/ or /proc/interrupts. > > Unmapping of the IRQs at this point of the .remove() method is safe, > since the PCIe bus is already unregistered, and all its devices are > unbound from their drivers and removed. If there was indeed any > remaining use of PCIe resources, then it would mean that PCIe hotplug > code is broken, and we have bigger problems. > > Fixes: ec075262648f ("PCI: mvebu: Implement support for legacy INTx interrupts") > Reported-by: Hajo Noerenberg Is there a URL for this report? > Signed-off-by: Pali Rohár > Reviewed-by: Marek Behún > [ Marek: refactored a little, added more explanation to commit message ] > Signed-off-by: Marek Behún > Reviewed-by: Manivannan Sadhasivam > --- > Changes since v1: > - added explanation into commit message about why this is safe to do, > as suggested by Andy. The explanation originally comes from Pali: > https://lore.kernel.org/linux-arm-kernel/20220809133911.hqi7eyskcq2sojia@pali/ > --- > drivers/pci/controller/pci-mvebu.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c > index 29fe09c99e7d..91a02b23aeb1 100644 > --- a/drivers/pci/controller/pci-mvebu.c > +++ b/drivers/pci/controller/pci-mvebu.c > @@ -1683,8 +1683,15 @@ static void mvebu_pcie_remove(struct platform_device *pdev) > irq_set_chained_handler_and_data(irq, NULL, NULL); > > /* Remove IRQ domains. */ > - if (port->intx_irq_domain) > + if (port->intx_irq_domain) { > + for (int j = 0; j < PCI_NUM_INTX; j++) { > + int virq = irq_find_mapping(port->intx_irq_domain, j); > + > + if (virq > 0) > + irq_dispose_mapping(virq); I am not an IRQ expert, so all I can really do is compare this to usage in other drivers. There are 20+ drivers in drivers/pci/controller, and I don't see irq_dispose_mapping() usage similar to this elsewhere. Does that mean most or all of the other drivers have a similar defect? > + } > irq_domain_remove(port->intx_irq_domain); > + } > > /* Free config space for emulated root bridge. */ > pci_bridge_emul_cleanup(&port->bridge); > -- > 2.44.2 >