remove dead powernv code

remove dead powernv code

[Christoph Hellwig]

Hi all,

the powerpc powernv port has a fairly large chunk of code that never
had any upstream user.  We generally strive to not keep dead code
around, and this was affirmed at least years Maintainer summit.

[PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

This function has never been used since it has been added to the tree.
We also now have proper PCIe P2P APIs in the core kernel, and any new
P2P support should be using those.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/include/asm/opal.h            |  2 -
 arch/powerpc/include/asm/pnv-pci.h         |  2 -
 arch/powerpc/platforms/powernv/opal-call.c |  1 -
 arch/powerpc/platforms/powernv/pci.c       | 74 ----------------------
 arch/powerpc/platforms/powernv/pci.h       |  5 --
 5 files changed, 84 deletions(-)

diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index a55b01c90bb1..16c67f4eeb71 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -279,8 +279,6 @@ int64_t opal_xive_allocate_irq(uint32_t chip_id);
 int64_t opal_xive_free_irq(uint32_t girq);
 int64_t opal_xive_sync(uint32_t type, uint32_t id);
 int64_t opal_xive_dump(uint32_t type, uint32_t id);
-int64_t opal_pci_set_p2p(uint64_t phb_init, uint64_t phb_target,
-			uint64_t desc, uint16_t pe_number);
 
 int64_t opal_imc_counters_init(uint32_t type, uint64_t address,
 							uint64_t cpu_pir);
diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
index 630eb8b1b7ed..9fcb0bc462c6 100644
--- a/arch/powerpc/include/asm/pnv-pci.h
+++ b/arch/powerpc/include/asm/pnv-pci.h
@@ -26,8 +26,6 @@ extern int pnv_pci_get_presence_state(uint64_t id, uint8_t *state);
 extern int pnv_pci_get_power_state(uint64_t id, uint8_t *state);
 extern int pnv_pci_set_power_state(uint64_t id, uint8_t state,
 				   struct opal_msg *msg);
-extern int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target,
-			   u64 desc);
 
 extern int pnv_pci_enable_tunnel(struct pci_dev *dev, uint64_t *asnind);
 extern int pnv_pci_disable_tunnel(struct pci_dev *dev);
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index daad8c45c8e7..a89f0e31b468 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -267,7 +267,6 @@ OPAL_CALL(opal_npu_map_lpar,			OPAL_NPU_MAP_LPAR);
 OPAL_CALL(opal_imc_counters_init,		OPAL_IMC_COUNTERS_INIT);
 OPAL_CALL(opal_imc_counters_start,		OPAL_IMC_COUNTERS_START);
 OPAL_CALL(opal_imc_counters_stop,		OPAL_IMC_COUNTERS_STOP);
-OPAL_CALL(opal_pci_set_p2p,			OPAL_PCI_SET_P2P);
 OPAL_CALL(opal_get_powercap,			OPAL_GET_POWERCAP);
 OPAL_CALL(opal_set_powercap,			OPAL_SET_POWERCAP);
 OPAL_CALL(opal_get_power_shift_ratio,		OPAL_GET_POWER_SHIFT_RATIO);
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index ef9448a907c6..8d28f2932c3b 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -38,7 +38,6 @@
 #include "powernv.h"
 #include "pci.h"
 
-static DEFINE_MUTEX(p2p_mutex);
 static DEFINE_MUTEX(tunnel_mutex);
 
 int pnv_pci_get_slot_id(struct device_node *np, uint64_t *id)
@@ -861,79 +860,6 @@ void pnv_pci_dma_bus_setup(struct pci_bus *bus)
 	}
 }
 
-int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target, u64 desc)
-{
-	struct pci_controller *hose;
-	struct pnv_phb *phb_init, *phb_target;
-	struct pnv_ioda_pe *pe_init;
-	int rc;
-
-	if (!opal_check_token(OPAL_PCI_SET_P2P))
-		return -ENXIO;
-
-	hose = pci_bus_to_host(initiator->bus);
-	phb_init = hose->private_data;
-
-	hose = pci_bus_to_host(target->bus);
-	phb_target = hose->private_data;
-
-	pe_init = pnv_ioda_get_pe(initiator);
-	if (!pe_init)
-		return -ENODEV;
-
-	/*
-	 * Configuring the initiator's PHB requires to adjust its
-	 * TVE#1 setting. Since the same device can be an initiator
-	 * several times for different target devices, we need to keep
-	 * a reference count to know when we can restore the default
-	 * bypass setting on its TVE#1 when disabling. Opal is not
-	 * tracking PE states, so we add a reference count on the PE
-	 * in linux.
-	 *
-	 * For the target, the configuration is per PHB, so we keep a
-	 * target reference count on the PHB.
-	 */
-	mutex_lock(&p2p_mutex);
-
-	if (desc & OPAL_PCI_P2P_ENABLE) {
-		/* always go to opal to validate the configuration */
-		rc = opal_pci_set_p2p(phb_init->opal_id, phb_target->opal_id,
-				      desc, pe_init->pe_number);
-
-		if (rc != OPAL_SUCCESS) {
-			rc = -EIO;
-			goto out;
-		}
-
-		pe_init->p2p_initiator_count++;
-		phb_target->p2p_target_count++;
-	} else {
-		if (!pe_init->p2p_initiator_count ||
-			!phb_target->p2p_target_count) {
-			rc = -EINVAL;
-			goto out;
-		}
-
-		if (--pe_init->p2p_initiator_count == 0)
-			pnv_pci_ioda2_set_bypass(pe_init, true);
-
-		if (--phb_target->p2p_target_count == 0) {
-			rc = opal_pci_set_p2p(phb_init->opal_id,
-					      phb_target->opal_id, desc,
-					      pe_init->pe_number);
-			if (rc != OPAL_SUCCESS) {
-				rc = -EIO;
-				goto out;
-			}
-		}
-	}
-	rc = 0;
-out:
-	mutex_unlock(&p2p_mutex);
-	return rc;
-}
-EXPORT_SYMBOL_GPL(pnv_pci_set_p2p);
-
 struct device_node *pnv_pci_get_phb_node(struct pci_dev *dev)
 {
 	struct pci_controller *hose = pci_bus_to_host(dev->bus);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 8e36da379252..fbec1c76d7a7 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -78,9 +78,6 @@ struct pnv_ioda_pe {
 	struct pnv_ioda_pe	*master;
 	struct list_head	slaves;
 
-	/* PCI peer-to-peer*/
-	int			p2p_initiator_count;
-
 	/* Link in list of PE#s */
 	struct list_head	list;
 };
@@ -171,8 +168,6 @@ struct pnv_phb {
 	/* PHB and hub diagnostics */
 	unsigned int		diag_data_size;
 	u8			*diag_data;
-
-	int p2p_target_count;
 };
 
 extern struct pci_ops pnv_pci_ops;
-- 
2.20.1

[PATCH 2/3] powerpc/powernv: remove pnv_pci_{enable,disable}_tunnel

[Christoph Hellwig]

These have been unused ever since they've been added to the kernel.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/include/asm/pnv-pci.h        |  2 -
 arch/powerpc/platforms/powernv/pci-ioda.c |  4 +-
 arch/powerpc/platforms/powernv/pci.c      | 48 -----------------------
 arch/powerpc/platforms/powernv/pci.h      |  1 -
 4 files changed, 3 insertions(+), 52 deletions(-)

diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
index 9fcb0bc462c6..a8bd0aeeee03 100644
--- a/arch/powerpc/include/asm/pnv-pci.h
+++ b/arch/powerpc/include/asm/pnv-pci.h
@@ -27,8 +27,6 @@ extern int pnv_pci_get_power_state(uint64_t id, uint8_t *state);
 extern int pnv_pci_set_power_state(uint64_t id, uint8_t state,
 				   struct opal_msg *msg);
 
-extern int pnv_pci_enable_tunnel(struct pci_dev *dev, uint64_t *asnind);
-extern int pnv_pci_disable_tunnel(struct pci_dev *dev);
 extern int pnv_pci_set_tunnel_bar(struct pci_dev *dev, uint64_t addr,
 				  int enable);
 extern int pnv_pci_get_as_notify_info(struct task_struct *task, u32 *lpid,
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 3ead4c237ed0..ed73ba8e44a6 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -54,6 +54,8 @@
 static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK",
 					      "NPU_OCAPI" };
 
+static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
+
 void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
 			    const char *fmt, ...)
 {
@@ -2359,7 +2361,7 @@ static long pnv_pci_ioda2_set_window(struct iommu_table_group *table_group,
 	return 0;
 }
 
-void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
+static void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable)
 {
 	uint16_t window_id = (pe->pe_number << 1 ) + 1;
 	int64_t rc;
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 8d28f2932c3b..2bac62a7123d 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -868,54 +868,6 @@ struct device_node *pnv_pci_get_phb_node(struct pci_dev *dev)
 }
 EXPORT_SYMBOL(pnv_pci_get_phb_node);
 
-int pnv_pci_enable_tunnel(struct pci_dev *dev, u64 *asnind)
-{
-	struct device_node *np;
-	const __be32 *prop;
-	struct pnv_ioda_pe *pe;
-	uint16_t window_id;
-	int rc;
-
-	if (!radix_enabled())
-		return -ENXIO;
-
-	if (!(np = pnv_pci_get_phb_node(dev)))
-		return -ENXIO;
-
-	prop = of_get_property(np, "ibm,phb-indications", NULL);
-	of_node_put(np);
-
-	if (!prop || !prop[1])
-		return -ENXIO;
-
-	*asnind = (u64)be32_to_cpu(prop[1]);
-	pe = pnv_ioda_get_pe(dev);
-	if (!pe)
-		return -ENODEV;
-
-	/* Increase real window size to accept as_notify messages. */
-	window_id = (pe->pe_number << 1 ) + 1;
-	rc = opal_pci_map_pe_dma_window_real(pe->phb->opal_id, pe->pe_number,
-					     window_id, pe->tce_bypass_base,
-					     (uint64_t)1 << 48);
-	return opal_error_code(rc);
-}
-EXPORT_SYMBOL_GPL(pnv_pci_enable_tunnel);
-
-int pnv_pci_disable_tunnel(struct pci_dev *dev)
-{
-	struct pnv_ioda_pe *pe;
-
-	pe = pnv_ioda_get_pe(dev);
-	if (!pe)
-		return -ENODEV;
-
-	/* Restore default real window size. */
-	pnv_pci_ioda2_set_bypass(pe, true);
-	return 0;
-}
-EXPORT_SYMBOL_GPL(pnv_pci_disable_tunnel);
-
 int pnv_pci_set_tunnel_bar(struct pci_dev *dev, u64 addr, int enable)
 {
 	__be64 val;
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index fbec1c76d7a7..f0a6ff56e349 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -194,7 +194,6 @@ extern int pnv_setup_msi_irqs(struct pci_dev *pdev, int nvec, int type);
 extern void pnv_teardown_msi_irqs(struct pci_dev *pdev);
 extern struct pnv_ioda_pe *pnv_ioda_get_pe(struct pci_dev *dev);
 extern void pnv_set_msi_irq_chip(struct pnv_phb *phb, unsigned int virq);
-extern void pnv_pci_ioda2_set_bypass(struct pnv_ioda_pe *pe, bool enable);
 extern unsigned long pnv_pci_ioda2_get_table_size(__u32 page_shift,
 		__u64 window_size, __u32 levels);
 extern int pnv_eeh_post_init(void);
-- 
2.20.1

[PATCH 3/3] powerpc/powernv: remove dead NPU DMA code

[Christoph Hellwig]

None of these routines were ever used since they were added to the
kernel.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/include/asm/book3s/64/mmu.h |   3 -
 arch/powerpc/include/asm/powernv.h       |  22 -
 arch/powerpc/mm/mmu_context_book3s64.c   |   2 -
 arch/powerpc/platforms/powernv/npu-dma.c | 556 -----------------------
 4 files changed, 583 deletions(-)

diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 1ceee000c18d..d35197d376fd 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -120,9 +120,6 @@ typedef struct {
 	/* Number of users of the external (Nest) MMU */
 	atomic_t copros;
 
-	/* NPU NMMU context */
-	struct npu_context *npu_context;
-
 #ifdef CONFIG_PPC_MM_SLICES
 	 /* SLB page size encodings*/
 	unsigned char low_slices_psize[BITS_PER_LONG / BITS_PER_BYTE];
diff --git a/arch/powerpc/include/asm/powernv.h b/arch/powerpc/include/asm/powernv.h
index 05b552418519..40f868c5e93c 100644
--- a/arch/powerpc/include/asm/powernv.h
+++ b/arch/powerpc/include/asm/powernv.h
@@ -11,35 +11,13 @@
 #define _ASM_POWERNV_H
 
 #ifdef CONFIG_PPC_POWERNV
-#define NPU2_WRITE 1
 extern void powernv_set_nmmu_ptcr(unsigned long ptcr);
-extern struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
-			unsigned long flags,
-			void (*cb)(struct npu_context *, void *),
-			void *priv);
-extern void pnv_npu2_destroy_context(struct npu_context *context,
-				struct pci_dev *gpdev);
-extern int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
-				unsigned long *flags, unsigned long *status,
-				int count);
 
 void pnv_program_cpu_hotplug_lpcr(unsigned int cpu, u64 lpcr_val);
 
 void pnv_tm_init(void);
 #else
 static inline void powernv_set_nmmu_ptcr(unsigned long ptcr) { }
-static inline struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
-			unsigned long flags,
-			struct npu_context *(*cb)(struct npu_context *, void *),
-			void *priv) { return ERR_PTR(-ENODEV); }
-static inline void pnv_npu2_destroy_context(struct npu_context *context,
-					struct pci_dev *gpdev) { }
-
-static inline int pnv_npu2_handle_fault(struct npu_context *context,
-					uintptr_t *ea, unsigned long *flags,
-					unsigned long *status, int count) {
-	return -ENODEV;
-}
 
 static inline void pnv_tm_init(void) { }
 #endif
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index f720c5cc0b5e..a5d45a57befa 100644
--- a/arch/powerpc/mm/mmu_context_book3s64.c
+++ b/arch/powerpc/mm/mmu_context_book3s64.c
@@ -117,8 +117,6 @@ static int radix__init_new_context(struct mm_struct *mm)
 	 */
 	asm volatile("ptesync;isync" : : : "memory");
 
-	mm->context.npu_context = NULL;
-
 	return index;
 }
 
diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index dc23d9d2a7d9..5495d5b786aa 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -22,12 +22,6 @@
 
 #include "pci.h"
 
-/*
- * spinlock to protect initialisation of an npu_context for a particular
- * mm_struct.
- */
-static DEFINE_SPINLOCK(npu_context_lock);
-
 static struct pci_dev *get_pci_dev(struct device_node *dn)
 {
 	struct pci_dn *pdn = PCI_DN(dn);
@@ -362,15 +356,6 @@ struct npu_comp {
 /* An NPU descriptor, valid for POWER9 only */
 struct npu {
 	int index;
-	__be64 *mmio_atsd_regs[NV_NMMU_ATSD_REGS];
-	unsigned int mmio_atsd_count;
-
-	/* Bitmask for MMIO register usage */
-	unsigned long mmio_atsd_usage;
-
-	/* Do we need to explicitly flush the nest mmu? */
-	bool nmmu_flush;
-
 	struct npu_comp npucomp;
 };
 
@@ -627,534 +612,8 @@ struct iommu_table_group *pnv_npu_compound_attach(struct pnv_ioda_pe *pe)
 }
 #endif /* CONFIG_IOMMU_API */
 
-/* Maximum number of nvlinks per npu */
-#define NV_MAX_LINKS 6
-
-/* Maximum index of npu2 hosts in the system. Always < NV_MAX_NPUS */
-static int max_npu2_index;
-
-struct npu_context {
-	struct mm_struct *mm;
-	struct pci_dev *npdev[NV_MAX_NPUS][NV_MAX_LINKS];
-	struct mmu_notifier mn;
-	struct kref kref;
-	bool nmmu_flush;
-
-	/* Callback to stop translation requests on a given GPU */
-	void (*release_cb)(struct npu_context *context, void *priv);
-
-	/*
-	 * Private pointer passed to the above callback for usage by
-	 * device drivers.
-	 */
-	void *priv;
-};
-
-struct mmio_atsd_reg {
-	struct npu *npu;
-	int reg;
-};
-
-/*
- * Find a free MMIO ATSD register and mark it in use. Return -ENOSPC
- * if none are available.
- */
-static int get_mmio_atsd_reg(struct npu *npu)
-{
-	int i;
-
-	for (i = 0; i < npu->mmio_atsd_count; i++) {
-		if (!test_bit(i, &npu->mmio_atsd_usage))
-			if (!test_and_set_bit_lock(i, &npu->mmio_atsd_usage))
-				return i;
-	}
-
-	return -ENOSPC;
-}
-
-static void put_mmio_atsd_reg(struct npu *npu, int reg)
-{
-	clear_bit_unlock(reg, &npu->mmio_atsd_usage);
-}
-
-/* MMIO ATSD register offsets */
-#define XTS_ATSD_LAUNCH 0
-#define XTS_ATSD_AVA    1
-#define XTS_ATSD_STAT   2
-
-static unsigned long get_atsd_launch_val(unsigned long pid, unsigned long psize)
-{
-	unsigned long launch = 0;
-
-	if (psize == MMU_PAGE_COUNT) {
-		/* IS set to invalidate entire matching PID */
-		launch |= PPC_BIT(12);
-	} else {
-		/* AP set to invalidate region of psize */
-		launch |= (u64)mmu_get_ap(psize) << PPC_BITLSHIFT(17);
-	}
-
-	/* PRS set to process-scoped */
-	launch |= PPC_BIT(13);
-
-	/* PID */
-	launch |= pid << PPC_BITLSHIFT(38);
-
-	/* Leave "No flush" (bit 39) 0 so every ATSD performs a flush */
-
-	return launch;
-}
-
-static void mmio_atsd_regs_write(struct mmio_atsd_reg
-			mmio_atsd_reg[NV_MAX_NPUS], unsigned long offset,
-			unsigned long val)
-{
-	struct npu *npu;
-	int i, reg;
-
-	for (i = 0; i <= max_npu2_index; i++) {
-		reg = mmio_atsd_reg[i].reg;
-		if (reg < 0)
-			continue;
-
-		npu = mmio_atsd_reg[i].npu;
-		__raw_writeq_be(val, npu->mmio_atsd_regs[reg] + offset);
-	}
-}
-
-static void mmio_invalidate_pid(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS],
-				unsigned long pid)
-{
-	unsigned long launch = get_atsd_launch_val(pid, MMU_PAGE_COUNT);
-
-	/* Invalidating the entire process doesn't use a va */
-	mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
-}
-
-static void mmio_invalidate_range(struct mmio_atsd_reg
-			mmio_atsd_reg[NV_MAX_NPUS], unsigned long pid,
-			unsigned long start, unsigned long psize)
-{
-	unsigned long launch = get_atsd_launch_val(pid, psize);
-
-	/* Write all VAs first */
-	mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_AVA, start);
-
-	/* Issue one barrier for all address writes */
-	eieio();
-
-	/* Launch */
-	mmio_atsd_regs_write(mmio_atsd_reg, XTS_ATSD_LAUNCH, launch);
-}
-
-#define mn_to_npu_context(x) container_of(x, struct npu_context, mn)
-
-static void mmio_invalidate_wait(
-	struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
-{
-	struct npu *npu;
-	int i, reg;
-
-	/* Wait for all invalidations to complete */
-	for (i = 0; i <= max_npu2_index; i++) {
-		if (mmio_atsd_reg[i].reg < 0)
-			continue;
-
-		/* Wait for completion */
-		npu = mmio_atsd_reg[i].npu;
-		reg = mmio_atsd_reg[i].reg;
-		while (__raw_readq(npu->mmio_atsd_regs[reg] + XTS_ATSD_STAT))
-			cpu_relax();
-	}
-}
-
-/*
- * Acquires all the address translation shootdown (ATSD) registers required to
- * launch an ATSD on all links this npu_context is active on.
- */
-static void acquire_atsd_reg(struct npu_context *npu_context,
-			struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
-{
-	int i, j;
-	struct npu *npu;
-	struct pci_dev *npdev;
-
-	for (i = 0; i <= max_npu2_index; i++) {
-		mmio_atsd_reg[i].reg = -1;
-		for (j = 0; j < NV_MAX_LINKS; j++) {
-			/*
-			 * There are no ordering requirements with respect to
-			 * the setup of struct npu_context, but to ensure
-			 * consistent behaviour we need to ensure npdev[][] is
-			 * only read once.
-			 */
-			npdev = READ_ONCE(npu_context->npdev[i][j]);
-			if (!npdev)
-				continue;
-
-			npu = pci_bus_to_host(npdev->bus)->npu;
-			if (!npu)
-				continue;
-
-			mmio_atsd_reg[i].npu = npu;
-			mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
-			while (mmio_atsd_reg[i].reg < 0) {
-				mmio_atsd_reg[i].reg = get_mmio_atsd_reg(npu);
-				cpu_relax();
-			}
-			break;
-		}
-	}
-}
-
-/*
- * Release previously acquired ATSD registers. To avoid deadlocks the registers
- * must be released in the same order they were acquired above in
- * acquire_atsd_reg.
- */
-static void release_atsd_reg(struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS])
-{
-	int i;
-
-	for (i = 0; i <= max_npu2_index; i++) {
-		/*
-		 * We can't rely on npu_context->npdev[][] being the same here
-		 * as when acquire_atsd_reg() was called, hence we use the
-		 * values stored in mmio_atsd_reg during the acquire phase
-		 * rather than re-reading npdev[][].
-		 */
-		if (mmio_atsd_reg[i].reg < 0)
-			continue;
-
-		put_mmio_atsd_reg(mmio_atsd_reg[i].npu, mmio_atsd_reg[i].reg);
-	}
-}
-
-/*
- * Invalidate a virtual address range
- */
-static void mmio_invalidate(struct npu_context *npu_context,
-			unsigned long start, unsigned long size)
-{
-	struct mmio_atsd_reg mmio_atsd_reg[NV_MAX_NPUS];
-	unsigned long pid = npu_context->mm->context.id;
-	unsigned long atsd_start = 0;
-	unsigned long end = start + size - 1;
-	int atsd_psize = MMU_PAGE_COUNT;
-
-	/*
-	 * Convert the input range into one of the supported sizes. If the range
-	 * doesn't fit, use the next larger supported size. Invalidation latency
-	 * is high, so over-invalidation is preferred to issuing multiple
-	 * invalidates.
-	 *
-	 * A 4K page size isn't supported by NPU/GPU ATS, so that case is
-	 * ignored.
-	 */
-	if (size == SZ_64K) {
-		atsd_start = start;
-		atsd_psize = MMU_PAGE_64K;
-	} else if (ALIGN_DOWN(start, SZ_2M) == ALIGN_DOWN(end, SZ_2M)) {
-		atsd_start = ALIGN_DOWN(start, SZ_2M);
-		atsd_psize = MMU_PAGE_2M;
-	} else if (ALIGN_DOWN(start, SZ_1G) == ALIGN_DOWN(end, SZ_1G)) {
-		atsd_start = ALIGN_DOWN(start, SZ_1G);
-		atsd_psize = MMU_PAGE_1G;
-	}
-
-	if (npu_context->nmmu_flush)
-		/*
-		 * Unfortunately the nest mmu does not support flushing specific
-		 * addresses so we have to flush the whole mm once before
-		 * shooting down the GPU translation.
-		 */
-		flush_all_mm(npu_context->mm);
-
-	/*
-	 * Loop over all the NPUs this process is active on and launch
-	 * an invalidate.
-	 */
-	acquire_atsd_reg(npu_context, mmio_atsd_reg);
-
-	if (atsd_psize == MMU_PAGE_COUNT)
-		mmio_invalidate_pid(mmio_atsd_reg, pid);
-	else
-		mmio_invalidate_range(mmio_atsd_reg, pid, atsd_start,
-					atsd_psize);
-
-	mmio_invalidate_wait(mmio_atsd_reg);
-
-	/*
-	 * The GPU requires two flush ATSDs to ensure all entries have been
-	 * flushed. We use PID 0 as it will never be used for a process on the
-	 * GPU.
-	 */
-	mmio_invalidate_pid(mmio_atsd_reg, 0);
-	mmio_invalidate_wait(mmio_atsd_reg);
-	mmio_invalidate_pid(mmio_atsd_reg, 0);
-	mmio_invalidate_wait(mmio_atsd_reg);
-
-	release_atsd_reg(mmio_atsd_reg);
-}
-
-static void pnv_npu2_mn_release(struct mmu_notifier *mn,
-				struct mm_struct *mm)
-{
-	struct npu_context *npu_context = mn_to_npu_context(mn);
-
-	/* Call into device driver to stop requests to the NMMU */
-	if (npu_context->release_cb)
-		npu_context->release_cb(npu_context, npu_context->priv);
-
-	/*
-	 * There should be no more translation requests for this PID, but we
-	 * need to ensure any entries for it are removed from the TLB.
-	 */
-	mmio_invalidate(npu_context, 0, ~0UL);
-}
-
-static void pnv_npu2_mn_invalidate_range(struct mmu_notifier *mn,
-					struct mm_struct *mm,
-					unsigned long start, unsigned long end)
-{
-	struct npu_context *npu_context = mn_to_npu_context(mn);
-	mmio_invalidate(npu_context, start, end - start);
-}
-
-static const struct mmu_notifier_ops nv_nmmu_notifier_ops = {
-	.release = pnv_npu2_mn_release,
-	.invalidate_range = pnv_npu2_mn_invalidate_range,
-};
-
-/*
- * Call into OPAL to setup the nmmu context for the current task in
- * the NPU. This must be called to setup the context tables before the
- * GPU issues ATRs. pdev should be a pointed to PCIe GPU device.
- *
- * A release callback should be registered to allow a device driver to
- * be notified that it should not launch any new translation requests
- * as the final TLB invalidate is about to occur.
- *
- * Returns an error if there no contexts are currently available or a
- * npu_context which should be passed to pnv_npu2_handle_fault().
- *
- * mmap_sem must be held in write mode and must not be called from interrupt
- * context.
- */
-struct npu_context *pnv_npu2_init_context(struct pci_dev *gpdev,
-			unsigned long flags,
-			void (*cb)(struct npu_context *, void *),
-			void *priv)
-{
-	int rc;
-	u32 nvlink_index;
-	struct device_node *nvlink_dn;
-	struct mm_struct *mm = current->mm;
-	struct npu *npu;
-	struct npu_context *npu_context;
-	struct pci_controller *hose;
-
-	/*
-	 * At present we don't support GPUs connected to multiple NPUs and I'm
-	 * not sure the hardware does either.
-	 */
-	struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
-
-	if (!npdev)
-		/* No nvlink associated with this GPU device */
-		return ERR_PTR(-ENODEV);
-
-	/* We only support DR/PR/HV in pnv_npu2_map_lpar_dev() */
-	if (flags & ~(MSR_DR | MSR_PR | MSR_HV))
-		return ERR_PTR(-EINVAL);
-
-	nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
-	if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
-							&nvlink_index)))
-		return ERR_PTR(-ENODEV);
-
-	if (!mm || mm->context.id == 0) {
-		/*
-		 * Kernel thread contexts are not supported and context id 0 is
-		 * reserved on the GPU.
-		 */
-		return ERR_PTR(-EINVAL);
-	}
-
-	hose = pci_bus_to_host(npdev->bus);
-	npu = hose->npu;
-	if (!npu)
-		return ERR_PTR(-ENODEV);
-
-	/*
-	 * We store the npu pci device so we can more easily get at the
-	 * associated npus.
-	 */
-	spin_lock(&npu_context_lock);
-	npu_context = mm->context.npu_context;
-	if (npu_context) {
-		if (npu_context->release_cb != cb ||
-			npu_context->priv != priv) {
-			spin_unlock(&npu_context_lock);
-			return ERR_PTR(-EINVAL);
-		}
-
-		WARN_ON(!kref_get_unless_zero(&npu_context->kref));
-	}
-	spin_unlock(&npu_context_lock);
-
-	if (!npu_context) {
-		/*
-		 * We can set up these fields without holding the
-		 * npu_context_lock as the npu_context hasn't been returned to
-		 * the caller meaning it can't be destroyed. Parallel allocation
-		 * is protected against by mmap_sem.
-		 */
-		rc = -ENOMEM;
-		npu_context = kzalloc(sizeof(struct npu_context), GFP_KERNEL);
-		if (npu_context) {
-			kref_init(&npu_context->kref);
-			npu_context->mm = mm;
-			npu_context->mn.ops = &nv_nmmu_notifier_ops;
-			rc = __mmu_notifier_register(&npu_context->mn, mm);
-		}
-
-		if (rc) {
-			kfree(npu_context);
-			return ERR_PTR(rc);
-		}
-
-		mm->context.npu_context = npu_context;
-	}
-
-	npu_context->release_cb = cb;
-	npu_context->priv = priv;
-
-	/*
-	 * npdev is a pci_dev pointer setup by the PCI code. We assign it to
-	 * npdev[][] to indicate to the mmu notifiers that an invalidation
-	 * should also be sent over this nvlink. The notifiers don't use any
-	 * other fields in npu_context, so we just need to ensure that when they
-	 * deference npu_context->npdev[][] it is either a valid pointer or
-	 * NULL.
-	 */
-	WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], npdev);
-
-	if (!npu->nmmu_flush) {
-		/*
-		 * If we're not explicitly flushing ourselves we need to mark
-		 * the thread for global flushes
-		 */
-		npu_context->nmmu_flush = false;
-		mm_context_add_copro(mm);
-	} else
-		npu_context->nmmu_flush = true;
-
-	return npu_context;
-}
-EXPORT_SYMBOL(pnv_npu2_init_context);
-
-static void pnv_npu2_release_context(struct kref *kref)
-{
-	struct npu_context *npu_context =
-		container_of(kref, struct npu_context, kref);
-
-	if (!npu_context->nmmu_flush)
-		mm_context_remove_copro(npu_context->mm);
-
-	npu_context->mm->context.npu_context = NULL;
-}
-
-/*
- * Destroy a context on the given GPU. May free the npu_context if it is no
- * longer active on any GPUs. Must not be called from interrupt context.
- */
-void pnv_npu2_destroy_context(struct npu_context *npu_context,
-			struct pci_dev *gpdev)
-{
-	int removed;
-	struct npu *npu;
-	struct pci_dev *npdev = pnv_pci_get_npu_dev(gpdev, 0);
-	struct device_node *nvlink_dn;
-	u32 nvlink_index;
-	struct pci_controller *hose;
-
-	if (WARN_ON(!npdev))
-		return;
-
-	hose = pci_bus_to_host(npdev->bus);
-	npu = hose->npu;
-	if (!npu)
-		return;
-	nvlink_dn = of_parse_phandle(npdev->dev.of_node, "ibm,nvlink", 0);
-	if (WARN_ON(of_property_read_u32(nvlink_dn, "ibm,npu-link-index",
-							&nvlink_index)))
-		return;
-	WRITE_ONCE(npu_context->npdev[npu->index][nvlink_index], NULL);
-	spin_lock(&npu_context_lock);
-	removed = kref_put(&npu_context->kref, pnv_npu2_release_context);
-	spin_unlock(&npu_context_lock);
-
-	/*
-	 * We need to do this outside of pnv_npu2_release_context so that it is
-	 * outside the spinlock as mmu_notifier_destroy uses SRCU.
-	 */
-	if (removed) {
-		mmu_notifier_unregister(&npu_context->mn,
-					npu_context->mm);
-
-		kfree(npu_context);
-	}
-
-}
-EXPORT_SYMBOL(pnv_npu2_destroy_context);
-
-/*
- * Assumes mmap_sem is held for the contexts associated mm.
- */
-int pnv_npu2_handle_fault(struct npu_context *context, uintptr_t *ea,
-			unsigned long *flags, unsigned long *status, int count)
-{
-	u64 rc = 0, result = 0;
-	int i, is_write;
-	struct page *page[1];
-	const char __user *u;
-	char c;
-
-	/* mmap_sem should be held so the struct_mm must be present */
-	struct mm_struct *mm = context->mm;
-
-	WARN_ON(!rwsem_is_locked(&mm->mmap_sem));
-
-	for (i = 0; i < count; i++) {
-		is_write = flags[i] & NPU2_WRITE;
-		rc = get_user_pages_remote(NULL, mm, ea[i], 1,
-					is_write ? FOLL_WRITE : 0,
-					page, NULL, NULL);
-
-		if (rc != 1) {
-			status[i] = rc;
-			result = -EFAULT;
-			continue;
-		}
-
-		/* Make sure partition scoped tree gets a pte */
-		u = page_address(page[0]);
-		if (__get_user(c, u))
-			result = -EFAULT;
-
-		status[i] = 0;
-		put_page(page[0]);
-	}
-
-	return result;
-}
-EXPORT_SYMBOL(pnv_npu2_handle_fault);
-
 int pnv_npu2_init(struct pci_controller *hose)
 {
-	unsigned int i;
-	u64 mmio_atsd;
 	static int npu_index;
 	struct npu *npu;
 	int ret;
@@ -1163,33 +622,18 @@ int pnv_npu2_init(struct pci_controller *hose)
 	if (!npu)
 		return -ENOMEM;
 
-	npu->nmmu_flush = of_property_read_bool(hose->dn, "ibm,nmmu-flush");
-
-	for (i = 0; i < ARRAY_SIZE(npu->mmio_atsd_regs) &&
-			!of_property_read_u64_index(hose->dn, "ibm,mmio-atsd",
-				i, &mmio_atsd); i++)
-		npu->mmio_atsd_regs[i] = ioremap(mmio_atsd, 32);
-
-	pr_info("NPU%d: Found %d MMIO ATSD registers", hose->global_number, i);
-	npu->mmio_atsd_count = i;
-	npu->mmio_atsd_usage = 0;
 	npu_index++;
 	if (WARN_ON(npu_index >= NV_MAX_NPUS)) {
 		ret = -ENOSPC;
 		goto fail_exit;
 	}
-	max_npu2_index = npu_index;
 	npu->index = npu_index;
 	hose->npu = npu;
 
 	return 0;
 
 fail_exit:
-	for (i = 0; i < npu->mmio_atsd_count; ++i)
-		iounmap(npu->mmio_atsd_regs[i]);
-
 	kfree(npu);
-
 	return ret;
 }
 
-- 
2.20.1

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Frederic Barrat]

Hi,

The PCI p2p and tunnel code is used by the Mellanox CX5 driver, at least 
their latest, out of tree version, which is used for CORAL. My 
understanding is that they'll upstream it at some point, though I don't 
know what their schedule is like.

   Fred


Le 26/04/2019 à 14:49, Christoph Hellwig a écrit :
> This function has never been used since it has been added to the tree.
> We also now have proper PCIe P2P APIs in the core kernel, and any new
> P2P support should be using those.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   arch/powerpc/include/asm/opal.h            |  2 -
>   arch/powerpc/include/asm/pnv-pci.h         |  2 -
>   arch/powerpc/platforms/powernv/opal-call.c |  1 -
>   arch/powerpc/platforms/powernv/pci.c       | 74 ----------------------
>   arch/powerpc/platforms/powernv/pci.h       |  5 --
>   5 files changed, 84 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index a55b01c90bb1..16c67f4eeb71 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -279,8 +279,6 @@ int64_t opal_xive_allocate_irq(uint32_t chip_id);
>   int64_t opal_xive_free_irq(uint32_t girq);
>   int64_t opal_xive_sync(uint32_t type, uint32_t id);
>   int64_t opal_xive_dump(uint32_t type, uint32_t id);
> -int64_t opal_pci_set_p2p(uint64_t phb_init, uint64_t phb_target,
> -			uint64_t desc, uint16_t pe_number);
>   
>   int64_t opal_imc_counters_init(uint32_t type, uint64_t address,
>   							uint64_t cpu_pir);
> diff --git a/arch/powerpc/include/asm/pnv-pci.h b/arch/powerpc/include/asm/pnv-pci.h
> index 630eb8b1b7ed..9fcb0bc462c6 100644
> --- a/arch/powerpc/include/asm/pnv-pci.h
> +++ b/arch/powerpc/include/asm/pnv-pci.h
> @@ -26,8 +26,6 @@ extern int pnv_pci_get_presence_state(uint64_t id, uint8_t *state);
>   extern int pnv_pci_get_power_state(uint64_t id, uint8_t *state);
>   extern int pnv_pci_set_power_state(uint64_t id, uint8_t state,
>   				   struct opal_msg *msg);
> -extern int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target,
> -			   u64 desc);
>   
>   extern int pnv_pci_enable_tunnel(struct pci_dev *dev, uint64_t *asnind);
>   extern int pnv_pci_disable_tunnel(struct pci_dev *dev);
> diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
> index daad8c45c8e7..a89f0e31b468 100644
> --- a/arch/powerpc/platforms/powernv/opal-call.c
> +++ b/arch/powerpc/platforms/powernv/opal-call.c
> @@ -267,7 +267,6 @@ OPAL_CALL(opal_npu_map_lpar,			OPAL_NPU_MAP_LPAR);
>   OPAL_CALL(opal_imc_counters_init,		OPAL_IMC_COUNTERS_INIT);
>   OPAL_CALL(opal_imc_counters_start,		OPAL_IMC_COUNTERS_START);
>   OPAL_CALL(opal_imc_counters_stop,		OPAL_IMC_COUNTERS_STOP);
> -OPAL_CALL(opal_pci_set_p2p,			OPAL_PCI_SET_P2P);
>   OPAL_CALL(opal_get_powercap,			OPAL_GET_POWERCAP);
>   OPAL_CALL(opal_set_powercap,			OPAL_SET_POWERCAP);
>   OPAL_CALL(opal_get_power_shift_ratio,		OPAL_GET_POWER_SHIFT_RATIO);
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index ef9448a907c6..8d28f2932c3b 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -38,7 +38,6 @@
>   #include "powernv.h"
>   #include "pci.h"
>   
> -static DEFINE_MUTEX(p2p_mutex);
>   static DEFINE_MUTEX(tunnel_mutex);
>   
>   int pnv_pci_get_slot_id(struct device_node *np, uint64_t *id)
> @@ -861,79 +860,6 @@ void pnv_pci_dma_bus_setup(struct pci_bus *bus)
>   	}
>   }
>   
> -int pnv_pci_set_p2p(struct pci_dev *initiator, struct pci_dev *target, u64 desc)
> -{
> -	struct pci_controller *hose;
> -	struct pnv_phb *phb_init, *phb_target;
> -	struct pnv_ioda_pe *pe_init;
> -	int rc;
> -
> -	if (!opal_check_token(OPAL_PCI_SET_P2P))
> -		return -ENXIO;
> -
> -	hose = pci_bus_to_host(initiator->bus);
> -	phb_init = hose->private_data;
> -
> -	hose = pci_bus_to_host(target->bus);
> -	phb_target = hose->private_data;
> -
> -	pe_init = pnv_ioda_get_pe(initiator);
> -	if (!pe_init)
> -		return -ENODEV;
> -
> -	/*
> -	 * Configuring the initiator's PHB requires to adjust its
> -	 * TVE#1 setting. Since the same device can be an initiator
> -	 * several times for different target devices, we need to keep
> -	 * a reference count to know when we can restore the default
> -	 * bypass setting on its TVE#1 when disabling. Opal is not
> -	 * tracking PE states, so we add a reference count on the PE
> -	 * in linux.
> -	 *
> -	 * For the target, the configuration is per PHB, so we keep a
> -	 * target reference count on the PHB.
> -	 */
> -	mutex_lock(&p2p_mutex);
> -
> -	if (desc & OPAL_PCI_P2P_ENABLE) {
> -		/* always go to opal to validate the configuration */
> -		rc = opal_pci_set_p2p(phb_init->opal_id, phb_target->opal_id,
> -				      desc, pe_init->pe_number);
> -
> -		if (rc != OPAL_SUCCESS) {
> -			rc = -EIO;
> -			goto out;
> -		}
> -
> -		pe_init->p2p_initiator_count++;
> -		phb_target->p2p_target_count++;
> -	} else {
> -		if (!pe_init->p2p_initiator_count ||
> -			!phb_target->p2p_target_count) {
> -			rc = -EINVAL;
> -			goto out;
> -		}
> -
> -		if (--pe_init->p2p_initiator_count == 0)
> -			pnv_pci_ioda2_set_bypass(pe_init, true);
> -
> -		if (--phb_target->p2p_target_count == 0) {
> -			rc = opal_pci_set_p2p(phb_init->opal_id,
> -					      phb_target->opal_id, desc,
> -					      pe_init->pe_number);
> -			if (rc != OPAL_SUCCESS) {
> -				rc = -EIO;
> -				goto out;
> -			}
> -		}
> -	}
> -	rc = 0;
> -out:
> -	mutex_unlock(&p2p_mutex);
> -	return rc;
> -}
> -EXPORT_SYMBOL_GPL(pnv_pci_set_p2p);
> -
>   struct device_node *pnv_pci_get_phb_node(struct pci_dev *dev)
>   {
>   	struct pci_controller *hose = pci_bus_to_host(dev->bus);
> diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
> index 8e36da379252..fbec1c76d7a7 100644
> --- a/arch/powerpc/platforms/powernv/pci.h
> +++ b/arch/powerpc/platforms/powernv/pci.h
> @@ -78,9 +78,6 @@ struct pnv_ioda_pe {
>   	struct pnv_ioda_pe	*master;
>   	struct list_head	slaves;
>   
> -	/* PCI peer-to-peer*/
> -	int			p2p_initiator_count;
> -
>   	/* Link in list of PE#s */
>   	struct list_head	list;
>   };
> @@ -171,8 +168,6 @@ struct pnv_phb {
>   	/* PHB and hub diagnostics */
>   	unsigned int		diag_data_size;
>   	u8			*diag_data;
> -
> -	int p2p_target_count;
>   };
>   
>   extern struct pci_ops pnv_pci_ops;
> 

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Mon, May 06, 2019 at 10:46:11AM +0200, Frederic Barrat wrote:
> Hi,
>
> The PCI p2p and tunnel code is used by the Mellanox CX5 driver, at least 
> their latest, out of tree version, which is used for CORAL. My 
> understanding is that they'll upstream it at some point, though I don't 
> know what their schedule is like.

As said before, we only keep exports in tree for in-tree users.  If the
CX5 driver grows special P2P support it will have to use the proper
existing kernel infrastructure for that anyway.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Mon, May 06, 2019 at 10:46:11AM +0200, Frederic Barrat wrote:
> Hi,
>
> The PCI p2p and tunnel code is used by the Mellanox CX5 driver, at least 
> their latest, out of tree version, which is used for CORAL. My 
> understanding is that they'll upstream it at some point, though I don't 
> know what their schedule is like.

FYI, Max who wrote (at least larger parts of) that code is on Cc agreed
that all P2P code should go through the kernel P2P infrastructure and
might be able to spend some cycles on it.

Which still doesn't change anything about that fact that we [1]
generally don't add infrastructure for anything that is not in the
tree.

[1] well, powernv seems to have handles this a little oddly, and now is
on my special watchlist.

RE: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Max Gurtovoy]

Hi Greg/Christoph,
Can we leave it meanwhile till we'll find a general solution (for the upcoming kernel) ?
I guess we can somehow generalize the P2P initialization process for PPC and leave it empty for now for other archs.
Or maybe we can find some other solution (sysfs/configfs/module param), but it will take time since we'll need to work closely with the IBM pci guys that wrote this code.

-Max.


-----Original Message-----
From: Christoph Hellwig <hch@lst.de> 
Sent: Thursday, May 23, 2019 10:53 AM
To: Frederic Barrat <fbarrat@linux.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>; Benjamin Herrenschmidt <benh@kernel.crashing.org>; Paul Mackerras <paulus@samba.org>; Michael Ellerman <mpe@ellerman.id.au>; linuxppc-dev@lists.ozlabs.org; Max Gurtovoy <maxg@mellanox.com>
Subject: Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

On Mon, May 06, 2019 at 10:46:11AM +0200, Frederic Barrat wrote:
> Hi,
>
> The PCI p2p and tunnel code is used by the Mellanox CX5 driver, at 
> least their latest, out of tree version, which is used for CORAL. My 
> understanding is that they'll upstream it at some point, though I 
> don't know what their schedule is like.

FYI, Max who wrote (at least larger parts of) that code is on Cc agreed that all P2P code should go through the kernel P2P infrastructure and might be able to spend some cycles on it.

Which still doesn't change anything about that fact that we [1] generally don't add infrastructure for anything that is not in the tree.

[1] well, powernv seems to have handles this a little oddly, and now is on my special watchlist.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Tue, Jul 09, 2019 at 01:49:04PM +0000, Max Gurtovoy wrote:
> Hi Greg/Christoph,
> Can we leave it meanwhile till we'll find a general solution (for the upcoming kernel) ?
> I guess we can somehow generalize the P2P initialization process for PPC and leave it empty for now for other archs.
> Or maybe we can find some other solution (sysfs/configfs/module param), but it will take time since we'll need to work closely with the IBM pci guys that wrote this code.

We do not keep code without in-tree users around, especially not if
we have a better API with in-tree users.

AFAICS the only thing you'll need is to wire up the enable/disable
calls.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Max Gurtovoy]

On 7/9/2019 4:59 PM, Christoph Hellwig wrote:
> On Tue, Jul 09, 2019 at 01:49:04PM +0000, Max Gurtovoy wrote:
>> Hi Greg/Christoph,
>> Can we leave it meanwhile till we'll find a general solution (for the upcoming kernel) ?
>> I guess we can somehow generalize the P2P initialization process for PPC and leave it empty for now for other archs.
>> Or maybe we can find some other solution (sysfs/configfs/module param), but it will take time since we'll need to work closely with the IBM pci guys that wrote this code.
> We do not keep code without in-tree users around, especially not if
> we have a better API with in-tree users.
>
> AFAICS the only thing you'll need is to wire up the enable/disable
> calls.

I guess you're right, but we still need to know the time frame we have 
here since this should be tested carefully on the P9 hardware.

Are we ok with working on a solution during kernel-5.3 cycle ?

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Tue, Jul 09, 2019 at 05:31:37PM +0300, Max Gurtovoy wrote:
> Are we ok with working on a solution during kernel-5.3 cycle ?

You can start working on it any time, no need to ask for permission.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Max Gurtovoy]

On 7/9/2019 5:32 PM, Christoph Hellwig wrote:
> On Tue, Jul 09, 2019 at 05:31:37PM +0300, Max Gurtovoy wrote:
>> Are we ok with working on a solution during kernel-5.3 cycle ?
> You can start working on it any time, no need to ask for permission.

I just want to make sure we don't remove it from the kernel before we 
send a general API solution.

This way we'll make sure that all the kernel versions has this 
functionality...

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Tue, Jul 09, 2019 at 05:37:18PM +0300, Max Gurtovoy wrote:
>
> On 7/9/2019 5:32 PM, Christoph Hellwig wrote:
>> On Tue, Jul 09, 2019 at 05:31:37PM +0300, Max Gurtovoy wrote:
>>> Are we ok with working on a solution during kernel-5.3 cycle ?
>> You can start working on it any time, no need to ask for permission.
>
> I just want to make sure we don't remove it from the kernel before we send 
> a general API solution.

The code is gone in this merge window.

> This way we'll make sure that all the kernel versions has this 
> functionality...

Again, we do not provide functionality for out of tree modules.  We've
had the p2p API for about a year now, its not like you didn't have
plenty of time.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Max Gurtovoy]

On 7/9/2019 5:40 PM, Christoph Hellwig wrote:
> On Tue, Jul 09, 2019 at 05:37:18PM +0300, Max Gurtovoy wrote:
>> On 7/9/2019 5:32 PM, Christoph Hellwig wrote:
>>> On Tue, Jul 09, 2019 at 05:31:37PM +0300, Max Gurtovoy wrote:
>>>> Are we ok with working on a solution during kernel-5.3 cycle ?
>>> You can start working on it any time, no need to ask for permission.
>> I just want to make sure we don't remove it from the kernel before we send
>> a general API solution.
> The code is gone in this merge window.

Ok, so we must fix it to kernel-5.3 to make sure we're covered.

Understood.

>
>> This way we'll make sure that all the kernel versions has this
>> functionality...
> Again, we do not provide functionality for out of tree modules.  We've
> had the p2p API for about a year now, its not like you didn't have
> plenty of time.

I didn't know about the intention to remove this code...

Also this code was merged before the p2p API for p2pmem.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

[Christoph Hellwig]

On Tue, Jul 09, 2019 at 06:06:54PM +0300, Max Gurtovoy wrote:
> Also this code was merged before the p2p API for p2pmem.

Yes, without a single user or intention to submit a user, and without
covering the most useful use case (PCIe switches).  While at the same
time the people involved completely ignored the PCIe P2P discussions
that have been going on the PCI list for a long time.

This is a text book example of why code needs to be upstream first
with a broad discussion instead of slipping some crap in for out
of tree drivers.

Re: [PATCH 1/3] powerpc/powernv: remove the unused pnv_pci_set_p2p function

['gregkh@linuxfoundation.org']

On Tue, Jul 09, 2019 at 06:06:54PM +0300, Max Gurtovoy wrote:
> 
> On 7/9/2019 5:40 PM, Christoph Hellwig wrote:
> > On Tue, Jul 09, 2019 at 05:37:18PM +0300, Max Gurtovoy wrote:
> > > On 7/9/2019 5:32 PM, Christoph Hellwig wrote:
> > > > On Tue, Jul 09, 2019 at 05:31:37PM +0300, Max Gurtovoy wrote:
> > > > > Are we ok with working on a solution during kernel-5.3 cycle ?
> > > > You can start working on it any time, no need to ask for permission.
> > > I just want to make sure we don't remove it from the kernel before we send
> > > a general API solution.
> > The code is gone in this merge window.
> 
> Ok, so we must fix it to kernel-5.3 to make sure we're covered.
> 
> Understood.
> 
> > 
> > > This way we'll make sure that all the kernel versions has this
> > > functionality...
> > Again, we do not provide functionality for out of tree modules.  We've
> > had the p2p API for about a year now, its not like you didn't have
> > plenty of time.
> 
> I didn't know about the intention to remove this code...

The original email you responded to in this thread was received by you
back in May.  It is now July, 5.3 will not be out for 8-9 weeks.  There
has been plenty of time here...

greg k-h