[PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Reza Arbab]

With recent kernels, TCE tables for NPU devices are no longer being
configured. That task was performed by pnv_npu_try_dma_set_bypass(), a
function that got swept away in recent overhauling of dma code.

Patches 1-4 here bring the lost function back and reintegrate it with
the updated generic iommu bypass infrastructure.

Patch 5 fixes a regression in behavior when a requested dma mask can not
be fulfilled.

Patches 6-8 are cleanup. I put these later in the set because they
aren't bisectable until after the restored code is wired back in.

Patches 9-11 refactor pnv_pci_ioda_iommu_bypass_supported(). It seems
wrong for a boolean *_supported() function to have side effects. They
reintroduce a pci controller based dma_set_mask() hook. If that's
undesirable, these last three patches can be dropped.

Reza Arbab (11):
  Revert "powerpc/powernv: Remove unused pnv_npu_try_dma_set_bypass()
    function"
  powerpc/powernv: Add pnv_ioda_pe_iommu_bypass_supported()
  powerpc/powernv/npu: Change pnv_npu_try_dma_set_bypass() argument
  powerpc/powernv/npu: Wire up pnv_npu_try_dma_set_bypass()
  powerpc/powernv: Return failure for some uses of dma_set_mask()
  powerpc/powernv: Remove intermediate variable
  powerpc/powernv/npu: Simplify pnv_npu_try_dma_set_bypass() loop
  powerpc/powernv: Replace open coded pnv_ioda_get_pe()s
  Revert "powerpc/pci: remove the dma_set_mask pci_controller ops
    methods"
  powerpc/powernv: Add pnv_phb3_iommu_bypass_supported()
  powerpc/powernv: Add pnv_pci_ioda_dma_set_mask()

 arch/powerpc/include/asm/pci-bridge.h     |   2 +
 arch/powerpc/kernel/dma-iommu.c           |  19 ++++--
 arch/powerpc/kernel/dma-mask.c            |   9 +++
 arch/powerpc/platforms/powernv/Kconfig    |   1 +
 arch/powerpc/platforms/powernv/npu-dma.c  | 106 +++++++++++++++++++++++++++---
 arch/powerpc/platforms/powernv/pci-ioda.c |  71 ++++++++++++--------
 arch/powerpc/platforms/powernv/pci.h      |  10 ++-
 7 files changed, 177 insertions(+), 41 deletions(-)

-- 
1.8.3.1

[PATCH 01/11] Revert "powerpc/powernv: Remove unused pnv_npu_try_dma_set_bypass() function"

[Reza Arbab]

Revert commit b4d37a7b6934 ("powerpc/powernv: Remove unused
pnv_npu_try_dma_set_bypass() function") so that this function can be
reintegrated.

Fixes: 2d6ad41b2c21 ("powerpc/powernv: use the generic iommu bypass code")
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/platforms/powernv/npu-dma.c | 99 ++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index b95b9e3c4c98..5a8313654033 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -193,6 +193,105 @@ static long pnv_npu_unset_window(struct iommu_table_group *table_group, int num)
 	return 0;
 }
 
+/*
+ * Enables 32 bit DMA on NPU.
+ */
+static void pnv_npu_dma_set_32(struct pnv_ioda_pe *npe)
+{
+	struct pci_dev *gpdev;
+	struct pnv_ioda_pe *gpe;
+	int64_t rc;
+
+	/*
+	 * Find the assoicated PCI devices and get the dma window
+	 * information from there.
+	 */
+	if (!npe->pdev || !(npe->flags & PNV_IODA_PE_DEV))
+		return;
+
+	gpe = get_gpu_pci_dev_and_pe(npe, &gpdev);
+	if (!gpe)
+		return;
+
+	rc = pnv_npu_set_window(&npe->table_group, 0,
+			gpe->table_group.tables[0]);
+
+	/*
+	 * NVLink devices use the same TCE table configuration as
+	 * their parent device so drivers shouldn't be doing DMA
+	 * operations directly on these devices.
+	 */
+	set_dma_ops(&npe->pdev->dev, &dma_dummy_ops);
+}
+
+/*
+ * Enables bypass mode on the NPU. The NPU only supports one
+ * window per link, so bypass needs to be explicitly enabled or
+ * disabled. Unlike for a PHB3 bypass and non-bypass modes can't be
+ * active at the same time.
+ */
+static int pnv_npu_dma_set_bypass(struct pnv_ioda_pe *npe)
+{
+	struct pnv_phb *phb = npe->phb;
+	int64_t rc = 0;
+	phys_addr_t top = memblock_end_of_DRAM();
+
+	if (phb->type != PNV_PHB_NPU_NVLINK || !npe->pdev)
+		return -EINVAL;
+
+	rc = pnv_npu_unset_window(&npe->table_group, 0);
+	if (rc != OPAL_SUCCESS)
+		return rc;
+
+	/* Enable the bypass window */
+
+	top = roundup_pow_of_two(top);
+	dev_info(&npe->pdev->dev, "Enabling bypass for PE %x\n",
+			npe->pe_number);
+	rc = opal_pci_map_pe_dma_window_real(phb->opal_id,
+			npe->pe_number, npe->pe_number,
+			0 /* bypass base */, top);
+
+	if (rc == OPAL_SUCCESS)
+		pnv_pci_ioda2_tce_invalidate_entire(phb, false);
+
+	return rc;
+}
+
+void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass)
+{
+	int i;
+	struct pnv_phb *phb;
+	struct pci_dn *pdn;
+	struct pnv_ioda_pe *npe;
+	struct pci_dev *npdev;
+
+	for (i = 0; ; ++i) {
+		npdev = pnv_pci_get_npu_dev(gpdev, i);
+
+		if (!npdev)
+			break;
+
+		pdn = pci_get_pdn(npdev);
+		if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
+			return;
+
+		phb = pci_bus_to_host(npdev->bus)->private_data;
+
+		/* We only do bypass if it's enabled on the linked device */
+		npe = &phb->ioda.pe_array[pdn->pe_number];
+
+		if (bypass) {
+			dev_info(&npdev->dev,
+					"Using 64-bit DMA iommu bypass\n");
+			pnv_npu_dma_set_bypass(npe);
+		} else {
+			dev_info(&npdev->dev, "Using 32-bit DMA via iommu\n");
+			pnv_npu_dma_set_32(npe);
+		}
+	}
+}
+
 /* Switch ownership from platform code to external user (e.g. VFIO) */
 static void pnv_npu_take_ownership(struct iommu_table_group *table_group)
 {
-- 
1.8.3.1

[PATCH 02/11] powerpc/powernv: Add pnv_ioda_pe_iommu_bypass_supported()

[Reza Arbab]

This little calculation will be needed in other places. Move it to a
convenience function.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 8 +++-----
 arch/powerpc/platforms/powernv/pci.h      | 8 ++++++++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index c28d0d9b7ee0..8849218187d7 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1838,11 +1838,9 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 		return false;
 
 	pe = &phb->ioda.pe_array[pdn->pe_number];
-	if (pe->tce_bypass_enabled) {
-		u64 top = pe->tce_bypass_base + memblock_end_of_DRAM() - 1;
-		if (dma_mask >= top)
-			return true;
-	}
+
+	if (pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask))
+		return true;
 
 	/*
 	 * If the device can't set the TCE bypass bit but still wants
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index f914f0b14e4e..41f7dec3aee5 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -4,6 +4,7 @@
 
 #include <linux/compiler.h>		/* for __printf */
 #include <linux/iommu.h>
+#include <linux/memblock.h>
 #include <asm/iommu.h>
 #include <asm/msi_bitmap.h>
 
@@ -247,4 +248,11 @@ extern void pnv_pci_setup_iommu_table(struct iommu_table *tbl,
 		void *tce_mem, u64 tce_size,
 		u64 dma_offset, unsigned int page_shift);
 
+static inline bool pnv_ioda_pe_iommu_bypass_supported(struct pnv_ioda_pe *pe,
+						      u64 mask)
+{
+	return pe->tce_bypass_enabled &&
+	       mask >= pe->tce_bypass_base + memblock_end_of_DRAM() - 1;
+}
+
 #endif /* __POWERNV_PCI_H */
-- 
1.8.3.1

[PATCH 03/11] powerpc/powernv/npu: Change pnv_npu_try_dma_set_bypass() argument

[Reza Arbab]

To enable simpler calling code, change this function to find the value
of bypass instead of taking it as an argument.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/npu-dma.c | 12 +++++++++---
 arch/powerpc/platforms/powernv/pci.h     |  2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index 5a8313654033..a6b8c7ad36e4 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -258,13 +258,21 @@ static int pnv_npu_dma_set_bypass(struct pnv_ioda_pe *npe)
 	return rc;
 }
 
-void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass)
+void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask)
 {
+	struct pnv_ioda_pe *gpe = pnv_ioda_get_pe(gpdev);
 	int i;
 	struct pnv_phb *phb;
 	struct pci_dn *pdn;
 	struct pnv_ioda_pe *npe;
 	struct pci_dev *npdev;
+	bool bypass;
+
+	if (!gpe)
+		return;
+
+	/* We only do bypass if it's enabled on the linked device */
+	bypass = pnv_ioda_pe_iommu_bypass_supported(gpe, mask);
 
 	for (i = 0; ; ++i) {
 		npdev = pnv_pci_get_npu_dev(gpdev, i);
@@ -277,8 +285,6 @@ void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass)
 			return;
 
 		phb = pci_bus_to_host(npdev->bus)->private_data;
-
-		/* We only do bypass if it's enabled on the linked device */
 		npe = &phb->ioda.pe_array[pdn->pe_number];
 
 		if (bypass) {
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 41f7dec3aee5..21db0f4cfb11 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -211,7 +211,7 @@ extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
 	pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__)
 
 /* Nvlink functions */
-extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
+extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask);
 extern void pnv_pci_ioda2_tce_invalidate_entire(struct pnv_phb *phb, bool rm);
 extern struct pnv_ioda_pe *pnv_pci_npu_setup_iommu(struct pnv_ioda_pe *npe);
 extern struct iommu_table_group *pnv_try_setup_npu_table_group(
-- 
1.8.3.1

[PATCH 04/11] powerpc/powernv/npu: Wire up pnv_npu_try_dma_set_bypass()

[Reza Arbab]

Rework of pnv_pci_ioda_iommu_bypass_supported() dropped a call to
pnv_npu_try_dma_set_bypass(). Reintroduce this call, so that the DMA
bypass configuration of a GPU device is propagated to its corresponding
NPU devices.

Fixes: 2d6ad41b2c21 ("powerpc/powernv: use the generic iommu bypass code")
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 8849218187d7..70e834635971 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1833,14 +1833,13 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 	struct pnv_phb *phb = hose->private_data;
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 	struct pnv_ioda_pe *pe;
+	bool bypass;
 
 	if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
 		return false;
 
 	pe = &phb->ioda.pe_array[pdn->pe_number];
-
-	if (pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask))
-		return true;
+	bypass = pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask);
 
 	/*
 	 * If the device can't set the TCE bypass bit but still wants
@@ -1848,7 +1847,8 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 	 * bypass the 32-bit region and be usable for 64-bit DMAs.
 	 * The device needs to be able to address all of this space.
 	 */
-	if (dma_mask >> 32 &&
+	if (!bypass &&
+	    dma_mask >> 32 &&
 	    dma_mask > (memory_hotplug_max() + (1ULL << 32)) &&
 	    /* pe->pdev should be set if it's a single device, pe->pbus if not */
 	    (pe->device_count == 1 || !pe->pbus) &&
@@ -1859,10 +1859,14 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 			return false;
 		/* 4GB offset bypasses 32-bit space */
 		pdev->dev.archdata.dma_offset = (1ULL << 32);
-		return true;
+
+		bypass = true;
 	}
 
-	return false;
+	/* Update peer npu devices */
+	pnv_npu_try_dma_set_bypass(pdev, dma_mask);
+
+	return bypass;
 }
 
 static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
-- 
1.8.3.1

[PATCH 05/11] powerpc/powernv: Return failure for some uses of dma_set_mask()

[Reza Arbab]

Rework of pnv_pci_ioda_dma_set_mask() effectively reverted commit
253fd51e2f53 ("powerpc/powernv/pci: Return failure for some uses of
dma_set_mask()").

Reintroduce the desired behavior that an unfulfilled request for a DMA
mask between 32 and 64 bits will return error instead of silently
falling back to 32 bits.

Fixes: 2d6ad41b2c21 ("powerpc/powernv: use the generic iommu bypass code")
Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/kernel/dma-iommu.c           | 19 +++++++++++++++----
 arch/powerpc/platforms/powernv/pci-ioda.c |  8 ++++++--
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/dma-iommu.c b/arch/powerpc/kernel/dma-iommu.c
index e486d1d78de2..e1693760654b 100644
--- a/arch/powerpc/kernel/dma-iommu.c
+++ b/arch/powerpc/kernel/dma-iommu.c
@@ -122,10 +122,21 @@ int dma_iommu_dma_supported(struct device *dev, u64 mask)
 {
 	struct iommu_table *tbl = get_iommu_table_base(dev);
 
-	if (dev_is_pci(dev) && dma_iommu_bypass_supported(dev, mask)) {
-		dev->archdata.iommu_bypass = true;
-		dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
-		return 1;
+	if (dev_is_pci(dev)) {
+		if (dma_iommu_bypass_supported(dev, mask)) {
+			dev->archdata.iommu_bypass = true;
+			dev_dbg(dev, "iommu: 64-bit OK, using fixed ops\n");
+			return 1;
+		}
+
+		if (mask >> 32) {
+			dev_err(dev, "Warning: IOMMU dma bypass not supported: mask 0x%016llx\n",
+				mask);
+
+			/* A 64-bit request falls back to default ops */
+			if (mask != DMA_BIT_MASK(64))
+				return 0;
+		}
 	}
 
 	if (!tbl) {
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 70e834635971..b78b5e81f941 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1863,8 +1863,12 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 		bypass = true;
 	}
 
-	/* Update peer npu devices */
-	pnv_npu_try_dma_set_bypass(pdev, dma_mask);
+	/*
+	 * Update peer npu devices. We also do this for the special case where
+	 * a 64-bit dma mask can't be fulfilled and falls back to default.
+	 */
+	if (bypass || !(dma_mask >> 32) || dma_mask == DMA_BIT_MASK(64))
+		pnv_npu_try_dma_set_bypass(pdev, dma_mask);
 
 	return bypass;
 }
-- 
1.8.3.1

[PATCH 06/11] powerpc/powernv: Remove intermediate variable

[Reza Arbab]

Trim the pointless temporary variable.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index b78b5e81f941..319152d30bc3 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1854,9 +1854,9 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 	    (pe->device_count == 1 || !pe->pbus) &&
 	    phb->model == PNV_PHB_MODEL_PHB3) {
 		/* Configure the bypass mode */
-		s64 rc = pnv_pci_ioda_dma_64bit_bypass(pe);
-		if (rc)
+		if (pnv_pci_ioda_dma_64bit_bypass(pe))
 			return false;
+
 		/* 4GB offset bypasses 32-bit space */
 		pdev->dev.archdata.dma_offset = (1ULL << 32);
 
-- 
1.8.3.1

[PATCH 07/11] powerpc/powernv/npu: Simplify pnv_npu_try_dma_set_bypass() loop

[Reza Arbab]

Write this loop more compactly to improve readability.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/npu-dma.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index a6b8c7ad36e4..a77ce7d71634 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -261,12 +261,12 @@ static int pnv_npu_dma_set_bypass(struct pnv_ioda_pe *npe)
 void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask)
 {
 	struct pnv_ioda_pe *gpe = pnv_ioda_get_pe(gpdev);
-	int i;
 	struct pnv_phb *phb;
 	struct pci_dn *pdn;
 	struct pnv_ioda_pe *npe;
 	struct pci_dev *npdev;
 	bool bypass;
+	int i = 0;
 
 	if (!gpe)
 		return;
@@ -274,12 +274,7 @@ void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask)
 	/* We only do bypass if it's enabled on the linked device */
 	bypass = pnv_ioda_pe_iommu_bypass_supported(gpe, mask);
 
-	for (i = 0; ; ++i) {
-		npdev = pnv_pci_get_npu_dev(gpdev, i);
-
-		if (!npdev)
-			break;
-
+	while ((npdev = pnv_pci_get_npu_dev(gpdev, i++))) {
 		pdn = pci_get_pdn(npdev);
 		if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
 			return;
-- 
1.8.3.1

[PATCH 08/11] powerpc/powernv: Replace open coded pnv_ioda_get_pe()s

[Reza Arbab]

Collapse several open coded instances of pnv_ioda_get_pe().

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/npu-dma.c  | 22 +++++-----------------
 arch/powerpc/platforms/powernv/pci-ioda.c | 10 +++-------
 2 files changed, 8 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index a77ce7d71634..0010b21d45b8 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -97,24 +97,17 @@ struct pci_dev *pnv_pci_get_npu_dev(struct pci_dev *gpdev, int index)
 static struct pnv_ioda_pe *get_gpu_pci_dev_and_pe(struct pnv_ioda_pe *npe,
 						  struct pci_dev **gpdev)
 {
-	struct pnv_phb *phb;
-	struct pci_controller *hose;
 	struct pci_dev *pdev;
 	struct pnv_ioda_pe *pe;
-	struct pci_dn *pdn;
 
 	pdev = pnv_pci_get_gpu_dev(npe->pdev);
 	if (!pdev)
 		return NULL;
 
-	pdn = pci_get_pdn(pdev);
-	if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
+	pe = pnv_ioda_get_pe(pdev);
+	if (WARN_ON(!pe))
 		return NULL;
 
-	hose = pci_bus_to_host(pdev->bus);
-	phb = hose->private_data;
-	pe = &phb->ioda.pe_array[pdn->pe_number];
-
 	if (gpdev)
 		*gpdev = pdev;
 
@@ -261,9 +254,6 @@ static int pnv_npu_dma_set_bypass(struct pnv_ioda_pe *npe)
 void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask)
 {
 	struct pnv_ioda_pe *gpe = pnv_ioda_get_pe(gpdev);
-	struct pnv_phb *phb;
-	struct pci_dn *pdn;
-	struct pnv_ioda_pe *npe;
 	struct pci_dev *npdev;
 	bool bypass;
 	int i = 0;
@@ -275,12 +265,10 @@ void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, u64 mask)
 	bypass = pnv_ioda_pe_iommu_bypass_supported(gpe, mask);
 
 	while ((npdev = pnv_pci_get_npu_dev(gpdev, i++))) {
-		pdn = pci_get_pdn(npdev);
-		if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
-			return;
+		struct pnv_ioda_pe *npe = pnv_ioda_get_pe(npdev);
 
-		phb = pci_bus_to_host(npdev->bus)->private_data;
-		npe = &phb->ioda.pe_array[pdn->pe_number];
+		if (WARN_ON(!npe))
+			return;
 
 		if (bypass) {
 			dev_info(&npdev->dev,
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 319152d30bc3..6b932cfc0deb 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1829,16 +1829,12 @@ static int pnv_pci_ioda_dma_64bit_bypass(struct pnv_ioda_pe *pe)
 static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 		u64 dma_mask)
 {
-	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
-	struct pnv_phb *phb = hose->private_data;
-	struct pci_dn *pdn = pci_get_pdn(pdev);
-	struct pnv_ioda_pe *pe;
+	struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
 	bool bypass;
 
-	if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE))
+	if (WARN_ON(!pe))
 		return false;
 
-	pe = &phb->ioda.pe_array[pdn->pe_number];
 	bypass = pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask);
 
 	/*
@@ -1852,7 +1848,7 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 	    dma_mask > (memory_hotplug_max() + (1ULL << 32)) &&
 	    /* pe->pdev should be set if it's a single device, pe->pbus if not */
 	    (pe->device_count == 1 || !pe->pbus) &&
-	    phb->model == PNV_PHB_MODEL_PHB3) {
+	    pe->phb->model == PNV_PHB_MODEL_PHB3) {
 		/* Configure the bypass mode */
 		if (pnv_pci_ioda_dma_64bit_bypass(pe))
 			return false;
-- 
1.8.3.1

[PATCH 09/11] Revert "powerpc/pci: remove the dma_set_mask pci_controller ops methods"

[Reza Arbab]

Bring back the pci controller based hook in dma_set_mask(), as it will
have a user again.

This reverts commit 662acad4067a ("powerpc/pci: remove the dma_set_mask
pci_controller ops methods"). The callback signature has been adjusted
with void return to fit its caller.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/include/asm/pci-bridge.h | 2 ++
 arch/powerpc/kernel/dma-mask.c        | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index ea6ec65970ef..8512dcd053c5 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -43,6 +43,8 @@ struct pci_controller_ops {
 	void		(*teardown_msi_irqs)(struct pci_dev *pdev);
 #endif
 
+	void		(*dma_set_mask)(struct pci_dev *pdev, u64 dma_mask);
+
 	void		(*shutdown)(struct pci_controller *hose);
 };
 
diff --git a/arch/powerpc/kernel/dma-mask.c b/arch/powerpc/kernel/dma-mask.c
index ffbbbc432612..35b5fd1b03a6 100644
--- a/arch/powerpc/kernel/dma-mask.c
+++ b/arch/powerpc/kernel/dma-mask.c
@@ -2,11 +2,20 @@
 
 #include <linux/dma-mapping.h>
 #include <linux/export.h>
+#include <linux/pci.h>
 #include <asm/machdep.h>
 
 void arch_dma_set_mask(struct device *dev, u64 dma_mask)
 {
 	if (ppc_md.dma_set_mask)
 		ppc_md.dma_set_mask(dev, dma_mask);
+
+	if (dev_is_pci(dev)) {
+		struct pci_dev *pdev = to_pci_dev(dev);
+		struct pci_controller *phb = pci_bus_to_host(pdev->bus);
+
+		if (phb->controller_ops.dma_set_mask)
+			phb->controller_ops.dma_set_mask(pdev, dma_mask);
+	}
 }
 EXPORT_SYMBOL(arch_dma_set_mask);
-- 
1.8.3.1

[PATCH 10/11] powerpc/powernv: Add pnv_phb3_iommu_bypass_supported()

[Reza Arbab]

Move this code to its own function for reuse. As a side benefit,
rearrange the comments and spread things out for readability.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/pci-ioda.c | 37 +++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 6b932cfc0deb..57e6a43d9a3a 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1826,6 +1826,30 @@ static int pnv_pci_ioda_dma_64bit_bypass(struct pnv_ioda_pe *pe)
 	return -EIO;
 }
 
+/*
+ * If the device can't set the TCE bypass bit but still wants
+ * to access 4GB or more, on PHB3 we can reconfigure TVE#0 to
+ * bypass the 32-bit region and be usable for 64-bit DMAs.
+ */
+static bool pnv_phb3_iommu_bypass_supported(struct pnv_ioda_pe *pe, u64 mask)
+{
+	if (pe->phb->model != PNV_PHB_MODEL_PHB3)
+		return false;
+
+	/* pe->pdev should be set if it's a single device, pe->pbus if not */
+	if (pe->pbus && pe->device_count != 1)
+		return false;
+
+	if (!(mask >> 32))
+		return false;
+
+	/* The device needs to be able to address all of this space. */
+	if (mask <= memory_hotplug_max() + (1ULL << 32))
+		return false;
+
+	return true;
+}
+
 static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 		u64 dma_mask)
 {
@@ -1837,18 +1861,7 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 
 	bypass = pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask);
 
-	/*
-	 * If the device can't set the TCE bypass bit but still wants
-	 * to access 4GB or more, on PHB3 we can reconfigure TVE#0 to
-	 * bypass the 32-bit region and be usable for 64-bit DMAs.
-	 * The device needs to be able to address all of this space.
-	 */
-	if (!bypass &&
-	    dma_mask >> 32 &&
-	    dma_mask > (memory_hotplug_max() + (1ULL << 32)) &&
-	    /* pe->pdev should be set if it's a single device, pe->pbus if not */
-	    (pe->device_count == 1 || !pe->pbus) &&
-	    pe->phb->model == PNV_PHB_MODEL_PHB3) {
+	if (!bypass && pnv_phb3_iommu_bypass_supported(pe, dma_mask)) {
 		/* Configure the bypass mode */
 		if (pnv_pci_ioda_dma_64bit_bypass(pe))
 			return false;
-- 
1.8.3.1

[PATCH 11/11] powerpc/powernv: Add pnv_pci_ioda_dma_set_mask()

[Reza Arbab]

Change pnv_pci_ioda_iommu_bypass_supported() to have no side effects, by
separating the part of the function that determines if bypass is
supported from the part that actually attempts to configure it.

Move the latter to a controller-specific dma_set_mask() callback.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/Kconfig    |  1 +
 arch/powerpc/platforms/powernv/pci-ioda.c | 30 ++++++++++++++++--------------
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 938803eab0ad..6e6e27841764 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -17,6 +17,7 @@ config PPC_POWERNV
 	select PPC_DOORBELL
 	select MMU_NOTIFIER
 	select FORCE_SMP
+	select ARCH_HAS_DMA_SET_MASK
 	default y
 
 config OPAL_PRD
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 57e6a43d9a3a..5291464930ed 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1854,32 +1854,33 @@ static bool pnv_pci_ioda_iommu_bypass_supported(struct pci_dev *pdev,
 		u64 dma_mask)
 {
 	struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
-	bool bypass;
 
 	if (WARN_ON(!pe))
 		return false;
 
-	bypass = pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask);
+	return pnv_ioda_pe_iommu_bypass_supported(pe, dma_mask) ||
+	       pnv_phb3_iommu_bypass_supported(pe, dma_mask);
+}
+
+static void pnv_pci_ioda_dma_set_mask(struct pci_dev *pdev, u64 mask)
+{
+	struct pnv_ioda_pe *pe = pnv_ioda_get_pe(pdev);
+
+	if (!pe)
+		return;
 
-	if (!bypass && pnv_phb3_iommu_bypass_supported(pe, dma_mask)) {
+	if (!pnv_ioda_pe_iommu_bypass_supported(pe, mask) &&
+	    pnv_phb3_iommu_bypass_supported(pe, mask)) {
 		/* Configure the bypass mode */
 		if (pnv_pci_ioda_dma_64bit_bypass(pe))
-			return false;
+			return;
 
 		/* 4GB offset bypasses 32-bit space */
 		pdev->dev.archdata.dma_offset = (1ULL << 32);
-
-		bypass = true;
 	}
 
-	/*
-	 * Update peer npu devices. We also do this for the special case where
-	 * a 64-bit dma mask can't be fulfilled and falls back to default.
-	 */
-	if (bypass || !(dma_mask >> 32) || dma_mask == DMA_BIT_MASK(64))
-		pnv_npu_try_dma_set_bypass(pdev, dma_mask);
-
-	return bypass;
+	/* Update peer npu devices */
+	pnv_npu_try_dma_set_bypass(pdev, mask);
 }
 
 static void pnv_ioda_setup_bus_dma(struct pnv_ioda_pe *pe, struct pci_bus *bus)
@@ -3612,6 +3613,7 @@ static void pnv_pci_ioda_shutdown(struct pci_controller *hose)
 static const struct pci_controller_ops pnv_pci_ioda_controller_ops = {
 	.dma_dev_setup		= pnv_pci_dma_dev_setup,
 	.dma_bus_setup		= pnv_pci_dma_bus_setup,
+	.dma_set_mask		= pnv_pci_ioda_dma_set_mask,
 	.iommu_bypass_supported	= pnv_pci_ioda_iommu_bypass_supported,
 	.setup_msi_irqs		= pnv_setup_msi_irqs,
 	.teardown_msi_irqs	= pnv_teardown_msi_irqs,
-- 
1.8.3.1

Re: [PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Christoph Hellwig]

On Wed, Oct 30, 2019 at 11:59:49AM -0500, Reza Arbab wrote:
> With recent kernels, TCE tables for NPU devices are no longer being
> configured. That task was performed by pnv_npu_try_dma_set_bypass(), a
> function that got swept away in recent overhauling of dma code.
> 
> Patches 1-4 here bring the lost function back and reintegrate it with
> the updated generic iommu bypass infrastructure.
> 
> Patch 5 fixes a regression in behavior when a requested dma mask can not
> be fulfilled.
> 
> Patches 6-8 are cleanup. I put these later in the set because they
> aren't bisectable until after the restored code is wired back in.
> 
> Patches 9-11 refactor pnv_pci_ioda_iommu_bypass_supported(). It seems
> wrong for a boolean *_supported() function to have side effects. They
> reintroduce a pci controller based dma_set_mask() hook. If that's
> undesirable, these last three patches can be dropped.

How do you even use this code?  Nothing in the kernel even calls
dma_set_mask for NPU devices, as we only suport vfio pass through.

Re: [PATCH 11/11] powerpc/powernv: Add pnv_pci_ioda_dma_set_mask()

[Christoph Hellwig]

On Wed, Oct 30, 2019 at 12:00:00PM -0500, Reza Arbab wrote:
> Change pnv_pci_ioda_iommu_bypass_supported() to have no side effects, by
> separating the part of the function that determines if bypass is
> supported from the part that actually attempts to configure it.
> 
> Move the latter to a controller-specific dma_set_mask() callback.

Nak, the dma_set_mask overrides are going away.  But as said in the
reply to the cover letter I don't even see how you could end up calling
this code.

Re: [PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Reza Arbab]

On Wed, Oct 30, 2019 at 06:53:41PM +0100, Christoph Hellwig wrote:
>How do you even use this code?  Nothing in the kernel even calls
>dma_set_mask for NPU devices, as we only suport vfio pass through.

You use it by calling dma_set_mask() for the *GPU* device. The purpose 
of pnv_npu_try_dma_set_bypass() is to then propagate the same bypass 
configuration to all the NPU devices associated with that GPU.

-- 
Reza Arbab

Re: [PATCH 11/11] powerpc/powernv: Add pnv_pci_ioda_dma_set_mask()

[Reza Arbab]

On Wed, Oct 30, 2019 at 06:55:18PM +0100, Christoph Hellwig wrote:
>On Wed, Oct 30, 2019 at 12:00:00PM -0500, Reza Arbab wrote:
>> Change pnv_pci_ioda_iommu_bypass_supported() to have no side effects, by
>> separating the part of the function that determines if bypass is
>> supported from the part that actually attempts to configure it.
>>
>> Move the latter to a controller-specific dma_set_mask() callback.
>
>Nak, the dma_set_mask overrides are going away.  But as said in the
>reply to the cover letter I don't even see how you could end up calling
>this code.

Okay. As mentioned in the cover letter these last few patches could be 
omitted if that's the case.

-- 
Reza Arbab

Re: [PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Christoph Hellwig]

On Wed, Oct 30, 2019 at 01:08:51PM -0500, Reza Arbab wrote:
> On Wed, Oct 30, 2019 at 06:53:41PM +0100, Christoph Hellwig wrote:
>> How do you even use this code?  Nothing in the kernel even calls
>> dma_set_mask for NPU devices, as we only suport vfio pass through.
>
> You use it by calling dma_set_mask() for the *GPU* device. The purpose of 
> pnv_npu_try_dma_set_bypass() is to then propagate the same bypass 
> configuration to all the NPU devices associated with that GPU.

Which in-kernel driver, which PCI ID?

Re: [PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Reza Arbab]

On Wed, Oct 30, 2019 at 07:13:59PM +0100, Christoph Hellwig wrote:
>On Wed, Oct 30, 2019 at 01:08:51PM -0500, Reza Arbab wrote:
>> On Wed, Oct 30, 2019 at 06:53:41PM +0100, Christoph Hellwig wrote:
>>> How do you even use this code?  Nothing in the kernel even calls
>>> dma_set_mask for NPU devices, as we only suport vfio pass through.
>>
>> You use it by calling dma_set_mask() for the *GPU* device. The purpose of
>> pnv_npu_try_dma_set_bypass() is to then propagate the same bypass
>> configuration to all the NPU devices associated with that GPU.
>
>Which in-kernel driver, which PCI ID?

Aha, it's this again. Didn't catch your meaning at first. Point taken.

-- 
Reza Arbab

Re: [PATCH 00/11] powerpv/powernv: Restore pnv_npu_try_dma_set_bypass()

[Christoph Hellwig]

On Wed, Oct 30, 2019 at 01:32:01PM -0500, Reza Arbab wrote:
> Aha, it's this again. Didn't catch your meaning at first. Point taken.

It's not _me_.  It that you (plural) keep ignoring how Linux development
works.