On Fri, Jul 07, 2017 at 10:53:13AM -0400, Sinan Kaya wrote: > According to extended tags ECN document, all PCIe receivers are expected > to support extended tags support. It should be safe to enable extended > tags on endpoints without checking compatibility. > > This assumption seems to be working fine except for the legacy systems. > The ECN has been written against PCIE spec version 2.0. Therefore, we need > to exclude all version 1.0 devices from this change as there is HW out > there that can't handle extended tags. > > Note that the default value of Extended Tags Enable bit is implementation > specific. Therefore, we are clearing the bit by default when incompatible > HW is found without assuming that value is zero. The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a Requester. As far as I can see, it has nothing to do with whether the function supports 8-bit tags as a *Completer*, so the implicit assumption of the spec is that all Completers always support 8-bit tags. My guess is that's why the ECN thought it would be safe to enable extended tags by default. If that's the case, this is just a defect in the device (the Completer), and we should blacklist it. Looking at the PCIe Capability version might happen to correlate with Completer support for 8-bit tags, but that looks like just a coincidence to me. > Reported-by: Wim ten Have <wim.ten.have@xxxxxxxxxx> > Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674 > Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported") > Signed-off-by: Sinan Kaya <okaya@xxxxxxxxxxxxxx> > --- > drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 39 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 19c8950..e3b3c18 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp) > */ > } > > -static void pci_configure_extended_tags(struct pci_dev *dev) > +static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data) > { > + bool *found = data; > + int rc; > + u16 flags; > + > + if (!pci_is_pcie(dev)) > + return 0; > + > + rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags); > + if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2)) > + *found = true; > + > + return 0; > +} > + > +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy) > +{ > + bool supported = !*(bool *)legacy; > u32 dev_cap; > + u16 flags; > int ret; > > if (!pci_is_pcie(dev)) > - return; > + return 0; > + > + ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags); > + if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2)) > + return 0; > > ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap); > - if (ret) > - return; > + if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG))) > + return 0; > > - if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG) > + if (supported) { > + dev_dbg(&dev->dev, "setting extended tags capability\n"); > pcie_capability_set_word(dev, PCI_EXP_DEVCTL, > PCI_EXP_DEVCTL_EXT_TAG); > + } else { > + dev_dbg(&dev->dev, "clearing extended tags capability\n"); > + pcie_capability_clear_word(dev, PCI_EXP_DEVCTL, > + PCI_EXP_DEVCTL_EXT_TAG); > + } > + > + return 0; > } > > static void pci_configure_device(struct pci_dev *dev) > @@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev) > int ret; > > pci_configure_mps(dev); > - pci_configure_extended_tags(dev); > > memset(&hpp, 0, sizeof(hpp)); > ret = pci_get_hp_params(dev, &hpp); > @@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data) > void pcie_bus_configure_settings(struct pci_bus *bus) > { > u8 smpss = 0; > + bool legacy_found = false; > > if (!bus->self) > return; > @@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus) > > pcie_bus_configure_set(bus->self, &smpss); > pci_walk_bus(bus, pcie_bus_configure_set, &smpss); > + > + pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found); > + pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found); > } > EXPORT_SYMBOL_GPL(pcie_bus_configure_settings); > > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
