On Mon, 2013-02-04 at 13:28 -0700, Alex Williamson wrote: > On Mon, 2013-02-04 at 10:36 -0800, Stephen Hemminger wrote: > > > I think drivers/pci/search.c is identical between 3.7 and 3.8-rc1. Is > > > this the first time you've turned on the IOMMU on that box? > > > > It exists in 3.7 and earlier kernels, just haven't turned on same config. > > > > > It's the same warning as in this bugzilla: > > > https://bugzilla.kernel.org/show_bug.cgi?id=44881, and there's a patch > > > there at https://bugzilla.kernel.org/show_bug.cgi?id=44881#c11, but > > > it's just a quirk that turns off VT-d if we find certain broken > > > bridges. It doesn't look like you have any of those (although I don't > > > know what you have at 05:00.0). > > > > > > Bjorn > > > > This is a standard ASUS motherboard, and don't want to disable VT-d. > > Stephen, > > Can you give the lspci -vvv of device 5:00.0 to see if it's one we've > seen before? Does the patch below help? > > Bjorn, I think we need to quirk it somehow. So far they've all been > PCI-to-PCI bridges attached to root ports where we expect it's actually > a PCIe-to-PCI bridge. Seems like maybe we could have the same attached > to a downstream port. The patch below avoids the WARN and gives us a > device, but of course pci_is_pcie reports wrong for this device and may > cause some trickle down breakage. A more complete option might be to > add a is_pcie flag to the device that can be set independent of > pcie_cap. We'd need to check all the callers for assumptions, but then > we could put the quirk in one place and hopefully fix everything. > Thoughts? Thanks, This latter approach seems like it might be easier than I expected since all the users are so well filtered through the access functions. A quick look through who uses pci_is_pcie seems like this might be complete, but more eyes are required. I'll upload this to the bz for those reporters to test as well. Thoughts? Thanks, Alex commit 60d668a3cdeeb0e29570cf0043736436c146bde8 Author: Alex Williamson <alex.williamson@xxxxxxxxxx> Date: Mon Feb 4 15:34:34 2013 -0700 pci: Handle unadvertised PCIe bridges There seem to be several PCIe-to-PCI bridges out in the wild that blatantly ignore the PCIe specification and do not expose a PCIe capability. We can attempt to deduce their existence by looking for PCI bridges directly connected to root ports or downstream ports. What this means is that pci_is_pcie() does not imply PCIe capability and we un-deprecate is_pcie to denote the difference. All the accesses seem to go through pcie_capability_reg_implemented, so we can significantly limit the footprint of this change by checking things there. Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx> diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 3af0478..3df24e7 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -511,7 +511,7 @@ static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev) static bool pcie_capability_reg_implemented(struct pci_dev *dev, int pos) { - if (!pci_is_pcie(dev)) + if (!pci_is_pcie(dev) || !pci_pcie_cap(dev)) return false; switch (pos) { diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 6186f03..0a87b6b 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -926,20 +926,46 @@ static void pci_read_irq(struct pci_dev *dev) dev->irq = irq; } +static bool is_unadvertised_pcie_bridge(struct pci_dev *pdev) +{ + struct pci_dev *parent; + + if (pdev->hdr_type != PCI_HEADER_TYPE_BRIDGE || + pci_find_capability(pdev, PCI_CAP_ID_EXP) || + pci_is_root_bus(pdev->bus)) + return false; + + parent = pdev->bus->self; + + if (pci_is_pcie(parent) && + (pci_pcie_type(parent) == PCI_EXP_TYPE_ROOT_PORT || + pci_pcie_type(parent) == PCI_EXP_TYPE_DOWNSTREAM)) { + pr_warn("WARNING: Your hardware is broken, device %s appears to be a\n Legacy PCI device attached directly to a PCIe device which is not a\n PCIe-to-PCI bridge. Per section 7.8 of the PCI Express 3.0 spec, the\n PCI express capability structure is required for PCI express device\nfunctions.\n", + pci_name(pdev)); + return true; + } + + return false; +} + void set_pcie_port_type(struct pci_dev *pdev) { int pos; - u16 reg16; + u16 flags, caps = 0; pos = pci_find_capability(pdev, PCI_CAP_ID_EXP); - if (!pos) + if (pos) { + pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &flags); + pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &caps); + } else if (is_unadvertised_pcie_bridge(pdev)) + flags = PCI_EXP_TYPE_PCI_BRIDGE << 4; + else return; + pdev->is_pcie = 1; pdev->pcie_cap = pos; - pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, ®16); - pdev->pcie_flags_reg = reg16; - pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, ®16); - pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD; + pdev->pcie_flags_reg = flags; + pdev->pcie_mpss = caps & PCI_EXP_DEVCAP_PAYLOAD; } void set_pcie_hotplug_bridge(struct pci_dev *pdev) diff --git a/include/linux/pci.h b/include/linux/pci.h index 15472d6..c9ef42c 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -300,8 +300,8 @@ struct pci_dev { unsigned int msix_enabled:1; unsigned int ari_enabled:1; /* ARI forwarding */ unsigned int is_managed:1; - unsigned int is_pcie:1; /* Obsolete. Will be removed. - Use pci_is_pcie() instead */ + unsigned int is_pcie:1; /* Don't access directly, + use pci_is_pcie() instead */ unsigned int needs_freset:1; /* Dev requires fundamental reset */ unsigned int state_saved:1; unsigned int is_physfn:1; @@ -1689,7 +1689,7 @@ static inline int pci_pcie_cap(struct pci_dev *dev) */ static inline bool pci_is_pcie(struct pci_dev *dev) { - return !!pci_pcie_cap(dev); + return dev->is_pcie; } /** -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
