[PATCH 0/3] pci/iommu: Quirk non-compliant PCIe-to-PCI bridges

[PATCH 0/3] pci/iommu: Quirk non-compliant PCIe-to-PCI bridges

[Alex Williamson]

This series tries to address:

https://bugzilla.kernel.org/show_bug.cgi?id=44881

Where pci_find_upstream_pcie_bridge() gets lost trying to find the
upstream PCIe-to-PCI bridge for a device because the bridge doesn't
expose a PCIe capability.  To do this, we add a pci_is_pcie_bridge
function which includes a quirk to look to the next upstream device
as a sanity check.  We can then replace pci_find_upstream_pcie_bridge
with a function that's a bit more generic and less tied to intel-iommu
eccentricities.

This works on my VT-d system, but I don't have any of the offending
bridges.  Appreciate testing from those who do.  I'll link this series
to the bz above for more testing.  Thanks,

Alex

---

Alex Williamson (3):
      pci: Add PCI walk function and PCIe bridge test
      intel-iommu: Convert to pci_walk_up_to_first_match
      pci: Remove pci_find_pcie_upstream_bridge


 drivers/iommu/intel-iommu.c         |   77 ++++++++++++++++++++++-------------
 drivers/iommu/intel_irq_remapping.c |   13 ++++--
 drivers/pci/search.c                |   74 ++++++++++++++++++++++------------
 include/linux/pci.h                 |   26 +++++++++---
 4 files changed, 124 insertions(+), 66 deletions(-)

[PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Alex Williamson]

These will replace pci_find_upstream_pcie_bridge, which is difficult
to use and rather specific to intel-iommu usage.  A quirked
pci_is_pcie_bridge function is provided to work around non-compliant
PCIe-to-PCI bridges such as those found in
https://bugzilla.kernel.org/show_bug.cgi?id=44881

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/search.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  |   23 ++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index d0627fa..0357f74 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -17,6 +17,63 @@
 DECLARE_RWSEM(pci_bus_sem);
 EXPORT_SYMBOL_GPL(pci_bus_sem);
 
+/* Test for PCIe bridges. */
+bool pci_is_pcie_bridge(struct pci_dev *pdev)
+{
+	if (!pci_is_bridge(pdev))
+		return false;
+
+	if (pci_is_pcie(pdev))
+		return true;
+
+#ifdef CONFIG_PCI_QUIRKS
+	/*
+	 * If we're not on the root bus, look one device upstream of the
+	 * current device.  If that device is PCIe and is not a PCIe-to-PCI
+	 * bridge, then the current device is effectively PCIe as it must
+	 * be the PCIe-to-PCI bridge.  This handles several bridges that
+	 * violate the PCIe spec by not exposing a PCIe capability:
+	 * https://bugzilla.kernel.org/show_bug.cgi?id=44881
+	 */
+	if (!pci_is_root_bus(pdev->bus)) {
+		struct pci_dev *parent = pdev->bus->self;
+
+		if (pci_is_pcie(parent) &&
+		    pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
+
+			return true;
+	}
+#endif
+	return false;
+}
+
+/*
+ * Walk upstream from the given pdev for the first device returning
+ * true for the provided match function.  If no match is found, return
+ * NULL.  *last records the previous step in the walk.
+ */
+struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
+					   bool (*match)(struct pci_dev *),
+					   struct pci_dev **last)
+{
+	*last = NULL;
+
+	if (match(pdev))
+		return pdev;
+
+	*last = pdev;
+
+	while (!pci_is_root_bus(pdev->bus)) {
+		*last = pdev;
+		pdev = pdev->bus->self;
+
+		if (match(pdev))
+			return pdev;
+	}
+
+	return NULL;
+}
+
 /*
  * find the upstream PCIe-to-PCI bridge of a PCI device
  * if the device is PCIE, return NULL
diff --git a/include/linux/pci.h b/include/linux/pci.h
index bd8ec30..e87423a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1855,6 +1855,29 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 #endif
 
 /**
+ * pci_walk_up_to_first_match - Generic upstream search function
+ * @pdev: starting PCI device to search
+ * @match: match function to call on each device (true = match)
+ * @last: last device examined prior to returned device
+ *
+ * Walk upstream from the given device, calling match() at each device.
+ * Returns the first device matching match().  If the root bus is reached
+ * without finding a match, return NULL.  last returns the N-1 step in
+ * the search path.
+ */
+struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
+					   bool (*match)(struct pci_dev *),
+					   struct pci_dev **last);
+
+/**
+ * pci_is_pcie_bridge - Match a PCIe bridge device
+ * @pdev: device to test
+ *
+ * Return true if the given device is a PCIe bridge, false otherwise.
+ */
+bool pci_is_pcie_bridge(struct pci_dev *pdev);
+
+/**
  * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
  * @pdev: the PCI device
  *

RE: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Sethi Varun-B16395]

Would these functions be used outside drivers/iommu? We recently added pci.h under drivers/iommu, maybe we can add a new file for these functions as well.

-Varun
> -----Original Message-----
> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> Sent: Saturday, May 11, 2013 2:49 AM
> To: bhelgaas@google.com; stephen@networkplumber.org
> Cc: linux-pci@vger.kernel.org; iommu@lists.linux-foundation.org;
> dwmw2@infradead.org
> Subject: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test
> 
> These will replace pci_find_upstream_pcie_bridge, which is difficult to
> use and rather specific to intel-iommu usage.  A quirked
> pci_is_pcie_bridge function is provided to work around non-compliant
> PCIe-to-PCI bridges such as those found in
> https://bugzilla.kernel.org/show_bug.cgi?id=44881
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/pci/search.c |   57
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h  |   23 ++++++++++++++++++++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c index
> d0627fa..0357f74 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -17,6 +17,63 @@
>  DECLARE_RWSEM(pci_bus_sem);
>  EXPORT_SYMBOL_GPL(pci_bus_sem);
> 
> +/* Test for PCIe bridges. */
> +bool pci_is_pcie_bridge(struct pci_dev *pdev) {
> +	if (!pci_is_bridge(pdev))
> +		return false;
> +
> +	if (pci_is_pcie(pdev))
> +		return true;
> +
> +#ifdef CONFIG_PCI_QUIRKS
> +	/*
> +	 * If we're not on the root bus, look one device upstream of the
> +	 * current device.  If that device is PCIe and is not a PCIe-to-PCI
> +	 * bridge, then the current device is effectively PCIe as it must
> +	 * be the PCIe-to-PCI bridge.  This handles several bridges that
> +	 * violate the PCIe spec by not exposing a PCIe capability:
> +	 * https://bugzilla.kernel.org/show_bug.cgi?id=44881
> +	 */
> +	if (!pci_is_root_bus(pdev->bus)) {
> +		struct pci_dev *parent = pdev->bus->self;
> +
> +		if (pci_is_pcie(parent) &&
> +		    pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
> +
> +			return true;
> +	}
> +#endif
> +	return false;
> +}
> +
> +/*
> + * Walk upstream from the given pdev for the first device returning
> + * true for the provided match function.  If no match is found, return
> + * NULL.  *last records the previous step in the walk.
> + */
> +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> +					   bool (*match)(struct pci_dev *),
> +					   struct pci_dev **last)
> +{
> +	*last = NULL;
> +
> +	if (match(pdev))
> +		return pdev;
> +
> +	*last = pdev;
> +
> +	while (!pci_is_root_bus(pdev->bus)) {
> +		*last = pdev;
> +		pdev = pdev->bus->self;
> +
> +		if (match(pdev))
> +			return pdev;
> +	}
> +
> +	return NULL;
> +}
> +
>  /*
>   * find the upstream PCIe-to-PCI bridge of a PCI device
>   * if the device is PCIE, return NULL
> diff --git a/include/linux/pci.h b/include/linux/pci.h index
> bd8ec30..e87423a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1855,6 +1855,29 @@ static inline struct eeh_dev
> *pci_dev_to_eeh_dev(struct pci_dev *pdev)  #endif
> 
>  /**
> + * pci_walk_up_to_first_match - Generic upstream search function
> + * @pdev: starting PCI device to search
> + * @match: match function to call on each device (true = match)
> + * @last: last device examined prior to returned device
> + *
> + * Walk upstream from the given device, calling match() at each device.
> + * Returns the first device matching match().  If the root bus is
> +reached
> + * without finding a match, return NULL.  last returns the N-1 step in
> + * the search path.
> + */
> +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> +					   bool (*match)(struct pci_dev *),
> +					   struct pci_dev **last);
> +
> +/**
> + * pci_is_pcie_bridge - Match a PCIe bridge device
> + * @pdev: device to test
> + *
> + * Return true if the given device is a PCIe bridge, false otherwise.
> + */
> +bool pci_is_pcie_bridge(struct pci_dev *pdev);
> +
> +/**
>   * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a
> device
>   * @pdev: the PCI device
>   *
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Alex Williamson]

On Mon, 2013-05-13 at 13:51 +0000, Sethi Varun-B16395 wrote:
> Would these functions be used outside drivers/iommu? We recently added pci.h under drivers/iommu, maybe we can add a new file for these functions as well.

The intention is to make them generic enough for pci-core, unlike
pci_find_upstream_pcie_bridge, which was rather specific to iommu usage.
Thanks,

Alex

> > -----Original Message-----
> > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> > Sent: Saturday, May 11, 2013 2:49 AM
> > To: bhelgaas@google.com; stephen@networkplumber.org
> > Cc: linux-pci@vger.kernel.org; iommu@lists.linux-foundation.org;
> > dwmw2@infradead.org
> > Subject: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test
> > 
> > These will replace pci_find_upstream_pcie_bridge, which is difficult to
> > use and rather specific to intel-iommu usage.  A quirked
> > pci_is_pcie_bridge function is provided to work around non-compliant
> > PCIe-to-PCI bridges such as those found in
> > https://bugzilla.kernel.org/show_bug.cgi?id=44881
> > 
> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  drivers/pci/search.c |   57
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/pci.h  |   23 ++++++++++++++++++++
> >  2 files changed, 80 insertions(+)
> > 
> > diff --git a/drivers/pci/search.c b/drivers/pci/search.c index
> > d0627fa..0357f74 100644
> > --- a/drivers/pci/search.c
> > +++ b/drivers/pci/search.c
> > @@ -17,6 +17,63 @@
> >  DECLARE_RWSEM(pci_bus_sem);
> >  EXPORT_SYMBOL_GPL(pci_bus_sem);
> > 
> > +/* Test for PCIe bridges. */
> > +bool pci_is_pcie_bridge(struct pci_dev *pdev) {
> > +	if (!pci_is_bridge(pdev))
> > +		return false;
> > +
> > +	if (pci_is_pcie(pdev))
> > +		return true;
> > +
> > +#ifdef CONFIG_PCI_QUIRKS
> > +	/*
> > +	 * If we're not on the root bus, look one device upstream of the
> > +	 * current device.  If that device is PCIe and is not a PCIe-to-PCI
> > +	 * bridge, then the current device is effectively PCIe as it must
> > +	 * be the PCIe-to-PCI bridge.  This handles several bridges that
> > +	 * violate the PCIe spec by not exposing a PCIe capability:
> > +	 * https://bugzilla.kernel.org/show_bug.cgi?id=44881
> > +	 */
> > +	if (!pci_is_root_bus(pdev->bus)) {
> > +		struct pci_dev *parent = pdev->bus->self;
> > +
> > +		if (pci_is_pcie(parent) &&
> > +		    pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
> > +
> > +			return true;
> > +	}
> > +#endif
> > +	return false;
> > +}
> > +
> > +/*
> > + * Walk upstream from the given pdev for the first device returning
> > + * true for the provided match function.  If no match is found, return
> > + * NULL.  *last records the previous step in the walk.
> > + */
> > +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> > +					   bool (*match)(struct pci_dev *),
> > +					   struct pci_dev **last)
> > +{
> > +	*last = NULL;
> > +
> > +	if (match(pdev))
> > +		return pdev;
> > +
> > +	*last = pdev;
> > +
> > +	while (!pci_is_root_bus(pdev->bus)) {
> > +		*last = pdev;
> > +		pdev = pdev->bus->self;
> > +
> > +		if (match(pdev))
> > +			return pdev;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> >  /*
> >   * find the upstream PCIe-to-PCI bridge of a PCI device
> >   * if the device is PCIE, return NULL
> > diff --git a/include/linux/pci.h b/include/linux/pci.h index
> > bd8ec30..e87423a 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1855,6 +1855,29 @@ static inline struct eeh_dev
> > *pci_dev_to_eeh_dev(struct pci_dev *pdev)  #endif
> > 
> >  /**
> > + * pci_walk_up_to_first_match - Generic upstream search function
> > + * @pdev: starting PCI device to search
> > + * @match: match function to call on each device (true = match)
> > + * @last: last device examined prior to returned device
> > + *
> > + * Walk upstream from the given device, calling match() at each device.
> > + * Returns the first device matching match().  If the root bus is
> > +reached
> > + * without finding a match, return NULL.  last returns the N-1 step in
> > + * the search path.
> > + */
> > +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> > +					   bool (*match)(struct pci_dev *),
> > +					   struct pci_dev **last);
> > +
> > +/**
> > + * pci_is_pcie_bridge - Match a PCIe bridge device
> > + * @pdev: device to test
> > + *
> > + * Return true if the given device is a PCIe bridge, false otherwise.
> > + */
> > +bool pci_is_pcie_bridge(struct pci_dev *pdev);
> > +
> > +/**
> >   * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a
> > device
> >   * @pdev: the PCI device
> >   *
> > 
> > _______________________________________________
> > iommu mailing list
> > iommu@lists.linux-foundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 
> 

RE: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Sethi Varun-B16395]

QW55IGlkZWEgd2hlbiB3b3VsZCB0aGlzIHBhdGNoIHNldCBiZSBpbnRlZ3JhdGVkPyBGcmVlc2Nh
bGUgUEFNVSBkcml2ZXIgZGVwZW5kcyBvbiB0aGlzIHBhdGNoLiBDdXJyZW50bHkgSSBhbSB1c2lu
ZyB0aGUgcGNpX2ZpbmRfcGNpZV91cHN0cmVhbV9icmlkZ2Ugcm91dGluZSBpbiB0aGUgZGV2aWNl
IGdyb3VwIGNyZWF0aW9uIHJvdXRpbmUuDQoNCi1WYXJ1bg0KDQo+IC0tLS0tT3JpZ2luYWwgTWVz
c2FnZS0tLS0tDQo+IEZyb206IEFsZXggV2lsbGlhbXNvbiBbbWFpbHRvOmFsZXgud2lsbGlhbXNv
bkByZWRoYXQuY29tXQ0KPiBTZW50OiBNb25kYXksIE1heSAxMywgMjAxMyA4OjE5IFBNDQo+IFRv
OiBTZXRoaSBWYXJ1bi1CMTYzOTUNCj4gQ2M6IGJoZWxnYWFzQGdvb2dsZS5jb207IHN0ZXBoZW5A
bmV0d29ya3BsdW1iZXIub3JnOyBsaW51eC0NCj4gcGNpQHZnZXIua2VybmVsLm9yZzsgaW9tbXVA
bGlzdHMubGludXgtZm91bmRhdGlvbi5vcmc7DQo+IGR3bXcyQGluZnJhZGVhZC5vcmcNCj4gU3Vi
amVjdDogUmU6IFtQQVRDSCAxLzNdIHBjaTogQWRkIFBDSSB3YWxrIGZ1bmN0aW9uIGFuZCBQQ0ll
IGJyaWRnZSB0ZXN0DQo+IA0KPiBPbiBNb24sIDIwMTMtMDUtMTMgYXQgMTM6NTEgKzAwMDAsIFNl
dGhpIFZhcnVuLUIxNjM5NSB3cm90ZToNCj4gPiBXb3VsZCB0aGVzZSBmdW5jdGlvbnMgYmUgdXNl
ZCBvdXRzaWRlIGRyaXZlcnMvaW9tbXU/IFdlIHJlY2VudGx5IGFkZGVkDQo+IHBjaS5oIHVuZGVy
IGRyaXZlcnMvaW9tbXUsIG1heWJlIHdlIGNhbiBhZGQgYSBuZXcgZmlsZSBmb3IgdGhlc2UNCj4g
ZnVuY3Rpb25zIGFzIHdlbGwuDQo+IA0KPiBUaGUgaW50ZW50aW9uIGlzIHRvIG1ha2UgdGhlbSBn
ZW5lcmljIGVub3VnaCBmb3IgcGNpLWNvcmUsIHVubGlrZQ0KPiBwY2lfZmluZF91cHN0cmVhbV9w
Y2llX2JyaWRnZSwgd2hpY2ggd2FzIHJhdGhlciBzcGVjaWZpYyB0byBpb21tdSB1c2FnZS4NCj4g
VGhhbmtzLA0KPiANCj4gQWxleA0KPiANCj4gPiA+IC0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0t
DQo+ID4gPiBGcm9tOiBpb21tdS1ib3VuY2VzQGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnIFtt
YWlsdG86aW9tbXUtDQo+ID4gPiBib3VuY2VzQGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnXSBP
biBCZWhhbGYgT2YgQWxleCBXaWxsaWFtc29uDQo+ID4gPiBTZW50OiBTYXR1cmRheSwgTWF5IDEx
LCAyMDEzIDI6NDkgQU0NCj4gPiA+IFRvOiBiaGVsZ2Fhc0Bnb29nbGUuY29tOyBzdGVwaGVuQG5l
dHdvcmtwbHVtYmVyLm9yZw0KPiA+ID4gQ2M6IGxpbnV4LXBjaUB2Z2VyLmtlcm5lbC5vcmc7IGlv
bW11QGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnOw0KPiA+ID4gZHdtdzJAaW5mcmFkZWFkLm9y
Zw0KPiA+ID4gU3ViamVjdDogW1BBVENIIDEvM10gcGNpOiBBZGQgUENJIHdhbGsgZnVuY3Rpb24g
YW5kIFBDSWUgYnJpZGdlIHRlc3QNCj4gPiA+DQo+ID4gPiBUaGVzZSB3aWxsIHJlcGxhY2UgcGNp
X2ZpbmRfdXBzdHJlYW1fcGNpZV9icmlkZ2UsIHdoaWNoIGlzIGRpZmZpY3VsdA0KPiA+ID4gdG8g
dXNlIGFuZCByYXRoZXIgc3BlY2lmaWMgdG8gaW50ZWwtaW9tbXUgdXNhZ2UuICBBIHF1aXJrZWQN
Cj4gPiA+IHBjaV9pc19wY2llX2JyaWRnZSBmdW5jdGlvbiBpcyBwcm92aWRlZCB0byB3b3JrIGFy
b3VuZCBub24tY29tcGxpYW50DQo+ID4gPiBQQ0llLXRvLVBDSSBicmlkZ2VzIHN1Y2ggYXMgdGhv
c2UgZm91bmQgaW4NCj4gPiA+IGh0dHBzOi8vYnVnemlsbGEua2VybmVsLm9yZy9zaG93X2J1Zy5j
Z2k/aWQ9NDQ4ODENCj4gPiA+DQo+ID4gPiBTaWduZWQtb2ZmLWJ5OiBBbGV4IFdpbGxpYW1zb24g
PGFsZXgud2lsbGlhbXNvbkByZWRoYXQuY29tPg0KPiA+ID4gLS0tDQo+ID4gPiAgZHJpdmVycy9w
Y2kvc2VhcmNoLmMgfCAgIDU3DQo+ID4gPiArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KysrKysrKysrKysrKysrKysrKw0KPiA+ID4gIGluY2x1ZGUvbGludXgvcGNpLmggIHwgICAyMyAr
KysrKysrKysrKysrKysrKysrKw0KPiA+ID4gIDIgZmlsZXMgY2hhbmdlZCwgODAgaW5zZXJ0aW9u
cygrKQ0KPiA+ID4NCj4gPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL3BjaS9zZWFyY2guYyBiL2Ry
aXZlcnMvcGNpL3NlYXJjaC5jIGluZGV4DQo+ID4gPiBkMDYyN2ZhLi4wMzU3Zjc0IDEwMDY0NA0K
PiA+ID4gLS0tIGEvZHJpdmVycy9wY2kvc2VhcmNoLmMNCj4gPiA+ICsrKyBiL2RyaXZlcnMvcGNp
L3NlYXJjaC5jDQo+ID4gPiBAQCAtMTcsNiArMTcsNjMgQEANCj4gPiA+ICBERUNMQVJFX1JXU0VN
KHBjaV9idXNfc2VtKTsNCj4gPiA+ICBFWFBPUlRfU1lNQk9MX0dQTChwY2lfYnVzX3NlbSk7DQo+
ID4gPg0KPiA+ID4gKy8qIFRlc3QgZm9yIFBDSWUgYnJpZGdlcy4gKi8NCj4gPiA+ICtib29sIHBj
aV9pc19wY2llX2JyaWRnZShzdHJ1Y3QgcGNpX2RldiAqcGRldikgew0KPiA+ID4gKwlpZiAoIXBj
aV9pc19icmlkZ2UocGRldikpDQo+ID4gPiArCQlyZXR1cm4gZmFsc2U7DQo+ID4gPiArDQo+ID4g
PiArCWlmIChwY2lfaXNfcGNpZShwZGV2KSkNCj4gPiA+ICsJCXJldHVybiB0cnVlOw0KPiA+ID4g
Kw0KPiA+ID4gKyNpZmRlZiBDT05GSUdfUENJX1FVSVJLUw0KPiA+ID4gKwkvKg0KPiA+ID4gKwkg
KiBJZiB3ZSdyZSBub3Qgb24gdGhlIHJvb3QgYnVzLCBsb29rIG9uZSBkZXZpY2UgdXBzdHJlYW0g
b2YgdGhlDQo+ID4gPiArCSAqIGN1cnJlbnQgZGV2aWNlLiAgSWYgdGhhdCBkZXZpY2UgaXMgUENJ
ZSBhbmQgaXMgbm90IGEgUENJZS10by1QQ0kNCj4gPiA+ICsJICogYnJpZGdlLCB0aGVuIHRoZSBj
dXJyZW50IGRldmljZSBpcyBlZmZlY3RpdmVseSBQQ0llIGFzIGl0IG11c3QNCj4gPiA+ICsJICog
YmUgdGhlIFBDSWUtdG8tUENJIGJyaWRnZS4gIFRoaXMgaGFuZGxlcyBzZXZlcmFsIGJyaWRnZXMg
dGhhdA0KPiA+ID4gKwkgKiB2aW9sYXRlIHRoZSBQQ0llIHNwZWMgYnkgbm90IGV4cG9zaW5nIGEg
UENJZSBjYXBhYmlsaXR5Og0KPiA+ID4gKwkgKiBodHRwczovL2J1Z3ppbGxhLmtlcm5lbC5vcmcv
c2hvd19idWcuY2dpP2lkPTQ0ODgxDQo+ID4gPiArCSAqLw0KPiA+ID4gKwlpZiAoIXBjaV9pc19y
b290X2J1cyhwZGV2LT5idXMpKSB7DQo+ID4gPiArCQlzdHJ1Y3QgcGNpX2RldiAqcGFyZW50ID0g
cGRldi0+YnVzLT5zZWxmOw0KPiA+ID4gKw0KPiA+ID4gKwkJaWYgKHBjaV9pc19wY2llKHBhcmVu
dCkgJiYNCj4gPiA+ICsJCSAgICBwY2lfcGNpZV90eXBlKHBhcmVudCkgIT0gUENJX0VYUF9UWVBF
X1BDSV9CUklER0UpDQo+ID4gPiArDQo+ID4gPiArCQkJcmV0dXJuIHRydWU7DQo+ID4gPiArCX0N
Cj4gPiA+ICsjZW5kaWYNCj4gPiA+ICsJcmV0dXJuIGZhbHNlOw0KPiA+ID4gK30NCj4gPiA+ICsN
Cj4gPiA+ICsvKg0KPiA+ID4gKyAqIFdhbGsgdXBzdHJlYW0gZnJvbSB0aGUgZ2l2ZW4gcGRldiBm
b3IgdGhlIGZpcnN0IGRldmljZSByZXR1cm5pbmcNCj4gPiA+ICsgKiB0cnVlIGZvciB0aGUgcHJv
dmlkZWQgbWF0Y2ggZnVuY3Rpb24uICBJZiBubyBtYXRjaCBpcyBmb3VuZCwNCj4gPiA+ICtyZXR1
cm4NCj4gPiA+ICsgKiBOVUxMLiAgKmxhc3QgcmVjb3JkcyB0aGUgcHJldmlvdXMgc3RlcCBpbiB0
aGUgd2Fsay4NCj4gPiA+ICsgKi8NCj4gPiA+ICtzdHJ1Y3QgcGNpX2RldiAqcGNpX3dhbGtfdXBf
dG9fZmlyc3RfbWF0Y2goc3RydWN0IHBjaV9kZXYgKnBkZXYsDQo+ID4gPiArCQkJCQkgICBib29s
ICgqbWF0Y2gpKHN0cnVjdCBwY2lfZGV2ICopLA0KPiA+ID4gKwkJCQkJICAgc3RydWN0IHBjaV9k
ZXYgKipsYXN0KQ0KPiA+ID4gK3sNCj4gPiA+ICsJKmxhc3QgPSBOVUxMOw0KPiA+ID4gKw0KPiA+
ID4gKwlpZiAobWF0Y2gocGRldikpDQo+ID4gPiArCQlyZXR1cm4gcGRldjsNCj4gPiA+ICsNCj4g
PiA+ICsJKmxhc3QgPSBwZGV2Ow0KPiA+ID4gKw0KPiA+ID4gKwl3aGlsZSAoIXBjaV9pc19yb290
X2J1cyhwZGV2LT5idXMpKSB7DQo+ID4gPiArCQkqbGFzdCA9IHBkZXY7DQo+ID4gPiArCQlwZGV2
ID0gcGRldi0+YnVzLT5zZWxmOw0KPiA+ID4gKw0KPiA+ID4gKwkJaWYgKG1hdGNoKHBkZXYpKQ0K
PiA+ID4gKwkJCXJldHVybiBwZGV2Ow0KPiA+ID4gKwl9DQo+ID4gPiArDQo+ID4gPiArCXJldHVy
biBOVUxMOw0KPiA+ID4gK30NCj4gPiA+ICsNCj4gPiA+ICAvKg0KPiA+ID4gICAqIGZpbmQgdGhl
IHVwc3RyZWFtIFBDSWUtdG8tUENJIGJyaWRnZSBvZiBhIFBDSSBkZXZpY2UNCj4gPiA+ICAgKiBp
ZiB0aGUgZGV2aWNlIGlzIFBDSUUsIHJldHVybiBOVUxMIGRpZmYgLS1naXQNCj4gPiA+IGEvaW5j
bHVkZS9saW51eC9wY2kuaCBiL2luY2x1ZGUvbGludXgvcGNpLmggaW5kZXggYmQ4ZWMzMC4uZTg3
NDIzYQ0KPiA+ID4gMTAwNjQ0DQo+ID4gPiAtLS0gYS9pbmNsdWRlL2xpbnV4L3BjaS5oDQo+ID4g
PiArKysgYi9pbmNsdWRlL2xpbnV4L3BjaS5oDQo+ID4gPiBAQCAtMTg1NSw2ICsxODU1LDI5IEBA
IHN0YXRpYyBpbmxpbmUgc3RydWN0IGVlaF9kZXYNCj4gPiA+ICpwY2lfZGV2X3RvX2VlaF9kZXYo
c3RydWN0IHBjaV9kZXYgKnBkZXYpICAjZW5kaWYNCj4gPiA+DQo+ID4gPiAgLyoqDQo+ID4gPiAr
ICogcGNpX3dhbGtfdXBfdG9fZmlyc3RfbWF0Y2ggLSBHZW5lcmljIHVwc3RyZWFtIHNlYXJjaCBm
dW5jdGlvbg0KPiA+ID4gKyAqIEBwZGV2OiBzdGFydGluZyBQQ0kgZGV2aWNlIHRvIHNlYXJjaA0K
PiA+ID4gKyAqIEBtYXRjaDogbWF0Y2ggZnVuY3Rpb24gdG8gY2FsbCBvbiBlYWNoIGRldmljZSAo
dHJ1ZSA9IG1hdGNoKQ0KPiA+ID4gKyAqIEBsYXN0OiBsYXN0IGRldmljZSBleGFtaW5lZCBwcmlv
ciB0byByZXR1cm5lZCBkZXZpY2UNCj4gPiA+ICsgKg0KPiA+ID4gKyAqIFdhbGsgdXBzdHJlYW0g
ZnJvbSB0aGUgZ2l2ZW4gZGV2aWNlLCBjYWxsaW5nIG1hdGNoKCkgYXQgZWFjaA0KPiBkZXZpY2Uu
DQo+ID4gPiArICogUmV0dXJucyB0aGUgZmlyc3QgZGV2aWNlIG1hdGNoaW5nIG1hdGNoKCkuICBJ
ZiB0aGUgcm9vdCBidXMgaXMNCj4gPiA+ICtyZWFjaGVkDQo+ID4gPiArICogd2l0aG91dCBmaW5k
aW5nIGEgbWF0Y2gsIHJldHVybiBOVUxMLiAgbGFzdCByZXR1cm5zIHRoZSBOLTEgc3RlcA0KPiA+
ID4gK2luDQo+ID4gPiArICogdGhlIHNlYXJjaCBwYXRoLg0KPiA+ID4gKyAqLw0KPiA+ID4gK3N0
cnVjdCBwY2lfZGV2ICpwY2lfd2Fsa191cF90b19maXJzdF9tYXRjaChzdHJ1Y3QgcGNpX2RldiAq
cGRldiwNCj4gPiA+ICsJCQkJCSAgIGJvb2wgKCptYXRjaCkoc3RydWN0IHBjaV9kZXYgKiksDQo+
ID4gPiArCQkJCQkgICBzdHJ1Y3QgcGNpX2RldiAqKmxhc3QpOw0KPiA+ID4gKw0KPiA+ID4gKy8q
Kg0KPiA+ID4gKyAqIHBjaV9pc19wY2llX2JyaWRnZSAtIE1hdGNoIGEgUENJZSBicmlkZ2UgZGV2
aWNlDQo+ID4gPiArICogQHBkZXY6IGRldmljZSB0byB0ZXN0DQo+ID4gPiArICoNCj4gPiA+ICsg
KiBSZXR1cm4gdHJ1ZSBpZiB0aGUgZ2l2ZW4gZGV2aWNlIGlzIGEgUENJZSBicmlkZ2UsIGZhbHNl
DQo+IG90aGVyd2lzZS4NCj4gPiA+ICsgKi8NCj4gPiA+ICtib29sIHBjaV9pc19wY2llX2JyaWRn
ZShzdHJ1Y3QgcGNpX2RldiAqcGRldik7DQo+ID4gPiArDQo+ID4gPiArLyoqDQo+ID4gPiAgICog
cGNpX2ZpbmRfdXBzdHJlYW1fcGNpZV9icmlkZ2UgLSBmaW5kIHVwc3RyZWFtIFBDSWUtdG8tUENJ
IGJyaWRnZQ0KPiA+ID4gb2YgYSBkZXZpY2UNCj4gPiA+ICAgKiBAcGRldjogdGhlIFBDSSBkZXZp
Y2UNCj4gPiA+ICAgKg0KPiA+ID4NCj4gPiA+IF9fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fX19fX19fX19fDQo+ID4gPiBpb21tdSBtYWlsaW5nIGxpc3QNCj4gPiA+IGlvbW11
QGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnDQo+ID4gPiBodHRwczovL2xpc3RzLmxpbnV4Zm91
bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9pb21tdQ0KPiA+DQo+ID4NCj4gDQo+IA0KPiAN
Cg0K

Re: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Alex Williamson]

On Wed, 2013-05-22 at 14:34 +0000, Sethi Varun-B16395 wrote:
> Any idea when would this patch set be integrated? Freescale PAMU
> driver depends on this patch. Currently I am using the
> pci_find_pcie_upstream_bridge routine in the device group creation
> routine.

I don't know, Bjorn?  The bugzilla referenced in patch 0/3 has several
reports from users that this series solves the problem for them.
Thanks,

Alex

> > -----Original Message-----
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Monday, May 13, 2013 8:19 PM
> > To: Sethi Varun-B16395
> > Cc: bhelgaas@google.com; stephen@networkplumber.org; linux-
> > pci@vger.kernel.org; iommu@lists.linux-foundation.org;
> > dwmw2@infradead.org
> > Subject: Re: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test
> > 
> > On Mon, 2013-05-13 at 13:51 +0000, Sethi Varun-B16395 wrote:
> > > Would these functions be used outside drivers/iommu? We recently added
> > pci.h under drivers/iommu, maybe we can add a new file for these
> > functions as well.
> > 
> > The intention is to make them generic enough for pci-core, unlike
> > pci_find_upstream_pcie_bridge, which was rather specific to iommu usage.
> > Thanks,
> > 
> > Alex
> > 
> > > > -----Original Message-----
> > > > From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> > > > bounces@lists.linux-foundation.org] On Behalf Of Alex Williamson
> > > > Sent: Saturday, May 11, 2013 2:49 AM
> > > > To: bhelgaas@google.com; stephen@networkplumber.org
> > > > Cc: linux-pci@vger.kernel.org; iommu@lists.linux-foundation.org;
> > > > dwmw2@infradead.org
> > > > Subject: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test
> > > >
> > > > These will replace pci_find_upstream_pcie_bridge, which is difficult
> > > > to use and rather specific to intel-iommu usage.  A quirked
> > > > pci_is_pcie_bridge function is provided to work around non-compliant
> > > > PCIe-to-PCI bridges such as those found in
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=44881
> > > >
> > > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > > > ---
> > > >  drivers/pci/search.c |   57
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/linux/pci.h  |   23 ++++++++++++++++++++
> > > >  2 files changed, 80 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/search.c b/drivers/pci/search.c index
> > > > d0627fa..0357f74 100644
> > > > --- a/drivers/pci/search.c
> > > > +++ b/drivers/pci/search.c
> > > > @@ -17,6 +17,63 @@
> > > >  DECLARE_RWSEM(pci_bus_sem);
> > > >  EXPORT_SYMBOL_GPL(pci_bus_sem);
> > > >
> > > > +/* Test for PCIe bridges. */
> > > > +bool pci_is_pcie_bridge(struct pci_dev *pdev) {
> > > > +	if (!pci_is_bridge(pdev))
> > > > +		return false;
> > > > +
> > > > +	if (pci_is_pcie(pdev))
> > > > +		return true;
> > > > +
> > > > +#ifdef CONFIG_PCI_QUIRKS
> > > > +	/*
> > > > +	 * If we're not on the root bus, look one device upstream of the
> > > > +	 * current device.  If that device is PCIe and is not a PCIe-to-PCI
> > > > +	 * bridge, then the current device is effectively PCIe as it must
> > > > +	 * be the PCIe-to-PCI bridge.  This handles several bridges that
> > > > +	 * violate the PCIe spec by not exposing a PCIe capability:
> > > > +	 * https://bugzilla.kernel.org/show_bug.cgi?id=44881
> > > > +	 */
> > > > +	if (!pci_is_root_bus(pdev->bus)) {
> > > > +		struct pci_dev *parent = pdev->bus->self;
> > > > +
> > > > +		if (pci_is_pcie(parent) &&
> > > > +		    pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
> > > > +
> > > > +			return true;
> > > > +	}
> > > > +#endif
> > > > +	return false;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Walk upstream from the given pdev for the first device returning
> > > > + * true for the provided match function.  If no match is found,
> > > > +return
> > > > + * NULL.  *last records the previous step in the walk.
> > > > + */
> > > > +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> > > > +					   bool (*match)(struct pci_dev *),
> > > > +					   struct pci_dev **last)
> > > > +{
> > > > +	*last = NULL;
> > > > +
> > > > +	if (match(pdev))
> > > > +		return pdev;
> > > > +
> > > > +	*last = pdev;
> > > > +
> > > > +	while (!pci_is_root_bus(pdev->bus)) {
> > > > +		*last = pdev;
> > > > +		pdev = pdev->bus->self;
> > > > +
> > > > +		if (match(pdev))
> > > > +			return pdev;
> > > > +	}
> > > > +
> > > > +	return NULL;
> > > > +}
> > > > +
> > > >  /*
> > > >   * find the upstream PCIe-to-PCI bridge of a PCI device
> > > >   * if the device is PCIE, return NULL diff --git
> > > > a/include/linux/pci.h b/include/linux/pci.h index bd8ec30..e87423a
> > > > 100644
> > > > --- a/include/linux/pci.h
> > > > +++ b/include/linux/pci.h
> > > > @@ -1855,6 +1855,29 @@ static inline struct eeh_dev
> > > > *pci_dev_to_eeh_dev(struct pci_dev *pdev)  #endif
> > > >
> > > >  /**
> > > > + * pci_walk_up_to_first_match - Generic upstream search function
> > > > + * @pdev: starting PCI device to search
> > > > + * @match: match function to call on each device (true = match)
> > > > + * @last: last device examined prior to returned device
> > > > + *
> > > > + * Walk upstream from the given device, calling match() at each
> > device.
> > > > + * Returns the first device matching match().  If the root bus is
> > > > +reached
> > > > + * without finding a match, return NULL.  last returns the N-1 step
> > > > +in
> > > > + * the search path.
> > > > + */
> > > > +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> > > > +					   bool (*match)(struct pci_dev *),
> > > > +					   struct pci_dev **last);
> > > > +
> > > > +/**
> > > > + * pci_is_pcie_bridge - Match a PCIe bridge device
> > > > + * @pdev: device to test
> > > > + *
> > > > + * Return true if the given device is a PCIe bridge, false
> > otherwise.
> > > > + */
> > > > +bool pci_is_pcie_bridge(struct pci_dev *pdev);
> > > > +
> > > > +/**
> > > >   * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge
> > > > of a device
> > > >   * @pdev: the PCI device
> > > >   *
> > > >
> > > > _______________________________________________
> > > > iommu mailing list
> > > > iommu@lists.linux-foundation.org
> > > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> > >
> > >
> > 
> > 
> > 
> 

Re: [PATCH 1/3] pci: Add PCI walk function and PCIe bridge test

[Bjorn Helgaas]

On Fri, May 10, 2013 at 3:18 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> These will replace pci_find_upstream_pcie_bridge, which is difficult
> to use and rather specific to intel-iommu usage.  A quirked
> pci_is_pcie_bridge function is provided to work around non-compliant
> PCIe-to-PCI bridges such as those found in
> https://bugzilla.kernel.org/show_bug.cgi?id=44881
>
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/pci/search.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h  |   23 ++++++++++++++++++++
>  2 files changed, 80 insertions(+)
>
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index d0627fa..0357f74 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -17,6 +17,63 @@
>  DECLARE_RWSEM(pci_bus_sem);
>  EXPORT_SYMBOL_GPL(pci_bus_sem);
>
> +/* Test for PCIe bridges. */
> +bool pci_is_pcie_bridge(struct pci_dev *pdev)
> +{
> +       if (!pci_is_bridge(pdev))
> +               return false;
> +
> +       if (pci_is_pcie(pdev))
> +               return true;
> +
> +#ifdef CONFIG_PCI_QUIRKS
> +       /*
> +        * If we're not on the root bus, look one device upstream of the
> +        * current device.  If that device is PCIe and is not a PCIe-to-PCI
> +        * bridge, then the current device is effectively PCIe as it must
> +        * be the PCIe-to-PCI bridge.  This handles several bridges that
> +        * violate the PCIe spec by not exposing a PCIe capability:
> +        * https://bugzilla.kernel.org/show_bug.cgi?id=44881
> +        */
> +       if (!pci_is_root_bus(pdev->bus)) {
> +               struct pci_dev *parent = pdev->bus->self;
> +
> +               if (pci_is_pcie(parent) &&
> +                   pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
> +
> +                       return true;
> +       }
> +#endif
> +       return false;
> +}

I like this strategy.  But I'd rather it not be a general-purpose PCI
interface, because if pci_is_pcie_bridge() is true, people will assume
they can perform PCIe operations on the device, and they can't.  The
only use for this is to figure out the source ID the IOMMU will see,
so I think this should just go in the IOMMU code.

> +/*
> + * Walk upstream from the given pdev for the first device returning
> + * true for the provided match function.  If no match is found, return
> + * NULL.  *last records the previous step in the walk.
> + */
> +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> +                                          bool (*match)(struct pci_dev *),
> +                                          struct pci_dev **last)
> +{
> +       *last = NULL;
> +
> +       if (match(pdev))
> +               return pdev;
> +
> +       *last = pdev;
> +
> +       while (!pci_is_root_bus(pdev->bus)) {
> +               *last = pdev;
> +               pdev = pdev->bus->self;
> +
> +               if (match(pdev))
> +                       return pdev;
> +       }
> +
> +       return NULL;
> +}

Same here.  I don't really see much potential for other uses of this,
so it seems like you might as well just put this in the IOMMU code and
make it call pci_is_pcie_bridge() directly.

The "source ID == upstream PCIe bridge" mapping is deeply ingrained in
your skull, but I think it would make the intent of the code clearer
if the function names mentioned the source ID somehow.  Otherwise new
readers like me have to come up with that association on our own.  But
since I'm proposing putting all this in the IOMMU code, it's totally
up to you :)

Bjorn

> +
>  /*
>   * find the upstream PCIe-to-PCI bridge of a PCI device
>   * if the device is PCIE, return NULL
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index bd8ec30..e87423a 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1855,6 +1855,29 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
>  #endif
>
>  /**
> + * pci_walk_up_to_first_match - Generic upstream search function
> + * @pdev: starting PCI device to search
> + * @match: match function to call on each device (true = match)
> + * @last: last device examined prior to returned device
> + *
> + * Walk upstream from the given device, calling match() at each device.
> + * Returns the first device matching match().  If the root bus is reached
> + * without finding a match, return NULL.  last returns the N-1 step in
> + * the search path.
> + */
> +struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
> +                                          bool (*match)(struct pci_dev *),
> +                                          struct pci_dev **last);
> +
> +/**
> + * pci_is_pcie_bridge - Match a PCIe bridge device
> + * @pdev: device to test
> + *
> + * Return true if the given device is a PCIe bridge, false otherwise.
> + */
> +bool pci_is_pcie_bridge(struct pci_dev *pdev);
> +
> +/**
>   * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
>   * @pdev: the PCI device
>   *
>

[PATCH 2/3] intel-iommu: Convert to pci_walk_up_to_first_match

[Alex Williamson]

pci_find_upstream_pcie_bridge() attempts to return the PCIe-to-PCI
bridge upstream from the provided device.  It currently returns NULL
if 1) the provided device is PCIe (ie. it does not have such an
upstream bridge), 2) the device is on the root bus (a subtle case of
its claim to return the parent device when not connected to a PCIe
bridge), or 3) when it gets lost walking the bus and finds itself on
a PCIe device that is not a PCIe-to-PCI bridge.  This latter case not
only generates a WARN, but treats the provided device the same as if
it were PCIe.

We can replace 1) and 2) with explicit tests in the IOMMU code as this
is the code that knows the visibility of devices at the root bus.
That leaves the bus walk case 3), where the NULL return is actually an
error.

We hit this error because some PCIe-to-PCI bridge vendors ignore the
PCIe spec requirement that all PCIe devices must include a PCIe
capability, which is all that pci_is_pcie() actually tests.  When
quirks are enabled, pci_is_pcie_bridge() adds one more test by looking
at the next upstream device to test whether it is PCIe with a type
code of a PCIe-to-PCI/PCI-X bridge.  If it does not have this type
code, then by deduction, the current bridge must be a PCIe-to-PCI
bridge and thus be a PCIe bridge.

Therefore, pci_find_upstream_pcie_bridge() is replaced by an
explicit call to (pci_is_pcie() || pci_is_root_bus()) plus
pci_walk_up_to_first_match() with the pci_is_pcie_bridge() match
function.  This either returns the first PCIe bridge, which must
be a PCIe-to-PCI bridge or NULL if not found.  In the NULL case, we
know the original device is not on the root bus (already tested for
that), so we can safely walk from the provided device to the last
device and fall into the legacy PCI bridge code.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/iommu/intel-iommu.c         |   77 ++++++++++++++++++++++-------------
 drivers/iommu/intel_irq_remapping.c |   13 ++++--
 2 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0099667..277c18f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1688,9 +1688,13 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev,
 		return ret;
 
 	/* dependent device mapping */
-	tmp = pci_find_upstream_pcie_bridge(pdev);
-	if (!tmp)
+	if (pci_is_pcie(pdev) || pci_is_root_bus(pdev->bus))
 		return 0;
+
+	tmp = pci_walk_up_to_first_match(pdev, pci_is_pcie_bridge, &parent);
+	if (!tmp)
+		tmp = parent;
+
 	/* Secondary interface's bus number and devfn 0 */
 	parent = pdev->bus->self;
 	while (parent != tmp) {
@@ -1702,7 +1706,7 @@ domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev,
 			return ret;
 		parent = parent->bus->self;
 	}
-	if (pci_is_pcie(tmp)) /* this is a PCIe-to-PCI bridge */
+	if (pci_is_pcie_bridge(tmp)) /* this is a PCIe-to-PCI bridge */
 		return domain_context_mapping_one(domain,
 					pci_domain_nr(tmp->subordinate),
 					tmp->subordinate->number, 0,
@@ -1729,10 +1733,15 @@ static int domain_context_mapped(struct pci_dev *pdev)
 	ret = device_context_mapped(iommu, pdev->bus->number, pdev->devfn);
 	if (!ret)
 		return ret;
+
 	/* dependent device mapping */
-	tmp = pci_find_upstream_pcie_bridge(pdev);
+	if (pci_is_pcie(pdev) || pci_is_root_bus(pdev->bus))
+		return 0;
+
+	tmp = pci_walk_up_to_first_match(pdev, pci_is_pcie_bridge, &parent);
 	if (!tmp)
-		return ret;
+		tmp = parent;
+
 	/* Secondary interface's bus number and devfn 0 */
 	parent = pdev->bus->self;
 	while (parent != tmp) {
@@ -1742,7 +1751,7 @@ static int domain_context_mapped(struct pci_dev *pdev)
 			return ret;
 		parent = parent->bus->self;
 	}
-	if (pci_is_pcie(tmp))
+	if (pci_is_pcie_bridge(tmp))
 		return device_context_mapped(iommu, tmp->subordinate->number,
 					     0);
 	else
@@ -1973,7 +1982,7 @@ static struct dmar_domain *get_domain_for_dev(struct pci_dev *pdev, int gaw)
 	struct intel_iommu *iommu;
 	struct dmar_drhd_unit *drhd;
 	struct device_domain_info *info, *tmp;
-	struct pci_dev *dev_tmp;
+	struct pci_dev *dev_tmp = NULL;
 	unsigned long flags;
 	int bus = 0, devfn = 0;
 	int segment;
@@ -1985,9 +1994,15 @@ static struct dmar_domain *get_domain_for_dev(struct pci_dev *pdev, int gaw)
 
 	segment = pci_domain_nr(pdev->bus);
 
-	dev_tmp = pci_find_upstream_pcie_bridge(pdev);
-	if (dev_tmp) {
-		if (pci_is_pcie(dev_tmp)) {
+	if (!pci_is_pcie(pdev) && !pci_is_root_bus(pdev->bus)) {
+		struct pci_dev *last;
+
+		dev_tmp = pci_walk_up_to_first_match(pdev, pci_is_pcie_bridge,
+						     &last);
+		if (!dev_tmp)
+			dev_tmp = last;
+
+		if (pci_is_pcie_bridge(dev_tmp)) {
 			bus = dev_tmp->subordinate->number;
 			devfn = 0;
 		} else {
@@ -3738,26 +3753,24 @@ static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 {
 	struct pci_dev *tmp, *parent;
 
-	if (!iommu || !pdev)
+	if (!iommu || !pdev || pci_is_pcie(pdev) || pci_is_root_bus(pdev->bus))
 		return;
 
 	/* dependent device detach */
-	tmp = pci_find_upstream_pcie_bridge(pdev);
+	tmp = pci_walk_up_to_first_match(pdev, pci_is_pcie_bridge, &parent);
+	if (!tmp)
+		tmp = parent;
+
 	/* Secondary interface's bus number and devfn 0 */
-	if (tmp) {
-		parent = pdev->bus->self;
-		while (parent != tmp) {
-			iommu_detach_dev(iommu, parent->bus->number,
-					 parent->devfn);
-			parent = parent->bus->self;
-		}
-		if (pci_is_pcie(tmp)) /* this is a PCIe-to-PCI bridge */
-			iommu_detach_dev(iommu,
-				tmp->subordinate->number, 0);
-		else /* this is a legacy PCI bridge */
-			iommu_detach_dev(iommu, tmp->bus->number,
-					 tmp->devfn);
+	parent = pdev->bus->self;
+	while (parent != tmp) {
+		iommu_detach_dev(iommu, parent->bus->number, parent->devfn);
+		parent = parent->bus->self;
 	}
+	if (pci_is_pcie_bridge(tmp)) /* this is a PCIe-to-PCI bridge */
+		iommu_detach_dev(iommu, tmp->subordinate->number, 0);
+	else /* this is a legacy PCI bridge */
+		iommu_detach_dev(iommu, tmp->bus->number, tmp->devfn);
 }
 
 static void domain_remove_one_dev_info(struct dmar_domain *domain,
@@ -4148,7 +4161,7 @@ static void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
 static int intel_iommu_add_device(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
-	struct pci_dev *bridge, *dma_pdev = NULL;
+	struct pci_dev *dma_pdev = NULL;
 	struct iommu_group *group;
 	int ret;
 
@@ -4156,9 +4169,15 @@ static int intel_iommu_add_device(struct device *dev)
 			     pdev->bus->number, pdev->devfn))
 		return -ENODEV;
 
-	bridge = pci_find_upstream_pcie_bridge(pdev);
-	if (bridge) {
-		if (pci_is_pcie(bridge))
+	if (!pci_is_pcie(pdev) && !pci_is_root_bus(pdev->bus)) {
+		struct pci_dev *bridge, *last;
+
+		bridge = pci_walk_up_to_first_match(pdev, pci_is_pcie_bridge,
+						    &last);
+		if (!bridge)
+			bridge = last;
+
+		if (pci_is_pcie_bridge(bridge))
 			dma_pdev = pci_get_domain_bus_and_slot(
 						pci_domain_nr(pdev->bus),
 						bridge->subordinate->number, 0);
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index f3b8f23..873da76 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -373,7 +373,6 @@ static int set_hpet_sid(struct irte *irte, u8 id)
 
 static int set_msi_sid(struct irte *irte, struct pci_dev *dev)
 {
-	struct pci_dev *bridge;
 
 	if (!irte || !dev)
 		return -1;
@@ -385,9 +384,15 @@ static int set_msi_sid(struct irte *irte, struct pci_dev *dev)
 		return 0;
 	}
 
-	bridge = pci_find_upstream_pcie_bridge(dev);
-	if (bridge) {
-		if (pci_is_pcie(bridge))/* this is a PCIe-to-PCI/PCIX bridge */
+	if (!pci_is_pcie(dev) && !pci_is_root_bus(dev->bus)) {
+		struct pci_dev *bridge, *last;
+
+		bridge = pci_walk_up_to_first_match(dev,
+						    pci_is_pcie_bridge, &last);
+		if (!bridge)
+			bridge = last;
+
+		if (pci_is_pcie_bridge(bridge)) /* PCIe-to-PCI/PCIX bridge */
 			set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
 				(bridge->bus->number << 8) | dev->bus->number);
 		else /* this is a legacy PCI bridge */

[PATCH 3/3] pci: Remove pci_find_pcie_upstream_bridge

[Alex Williamson]

This function no longer has any users.

Signed-off-by: Alex williamson <alex.williamson@redhat.com>
---
 drivers/pci/search.c |   35 -----------------------------------
 include/linux/pci.h  |   11 -----------
 2 files changed, 46 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 0357f74..bdbff05 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -74,41 +74,6 @@ struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
 	return NULL;
 }
 
-/*
- * find the upstream PCIe-to-PCI bridge of a PCI device
- * if the device is PCIE, return NULL
- * if the device isn't connected to a PCIe bridge (that is its parent is a
- * legacy PCI bridge and the bridge is directly connected to bus 0), return its
- * parent
- */
-struct pci_dev *
-pci_find_upstream_pcie_bridge(struct pci_dev *pdev)
-{
-	struct pci_dev *tmp = NULL;
-
-	if (pci_is_pcie(pdev))
-		return NULL;
-	while (1) {
-		if (pci_is_root_bus(pdev->bus))
-			break;
-		pdev = pdev->bus->self;
-		/* a p2p bridge */
-		if (!pci_is_pcie(pdev)) {
-			tmp = pdev;
-			continue;
-		}
-		/* PCI device should connect to a PCIe bridge */
-		if (pci_pcie_type(pdev) != PCI_EXP_TYPE_PCI_BRIDGE) {
-			/* Busted hardware? */
-			WARN_ON_ONCE(1);
-			return NULL;
-		}
-		return pdev;
-	}
-
-	return tmp;
-}
-
 static struct pci_bus *pci_do_find_bus(struct pci_bus *bus, unsigned char busnr)
 {
 	struct pci_bus* child;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e87423a..9d9dcf5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1877,15 +1877,4 @@ struct pci_dev *pci_walk_up_to_first_match(struct pci_dev *pdev,
  */
 bool pci_is_pcie_bridge(struct pci_dev *pdev);
 
-/**
- * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
- * @pdev: the PCI device
- *
- * if the device is PCIE, return NULL
- * if the device isn't connected to a PCIe bridge (that is its parent is a
- * legacy PCI bridge and the bridge is directly connected to bus 0), return its
- * parent
- */
-struct pci_dev *pci_find_upstream_pcie_bridge(struct pci_dev *pdev);
-
 #endif /* LINUX_PCI_H */