[RFC PATCH 0/2] pci/iommu: PCIe requester ID interface

[RFC PATCH 0/2] pci/iommu: PCIe requester ID interface

[Alex Williamson]

I took Bjorn's requester ID interface, or more appropriately my
interpretation of the interface, through a full (apparently) working
implementation.  This is the result.  It's quite a nice cleanup for
intel-iommu.  I think this kills two birds with one stone, we quirk
PCIe-to-PCI bridges that don't have a PCIe capability (bz44881) and we
also apply DMA quirks to all devices for dma_ops, which should fix
devices like Ricoh and Marvell chips that do DMA from the wrong
requester ID (still no ghost requester ID support, but that may be
easier to add now).

If this seems like the right direction I'll update other users of
pci_find_upstream_pcie_bridge() and abolish that function.  Thanks,

Alex

---

Alex Williamson (2):
      pci: Create PCIe requester ID interface
      iommu/intel: Make use of PCIe requester ID interface


 drivers/iommu/intel-iommu.c |  164 ++++++++++++++++-------------------------
 drivers/pci/search.c        |  170 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h         |    7 ++

[RFC PATCH 1/2] pci: Create PCIe requester ID interface

[Alex Williamson]

This provides interfaces for drivers to discover the visible PCIe
requester ID for a device, for things like IOMMU setup, and iterate
over the device chain from requestee to requester, including DMA
quirks at each step.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/search.c |  170 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h  |    7 ++
 2 files changed, 177 insertions(+)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index d0627fa..2cca84b 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -18,6 +18,176 @@ DECLARE_RWSEM(pci_bus_sem);
 EXPORT_SYMBOL_GPL(pci_bus_sem);
 
 /*
+ * pci_has_pcie_requester_id - Does @dev have a PCIe requester ID
+ * @dev: device to test
+ */
+static bool pci_has_pcie_requester_id(struct pci_dev *dev)
+{
+	/*
+	 * XXX There's no indicator of the bus type, conventional PCI vs
+	 * PCI-X vs PCI-e, but we assume that a caller looking for a PCIe
+	 * requester ID is a native PCIe based system (such as VT-d or
+	 * AMD-Vi).  It's common that PCIe root complex devices do not
+	 * include a PCIe capability, but we can assume they are PCIe
+	 * devices based on their topology.
+	 */
+	if (pci_is_pcie(dev) || pci_is_root_bus(dev->bus))
+		return true;
+
+	/*
+	 * PCI-X devices have a requester ID, but the bridge may still take
+	 * ownership of transactions and create a requester ID.  We therefore
+	 * assume that the PCI-X requester ID is not the same one used on PCIe.
+	 */
+
+#ifdef CONFIG_PCI_QUIRKS
+	/*
+	 * Quirk for PCIe-to-PCI bridges which do not expose a PCIe capability.
+	 * If the device is a bridge, look to the next device upstream of it.
+	 * If that device is PCIe and not a PCIe-to-PCI bridge, then by
+	 * deduction, the device must be PCIe and therefore has a requester ID.
+	 */
+	if (dev->subordinate) {
+		struct pci_dev *parent = dev->bus->self;
+
+		if (pci_is_pcie(parent) &&
+		    pci_pcie_type(parent) != PCI_EXP_TYPE_PCI_BRIDGE)
+			return true;
+	}
+#endif
+
+	return false;
+}
+
+/*
+ * pci_has_visible_pcie_requester_id - Can @bridge see @dev's requester ID?
+ * @dev: requester device
+ * @bridge: upstream bridge (or NULL for root bus)
+ */
+static bool pci_has_visible_pcie_requester_id(struct pci_dev *dev,
+					      struct pci_dev *bridge)
+{
+	/*
+	 * The entire path must be tested, if any step does not have a
+	 * requester ID, the chain is broken.  This allows us to support
+	 * topologies with PCIe requester ID gaps, ex: PCIe-PCI-PCIe
+	 */
+	while (dev != bridge) {
+		if (!pci_has_pcie_requester_id(dev))
+			return false;
+
+		if (pci_is_root_bus(dev->bus))
+			return !bridge; /* false if we don't hit @bridge */
+
+		dev = dev->bus->self;
+	}
+
+	return true;
+}
+
+/*
+ * pci_get_visible_pcie_requester - Get requester for @requestee below @bridge
+ * @requestee: requester device
+ * @bridge: upstream bridge (or NULL for root bus)
+ */
+struct pci_dev *pci_get_visible_pcie_requester(struct pci_dev *requestee,
+					       struct pci_dev *bridge)
+{
+	struct pci_dev *requester = requestee;
+
+	while (requester != bridge) {
+		requester = pci_get_dma_source(requester);
+		pci_dev_put(requester); /* XXX skip ref cnt */
+
+		if (pci_has_visible_pcie_requester_id(requester, bridge))
+			return requester;
+
+		if (pci_is_root_bus(requester->bus))
+			return NULL; /* @bridge not parent to @requestee */
+
+		requester = requester->bus->self;
+	}
+
+	return requester;
+}
+
+#define PCI_REQUESTER_ID(dev)	(((dev)->bus->number << 8) | (dev)->devfn)
+#define PCI_BRIDGE_REQUESTER_ID(dev)	((dev)->subordinate->number << 8)
+
+u16 pci_requester_id(struct pci_dev *requester, struct pci_dev *requestee)
+{
+	if ((requester == requestee) ||
+	    (!pci_is_pcie(requester) && pci_is_root_bus(requester->bus)))
+		return PCI_REQUESTER_ID(requester);
+
+	return PCI_BRIDGE_REQUESTER_ID(requester);
+}
+
+static int pci_do_requester_callback(struct pci_dev *dev,
+				     int (*fn)(struct pci_dev *,
+					       u16 id, void *),
+				     void *data)
+{
+	struct pci_dev *dma_dev;
+	int ret;
+
+	ret = fn(dev, PCI_REQUESTER_ID(dev), data);
+	if (ret)
+		return ret;
+
+	dma_dev = pci_get_dma_source(dev);
+	pci_dev_put(dma_dev); /* XXX skip ref cnt */
+	if (dma_dev == dev)
+		return 0;
+
+	return fn(dma_dev, PCI_REQUESTER_ID(dma_dev), data);
+}
+
+/*
+ * pcie_for_each_requester - Call callback @fn on each devices and DMA source
+ *                           from @requestee to the PCIe requester ID visible
+ *                           to @bridge.
+ * @requestee: Starting device
+ * @bridge: upstream bridge (or NULL for root bus)
+ * @fn: callback function
+ * @data: data to pass to callback
+ */
+int pcie_for_each_requester(struct pci_dev *requestee, struct pci_dev *bridge,
+			    int (*fn)(struct pci_dev *, u16 id, void *),
+			    void *data)
+{
+	struct pci_dev *requester;
+	struct pci_dev *dev;
+	int ret = 0;
+
+	requester = pci_get_visible_pcie_requester(requestee, bridge);
+	if (!requester)
+		return -EINVAL;
+
+	for (dev = requestee; dev != requester; dev = dev->bus->self) {
+		ret = pci_do_requester_callback(dev, fn, data);
+		if (ret)
+			return ret;
+
+		if (pci_is_root_bus(dev->bus))
+			return -EINVAL;
+	}
+
+	/*
+	 * If the requester is not the same as the requestee, then the
+	 * requester is a bridge and uses the bridge requester ID.  However,
+	 * an exception to this rule is if the bridge is a legacy PCI bridge
+	 * attached to the root complex.  In this case, we handle it like
+	 * any other device.
+	 */
+	if (requester != requestee &&
+	    !(!pci_is_pcie(requester) && pci_is_root_bus(requester->bus)))
+		return fn(requester, PCI_BRIDGE_REQUESTER_ID(requester), data);
+
+	return pci_do_requester_callback(requester, fn, data);
+}
+
+/*
  * 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
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3a24e4f..b33824d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1873,6 +1873,13 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 }
 #endif
 
+struct pci_dev *pci_get_visible_pcie_requester(struct pci_dev *requestee,
+					       struct pci_dev *bridge);
+u16 pci_requester_id(struct pci_dev *requester, struct pci_dev *requestee);
+int pcie_for_each_requester(struct pci_dev *requestee, struct pci_dev *bridge,
+			    int (*fn)(struct pci_dev *, u16 id, void *),
+			    void *data);
+
 /**
  * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
  * @pdev: the PCI device

[RFC PATCH 2/2] iommu/intel: Make use of PCIe requester ID interface

[Alex Williamson]

This eliminates uses of pci_find_upstream_pcie_bridge() and
incorporates DMA quirks into dma_ops path.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/iommu/intel-iommu.c |  164 +++++++++++++++++--------------------------
 1 file changed, 64 insertions(+), 100 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b4f0e28..1e210e1 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1675,80 +1675,59 @@ static int domain_context_mapping_one(struct dmar_domain *domain, int segment,
 	return 0;
 }
 
-static int
-domain_context_mapping(struct dmar_domain *domain, struct pci_dev *pdev,
-			int translation)
+struct context_mapping_info {
+	struct dmar_domain *domain;
+	int translation;
+};
+
+static int context_mapping(struct pci_dev *dev, u16 requester_id, void *data)
 {
-	int ret;
-	struct pci_dev *tmp, *parent;
+	struct context_mapping_info *info = data;
+	u8 bus = requester_id >> 8;
+	u8 devfn = requester_id & 0xFF;
 
-	ret = domain_context_mapping_one(domain, pci_domain_nr(pdev->bus),
-					 pdev->bus->number, pdev->devfn,
-					 translation);
-	if (ret)
-		return ret;
+	return domain_context_mapping_one(info->domain, pci_domain_nr(dev->bus),
+					  bus, devfn, info->translation);
+}
 
-	/* dependent device mapping */
-	tmp = pci_find_upstream_pcie_bridge(pdev);
-	if (!tmp)
-		return 0;
-	/* Secondary interface's bus number and devfn 0 */
-	parent = pdev->bus->self;
-	while (parent != tmp) {
-		ret = domain_context_mapping_one(domain,
-						 pci_domain_nr(parent->bus),
-						 parent->bus->number,
-						 parent->devfn, translation);
-		if (ret)
-			return ret;
-		parent = parent->bus->self;
-	}
-	if (pci_is_pcie(tmp)) /* this is a PCIe-to-PCI bridge */
-		return domain_context_mapping_one(domain,
-					pci_domain_nr(tmp->subordinate),
-					tmp->subordinate->number, 0,
-					translation);
-	else /* this is a legacy PCI bridge */
-		return domain_context_mapping_one(domain,
-						  pci_domain_nr(tmp->bus),
-						  tmp->bus->number,
-						  tmp->devfn,
-						  translation);
+static int domain_context_mapping(struct dmar_domain *domain,
+				  struct pci_dev *pdev, int translation)
+{
+	struct context_mapping_info info = {
+		.domain = domain,
+		.translation = translation,
+	};
+
+	return pcie_for_each_requester(pdev, NULL, context_mapping, &info);
+}
+
+static int is_context_mapped(struct pci_dev *dev, u16 requester_id, void *data)
+{
+	struct intel_iommu *iommu = data;
+	u8 bus = requester_id >> 8;
+	u8 devfn = requester_id & 0xFF;
+
+	if (!device_context_mapped(iommu, bus, devfn))
+		return 1; /* stop */
+
+	return 0;
 }
 
 static int domain_context_mapped(struct pci_dev *pdev)
 {
-	int ret;
-	struct pci_dev *tmp, *parent;
 	struct intel_iommu *iommu;
+	int ret;
 
-	iommu = device_to_iommu(pci_domain_nr(pdev->bus), pdev->bus->number,
-				pdev->devfn);
+	iommu = device_to_iommu(pci_domain_nr(pdev->bus),
+				pdev->bus->number, pdev->devfn);
 	if (!iommu)
 		return -ENODEV;
 
-	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 (!tmp)
+	ret = pcie_for_each_requester(pdev, NULL, is_context_mapped, iommu);
+	if (ret < 0)
 		return ret;
-	/* Secondary interface's bus number and devfn 0 */
-	parent = pdev->bus->self;
-	while (parent != tmp) {
-		ret = device_context_mapped(iommu, parent->bus->number,
-					    parent->devfn);
-		if (!ret)
-			return ret;
-		parent = parent->bus->self;
-	}
-	if (pci_is_pcie(tmp))
-		return device_context_mapped(iommu, tmp->subordinate->number,
-					     0);
-	else
-		return device_context_mapped(iommu, tmp->bus->number,
-					     tmp->devfn);
+
+	return (ret == 0);
 }
 
 /* Returns a number of VTD pages, but aligned to MM page size */
@@ -1986,15 +1965,12 @@ 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)) {
-			bus = dev_tmp->subordinate->number;
-			devfn = 0;
-		} else {
-			bus = dev_tmp->bus->number;
-			devfn = dev_tmp->devfn;
-		}
+	dev_tmp = pci_get_visible_pcie_requester(pdev, NULL);
+	if (dev_tmp && dev_tmp != pdev) {
+		u16 requester_id = pci_requester_id(dev_tmp, pdev);
+		u8 bus = requester_id >> 8;
+		u8 devfn = requester_id & 0xFF;
+
 		spin_lock_irqsave(&device_domain_lock, flags);
 		list_for_each_entry(info, &device_domain_list, global) {
 			if (info->segment == segment &&
@@ -3749,31 +3725,24 @@ int __init intel_iommu_init(void)
 	return 0;
 }
 
+static int detach_requester(struct pci_dev *dev, u16 requester_id, void *data)
+{
+	struct intel_iommu *iommu = data;
+	u8 bus = requester_id >> 8;
+	u8 devfn = requester_id & 0xFF;
+
+	iommu_detach_dev(iommu, bus, devfn);
+	return 0;
+}
+
 static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 					   struct pci_dev *pdev)
 {
-	struct pci_dev *tmp, *parent;
-
 	if (!iommu || !pdev)
 		return;
 
-	/* dependent device detach */
-	tmp = pci_find_upstream_pcie_bridge(pdev);
-	/* 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);
-	}
+	/* XXX What if there's something else using his path? */
+	pcie_for_each_requester(pdev, NULL, detach_requester, iommu);
 }
 
 static void domain_remove_one_dev_info(struct dmar_domain *domain,
@@ -4158,7 +4127,7 @@ static int intel_iommu_domain_has_cap(struct iommu_domain *domain,
 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;
 
@@ -4166,16 +4135,11 @@ 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))
-			dma_pdev = pci_get_domain_bus_and_slot(
-						pci_domain_nr(pdev->bus),
-						bridge->subordinate->number, 0);
-		if (!dma_pdev)
-			dma_pdev = pci_dev_get(bridge);
-	} else
-		dma_pdev = pci_dev_get(pdev);
+	dma_pdev = pci_get_visible_pcie_requester(pdev, NULL);
+	if (!dma_pdev)
+		return -EINVAL;
+
+	dma_pdev = pci_dev_get(dma_pdev);
 
 	/* Account for quirked devices */
 	swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));