Le 27/04/2018 10:43, Ard Biesheuvel a écrit :
> (add Bjorn and linux-pci)
>
> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@xxxxxxxxxxx> 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:
