diff mbox

PCI warning on boot 3.8.0-rc1

Message ID 1360009684.11144.539.camel@bling.home (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Alex Williamson Feb. 4, 2013, 8:28 p.m. UTC 
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,

Alex




--
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

Comments

Stephen Hemminger Feb. 4, 2013, 8:30 p.m. UTC | #1 
On Mon, 04 Feb 2013 13:28:04 -0700
Alex Williamson <alex.williamson@redhat.com> 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?


$ sudo lspci -vvv -s 5:00.0
05:00.0 PCI bridge: ASMedia Technology Inc. ASM1083/1085 PCIe to PCI Bridge (rev 03) (prog-if 01 [Subtractive decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Bus: primary=05, secondary=06, subordinate=06, sec-latency=32
	I/O behind bridge: 0000f000-00000fff
	Memory behind bridge: fff00000-000fffff
	Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff
	Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: [c0] Subsystem: ASUSTeK Computer Inc. Device 8489


> 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,
> 
> Alex
> 
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index bf969ba..65ae270 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -42,6 +42,15 @@ pci_find_upstream_pcie_bridge(struct pci_dev *pdev)
>  		}
>  		/* PCI device should connect to a PCIe bridge */
>  		if (pci_pcie_type(pdev) != PCI_EXP_TYPE_PCI_BRIDGE) {
> +			/*
> +			 * Not all PCIe-to-PCI bridges expose a PCIe
> +			 * capability.  If we make it to a PCIe root port
> +			 * and the previous device was a PCI-to-PCI bridge,
> +			 * assume it was really a PCIe-to-PCI bridge.
> +			 */
> +			if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
> +			    tmp && tmp->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> +				return tmp;
>  			/* Busted hardware? */
>  			WARN_ON_ONCE(1);
>  			return NULL;
> 
> 

--
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
Alex Williamson Feb. 4, 2013, 8:36 p.m. UTC | #2 
On Mon, 2013-02-04 at 12:30 -0800, Stephen Hemminger wrote:
> On Mon, 04 Feb 2013 13:28:04 -0700
> Alex Williamson <alex.williamson@redhat.com> 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?
> 
> 
> $ sudo lspci -vvv -s 5:00.0
> 05:00.0 PCI bridge: ASMedia Technology Inc. ASM1083/1085 PCIe to PCI Bridge (rev 03) (prog-if 01 [Subtractive decode])

Someone has even decided to name this one a PCIe-to-PCI bridge despite
it lacking a PCIe capability.  Thanks,

Alex

> 	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
> 	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
> 	Latency: 0, Cache Line Size: 64 bytes
> 	Bus: primary=05, secondary=06, subordinate=06, sec-latency=32
> 	I/O behind bridge: 0000f000-00000fff
> 	Memory behind bridge: fff00000-000fffff
> 	Prefetchable memory behind bridge: 00000000fff00000-00000000000fffff
> 	Secondary status: 66MHz+ FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
> 	BridgeCtl: Parity- SERR- NoISA- VGA- MAbort- >Reset- FastB2B-
> 		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
> 	Capabilities: [c0] Subsystem: ASUSTeK Computer Inc. Device 8489
> 
> 
> > 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,
> > 
> > Alex
> > 
> > 
> > diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> > index bf969ba..65ae270 100644
> > --- a/drivers/pci/search.c
> > +++ b/drivers/pci/search.c
> > @@ -42,6 +42,15 @@ pci_find_upstream_pcie_bridge(struct pci_dev *pdev)
> >  		}
> >  		/* PCI device should connect to a PCIe bridge */
> >  		if (pci_pcie_type(pdev) != PCI_EXP_TYPE_PCI_BRIDGE) {
> > +			/*
> > +			 * Not all PCIe-to-PCI bridges expose a PCIe
> > +			 * capability.  If we make it to a PCIe root port
> > +			 * and the previous device was a PCI-to-PCI bridge,
> > +			 * assume it was really a PCIe-to-PCI bridge.
> > +			 */
> > +			if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
> > +			    tmp && tmp->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> > +				return tmp;
> >  			/* Busted hardware? */
> >  			WARN_ON_ONCE(1);
> >  			return NULL;
> > 
> > 
> 



--
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 mbox

Patch

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index bf969ba..65ae270 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -42,6 +42,15 @@  pci_find_upstream_pcie_bridge(struct pci_dev *pdev)
 		}
 		/* PCI device should connect to a PCIe bridge */
 		if (pci_pcie_type(pdev) != PCI_EXP_TYPE_PCI_BRIDGE) {
+			/*
+			 * Not all PCIe-to-PCI bridges expose a PCIe
+			 * capability.  If we make it to a PCIe root port
+			 * and the previous device was a PCI-to-PCI bridge,
+			 * assume it was really a PCIe-to-PCI bridge.
+			 */
+			if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT &&
+			    tmp && tmp->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+				return tmp;
 			/* Busted hardware? */
 			WARN_ON_ONCE(1);
 			return NULL;