From patchwork Mon Feb 4 22:41:24 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Williamson X-Patchwork-Id: 2095341 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: X-Original-To: patchwork-linux-pci@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 9E3983FC23 for ; Mon, 4 Feb 2013 22:41:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755911Ab3BDWld (ORCPT ); Mon, 4 Feb 2013 17:41:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2740 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755809Ab3BDWld (ORCPT ); Mon, 4 Feb 2013 17:41:33 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r14MfQPB010987 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Mon, 4 Feb 2013 17:41:26 -0500 Received: from [10.3.113.8] ([10.3.113.8]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r14MfO8f029208; Mon, 4 Feb 2013 17:41:25 -0500 Message-ID: <1360017684.11144.547.camel@bling.home> Subject: Re: PCI warning on boot 3.8.0-rc1 From: Alex Williamson To: Stephen Hemminger Cc: Bjorn Helgaas , linux-pci@vger.kernel.org, David Woodhouse , iommu@lists.linux-foundation.org Date: Mon, 04 Feb 2013 15:41:24 -0700 In-Reply-To: <1360009684.11144.539.camel@bling.home> References: <20130116143853.339dd89b@nehalam.linuxnetplumber.net> <20130204103645.38ae2d5e@nehalam.linuxnetplumber.net> <1360009684.11144.539.camel@bling.home> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org 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 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 --- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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; } /**