[PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Alex Williamson]

For testing, this version can be found in my git tree:

git://github.com/awilliam/linux-vfio.git dma-alias-v4

Please report any issues.

v4:
 - Change dma_func_alias to dma_alias_devfn, holding a single
   devfn to alias, thereby supporting aliases to the wrong slot.
   The DMA alias iterator is easily changed, but IOMMU grouping
   requires significant rework.  This is now done in IOMMU code
   rather than PCI code.

 - AMD-Vi - try to incorporate IVRS aliases dynamically into
   PCI alias quirks to make sure that our grouping remains the
   same.  Potentially this could end up reporting BIOS aliases
   that we can add to our list of quirks.

v3:
 - Found several instances where I had PCI_SLOT when I meant
   PCI_FUNC.  Thanks to Andrew for spotting this.  This should
   fix the problem he was having with Ricoh quirks.  We also
   pruned down the func0 quirks to only those that we know are
   needed.  We can always add them back later.

 - Found a case in intel-iommu of using dev_is_pci() where I
   really wanted !dev_is_pci().  Fixed.

v2:
 - Several new Marvell controllers added to quirks.  There's been
   a lot of success reported with this series in
   https://bugzilla.kernel.org/show_bug.cgi?id=42679

 - Add quirk for ASMedia and Tundra PCIe-to-PCI bridges that do
   not expose a PCIe capability.  These have been shown to use
   the standard PCIe-to-PCI bridge requester ID.

 - Fix copy/paste duplicate Ricoh quirk ID

 - Fixed AMD IOMMU for the "ghost" function case where the DMA
   alias is for an absent device.  The iommu rlookup table and
   data fields need to be initializes.

 - Fixed Intel interrupt remapping, I wasn't passing the target
   bus number, only the alias bus number.

These patches are split across PCI and IOMMU, but I've front-loaded
all of the PCI infrastructure so that the first 7 patches can be
applied to PCI-core, the IOMMU maintainers can pickup their patches,
then we can finish with dead code removal.  Bjorn might also be
willing to carry the IOMMU changes if the maintainers want to ack
them.

Original description:

This series attempts to fix a couple issues we've had outstanding in
the PCI/IOMMU code for a while.  The first issue is with devices that
use the wrong requester ID for DMA transactions.  We already have a
sort of half-baked attempt to fix this for several Ricoh devices, but
the fix only helps them be useful through IOMMU groups, not the
general DMA case.  There are also several Marvell devices which use
use a different wrong requester ID and don't even fit into the DMA
source idea.  This series creates a DMA alias iterator that will
step through each possible alias of a device, allowing IOMMUs to
insert mappings for both the device and its aliases.

Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge()
function, which is known to blowup when it finds itself suddenly at
a PCIe device without crossing a PCIe-to-PCI bridge (as identified by
the PCIe capability).  It also likes to make the invalid assumption
that a PCIe device never has its requester ID masked by any usptream
bus.  We can fix this using the above new DMA alias iterator, since
that's effectively what this function was meant to do.

Finally, with all these helpers, it makes sense to consolidate code
for determining IOMMU groups.  The first step in finding the root
of a group is finding the final upstream DMA alias for the device,
then applying additional ACS rules and incorporating device specific
aliases.  As this is all common to PCI, create a single implementation
and remove piles of code from the individual IOMMU drivers.

This series allows devices like the Marvell 88SE9123 to finally work
on Linux with either AMD-Vi or VT-d enabled on the box.  I've
collected device IDs from various bugs to support as many SKUs of
these devices as possible, but I'm sure there are others that I've
missed.

This should also enable motherboards with an onboard ASmedia
ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled.  I've
acquired an adapter board with this chip, but it actually exposes
a PCIe capability, unlike most of the onboard controllers.  Therefore
I expect this series will fix the WARN_ON currently hit during boot,
but there's a 50/50 chance whether the device behaves like a PCI
bridge or a PCIe bridge with regard to the requester ID that it uses
to take ownership of the transaction.  If it turns out to use the
PCIe bridge model, I expect we can quirk it using a dev_flags bit
to identify a PCI bridge that takes ownership as if it was a PCIe
bridge.

Please test and provide feedback.  I expect IOMMU group topology
should not change from this series, but if a case is found where it
does, please share.  Also, if there are additional quirks we need
to add, please either file new or add to the existing bugs.  Thanks,

Alex

---

Alex Williamson (16):
      PCI: Add DMA alias iterator
      PCI: define pci_dev_flags as bit shifts
      PCI: quirk pci_for_each_dma_alias()
      PCI: quirk dma_alias_devfn for Ricoh devices
      PCI: quirk dma_alias_devfn for Marvell devices
      PCI: Quirk pci_for_each_dma_alias() for bridges
      PCI: Add quirks for ASMedia and Tundra bridges
      iommu: Create central IOMMU group lookup/creation interface
      iommu/amd: Update to use PCI DMA aliases
      iommu/amd: Use iommu_group_get_for_dev()
      iommu/intel: Use iommu_group_get_for_dev()
      iommu/intel: Update to use PCI DMA aliases
      iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
      iommu: Remove pci.h
      PCI: Remove pci_find_upstream_pcie_bridge()
      PCI: Remove pci_get_dma_source()


 drivers/iommu/amd_iommu.c           |  214 +++++++-----------------
 drivers/iommu/amd_iommu_types.h     |    1 
 drivers/iommu/fsl_pamu_domain.c     |   66 --------
 drivers/iommu/intel-iommu.c         |  307 +++++++++++++----------------------
 drivers/iommu/intel_irq_remapping.c |   55 ++++--
 drivers/iommu/iommu.c               |  181 +++++++++++++++++++++
 drivers/iommu/pci.h                 |   29 ---
 drivers/pci/quirks.c                |  116 ++++++++-----
 drivers/pci/search.c                |  104 +++++++++---
 include/linux/iommu.h               |    1 
 include/linux/pci.h                 |   31 +---
 11 files changed, 557 insertions(+), 548 deletions(-)
 delete mode 100644 drivers/iommu/pci.h

[PATCH v4 01/16] PCI: Add DMA alias iterator

[Alex Williamson]

In a mixed PCI/PCI-X/PCI-e topology, bridges can take ownership of
transactions, replacing the original requester ID with their own.
Sometimes we just want to know the resulting device or resulting
alias, sometimes we want each step in the chain.  This iterator
allows either usage.  When an endpoint is connected via an unbroken
chain of PCIe switches and root ports, it has no alias and its
requester ID is visible to the root bus.  When PCI/X get in the way,
we pick up aliases for bridges.

The reason why we potentially care about each step in the path is
because of PCI-X.  PCI-X has the concept of a requester ID, but
bridges may or may not take ownership of various types of
transactions.  We therefore leave it to the consumer of this function
to prune out what they don't care about rather than attempt to flatten
the alias ourselves.

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

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 4a1b972..5601cdb 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -18,6 +18,76 @@ DECLARE_RWSEM(pci_bus_sem);
 EXPORT_SYMBOL_GPL(pci_bus_sem);
 
 /*
+ * pci_for_each_dma_alias - Iterate over DMA aliases for a device
+ * @pdev: starting downstream device
+ * @fn: function to call for each alias
+ * @data: opaque data to pass to @fn
+ *
+ * Starting @pdev, walk up the bus calling @fn for each possible alias
+ * of @pdev at the root bus.
+ */
+int pci_for_each_dma_alias(struct pci_dev *pdev,
+			   int (*fn)(struct pci_dev *pdev,
+				     u16 alias, void *data), void *data)
+{
+	struct pci_bus *bus;
+	int ret;
+
+	ret = fn(pdev, PCI_DEVID(pdev->bus->number, pdev->devfn), data);
+	if (ret)
+		return ret;
+
+	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
+		struct pci_dev *tmp;
+
+		/* Skip virtual buses */
+		if (!bus->self)
+			continue;
+
+		tmp = bus->self;
+
+		/*
+		 * PCIe-to-PCI/X bridges alias transactions from downstream
+		 * devices using the subordinate bus number (PCI Express to
+		 * PCI/PCI-X Bridge Spec, rev 1.0, sec 2.3).  For all cases
+		 * where the upstream bus is PCI/X we alias to the bridge
+		 * (there are various conditions in the previous reference
+		 * where the bridge may take ownership of transactions, even
+		 * when the secondary interface is PCI-X).
+		 */
+		if (pci_is_pcie(tmp)) {
+			switch (pci_pcie_type(tmp)) {
+			case PCI_EXP_TYPE_ROOT_PORT:
+			case PCI_EXP_TYPE_UPSTREAM:
+			case PCI_EXP_TYPE_DOWNSTREAM:
+				continue;
+			case PCI_EXP_TYPE_PCI_BRIDGE:
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->subordinate->number,
+						   PCI_DEVFN(0, 0)), data);
+				if (ret)
+					return ret;
+				continue;
+			case PCI_EXP_TYPE_PCIE_BRIDGE:
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->bus->number,
+						   tmp->devfn), data);
+				if (ret)
+					return ret;
+				continue;
+			}
+		} else {
+			ret = fn(tmp, PCI_DEVID(tmp->bus->number, tmp->devfn),
+				 data);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return ret;
+}
+
+/*
  * 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 aab57b4..14b074b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1795,6 +1795,10 @@ static inline struct eeh_dev *pci_dev_to_eeh_dev(struct pci_dev *pdev)
 }
 #endif
 
+int pci_for_each_dma_alias(struct pci_dev *pdev,
+			   int (*fn)(struct pci_dev *pdev,
+				     u16 alias, void *data), void *data);
+
 /**
  * pci_find_upstream_pcie_bridge - find upstream PCIe-to-PCI bridge of a device
  * @pdev: the PCI device

[PATCH v4 02/16] PCI: define pci_dev_flags as bit shifts

[Alex Williamson]

We're only a few entries away from where using the decimal value
becomes cumbersome.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 include/linux/pci.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 14b074b..545903d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -164,13 +164,13 @@ enum pci_dev_flags {
 	/* INTX_DISABLE in PCI_COMMAND register disables MSI
 	 * generation too.
 	 */
-	PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force pci_dev_flags_t) 1,
+	PCI_DEV_FLAGS_MSI_INTX_DISABLE_BUG = (__force pci_dev_flags_t) (1 << 0),
 	/* Device configuration is irrevocably lost if disabled into D3 */
-	PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t) 2,
+	PCI_DEV_FLAGS_NO_D3 = (__force pci_dev_flags_t) (1 << 1),
 	/* Provide indication device is assigned by a Virtual Machine Manager */
-	PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) 4,
+	PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) (1 << 2),
 	/* Flag for quirk use to store if quirk-specific ACS is enabled */
-	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) 8,
+	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
 };
 
 enum pci_irq_reroute_variant {

[PATCH v4 03/16] PCI: quirk pci_for_each_dma_alias()

[Alex Williamson]

Some devices are broken and use a requester ID other than their
physical devfn.  Add a byte, using an existing gap in the pci_dev
structure, to store an alternate "alias" devfn.  A bit in the
dev_flags tells us when this is valid.  We then add the alias as
one more step in the pci_for_each_dma_alias() iterator.

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

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 5601cdb..2c19f3f 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -37,6 +37,17 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
+	/*
+	 * If the device is broken and uses an alias requester ID for
+	 * DMA, iterate over that too.
+	 */
+	if (unlikely(pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN)) {
+		ret = fn(pdev, PCI_DEVID(pdev->bus->number,
+					 pdev->dma_alias_devfn), data);
+		if (ret)
+			return ret;
+	}
+
 	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
 		struct pci_dev *tmp;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 545903d..9d4035c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -171,6 +171,8 @@ enum pci_dev_flags {
 	PCI_DEV_FLAGS_ASSIGNED = (__force pci_dev_flags_t) (1 << 2),
 	/* Flag for quirk use to store if quirk-specific ACS is enabled */
 	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
+	/* Flag to indicate the device uses dma_alias_devfn */
+	PCI_DEV_FLAGS_DMA_ALIAS_DEVFN = (__force pci_dev_flags_t) (1 << 4),
 };
 
 enum pci_irq_reroute_variant {
@@ -268,6 +270,7 @@ struct pci_dev {
 	u8		rom_base_reg;	/* which config register controls the ROM */
 	u8		pin;		/* which interrupt pin this device uses */
 	u16		pcie_flags_reg;	/* cached PCIe Capabilities Register */
+	u8		dma_alias_devfn;/* devfn of DMA alias, if any */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this

[PATCH v4 04/16] PCI: quirk dma_alias_devfn for Ricoh devices

[Alex Williamson]

The existing quirk for these devices doesn't really solve the problem;
re-implement it using the DMA alias iterator.  We'll come back later
and remove the existing quirk and dma_source interface.  Note that
device ID 0xe822 is typically function 0 and 0xe230 has been tested to
not need the quirk and are therefore removed versus the equivalent
dma_source quirk.  If there exist in other configurations we can
re-add them.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/quirks.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index e729206..bc8ebd9 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3333,6 +3333,22 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 	return -ENOTTY;
 }
 
+static void quirk_dma_func0_alias(struct pci_dev *dev)
+{
+	if (PCI_FUNC(dev->devfn) != 0) {
+		dev->dma_alias_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 0);
+		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
+	}
+}
+
+/*
+ * https://bugzilla.redhat.com/show_bug.cgi?id=605888
+ *
+ * Some Ricoh devices use function 0 as the PCIe requester ID for DMA.
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe832, quirk_dma_func0_alias);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
+
 static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
 {
 	if (!PCI_FUNC(dev->devfn))

[PATCH v4 05/16] PCI: quirk dma_alias_devfn for Marvell devices

[Alex Williamson]

Several Marvell devices and a JMicron device have a similar DMA
requester ID problem to Ricoh, except they use function 1 as the
PCIe requester ID.  Add a quirk for these to populate the DMA
function alias bitmap.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/quirks.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index bc8ebd9..923689f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3349,6 +3349,42 @@ static void quirk_dma_func0_alias(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe832, quirk_dma_func0_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
 
+static void quirk_dma_func1_alias(struct pci_dev *dev)
+{
+	if (PCI_FUNC(dev->devfn) != 1) {
+		dev->dma_alias_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 1);
+		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
+	}
+}
+
+/*
+ * Marvell 88SE9123 uses function 1 as the requester ID for DMA.  In some
+ * SKUs function 1 is present and is a legacy IDE controller, in other
+ * SKUs this function is not present, making this a ghost requester.
+ * https://bugzilla.kernel.org/show_bug.cgi?id=42679
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9123,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c14 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9130,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c47 + c57 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9172,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c59 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x917a,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c46 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x91a0,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c49 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9230,
+			 quirk_dma_func1_alias);
+/* https://bugs.gentoo.org/show_bug.cgi?id=497630 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_JMICRON,
+			 PCI_DEVICE_ID_JMICRON_JMB388_ESD,
+			 quirk_dma_func1_alias);
+
 static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
 {
 	if (!PCI_FUNC(dev->devfn))

[PATCH v4 06/16] PCI: Quirk pci_for_each_dma_alias() for bridges

[Alex Williamson]

Several PCIe-to-PCI bridges fail to provide a PCIe capability, causing
us to handle them as conventional PCI devices.  In some cases, this
may be correct, in others it's not.  Add a dev_flag bit to identify
devices to be handled as standard PCIe-to-PCI bridges.

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

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 2c19f3f..df38f73 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -88,8 +88,14 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 				continue;
 			}
 		} else {
-			ret = fn(tmp, PCI_DEVID(tmp->bus->number, tmp->devfn),
-				 data);
+			if (tmp->dev_flags & PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS)
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->subordinate->number,
+						   PCI_DEVFN(0, 0)), data);
+			else
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->bus->number,
+						   tmp->devfn), data);
 			if (ret)
 				return ret;
 		}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9d4035c..85ab35e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -173,6 +173,8 @@ enum pci_dev_flags {
 	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
 	/* Flag to indicate the device uses dma_alias_devfn */
 	PCI_DEV_FLAGS_DMA_ALIAS_DEVFN = (__force pci_dev_flags_t) (1 << 4),
+	/* Use a PCIe-to-PCI bridge alias even if !pci_is_pcie */
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 5),
 };
 
 enum pci_irq_reroute_variant {

[PATCH v4 07/16] PCI: Add quirks for ASMedia and Tundra bridges

[Alex Williamson]

The quirk is intended to be extremely generic, but we only apply it
to known offending devices.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/quirks.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 923689f..ab620ac 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3385,6 +3385,29 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_JMICRON,
 			 PCI_DEVICE_ID_JMICRON_JMB388_ESD,
 			 quirk_dma_func1_alias);
 
+/*
+ * A few PCIe-to-PCI bridges fail to expose a PCIe capability, resulting in
+ * using the wrong DMA alias for the device.  Some of these devices can be
+ * used as either forward or reverse bridges, so we need to test whether the
+ * device is operating in the correct mode.  We could probably apply this
+ * quirk to PCI_ANY_ID, but for now we'll just use known offenders.  The test
+ * is for a non-root, non-PCIe bridge where the upstream device is PCIe and
+ * is not a PCIe-to-PCI bridge, then @pdev is actually a PCIe-to-PCI bridge.
+ */
+static void quirk_use_pcie_bridge_dma_alias(struct pci_dev *pdev)
+{
+	if (!pci_is_root_bus(pdev->bus) &&
+	    pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE &&
+	    !pci_is_pcie(pdev) && pci_is_pcie(pdev->bus->self) &&
+	    pci_pcie_type(pdev->bus->self) != PCI_EXP_TYPE_PCI_BRIDGE)
+		pdev->dev_flags |= PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS;
+}
+/* ASM1083/1085, https://bugzilla.kernel.org/show_bug.cgi?id=44881#c46 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ASMEDIA, 0x1080,
+			 quirk_use_pcie_bridge_dma_alias);
+/* Tundra 8113, https://bugzilla.kernel.org/show_bug.cgi?id=44881#c43 */
+DECLARE_PCI_FIXUP_HEADER(0x10e3, 0x8113, quirk_use_pcie_bridge_dma_alias);
+
 static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
 {
 	if (!PCI_FUNC(dev->devfn))

[PATCH v4 08/16] iommu: Create central IOMMU group lookup/creation interface

[Alex Williamson]

Currently each IOMMU driver that supports IOMMU groups has its own
code for discovering the base device used in grouping.  This code
is generally not specific to the IOMMU hardware, but to the bus of
the devices managed by the IOMMU.  We can therefore create a common
interface for supporting devices on different buses.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/iommu/iommu.c |  181 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/iommu.h |    1 
 2 files changed, 182 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e5555fc..db55fe6 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -29,6 +29,7 @@
 #include <linux/idr.h>
 #include <linux/notifier.h>
 #include <linux/err.h>
+#include <linux/pci.h>
 #include <trace/events/iommu.h>
 
 static struct kset *iommu_group_kset;
@@ -514,6 +515,186 @@ int iommu_group_id(struct iommu_group *group)
 }
 EXPORT_SYMBOL_GPL(iommu_group_id);
 
+/*
+ * To consider a PCI device isolated, we require ACS to support Source
+ * Validation, Request Redirection, Completer Redirection, and Upstream
+ * Forwarding.  This effectively means that devices cannot spoof their
+ * requester ID, requests and completions cannot be redirected, and all
+ * transactions are forwarded upstream, even as it passes through a
+ * bridge where the target device is downstream.
+ */
+#define REQ_ACS_FLAGS   (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
+
+struct group_for_pci_data {
+	struct pci_dev *pdev;
+	struct iommu_group *group;
+};
+
+/*
+ * DMA alias iterator callback, return the last seen device.  Stop and return
+ * the IOMMU group if we find one along the way.
+ */
+static int get_pci_alias_or_group(struct pci_dev *pdev, u16 alias, void *opaque)
+{
+	struct group_for_pci_data *data = opaque;
+
+	data->pdev = pdev;
+	data->group = iommu_group_get(&pdev->dev);
+
+	return data->group != NULL;
+}
+
+/*
+ * Use standard PCI bus topology, isolation features, and DMA alias quirks
+ * to find or create an IOMMU group for a device.
+ */
+static struct iommu_group *iommu_group_get_for_pci_dev(struct pci_dev *pdev)
+{
+	struct group_for_pci_data data;
+	struct pci_bus *bus;
+	struct iommu_group *group = NULL;
+	struct pci_dev *tmp;
+
+	/*
+	 * Find the upstream DMA alias for the device.  A device must not
+	 * be aliased due to topology in order to have its own IOMMU group.
+	 * If we find an alias along the way that already belongs to a
+	 * group, use it.
+	 */
+	if (pci_for_each_dma_alias(pdev, get_pci_alias_or_group, &data))
+		return data.group;
+
+	pdev = data.pdev;
+
+	/*
+	 * Continue upstream from the point of minimum IOMMU granularity
+	 * due to aliases to the point where devices are protected from
+	 * peer-to-peer DMA by PCI ACS.  Again, if we find an existing
+	 * group, use it.
+	 */
+	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
+		if (!bus->self)
+			continue;
+
+		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
+			break;
+
+		pdev = bus->self;
+
+		group = iommu_group_get(&pdev->dev);
+		if (group)
+			return group;
+	}
+
+	/*
+	 * Next we need to consider DMA alias quirks.  If one device aliases
+	 * to another, they should be grouped together.  It's theoretically
+	 * possible that aliases could create chains of devices where each
+	 * device aliases another device.  If we then factor in multifunction
+	 * ACS grouping requirements, each alias could incorporate a new slot
+	 * with multiple functions, each with aliases.  This is all extremely
+	 * unlikely as DMA alias quirks are typically only used for PCIe
+	 * devices where we usually have a single slot per bus.  Furthermore,
+	 * the alias quirk is usually to another function within the slot
+	 * (and ACS multifunction is not supported) or to a different slot
+	 * that doesn't physically exist.  The likely scenario is therefore
+	 * that everything on the bus gets grouped together.  To reduce the
+	 * problem space, share the IOMMU group for all devices on the bus
+	 * if a DMA alias quirk is present on the bus.
+	 */
+	tmp = NULL;
+	for_each_pci_dev(tmp) {
+		if (tmp->bus != pdev->bus ||
+		    !(tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN))
+			continue;
+
+		pci_dev_put(tmp);
+		tmp = NULL;
+
+		/* We have an alias quirk, search for an existing group */
+		for_each_pci_dev(tmp) {
+			struct iommu_group *group_tmp;
+			if (tmp->bus != pdev->bus)
+				continue;
+
+			group_tmp = iommu_group_get(&tmp->dev);
+			if (!group) {
+				group = group_tmp;
+				continue;
+			}
+
+			if (group_tmp) {
+				WARN_ON(group != group_tmp);
+				iommu_group_put(group_tmp);
+			}
+		}
+
+		return group ? group : iommu_group_alloc();
+	}
+
+	/*
+	 * Non-multifunction devices or multifunction devices supporting
+	 * ACS get their own group.
+	 */
+	if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
+		return iommu_group_alloc();
+
+	/*
+	 * Multifunction devices not supporting ACS share a group with other
+	 * similar devices in the same slot.
+	 */
+	tmp = NULL;
+	for_each_pci_dev(tmp) {
+		if (tmp == pdev || tmp->bus != pdev->bus ||
+		    PCI_SLOT(tmp->devfn) !=  PCI_SLOT(pdev->devfn) ||
+		    pci_acs_enabled(tmp, REQ_ACS_FLAGS))
+			continue;
+
+		group = iommu_group_get(&tmp->dev);
+		if (group) {
+			pci_dev_put(tmp);
+			return group;
+		}
+	}
+
+	/* No shared group found, allocate new */
+	return iommu_group_alloc();
+}
+
+/**
+ * iommu_group_get_for_dev - Find or create the IOMMU group for a device
+ * @dev: target device
+ *
+ * This function is intended to be called by IOMMU drivers and extended to
+ * support common, bus-defined algorithms when determining or creating the
+ * IOMMU group for a device.  On success, the caller will hold a reference
+ * to the returned IOMMU group, which will already include the provided
+ * device.  The reference should be released with iommu_group_put().
+ */
+struct iommu_group *iommu_group_get_for_dev(struct device *dev)
+{
+	struct iommu_group *group = ERR_PTR(-EIO);
+	int ret;
+
+	group = iommu_group_get(dev);
+	if (group)
+		return group;
+
+	if (dev_is_pci(dev))
+		group = iommu_group_get_for_pci_dev(to_pci_dev(dev));
+
+	if (IS_ERR(group))
+		return group;
+
+	ret = iommu_group_add_device(group, dev);
+	if (ret) {
+		iommu_group_put(group);
+		return ERR_PTR(ret);
+	}
+
+	return group;
+}
+
 static int add_iommu_group(struct device *dev, void *data)
 {
 	struct iommu_ops *ops = data;
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b96a5b2..a2e5843 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -181,6 +181,7 @@ extern int iommu_group_register_notifier(struct iommu_group *group,
 extern int iommu_group_unregister_notifier(struct iommu_group *group,
 					   struct notifier_block *nb);
 extern int iommu_group_id(struct iommu_group *group);
+extern struct iommu_group *iommu_group_get_for_dev(struct device *dev);
 
 extern int iommu_domain_get_attr(struct iommu_domain *domain, enum iommu_attr,
 				 void *data);

[PATCH v4 09/16] iommu/amd: Update to use PCI DMA aliases

[Alex Williamson]

AMD-Vi already has a concept of an alias provided via the IVRS table.
Now that PCI-core also understands aliases, we need to incorporate
both aspects when programming the IOMMU.  IVRS is generally quite
reliable, so we continue to prefer it when an alias is present.  For
cases where we have an IVRS alias that does not match the PCI alias
or where PCI does not report an alias, report the mismatch to allow
us to collect more quirks and dynamically incorporate the alias into
the device alias quirks where possible.

This should allow AMD-Vi to work with devices like Marvell and Ricoh
with DMA function alias quirks unknown to the BIOS.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu.c |   78 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 57068e8..a73a7ed 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -427,6 +427,68 @@ use_group:
 	return use_dev_data_iommu_group(dev_data->alias_data, dev);
 }
 
+static int __last_alias(struct pci_dev *pdev, u16 alias, void *data)
+{
+	*(u16 *)data = alias;
+	return 0;
+}
+
+static u16 get_alias(struct device *dev)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	u16 devid, ivrs_alias, pci_alias;
+
+	devid = get_device_id(dev);
+	ivrs_alias = amd_iommu_alias_table[devid];
+	pci_for_each_dma_alias(pdev, __last_alias, &pci_alias);
+
+	if (ivrs_alias == pci_alias)
+		return ivrs_alias;
+
+	/*
+	 * DMA alias showdown
+	 *
+	 * The IVRS is fairly reliable in telling us about aliases, but it
+	 * can't know about every screwy device.  If we don't have an IVRS
+	 * reported alias, use the PCI reported alias.  In that case we may
+	 * still need to initialize the rlookup and dev_table entries if the
+	 * alias is to a non-existent device.
+	 */
+	if (ivrs_alias == devid) {
+		if (!amd_iommu_rlookup_table[pci_alias]) {
+			amd_iommu_rlookup_table[pci_alias] =
+				amd_iommu_rlookup_table[devid];
+			memcpy(amd_iommu_dev_table[pci_alias].data,
+			       amd_iommu_dev_table[devid].data,
+			       sizeof(amd_iommu_dev_table[pci_alias].data));
+		}
+
+		return pci_alias;
+	}
+
+	pr_info("AMD-Vi: Using IVRS reported alias %02x:%02x.%d "
+		"for device %s[%04x:%04x], kernel reported alias "
+		"%02x:%02x.%d\n", PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias),
+		PCI_FUNC(ivrs_alias), dev_name(dev), pdev->vendor, pdev->device,
+		PCI_BUS_NUM(pci_alias), PCI_SLOT(pci_alias),
+		PCI_FUNC(pci_alias));
+
+	/*
+	 * If we don't have a PCI DMA alias and the IVRS alias is on the same
+	 * bus, then the IVRS table may know about a quirk that we don't.
+	 */
+	if (pci_alias == devid &&
+	    PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) {
+		pdev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
+		pdev->dma_alias_devfn = ivrs_alias & 0xff;
+		pr_info("AMD-Vi: Added PCI DMA alias %02x.%d for %s\n",
+			PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias),
+			dev_name(dev));
+	}
+
+	return ivrs_alias;
+}
+
 static int iommu_init_device(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -441,7 +503,8 @@ static int iommu_init_device(struct device *dev)
 	if (!dev_data)
 		return -ENOMEM;
 
-	alias = amd_iommu_alias_table[dev_data->devid];
+	alias = get_alias(dev);
+
 	if (alias != dev_data->devid) {
 		struct iommu_dev_data *alias_data;
 
@@ -489,12 +552,19 @@ static void iommu_ignore_device(struct device *dev)
 
 static void iommu_uninit_device(struct device *dev)
 {
+	struct iommu_dev_data *dev_data = search_dev_data(get_device_id(dev));
+
+	if (!dev_data)
+		return;
+
 	iommu_group_remove_device(dev);
 
+	/* Unlink from alias, it may change if another device is re-plugged */
+	dev_data->alias_data = NULL;
+
 	/*
-	 * Nothing to do here - we keep dev_data around for unplugged devices
-	 * and reuse it when the device is re-plugged - not doing so would
-	 * introduce a ton of races.
+	 * We keep dev_data around for unplugged devices and reuse it when the
+	 * device is re-plugged - not doing so would introduce a ton of races.
 	 */
 }
 

[PATCH v4 10/16] iommu/amd: Use iommu_group_get_for_dev()

[Alex Williamson]

The common iommu_group_get_for_dev() allows us to greatly simplify
our group lookup for a new device.  Also, since we insert IVRS
aliases into the PCI DMA alias quirks, we should alway come up with
the same results as the existing code.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu.c       |  164 +--------------------------------------
 drivers/iommu/amd_iommu_types.h |    1 
 2 files changed, 5 insertions(+), 160 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a73a7ed..ca85615 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -46,7 +46,6 @@
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
 #include "irq_remapping.h"
-#include "pci.h"
 
 #define CMD_SET_TYPE(cmd, t) ((cmd)->data[1] |= ((t) << 28))
 
@@ -133,9 +132,6 @@ static void free_dev_data(struct iommu_dev_data *dev_data)
 	list_del(&dev_data->dev_data_list);
 	spin_unlock_irqrestore(&dev_data_list_lock, flags);
 
-	if (dev_data->group)
-		iommu_group_put(dev_data->group);
-
 	kfree(dev_data);
 }
 
@@ -264,167 +260,17 @@ static bool check_device(struct device *dev)
 	return true;
 }
 
-static struct pci_bus *find_hosted_bus(struct pci_bus *bus)
-{
-	while (!bus->self) {
-		if (!pci_is_root_bus(bus))
-			bus = bus->parent;
-		else
-			return ERR_PTR(-ENODEV);
-	}
-
-	return bus;
-}
-
-#define REQ_ACS_FLAGS	(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
-
-static struct pci_dev *get_isolation_root(struct pci_dev *pdev)
-{
-	struct pci_dev *dma_pdev = pdev;
-
-	/* Account for quirked devices */
-	swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
-
-	/*
-	 * If it's a multifunction device that does not support our
-	 * required ACS flags, add to the same group as lowest numbered
-	 * function that also does not suport the required ACS flags.
-	 */
-	if (dma_pdev->multifunction &&
-	    !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) {
-		u8 i, slot = PCI_SLOT(dma_pdev->devfn);
-
-		for (i = 0; i < 8; i++) {
-			struct pci_dev *tmp;
-
-			tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i));
-			if (!tmp)
-				continue;
-
-			if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) {
-				swap_pci_ref(&dma_pdev, tmp);
-				break;
-			}
-			pci_dev_put(tmp);
-		}
-	}
-
-	/*
-	 * Devices on the root bus go through the iommu.  If that's not us,
-	 * find the next upstream device and test ACS up to the root bus.
-	 * Finding the next device may require skipping virtual buses.
-	 */
-	while (!pci_is_root_bus(dma_pdev->bus)) {
-		struct pci_bus *bus = find_hosted_bus(dma_pdev->bus);
-		if (IS_ERR(bus))
-			break;
-
-		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
-			break;
-
-		swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
-	}
-
-	return dma_pdev;
-}
-
-static int use_pdev_iommu_group(struct pci_dev *pdev, struct device *dev)
-{
-	struct iommu_group *group = iommu_group_get(&pdev->dev);
-	int ret;
-
-	if (!group) {
-		group = iommu_group_alloc();
-		if (IS_ERR(group))
-			return PTR_ERR(group);
-
-		WARN_ON(&pdev->dev != dev);
-	}
-
-	ret = iommu_group_add_device(group, dev);
-	iommu_group_put(group);
-	return ret;
-}
-
-static int use_dev_data_iommu_group(struct iommu_dev_data *dev_data,
-				    struct device *dev)
-{
-	if (!dev_data->group) {
-		struct iommu_group *group = iommu_group_alloc();
-		if (IS_ERR(group))
-			return PTR_ERR(group);
-
-		dev_data->group = group;
-	}
-
-	return iommu_group_add_device(dev_data->group, dev);
-}
-
 static int init_iommu_group(struct device *dev)
 {
-	struct iommu_dev_data *dev_data;
 	struct iommu_group *group;
-	struct pci_dev *dma_pdev;
-	int ret;
 
-	group = iommu_group_get(dev);
-	if (group) {
-		iommu_group_put(group);
-		return 0;
-	}
+	group = iommu_group_get_for_dev(dev);
 
-	dev_data = find_dev_data(get_device_id(dev));
-	if (!dev_data)
-		return -ENOMEM;
-
-	if (dev_data->alias_data) {
-		u16 alias;
-		struct pci_bus *bus;
-
-		if (dev_data->alias_data->group)
-			goto use_group;
+	if (IS_ERR(group))
+		return PTR_ERR(group);
 
-		/*
-		 * If the alias device exists, it's effectively just a first
-		 * level quirk for finding the DMA source.
-		 */
-		alias = amd_iommu_alias_table[dev_data->devid];
-		dma_pdev = pci_get_bus_and_slot(alias >> 8, alias & 0xff);
-		if (dma_pdev) {
-			dma_pdev = get_isolation_root(dma_pdev);
-			goto use_pdev;
-		}
-
-		/*
-		 * If the alias is virtual, try to find a parent device
-		 * and test whether the IOMMU group is actualy rooted above
-		 * the alias.  Be careful to also test the parent device if
-		 * we think the alias is the root of the group.
-		 */
-		bus = pci_find_bus(0, alias >> 8);
-		if (!bus)
-			goto use_group;
-
-		bus = find_hosted_bus(bus);
-		if (IS_ERR(bus) || !bus->self)
-			goto use_group;
-
-		dma_pdev = get_isolation_root(pci_dev_get(bus->self));
-		if (dma_pdev != bus->self || (dma_pdev->multifunction &&
-		    !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)))
-			goto use_pdev;
-
-		pci_dev_put(dma_pdev);
-		goto use_group;
-	}
-
-	dma_pdev = get_isolation_root(pci_dev_get(to_pci_dev(dev)));
-use_pdev:
-	ret = use_pdev_iommu_group(dma_pdev, dev);
-	pci_dev_put(dma_pdev);
-	return ret;
-use_group:
-	return use_dev_data_iommu_group(dev_data->alias_data, dev);
+	iommu_group_put(group);
+	return 0;
 }
 
 static int __last_alias(struct pci_dev *pdev, u16 alias, void *data)
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index f1a5abf..7277a20 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -432,7 +432,6 @@ struct iommu_dev_data {
 	struct iommu_dev_data *alias_data;/* The alias dev_data */
 	struct protection_domain *domain; /* Domain the device is bound to */
 	atomic_t bind;			  /* Domain attach reference count */
-	struct iommu_group *group;	  /* IOMMU group for virtual aliases */
 	u16 devid;			  /* PCI Device ID */
 	bool iommu_v2;			  /* Device can make use of IOMMUv2 */
 	bool passthrough;		  /* Default for device is pt_domain */

[PATCH v4 11/16] iommu/intel: Use iommu_group_get_for_dev()

[Alex Williamson]

The IOMMU code now provides a common interface for finding or
creating an IOMMU group for a device on PCI buses.  Make use of it
and remove piles of code.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/intel-iommu.c |   79 ++-----------------------------------------
 1 file changed, 4 insertions(+), 75 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f256ffc..5f0f352 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -44,7 +44,6 @@
 #include <asm/iommu.h>
 
 #include "irq_remapping.h"
-#include "pci.h"
 
 #define ROOT_SIZE		VTD_PAGE_SIZE
 #define CONTEXT_SIZE		VTD_PAGE_SIZE
@@ -4359,91 +4358,21 @@ static int intel_iommu_domain_has_cap(struct iommu_domain *domain,
 	return 0;
 }
 
-#define REQ_ACS_FLAGS	(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
-
 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 iommu_group *group;
-	int ret;
 	u8 bus, devfn;
 
 	if (!device_to_iommu(dev, &bus, &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);
-
-	/* Account for quirked devices */
-	swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
-
-	/*
-	 * If it's a multifunction device that does not support our
-	 * required ACS flags, add to the same group as lowest numbered
-	 * function that also does not suport the required ACS flags.
-	 */
-	if (dma_pdev->multifunction &&
-	    !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) {
-		u8 i, slot = PCI_SLOT(dma_pdev->devfn);
-
-		for (i = 0; i < 8; i++) {
-			struct pci_dev *tmp;
-
-			tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i));
-			if (!tmp)
-				continue;
-
-			if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) {
-				swap_pci_ref(&dma_pdev, tmp);
-				break;
-			}
-			pci_dev_put(tmp);
-		}
-	}
-
-	/*
-	 * Devices on the root bus go through the iommu.  If that's not us,
-	 * find the next upstream device and test ACS up to the root bus.
-	 * Finding the next device may require skipping virtual buses.
-	 */
-	while (!pci_is_root_bus(dma_pdev->bus)) {
-		struct pci_bus *bus = dma_pdev->bus;
-
-		while (!bus->self) {
-			if (!pci_is_root_bus(bus))
-				bus = bus->parent;
-			else
-				goto root_bus;
-		}
-
-		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
-			break;
-
-		swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
-	}
+	group = iommu_group_get_for_dev(dev);
 
-root_bus:
-	group = iommu_group_get(&dma_pdev->dev);
-	pci_dev_put(dma_pdev);
-	if (!group) {
-		group = iommu_group_alloc();
-		if (IS_ERR(group))
-			return PTR_ERR(group);
-	}
-
-	ret = iommu_group_add_device(group, dev);
+	if (IS_ERR(group))
+		return PTR_ERR(group);
 
 	iommu_group_put(group);
-	return ret;
+	return 0;
 }
 
 static void intel_iommu_remove_device(struct device *dev)

[PATCH v4 12/16] iommu/intel: Update to use PCI DMA aliases

[Alex Williamson]

VT-d code currently makes use of pci_find_upstream_pcie_bridge() in
order to find the topology based alias of a device.  This function has
a few problems.  First, it doesn't check the entire alias path of the
device to the root bus, therefore if a PCIe device is masked upstream,
the wrong result is produced.  Also, it's known to get confused and
give up when it crosses a bridge from a conventional PCI bus to a PCIe
bus that lacks a PCIe capability.  The PCI-core provided DMA alias
support solves both of these problems and additionally adds support
for DMA function quirks allowing VT-d to work with devices like
Marvell and Ricoh with known broken requester IDs.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/intel-iommu.c         |  228 ++++++++++++++++-------------------
 drivers/iommu/intel_irq_remapping.c |   55 ++++++--
 2 files changed, 145 insertions(+), 138 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 5f0f352..c4f11c0 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1840,54 +1840,56 @@ static int domain_context_mapping_one(struct dmar_domain *domain,
 	return 0;
 }
 
+struct domain_context_mapping_data {
+	struct dmar_domain *domain;
+	struct intel_iommu *iommu;
+	int translation;
+};
+
+static int domain_context_mapping_cb(struct pci_dev *pdev,
+				     u16 alias, void *opaque)
+{
+	struct domain_context_mapping_data *data = opaque;
+
+	return domain_context_mapping_one(data->domain, data->iommu,
+					  PCI_BUS_NUM(alias), alias & 0xff,
+					  data->translation);
+}
+
 static int
 domain_context_mapping(struct dmar_domain *domain, struct device *dev,
 		       int translation)
 {
-	int ret;
-	struct pci_dev *pdev, *tmp, *parent;
 	struct intel_iommu *iommu;
 	u8 bus, devfn;
+	struct domain_context_mapping_data data;
 
 	iommu = device_to_iommu(dev, &bus, &devfn);
 	if (!iommu)
 		return -ENODEV;
 
-	ret = domain_context_mapping_one(domain, iommu, bus, devfn,
-					 translation);
-	if (ret || !dev_is_pci(dev))
-		return ret;
-
-	/* dependent device mapping */
-	pdev = to_pci_dev(dev);
-	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, iommu,
-						 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, iommu,
-					tmp->subordinate->number, 0,
-					translation);
-	else /* this is a legacy PCI bridge */
-		return domain_context_mapping_one(domain, iommu,
-						  tmp->bus->number,
-						  tmp->devfn,
+	if (!dev_is_pci(dev))
+		return domain_context_mapping_one(domain, iommu, bus, devfn,
 						  translation);
+
+	data.domain = domain;
+	data.iommu = iommu;
+	data.translation = translation;
+
+	return pci_for_each_dma_alias(to_pci_dev(dev),
+				      &domain_context_mapping_cb, &data);
+}
+
+static int domain_context_mapped_cb(struct pci_dev *pdev,
+				    u16 alias, void *opaque)
+{
+	struct intel_iommu *iommu = opaque;
+
+	return !device_context_mapped(iommu, PCI_BUS_NUM(alias), alias & 0xff);
 }
 
 static int domain_context_mapped(struct device *dev)
 {
-	int ret;
-	struct pci_dev *pdev, *tmp, *parent;
 	struct intel_iommu *iommu;
 	u8 bus, devfn;
 
@@ -1895,30 +1897,11 @@ static int domain_context_mapped(struct device *dev)
 	if (!iommu)
 		return -ENODEV;
 
-	ret = device_context_mapped(iommu, bus, devfn);
-	if (!ret || !dev_is_pci(dev))
-		return ret;
+	if (!dev_is_pci(dev))
+		return device_context_mapped(iommu, bus, devfn);
 
-	/* dependent device mapping */
-	pdev = to_pci_dev(dev);
-	tmp = pci_find_upstream_pcie_bridge(pdev);
-	if (!tmp)
-		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 !pci_for_each_dma_alias(to_pci_dev(dev),
+				       domain_context_mapped_cb, iommu);
 }
 
 /* Returns a number of VTD pages, but aligned to MM page size */
@@ -2207,79 +2190,86 @@ static struct dmar_domain *dmar_insert_dev_info(struct intel_iommu *iommu,
 	return domain;
 }
 
+static int get_last_alias(struct pci_dev *pdev, u16 alias, void *opaque)
+{
+	*(u16 *)opaque = alias;
+	return 0;
+}
+
 /* domain is initialized */
 static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
 {
-	struct dmar_domain *domain, *free = NULL;
-	struct intel_iommu *iommu = NULL;
+	struct dmar_domain *domain, *tmp;
+	struct intel_iommu *iommu;
 	struct device_domain_info *info;
-	struct pci_dev *dev_tmp = NULL;
+	u16 dma_alias;
 	unsigned long flags;
-	u8 bus, devfn, bridge_bus, bridge_devfn;
+	u8 bus, devfn;
 
 	domain = find_domain(dev);
 	if (domain)
 		return domain;
 
+	iommu = device_to_iommu(dev, &bus, &devfn);
+	if (!iommu)
+		return NULL;
+
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
-		u16 segment;
 
-		segment = pci_domain_nr(pdev->bus);
-		dev_tmp = pci_find_upstream_pcie_bridge(pdev);
-		if (dev_tmp) {
-			if (pci_is_pcie(dev_tmp)) {
-				bridge_bus = dev_tmp->subordinate->number;
-				bridge_devfn = 0;
-			} else {
-				bridge_bus = dev_tmp->bus->number;
-				bridge_devfn = dev_tmp->devfn;
-			}
-			spin_lock_irqsave(&device_domain_lock, flags);
-			info = dmar_search_domain_by_dev_info(segment,
-							      bridge_bus,
-							      bridge_devfn);
-			if (info) {
-				iommu = info->iommu;
-				domain = info->domain;
-			}
-			spin_unlock_irqrestore(&device_domain_lock, flags);
-			/* pcie-pci bridge already has a domain, uses it */
-			if (info)
-				goto found_domain;
+		pci_for_each_dma_alias(pdev, get_last_alias, &dma_alias);
+
+		spin_lock_irqsave(&device_domain_lock, flags);
+		info = dmar_search_domain_by_dev_info(pci_domain_nr(pdev->bus),
+						      PCI_BUS_NUM(dma_alias),
+						      dma_alias & 0xff);
+		if (info) {
+			iommu = info->iommu;
+			domain = info->domain;
 		}
-	}
+		spin_unlock_irqrestore(&device_domain_lock, flags);
 
-	iommu = device_to_iommu(dev, &bus, &devfn);
-	if (!iommu)
-		goto error;
+		/* DMA alias already has a domain, uses it */
+		if (info)
+			goto found_domain;
+	}
 
 	/* Allocate and initialize new domain for the device */
 	domain = alloc_domain(false);
 	if (!domain)
-		goto error;
+		return NULL;
+
 	if (iommu_attach_domain(domain, iommu)) {
 		free_domain_mem(domain);
-		domain = NULL;
-		goto error;
+		return NULL;
 	}
-	free = domain;
-	if (domain_init(domain, gaw))
-		goto error;
 
-	/* register pcie-to-pci device */
-	if (dev_tmp) {
-		domain = dmar_insert_dev_info(iommu, bridge_bus, bridge_devfn,
-					      NULL, domain);
+	if (domain_init(domain, gaw)) {
+		domain_exit(domain);
+		return NULL;
+	}
+
+	/* register PCI DMA alias device */
+	if (dev_is_pci(dev)) {
+		tmp = dmar_insert_dev_info(iommu, PCI_BUS_NUM(dma_alias),
+					   dma_alias & 0xff, NULL, domain);
+
+		if (!tmp || tmp != domain) {
+			domain_exit(domain);
+			domain = tmp;
+		}
+
 		if (!domain)
-			goto error;
+			return NULL;
 	}
 
 found_domain:
-	domain = dmar_insert_dev_info(iommu, bus, devfn, dev, domain);
-error:
-	if (free != domain)
-		domain_exit(free);
+	tmp = dmar_insert_dev_info(iommu, bus, devfn, dev, domain);
+
+	if (!tmp || tmp != domain) {
+		domain_exit(domain);
+		domain = tmp;
+	}
 
 	return domain;
 }
@@ -4029,33 +4019,27 @@ out_free_dmar:
 	return ret;
 }
 
+static int iommu_detach_dev_cb(struct pci_dev *pdev, u16 alias, void *opaque)
+{
+	struct intel_iommu *iommu = opaque;
+
+	iommu_detach_dev(iommu, PCI_BUS_NUM(alias), alias & 0xff);
+	return 0;
+}
+
+/*
+ * NB - intel-iommu lacks any sort of reference counting for the users of
+ * dependent devices.  If multiple endpoints have intersecting dependent
+ * devices, unbinding the driver from any one of them will possibly leave
+ * the others unable to operate.
+ */
 static void iommu_detach_dependent_devices(struct intel_iommu *iommu,
 					   struct device *dev)
 {
-	struct pci_dev *tmp, *parent, *pdev;
-
 	if (!iommu || !dev || !dev_is_pci(dev))
 		return;
 
-	pdev = to_pci_dev(dev);
-
-	/* 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);
-	}
+	pci_for_each_dma_alias(to_pci_dev(dev), &iommu_detach_dev_cb, iommu);
 }
 
 static void domain_remove_one_dev_info(struct dmar_domain *domain,
diff --git a/drivers/iommu/intel_irq_remapping.c b/drivers/iommu/intel_irq_remapping.c
index 9b17489..757e0b0 100644
--- a/drivers/iommu/intel_irq_remapping.c
+++ b/drivers/iommu/intel_irq_remapping.c
@@ -369,29 +369,52 @@ static int set_hpet_sid(struct irte *irte, u8 id)
 	return 0;
 }
 
+struct set_msi_sid_data {
+	struct pci_dev *pdev;
+	u16 alias;
+};
+
+static int set_msi_sid_cb(struct pci_dev *pdev, u16 alias, void *opaque)
+{
+	struct set_msi_sid_data *data = opaque;
+
+	data->pdev = pdev;
+	data->alias = alias;
+
+	return 0;
+}
+
 static int set_msi_sid(struct irte *irte, struct pci_dev *dev)
 {
-	struct pci_dev *bridge;
+	struct set_msi_sid_data data;
 
 	if (!irte || !dev)
 		return -1;
 
-	/* PCIe device or Root Complex integrated PCI device */
-	if (pci_is_pcie(dev) || !dev->bus->parent) {
-		set_irte_sid(irte, SVT_VERIFY_SID_SQ, SQ_ALL_16,
-			     (dev->bus->number << 8) | dev->devfn);
-		return 0;
-	}
+	pci_for_each_dma_alias(dev, set_msi_sid_cb, &data);
 
-	bridge = pci_find_upstream_pcie_bridge(dev);
-	if (bridge) {
-		if (pci_is_pcie(bridge))/* this is a 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 */
-			set_irte_sid(irte, SVT_VERIFY_SID_SQ, SQ_ALL_16,
-				(bridge->bus->number << 8) | bridge->devfn);
-	}
+	/*
+	 * DMA alias provides us with a PCI device and alias.  The only case
+	 * where the it will return an alias on a different bus than the
+	 * device is the case of a PCIe-to-PCI bridge, where the alias is for
+	 * the subordinate bus.  In this case we can only verify the bus.
+	 *
+	 * If the alias device is on a different bus than our source device
+	 * then we have a topology based alias, use it.
+	 *
+	 * Otherwise, the alias is for a device DMA quirk and we cannot
+	 * assume that MSI uses the same requester ID.  Therefore use the
+	 * original device.
+	 */
+	if (PCI_BUS_NUM(data.alias) != data.pdev->bus->number)
+		set_irte_sid(irte, SVT_VERIFY_BUS, SQ_ALL_16,
+			     PCI_DEVID(PCI_BUS_NUM(data.alias),
+				       dev->bus->number));
+	else if (data.pdev->bus->number != dev->bus->number)
+		set_irte_sid(irte, SVT_VERIFY_SID_SQ, SQ_ALL_16, data.alias);
+	else
+		set_irte_sid(irte, SVT_VERIFY_SID_SQ, SQ_ALL_16,
+			     PCI_DEVID(dev->bus->number, dev->devfn));
 
 	return 0;
 }

[PATCH v4 13/16] iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups

[Alex Williamson]

Drop custom code and use IOMMU provided grouping support for PCI.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Varun Sethi <varun.sethi@freescale.com>
---
 drivers/iommu/fsl_pamu_domain.c |   66 +--------------------------------------
 1 file changed, 1 insertion(+), 65 deletions(-)

diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index 93072ba..d02d668 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -38,7 +38,6 @@
 #include <sysdev/fsl_pci.h>
 
 #include "fsl_pamu_domain.h"
-#include "pci.h"
 
 /*
  * Global spinlock that needs to be held while
@@ -892,8 +891,6 @@ static int fsl_pamu_get_domain_attr(struct iommu_domain *domain,
 	return ret;
 }
 
-#define REQ_ACS_FLAGS	(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF)
-
 static struct iommu_group *get_device_iommu_group(struct device *dev)
 {
 	struct iommu_group *group;
@@ -950,74 +947,13 @@ static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
 	struct pci_controller *pci_ctl;
 	bool pci_endpt_partioning;
 	struct iommu_group *group = NULL;
-	struct pci_dev *bridge, *dma_pdev = NULL;
 
 	pci_ctl = pci_bus_to_host(pdev->bus);
 	pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
 	/* We can partition PCIe devices so assign device group to the device */
 	if (pci_endpt_partioning) {
-		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);
-
-		/* Account for quirked devices */
-		swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
-
-		/*
-		 * If it's a multifunction device that does not support our
-		 * required ACS flags, add to the same group as lowest numbered
-		 * function that also does not suport the required ACS flags.
-		 */
-		if (dma_pdev->multifunction &&
-		    !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) {
-			u8 i, slot = PCI_SLOT(dma_pdev->devfn);
-
-			for (i = 0; i < 8; i++) {
-				struct pci_dev *tmp;
-
-				tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot, i));
-				if (!tmp)
-					continue;
-
-				if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) {
-					swap_pci_ref(&dma_pdev, tmp);
-					break;
-				}
-				pci_dev_put(tmp);
-			}
-		}
-
-		/*
-		 * Devices on the root bus go through the iommu.  If that's not us,
-		 * find the next upstream device and test ACS up to the root bus.
-		 * Finding the next device may require skipping virtual buses.
-		 */
-		while (!pci_is_root_bus(dma_pdev->bus)) {
-			struct pci_bus *bus = dma_pdev->bus;
-
-			while (!bus->self) {
-				if (!pci_is_root_bus(bus))
-					bus = bus->parent;
-				else
-					goto root_bus;
-			}
-
-			if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
-				break;
-
-			swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
-		}
+		group = iommu_group_get_for_dev(&pdev->dev);
 
-root_bus:
-		group = get_device_iommu_group(&dma_pdev->dev);
-		pci_dev_put(dma_pdev);
 		/*
 		 * PCIe controller is not a paritionable entity
 		 * free the controller device iommu_group.

[PATCH v4 14/16] iommu: Remove pci.h

[Alex Williamson]

The single helper here no longer has any users.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Cc: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/pci.h |   29 -----------------------------
 1 file changed, 29 deletions(-)
 delete mode 100644 drivers/iommu/pci.h

diff --git a/drivers/iommu/pci.h b/drivers/iommu/pci.h
deleted file mode 100644
index 352d80a..0000000
--- a/drivers/iommu/pci.h
+++ /dev/null
@@ -1,29 +0,0 @@
-/*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License, version 2, as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
- *
- * Copyright (C) 2013 Red Hat, Inc.
- * Copyright (C) 2013 Freescale Semiconductor, Inc.
- *
- */
-#ifndef __IOMMU_PCI_H
-#define __IOMMU_PCI_H
-
-/* Helper function for swapping pci device reference */
-static inline void swap_pci_ref(struct pci_dev **from, struct pci_dev *to)
-{
-	pci_dev_put(*from);
-	*from = to;
-}
-
-#endif  /* __IOMMU_PCI_H */

[PATCH v4 15/16] PCI: Remove pci_find_upstream_pcie_bridge()

[Alex Williamson]

It's broken and has no 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 df38f73..b8e65c2 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -104,41 +104,6 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	return ret;
 }
 
-/*
- * 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 85ab35e..903c7d6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1804,15 +1804,4 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
 				     u16 alias, void *data), void *data);
 
-/**
- * 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 */

[PATCH v4 16/16] PCI: Remove pci_get_dma_source()

[Alex Williamson]

It has no users; replaced by dma_alias_devfn.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 drivers/pci/quirks.c |   51 --------------------------------------------------
 include/linux/pci.h  |    5 -----
 2 files changed, 56 deletions(-)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index ab620ac..f6a42bc 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3408,57 +3408,6 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ASMEDIA, 0x1080,
 /* Tundra 8113, https://bugzilla.kernel.org/show_bug.cgi?id=44881#c43 */
 DECLARE_PCI_FIXUP_HEADER(0x10e3, 0x8113, quirk_use_pcie_bridge_dma_alias);
 
-static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
-{
-	if (!PCI_FUNC(dev->devfn))
-		return pci_dev_get(dev);
-
-	return pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
-}
-
-static const struct pci_dev_dma_source {
-	u16 vendor;
-	u16 device;
-	struct pci_dev *(*dma_source)(struct pci_dev *dev);
-} pci_dev_dma_source[] = {
-	/*
-	 * https://bugzilla.redhat.com/show_bug.cgi?id=605888
-	 *
-	 * Some Ricoh devices use the function 0 source ID for DMA on
-	 * other functions of a multifunction device.  The DMA devices
-	 * is therefore function 0, which will have implications of the
-	 * iommu grouping of these devices.
-	 */
-	{ PCI_VENDOR_ID_RICOH, 0xe822, pci_func_0_dma_source },
-	{ PCI_VENDOR_ID_RICOH, 0xe230, pci_func_0_dma_source },
-	{ PCI_VENDOR_ID_RICOH, 0xe832, pci_func_0_dma_source },
-	{ PCI_VENDOR_ID_RICOH, 0xe476, pci_func_0_dma_source },
-	{ 0 }
-};
-
-/*
- * IOMMUs with isolation capabilities need to be programmed with the
- * correct source ID of a device.  In most cases, the source ID matches
- * the device doing the DMA, but sometimes hardware is broken and will
- * tag the DMA as being sourced from a different device.  This function
- * allows that translation.  Note that the reference count of the
- * returned device is incremented on all paths.
- */
-struct pci_dev *pci_get_dma_source(struct pci_dev *dev)
-{
-	const struct pci_dev_dma_source *i;
-
-	for (i = pci_dev_dma_source; i->dma_source; i++) {
-		if ((i->vendor == dev->vendor ||
-		     i->vendor == (u16)PCI_ANY_ID) &&
-		    (i->device == dev->device ||
-		     i->device == (u16)PCI_ANY_ID))
-			return i->dma_source(dev);
-	}
-
-	return pci_dev_get(dev);
-}
-
 /*
  * AMD has indicated that the devices below do not support peer-to-peer
  * in any system where they are found in the southbridge with an AMD
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 903c7d6..388857c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1533,16 +1533,11 @@ enum pci_fixup_pass {
 
 #ifdef CONFIG_PCI_QUIRKS
 void pci_fixup_device(enum pci_fixup_pass pass, struct pci_dev *dev);
-struct pci_dev *pci_get_dma_source(struct pci_dev *dev);
 int pci_dev_specific_acs_enabled(struct pci_dev *dev, u16 acs_flags);
 void pci_dev_specific_enable_acs(struct pci_dev *dev);
 #else
 static inline void pci_fixup_device(enum pci_fixup_pass pass,
 				    struct pci_dev *dev) { }
-static inline struct pci_dev *pci_get_dma_source(struct pci_dev *dev)
-{
-	return pci_dev_get(dev);
-}
 static inline int pci_dev_specific_acs_enabled(struct pci_dev *dev,
 					       u16 acs_flags)
 {

Re: [PATCH v4 05/16] PCI: quirk dma_alias_devfn for Marvell devices

[George Spelvin]

Tested-by: George Spelvin <linux@horizon.com>

(I'm running the whole series, but 1b4b:9172 are the devices I have.)

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Pat Erley]

On 05/22/2014 06:07 PM, Alex Williamson wrote:
> For testing, this version can be found in my git tree:
>
> git://github.com/awilliam/linux-vfio.git dma-alias-v4
>
> Please report any issues.
>
> v4:
>   - Change dma_func_alias to dma_alias_devfn, holding a single
>     devfn to alias, thereby supporting aliases to the wrong slot.
>     The DMA alias iterator is easily changed, but IOMMU grouping
>     requires significant rework.  This is now done in IOMMU code
>     rather than PCI code.
>
>   - AMD-Vi - try to incorporate IVRS aliases dynamically into
>     PCI alias quirks to make sure that our grouping remains the
>     same.  Potentially this could end up reporting BIOS aliases
>     that we can add to our list of quirks.
>
> v3:
>   - Found several instances where I had PCI_SLOT when I meant
>     PCI_FUNC.  Thanks to Andrew for spotting this.  This should
>     fix the problem he was having with Ricoh quirks.  We also
>     pruned down the func0 quirks to only those that we know are
>     needed.  We can always add them back later.
>
>   - Found a case in intel-iommu of using dev_is_pci() where I
>     really wanted !dev_is_pci().  Fixed.
>
> v2:
>   - Several new Marvell controllers added to quirks.  There's been
>     a lot of success reported with this series in
>     https://bugzilla.kernel.org/show_bug.cgi?id=42679
>
>   - Add quirk for ASMedia and Tundra PCIe-to-PCI bridges that do
>     not expose a PCIe capability.  These have been shown to use
>     the standard PCIe-to-PCI bridge requester ID.
>
>   - Fix copy/paste duplicate Ricoh quirk ID
>
>   - Fixed AMD IOMMU for the "ghost" function case where the DMA
>     alias is for an absent device.  The iommu rlookup table and
>     data fields need to be initializes.
>
>   - Fixed Intel interrupt remapping, I wasn't passing the target
>     bus number, only the alias bus number.
>
> These patches are split across PCI and IOMMU, but I've front-loaded
> all of the PCI infrastructure so that the first 7 patches can be
> applied to PCI-core, the IOMMU maintainers can pickup their patches,
> then we can finish with dead code removal.  Bjorn might also be
> willing to carry the IOMMU changes if the maintainers want to ack
> them.
>
> Original description:
>
> This series attempts to fix a couple issues we've had outstanding in
> the PCI/IOMMU code for a while.  The first issue is with devices that
> use the wrong requester ID for DMA transactions.  We already have a
> sort of half-baked attempt to fix this for several Ricoh devices, but
> the fix only helps them be useful through IOMMU groups, not the
> general DMA case.  There are also several Marvell devices which use
> use a different wrong requester ID and don't even fit into the DMA
> source idea.  This series creates a DMA alias iterator that will
> step through each possible alias of a device, allowing IOMMUs to
> insert mappings for both the device and its aliases.
>
> Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge()
> function, which is known to blowup when it finds itself suddenly at
> a PCIe device without crossing a PCIe-to-PCI bridge (as identified by
> the PCIe capability).  It also likes to make the invalid assumption
> that a PCIe device never has its requester ID masked by any usptream
> bus.  We can fix this using the above new DMA alias iterator, since
> that's effectively what this function was meant to do.
>
> Finally, with all these helpers, it makes sense to consolidate code
> for determining IOMMU groups.  The first step in finding the root
> of a group is finding the final upstream DMA alias for the device,
> then applying additional ACS rules and incorporating device specific
> aliases.  As this is all common to PCI, create a single implementation
> and remove piles of code from the individual IOMMU drivers.
>
> This series allows devices like the Marvell 88SE9123 to finally work
> on Linux with either AMD-Vi or VT-d enabled on the box.  I've
> collected device IDs from various bugs to support as many SKUs of
> these devices as possible, but I'm sure there are others that I've
> missed.
>
> This should also enable motherboards with an onboard ASmedia
> ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled.  I've
> acquired an adapter board with this chip, but it actually exposes
> a PCIe capability, unlike most of the onboard controllers.  Therefore
> I expect this series will fix the WARN_ON currently hit during boot,
> but there's a 50/50 chance whether the device behaves like a PCI
> bridge or a PCIe bridge with regard to the requester ID that it uses
> to take ownership of the transaction.  If it turns out to use the
> PCIe bridge model, I expect we can quirk it using a dev_flags bit
> to identify a PCI bridge that takes ownership as if it was a PCIe
> bridge.
>
> Please test and provide feedback.  I expect IOMMU group topology
> should not change from this series, but if a case is found where it
> does, please share.  Also, if there are additional quirks we need
> to add, please either file new or add to the existing bugs.  Thanks,
>
> Alex
>
> ---
>
> Alex Williamson (16):
>        PCI: Add DMA alias iterator
>        PCI: define pci_dev_flags as bit shifts
>        PCI: quirk pci_for_each_dma_alias()
>        PCI: quirk dma_alias_devfn for Ricoh devices
>        PCI: quirk dma_alias_devfn for Marvell devices
>        PCI: Quirk pci_for_each_dma_alias() for bridges
>        PCI: Add quirks for ASMedia and Tundra bridges
>        iommu: Create central IOMMU group lookup/creation interface
>        iommu/amd: Update to use PCI DMA aliases
>        iommu/amd: Use iommu_group_get_for_dev()
>        iommu/intel: Use iommu_group_get_for_dev()
>        iommu/intel: Update to use PCI DMA aliases
>        iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
>        iommu: Remove pci.h
>        PCI: Remove pci_find_upstream_pcie_bridge()
>        PCI: Remove pci_get_dma_source()
>
>
This series allows my W510 to boot with firewire and VT-d enabled.  I've 
been working around this issue for 2 years.  Feel free to CC me on any 
further iterations of this series.

17:00.0 SD Host controller: Ricoh Co Ltd MMC/SD Host Controller (rev 01)
17:00.3 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 PCIe IEEE 1394
         Controller (rev 01)

17:00.0 0805: 1180:e822 (rev 01)
17:00.3 0c00: 1180:e832 (rev 01)

Tested-by: Pat Erley <pat-lkml@erley.org>

Re: [PATCH v4 05/16] PCI: quirk dma_alias_devfn for Marvell devices

[Bjorn Helgaas]

On Thu, May 22, 2014 at 05:07:55PM -0600, Alex Williamson wrote:
> Several Marvell devices and a JMicron device have a similar DMA
> requester ID problem to Ricoh, except they use function 1 as the
> PCIe requester ID.  Add a quirk for these to populate the DMA
> function alias bitmap.

What's the DMA function alias bitmap?

> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/pci/quirks.c |   36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index bc8ebd9..923689f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3349,6 +3349,42 @@ static void quirk_dma_func0_alias(struct pci_dev *dev)
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe832, quirk_dma_func0_alias);
>  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
>  
> +static void quirk_dma_func1_alias(struct pci_dev *dev)
> +{
> +	if (PCI_FUNC(dev->devfn) != 1) {
> +		dev->dma_alias_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 1);
> +		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
> +	}
> +}
> +
> +/*
> + * Marvell 88SE9123 uses function 1 as the requester ID for DMA.  In some
> + * SKUs function 1 is present and is a legacy IDE controller, in other
> + * SKUs this function is not present, making this a ghost requester.
> + * https://bugzilla.kernel.org/show_bug.cgi?id=42679
> + */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9123,
> +			 quirk_dma_func1_alias);
> +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c14 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9130,
> +			 quirk_dma_func1_alias);
> +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c47 + c57 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9172,
> +			 quirk_dma_func1_alias);
> +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c59 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x917a,
> +			 quirk_dma_func1_alias);
> +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c46 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x91a0,
> +			 quirk_dma_func1_alias);
> +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c49 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9230,
> +			 quirk_dma_func1_alias);
> +/* https://bugs.gentoo.org/show_bug.cgi?id=497630 */
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_JMICRON,
> +			 PCI_DEVICE_ID_JMICRON_JMB388_ESD,
> +			 quirk_dma_func1_alias);
> +
>  static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
>  {
>  	if (!PCI_FUNC(dev->devfn))
> 

Re: [PATCH v4 06/16] PCI: Quirk pci_for_each_dma_alias() for bridges

[Bjorn Helgaas]

On Thu, May 22, 2014 at 05:08:01PM -0600, Alex Williamson wrote:
> Several PCIe-to-PCI bridges fail to provide a PCIe capability, causing
> us to handle them as conventional PCI devices.  In some cases, this
> may be correct, in others it's not.  Add a dev_flag bit to identify
> devices to be handled as standard PCIe-to-PCI bridges.

Can you expand on the "in some cases, this may be correct, in others it's
not"?  Do you mean that for some *devices* it's correct to handle them as
conventional PCI, or in some *situations* it's correct?  Something else?

I'd like to either remove that sentence or add a little more information to
make it useful.

I guess this probably goes along with the tests in
quirk_use_pcie_bridge_dma_alias().

> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  drivers/pci/search.c |   10 ++++++++--
>  include/linux/pci.h  |    2 ++
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index 2c19f3f..df38f73 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -88,8 +88,14 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>  				continue;
>  			}
>  		} else {
> -			ret = fn(tmp, PCI_DEVID(tmp->bus->number, tmp->devfn),
> -				 data);
> +			if (tmp->dev_flags & PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS)
> +				ret = fn(tmp,
> +					 PCI_DEVID(tmp->subordinate->number,
> +						   PCI_DEVFN(0, 0)), data);
> +			else
> +				ret = fn(tmp,
> +					 PCI_DEVID(tmp->bus->number,
> +						   tmp->devfn), data);
>  			if (ret)
>  				return ret;
>  		}
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 9d4035c..85ab35e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -173,6 +173,8 @@ enum pci_dev_flags {
>  	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
>  	/* Flag to indicate the device uses dma_alias_devfn */
>  	PCI_DEV_FLAGS_DMA_ALIAS_DEVFN = (__force pci_dev_flags_t) (1 << 4),
> +	/* Use a PCIe-to-PCI bridge alias even if !pci_is_pcie */
> +	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 5),
>  };
>  
>  enum pci_irq_reroute_variant {
> 

Re: [PATCH v4 05/16] PCI: quirk dma_alias_devfn for Marvell devices

[Alex Williamson]

On Wed, 2014-05-28 at 11:55 -0600, Bjorn Helgaas wrote:
> On Thu, May 22, 2014 at 05:07:55PM -0600, Alex Williamson wrote:
> > Several Marvell devices and a JMicron device have a similar DMA
> > requester ID problem to Ricoh, except they use function 1 as the
> > PCIe requester ID.  Add a quirk for these to populate the DMA
> > function alias bitmap.
> 
> What's the DMA function alias bitmap?

Guess I missed updating this comment for v4.  Let me know if you have
other comments and I can either resend this individually or respin the
series.  Thanks,

Alex

> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  drivers/pci/quirks.c |   36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index bc8ebd9..923689f 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -3349,6 +3349,42 @@ static void quirk_dma_func0_alias(struct pci_dev *dev)
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe832, quirk_dma_func0_alias);
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
> >  
> > +static void quirk_dma_func1_alias(struct pci_dev *dev)
> > +{
> > +	if (PCI_FUNC(dev->devfn) != 1) {
> > +		dev->dma_alias_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 1);
> > +		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
> > +	}
> > +}
> > +
> > +/*
> > + * Marvell 88SE9123 uses function 1 as the requester ID for DMA.  In some
> > + * SKUs function 1 is present and is a legacy IDE controller, in other
> > + * SKUs this function is not present, making this a ghost requester.
> > + * https://bugzilla.kernel.org/show_bug.cgi?id=42679
> > + */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9123,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c14 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9130,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c47 + c57 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9172,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c59 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x917a,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c46 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x91a0,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c49 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9230,
> > +			 quirk_dma_func1_alias);
> > +/* https://bugs.gentoo.org/show_bug.cgi?id=497630 */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_JMICRON,
> > +			 PCI_DEVICE_ID_JMICRON_JMB388_ESD,
> > +			 quirk_dma_func1_alias);
> > +
> >  static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
> >  {
> >  	if (!PCI_FUNC(dev->devfn))
> > 

Re: [PATCH v4 06/16] PCI: Quirk pci_for_each_dma_alias() for bridges

[Alex Williamson]

On Wed, 2014-05-28 at 12:00 -0600, Bjorn Helgaas wrote:
> On Thu, May 22, 2014 at 05:08:01PM -0600, Alex Williamson wrote:
> > Several PCIe-to-PCI bridges fail to provide a PCIe capability, causing
> > us to handle them as conventional PCI devices.  In some cases, this
> > may be correct, in others it's not.  Add a dev_flag bit to identify
> > devices to be handled as standard PCIe-to-PCI bridges.
> 
> Can you expand on the "in some cases, this may be correct, in others it's
> not"?  Do you mean that for some *devices* it's correct to handle them as
> conventional PCI, or in some *situations* it's correct?  Something else?

Yes, the example that I know of is the PCI bridge found on the root
complex of many Intel chipsets.  Older generations do not have a PCIe
capability and the requester ID behaves as a conventional PCI device,
using the bridge itself as the requester ID, newer version include a
PCIe capability and use the secondary bus as the requester ID.  Those
devices work correctly without this quirk.  The bridges in the next
patch obviously don't.

> I'd like to either remove that sentence or add a little more information to
> make it useful.

Ok, I'll note some of the above in a new commit log.

> I guess this probably goes along with the tests in
> quirk_use_pcie_bridge_dma_alias().

Yep.  For that, we need to look at the upstream device of the bridge, so
it won't flag any root complex bridges like those above, which is why it
could probably be applied to any bridge.  Thanks,

Alex

> > Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> > ---
> >  drivers/pci/search.c |   10 ++++++++--
> >  include/linux/pci.h  |    2 ++
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> > index 2c19f3f..df38f73 100644
> > --- a/drivers/pci/search.c
> > +++ b/drivers/pci/search.c
> > @@ -88,8 +88,14 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
> >  				continue;
> >  			}
> >  		} else {
> > -			ret = fn(tmp, PCI_DEVID(tmp->bus->number, tmp->devfn),
> > -				 data);
> > +			if (tmp->dev_flags & PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS)
> > +				ret = fn(tmp,
> > +					 PCI_DEVID(tmp->subordinate->number,
> > +						   PCI_DEVFN(0, 0)), data);
> > +			else
> > +				ret = fn(tmp,
> > +					 PCI_DEVID(tmp->bus->number,
> > +						   tmp->devfn), data);
> >  			if (ret)
> >  				return ret;
> >  		}
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 9d4035c..85ab35e 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -173,6 +173,8 @@ enum pci_dev_flags {
> >  	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
> >  	/* Flag to indicate the device uses dma_alias_devfn */
> >  	PCI_DEV_FLAGS_DMA_ALIAS_DEVFN = (__force pci_dev_flags_t) (1 << 4),
> > +	/* Use a PCIe-to-PCI bridge alias even if !pci_is_pcie */
> > +	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 5),
> >  };
> >  
> >  enum pci_irq_reroute_variant {
> > 

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Bjorn Helgaas]

On Thu, May 22, 2014 at 05:07:23PM -0600, Alex Williamson wrote:
> For testing, this version can be found in my git tree:
> 
> git://github.com/awilliam/linux-vfio.git dma-alias-v4
> 
> Please report any issues.
> 
> v4:
>  - Change dma_func_alias to dma_alias_devfn, holding a single
>    devfn to alias, thereby supporting aliases to the wrong slot.
>    The DMA alias iterator is easily changed, but IOMMU grouping
>    requires significant rework.  This is now done in IOMMU code
>    rather than PCI code.
> 
>  - AMD-Vi - try to incorporate IVRS aliases dynamically into
>    PCI alias quirks to make sure that our grouping remains the
>    same.  Potentially this could end up reporting BIOS aliases
>    that we can add to our list of quirks.
> 
> v3:
>  - Found several instances where I had PCI_SLOT when I meant
>    PCI_FUNC.  Thanks to Andrew for spotting this.  This should
>    fix the problem he was having with Ricoh quirks.  We also
>    pruned down the func0 quirks to only those that we know are
>    needed.  We can always add them back later.
> 
>  - Found a case in intel-iommu of using dev_is_pci() where I
>    really wanted !dev_is_pci().  Fixed.
> 
> v2:
>  - Several new Marvell controllers added to quirks.  There's been
>    a lot of success reported with this series in
>    https://bugzilla.kernel.org/show_bug.cgi?id=42679
> 
>  - Add quirk for ASMedia and Tundra PCIe-to-PCI bridges that do
>    not expose a PCIe capability.  These have been shown to use
>    the standard PCIe-to-PCI bridge requester ID.
> 
>  - Fix copy/paste duplicate Ricoh quirk ID
> 
>  - Fixed AMD IOMMU for the "ghost" function case where the DMA
>    alias is for an absent device.  The iommu rlookup table and
>    data fields need to be initializes.
> 
>  - Fixed Intel interrupt remapping, I wasn't passing the target
>    bus number, only the alias bus number.
> 
> These patches are split across PCI and IOMMU, but I've front-loaded
> all of the PCI infrastructure so that the first 7 patches can be
> applied to PCI-core, the IOMMU maintainers can pickup their patches,
> then we can finish with dead code removal.  Bjorn might also be
> willing to carry the IOMMU changes if the maintainers want to ack
> them.

I put 1-7 on a pci/iommu branch for v3.16.  I'm happy to include the rest,
too, given acks from Joerg and David.  Or if they prefer to take them all,
which might be easier than coordinating two trees, especially since there's
PCI stuff at the beginning and end, here's my ack for the PCI bits (patches
1-7 and 15-16):

    Acked-by: Bjorn Helgaas <bhelgaas@google.com>

If you want to send me updated changelogs for patches 5 & 6, I'll drop
those in.

Didn't you have more testing reports?  I see George's, but I thought there
were some others, too.

> Original description:
> 
> This series attempts to fix a couple issues we've had outstanding in
> the PCI/IOMMU code for a while.  The first issue is with devices that
> use the wrong requester ID for DMA transactions.  We already have a
> sort of half-baked attempt to fix this for several Ricoh devices, but
> the fix only helps them be useful through IOMMU groups, not the
> general DMA case.  There are also several Marvell devices which use
> use a different wrong requester ID and don't even fit into the DMA
> source idea.  This series creates a DMA alias iterator that will
> step through each possible alias of a device, allowing IOMMUs to
> insert mappings for both the device and its aliases.
> 
> Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge()
> function, which is known to blowup when it finds itself suddenly at
> a PCIe device without crossing a PCIe-to-PCI bridge (as identified by
> the PCIe capability).  It also likes to make the invalid assumption
> that a PCIe device never has its requester ID masked by any usptream
> bus.  We can fix this using the above new DMA alias iterator, since
> that's effectively what this function was meant to do.
> 
> Finally, with all these helpers, it makes sense to consolidate code
> for determining IOMMU groups.  The first step in finding the root
> of a group is finding the final upstream DMA alias for the device,
> then applying additional ACS rules and incorporating device specific
> aliases.  As this is all common to PCI, create a single implementation
> and remove piles of code from the individual IOMMU drivers.
> 
> This series allows devices like the Marvell 88SE9123 to finally work
> on Linux with either AMD-Vi or VT-d enabled on the box.  I've
> collected device IDs from various bugs to support as many SKUs of
> these devices as possible, but I'm sure there are others that I've
> missed.
> 
> This should also enable motherboards with an onboard ASmedia
> ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled.  I've
> acquired an adapter board with this chip, but it actually exposes
> a PCIe capability, unlike most of the onboard controllers.  Therefore
> I expect this series will fix the WARN_ON currently hit during boot,
> but there's a 50/50 chance whether the device behaves like a PCI
> bridge or a PCIe bridge with regard to the requester ID that it uses
> to take ownership of the transaction.  If it turns out to use the
> PCIe bridge model, I expect we can quirk it using a dev_flags bit
> to identify a PCI bridge that takes ownership as if it was a PCIe
> bridge.
> 
> Please test and provide feedback.  I expect IOMMU group topology
> should not change from this series, but if a case is found where it
> does, please share.  Also, if there are additional quirks we need
> to add, please either file new or add to the existing bugs.  Thanks,
> 
> Alex
> 
> ---
> 
> Alex Williamson (16):
>       PCI: Add DMA alias iterator
>       PCI: define pci_dev_flags as bit shifts
>       PCI: quirk pci_for_each_dma_alias()
>       PCI: quirk dma_alias_devfn for Ricoh devices
>       PCI: quirk dma_alias_devfn for Marvell devices
>       PCI: Quirk pci_for_each_dma_alias() for bridges
>       PCI: Add quirks for ASMedia and Tundra bridges
>       iommu: Create central IOMMU group lookup/creation interface
>       iommu/amd: Update to use PCI DMA aliases
>       iommu/amd: Use iommu_group_get_for_dev()
>       iommu/intel: Use iommu_group_get_for_dev()
>       iommu/intel: Update to use PCI DMA aliases
>       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
>       iommu: Remove pci.h
>       PCI: Remove pci_find_upstream_pcie_bridge()
>       PCI: Remove pci_get_dma_source()
> 
> 
>  drivers/iommu/amd_iommu.c           |  214 +++++++-----------------
>  drivers/iommu/amd_iommu_types.h     |    1 
>  drivers/iommu/fsl_pamu_domain.c     |   66 --------
>  drivers/iommu/intel-iommu.c         |  307 +++++++++++++----------------------
>  drivers/iommu/intel_irq_remapping.c |   55 ++++--
>  drivers/iommu/iommu.c               |  181 +++++++++++++++++++++
>  drivers/iommu/pci.h                 |   29 ---
>  drivers/pci/quirks.c                |  116 ++++++++-----
>  drivers/pci/search.c                |  104 +++++++++---
>  include/linux/iommu.h               |    1 
>  include/linux/pci.h                 |   31 +---
>  11 files changed, 557 insertions(+), 548 deletions(-)
>  delete mode 100644 drivers/iommu/pci.h

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Alex Williamson]

On Wed, 2014-05-28 at 14:29 -0600, Bjorn Helgaas wrote:
> On Thu, May 22, 2014 at 05:07:23PM -0600, Alex Williamson wrote:
> > For testing, this version can be found in my git tree:
> > 
> > git://github.com/awilliam/linux-vfio.git dma-alias-v4
> > 
> > Please report any issues.
> > 
> > v4:
> >  - Change dma_func_alias to dma_alias_devfn, holding a single
> >    devfn to alias, thereby supporting aliases to the wrong slot.
> >    The DMA alias iterator is easily changed, but IOMMU grouping
> >    requires significant rework.  This is now done in IOMMU code
> >    rather than PCI code.
> > 
> >  - AMD-Vi - try to incorporate IVRS aliases dynamically into
> >    PCI alias quirks to make sure that our grouping remains the
> >    same.  Potentially this could end up reporting BIOS aliases
> >    that we can add to our list of quirks.
> > 
> > v3:
> >  - Found several instances where I had PCI_SLOT when I meant
> >    PCI_FUNC.  Thanks to Andrew for spotting this.  This should
> >    fix the problem he was having with Ricoh quirks.  We also
> >    pruned down the func0 quirks to only those that we know are
> >    needed.  We can always add them back later.
> > 
> >  - Found a case in intel-iommu of using dev_is_pci() where I
> >    really wanted !dev_is_pci().  Fixed.
> > 
> > v2:
> >  - Several new Marvell controllers added to quirks.  There's been
> >    a lot of success reported with this series in
> >    https://bugzilla.kernel.org/show_bug.cgi?id=42679
> > 
> >  - Add quirk for ASMedia and Tundra PCIe-to-PCI bridges that do
> >    not expose a PCIe capability.  These have been shown to use
> >    the standard PCIe-to-PCI bridge requester ID.
> > 
> >  - Fix copy/paste duplicate Ricoh quirk ID
> > 
> >  - Fixed AMD IOMMU for the "ghost" function case where the DMA
> >    alias is for an absent device.  The iommu rlookup table and
> >    data fields need to be initializes.
> > 
> >  - Fixed Intel interrupt remapping, I wasn't passing the target
> >    bus number, only the alias bus number.
> > 
> > These patches are split across PCI and IOMMU, but I've front-loaded
> > all of the PCI infrastructure so that the first 7 patches can be
> > applied to PCI-core, the IOMMU maintainers can pickup their patches,
> > then we can finish with dead code removal.  Bjorn might also be
> > willing to carry the IOMMU changes if the maintainers want to ack
> > them.
> 
> I put 1-7 on a pci/iommu branch for v3.16.  I'm happy to include the rest,
> too, given acks from Joerg and David.  Or if they prefer to take them all,
> which might be easier than coordinating two trees, especially since there's
> PCI stuff at the beginning and end, here's my ack for the PCI bits (patches
> 1-7 and 15-16):
> 
>     Acked-by: Bjorn Helgaas <bhelgaas@google.com>

Awesome, thanks!

> If you want to send me updated changelogs for patches 5 & 6, I'll drop
> those in.

Yep, I'll send changelog updates for 5 & 6 in a few minutes.

> Didn't you have more testing reports?  I see George's, but I thought there
> were some others, too.

I also see:

https://lkml.org/lkml/2014/5/28/24
Tested-by: Pat Erley <pat-lkml@erley.org>

There are several reports of success in the bzs, but I don't see any
official tested-by tags there.  Thanks,

Alex

> > Original description:
> > 
> > This series attempts to fix a couple issues we've had outstanding in
> > the PCI/IOMMU code for a while.  The first issue is with devices that
> > use the wrong requester ID for DMA transactions.  We already have a
> > sort of half-baked attempt to fix this for several Ricoh devices, but
> > the fix only helps them be useful through IOMMU groups, not the
> > general DMA case.  There are also several Marvell devices which use
> > use a different wrong requester ID and don't even fit into the DMA
> > source idea.  This series creates a DMA alias iterator that will
> > step through each possible alias of a device, allowing IOMMUs to
> > insert mappings for both the device and its aliases.
> > 
> > Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge()
> > function, which is known to blowup when it finds itself suddenly at
> > a PCIe device without crossing a PCIe-to-PCI bridge (as identified by
> > the PCIe capability).  It also likes to make the invalid assumption
> > that a PCIe device never has its requester ID masked by any usptream
> > bus.  We can fix this using the above new DMA alias iterator, since
> > that's effectively what this function was meant to do.
> > 
> > Finally, with all these helpers, it makes sense to consolidate code
> > for determining IOMMU groups.  The first step in finding the root
> > of a group is finding the final upstream DMA alias for the device,
> > then applying additional ACS rules and incorporating device specific
> > aliases.  As this is all common to PCI, create a single implementation
> > and remove piles of code from the individual IOMMU drivers.
> > 
> > This series allows devices like the Marvell 88SE9123 to finally work
> > on Linux with either AMD-Vi or VT-d enabled on the box.  I've
> > collected device IDs from various bugs to support as many SKUs of
> > these devices as possible, but I'm sure there are others that I've
> > missed.
> > 
> > This should also enable motherboards with an onboard ASmedia
> > ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled.  I've
> > acquired an adapter board with this chip, but it actually exposes
> > a PCIe capability, unlike most of the onboard controllers.  Therefore
> > I expect this series will fix the WARN_ON currently hit during boot,
> > but there's a 50/50 chance whether the device behaves like a PCI
> > bridge or a PCIe bridge with regard to the requester ID that it uses
> > to take ownership of the transaction.  If it turns out to use the
> > PCIe bridge model, I expect we can quirk it using a dev_flags bit
> > to identify a PCI bridge that takes ownership as if it was a PCIe
> > bridge.
> > 
> > Please test and provide feedback.  I expect IOMMU group topology
> > should not change from this series, but if a case is found where it
> > does, please share.  Also, if there are additional quirks we need
> > to add, please either file new or add to the existing bugs.  Thanks,
> > 
> > Alex
> > 
> > ---
> > 
> > Alex Williamson (16):
> >       PCI: Add DMA alias iterator
> >       PCI: define pci_dev_flags as bit shifts
> >       PCI: quirk pci_for_each_dma_alias()
> >       PCI: quirk dma_alias_devfn for Ricoh devices
> >       PCI: quirk dma_alias_devfn for Marvell devices
> >       PCI: Quirk pci_for_each_dma_alias() for bridges
> >       PCI: Add quirks for ASMedia and Tundra bridges
> >       iommu: Create central IOMMU group lookup/creation interface
> >       iommu/amd: Update to use PCI DMA aliases
> >       iommu/amd: Use iommu_group_get_for_dev()
> >       iommu/intel: Use iommu_group_get_for_dev()
> >       iommu/intel: Update to use PCI DMA aliases
> >       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
> >       iommu: Remove pci.h
> >       PCI: Remove pci_find_upstream_pcie_bridge()
> >       PCI: Remove pci_get_dma_source()
> > 
> > 
> >  drivers/iommu/amd_iommu.c           |  214 +++++++-----------------
> >  drivers/iommu/amd_iommu_types.h     |    1 
> >  drivers/iommu/fsl_pamu_domain.c     |   66 --------
> >  drivers/iommu/intel-iommu.c         |  307 +++++++++++++----------------------
> >  drivers/iommu/intel_irq_remapping.c |   55 ++++--
> >  drivers/iommu/iommu.c               |  181 +++++++++++++++++++++
> >  drivers/iommu/pci.h                 |   29 ---
> >  drivers/pci/quirks.c                |  116 ++++++++-----
> >  drivers/pci/search.c                |  104 +++++++++---
> >  include/linux/iommu.h               |    1 
> >  include/linux/pci.h                 |   31 +---
> >  11 files changed, 557 insertions(+), 548 deletions(-)
> >  delete mode 100644 drivers/iommu/pci.h

[PATCH v4.1 05/16] PCI: quirk dma_alias_devfn for Marvell devices

[Alex Williamson]

Several Marvell devices and a JMicron device have a similar DMA
requester ID problem to Ricoh, except they use function 1 as the
PCIe requester ID.  Add a quirk for these to populate the DMA
alias with the correct devfn.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

v4.1: minor commitlog tweak, no code chage

 drivers/pci/quirks.c |   36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index bc8ebd9..923689f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3349,6 +3349,42 @@ static void quirk_dma_func0_alias(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe832, quirk_dma_func0_alias);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_RICOH, 0xe476, quirk_dma_func0_alias);
 
+static void quirk_dma_func1_alias(struct pci_dev *dev)
+{
+	if (PCI_FUNC(dev->devfn) != 1) {
+		dev->dma_alias_devfn = PCI_DEVFN(PCI_SLOT(dev->devfn), 1);
+		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
+	}
+}
+
+/*
+ * Marvell 88SE9123 uses function 1 as the requester ID for DMA.  In some
+ * SKUs function 1 is present and is a legacy IDE controller, in other
+ * SKUs this function is not present, making this a ghost requester.
+ * https://bugzilla.kernel.org/show_bug.cgi?id=42679
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9123,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c14 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9130,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c47 + c57 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9172,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c59 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x917a,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c46 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x91a0,
+			 quirk_dma_func1_alias);
+/* https://bugzilla.kernel.org/show_bug.cgi?id=42679#c49 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL_EXT, 0x9230,
+			 quirk_dma_func1_alias);
+/* https://bugs.gentoo.org/show_bug.cgi?id=497630 */
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_JMICRON,
+			 PCI_DEVICE_ID_JMICRON_JMB388_ESD,
+			 quirk_dma_func1_alias);
+
 static struct pci_dev *pci_func_0_dma_source(struct pci_dev *dev)
 {
 	if (!PCI_FUNC(dev->devfn))

[PATCH v4.1 06/16] PCI: Quirk pci_for_each_dma_alias() for bridges

[Alex Williamson]

Several PCIe-to-PCI bridges fail to provide a PCIe capability, causing
us to handle them as conventional PCI devices when they really use the
requester ID of the secondary bus.  We need to differentiate these
from PCIe-to-PCI bridges that actually use the conventional PCI ID
when a PCIe capability is not present, such as those found on the root
complex of may Intel chipsets.  Add a dev_flag bit to identify devices
to be handled as standard PCIe-to-PCI bridges.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

v4.1: expand commitlog, no code change

 drivers/pci/search.c |   10 ++++++++--
 include/linux/pci.h  |    2 ++
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 2c19f3f..df38f73 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -88,8 +88,14 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 				continue;
 			}
 		} else {
-			ret = fn(tmp, PCI_DEVID(tmp->bus->number, tmp->devfn),
-				 data);
+			if (tmp->dev_flags & PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS)
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->subordinate->number,
+						   PCI_DEVFN(0, 0)), data);
+			else
+				ret = fn(tmp,
+					 PCI_DEVID(tmp->bus->number,
+						   tmp->devfn), data);
 			if (ret)
 				return ret;
 		}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9d4035c..85ab35e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -173,6 +173,8 @@ enum pci_dev_flags {
 	PCI_DEV_FLAGS_ACS_ENABLED_QUIRK = (__force pci_dev_flags_t) (1 << 3),
 	/* Flag to indicate the device uses dma_alias_devfn */
 	PCI_DEV_FLAGS_DMA_ALIAS_DEVFN = (__force pci_dev_flags_t) (1 << 4),
+	/* Use a PCIe-to-PCI bridge alias even if !pci_is_pcie */
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 5),
 };
 
 enum pci_irq_reroute_variant {

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Andrew Cooks]

On Thu, May 29, 2014 at 4:29 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, May 22, 2014 at 05:07:23PM -0600, Alex Williamson wrote:
>> For testing, this version can be found in my git tree:
>>
>> git://github.com/awilliam/linux-vfio.git dma-alias-v4
>>
>> Please report any issues.
>>
>> v4:
>>  - Change dma_func_alias to dma_alias_devfn, holding a single
>>    devfn to alias, thereby supporting aliases to the wrong slot.
>>    The DMA alias iterator is easily changed, but IOMMU grouping
>>    requires significant rework.  This is now done in IOMMU code
>>    rather than PCI code.
>>
>>  - AMD-Vi - try to incorporate IVRS aliases dynamically into
>>    PCI alias quirks to make sure that our grouping remains the
>>    same.  Potentially this could end up reporting BIOS aliases
>>    that we can add to our list of quirks.
>>
>> v3:
>>  - Found several instances where I had PCI_SLOT when I meant
>>    PCI_FUNC.  Thanks to Andrew for spotting this.  This should
>>    fix the problem he was having with Ricoh quirks.  We also
>>    pruned down the func0 quirks to only those that we know are
>>    needed.  We can always add them back later.
>>
>>  - Found a case in intel-iommu of using dev_is_pci() where I
>>    really wanted !dev_is_pci().  Fixed.
>>
>> v2:
>>  - Several new Marvell controllers added to quirks.  There's been
>>    a lot of success reported with this series in
>>    https://bugzilla.kernel.org/show_bug.cgi?id=42679
>>
>>  - Add quirk for ASMedia and Tundra PCIe-to-PCI bridges that do
>>    not expose a PCIe capability.  These have been shown to use
>>    the standard PCIe-to-PCI bridge requester ID.
>>
>>  - Fix copy/paste duplicate Ricoh quirk ID
>>
>>  - Fixed AMD IOMMU for the "ghost" function case where the DMA
>>    alias is for an absent device.  The iommu rlookup table and
>>    data fields need to be initializes.
>>
>>  - Fixed Intel interrupt remapping, I wasn't passing the target
>>    bus number, only the alias bus number.
>>
>> These patches are split across PCI and IOMMU, but I've front-loaded
>> all of the PCI infrastructure so that the first 7 patches can be
>> applied to PCI-core, the IOMMU maintainers can pickup their patches,
>> then we can finish with dead code removal.  Bjorn might also be
>> willing to carry the IOMMU changes if the maintainers want to ack
>> them.
>
> I put 1-7 on a pci/iommu branch for v3.16.  I'm happy to include the rest,
> too, given acks from Joerg and David.  Or if they prefer to take them all,
> which might be easier than coordinating two trees, especially since there's
> PCI stuff at the beginning and end, here's my ack for the PCI bits (patches
> 1-7 and 15-16):
>
>     Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> If you want to send me updated changelogs for patches 5 & 6, I'll drop
> those in.
>
> Didn't you have more testing reports?  I see George's, but I thought there
> were some others, too.
>
Tested-by: Andrew Cooks <acooks@linux.com>

I've reviewed parts of this patch set, over multiple iterations, if
that's worth anything.

>> Original description:
>>
>> This series attempts to fix a couple issues we've had outstanding in
>> the PCI/IOMMU code for a while.  The first issue is with devices that
>> use the wrong requester ID for DMA transactions.  We already have a
>> sort of half-baked attempt to fix this for several Ricoh devices, but
>> the fix only helps them be useful through IOMMU groups, not the
>> general DMA case.  There are also several Marvell devices which use
>> use a different wrong requester ID and don't even fit into the DMA
>> source idea.  This series creates a DMA alias iterator that will
>> step through each possible alias of a device, allowing IOMMUs to
>> insert mappings for both the device and its aliases.
>>
>> Hand-in-hand with this is our broken pci_find_upstream_pcie_bridge()
>> function, which is known to blowup when it finds itself suddenly at
>> a PCIe device without crossing a PCIe-to-PCI bridge (as identified by
>> the PCIe capability).  It also likes to make the invalid assumption
>> that a PCIe device never has its requester ID masked by any usptream
>> bus.  We can fix this using the above new DMA alias iterator, since
>> that's effectively what this function was meant to do.
>>
>> Finally, with all these helpers, it makes sense to consolidate code
>> for determining IOMMU groups.  The first step in finding the root
>> of a group is finding the final upstream DMA alias for the device,
>> then applying additional ACS rules and incorporating device specific
>> aliases.  As this is all common to PCI, create a single implementation
>> and remove piles of code from the individual IOMMU drivers.
>>
>> This series allows devices like the Marvell 88SE9123 to finally work
>> on Linux with either AMD-Vi or VT-d enabled on the box.  I've
>> collected device IDs from various bugs to support as many SKUs of
>> these devices as possible, but I'm sure there are others that I've
>> missed.
>>
>> This should also enable motherboards with an onboard ASmedia
>> ASM1083/1085 PCIe-to-PCI bridge to work with VT-d enabled.  I've
>> acquired an adapter board with this chip, but it actually exposes
>> a PCIe capability, unlike most of the onboard controllers.  Therefore
>> I expect this series will fix the WARN_ON currently hit during boot,
>> but there's a 50/50 chance whether the device behaves like a PCI
>> bridge or a PCIe bridge with regard to the requester ID that it uses
>> to take ownership of the transaction.  If it turns out to use the
>> PCIe bridge model, I expect we can quirk it using a dev_flags bit
>> to identify a PCI bridge that takes ownership as if it was a PCIe
>> bridge.
>>
>> Please test and provide feedback.  I expect IOMMU group topology
>> should not change from this series, but if a case is found where it
>> does, please share.  Also, if there are additional quirks we need
>> to add, please either file new or add to the existing bugs.  Thanks,
>>
>> Alex
>>
>> ---
>>
>> Alex Williamson (16):
>>       PCI: Add DMA alias iterator
>>       PCI: define pci_dev_flags as bit shifts
>>       PCI: quirk pci_for_each_dma_alias()
>>       PCI: quirk dma_alias_devfn for Ricoh devices
>>       PCI: quirk dma_alias_devfn for Marvell devices
>>       PCI: Quirk pci_for_each_dma_alias() for bridges
>>       PCI: Add quirks for ASMedia and Tundra bridges
>>       iommu: Create central IOMMU group lookup/creation interface
>>       iommu/amd: Update to use PCI DMA aliases
>>       iommu/amd: Use iommu_group_get_for_dev()
>>       iommu/intel: Use iommu_group_get_for_dev()
>>       iommu/intel: Update to use PCI DMA aliases
>>       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
>>       iommu: Remove pci.h
>>       PCI: Remove pci_find_upstream_pcie_bridge()
>>       PCI: Remove pci_get_dma_source()
>>
>>
>>  drivers/iommu/amd_iommu.c           |  214 +++++++-----------------
>>  drivers/iommu/amd_iommu_types.h     |    1
>>  drivers/iommu/fsl_pamu_domain.c     |   66 --------
>>  drivers/iommu/intel-iommu.c         |  307 +++++++++++++----------------------
>>  drivers/iommu/intel_irq_remapping.c |   55 ++++--
>>  drivers/iommu/iommu.c               |  181 +++++++++++++++++++++
>>  drivers/iommu/pci.h                 |   29 ---
>>  drivers/pci/quirks.c                |  116 ++++++++-----
>>  drivers/pci/search.c                |  104 +++++++++---
>>  include/linux/iommu.h               |    1
>>  include/linux/pci.h                 |   31 +---
>>  11 files changed, 557 insertions(+), 548 deletions(-)
>>  delete mode 100644 drivers/iommu/pci.h

RE: [PATCH v4 13/16] iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups

[Varun Sethi]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4075 bytes --]



> -----Original Message-----
> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Friday, May 23, 2014 4:39 AM
> To: linux-pci@vger.kernel.org; iommu@lists.linux-foundation.org
> Cc: linux-kernel@vger.kernel.org; acooks@gmail.com; linux@horizon.com;
> bhelgaas@google.com; Sethi Varun-B16395; eddy0596@gmail.com
> Subject: [PATCH v4 13/16] iommu/fsl: Use iommu_group_get_for_dev() for
> IOMMU groups
> 
> Drop custom code and use IOMMU provided grouping support for PCI.
> 
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> Cc: Varun Sethi <varun.sethi@freescale.com>
> ---
>  drivers/iommu/fsl_pamu_domain.c |   66 +--------------------------------
> ------
>  1 file changed, 1 insertion(+), 65 deletions(-)
> 
> diff --git a/drivers/iommu/fsl_pamu_domain.c
> b/drivers/iommu/fsl_pamu_domain.c index 93072ba..d02d668 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -38,7 +38,6 @@
>  #include <sysdev/fsl_pci.h>
> 
>  #include "fsl_pamu_domain.h"
> -#include "pci.h"
> 
>  /*
>   * Global spinlock that needs to be held while @@ -892,8 +891,6 @@
> static int fsl_pamu_get_domain_attr(struct iommu_domain *domain,
>  	return ret;
>  }
> 
> -#define REQ_ACS_FLAGS	(PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR |
> PCI_ACS_UF)
> -
>  static struct iommu_group *get_device_iommu_group(struct device *dev)  {
>  	struct iommu_group *group;
> @@ -950,74 +947,13 @@ static struct iommu_group
> *get_pci_device_group(struct pci_dev *pdev)
>  	struct pci_controller *pci_ctl;
>  	bool pci_endpt_partioning;
>  	struct iommu_group *group = NULL;
> -	struct pci_dev *bridge, *dma_pdev = NULL;
> 
>  	pci_ctl = pci_bus_to_host(pdev->bus);
>  	pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
>  	/* We can partition PCIe devices so assign device group to the
> device */
>  	if (pci_endpt_partioning) {
> -		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);
> -
> -		/* Account for quirked devices */
> -		swap_pci_ref(&dma_pdev, pci_get_dma_source(dma_pdev));
> -
> -		/*
> -		 * If it's a multifunction device that does not support our
> -		 * required ACS flags, add to the same group as lowest
> numbered
> -		 * function that also does not suport the required ACS flags.
> -		 */
> -		if (dma_pdev->multifunction &&
> -		    !pci_acs_enabled(dma_pdev, REQ_ACS_FLAGS)) {
> -			u8 i, slot = PCI_SLOT(dma_pdev->devfn);
> -
> -			for (i = 0; i < 8; i++) {
> -				struct pci_dev *tmp;
> -
> -				tmp = pci_get_slot(dma_pdev->bus, PCI_DEVFN(slot,
> i));
> -				if (!tmp)
> -					continue;
> -
> -				if (!pci_acs_enabled(tmp, REQ_ACS_FLAGS)) {
> -					swap_pci_ref(&dma_pdev, tmp);
> -					break;
> -				}
> -				pci_dev_put(tmp);
> -			}
> -		}
> -
> -		/*
> -		 * Devices on the root bus go through the iommu.  If that's
> not us,
> -		 * find the next upstream device and test ACS up to the root
> bus.
> -		 * Finding the next device may require skipping virtual
> buses.
> -		 */
> -		while (!pci_is_root_bus(dma_pdev->bus)) {
> -			struct pci_bus *bus = dma_pdev->bus;
> -
> -			while (!bus->self) {
> -				if (!pci_is_root_bus(bus))
> -					bus = bus->parent;
> -				else
> -					goto root_bus;
> -			}
> -
> -			if (pci_acs_path_enabled(bus->self, NULL,
> REQ_ACS_FLAGS))
> -				break;
> -
> -			swap_pci_ref(&dma_pdev, pci_dev_get(bus->self));
> -		}
> +		group = iommu_group_get_for_dev(&pdev->dev);
> 
> -root_bus:
> -		group = get_device_iommu_group(&dma_pdev->dev);
> -		pci_dev_put(dma_pdev);
>  		/*
>  		 * PCIe controller is not a paritionable entity
>  		 * free the controller device iommu_group.
Acknowledged-by: Varun Sethi <varun.sethi@freescale.com>
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Alex Williamson]

On Thu, 2014-05-22 at 17:07 -0600, Alex Williamson wrote:
> Alex Williamson (16):
>       PCI: Add DMA alias iterator
>       PCI: define pci_dev_flags as bit shifts
>       PCI: quirk pci_for_each_dma_alias()
>       PCI: quirk dma_alias_devfn for Ricoh devices
>       PCI: quirk dma_alias_devfn for Marvell devices
>       PCI: Quirk pci_for_each_dma_alias() for bridges
>       PCI: Add quirks for ASMedia and Tundra bridges


Hi Joerg & David,

Bjorn has accepted the above patches for v3.16 and they currently live
in his pci/iommu branch.  This series has been shown by numerous users
to make devices with buggy DMA issues work in the presence of an IOMMU
and it would be really great if we could get an opinion on both the
common IOMMU changes as well as the changes for AMD-Vi and Intel VT-d.
Varun has already ack'd the changes for fsl.  Please let us know how
you'd like to proceed.  Can we get acks and allow Bjorn to shepherd the
whole series in?  Would you prefer to pull the changes via your
respective trees and let Bjorn follow-up with the code removal?  Do you
have any outstanding issues with the patches to your areas?  Thanks,

Alex

>       iommu: Create central IOMMU group lookup/creation interface
>       iommu/amd: Update to use PCI DMA aliases
>       iommu/amd: Use iommu_group_get_for_dev()
>       iommu/intel: Use iommu_group_get_for_dev()
>       iommu/intel: Update to use PCI DMA aliases
>       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
>       iommu: Remove pci.h
>       PCI: Remove pci_find_upstream_pcie_bridge()
>       PCI: Remove pci_get_dma_source()
> 
> 
>  drivers/iommu/amd_iommu.c           |  214 +++++++-----------------
>  drivers/iommu/amd_iommu_types.h     |    1 
>  drivers/iommu/fsl_pamu_domain.c     |   66 --------
>  drivers/iommu/intel-iommu.c         |  307 +++++++++++++----------------------
>  drivers/iommu/intel_irq_remapping.c |   55 ++++--
>  drivers/iommu/iommu.c               |  181 +++++++++++++++++++++
>  drivers/iommu/pci.h                 |   29 ---
>  drivers/pci/quirks.c                |  116 ++++++++-----
>  drivers/pci/search.c                |  104 +++++++++---
>  include/linux/iommu.h               |    1 
>  include/linux/pci.h                 |   31 +---
>  11 files changed, 557 insertions(+), 548 deletions(-)
>  delete mode 100644 drivers/iommu/pci.h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Joerg Roedel]

Hi Alex,

On Thu, May 22, 2014 at 05:07:23PM -0600, Alex Williamson wrote:
> Alex Williamson (16):
>       PCI: Add DMA alias iterator
>       PCI: define pci_dev_flags as bit shifts
>       PCI: quirk pci_for_each_dma_alias()
>       PCI: quirk dma_alias_devfn for Ricoh devices
>       PCI: quirk dma_alias_devfn for Marvell devices
>       PCI: Quirk pci_for_each_dma_alias() for bridges
>       PCI: Add quirks for ASMedia and Tundra bridges
>       iommu: Create central IOMMU group lookup/creation interface
>       iommu/amd: Update to use PCI DMA aliases
>       iommu/amd: Use iommu_group_get_for_dev()
>       iommu/intel: Use iommu_group_get_for_dev()
>       iommu/intel: Update to use PCI DMA aliases
>       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
>       iommu: Remove pci.h
>       PCI: Remove pci_find_upstream_pcie_bridge()
>       PCI: Remove pci_get_dma_source()

Sorry for the delay, I had a look at the generic IOMMU and the AMD part
now. It looks good to me so far, but I still have to review the VT-d
changes and give it all some testing on my machines. I really like the
code simplification in the IOMMU drivers and also feel more comfortable
when the IVRS table is still taken into consideration for getting
aliases.


	Joerg

Re: [PATCH v4 00/16] PCI/iommu: Fix DMA alias problems

[Alex Williamson]

On Mon, 2014-06-16 at 16:47 +0200, Joerg Roedel wrote:
> Hi Alex,
> 
> On Thu, May 22, 2014 at 05:07:23PM -0600, Alex Williamson wrote:
> > Alex Williamson (16):
> >       PCI: Add DMA alias iterator
> >       PCI: define pci_dev_flags as bit shifts
> >       PCI: quirk pci_for_each_dma_alias()
> >       PCI: quirk dma_alias_devfn for Ricoh devices
> >       PCI: quirk dma_alias_devfn for Marvell devices
> >       PCI: Quirk pci_for_each_dma_alias() for bridges
> >       PCI: Add quirks for ASMedia and Tundra bridges
> >       iommu: Create central IOMMU group lookup/creation interface
> >       iommu/amd: Update to use PCI DMA aliases
> >       iommu/amd: Use iommu_group_get_for_dev()
> >       iommu/intel: Use iommu_group_get_for_dev()
> >       iommu/intel: Update to use PCI DMA aliases
> >       iommu/fsl: Use iommu_group_get_for_dev() for IOMMU groups
> >       iommu: Remove pci.h
> >       PCI: Remove pci_find_upstream_pcie_bridge()
> >       PCI: Remove pci_get_dma_source()
> 
> Sorry for the delay, I had a look at the generic IOMMU and the AMD part
> now. It looks good to me so far, but I still have to review the VT-d
> changes and give it all some testing on my machines. I really like the
> code simplification in the IOMMU drivers and also feel more comfortable
> when the IVRS table is still taken into consideration for getting
> aliases.

Great, thanks Joerg.  Let me know if you'd like a new series with just
the IOMMU bits.  Thanks,

Alex