From patchwork Wed Oct 7 20:05:49 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 7347141 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@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 F195D9F302 for ; Wed, 7 Oct 2015 20:06:08 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E9047203C0 for ; Wed, 7 Oct 2015 20:06:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0E53320295 for ; Wed, 7 Oct 2015 20:06:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752654AbbJGUF7 (ORCPT ); Wed, 7 Oct 2015 16:05:59 -0400 Received: from mout.kundenserver.de ([212.227.126.131]:52039 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752414AbbJGUF7 (ORCPT ); Wed, 7 Oct 2015 16:05:59 -0400 Received: from wuerfel.localnet ([149.172.15.242]) by mrelayeu.kundenserver.de (mreue004) with ESMTPSA (Nemesis) id 0MWvbc-1a4hVY30ZI-00VxpC; Wed, 07 Oct 2015 22:05:56 +0200 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Krzysztof =?utf-8?B?SGHFgmFzYQ==?= , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Bjorn Helgaas Subject: [PATCH v2] ARM: cns3xxx: pci: avoid potential stack overflow Date: Wed, 07 Oct 2015 22:05:49 +0200 Message-ID: <12713701.B4epxIQJNF@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <201510072201.51697.arnd@arndb.de> References: <2273145.0Ls8cProhs@wuerfel> <201510072201.51697.arnd@arndb.de> MIME-Version: 1.0 X-Provags-ID: V03:K0:PHCJuf/WqUkEGrbcckQanvIrFPq9Wd5Krkd60Dl57R4W5ZV4YwV rQJv4MKjudf124lPFZEoiNhHqILnmq1UVQTsclZueW3lt3RqgINMBJu7xM6ImyE5P2RvUAY s9grhikBCxdCmaL7ckMFWi5q8tosPiuBWJmqgqAWUpQrdIXRG26ItmC7Riq4a1ktWtg5LHG KE1HgBZBe448wb2WsIvCQ== X-UI-Out-Filterresults: notjunk:1; V01:K0:elJCFYHyKJg=:JfHAEGM4AgErfApJpsdzQx wZPrHHSOmDf+QnEN5iImDcpQg6qlwXC4l9aW3HduMb9vdNnuoW4AC9lU5bqJ0boaelDgctF54 /mHemnLUN6/jwNtzRApvg5JkSOYGWq8BIJUYwF7IY8c+nmhBkfHV0yfex2+upLpWxcWMQWvhS +a4Cer8FJKE2ZD2VsHZdfFBtsC93b4OgGW2M86cml7KnCn1MszAHQYqM1tDIwFWDDWaKNw0B7 2lvaaHkBLHQ58j8aNvecAaOFi3RRwdJWUXL5Ms1AGvsJYs6IO2JRyO4ARbJm4cbH8nwCqx2eU OuzujKgCPeDqZVJB0MEzr9kEmrXc4BGYOZau6LMiHsxvjXyUSqwibHV5qLAd0EKgiIubCz9i8 TXznoxffn25aQHfFap4sIXmMFyZ3S5/ZWGe9us3E7k7bPYgWG6zKl0fN7qSZ/sc+p4EOTmgn+ W5uDrw3CtUNhKB3bOa9RQH97QiQ/kgmrCqOQZEdT3LW2XrIwRqVqktvl5y5zZj4JXfzH6viEK V6dRZ/d79RzU2gOkvspU1d0W0Yf525mMir5/obIpZGO/vExKsPtVhU4gOgDlQx9AuDmGWnYVo xWD70RTpMh++1fvS2wK2z00f81pQF2YkhQq5GD6rqLdoUeTNByYlkd1hVMrWWIMuI2f2ir48h U0K3CnpLhfCcR5fCW0wKTE2mmBKZZsOEllJN8xTma33wvG0qMFs6cAt1GMYOYBFpgn8p/QYIL 52wyS8rzse6pBfrQ Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 The cns3xxx_pcie_hw_init function uses excessive kernel stack space because of a hack that puts a fake struct pci_sys_data and struct pci_bus on the stack in order to call the generic pci_bus_read_config accessors, which causes a warning in ARM allmodconfig builds: arch/arm/mach-cns3xxx/pcie.c:266:1: warning: the frame size of 1080 bytes is larger than 1024 bytes I've spent a few hours trying to find out what exactly this code is wants to achieve here. The obvious part is setting up the host_regs using config space accessors, and this can simply be changed to use direct MMIO accesses, as I do in this patch. The second part is how the driver sets up the Max_Read_Request_Size value for the first device/function on bus 1, i.e. the device plugged directly into the PCIe root port. For all I can tell, this is in fact incomplete, as it does not perform the same setting on devices attached to a PCIe switch, or multi-function devices. The solution for this part fortunately is even easier: if we just set the global pcie_bus_config variable to PCIE_BUS_PEER2PEER, all PCIe devices in the system are limited to 128 byte MPS, which in turn limits the MRRS to 128 bytes for all devices, and we no longer even need to touch any devices. With those two changes in place, we no longer need the fake pci_sys_data/pci_bus structures for faking config space writes, and the stack usage goes down as well. Signed-off-by: Arnd Bergmann Acked-by: Krzysztof Ha?asa --- New approach based on Krzysztof's feedback about the previous version, should be cleaner now, but still needs critical review arch/arm/mach-cns3xxx/pcie.c | 71 +++++++++++++++++++++++++++++---------------------------------------- 1 file changed, 30 insertions(+), 41 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c index c622c306c390..47905a50e075 100644 --- a/arch/arm/mach-cns3xxx/pcie.c +++ b/arch/arm/mach-cns3xxx/pcie.c @@ -65,8 +65,9 @@ static void __iomem *cns3xxx_pci_map_bus(struct pci_bus *bus, /* * The CNS PCI bridge doesn't fit into the PCI hierarchy, though - * we still want to access it. For this to work, we must place - * the first device on the same bus as the CNS PCI bridge. + * we still want to access it. + * We place the host bridge on bus 0, and the directly connected + * device on bus 1, slot 0. */ if (busno == 0) { /* internal PCIe bus, host bridge device */ if (devfn == 0) /* device# and function# are ignored by hw */ @@ -211,58 +212,46 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci) } } +static void cns3xxx_write_config(struct cns3xxx_pcie *cnspci, + int where, int size, u32 val) +{ + void __iomem *base = cnspci->host_regs + (where & 0xffc); + u32 v; + u32 mask = (0x1ull << (size * 8)) - 1; + int shift = (where % 4) * 8; + + v = readl_relaxed(base + (where & 0xffc)); + + v &= ~(mask << shift); + v |= (val & mask) << shift; + + writel_relaxed(v, base + (where & 0xffc)); + readl_relaxed(base + (where & 0xffc)); +} + static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci) { - int port = cnspci->port; - struct pci_sys_data sd = { - .private_data = cnspci, - }; - struct pci_bus bus = { - .number = 0, - .ops = &cns3xxx_pcie_ops, - .sysdata = &sd, - }; u16 mem_base = cnspci->res_mem.start >> 16; u16 mem_limit = cnspci->res_mem.end >> 16; u16 io_base = cnspci->res_io.start >> 16; u16 io_limit = cnspci->res_io.end >> 16; - u32 devfn = 0; - u8 tmp8; - u16 pos; - u16 dc; - - pci_bus_write_config_byte(&bus, devfn, PCI_PRIMARY_BUS, 0); - pci_bus_write_config_byte(&bus, devfn, PCI_SECONDARY_BUS, 1); - pci_bus_write_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, 1); - pci_bus_read_config_byte(&bus, devfn, PCI_PRIMARY_BUS, &tmp8); - pci_bus_read_config_byte(&bus, devfn, PCI_SECONDARY_BUS, &tmp8); - pci_bus_read_config_byte(&bus, devfn, PCI_SUBORDINATE_BUS, &tmp8); - - pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_BASE, mem_base); - pci_bus_write_config_word(&bus, devfn, PCI_MEMORY_LIMIT, mem_limit); - pci_bus_write_config_word(&bus, devfn, PCI_IO_BASE_UPPER16, io_base); - pci_bus_write_config_word(&bus, devfn, PCI_IO_LIMIT_UPPER16, io_limit); + cns3xxx_write_config(cnspci, PCI_PRIMARY_BUS, 1, 0); + cns3xxx_write_config(cnspci, PCI_SECONDARY_BUS, 1, 1); + cns3xxx_write_config(cnspci, PCI_SUBORDINATE_BUS, 1, 1); + cns3xxx_write_config(cnspci, PCI_MEMORY_BASE, 2, mem_base); + cns3xxx_write_config(cnspci, PCI_MEMORY_LIMIT, 2, mem_limit); + cns3xxx_write_config(cnspci, PCI_IO_BASE_UPPER16, 2, io_base); + cns3xxx_write_config(cnspci, PCI_IO_LIMIT_UPPER16, 2, io_limit); if (!cnspci->linked) return; /* Set Device Max_Read_Request_Size to 128 byte */ - bus.number = 1; /* directly connected PCIe device */ - devfn = PCI_DEVFN(0, 0); - pos = pci_bus_find_capability(&bus, devfn, PCI_CAP_ID_EXP); - pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc); - if (dc & PCI_EXP_DEVCTL_READRQ) { - dc &= ~PCI_EXP_DEVCTL_READRQ; - pci_bus_write_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, dc); - pci_bus_read_config_word(&bus, devfn, pos + PCI_EXP_DEVCTL, &dc); - if (dc & PCI_EXP_DEVCTL_READRQ) - pr_warn("PCIe: Unable to set device Max_Read_Request_Size\n"); - else - pr_info("PCIe: Max_Read_Request_Size set to 128 bytes\n"); - } + pcie_bus_config = PCIE_BUS_PEER2PEER; + /* Disable PCIe0 Interrupt Mask INTA to INTD */ - __raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(port)); + __raw_writel(~0x3FFF, MISC_PCIE_INT_MASK(cnspci->port)); } static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,