From patchwork Thu May 3 22:31:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 10379393 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id D07EF60159 for ; Thu, 3 May 2018 22:31:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id AFA29283D9 for ; Thu, 3 May 2018 22:31:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id A3EF5292A7; Thu, 3 May 2018 22:31:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 0247F283D9 for ; Thu, 3 May 2018 22:31:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NC/5AQwivvSpZrF3pLHvuwExRIGMR50qc1ca09G/UmQ=; b=tbYxVGxvSIwO5F B/Q+yMZhrepn/TWdiYJz/YcOu+23UJizszlfUfAa0JQb3hiKnLz3uw5hF/CELlaHGjDOHSNPgfA0p 5wz5gQ0IYSQ61BlwrSmP2ASSA+m5qWMiGzlDx6fUe1b8OaZWAF+EJQcTbT7mIv1encDECp0RuK8Vr kPSFwkWJorfsu3aLhxx+nsfc+8IwLDhvBSFCf/CKiIRyZXBJIG9qxKk5eDTdF0Cx3tS0SJpc/sHip yh9Xf+kqH6ZDuS/qvg6dnh7+I9MKV0tDQje09Yqfa89j3xWnpJhLsmK075TTiLJAlzILOJ2+3wW3n HHJG5mkK6Z4KdL7Tg/lQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fEMlO-00069H-P7; Thu, 03 May 2018 22:31:46 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fEMlK-000680-C7 for linux-arm-kernel@lists.infradead.org; Thu, 03 May 2018 22:31:44 +0000 Received: from localhost (unknown [69.71.5.252]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1AA6A2176D; Thu, 3 May 2018 22:31:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1525386691; bh=NeYxpugymbat4F0E62PCHIOEhQgBCiwnOnKRlxS01t0=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LobaXX6Hf+lZw3qy219+Rh0PJZulmYBXdlM1lAy30Rdx4KBQnGxVmMDt6qV6ax/bA UX3dlcCqcQPOkAIex+OrW0pQRJnIkMZFk6YHmr8EM/ZwLPqsO3y1+6hvCdQ0G8e6+2 C+hre0lVUFLMXGKL42LYVlLaj77Q5BzQtjHnXMko= Date: Thu, 3 May 2018 17:31:27 -0500 From: Bjorn Helgaas To: Gilles Buloz Subject: [PATCH] PCI: Check whether bridges allow access to extended config space Message-ID: <20180503223127.GB15790@bhelgaas-glaptop.roam.corp.google.com> References: <20180427165627.GA8199@bhelgaas-glaptop.roam.corp.google.com> <5AE6D7E2.9030506@kontron.com> <5AE71BF4.2010200@kontron.com> <20180430170447.GA95643@bhelgaas-glaptop.roam.corp.google.com> <5AE75809.30701@kontron.com> <5AE9B5BB.2080003@kontron.com> <20180502132501.GE11698@bhelgaas-glaptop.roam.corp.google.com> <5AE9C1AB.8020403@kontron.com> <20180502172341.GA123831@bhelgaas-glaptop.roam.corp.google.com> <5AEB033A.4060407@kontron.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5AEB033A.4060407@kontron.com> User-Agent: Mutt/1.9.2 (2017-12-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180503_153142_454188_DF641AAD X-CRM114-Status: GOOD ( 26.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Ard Biesheuvel , linux-pci , linux-kernel@vger.kernel.org, "Minghuan.Lian@nxp.com" , Bjorn Helgaas , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP [+cc LKML] On Thu, May 03, 2018 at 12:40:27PM +0000, Gilles Buloz wrote: > Subject: [PATCH] For exception at PCI probe due to bridge reporting UR > > Even if a device supports extended config access, no such access must be > done to this device If there's a bridge not supporting that in the path > to this device. Doing such access with UR reporting enabled on the root > bridge leads to an exception. > > This is the case on a LS1043A CPU (NXP QorIQ Layerscape) platform with > the following bus topology : > LS1043 PCIe root > -> PEX8112 PCIe-to-PCI bridge (not supporting ext cfg on PCI side) > -> PMC slot connector (for legacy PMC modules) > With a PMC module topology as follows : > PMC connector > -> PCI-to-PCIe bridge > -> PCIe switch (4 ports) > -> 4 PCIe devices (one on each port) > In this case all devices behind the PEX8112 are supporting extended config > access but this is prohibited by the PEX8112. Without this patch, an > exception (synchronous abort) occurs in pci_cfg_space_size_ext(). > > This patch checks the parent bridge of each allocated child bus to know if > extended config access is supported on the child bus, and sets a flag in > child->bus_flags if not supported. This flag is inherited by all children > buses of this child bus and then is checked to avoid this unsupported > accesses to every device on these buses. Hi Gilles, Thanks for the patch! I reworked it a little bit to simplify the code in pci_alloc_child_bus(). Can you test it and make sure I didn't break anything? commit cbaaa85b558a8f974e301fa0364d568efaf491ce Author: Gilles Buloz Date: Thu May 3 15:21:44 2018 -0500 PCI: Check whether bridges allow access to extended config space Even if a device supports extended config space, i.e., it is a PCI-X Mode 2 or a PCI Express device, the extended space may not be accessible if there's a conventional PCI bus in the path to it. We currently figure that out in pci_cfg_space_size() by reading the first dword of extended config space. On most platforms that returns ~0 data if the space is inaccessible, but it may set error bits in PCI status registers, and on some platforms it causes exceptions that we currently don't recover from. For example, a PCIe-to-conventional PCI bridge treats config transactions with a non-zero Extended Register Address as an Unsupported Request on PCIe and a received Master-Abort on the destination bus (see PCI Express to PCI/PCI-X Bridge spec, r1.0, sec 4.1.3). A sample case is a LS1043A CPU (NXP QorIQ Layerscape) platform with the following bus topology: LS1043 PCIe Root Port -> PEX8112 PCIe-to-PCI bridge (doesn't support ext cfg on PCI side) -> PMC slot connector (for legacy PMC modules) With a PMC module topology as follows: PMC connector -> PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe devices (one on each port) The PCIe devices on the PMC module support extended config space, but we can't reach it because the PEX8112 can't generate accesses to the extended space on its secondary bus. Attempts to access it cause Unsupported Request errors, which result in synchronous aborts on this platform. To avoid these errors, check whether bridges are capable of generating extended config space addresses on their secondary interfaces. If they can't, we restrict devices below the bridge to only the 256-byte PCI-compatible config space. Signed-off-by: Gilles Buloz [bhelgaas: changelog, rework patch so bus_flags testing is all in pci_bridge_child_ext_cfg_accessible()] Signed-off-by: Bjorn Helgaas diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index ac91b6fd0bcd..7b1b7b2e01e4 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -882,6 +882,45 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge) return err; } +static bool pci_bridge_child_ext_cfg_accessible(struct pci_dev *bridge) +{ + int pos; + u32 status; + + /* + * If extended config space isn't accessible on a bridge's primary + * bus, we certainly can't access it on the secondary bus. + */ + if (bridge->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) + return false; + + /* + * PCIe Root Ports and switch ports are PCIe on both sides, so if + * extended config space is accessible on the primary, it's also + * accessible on the secondary. + */ + if (pci_is_pcie(bridge) && + (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT || + pci_pcie_type(bridge) == PCI_EXP_TYPE_UPSTREAM || + pci_pcie_type(bridge) == PCI_EXP_TYPE_DOWNSTREAM)) + return true; + + /* + * For the other bridge types: + * - PCI-to-PCI bridges + * - PCIe-to-PCI/PCI-X forward bridges + * - PCI/PCI-X-to-PCIe reverse bridges + * extended config space on the secondary side is only accessible + * if the bridge supports PCI-X Mode 2. + */ + pos = pci_find_capability(bridge, PCI_CAP_ID_PCIX); + if (!pos) + return false; + + pci_read_config_dword(bridge, pos + PCI_X_STATUS, &status); + return status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ); +} + static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, struct pci_dev *bridge, int busnr) { @@ -923,6 +962,16 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent, pci_set_bus_of_node(child); pci_set_bus_speed(child); + /* + * Check whether extended config space is accessible on the child + * bus. Note that we currently assume it is always accessible on + * the root bus. + */ + if (!pci_bridge_child_ext_cfg_accessible(bridge)) { + child->bus_flags |= PCI_BUS_FLAGS_NO_EXTCFG; + pci_info(child, "extended config space not accessible on secondary bus\n"); + } + /* Set up default resource pointers and names */ for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) { child->resource[i] = &bridge->resource[PCI_BRIDGE_RESOURCES+i]; @@ -1393,6 +1442,9 @@ int pci_cfg_space_size(struct pci_dev *dev) u32 status; u16 class; + if (dev->bus->bus_flags & PCI_BUS_FLAGS_NO_EXTCFG) + return PCI_CFG_SPACE_SIZE; + class = dev->class >> 8; if (class == PCI_CLASS_BRIDGE_HOST) return pci_cfg_space_size_ext(dev); diff --git a/include/linux/pci.h b/include/linux/pci.h index 230615620a4a..f7aa6d9f8999 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -217,6 +217,7 @@ enum pci_bus_flags { PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2, PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4, + PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8, }; /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */