From patchwork Fri Apr 27 16:56:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bjorn Helgaas X-Patchwork-Id: 10369415 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 E05FB601D3 for ; Fri, 27 Apr 2018 16:57:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id CF18427F8C for ; Fri, 27 Apr 2018 16:57:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C397B2841D; Fri, 27 Apr 2018 16:57:40 +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=ham 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 0EAB428469 for ; Fri, 27 Apr 2018 16:57:39 +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=aLDyCcKJNWlSPkli86kZCr/qxjN3cWbCONdp0iSeVUc=; b=nEYiOPHCUGV1WN lYFdpjjqZ5wUOspk0/2Tce+/AT+Z9BdHc78W5AR9aDqv0TQyuvtaRmWCDwgzSbMnWdkYCQvAsRuRH LiCae3NXI/voZ0E0jFy0tyGeNWzrwBrjcs3MyCPWszMrdLhXUlX2fdTfQlXzQHBfw6bgkHZ5ZZNyF 5yvuDlckFP2l8fjnSgAXWuFtwELYTufxS+umjJKocB5BNAMDqAPgXWexAr9+I0NlAFiImcbzwyn/U PaopvoP+fR1QLBJXg9ENiPzHNFiEjdW47rlfYGEFPOfr5tOeX4909jrUf2ZTBTEUc4AYUvufhEGBW PJPiltOwMa8nura02Eig==; 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 1fC6gd-0005J6-9b; Fri, 27 Apr 2018 16:57:31 +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 1fC6fn-0004zt-To for linux-arm-kernel@lists.infradead.org; Fri, 27 Apr 2018 16:56:49 +0000 Received: from localhost (unknown [150.199.191.185]) (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 8C4D4218BC; Fri, 27 Apr 2018 16:56:28 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8C4D4218BC Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=helgaas@kernel.org Date: Fri, 27 Apr 2018 11:56:27 -0500 From: Bjorn Helgaas To: Gilles Buloz Subject: Re: LS1043A : "synchronous abort" at boot due to PCI config read Message-ID: <20180427165627.GA8199@bhelgaas-glaptop.roam.corp.google.com> References: <5AD0E995.3090802@kontron.com> <5AE317AB.4020404@kontron.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5AE317AB.4020404@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-20180427_095640_016257_F2647945 X-CRM114-Status: GOOD ( 41.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: Bjorn Helgaas , linux-pci , "minghuan.Lian@freescale.com" , "linux-arm-kernel@lists.infradead.org" , Ard Biesheuvel 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 On Fri, Apr 27, 2018 at 12:29:32PM +0000, Gilles Buloz wrote: > Le 27/04/2018 10:43, Ard Biesheuvel a écrit : > > (add Bjorn and linux-pci) > > > > On 13 April 2018 at 19:32, Gilles Buloz wrote: > >> Dear developers, > >> > >> I currently have two functional workarounds for this issue but > >> would like to know which one you would recommend, if any :-) I'm > >> using a LS1043A CPU (NXP QorIQ Layerscape) and get a "synchronous > >> external abort" when booting because of a PCI config read during > >> PCI scan. > >> > >> I'm using a custom hardware (based on LS1043ARDB) having a > >> PEX8112 PCIe-to-PCI bridge connected to the LS1043A to have a PCI > >> slot for legacy devices. This bridge only supports PCI-Compatible > >> config accesses (offset 0x00-0xFF). I would guess the PEX8112 itself has 4K of config space, but it only forwards 256 bytes of config space to the conventional PCI secondary bus. > >> On this PCI slot I connect a PCI module made of a PCI-to-PCIe > >> bridge plus PCIe devices behind. > >> > >> The problem occurs when the kernel probes the PCIe devices : as > >> they are PCIe devices, the kernel does a PCI config read access > >> at offset 0x100 to check if "PCIe extended capability registers" > >> are accessible (see drivers/pci/probe.c, function > >> pci_cfg_space_size_ext()). Unfortunately the PEX8112 PCIe-to-PCI > >> bridge that is in the path reports an error to the CPU for this > >> access, and it seems there's no way to disable that on this > >> bridge. > >> > >> The first workaround I found was to patch > >> drivers/pci/host/pci-layerscape.c to have PCIE_ABSERR_SETTING set > >> to 0x9400 instead of 0x9401 (for PCIE_ABSERR register) to disable > >> error reporting. This only impacts an NXP part of the Linux > >> kernel code, but I'm not sure this is a good idea (however it > >> seems to be like that on Intel platforms where even MEM accesses > >> to a no-device address return FF without any error). > >> > >> I've also tried another workaround that works : patch > >> drivers/pci/probe.c to use bus_flags to remember if a bus is > >> behind a bridge without extended address capability, to avoid PCi > >> config read accesses at offset 0x100 in pci_cfg_space_size() / > >> pci_cfg_space_size_ext(). But this patch impacts the generic PCI > >> probe method of Linux. > >> > >> Any Idea to properly handle that issue ? > >> > > This seems like a rather unusual configuration, but I guess that > > if the first bridge/switch advertises its inability to support > > extended config space accesses, we should not be performing them > > on any of its subordinate buses. How does the PEX8112 advertise > > this limitation? > > > > That said, I wonder if it is reasonable in the first place to > > expect that a PCIe device works as expected passing through a > > legacy PCI layer like that. > > > The PEX8112 PCIe-to-PCI bridge has capability PCI_CAP_ID_EXP, but > has no PCI_CAP_ID_PCIX capability. As I understand the lack of > PCI_CAP_ID_PCIX is advertising this limitation on the PCI side (no > support for PCI config offset >=0x100). Sounds right to me. > Also I guess in the case of a bridge having PCI_CAP_ID_PCIX, this > limitation would be advertised by the lack of PCI_X_STATUS_266MHZ > and PCI_X_STATUS_533MHZ (as done in drivers/pci/probe.c at > pci_cfg_space_size()) Also sounds right. Per the PCI-X spec, checking for PCI_X_STATUS_266MHZ should be enough, but it shouldn't hurt to check for either PCI_X_STATUS_266MHZ or PCI_X_STATUS_533MHZ. > I'm currently using the attached patch (for kernel 4.1.35-rt41 from > NXP Yocto BSP). It uses bus_flags to remember if a bus is behind a > bridge without extended address capability to avoid PCi config > accesses at offset >= 0x100. Thanks to this patch I now have a > functional system with functional PCI/PCIe devices. The patch seems like it's looking at the right things, but I don't want to build it into pci_scan_bridge_extend() because that function is much too complicated already. I'd rather build it into pci_cfg_space_size() or pci_cfg_space_size_ext() somehow. Maybe something along these lines? This doesn't account for the case of a PCIe-to-PCI-X Mode 2 bridge; in that case, I think all 4K would be accessible on the PCI-X side. > --- include/linux/pci.h.orig 2018-03-26 16:51:18.050000000 +0000 > +++ include/linux/pci.h 2018-03-26 16:51:27.660000000 +0000 > @@ -193,6 +193,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_COMPAT_CFG_SPACE = (__force pci_bus_flags_t) 4, > }; > > /* These values come from the PCI Express Spec */ > --- drivers/pci/probe.c.orig 2018-01-22 09:29:52.000000000 +0000 > +++ drivers/pci/probe.c 2018-03-26 16:54:30.830000000 +0000 > @@ -827,6 +827,28 @@ > child->primary = primary; > pci_bus_insert_busn_res(child, secondary, subordinate); > child->bridge_ctl = bctl; > + > + { > + int pos; > + u32 status; > + bool pci_compat_cfg_space = false; > + > + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)) { > + /* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse mode, we have to check for PCI-X capabilities */ > + pos = pci_find_capability(dev, PCI_CAP_ID_PCIX); > + if (pos) { > + pci_read_config_dword(dev, pos + PCI_X_STATUS, &status); > + if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ))) > + pci_compat_cfg_space = true; > + } else { > + pci_compat_cfg_space = true; > + } > + if (pci_compat_cfg_space) { > + dev_info(&dev->dev, "[%04x:%04x] Child bus limited to PCI-Compatible config space\n", dev->vendor, dev->device); > + child->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE; > + } > + } > + } > } > > cmax = pci_scan_child_bus(child); > @@ -1098,6 +1120,11 @@ > goto fail; > } > > + if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) { > + dev_info(&dev->dev, "[%04x:%04x] PCI-Compatible config space only due to parent bus(es)\n", dev->vendor, dev->device); > + return PCI_CFG_SPACE_SIZE; > + } > + > return pci_cfg_space_size_ext(dev); > > fail: > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index ac91b6fd0bcd..d8b091f0bcd1 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev) * pci_cfg_space_size - Get the configuration space size of the PCI device * @dev: PCI device * - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devices + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express devices * have 4096 bytes. Even if the device is capable, that doesn't mean we can * access it. Maybe we don't have a way to generate extended config space * accesses, or the device is behind a reverse Express bridge. So we try @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev) */ static int pci_cfg_space_size_ext(struct pci_dev *dev) { + struct pci_dev *bridge = pci_upstream_bridge(dev); u32 status; int pos = PCI_CFG_SPACE_SIZE; + if (bridge && pci_is_pcie(bridge) && + pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE) + return PCI_CFG_SPACE_SIZE; + if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL) return PCI_CFG_SPACE_SIZE; if (status == 0xffffffff || pci_ext_cfg_is_aliased(dev))