From patchwork Fri Dec 4 08:20:40 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Fedin X-Patchwork-Id: 7765941 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E83079F30B for ; Fri, 4 Dec 2015 08:23:34 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 0A94F20452 for ; Fri, 4 Dec 2015 08:23:34 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1543C2044B for ; Fri, 4 Dec 2015 08:23:33 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1a4lc6-0003bA-Nf; Fri, 04 Dec 2015 08:21:10 +0000 Received: from mailout1.w1.samsung.com ([210.118.77.11]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1a4lc2-0003Zt-SX for linux-arm-kernel@lists.infradead.org; Fri, 04 Dec 2015 08:21:08 +0000 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout1.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NYT00DQQR6HYO50@mailout1.w1.samsung.com> for linux-arm-kernel@lists.infradead.org; Fri, 04 Dec 2015 08:20:41 +0000 (GMT) X-AuditID: cbfec7f4-f79026d00000418a-8d-56614cd9d728 Received: from eusync3.samsung.com ( [203.254.199.213]) by eucpsbgm1.samsung.com (EUCPMTA) with SMTP id 10.53.16778.9DC41665; Fri, 4 Dec 2015 08:20:41 +0000 (GMT) Received: from fedinw7x64 ([106.109.131.169]) by eusync3.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0NYT00DQ1R6GZN40@eusync3.samsung.com>; Fri, 04 Dec 2015 08:20:41 +0000 (GMT) From: Pavel Fedin To: 'Tyler Baker' References: <1448624071-9552-1-git-send-email-p.fedin@samsung.com> In-reply-to: Subject: RE: [PATCH v2] ARM: Enable GICv2m on 32-bit virt machine Date: Fri, 04 Dec 2015 11:20:40 +0300 Message-id: <00a101d12e6c$aa073600$fe15a200$@samsung.com> MIME-version: 1.0 X-Mailer: Microsoft Outlook 14.0 Thread-index: AQLcu/1x91vtDRJS19KDBA0hj/mA7ACU8MaanJ6pOlA= Content-language: ru X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrCLMWRmVeSWpSXmKPExsVy+t/xq7o3fRLDDC5NU7V4uvkxk8Wmx9dY LW5f5rX4e+cfm8XHHfdYHFg91sxbw+jR0tzD5rFpVSebx51re9g8Ni+pD2CN4rJJSc3JLEst 0rdL4MqYv8C34K5KxfKrQg2MG2S7GDk5JARMJL5cf84GYYtJXLi3Hsjm4hASWMooMb1jHjNI QkjgO6PEtAmWIDabgLrE6a8fWEBsEQEtiWfL2lhBGpgF9jBK7F7ezgTR3ckoce3Xf7CxnALB EhP/72EFsYUFHCW2th9nBLFZBFQlfhzawQRi8wpYShy9dJANwhaU+DH5HtgGZqAN63ceZ4Kw 5SU2r3nLDHGqgsSOs68ZIa6wkjiz4CMbRI2IxLR/95gnMArNQjJqFpJRs5CMmoWkZQEjyypG 0dTS5ILipPRcQ73ixNzi0rx0veT83E2MkLj4soNx8TGrQ4wCHIxKPLwMmxLChFgTy4orcw8x SnAwK4nwMsskhgnxpiRWVqUW5ccXleakFh9ilOZgURLnnbvrfYiQQHpiSWp2ampBahFMlomD U6qBcf6+jFMrK7a9XBY+id+Z5yuP7dNl8zQ+H7l9sJLt2cJ7Hyy7o0JEtjP9DrznmXY9ft3W 9piW9dKtk1Z+WpP9JTliU/s2/qVFz5avXMOywSJQJ8jLqUD41NzqpbacK/P8eMN4s3mm7xNX 25vwvLRy8octMjPvz5Dxm2vfI8QSa3X1oFVjyTS/WiWW4oxEQy3mouJEACNT8CiHAgAA X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151204_002107_107000_248E419A X-CRM114-Status: GOOD ( 18.77 ) X-Spam-Score: -6.9 (------) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: 'Marc Zyngier' , 'Russell King' , 'linux-arm-kernel' , 'Kevin's boot bot' Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hello! > today. Specifically, it is causing boot regressions with > multi_v7_defconfig variants on the tegra124-jetson-tk1, > tegra30-beaver, and the armada-370-mirabox platforms, all of which > have PCI support. I had the bot bisect[2] the boot failures, and it > pointed to this commit. Ouch... What a simple thing and how many problems... I have examined the code and i know the source of the problem. CONFIG_ARM_GIC_V2M automatically enables CONFIG_PCI_MSI_IRQ_DOMAIN, and there we have pci_msi_setup_msi_irqs(), which first tries to call domain ops, then, if there's no irqdomain for the device, then try oldstyle arch_setup_msi_irqs(). The problem is that with new GENERIC_MSI_IRQ_DOMAIN we always have domain != NULL, and backwards compatibility logic doesn't work anymore. So far, we have the following call chain: pci_msi_setup_msi_irqs -> pci_msi_domain_alloc_irqs -> msi_domain_alloc_irqs. And here fun begins: struct msi_domain_info *info = domain->host_data; struct msi_domain_ops *ops = info->ops; ... ret = ops->msi_check(domain, info, dev); I intentionally left only these three lines because they are enough for the crash. They assume that domain->host_data holds struct msi_domain_info *, which is plain wrong with that legacy code. 1. Tegra pci-tegra.c does this: msi->domain = irq_domain_add_linear(pcie->dev->of_node, INT_PCI_MSI_NR, &msi_domain_ops, &msi->chip); where msi->chip is oldstyle struct msi_controller. And __irq_domain_add() simply inserts this pointer into domain->host_data. This causes msi_domain_alloc_irqs() to go nowhere by dereferencing domain->host_data with a wrong type. 2. Armada. I failed to find where exactly MSI irqdomain is created, this should be done somewhere by default, because mvebu PCI host driver (is it the right one?) does not use any irqdomain operations at all. So, i suppose we have some empty domain with domain->host_data set to NULL, therefore msi_domain_alloc_irqs() goes to NULL dereference early in the beginning. Proposed solution ----------------- I have studied the code a bit more, and i see that proper MSI domains should have domain->ops == &msi_domain_ops. Based on this, i can suggest the following fix (copypasted from console, so tabs lost, don't pay attention please): $ git diff Tyler, can you apply this and see what happens? Unfortunately i don't have any of these machines here, so cannot test by myself. And qemu also cannot emulate them either. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 53e4632..8531f89 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -43,10 +43,10 @@ static struct irq_domain *pci_msi_get_domain(struct pci_dev *dev) struct irq_domain *domain; domain = dev_get_msi_domain(&dev->dev); - if (domain) - return domain; + if (!domain) + domain = arch_get_pci_msi_domain(dev); - return arch_get_pci_msi_domain(dev); + return irq_domain_is_generic_msi(domain) ? domain : NULL; } static int pci_msi_setup_msi_irqs(struct pci_dev *dev, int nvec, int type) diff --git a/include/linux/msi.h b/include/linux/msi.h index f71a25e..470d285 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -268,6 +268,7 @@ int msi_domain_set_affinity(struct irq_data *data, const struct cpumask *mask, struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, struct msi_domain_info *info, struct irq_domain *parent); +bool irq_domain_is_generic_msi(struct irqdomain *domain); int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec); void msi_domain_free_irqs(struct irq_domain *domain, struct device *dev); diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c index 6b0c0b7..22dbc7f 100644 --- a/kernel/irq/msi.c +++ b/kernel/irq/msi.c @@ -253,6 +253,17 @@ struct irq_domain *msi_create_irq_domain(struct fwnode_handle *fwnode, } /** + * irq_domain_is_generic_msi - Check whether the irqdomain belongs to us + * @domain: The domain to check + * + * Returns: test result (true or false) + */ +bool irq_domain_is_generic_msi(struct irqdomain *domain) +{ + return domain && (domain->ops == &msi_domain_ops); +} + +/** * msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain * @domain: The domain to allocate from * @dev: Pointer to device struct of the device for which the interrupts