diff for duplicates of <5AE75809.30701@kontron.com> diff --git a/a/1.txt b/N1/1.txt index 3eaac69..27c01f0 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,13 +1,12 @@ -Le 30/04/2018 19:04, Bjorn Helgaas a =E9crit : +Le 30/04/2018 19:04, Bjorn Helgaas a ?crit : > On Mon, Apr 30, 2018 at 01:36:53PM +0000, Gilles Buloz wrote: ->> Le 30/04/2018 10:46, Gilles BULOZ a =E9crit : ->>> Le 27/04/2018 18:56, Bjorn Helgaas a =E9crit : +>> Le 30/04/2018 10:46, Gilles BULOZ a ?crit : +>>> Le 27/04/2018 18:56, Bjorn Helgaas a ?crit : >>>> On Fri, Apr 27, 2018 at 12:29:32PM +0000, Gilles Buloz wrote: ->>>>> Le 27/04/2018 10:43, Ard Biesheuvel a =E9crit : +>>>>> 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@kontron.com> w= -rote: +>>>>>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrote: >>>>>>> Dear developers, >>>>>>> >>>>>>> I currently have two functional workarounds for this issue but @@ -66,22 +65,21 @@ rote: >>>>> 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 >=3D0x100). +>>>>> 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_266M= -HZ +>>>> 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 >=3D 0x100. Thanks to this patch I now have a +>>>>> 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 @@ -96,95 +94,77 @@ HZ >>>> 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_de= -v *dev) ->>>> * pci_cfg_space_size - Get the configuration space size of the PCI= - device +>>>> @@ -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 de= -vices ->>>> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Expre= -ss devices ->>>> * have 4096 bytes. Even if the device is capable, that doesn't me= -an we can ->>>> * access it. Maybe we don't have a way to generate extended confi= -g 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_d= -ev *dev) +>>>> - * 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 =3D pci_upstream_bridge(dev); +>>>> + struct pci_dev *bridge = pci_upstream_bridge(dev); >>>> u32 status; ->>>> int pos =3D PCI_CFG_SPACE_SIZE; +>>>> int pos = PCI_CFG_SPACE_SIZE; >>>> + if (bridge && pci_is_pcie(bridge) && ->>>> + pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE) +>>>> + pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE) >>>> + return PCI_CFG_SPACE_SIZE; >>>> + ->>>> if (pci_read_config_dword(dev, pos, &status) !=3D PCIBIOS_SUCCE= -SSFUL) +>>>> if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL) >>>> return PCI_CFG_SPACE_SIZE; ->>>> if (status =3D=3D 0xffffffff || pci_ext_cfg_is_aliased(dev)) +>>>> if (status == 0xffffffff || pci_ext_cfg_is_aliased(dev)) >>>> >>>>> --- 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 =3D (__force pci_bus_flags_t) 1, ->>>>> PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2, ->>>>> + PCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4, +>>>>> 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 =3D primary; ->>>>> pci_bus_insert_busn_res(child, secondary, subordinate)= -; ->>>>> child->bridge_ctl =3D bctl; +>>>>> child->primary = primary; +>>>>> pci_bus_insert_busn_res(child, secondary, subordinate); +>>>>> child->bridge_ctl = bctl; >>>>> + >>>>> + { >>>>> + int pos; >>>>> + u32 status; ->>>>> + bool pci_compat_cfg_space =3D false; +>>>>> + bool pci_compat_cfg_space = false; >>>>> + ->>>>> + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) =3D=3D = -PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) =3D=3D +>>>>> + 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 fo= -rward or reverse mode, we have to check for PCI-X +>>>>> + /* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse mode, we have to check for PCI-X >>>>> capabilities */ ->>>>> + pos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX= -); +>>>>> + pos = pci_find_capability(dev, PCI_CAP_ID_PCIX); >>>>> + if (pos) { ->>>>> + pci_read_config_dword(dev, pos + PCI_X_STATU= -S, &status); ->>>>> + if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_= -STATUS_533MHZ))) ->>>>> + pci_compat_cfg_space =3D true; +>>>>> + 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 =3D true; +>>>>> + pci_compat_cfg_space = true; >>>>> + } >>>>> + if (pci_compat_cfg_space) { ->>>>> + dev_info(&dev->dev, "[%04x:%04x] Child bus l= -imited to PCI-Compatible config space\n", dev->vendor, +>>>>> + dev_info(&dev->dev, "[%04x:%04x] Child bus limited to PCI-Compatible config space\n", dev->vendor, >>>>> dev->device); ->>>>> + child->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_C= -FG_SPACE; +>>>>> + child->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE; >>>>> + } >>>>> + } >>>>> + } >>>>> } ->>>>> cmax =3D pci_scan_child_bus(child); +>>>>> 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); +>>>>> + 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; >>>>> + } >>>>> + @@ -196,8 +176,7 @@ FG_SPACE; >>> of device *dev being checked. I understand the purpose, but I >>> think this fails for my config that is : >>> ->>> LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector ->= - PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe +>>> LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe >>> devices (one on each port) >>> >>> because : @@ -222,11 +201,10 @@ FG_SPACE; > actually see the inheritance in the patch below -- I thought there > would be something like this: > -> dev =3D bus->self; -> parent_bus =3D dev->bus; -> if (parent_bus && parent_bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPA= -CE) -> bus->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE; +> dev = bus->self; +> parent_bus = dev->bus; +> if (parent_bus && parent_bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) +> bus->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE; > > pci_scan_bridge_extend() calls pci_add_new_bus() from two places. You > added a call to pci_bus_check_compat_cfg_space() at one of them, and @@ -260,83 +238,78 @@ CE) >> Sorry, this is for old kernel 4.1.35 but just to clarify what I >> propose (also applies to 4.16.6 by changing value of >> PCI_BUS_FLAGS_COMPAT_CFG_SPACE in pci.h to 8). ->> --- include/linux/pci.h.orig=092018-03-26 16:51:18.050000000 +0000 ->> +++ include/linux/pci.h=092018-04-30 09:50:57.660000000 +0000 +>> --- include/linux/pci.h.orig 2018-03-26 16:51:18.050000000 +0000 +>> +++ include/linux/pci.h 2018-04-30 09:50:57.660000000 +0000 >> @@ -193,6 +193,7 @@ >> enum pci_bus_flags { ->> =09PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1, ->> =09PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2, ->> +=09PCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4, +>> 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, >> }; ->> =20 +>> >> /* These values come from the PCI Express Spec */ ->> --- drivers/pci/probe.c.orig=092018-01-22 09:29:52.000000000 +0000 ->> +++ drivers/pci/probe.c=092018-04-30 13:29:50.600000000 +0000 +>> --- drivers/pci/probe.c.orig 2018-01-22 09:29:52.000000000 +0000 +>> +++ drivers/pci/probe.c 2018-04-30 13:29:50.600000000 +0000 >> @@ -754,6 +754,35 @@ ->> =09=09=09=09=09 PCI_EXP_RTCTL_CRSSVE); +>> PCI_EXP_RTCTL_CRSSVE); >> } ->> =20 +>> >> +static void pci_bus_check_compat_cfg_space(struct pci_bus *bus) >> +{ ->> +=09struct pci_dev *dev =3D bus->self; ->> +=09bool pci_compat_cfg_space =3D false; ->> +=09int pos; ->> +=09u32 status; +>> + struct pci_dev *dev = bus->self; +>> + bool pci_compat_cfg_space = false; +>> + int pos; +>> + u32 status; >> + ->> +=09if (bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) ->> +=09=09return; +>> + if (bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) +>> + return; >> + ->> +=09if (!pci_is_pcie(dev) || /* PCI/PCI bridge */ ->> +=09 (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/= -PCI bridge in forward mode */ ->> +=09 (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/= -PCI bridge in reverse mode */ ->> +=09=09pos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX); ->> +=09=09if (pos) { ->> +=09=09=09pci_read_config_dword(dev, pos + PCI_X_STATUS, &status); ->> +=09=09=09if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ))) ->> +=09=09=09=09pci_compat_cfg_space =3D true; ->> +=09=09} else { ->> +=09=09=09pci_compat_cfg_space =3D true; ->> +=09=09} ->> +=09=09if (pci_compat_cfg_space) { ->> +=09=09=09dev_info(&dev->dev, "bus %02x limited to PCI-Compatible config= - space\n", ->> +=09=09=09=09 bus->number); ->> +=09=09=09bus->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE; ->> +=09=09} ->> +=09} +>> + if (!pci_is_pcie(dev) || /* PCI/PCI bridge */ +>> + (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/PCI bridge in forward mode */ +>> + (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/PCI bridge in reverse mode */ +>> + 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, "bus %02x limited to PCI-Compatible config space\n", +>> + bus->number); +>> + bus->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE; +>> + } +>> + } >> +} >> + >> /* >> * If it's a bridge, configure it and scan the bus behind it. >> * For CardBus bridges, we don't scan behind as the devices will >> @@ -827,6 +856,7 @@ ->> =09=09=09child->primary =3D primary; ->> =09=09=09pci_bus_insert_busn_res(child, secondary, subordinate); ->> =09=09=09child->bridge_ctl =3D bctl; ->> +=09=09=09pci_bus_check_compat_cfg_space(child); ->> =09=09} ->> =20 ->> =09=09cmax =3D pci_scan_child_bus(child); +>> child->primary = primary; +>> pci_bus_insert_busn_res(child, secondary, subordinate); +>> child->bridge_ctl = bctl; +>> + pci_bus_check_compat_cfg_space(child); +>> } +>> +>> cmax = pci_scan_child_bus(child); >> @@ -1084,6 +1114,9 @@ ->> =09u32 status; ->> =09u16 class; ->> =20 ->> +=09if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) ->> +=09=09return PCI_CFG_SPACE_SIZE; +>> u32 status; +>> u16 class; +>> +>> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) +>> + return PCI_CFG_SPACE_SIZE; >> + ->> =09class =3D dev->class >> 8; ->> =09if (class =3D=3D PCI_CLASS_BRIDGE_HOST) ->> =09=09return pci_cfg_space_size_ext(dev); +>> class = dev->class >> 8; +>> if (class == PCI_CLASS_BRIDGE_HOST) +>> return pci_cfg_space_size_ext(dev); > The inheritence is made by this line in pci_alloc_child_bus() : - child->bus_flags =3D parent->bus_flags; -So once we detect a limitation on a bridge impacting a child bus and that w= -e set the flag in child->bus_flags, this flag is=20 + child->bus_flags = parent->bus_flags; +So once we detect a limitation on a bridge impacting a child bus and that we set the flag in child->bus_flags, this flag is automatically present in the child->bus_flags of all its children buses. -I agree with your remarks and will create a function named pci_bus_check_co= -mpat_cfg_space() that will be called from=20 +I agree with your remarks and will create a function named pci_bus_check_compat_cfg_space() that will be called from pci_alloc_child_bus(). I'll test that on Wednesday 2th and will give you my feedback. diff --git a/a/content_digest b/N1/content_digest index 76e8570..52c6461 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -5,27 +5,21 @@ "ref\05AE6D7E2.9030506@kontron.com\0" "ref\05AE71BF4.2010200@kontron.com\0" "ref\020180430170447.GA95643@bhelgaas-glaptop.roam.corp.google.com\0" - "From\0Gilles Buloz <Gilles.Buloz@kontron.com>\0" - "Subject\0Re: LS1043A : \"synchronous abort\" at boot due to PCI config read\0" + "From\0Gilles.Buloz@kontron.com (Gilles Buloz)\0" + "Subject\0LS1043A : \"synchronous abort\" at boot due to PCI config read\0" "Date\0Mon, 30 Apr 2018 17:53:14 +0000\0" - "To\0Bjorn Helgaas <helgaas@kernel.org>\0" - "Cc\0Bjorn Helgaas <bhelgaas@google.com>" - linux-pci <linux-pci@vger.kernel.org> - Minghuan.Lian@nxp.com <Minghuan.Lian@nxp.com> - linux-arm-kernel@lists.infradead.org <linux-arm-kernel@lists.infradead.org> - " Ard Biesheuvel <ard.biesheuvel@linaro.org>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" - "Le 30/04/2018 19:04, Bjorn Helgaas a =E9crit :\n" + "Le 30/04/2018 19:04, Bjorn Helgaas a ?crit :\n" "> On Mon, Apr 30, 2018 at 01:36:53PM +0000, Gilles Buloz wrote:\n" - ">> Le 30/04/2018 10:46, Gilles BULOZ a =E9crit :\n" - ">>> Le 27/04/2018 18:56, Bjorn Helgaas a =E9crit :\n" + ">> Le 30/04/2018 10:46, Gilles BULOZ a ?crit :\n" + ">>> Le 27/04/2018 18:56, Bjorn Helgaas a ?crit :\n" ">>>> On Fri, Apr 27, 2018 at 12:29:32PM +0000, Gilles Buloz wrote:\n" - ">>>>> Le 27/04/2018 10:43, Ard Biesheuvel a =E9crit :\n" + ">>>>> Le 27/04/2018 10:43, Ard Biesheuvel a ?crit :\n" ">>>>>> (add Bjorn and linux-pci)\n" ">>>>>>\n" - ">>>>>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> w=\n" - "rote:\n" + ">>>>>> On 13 April 2018 at 19:32, Gilles Buloz <Gilles.Buloz@kontron.com> wrote:\n" ">>>>>>> Dear developers,\n" ">>>>>>>\n" ">>>>>>> I currently have two functional workarounds for this issue but\n" @@ -84,22 +78,21 @@ ">>>>> The PEX8112 PCIe-to-PCI bridge has capability PCI_CAP_ID_EXP, but\n" ">>>>> has no PCI_CAP_ID_PCIX capability. As I understand the lack of\n" ">>>>> PCI_CAP_ID_PCIX is advertising this limitation on the PCI side (no\n" - ">>>>> support for PCI config offset >=3D0x100).\n" + ">>>>> support for PCI config offset >=0x100).\n" ">>>> Sounds right to me.\n" ">>>>\n" ">>>>> Also I guess in the case of a bridge having PCI_CAP_ID_PCIX, this\n" ">>>>> limitation would be advertised by the lack of PCI_X_STATUS_266MHZ\n" ">>>>> and PCI_X_STATUS_533MHZ (as done in drivers/pci/probe.c at\n" ">>>>> pci_cfg_space_size())\n" - ">>>> Also sounds right. Per the PCI-X spec, checking for PCI_X_STATUS_266M=\n" - "HZ\n" + ">>>> Also sounds right. Per the PCI-X spec, checking for PCI_X_STATUS_266MHZ\n" ">>>> should be enough, but it shouldn't hurt to check for either\n" ">>>> PCI_X_STATUS_266MHZ or PCI_X_STATUS_533MHZ.\n" ">>>>\n" ">>>>> I'm currently using the attached patch (for kernel 4.1.35-rt41 from\n" ">>>>> NXP Yocto BSP). It uses bus_flags to remember if a bus is behind a\n" ">>>>> bridge without extended address capability to avoid PCi config\n" - ">>>>> accesses at offset >=3D 0x100. Thanks to this patch I now have a\n" + ">>>>> accesses at offset >= 0x100. Thanks to this patch I now have a\n" ">>>>> functional system with functional PCI/PCIe devices.\n" ">>>> The patch seems like it's looking at the right things, but I don't\n" ">>>> want to build it into pci_scan_bridge_extend() because that function\n" @@ -114,95 +107,77 @@ ">>>> index ac91b6fd0bcd..d8b091f0bcd1 100644\n" ">>>> --- a/drivers/pci/probe.c\n" ">>>> +++ b/drivers/pci/probe.c\n" - ">>>> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_de=\n" - "v *dev)\n" - ">>>> * pci_cfg_space_size - Get the configuration space size of the PCI=\n" - " device\n" + ">>>> @@ -1367,7 +1367,7 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev)\n" + ">>>> * pci_cfg_space_size - Get the configuration space size of the PCI device\n" ">>>> * @dev: PCI device\n" ">>>> *\n" - ">>>> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express de=\n" - "vices\n" - ">>>> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Expre=\n" - "ss devices\n" - ">>>> * have 4096 bytes. Even if the device is capable, that doesn't me=\n" - "an we can\n" - ">>>> * access it. Maybe we don't have a way to generate extended confi=\n" - "g space\n" - ">>>> * accesses, or the device is behind a reverse Express bridge. So =\n" - "we try\n" - ">>>> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_d=\n" - "ev *dev)\n" + ">>>> - * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI Express devices\n" + ">>>> + * Regular PCI devices have 256 bytes, but PCI-X Mode 2 and PCI Express devices\n" + ">>>> * have 4096 bytes. Even if the device is capable, that doesn't mean we can\n" + ">>>> * access it. Maybe we don't have a way to generate extended config space\n" + ">>>> * accesses, or the device is behind a reverse Express bridge. So we try\n" + ">>>> @@ -1376,9 +1376,14 @@ static bool pci_ext_cfg_is_aliased(struct pci_dev *dev)\n" ">>>> */\n" ">>>> static int pci_cfg_space_size_ext(struct pci_dev *dev)\n" ">>>> {\n" - ">>>> + struct pci_dev *bridge =3D pci_upstream_bridge(dev);\n" + ">>>> + struct pci_dev *bridge = pci_upstream_bridge(dev);\n" ">>>> u32 status;\n" - ">>>> int pos =3D PCI_CFG_SPACE_SIZE;\n" + ">>>> int pos = PCI_CFG_SPACE_SIZE;\n" ">>>> + if (bridge && pci_is_pcie(bridge) &&\n" - ">>>> + pci_pcie_type(bridge) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)\n" + ">>>> + pci_pcie_type(bridge) == PCI_EXP_TYPE_PCI_BRIDGE)\n" ">>>> + return PCI_CFG_SPACE_SIZE;\n" ">>>> +\n" - ">>>> if (pci_read_config_dword(dev, pos, &status) !=3D PCIBIOS_SUCCE=\n" - "SSFUL)\n" + ">>>> if (pci_read_config_dword(dev, pos, &status) != PCIBIOS_SUCCESSFUL)\n" ">>>> return PCI_CFG_SPACE_SIZE;\n" - ">>>> if (status =3D=3D 0xffffffff || pci_ext_cfg_is_aliased(dev))\n" + ">>>> if (status == 0xffffffff || pci_ext_cfg_is_aliased(dev))\n" ">>>>\n" ">>>>> --- include/linux/pci.h.orig 2018-03-26 16:51:18.050000000 +0000\n" ">>>>> +++ include/linux/pci.h 2018-03-26 16:51:27.660000000 +0000\n" ">>>>> @@ -193,6 +193,7 @@\n" ">>>>> enum pci_bus_flags {\n" - ">>>>> PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1,\n" - ">>>>> PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2,\n" - ">>>>> + PCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4,\n" + ">>>>> PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,\n" + ">>>>> PCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,\n" + ">>>>> + PCI_BUS_FLAGS_COMPAT_CFG_SPACE = (__force pci_bus_flags_t) 4,\n" ">>>>> };\n" ">>>>> /* These values come from the PCI Express Spec */\n" ">>>>> --- drivers/pci/probe.c.orig 2018-01-22 09:29:52.000000000 +0000\n" ">>>>> +++ drivers/pci/probe.c 2018-03-26 16:54:30.830000000 +0000\n" ">>>>> @@ -827,6 +827,28 @@\n" - ">>>>> child->primary =3D primary;\n" - ">>>>> pci_bus_insert_busn_res(child, secondary, subordinate)=\n" - ";\n" - ">>>>> child->bridge_ctl =3D bctl;\n" + ">>>>> child->primary = primary;\n" + ">>>>> pci_bus_insert_busn_res(child, secondary, subordinate);\n" + ">>>>> child->bridge_ctl = bctl;\n" ">>>>> +\n" ">>>>> + {\n" ">>>>> + int pos;\n" ">>>>> + u32 status;\n" - ">>>>> + bool pci_compat_cfg_space =3D false;\n" + ">>>>> + bool pci_compat_cfg_space = false;\n" ">>>>> +\n" - ">>>>> + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) =3D=3D =\n" - "PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) =3D=3D\n" + ">>>>> + if (!pci_is_pcie(dev) || (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || (pci_pcie_type(dev) ==\n" ">>>>> PCI_EXP_TYPE_PCI_BRIDGE)) {\n" - ">>>>> + /* for PCI/PCI bridges, or PCIe/PCI bridge in fo=\n" - "rward or reverse mode, we have to check for PCI-X\n" + ">>>>> + /* for PCI/PCI bridges, or PCIe/PCI bridge in forward or reverse mode, we have to check for PCI-X\n" ">>>>> capabilities */\n" - ">>>>> + pos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX=\n" - ");\n" + ">>>>> + pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);\n" ">>>>> + if (pos) {\n" - ">>>>> + pci_read_config_dword(dev, pos + PCI_X_STATU=\n" - "S, &status);\n" - ">>>>> + if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_=\n" - "STATUS_533MHZ)))\n" - ">>>>> + pci_compat_cfg_space =3D true;\n" + ">>>>> + pci_read_config_dword(dev, pos + PCI_X_STATUS, &status);\n" + ">>>>> + if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)))\n" + ">>>>> + pci_compat_cfg_space = true;\n" ">>>>> + } else {\n" - ">>>>> + pci_compat_cfg_space =3D true;\n" + ">>>>> + pci_compat_cfg_space = true;\n" ">>>>> + }\n" ">>>>> + if (pci_compat_cfg_space) {\n" - ">>>>> + dev_info(&dev->dev, \"[%04x:%04x] Child bus l=\n" - "imited to PCI-Compatible config space\\n\", dev->vendor,\n" + ">>>>> + dev_info(&dev->dev, \"[%04x:%04x] Child bus limited to PCI-Compatible config space\\n\", dev->vendor,\n" ">>>>> dev->device);\n" - ">>>>> + child->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_C=\n" - "FG_SPACE;\n" + ">>>>> + child->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" ">>>>> + }\n" ">>>>> + }\n" ">>>>> + }\n" ">>>>> }\n" - ">>>>> cmax =3D pci_scan_child_bus(child);\n" + ">>>>> cmax = pci_scan_child_bus(child);\n" ">>>>> @@ -1098,6 +1120,11 @@\n" ">>>>> goto fail;\n" ">>>>> }\n" ">>>>> + if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE) {\n" - ">>>>> + dev_info(&dev->dev, \"[%04x:%04x] PCI-Compatible config space=\n" - " only due to parent bus(es)\\n\", dev->vendor, dev->device);\n" + ">>>>> + dev_info(&dev->dev, \"[%04x:%04x] PCI-Compatible config space only due to parent bus(es)\\n\", dev->vendor, dev->device);\n" ">>>>> + return PCI_CFG_SPACE_SIZE;\n" ">>>>> + }\n" ">>>>> +\n" @@ -214,8 +189,7 @@ ">>> of device *dev being checked. I understand the purpose, but I\n" ">>> think this fails for my config that is :\n" ">>>\n" - ">>> LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector ->=\n" - " PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe\n" + ">>> LS1043 PCIe root -> PEX8112 PCIe-to-PCI bridge -> PMC slot connector -> PCI-to-PCIe bridge -> PCIe switch (4 ports) -> 4 PCIe\n" ">>> devices (one on each port)\n" ">>>\n" ">>> because :\n" @@ -240,11 +214,10 @@ "> actually see the inheritance in the patch below -- I thought there\n" "> would be something like this:\n" ">\n" - "> dev =3D bus->self;\n" - "> parent_bus =3D dev->bus;\n" - "> if (parent_bus && parent_bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPA=\n" - "CE)\n" - "> bus->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" + "> dev = bus->self;\n" + "> parent_bus = dev->bus;\n" + "> if (parent_bus && parent_bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE)\n" + "> bus->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" ">\n" "> pci_scan_bridge_extend() calls pci_add_new_bus() from two places. You\n" "> added a call to pci_bus_check_compat_cfg_space() at one of them, and\n" @@ -278,85 +251,80 @@ ">> Sorry, this is for old kernel 4.1.35 but just to clarify what I\n" ">> propose (also applies to 4.16.6 by changing value of\n" ">> PCI_BUS_FLAGS_COMPAT_CFG_SPACE in pci.h to 8).\n" - ">> --- include/linux/pci.h.orig=092018-03-26 16:51:18.050000000 +0000\n" - ">> +++ include/linux/pci.h=092018-04-30 09:50:57.660000000 +0000\n" + ">> --- include/linux/pci.h.orig\t2018-03-26 16:51:18.050000000 +0000\n" + ">> +++ include/linux/pci.h\t2018-04-30 09:50:57.660000000 +0000\n" ">> @@ -193,6 +193,7 @@\n" ">> enum pci_bus_flags {\n" - ">> =09PCI_BUS_FLAGS_NO_MSI =3D (__force pci_bus_flags_t) 1,\n" - ">> =09PCI_BUS_FLAGS_NO_MMRBC =3D (__force pci_bus_flags_t) 2,\n" - ">> +=09PCI_BUS_FLAGS_COMPAT_CFG_SPACE =3D (__force pci_bus_flags_t) 4,\n" + ">> \tPCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1,\n" + ">> \tPCI_BUS_FLAGS_NO_MMRBC = (__force pci_bus_flags_t) 2,\n" + ">> +\tPCI_BUS_FLAGS_COMPAT_CFG_SPACE = (__force pci_bus_flags_t) 4,\n" ">> };\n" - ">> =20\n" + ">> \n" ">> /* These values come from the PCI Express Spec */\n" - ">> --- drivers/pci/probe.c.orig=092018-01-22 09:29:52.000000000 +0000\n" - ">> +++ drivers/pci/probe.c=092018-04-30 13:29:50.600000000 +0000\n" + ">> --- drivers/pci/probe.c.orig\t2018-01-22 09:29:52.000000000 +0000\n" + ">> +++ drivers/pci/probe.c\t2018-04-30 13:29:50.600000000 +0000\n" ">> @@ -754,6 +754,35 @@\n" - ">> =09=09=09=09=09 PCI_EXP_RTCTL_CRSSVE);\n" + ">> \t\t\t\t\t PCI_EXP_RTCTL_CRSSVE);\n" ">> }\n" - ">> =20\n" + ">> \n" ">> +static void pci_bus_check_compat_cfg_space(struct pci_bus *bus)\n" ">> +{\n" - ">> +=09struct pci_dev *dev =3D bus->self;\n" - ">> +=09bool pci_compat_cfg_space =3D false;\n" - ">> +=09int pos;\n" - ">> +=09u32 status;\n" + ">> +\tstruct pci_dev *dev = bus->self;\n" + ">> +\tbool pci_compat_cfg_space = false;\n" + ">> +\tint pos;\n" + ">> +\tu32 status;\n" ">> +\n" - ">> +=09if (bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE)\n" - ">> +=09=09return;\n" + ">> +\tif (bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE)\n" + ">> +\t\treturn;\n" ">> +\n" - ">> +=09if (!pci_is_pcie(dev) || /* PCI/PCI bridge */\n" - ">> +=09 (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/=\n" - "PCI bridge in forward mode */\n" - ">> +=09 (pci_pcie_type(dev) =3D=3D PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/=\n" - "PCI bridge in reverse mode */\n" - ">> +=09=09pos =3D pci_find_capability(dev, PCI_CAP_ID_PCIX);\n" - ">> +=09=09if (pos) {\n" - ">> +=09=09=09pci_read_config_dword(dev, pos + PCI_X_STATUS, &status);\n" - ">> +=09=09=09if (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)))\n" - ">> +=09=09=09=09pci_compat_cfg_space =3D true;\n" - ">> +=09=09} else {\n" - ">> +=09=09=09pci_compat_cfg_space =3D true;\n" - ">> +=09=09}\n" - ">> +=09=09if (pci_compat_cfg_space) {\n" - ">> +=09=09=09dev_info(&dev->dev, \"bus %02x limited to PCI-Compatible config=\n" - " space\\n\",\n" - ">> +=09=09=09=09 bus->number);\n" - ">> +=09=09=09bus->bus_flags |=3D PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" - ">> +=09=09}\n" - ">> +=09}\n" + ">> +\tif (!pci_is_pcie(dev) || /* PCI/PCI bridge */\n" + ">> +\t (pci_pcie_type(dev) == PCI_EXP_TYPE_PCIE_BRIDGE) || /* PCIe/PCI bridge in forward mode */\n" + ">> +\t (pci_pcie_type(dev) == PCI_EXP_TYPE_PCI_BRIDGE)) { /* PCIe/PCI bridge in reverse mode */\n" + ">> +\t\tpos = pci_find_capability(dev, PCI_CAP_ID_PCIX);\n" + ">> +\t\tif (pos) {\n" + ">> +\t\t\tpci_read_config_dword(dev, pos + PCI_X_STATUS, &status);\n" + ">> +\t\t\tif (!(status & (PCI_X_STATUS_266MHZ | PCI_X_STATUS_533MHZ)))\n" + ">> +\t\t\t\tpci_compat_cfg_space = true;\n" + ">> +\t\t} else {\n" + ">> +\t\t\tpci_compat_cfg_space = true;\n" + ">> +\t\t}\n" + ">> +\t\tif (pci_compat_cfg_space) {\n" + ">> +\t\t\tdev_info(&dev->dev, \"bus %02x limited to PCI-Compatible config space\\n\",\n" + ">> +\t\t\t\t bus->number);\n" + ">> +\t\t\tbus->bus_flags |= PCI_BUS_FLAGS_COMPAT_CFG_SPACE;\n" + ">> +\t\t}\n" + ">> +\t}\n" ">> +}\n" ">> +\n" ">> /*\n" ">> * If it's a bridge, configure it and scan the bus behind it.\n" ">> * For CardBus bridges, we don't scan behind as the devices will\n" ">> @@ -827,6 +856,7 @@\n" - ">> =09=09=09child->primary =3D primary;\n" - ">> =09=09=09pci_bus_insert_busn_res(child, secondary, subordinate);\n" - ">> =09=09=09child->bridge_ctl =3D bctl;\n" - ">> +=09=09=09pci_bus_check_compat_cfg_space(child);\n" - ">> =09=09}\n" - ">> =20\n" - ">> =09=09cmax =3D pci_scan_child_bus(child);\n" + ">> \t\t\tchild->primary = primary;\n" + ">> \t\t\tpci_bus_insert_busn_res(child, secondary, subordinate);\n" + ">> \t\t\tchild->bridge_ctl = bctl;\n" + ">> +\t\t\tpci_bus_check_compat_cfg_space(child);\n" + ">> \t\t}\n" + ">> \n" + ">> \t\tcmax = pci_scan_child_bus(child);\n" ">> @@ -1084,6 +1114,9 @@\n" - ">> =09u32 status;\n" - ">> =09u16 class;\n" - ">> =20\n" - ">> +=09if (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE)\n" - ">> +=09=09return PCI_CFG_SPACE_SIZE;\n" + ">> \tu32 status;\n" + ">> \tu16 class;\n" + ">> \n" + ">> +\tif (dev->bus->bus_flags & PCI_BUS_FLAGS_COMPAT_CFG_SPACE)\n" + ">> +\t\treturn PCI_CFG_SPACE_SIZE;\n" ">> +\n" - ">> =09class =3D dev->class >> 8;\n" - ">> =09if (class =3D=3D PCI_CLASS_BRIDGE_HOST)\n" - ">> =09=09return pci_cfg_space_size_ext(dev);\n" + ">> \tclass = dev->class >> 8;\n" + ">> \tif (class == PCI_CLASS_BRIDGE_HOST)\n" + ">> \t\treturn pci_cfg_space_size_ext(dev);\n" ">\n" "The inheritence is made by this line in pci_alloc_child_bus() :\n" - " child->bus_flags =3D parent->bus_flags;\n" - "So once we detect a limitation on a bridge impacting a child bus and that w=\n" - "e set the flag in child->bus_flags, this flag is=20\n" + " child->bus_flags = parent->bus_flags;\n" + "So once we detect a limitation on a bridge impacting a child bus and that we set the flag in child->bus_flags, this flag is \n" "automatically present in the child->bus_flags of all its children buses.\n" "\n" - "I agree with your remarks and will create a function named pci_bus_check_co=\n" - "mpat_cfg_space() that will be called from=20\n" + "I agree with your remarks and will create a function named pci_bus_check_compat_cfg_space() that will be called from \n" "pci_alloc_child_bus().\n" I'll test that on Wednesday 2th and will give you my feedback. -cb7362fa5036a4a6ed9e1097f3348c624392c2024eefe4fe577468e16f481491 +236d180ddfe6b564adfbc829cf79d238bfb91a4a756ec680a5b124ed8dd9ce72
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.