From patchwork Fri Apr 27 12:29:32 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Gilles Buloz X-Patchwork-Id: 10368489 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 69C47602DC for ; Fri, 27 Apr 2018 12:30:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 53A9429374 for ; Fri, 27 Apr 2018 12:30:04 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 45685293C0; Fri, 27 Apr 2018 12:30:04 +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 8A1CF29374 for ; Fri, 27 Apr 2018 12:30:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type: MIME-Version:In-Reply-To:References:Message-ID:Date:Subject:To:From:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=iSApQNeGg8DuJ72JypvT4K+AvKXwNJJa+X9IB826pw8=; b=jvjT2Foyp6RWFxjF31+lITNVT sIFsI5VxrB4a6oiq7jZGafCzM5jPT5rCKA1sHrVFAU7nIBngyjoYiqC/AHjZLhVgc8ez6B24C71aB Ax78H7TM9qB8AmQBNFD0gxRo5aFsqgOW9TUKyvcG/ppxb2IInVGYU1KL/Km+GEBttlxadRpaTfhJT YnVuXaLPABqtnA+ZWvftCk732jJ2TUS6g41TUlr+69043j7Tk2vZx/9GY5yqKLK3PM325k2TvJ2Uo TJ1sCBCnpGG6MdzIIm7vu7i0MZ9m4/xTbtxFtfKtjmJlJPB5JSS2+5pMx4n6WrQytVVAH8lSxsJ9X iy4mID3hA==; 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 1fC2Vi-0002Tq-GB; Fri, 27 Apr 2018 12:29:58 +0000 Received: from eu-smtp-delivery-185.mimecast.com ([146.101.78.185]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fC2Vd-0002S0-Ry for linux-arm-kernel@lists.infradead.org; Fri, 27 Apr 2018 12:29:56 +0000 Received: from SDEMUCHB02.kontron.local (host-212-18-14-254.customer.m-online.net [212.18.14.254]) (Using TLS) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-123-p_pCxHB6MT-8Zu5smoovwg-2; Fri, 27 Apr 2018 13:29:35 +0100 Received: from SDEMUCMB01.kontron.local ([fe80::c99d:e06e:28fc:b20]) by SDEMUCHB02.kontron.local ([fe80::1e6:aa2f:691:659e%14]) with mapi id 14.03.0123.003; Fri, 27 Apr 2018 14:29:32 +0200 From: Gilles Buloz To: Ard Biesheuvel Subject: Re: LS1043A : "synchronous abort" at boot due to PCI config read Thread-Topic: LS1043A : "synchronous abort" at boot due to PCI config read Thread-Index: AQHT3gPjlj6/yW5VNkGytbg+ZL5f/qQUaTmA Date: Fri, 27 Apr 2018 12:29:32 +0000 Message-ID: <5AE317AB.4020404@kontron.com> References: <5AD0E995.3090802@kontron.com> In-Reply-To: Accept-Language: en-US, de-DE Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: user-agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.1.1 x-originating-ip: [172.20.161.89] MIME-Version: 1.0 X-MC-Unique: p_pCxHB6MT-8Zu5smoovwg-2 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180427_052954_206708_66EB2389 X-CRM114-Status: GOOD ( 28.43 ) 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 , "linux-arm-kernel@lists.infradead.org" , "minghuan.Lian@freescale.com" 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 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). >> 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). 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()) 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. --- 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: