[PATCH v2 1/2] pci: Add support for multiple DMA aliases

[PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Jacek Lawrynowicz]

This patch solves IOMMU support issues with PCIe non-transparent bridges
that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
the bridge, packet's RID is rewritten according to LUT programmed by
a driver. Modified packets are then passed to a destination bus and
processed upstream. The problem is that such packets seem to come from
non-existent nodes that are hidden behind NTB and are not discoverable
by a destination node, so IOMMU discards them. Adding DMA alias for a
given LUT entry allows IOMMU to create a proper mapping that enables
inter-node communication.

The current DMA alias implementation supports only single alias, so it's
not possible to connect more than two nodes when IOMMU is enabled. This
implementation enables all possible aliases on a given bus (256) that
are stored in a bitset. Alias devfn is directly translated to a bit
number. The bitset is not allocated for devices that have no need for
DMA aliases.

More details can be found in following article:
http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
---
 drivers/iommu/iommu.c | 11 ++++-------
 drivers/pci/pci.c     | 28 ++++++++++++++++++++++++++++
 drivers/pci/pci.h     |  2 ++
 drivers/pci/probe.c   |  1 +
 drivers/pci/quirks.c  | 16 +++++++---------
 drivers/pci/search.c  | 14 +++++++++-----
 include/linux/pci.h   | 15 ++++++++-------
 7 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..93a89ed 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 }
 
 /*
- * Look for aliases to or from the given device for exisiting groups.  The
- * dma_alias_devfn only supports aliases on the same bus, therefore the search
+ * Look for aliases to or from the given device for exisiting groups. DMA
+ * aliases are only supported on the same bus, therefore the search
  * space is quite small (especially since we're really only looking at pcie
  * device, and therefore only expect multiple slots on the root complex or
  * downstream switch ports).  It's conceivable though that a pair of
@@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 			continue;
 
 		/* We alias them or they alias us */
-		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     pdev->dma_alias_devfn == tmp->devfn) ||
-		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     tmp->dma_alias_devfn == pdev->devfn)) {
-
+		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
+		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
 			group = get_pci_alias_group(tmp, devfns);
 			if (group) {
 				pci_dev_put(tmp);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 602eb42..f114068 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4568,6 +4568,34 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 	return 0;
 }
 
+/**
+ * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
+ * @dev: the PCI device for which alias is added
+ * @devfn: alias slot and function
+ *
+ * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
+ * It should be called early, preferably as PCI fixup header quirk.
+ */
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
+{
+	if (!dev->dma_alias_mask)
+		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
+					      sizeof(long), GFP_KERNEL);
+	if (!dev->dma_alias_mask) {
+		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
+		return;
+	}
+	if (dev->dma_alias_mask)
+		set_bit(devfn, dev->dma_alias_mask);
+}
+
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
+{
+	return dev->dma_alias_mask &&
+	       test_bit(devfn, dev->dma_alias_mask);
+}
+EXPORT_SYMBOL_GPL(pci_dma_alias_is_enabled);
+
 bool pci_device_is_present(struct pci_dev *pdev)
 {
 	u32 v;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9a1660f..c31c1a5 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -335,4 +335,6 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6d7ab9b..23fc397 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1503,6 +1503,7 @@ static void pci_release_dev(struct device *dev)
 	pcibios_release_device(pci_dev);
 	pci_bus_put(pci_dev->bus);
 	kfree(pci_dev->driver_override);
+	kfree(pci_dev->dma_alias_mask);
 	kfree(pci_dev);
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0575a1e..b094061 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -25,6 +25,7 @@
 #include <linux/sched.h>
 #include <linux/ktime.h>
 #include <linux/mm.h>
+#include <linux/iommu.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
 #include "pci.h"
 
@@ -3582,8 +3583,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	}
 }
 
@@ -3598,8 +3598,7 @@ 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
 	}
 }
 
@@ -3667,11 +3666,10 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id) {
-		dev->dma_alias_devfn = id->driver_data;
-		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
-		dev_info(&dev->dev, "Enabling fixed DMA alias to %02x.%d\n",
-			 PCI_SLOT(dev->dma_alias_devfn),
-			 PCI_FUNC(dev->dma_alias_devfn));
+		pci_enable_dma_alias(dev, id->driver_data);
+		dev_info(&dev->dev, "Enabling fixed DMA alias to %02lx.%ld\n",
+			 PCI_SLOT(id->driver_data),
+			 PCI_FUNC(id->driver_data));
 	}
 }
 
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a20ce7d..33e0f03 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -40,11 +40,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	 * 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;
+	if (unlikely(pdev->dma_alias_mask)) {
+		u8 devfn;
+
+		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
+			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
+				 data);
+			if (ret)
+				return ret;
+		}
 	}
 
 	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 27df4a6..6c9e9e6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -172,16 +172,14 @@ 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),
 	/* 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),
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 4),
 	/* Do not use bus resets for device */
-	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
+	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 5),
 	/* Do not use PM reset even if device advertises NoSoftRst- */
-	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
+	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 6),
 	/* Get VPD from function 0 VPD */
-	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
+	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 7),
 };
 
 enum pci_irq_reroute_variant {
@@ -279,7 +277,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 */
+	unsigned long	*dma_alias_mask;/* mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this
@@ -1238,6 +1236,7 @@ resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, int resno);
 
 int pci_set_vga_state(struct pci_dev *pdev, bool decode,
 		      unsigned int command_bits, u32 flags);
+
 /* kmem_cache style wrapper around pci_alloc_consistent() */
 
 #include <linux/pci-dma.h>
@@ -1965,6 +1964,8 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
 				     u16 alias, void *data), void *data);
 
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn);
+
 /* helper functions for operation of device flag */
 static inline void pci_set_dev_assigned(struct pci_dev *pdev)
 {
-- 
2.1.4

[PATCH v2 2/2] pci: Add DMA alias quirk for mic_x200_dma

[Jacek Lawrynowicz]

MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
be added as aliases to the DMA device in order to allow buffer access
when IOMMU is enabled.

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
---
 drivers/pci/quirks.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b094061..bc23bc8 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3703,6 +3703,21 @@ DECLARE_PCI_FIXUP_HEADER(0x1283, 0x8892, quirk_use_pcie_bridge_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
 
 /*
+ * MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
+ * be added as aliases to the DMA device in order to allow buffer access
+ * when IOMMU is enabled.
+ */
+static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
+{
+	if (iommu_present(pdev->dev.bus)) {
+		pci_enable_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
+		pci_enable_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
+		pci_enable_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
+	}
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
+
+/*
  * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
  * class code.  Fix it.
  */
-- 
2.1.4

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

On Tue, 2016-01-26 at 11:31 +0100, Jacek Lawrynowicz wrote:
> 
> +bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
> +{
> +       return dev->dma_alias_mask &&
> +              test_bit(devfn, dev->dma_alias_mask);
> +}
> +EXPORT_SYMBOL_GPL(pci_dma_alias_is_enabled);

Why export this? 

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

RE: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Lawrynowicz, Jacek]

[-- Attachment #1: Type: text/plain, Size: 910 bytes --]

> -----Original Message-----
> From: David Woodhouse [mailto:dwmw2@infradead.org]
> Sent: Tuesday, January 26, 2016 12:59 PM
> To: Lawrynowicz, Jacek <jacek.lawrynowicz@intel.com>; linux-
> pci@vger.kernel.org
> Cc: bhelgaas@google.com; alex.williamson@redhat.com; jroedel@suse.de
> Subject: Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases
> 
> On Tue, 2016-01-26 at 11:31 +0100, Jacek Lawrynowicz wrote:
> >
> > +bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
> > +{
> > +       return dev->dma_alias_mask &&
> > +              test_bit(devfn, dev->dma_alias_mask);
> > +}
> > +EXPORT_SYMBOL_GPL(pci_dma_alias_is_enabled);
> 
> Why export this?

So it can be used in drivers/iommu/iommu.c in get_pci_alias_group().
Bjorn didn't want dma_alias_mask to be used there directly.

--
Jacek Lawrynowicz
Intel Technology Poland sp. z o.o.
KRS 101882 - ul. Slowackiego 173, 80-298 Gdansk



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 7756 bytes --]

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

On Tue, 2016-01-26 at 12:04 +0000, Lawrynowicz, Jacek wrote:
> 
> > Why export this?
> 
> So it can be used in drivers/iommu/iommu.c in get_pci_alias_group().
> Bjorn didn't want dma_alias_mask to be used there directly.

I do not want to contemplate a world in which the IOMMU code can be
modular...

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Bjorn Helgaas]

On Tue, Jan 26, 2016 at 12:04:47PM +0000, Lawrynowicz, Jacek wrote:
> > -----Original Message-----
> > From: David Woodhouse [mailto:dwmw2@infradead.org]
> > Sent: Tuesday, January 26, 2016 12:59 PM
> > To: Lawrynowicz, Jacek <jacek.lawrynowicz@intel.com>; linux-
> > pci@vger.kernel.org
> > Cc: bhelgaas@google.com; alex.williamson@redhat.com; jroedel@suse.de
> > Subject: Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases
> > 
> > On Tue, 2016-01-26 at 11:31 +0100, Jacek Lawrynowicz wrote:
> > >
> > > +bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
> > > +{
> > > +       return dev->dma_alias_mask &&
> > > +              test_bit(devfn, dev->dma_alias_mask);
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_dma_alias_is_enabled);
> > 
> > Why export this?
> 
> So it can be used in drivers/iommu/iommu.c in get_pci_alias_group().
> Bjorn didn't want dma_alias_mask to be used there directly.

Declaring pci_dma_alias_is_enabled() in include/linux/pci.h is
sufficient to allow drivers/iommu/iommu.c to use it.  The
EXPORT_SYMBOL_GPL() is not needed.

Bjorn

[PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Jacek Lawrynowicz]

This patch solves IOMMU support issues with PCIe non-transparent bridges
that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
the bridge, packet's RID is rewritten according to LUT programmed by
a driver. Modified packets are then passed to a destination bus and
processed upstream. The problem is that such packets seem to come from
non-existent nodes that are hidden behind NTB and are not discoverable
by a destination node, so IOMMU discards them. Adding DMA alias for a
given LUT entry allows IOMMU to create a proper mapping that enables
inter-node communication.

The current DMA alias implementation supports only single alias, so it's
not possible to connect more than two nodes when IOMMU is enabled. This
implementation enables all possible aliases on a given bus (256) that
are stored in a bitset. Alias devfn is directly translated to a bit
number. The bitset is not allocated for devices that have no need for
DMA aliases.

More details can be found in following article:
http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
---
 drivers/iommu/iommu.c | 11 ++++-------
 drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
 drivers/pci/pci.h     |  2 ++
 drivers/pci/probe.c   |  1 +
 drivers/pci/quirks.c  | 16 +++++++---------
 drivers/pci/search.c  | 14 +++++++++-----
 include/linux/pci.h   | 15 ++++++++-------
 7 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..93a89ed 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 }
 
 /*
- * Look for aliases to or from the given device for exisiting groups.  The
- * dma_alias_devfn only supports aliases on the same bus, therefore the search
+ * Look for aliases to or from the given device for exisiting groups. DMA
+ * aliases are only supported on the same bus, therefore the search
  * space is quite small (especially since we're really only looking at pcie
  * device, and therefore only expect multiple slots on the root complex or
  * downstream switch ports).  It's conceivable though that a pair of
@@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 			continue;
 
 		/* We alias them or they alias us */
-		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     pdev->dma_alias_devfn == tmp->devfn) ||
-		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     tmp->dma_alias_devfn == pdev->devfn)) {
-
+		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
+		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
 			group = get_pci_alias_group(tmp, devfns);
 			if (group) {
 				pci_dev_put(tmp);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 602eb42..cd38767 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4568,6 +4568,33 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 	return 0;
 }
 
+/**
+ * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
+ * @dev: the PCI device for which alias is added
+ * @devfn: alias slot and function
+ *
+ * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
+ * It should be called early, preferably as PCI fixup header quirk.
+ */
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
+{
+	if (!dev->dma_alias_mask)
+		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
+					      sizeof(long), GFP_KERNEL);
+	if (!dev->dma_alias_mask) {
+		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
+		return;
+	}
+	if (dev->dma_alias_mask)
+		set_bit(devfn, dev->dma_alias_mask);
+}
+
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
+{
+	return dev->dma_alias_mask &&
+	       test_bit(devfn, dev->dma_alias_mask);
+}
+
 bool pci_device_is_present(struct pci_dev *pdev)
 {
 	u32 v;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9a1660f..c31c1a5 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -335,4 +335,6 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6d7ab9b..23fc397 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1503,6 +1503,7 @@ static void pci_release_dev(struct device *dev)
 	pcibios_release_device(pci_dev);
 	pci_bus_put(pci_dev->bus);
 	kfree(pci_dev->driver_override);
+	kfree(pci_dev->dma_alias_mask);
 	kfree(pci_dev);
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0575a1e..b094061 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -25,6 +25,7 @@
 #include <linux/sched.h>
 #include <linux/ktime.h>
 #include <linux/mm.h>
+#include <linux/iommu.h>
 #include <asm/dma.h>	/* isa_dma_bridge_buggy */
 #include "pci.h"
 
@@ -3582,8 +3583,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	}
 }
 
@@ -3598,8 +3598,7 @@ 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
 	}
 }
 
@@ -3667,11 +3666,10 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id) {
-		dev->dma_alias_devfn = id->driver_data;
-		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
-		dev_info(&dev->dev, "Enabling fixed DMA alias to %02x.%d\n",
-			 PCI_SLOT(dev->dma_alias_devfn),
-			 PCI_FUNC(dev->dma_alias_devfn));
+		pci_enable_dma_alias(dev, id->driver_data);
+		dev_info(&dev->dev, "Enabling fixed DMA alias to %02lx.%ld\n",
+			 PCI_SLOT(id->driver_data),
+			 PCI_FUNC(id->driver_data));
 	}
 }
 
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a20ce7d..33e0f03 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -40,11 +40,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	 * 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;
+	if (unlikely(pdev->dma_alias_mask)) {
+		u8 devfn;
+
+		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
+			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
+				 data);
+			if (ret)
+				return ret;
+		}
 	}
 
 	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 27df4a6..6c9e9e6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -172,16 +172,14 @@ 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),
 	/* 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),
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 4),
 	/* Do not use bus resets for device */
-	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
+	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 5),
 	/* Do not use PM reset even if device advertises NoSoftRst- */
-	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
+	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 6),
 	/* Get VPD from function 0 VPD */
-	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
+	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 7),
 };
 
 enum pci_irq_reroute_variant {
@@ -279,7 +277,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 */
+	unsigned long	*dma_alias_mask;/* mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this
@@ -1238,6 +1236,7 @@ resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, int resno);
 
 int pci_set_vga_state(struct pci_dev *pdev, bool decode,
 		      unsigned int command_bits, u32 flags);
+
 /* kmem_cache style wrapper around pci_alloc_consistent() */
 
 #include <linux/pci-dma.h>
@@ -1965,6 +1964,8 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
 				     u16 alias, void *data), void *data);
 
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn);
+
 /* helper functions for operation of device flag */
 static inline void pci_set_dev_assigned(struct pci_dev *pdev)
 {
-- 
2.1.4

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]

On Tue, 2016-01-26 at 15:52 +0100, Jacek Lawrynowicz wrote:
> This patch solves IOMMU support issues with PCIe non-transparent bridges
> that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> the bridge, packet's RID is rewritten according to LUT programmed by
> a driver. Modified packets are then passed to a destination bus and
> processed upstream. The problem is that such packets seem to come from
> non-existent nodes that are hidden behind NTB and are not discoverable
> by a destination node, so IOMMU discards them. Adding DMA alias for a
> given LUT entry allows IOMMU to create a proper mapping that enables
> inter-node communication.
> 
> The current DMA alias implementation supports only single alias, so it's
> not possible to connect more than two nodes when IOMMU is enabled. This
> implementation enables all possible aliases on a given bus (256) that
> are stored in a bitset. Alias devfn is directly translated to a bit
> number. The bitset is not allocated for devices that have no need for
> DMA aliases.
> 
> More details can be found in following article:
> http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>

Acked-By: David Woodhouse <David.Woodhouse@intel.com>


... for both.

-- 
dwmw2


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Alex Williamson]

On Tue, 2016-01-26 at 11:31 +0100, Jacek Lawrynowicz wrote:
> This patch solves IOMMU support issues with PCIe non-transparent bridges
> that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> the bridge, packet's RID is rewritten according to LUT programmed by
> a driver. Modified packets are then passed to a destination bus and
> processed upstream. The problem is that such packets seem to come from
> non-existent nodes that are hidden behind NTB and are not discoverable
> by a destination node, so IOMMU discards them. Adding DMA alias for a
> given LUT entry allows IOMMU to create a proper mapping that enables
> inter-node communication.
> 
> The current DMA alias implementation supports only single alias, so it's
> not possible to connect more than two nodes when IOMMU is enabled. This
> implementation enables all possible aliases on a given bus (256) that
> are stored in a bitset. Alias devfn is directly translated to a bit
> number. The bitset is not allocated for devices that have no need for
> DMA aliases.
> 
> More details can be found in following article:
> http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> ---
>  drivers/iommu/iommu.c | 11 ++++-------
>  drivers/pci/pci.c     | 28 ++++++++++++++++++++++++++++
>  drivers/pci/pci.h     |  2 ++
>  drivers/pci/probe.c   |  1 +
>  drivers/pci/quirks.c  | 16 +++++++---------
>  drivers/pci/search.c  | 14 +++++++++-----
>  include/linux/pci.h   | 15 ++++++++-------
>  7 files changed, 59 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 0e3b009..93a89ed 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
>  }
>  
>  /*
> - * Look for aliases to or from the given device for exisiting groups.  The
> - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> + * Look for aliases to or from the given device for exisiting groups. DMA
> + * aliases are only supported on the same bus, therefore the search
>   * space is quite small (especially since we're really only looking at pcie
>   * device, and therefore only expect multiple slots on the root complex or
>   * downstream switch ports).  It's conceivable though that a pair of
> @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
>  			continue;
>  
>  		/* We alias them or they alias us */
> -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> -		     pdev->dma_alias_devfn == tmp->devfn) ||
> -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> -		     tmp->dma_alias_devfn == pdev->devfn)) {
> -
> +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
>  			group = get_pci_alias_group(tmp, devfns);
>  			if (group) {
>  				pci_dev_put(tmp);
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 602eb42..f114068 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4568,6 +4568,34 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>  	return 0;
>  }
>  
> +/**
> + * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
> + * @dev: the PCI device for which alias is added
> + * @devfn: alias slot and function
> + *
> + * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
> + * It should be called early, preferably as PCI fixup header quirk.
> + */
> +void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
> +{
> +	if (!dev->dma_alias_mask)
> +		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
> +					      sizeof(long), GFP_KERNEL);
> +	if (!dev->dma_alias_mask) {
> +		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
> +		return;
> +	}
> +	if (dev->dma_alias_mask)
> +		set_bit(devfn, dev->dma_alias_mask);
> +}
> +
> +bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
> +{
> +	return dev->dma_alias_mask &&
> +	       test_bit(devfn, dev->dma_alias_mask);
> +}
> +EXPORT_SYMBOL_GPL(pci_dma_alias_is_enabled);
> +


This seems sort of redundant to pci_for_each_dma_alias().


>  bool pci_device_is_present(struct pci_dev *pdev)
>  {
>  	u32 v;
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 9a1660f..c31c1a5 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -335,4 +335,6 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  }
>  #endif
>  
> +void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn);
> +
>  #endif /* DRIVERS_PCI_H */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6d7ab9b..23fc397 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1503,6 +1503,7 @@ static void pci_release_dev(struct device *dev)
>  	pcibios_release_device(pci_dev);
>  	pci_bus_put(pci_dev->bus);
>  	kfree(pci_dev->driver_override);
> +	kfree(pci_dev->dma_alias_mask);
>  	kfree(pci_dev);
>  }
>  
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 0575a1e..b094061 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -25,6 +25,7 @@
>  #include 
>  #include 
>  #include 
> +#include <linux/iommu.h>


Stray change?


>  #include <asm/dma.h>	/* isa_dma_bridge_buggy */
>  #include "pci.h"
>  
> @@ -3582,8 +3583,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
>  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;
> +		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
>  	}
>  }
>  
> @@ -3598,8 +3598,7 @@ 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;
> +		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
>  	}
>  }
>  
> @@ -3667,11 +3666,10 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
>  
>  	id = pci_match_id(fixed_dma_alias_tbl, dev);
>  	if (id) {
> -		dev->dma_alias_devfn = id->driver_data;
> -		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
> -		dev_info(&dev->dev, "Enabling fixed DMA alias to %02x.%d\n",
> -			 PCI_SLOT(dev->dma_alias_devfn),
> -			 PCI_FUNC(dev->dma_alias_devfn));
> +		pci_enable_dma_alias(dev, id->driver_data);
> +		dev_info(&dev->dev, "Enabling fixed DMA alias to %02lx.%ld\n",
> +			 PCI_SLOT(id->driver_data),
> +			 PCI_FUNC(id->driver_data));
>  	}
>  }
>  
> diff --git a/drivers/pci/search.c b/drivers/pci/search.c
> index a20ce7d..33e0f03 100644
> --- a/drivers/pci/search.c
> +++ b/drivers/pci/search.c
> @@ -40,11 +40,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>  	 * 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;
> +	if (unlikely(pdev->dma_alias_mask)) {
> +		u8 devfn;
> +
> +		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
> +			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
> +				 data);
> +			if (ret)
> +				return ret;
> +		}
>  	}
>  
>  	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 27df4a6..6c9e9e6 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -172,16 +172,14 @@ 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),
>  	/* 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),
> +	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 4),
>  	/* Do not use bus resets for device */
> -	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
> +	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 5),
>  	/* Do not use PM reset even if device advertises NoSoftRst- */
> -	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
> +	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 6),
>  	/* Get VPD from function 0 VPD */
> -	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
> +	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 7),
>  };
>  
>  enum pci_irq_reroute_variant {
> @@ -279,7 +277,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 */
> +	unsigned long	*dma_alias_mask;/* mask of enabled devfn aliases */
>  
>  	struct pci_driver *driver;	/* which driver has allocated this device */
>  	u64		dma_mask;	/* Mask of the bits of bus address this
> @@ -1238,6 +1236,7 @@ resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev, int resno);
>  
>  int pci_set_vga_state(struct pci_dev *pdev, bool decode,
>  		      unsigned int command_bits, u32 flags);
> +

Another stray change

>  /* kmem_cache style wrapper around pci_alloc_consistent() */
>  
>  #include 
> @@ -1965,6 +1964,8 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
>  			   int (*fn)(struct pci_dev *pdev,
>  				     u16 alias, void *data), void *data);
>  
> +bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn);
> +
>  /* helper functions for operation of device flag */
>  static inline void pci_set_dev_assigned(struct pci_dev *pdev)
>  {

Re: [PATCH v2 2/2] pci: Add DMA alias quirk for mic_x200_dma

[Alex Williamson]

On Tue, 2016-01-26 at 11:31 +0100, Jacek Lawrynowicz wrote:
> MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
> be added as aliases to the DMA device in order to allow buffer access
> when IOMMU is enabled.
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> ---
>  drivers/pci/quirks.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index b094061..bc23bc8 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -3703,6 +3703,21 @@ DECLARE_PCI_FIXUP_HEADER(0x1283, 0x8892, quirk_use_pcie_bridge_dma_alias);
>  DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
>  
>  /*
> + * MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
> + * be added as aliases to the DMA device in order to allow buffer access
> + * when IOMMU is enabled.
> + */
> +static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
> +{
> +	if (iommu_present(pdev->dev.bus)) {

Why do we need this test?  The alias simply goes unused without an
IOMMU, right?

> +		pci_enable_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
> +		pci_enable_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
> +		pci_enable_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
> +	}
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
> +
> +/*
>   * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
>   * class code.  Fix it.
>   */

[PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Jacek Lawrynowicz]

This patch solves IOMMU support issues with PCIe non-transparent bridges
that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
the bridge, packet's RID is rewritten according to LUT programmed by
a driver. Modified packets are then passed to a destination bus and
processed upstream. The problem is that such packets seem to come from
non-existent nodes that are hidden behind NTB and are not discoverable
by a destination node, so IOMMU discards them. Adding DMA alias for a
given LUT entry allows IOMMU to create a proper mapping that enables
inter-node communication.

The current DMA alias implementation supports only single alias, so it's
not possible to connect more than two nodes when IOMMU is enabled. This
implementation enables all possible aliases on a given bus (256) that
are stored in a bitset. Alias devfn is directly translated to a bit
number. The bitset is not allocated for devices that have no need for
DMA aliases.

More details can be found in following article:
http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Acked-By: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/iommu.c | 11 ++++-------
 drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
 drivers/pci/pci.h     |  2 ++
 drivers/pci/probe.c   |  1 +
 drivers/pci/quirks.c  | 15 ++++++---------
 drivers/pci/search.c  | 14 +++++++++-----
 include/linux/pci.h   | 14 +++++++-------
 7 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..93a89ed 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 }
 
 /*
- * Look for aliases to or from the given device for exisiting groups.  The
- * dma_alias_devfn only supports aliases on the same bus, therefore the search
+ * Look for aliases to or from the given device for exisiting groups. DMA
+ * aliases are only supported on the same bus, therefore the search
  * space is quite small (especially since we're really only looking at pcie
  * device, and therefore only expect multiple slots on the root complex or
  * downstream switch ports).  It's conceivable though that a pair of
@@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 			continue;
 
 		/* We alias them or they alias us */
-		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     pdev->dma_alias_devfn == tmp->devfn) ||
-		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     tmp->dma_alias_devfn == pdev->devfn)) {
-
+		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
+		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
 			group = get_pci_alias_group(tmp, devfns);
 			if (group) {
 				pci_dev_put(tmp);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 602eb42..cd38767 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4568,6 +4568,33 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 	return 0;
 }
 
+/**
+ * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
+ * @dev: the PCI device for which alias is added
+ * @devfn: alias slot and function
+ *
+ * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
+ * It should be called early, preferably as PCI fixup header quirk.
+ */
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
+{
+	if (!dev->dma_alias_mask)
+		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
+					      sizeof(long), GFP_KERNEL);
+	if (!dev->dma_alias_mask) {
+		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
+		return;
+	}
+	if (dev->dma_alias_mask)
+		set_bit(devfn, dev->dma_alias_mask);
+}
+
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
+{
+	return dev->dma_alias_mask &&
+	       test_bit(devfn, dev->dma_alias_mask);
+}
+
 bool pci_device_is_present(struct pci_dev *pdev)
 {
 	u32 v;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9a1660f..c31c1a5 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -335,4 +335,6 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
+void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6d7ab9b..23fc397 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1503,6 +1503,7 @@ static void pci_release_dev(struct device *dev)
 	pcibios_release_device(pci_dev);
 	pci_bus_put(pci_dev->bus);
 	kfree(pci_dev->driver_override);
+	kfree(pci_dev->dma_alias_mask);
 	kfree(pci_dev);
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0575a1e..83c3da1 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3582,8 +3582,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	}
 }
 
@@ -3598,8 +3597,7 @@ 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;
+		pci_enable_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
 	}
 }
 
@@ -3667,11 +3665,10 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id) {
-		dev->dma_alias_devfn = id->driver_data;
-		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
-		dev_info(&dev->dev, "Enabling fixed DMA alias to %02x.%d\n",
-			 PCI_SLOT(dev->dma_alias_devfn),
-			 PCI_FUNC(dev->dma_alias_devfn));
+		pci_enable_dma_alias(dev, id->driver_data);
+		dev_info(&dev->dev, "Enabling fixed DMA alias to %02lx.%ld\n",
+			 PCI_SLOT(id->driver_data),
+			 PCI_FUNC(id->driver_data));
 	}
 }
 
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a20ce7d..33e0f03 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -40,11 +40,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	 * 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;
+	if (unlikely(pdev->dma_alias_mask)) {
+		u8 devfn;
+
+		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
+			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
+				 data);
+			if (ret)
+				return ret;
+		}
 	}
 
 	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 27df4a6..86c31b5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -172,16 +172,14 @@ 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),
 	/* 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),
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 4),
 	/* Do not use bus resets for device */
-	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
+	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 5),
 	/* Do not use PM reset even if device advertises NoSoftRst- */
-	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
+	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 6),
 	/* Get VPD from function 0 VPD */
-	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
+	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 7),
 };
 
 enum pci_irq_reroute_variant {
@@ -279,7 +277,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 */
+	unsigned long	*dma_alias_mask;/* mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this
@@ -1965,6 +1963,8 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 			   int (*fn)(struct pci_dev *pdev,
 				     u16 alias, void *data), void *data);
 
+bool pci_dma_alias_is_enabled(struct pci_dev *dev, u8 devfn);
+
 /* helper functions for operation of device flag */
 static inline void pci_set_dev_assigned(struct pci_dev *pdev)
 {
-- 
2.1.4

RE: [PATCH v2 2/2] pci: Add DMA alias quirk for mic_x200_dma

[Lawrynowicz, Jacek]

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1wY2ktb3duZXJAdmdl
ci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtcGNpLQ0KPiBvd25lckB2Z2VyLmtlcm5lbC5vcmdd
IE9uIEJlaGFsZiBPZiBBbGV4IFdpbGxpYW1zb24NCj4gU2VudDogVHVlc2RheSwgSmFudWFyeSAy
NiwgMjAxNiA1OjI4IFBNDQo+IFRvOiBMYXdyeW5vd2ljeiwgSmFjZWsgPGphY2VrLmxhd3J5bm93
aWN6QGludGVsLmNvbT47IGxpbnV4LQ0KPiBwY2lAdmdlci5rZXJuZWwub3JnDQo+IENjOiBiaGVs
Z2Fhc0Bnb29nbGUuY29tOyBkd213MkBpbmZyYWRlYWQub3JnOyBqcm9lZGVsQHN1c2UuZGUNCj4g
U3ViamVjdDogUmU6IFtQQVRDSCB2MiAyLzJdIHBjaTogQWRkIERNQSBhbGlhcyBxdWlyayBmb3Ig
bWljX3gyMDBfZG1hDQo+IA0KPiBPbiBUdWUsIDIwMTYtMDEtMjYgYXQgMTE6MzEgKzAxMDAsIEph
Y2VrIExhd3J5bm93aWN6IHdyb3RlOg0KPiA+IE1JQyB4MjAwIE5UQiBmb3J3YXJkcyBQQ0llIHRy
YWZmaWMgdXNpbmcgbXVsdGlwbGUgYWxpZW4gUklELiBUaGV5IGhhdmUNCj4gPiB0byBiZSBhZGRl
ZCBhcyBhbGlhc2VzIHRvIHRoZSBETUEgZGV2aWNlIGluIG9yZGVyIHRvIGFsbG93IGJ1ZmZlcg0K
PiA+IGFjY2VzcyB3aGVuIElPTU1VIGlzIGVuYWJsZWQuDQo+ID4NCj4gPiBTaWduZWQtb2ZmLWJ5
OiBKYWNlayBMYXdyeW5vd2ljeiA8amFjZWsubGF3cnlub3dpY3pAaW50ZWwuY29tPg0KPiA+IC0t
LQ0KPiA+IMKgZHJpdmVycy9wY2kvcXVpcmtzLmMgfCAxNSArKysrKysrKysrKysrKysNCj4gPiDC
oDEgZmlsZSBjaGFuZ2VkLCAxNSBpbnNlcnRpb25zKCspDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEv
ZHJpdmVycy9wY2kvcXVpcmtzLmMgYi9kcml2ZXJzL3BjaS9xdWlya3MuYyBpbmRleA0KPiA+IGIw
OTQwNjEuLmJjMjNiYzggMTAwNjQ0DQo+ID4gLS0tIGEvZHJpdmVycy9wY2kvcXVpcmtzLmMNCj4g
PiArKysgYi9kcml2ZXJzL3BjaS9xdWlya3MuYw0KPiA+IEBAIC0zNzAzLDYgKzM3MDMsMjEgQEAg
REVDTEFSRV9QQ0lfRklYVVBfSEVBREVSKDB4MTI4MywgMHg4ODkyLA0KPiA+IHF1aXJrX3VzZV9w
Y2llX2JyaWRnZV9kbWFfYWxpYXMpOw0KPiA+IMKgREVDTEFSRV9QQ0lfRklYVVBfSEVBREVSKDB4
ODA4NiwgMHgyNDRlLA0KPiA+IHF1aXJrX3VzZV9wY2llX2JyaWRnZV9kbWFfYWxpYXMpOw0KPiA+
DQo+ID4gwqAvKg0KPiA+ICsgKiBNSUMgeDIwMCBOVEIgZm9yd2FyZHMgUENJZSB0cmFmZmljIHVz
aW5nIG11bHRpcGxlIGFsaWVuIFJJRC4gVGhleQ0KPiA+ICtoYXZlIHRvDQo+ID4gKyAqIGJlIGFk
ZGVkIGFzIGFsaWFzZXMgdG8gdGhlIERNQSBkZXZpY2UgaW4gb3JkZXIgdG8gYWxsb3cgYnVmZmVy
DQo+ID4gK2FjY2Vzcw0KPiA+ICsgKiB3aGVuIElPTU1VIGlzIGVuYWJsZWQuDQo+ID4gKyAqLw0K
PiA+ICtzdGF0aWMgdm9pZCBxdWlya19taWNfeDIwMF9kbWFfYWxpYXMoc3RydWN0IHBjaV9kZXYg
KnBkZXYpIHsNCj4gPiArCWlmIChpb21tdV9wcmVzZW50KHBkZXYtPmRldi5idXMpKSB7DQo+IA0K
PiBXaHkgZG8gd2UgbmVlZCB0aGlzIHRlc3Q/IMKgVGhlIGFsaWFzIHNpbXBseSBnb2VzIHVudXNl
ZCB3aXRob3V0IGFuIElPTU1VLA0KPiByaWdodD8NCg0KUmlnaHQsIEkgd2FudGVkIHRvIGxpbWl0
IHdoZW4gdGhlIHF1aXJrIGlzIGFwcGxpZWQgYnV0IGl0IHdvcmtzIHdpdGhvdXQgdGhlIGNoZWNr
IGZvciBpb21tdS4NCkkgZ3Vlc3MgdGhhdCB0aGUgY29kZSB3aWxsIGJlIGEgYml0IHNpbXBsZXIg
d2l0aG91dCB0aGlzIGNoZWNrLiBJIHdpbGwgcmVtb3ZlIGl0Lg0KDQotLQ0KSmFjZWsgTGF3cnlu
b3dpY3oNCkludGVsIFRlY2hub2xvZ3kgUG9sYW5kIHNwLiB6IG8uby4NCktSUyAxMDE4ODIgLSB1
bC4gU2xvd2Fja2llZ28gMTczLCA4MC0yOTggR2RhbnNrDQoNCg0KDQo=

[PATCH v2 2/2] pci: Add DMA alias quirk for mic_x200_dma

[Jacek Lawrynowicz]

MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
be added as aliases to the DMA device in order to allow buffer access
when IOMMU is enabled.

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Acked-By: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/pci/quirks.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 83c3da1..350eac3 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3702,6 +3702,19 @@ DECLARE_PCI_FIXUP_HEADER(0x1283, 0x8892, quirk_use_pcie_bridge_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
 
 /*
+ * MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
+ * be added as aliases to the DMA device in order to allow buffer access
+ * when IOMMU is enabled.
+ */
+static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
+{
+	pci_enable_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
+	pci_enable_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
+	pci_enable_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
+
+/*
  * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
  * class code.  Fix it.
  */
-- 
2.1.4

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Bjorn Helgaas]

Hi Jacek,

When you post updated versions, please post the whole series with a
new version number.  I think there are three postings of [v2 1/2], and
I think they're different.  In any case, it's hard for me to keep them
straight.

On Tue, Jan 26, 2016 at 06:12:15PM +0100, Jacek Lawrynowicz wrote:
> This patch solves IOMMU support issues with PCIe non-transparent bridges
> that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> the bridge, packet's RID is rewritten according to LUT programmed by
> a driver. Modified packets are then passed to a destination bus and
> processed upstream. The problem is that such packets seem to come from
> non-existent nodes that are hidden behind NTB and are not discoverable
> by a destination node, so IOMMU discards them. Adding DMA alias for a
> given LUT entry allows IOMMU to create a proper mapping that enables
> inter-node communication.
> 
> The current DMA alias implementation supports only single alias, so it's
> not possible to connect more than two nodes when IOMMU is enabled. This
> implementation enables all possible aliases on a given bus (256) that
> are stored in a bitset. Alias devfn is directly translated to a bit
> number. The bitset is not allocated for devices that have no need for
> DMA aliases.
> 
> More details can be found in following article:
> http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> Acked-By: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  drivers/iommu/iommu.c | 11 ++++-------
>  drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
>  drivers/pci/pci.h     |  2 ++
>  drivers/pci/probe.c   |  1 +
>  drivers/pci/quirks.c  | 15 ++++++---------
>  drivers/pci/search.c  | 14 +++++++++-----
>  include/linux/pci.h   | 14 +++++++-------
>  7 files changed, 56 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 0e3b009..93a89ed 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
>  }
>  
>  /*
> - * Look for aliases to or from the given device for exisiting groups.  The
> - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> + * Look for aliases to or from the given device for exisiting groups. DMA

s/exisiting/existing/  (Not your change, but might as well fix the
typo since you're touching the line anyway.)

> + * aliases are only supported on the same bus, therefore the search
>   * space is quite small (especially since we're really only looking at pcie
>   * device, and therefore only expect multiple slots on the root complex or
>   * downstream switch ports).  It's conceivable though that a pair of
> @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
>  			continue;
>  
>  		/* We alias them or they alias us */
> -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> -		     pdev->dma_alias_devfn == tmp->devfn) ||
> -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> -		     tmp->dma_alias_devfn == pdev->devfn)) {
> -
> +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {

I was hoping for something that took two pci_dev pointers and returned
true if they were aliases of each other, e.g.,

  bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);

>  			group = get_pci_alias_group(tmp, devfns);
>  			if (group) {
>  				pci_dev_put(tmp);
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 602eb42..cd38767 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4568,6 +4568,33 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>  	return 0;
>  }
>  
> +/**
> + * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
> + * @dev: the PCI device for which alias is added
> + * @devfn: alias slot and function
> + *
> + * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
> + * It should be called early, preferably as PCI fixup header quirk.
> + */
> +void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)

I kind of liked "pci_add_dma_alias()."  "Enable" suggests that we can
turn them on and off, which isn't really the case here.

> +{
> +	if (!dev->dma_alias_mask)
> +		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
> +					      sizeof(long), GFP_KERNEL);
> +	if (!dev->dma_alias_mask) {
> +		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
> +		return;
> +	}
> +	if (dev->dma_alias_mask)
> +		set_bit(devfn, dev->dma_alias_mask);
> +}

Bjorn

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Alex Williamson]

On Tue, 2016-01-26 at 17:31 -0600, Bjorn Helgaas wrote:
> Hi Jacek,
> 
> When you post updated versions, please post the whole series with a
> new version number.  I think there are three postings of [v2 1/2], and
> I think they're different.  In any case, it's hard for me to keep them
> straight.
> 
> On Tue, Jan 26, 2016 at 06:12:15PM +0100, Jacek Lawrynowicz wrote:
> > This patch solves IOMMU support issues with PCIe non-transparent bridges
> > that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> > the bridge, packet's RID is rewritten according to LUT programmed by
> > a driver. Modified packets are then passed to a destination bus and
> > processed upstream. The problem is that such packets seem to come from
> > non-existent nodes that are hidden behind NTB and are not discoverable
> > by a destination node, so IOMMU discards them. Adding DMA alias for a
> > given LUT entry allows IOMMU to create a proper mapping that enables
> > inter-node communication.
> > 
> > The current DMA alias implementation supports only single alias, so it's
> > not possible to connect more than two nodes when IOMMU is enabled. This
> > implementation enables all possible aliases on a given bus (256) that
> > are stored in a bitset. Alias devfn is directly translated to a bit
> > number. The bitset is not allocated for devices that have no need for
> > DMA aliases.
> > 
> > More details can be found in following article:
> > http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> > 
> > Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> > Acked-By: David Woodhouse <David.Woodhouse@intel.com>
> > ---
> >  drivers/iommu/iommu.c | 11 ++++-------
> >  drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
> >  drivers/pci/pci.h     |  2 ++
> >  drivers/pci/probe.c   |  1 +
> >  drivers/pci/quirks.c  | 15 ++++++---------
> >  drivers/pci/search.c  | 14 +++++++++-----
> >  include/linux/pci.h   | 14 +++++++-------
> >  7 files changed, 56 insertions(+), 28 deletions(-)
> > 
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 0e3b009..93a89ed 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev
> > *pdev,
> >  }
> >  
> >  /*
> > - * Look for aliases to or from the given device for exisiting groups.  The
> > - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> > + * Look for aliases to or from the given device for exisiting groups. DMA
> 
> s/exisiting/existing/  (Not your change, but might as well fix the
> typo since you're touching the line anyway.)
> 
> > + * aliases are only supported on the same bus, therefore the search
> >   * space is quite small (especially since we're really only looking at pcie
> >   * device, and therefore only expect multiple slots on the root complex or
> >   * downstream switch ports).  It's conceivable though that a pair of
> > @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
> >  			continue;
> >  
> >  		/* We alias them or they alias us */
> > -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > -		     pdev->dma_alias_devfn == tmp->devfn) ||
> > -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > -		     tmp->dma_alias_devfn == pdev->devfn)) {
> > -
> > +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> > +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
> 
> I was hoping for something that took two pci_dev pointers and returned
> true if they were aliases of each other, e.g.,
> 
>   bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);


Such a thing could already be created with pci_for_each_dma_alias().


> >  			group = get_pci_alias_group(tmp, devfns);
> >  			if (group) {
> >  				pci_dev_put(tmp);
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 602eb42..cd38767 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4568,6 +4568,33 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
> >  	return 0;
> >  }
> >  
> > +/**
> > + * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
> > + * @dev: the PCI device for which alias is added
> > + * @devfn: alias slot and function
> > + *
> > + * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
> > + * It should be called early, preferably as PCI fixup header quirk.
> > + */
> > +void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
> 
> I kind of liked "pci_add_dma_alias()."  "Enable" suggests that we can
> turn them on and off, which isn't really the case here.
> 
> > +{
> > +	if (!dev->dma_alias_mask)
> > +		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
> > +					      sizeof(long), GFP_KERNEL);
> > +	if (!dev->dma_alias_mask) {
> > +		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
> > +		return;
> > +	}
> > +	if (dev->dma_alias_mask)
> > +		set_bit(devfn, dev->dma_alias_mask);
> > +}
> 
> Bjorn

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Bjorn Helgaas]

On Tue, Jan 26, 2016 at 04:42:23PM -0700, Alex Williamson wrote:
> On Tue, 2016-01-26 at 17:31 -0600, Bjorn Helgaas wrote:
> > Hi Jacek,
> > 
> > When you post updated versions, please post the whole series with a
> > new version number.  I think there are three postings of [v2 1/2], and
> > I think they're different.  In any case, it's hard for me to keep them
> > straight.
> > 
> > On Tue, Jan 26, 2016 at 06:12:15PM +0100, Jacek Lawrynowicz wrote:
> > > This patch solves IOMMU support issues with PCIe non-transparent bridges
> > > that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> > > the bridge, packet's RID is rewritten according to LUT programmed by
> > > a driver. Modified packets are then passed to a destination bus and
> > > processed upstream. The problem is that such packets seem to come from
> > > non-existent nodes that are hidden behind NTB and are not discoverable
> > > by a destination node, so IOMMU discards them. Adding DMA alias for a
> > > given LUT entry allows IOMMU to create a proper mapping that enables
> > > inter-node communication.
> > > 
> > > The current DMA alias implementation supports only single alias, so it's
> > > not possible to connect more than two nodes when IOMMU is enabled. This
> > > implementation enables all possible aliases on a given bus (256) that
> > > are stored in a bitset. Alias devfn is directly translated to a bit
> > > number. The bitset is not allocated for devices that have no need for
> > > DMA aliases.
> > > 
> > > More details can be found in following article:
> > > http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> > > 
> > > Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> > > Acked-By: David Woodhouse <David.Woodhouse@intel.com>
> > > ---
> > >  drivers/iommu/iommu.c | 11 ++++-------
> > >  drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
> > >  drivers/pci/pci.h     |  2 ++
> > >  drivers/pci/probe.c   |  1 +
> > >  drivers/pci/quirks.c  | 15 ++++++---------
> > >  drivers/pci/search.c  | 14 +++++++++-----
> > >  include/linux/pci.h   | 14 +++++++-------
> > >  7 files changed, 56 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 0e3b009..93a89ed 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev
> > > *pdev,
> > >  }
> > >  
> > >  /*
> > > - * Look for aliases to or from the given device for exisiting groups.  The
> > > - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> > > + * Look for aliases to or from the given device for exisiting groups. DMA
> > 
> > s/exisiting/existing/  (Not your change, but might as well fix the
> > typo since you're touching the line anyway.)
> > 
> > > + * aliases are only supported on the same bus, therefore the search
> > >   * space is quite small (especially since we're really only looking at pcie
> > >   * device, and therefore only expect multiple slots on the root complex or
> > >   * downstream switch ports).  It's conceivable though that a pair of
> > > @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
> > >  			continue;
> > >  
> > >  		/* We alias them or they alias us */
> > > -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > -		     pdev->dma_alias_devfn == tmp->devfn) ||
> > > -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > -		     tmp->dma_alias_devfn == pdev->devfn)) {
> > > -
> > > +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> > > +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
> > 
> > I was hoping for something that took two pci_dev pointers and returned
> > true if they were aliases of each other, e.g.,
> > 
> >   bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> 
> Such a thing could already be created with pci_for_each_dma_alias().

Does that mean we could get replace the use of for_each_pci_dev() with some
clever use of pci_for_each_dma_alias()?

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Alex Williamson]

On Tue, 2016-01-26 at 18:04 -0600, Bjorn Helgaas wrote:
> On Tue, Jan 26, 2016 at 04:42:23PM -0700, Alex Williamson wrote:
> > On Tue, 2016-01-26 at 17:31 -0600, Bjorn Helgaas wrote:
> > > Hi Jacek,
> > >  
> > > When you post updated versions, please post the whole series with a
> > > new version number.  I think there are three postings of [v2 1/2], and
> > > I think they're different.  In any case, it's hard for me to keep them
> > > straight.
> > >  
> > > On Tue, Jan 26, 2016 at 06:12:15PM +0100, Jacek Lawrynowicz wrote:
> > > > This patch solves IOMMU support issues with PCIe non-transparent bridges
> > > > that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> > > > the bridge, packet's RID is rewritten according to LUT programmed by
> > > > a driver. Modified packets are then passed to a destination bus and
> > > > processed upstream. The problem is that such packets seem to come from
> > > > non-existent nodes that are hidden behind NTB and are not discoverable
> > > > by a destination node, so IOMMU discards them. Adding DMA alias for a
> > > > given LUT entry allows IOMMU to create a proper mapping that enables
> > > > inter-node communication.
> > > >  
> > > > The current DMA alias implementation supports only single alias, so it's
> > > > not possible to connect more than two nodes when IOMMU is enabled. This
> > > > implementation enables all possible aliases on a given bus (256) that
> > > > are stored in a bitset. Alias devfn is directly translated to a bit
> > > > number. The bitset is not allocated for devices that have no need for
> > > > DMA aliases.
> > > >  
> > > > More details can be found in following article:
> > > > http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns
> > > > .pdf
> > > >  
> > > > Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> > > > Acked-By: David Woodhouse <David.Woodhouse@intel.com>
> > > > ---
> > > >  drivers/iommu/iommu.c | 11 ++++-------
> > > >  drivers/pci/pci.c     | 27 +++++++++++++++++++++++++++
> > > >  drivers/pci/pci.h     |  2 ++
> > > >  drivers/pci/probe.c   |  1 +
> > > >  drivers/pci/quirks.c  | 15 ++++++---------
> > > >  drivers/pci/search.c  | 14 +++++++++-----
> > > >  include/linux/pci.h   | 14 +++++++-------
> > > >  7 files changed, 56 insertions(+), 28 deletions(-)
> > > >  
> > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > > index 0e3b009..93a89ed 100644
> > > > --- a/drivers/iommu/iommu.c
> > > > +++ b/drivers/iommu/iommu.c
> > > > @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev
> > > > *pdev,
> > > >  }
> > > >  
> > > >  /*
> > > > - * Look for aliases to or from the given device for exisiting groups.  The
> > > > - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> > > > + * Look for aliases to or from the given device for exisiting groups. DMA
> > >  
> > > s/exisiting/existing/  (Not your change, but might as well fix the
> > > typo since you're touching the line anyway.)
> > >  
> > > > + * aliases are only supported on the same bus, therefore the search
> > > >   * space is quite small (especially since we're really only looking at pcie
> > > >   * device, and therefore only expect multiple slots on the root complex or
> > > >   * downstream switch ports).  It's conceivable though that a pair of
> > > > @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
> > > >  			continue;
> > > >  
> > > >  		/* We alias them or they alias us */
> > > > -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > -		     pdev->dma_alias_devfn == tmp->devfn) ||
> > > > -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > -		     tmp->dma_alias_devfn == pdev->devfn)) {
> > > > -
> > > > +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> > > > +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
> > >  
> > > I was hoping for something that took two pci_dev pointers and returned
> > > true if they were aliases of each other, e.g.,
> > >  
> > >   bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> > 
> > Such a thing could already be created with pci_for_each_dma_alias().
> 
> Does that mean we could get replace the use of for_each_pci_dev() with some
> clever use of pci_for_each_dma_alias()?

Not remotely, but if we're going to have an is-alias function, should it
be limited to aliases on the same bus?  What if I call it on a
conventional PCI device where everything on the bus is an alias, this
implementation of relying only on the bitmap doesn't even work.  Thanks,

Alex

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Lawrynowicz, Jacek]

On Tue, Jan 26, 2016 at 05:31:00PM -0600, Bjorn Helgaas wrote:
> Hi Jacek,
> 
> When you post updated versions, please post the whole series with a
> new version number.  I think there are three postings of [v2 1/2], and
> I think they're different.  In any case, it's hard for me to keep them
> straight.

OK, should I also create a new thread for each new version?

> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 0e3b009..93a89ed 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -660,8 +660,8 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
> >  }
> >  
> >  /*
> > - * Look for aliases to or from the given device for exisiting groups.  The
> > - * dma_alias_devfn only supports aliases on the same bus, therefore the search
> > + * Look for aliases to or from the given device for exisiting groups. DMA
> 
> s/exisiting/existing/  (Not your change, but might as well fix the
> typo since you're touching the line anyway.)

Sure

> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 602eb42..cd38767 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4568,6 +4568,33 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
> >  	return 0;
> >  }
> >  
> > +/**
> > + * pci_enable_dma_alias - Allows to set multiple devfn aliases for given device
> > + * @dev: the PCI device for which alias is added
> > + * @devfn: alias slot and function
> > + *
> > + * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
> > + * It should be called early, preferably as PCI fixup header quirk.
> > + */
> > +void pci_enable_dma_alias(struct pci_dev *dev, u8 devfn)
> 
> I kind of liked "pci_add_dma_alias()."  "Enable" suggests that we can
> turn them on and off, which isn't really the case here.

OK, I was going for enable/is_enabled but you are right that enable
suggest that alias can be disabled.

Jacek

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Lawrynowicz, Jacek]

On Tue, Jan 26, 2016 at 05:54:47PM -0700, Alex Williamson wrote:
> On Tue, 2016-01-26 at 18:04 -0600, Bjorn Helgaas wrote:
> > On Tue, Jan 26, 2016 at 04:42:23PM -0700, Alex Williamson wrote:
> > > On Tue, 2016-01-26 at 17:31 -0600, Bjorn Helgaas wrote:
> > > > > + * aliases are only supported on the same bus, therefore the search
> > > > >   * space is quite small (especially since we're really only looking at pcie
> > > > >   * device, and therefore only expect multiple slots on the root complex or
> > > > >   * downstream switch ports).  It's conceivable though that a pair of
> > > > > @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
> > > > >  			continue;
> > > > >  
> > > > >  		/* We alias them or they alias us */
> > > > > -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > > -		     pdev->dma_alias_devfn == tmp->devfn) ||
> > > > > -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > > -		     tmp->dma_alias_devfn == pdev->devfn)) {
> > > > > -
> > > > > +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> > > > > +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
> > > >  
> > > > I was hoping for something that took two pci_dev pointers and returned
> > > > true if they were aliases of each other, e.g.,
> > > >  
> > > >   bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> > > 
> > > Such a thing could already be created with pci_for_each_dma_alias().
> > 
> > Does that mean we could get replace the use of for_each_pci_dev() with some
> > clever use of pci_for_each_dma_alias()?
> 
> Not remotely, but if we're going to have an is-alias function, should it
> be limited to aliases on the same bus?  What if I call it on a
> conventional PCI device where everything on the bus is an alias, this
> implementation of relying only on the bitmap doesn't even work.  Thanks,

So should I change it to pci_dma_aliases() or leave it?

Jacek

Re: [PATCH v2 1/2] pci: Add support for multiple DMA aliases

[Alex Williamson]

On Wed, 2016-01-27 at 21:05 +0100, Lawrynowicz, Jacek wrote:
> On Tue, Jan 26, 2016 at 05:54:47PM -0700, Alex Williamson wrote:
> > On Tue, 2016-01-26 at 18:04 -0600, Bjorn Helgaas wrote:
> > > On Tue, Jan 26, 2016 at 04:42:23PM -0700, Alex Williamson wrote:
> > > > On Tue, 2016-01-26 at 17:31 -0600, Bjorn Helgaas wrote:
> > > > > > + * aliases are only supported on the same bus, therefore the search
> > > > > >   * space is quite small (especially since we're really only looking at pcie
> > > > > >   * device, and therefore only expect multiple slots on the root complex or
> > > > > >   * downstream switch ports).  It's conceivable though that a pair of
> > > > > > @@ -686,11 +686,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
> > > > > >  			continue;
> > > > > >  
> > > > > >  		/* We alias them or they alias us */
> > > > > > -		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > > > -		     pdev->dma_alias_devfn == tmp->devfn) ||
> > > > > > -		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
> > > > > > -		     tmp->dma_alias_devfn == pdev->devfn)) {
> > > > > > -
> > > > > > +		if (pci_dma_alias_is_enabled(pdev, tmp->devfn) ||
> > > > > > +		    pci_dma_alias_is_enabled(tmp, pdev->devfn)) {
> > > > >  
> > > > > I was hoping for something that took two pci_dev pointers and returned
> > > > > true if they were aliases of each other, e.g.,
> > > > >  
> > > > >   bool pci_dma_aliases(struct pci_dev *dev1, struct pci_dev *dev2);
> > > >  
> > > > Such a thing could already be created with pci_for_each_dma_alias().
> > >  
> > > Does that mean we could get replace the use of for_each_pci_dev() with some
> > > clever use of pci_for_each_dma_alias()?
> > 
> > Not remotely, but if we're going to have an is-alias function, should it
> > be limited to aliases on the same bus?  What if I call it on a
> > conventional PCI device where everything on the bus is an alias, this
> > implementation of relying only on the bitmap doesn't even work.  Thanks,
> 
> So should I change it to pci_dma_aliases() or leave it?

My complaint is not about the name, but about the scope of the function.
The function get_pci_alias_group() defines the scope to be aliases on
the same bus, so it gets by with only testing dma_alias_devfn on
devices.  You're trying to offload this to a helper function, which is
usually a good thing, but that helper doesn't bring with it the same
limited scope.  Anybody can call that helper, where maybe it's not
appropriate that the scope is so limited.  So if you really just want to
fix get_pci_alias_group() with the equivalent functionality for having
multiple devfn aliases, the helper should be static so that it's not
misinterpreted as a generic helper.  Thanks,

Alex

[PATCH v3 0/2] Fixed couple of issues pointed by Alex and Bjorn

[Jacek Lawrynowicz]

- changed pci_dma_alias_is_enabled to be a static helper
- renamed pci_enable_dma_alias to pci_add_dma_alias
- fixed "exisiting" typo

Jacek Lawrynowicz (2):
  pci: Add support for multiple DMA aliases
  pci: Add DMA alias quirk for mic_x200_dma

 drivers/iommu/iommu.c | 17 ++++++++++-------
 drivers/pci/pci.c     | 21 +++++++++++++++++++++
 drivers/pci/pci.h     |  2 ++
 drivers/pci/probe.c   |  1 +
 drivers/pci/quirks.c  | 28 +++++++++++++++++++---------
 drivers/pci/search.c  | 14 +++++++++-----
 include/linux/pci.h   | 12 +++++-------
 7 files changed, 67 insertions(+), 28 deletions(-)

-- 
2.1.4

[PATCH v3 1/2] pci: Add support for multiple DMA aliases

[Jacek Lawrynowicz]

This patch solves IOMMU support issues with PCIe non-transparent bridges
that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
the bridge, packet's RID is rewritten according to LUT programmed by
a driver. Modified packets are then passed to a destination bus and
processed upstream. The problem is that such packets seem to come from
non-existent nodes that are hidden behind NTB and are not discoverable
by a destination node, so IOMMU discards them. Adding DMA alias for a
given LUT entry allows IOMMU to create a proper mapping that enables
inter-node communication.

The current DMA alias implementation supports only single alias, so it's
not possible to connect more than two nodes when IOMMU is enabled. This
implementation enables all possible aliases on a given bus (256) that
are stored in a bitset. Alias devfn is directly translated to a bit
number. The bitset is not allocated for devices that have no need for
DMA aliases.

More details can be found in following article:
http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/iommu/iommu.c | 17 ++++++++++-------
 drivers/pci/pci.c     | 21 +++++++++++++++++++++
 drivers/pci/pci.h     |  2 ++
 drivers/pci/probe.c   |  1 +
 drivers/pci/quirks.c  | 15 ++++++---------
 drivers/pci/search.c  | 14 +++++++++-----
 include/linux/pci.h   | 12 +++++-------
 7 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0e3b009..a214e19 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -659,9 +659,15 @@ static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev,
 	return NULL;
 }
 
+static bool dma_alias_is_enabled(struct pci_dev *dev, u8 devfn)
+{
+	return dev->dma_alias_mask &&
+	       test_bit(devfn, dev->dma_alias_mask);
+}
+
 /*
- * Look for aliases to or from the given device for exisiting groups.  The
- * dma_alias_devfn only supports aliases on the same bus, therefore the search
+ * Look for aliases to or from the given device for existing groups. DMA
+ * aliases are only supported on the same bus, therefore the search
  * space is quite small (especially since we're really only looking at pcie
  * device, and therefore only expect multiple slots on the root complex or
  * downstream switch ports).  It's conceivable though that a pair of
@@ -686,11 +692,8 @@ static struct iommu_group *get_pci_alias_group(struct pci_dev *pdev,
 			continue;
 
 		/* We alias them or they alias us */
-		if (((pdev->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     pdev->dma_alias_devfn == tmp->devfn) ||
-		    ((tmp->dev_flags & PCI_DEV_FLAGS_DMA_ALIAS_DEVFN) &&
-		     tmp->dma_alias_devfn == pdev->devfn)) {
-
+		if (dma_alias_is_enabled(pdev, tmp->devfn) ||
+		    dma_alias_is_enabled(tmp, pdev->devfn)) {
 			group = get_pci_alias_group(tmp, devfns);
 			if (group) {
 				pci_dev_put(tmp);
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 602eb42..d07e99d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4568,6 +4568,27 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
 	return 0;
 }
 
+/**
+ * pci_add_dma_alias - Allows to set multiple devfn aliases for given device
+ * @dev: the PCI device for which alias is added
+ * @devfn: alias slot and function
+ *
+ * This helper encodes 8-bit devfn as bit number in dma_alias_mask.
+ * It should be called early, preferably as PCI fixup header quirk.
+ */
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn)
+{
+	if (!dev->dma_alias_mask)
+		dev->dma_alias_mask = kcalloc(BITS_TO_LONGS(U8_MAX),
+					      sizeof(long), GFP_KERNEL);
+	if (!dev->dma_alias_mask) {
+		dev_warn(&dev->dev, "Unable to allocate DMA alias mask.\n");
+		return;
+	}
+	if (dev->dma_alias_mask)
+		set_bit(devfn, dev->dma_alias_mask);
+}
+
 bool pci_device_is_present(struct pci_dev *pdev)
 {
 	u32 v;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 9a1660f..c5dc8dc 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -335,4 +335,6 @@ static inline int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 }
 #endif
 
+void pci_add_dma_alias(struct pci_dev *dev, u8 devfn);
+
 #endif /* DRIVERS_PCI_H */
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6d7ab9b..23fc397 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1503,6 +1503,7 @@ static void pci_release_dev(struct device *dev)
 	pcibios_release_device(pci_dev);
 	pci_bus_put(pci_dev->bus);
 	kfree(pci_dev->driver_override);
+	kfree(pci_dev->dma_alias_mask);
 	kfree(pci_dev);
 }
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 0575a1e..27c298c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3582,8 +3582,7 @@ int pci_dev_specific_reset(struct pci_dev *dev, int probe)
 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;
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));
 	}
 }
 
@@ -3598,8 +3597,7 @@ 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;
+		pci_add_dma_alias(dev, PCI_DEVFN(PCI_SLOT(dev->devfn), 1));
 	}
 }
 
@@ -3667,11 +3665,10 @@ static void quirk_fixed_dma_alias(struct pci_dev *dev)
 
 	id = pci_match_id(fixed_dma_alias_tbl, dev);
 	if (id) {
-		dev->dma_alias_devfn = id->driver_data;
-		dev->dev_flags |= PCI_DEV_FLAGS_DMA_ALIAS_DEVFN;
-		dev_info(&dev->dev, "Enabling fixed DMA alias to %02x.%d\n",
-			 PCI_SLOT(dev->dma_alias_devfn),
-			 PCI_FUNC(dev->dma_alias_devfn));
+		pci_add_dma_alias(dev, id->driver_data);
+		dev_info(&dev->dev, "Enabling fixed DMA alias to %02lx.%ld\n",
+			 PCI_SLOT(id->driver_data),
+			 PCI_FUNC(id->driver_data));
 	}
 }
 
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index a20ce7d..33e0f03 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -40,11 +40,15 @@ int pci_for_each_dma_alias(struct pci_dev *pdev,
 	 * 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;
+	if (unlikely(pdev->dma_alias_mask)) {
+		u8 devfn;
+
+		for_each_set_bit(devfn, pdev->dma_alias_mask, U8_MAX) {
+			ret = fn(pdev, PCI_DEVID(pdev->bus->number, devfn),
+				 data);
+			if (ret)
+				return ret;
+		}
 	}
 
 	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 27df4a6..4e36024 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -172,16 +172,14 @@ 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),
 	/* 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),
+	PCI_DEV_FLAG_PCIE_BRIDGE_ALIAS = (__force pci_dev_flags_t) (1 << 4),
 	/* Do not use bus resets for device */
-	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 6),
+	PCI_DEV_FLAGS_NO_BUS_RESET = (__force pci_dev_flags_t) (1 << 5),
 	/* Do not use PM reset even if device advertises NoSoftRst- */
-	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 7),
+	PCI_DEV_FLAGS_NO_PM_RESET = (__force pci_dev_flags_t) (1 << 6),
 	/* Get VPD from function 0 VPD */
-	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 8),
+	PCI_DEV_FLAGS_VPD_REF_F0 = (__force pci_dev_flags_t) (1 << 7),
 };
 
 enum pci_irq_reroute_variant {
@@ -279,7 +277,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 */
+	unsigned long	*dma_alias_mask;/* mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* which driver has allocated this device */
 	u64		dma_mask;	/* Mask of the bits of bus address this
-- 
2.1.4

[PATCH v3 2/2] pci: Add DMA alias quirk for mic_x200_dma

[Jacek Lawrynowicz]

MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
be added as aliases to the DMA device in order to allow buffer access
when IOMMU is enabled.

Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
Acked-by: David Woodhouse <David.Woodhouse@intel.com>
---
 drivers/pci/quirks.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 27c298c..96ab9dc 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3702,6 +3702,19 @@ DECLARE_PCI_FIXUP_HEADER(0x1283, 0x8892, quirk_use_pcie_bridge_dma_alias);
 DECLARE_PCI_FIXUP_HEADER(0x8086, 0x244e, quirk_use_pcie_bridge_dma_alias);
 
 /*
+ * MIC x200 NTB forwards PCIe traffic using multiple alien RID. They have to
+ * be added as aliases to the DMA device in order to allow buffer access
+ * when IOMMU is enabled.
+ */
+static void quirk_mic_x200_dma_alias(struct pci_dev *pdev)
+{
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x10, 0x0));
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x11, 0x0));
+	pci_add_dma_alias(pdev, PCI_DEVFN(0x12, 0x3));
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x2264, quirk_mic_x200_dma_alias);
+
+/*
  * Intersil/Techwell TW686[4589]-based video capture cards have an empty (zero)
  * class code.  Fix it.
  */
-- 
2.1.4

Re: [PATCH v3 1/2] pci: Add support for multiple DMA aliases

[Joerg Roedel]

On Sat, Jan 30, 2016 at 12:06:51PM +0100, Jacek Lawrynowicz wrote:
> This patch solves IOMMU support issues with PCIe non-transparent bridges
> that use Requester ID look-up tables (LUT), e.g. PEX8733. Before exiting
> the bridge, packet's RID is rewritten according to LUT programmed by
> a driver. Modified packets are then passed to a destination bus and
> processed upstream. The problem is that such packets seem to come from
> non-existent nodes that are hidden behind NTB and are not discoverable
> by a destination node, so IOMMU discards them. Adding DMA alias for a
> given LUT entry allows IOMMU to create a proper mapping that enables
> inter-node communication.
> 
> The current DMA alias implementation supports only single alias, so it's
> not possible to connect more than two nodes when IOMMU is enabled. This
> implementation enables all possible aliases on a given bus (256) that
> are stored in a bitset. Alias devfn is directly translated to a bit
> number. The bitset is not allocated for devices that have no need for
> DMA aliases.
> 
> More details can be found in following article:
> http://www.plxtech.com/files/pdf/technical/expresslane/RTC_Enabling%20MulitHostSystemDesigns.pdf
> 
> Signed-off-by: Jacek Lawrynowicz <jacek.lawrynowicz@intel.com>
> Acked-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
>  drivers/iommu/iommu.c | 17 ++++++++++-------
>  drivers/pci/pci.c     | 21 +++++++++++++++++++++
>  drivers/pci/pci.h     |  2 ++
>  drivers/pci/probe.c   |  1 +
>  drivers/pci/quirks.c  | 15 ++++++---------
>  drivers/pci/search.c  | 14 +++++++++-----
>  include/linux/pci.h   | 12 +++++-------
>  7 files changed, 54 insertions(+), 28 deletions(-)

Looks good to me.

Acked-by: Joerg Roedel <jroedel@suse.de>