Subject: [Very RFC 13/46] powerpc/eeh: Rework how pdev_probe() is used

From: Oliver O'Halloran <[email-masked]>
Date: Wed, 20 Nov 2019 12:28:26 +1100

Adjust how the EEH core uses the eeh_ops->probe_pdev() so that it returns
the eeh_dev for the passed-in pci_dev. Currently mapping an pci_dev to an
eeh_dev is done by finding the pci_dn for the pci_dev, then using the
back-pointer to the eeh_dev stashed in the pci_dn.

We want to move away from using pci_dn on PowerNV and moving the eeh_dev
lookup into probe_pdev() allows the EEH core to be oblivious of how the
mapping is actually done.

Signed-off-by: Oliver O'Halloran <[email-masked]>
---
 arch/powerpc/include/asm/eeh.h               | 16 +++++++--
 arch/powerpc/kernel/eeh.c                    | 34 ++++++++++++--------
 arch/powerpc/platforms/powernv/eeh-powernv.c | 20 +++++++++---
 arch/powerpc/platforms/pseries/eeh_pseries.c | 19 ++++++++++-
 4 files changed, 67 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 466b0165fbcf..e109bfd3dd57 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -215,8 +215,20 @@ enum {
 struct eeh_ops {
 	char *name;
 	int (*init)(void);
-	void (*probe_pdn)(struct pci_dn *pdn);    /* used on pseries */
-	void (*probe_pdev)(struct pci_dev *pdev); /* used on powernv */
+
+	/*
+	 * on pseries the eeh_dev is initialised before the pci_dev exists
+	 * using the contents of the pci_dn.
+	 */
+	void (*probe_pdn)(struct pci_dn *pdn);
+
+	/*
+	 * probe_pdev() is used to find, and possibly create, an eeh_dev
+	 * for a pci_dev. The EEH core binds the returned device to the
+	 * pci_dev.
+	 */
+	struct eeh_dev *(*probe_pdev)(struct pci_dev *pdev);
+
 	int (*set_option)(struct eeh_pe *pe, int option);
 	int (*get_pe_addr)(struct eeh_pe *pe);
 	int (*get_state)(struct eeh_pe *pe, int *delay);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 087a98b42a8c..58a8299ac417 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1099,17 +1099,24 @@ EXPORT_SYMBOL_GPL(eeh_add_device_tree_early);
  */
 void eeh_add_device_late(struct pci_dev *dev)
 {
-	struct pci_dn *pdn;
 	struct eeh_dev *edev;
 
 	if (!dev)
 		return;
 
-	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
-	edev = pdn_to_eeh_dev(pdn);
-	eeh_edev_dbg(edev, "Adding device\n");
-	if (edev->pdev == dev) {
-		eeh_edev_dbg(edev, "Device already referenced!\n");
+	pr_debug("EEH: Adding device %s\n", pci_name(dev));
+
+	/* pci_dev_to_eeh_dev() can only work if archdata.edev is already set */
+	edev = pci_dev_to_eeh_dev(dev);
+	if (edev) {
+		/* FIXME: I don't remember why this isn't an error, but it's not */
+		eeh_edev_dbg(edev, "Already bound to an eeh_dev!\n");
+		return;
+	}
+
+	edev = eeh_ops->probe_pdev(dev);
+	if (!edev) {
+		pr_debug("EEH: Adding device failed\n");
 		return;
 	}
 
@@ -1118,8 +1125,13 @@ void eeh_add_device_late(struct pci_dev *dev)
 	 * unbalanced kref to the device during unplug time, which
 	 * relies on pcibios_release_device(). So we have to remove
 	 * that here explicitly.
+	 *
+	 * FIXME: This really shouldn't be necessary. We should probably
+	 * tear down the EEH state when we detatch the pci_dev from the
+	 * bus. We might need to move the bus notifiers out of the platforms
+	 * first.
 	 */
-	if (edev->pdev) {
+	if (edev->pdev && edev->pdev != dev) {
 		eeh_rmv_from_parent_pe(edev);
 		eeh_addr_cache_rmv_dev(edev->pdev);
 		eeh_sysfs_remove_device(edev->pdev);
@@ -1130,17 +1142,11 @@ void eeh_add_device_late(struct pci_dev *dev)
 		 * into error handler afterwards.
 		 */
 		edev->mode |= EEH_DEV_NO_HANDLER;
-
-		edev->pdev = NULL;
-		dev->dev.archdata.edev = NULL;
 	}
 
-	if (eeh_ops->probe_pdev && eeh_has_flag(EEH_PROBE_MODE_DEV))
-		eeh_ops->probe_pdev(dev);
-
+	/* bind the pdev and the edev together */
 	edev->pdev = dev;
 	dev->dev.archdata.edev = edev;
-
 	eeh_addr_cache_insert_dev(dev);
 	eeh_sysfs_add_device(dev);
 }
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 8bd5317aa878..5250c4525544 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -348,9 +348,9 @@ static int pnv_eeh_find_ecap(struct pci_dn *pdn, int cap)
  * pnv_eeh_probe - Do probe on PCI device
  * @pdev: pci_dev to probe
  *
- * Creates (or finds an existing) edev for this pci_dev.
+ * Create, or find the existing, eeh_dev for this pci_dev.
  */
-static void pnv_eeh_probe_pdev(struct pci_dev *pdev)
+static struct eeh_dev *pnv_eeh_probe_pdev(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 	struct pci_controller *hose = pdn->phb;
@@ -367,11 +367,19 @@ static void pnv_eeh_probe_pdev(struct pci_dev *pdev)
 	 * the probing.
 	 */
 	if (!edev || edev->pe)
-		return;
+		return NULL;
+
+	/* already configured? */
+	if (edev->pdev) {
+		pr_debug("%s: found existing edev for %04x:%02x:%02x.%01x\n",
+			__func__, hose->global_number, config_addr >> 8,
+			PCI_SLOT(config_addr), PCI_FUNC(config_addr));
+		return edev;
+	}
 
 	/* Skip for PCI-ISA bridge */
 	if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_ISA)
-		return;
+		return NULL;
 
 	eeh_edev_dbg(edev, "Probing device\n");
 
@@ -401,7 +409,7 @@ static void pnv_eeh_probe_pdev(struct pci_dev *pdev)
 	ret = eeh_add_to_parent_pe(edev);
 	if (ret) {
 		eeh_edev_warn(edev, "Failed to add device to PE (code %d)\n", ret);
-		return;
+		return NULL;
 	}
 
 	/*
@@ -459,6 +467,8 @@ static void pnv_eeh_probe_pdev(struct pci_dev *pdev)
 	eeh_save_bars(edev);
 
 	eeh_edev_dbg(edev, "EEH enabled on device\n");
+
+	return edev;
 }
 
 /**
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 3ac23c884f4e..13a8c274554a 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -317,6 +317,23 @@ static void pseries_eeh_probe_pdn(struct pci_dn *pdn)
 	eeh_save_bars(edev);
 }
 
+/* Platform specific method to retrieve the eeh_dev for this pci_dev */
+static struct eeh_dev *pseries_eeh_probe_pdev(struct pci_dev *pdev)
+{
+	struct eeh_dev *edev;
+	struct pci_dn *pdn;
+
+	pdn = pci_get_pdn_by_devfn(pdev->bus, pdev->devfn);
+	if (!pdn)
+		return NULL;
+
+	edev = pdn_to_eeh_dev(pdn);
+	if (!edev || !edev->pe)
+		return NULL;
+
+	return edev;
+}
+
 /**
  * pseries_eeh_set_option - Initialize EEH or MMIO/DMA reenable
  * @pe: EEH PE
@@ -754,7 +771,7 @@ static struct eeh_ops pseries_eeh_ops = {
 	.name			= "pseries",
 	.init			= pseries_eeh_init,
 	.probe_pdn		= pseries_eeh_probe_pdn,
-	.probe_pdev 		= NULL,
+	.probe_pdev 		= pseries_eeh_probe_pdev,
 	.set_option		= pseries_eeh_set_option,
 	.get_pe_addr		= pseries_eeh_get_pe_addr,
 	.get_state		= pseries_eeh_get_state,
-- 
2.21.0