[PATCH v1 0/8] EEH Followup Fixes (II)

[PATCH v1 0/8] EEH Followup Fixes (II)

[Gavin Shan]

The series of patches bases on linux-poerpc-next and intends to resolve
the following problems:
 
	- On pSeries platform, the EEH doesn't work after PHB hotplug
	  with "drmgr". The root cause is that the EEH resources (
	  EEH devices, EEH caches) aren't released correctly. For the
	  problem, we add one hook (pcibios_stop_dev), which is called
	  on pci_stop_and_remove_device(). In pcibios_stop_dev(), we
	  release the EEH resources.
	- Another issue is that we need put the domain (PE or PHB) into
	  quite state while doing reset on that domain. However, some
	  deivces in the domain might not have EEH sensitive drivers, or
	  even don't have driver. Those deivces can't be put into quite
	  state and possibly keep issuing PCI-CFG or MMIO request during
	  resetting the domain. That possibly causes the failure of reset
	  and eventually failure of EEH recovery. For the issue, we introduces
	  so-called "partial hotplug". That means, those devices without driver or
	  without EEH sensitive driver are removed before doing reset, and
	  plugged (probed) into the system after reset.
	- We need traverse EEH devices of one specific PE with safe variant
	  of list tranverse function. The EEH device might be removed while
	  doing iteration.
	- When doing plug for PCI bus, we need check if we need reassign the
	  resources for subordinate devices (PCI_REASSIGN_ALL_RSRC) and do that
	  accordingly.

The patchset is verified on pSeires and PowerNV platforms:

pSeries Platform
-----------------

drmgr -c phb -r -s "PHB 513"
drmgr -c phb -a -s "PHB 513"
errinjct eeh -f 1 -s net/eth2

PowerNV Platform
-----------------

cd /sys/devices/pci0005:00/0005:00:00.0/0005:01:00.0/0005:02:08.0/0005:80:00.0/0005:90:01.0
while true; do od -x config > /dev/null; sleep 1; done
echo 1 > /sys/kernel/debug/powerpc/PCI0005/err_injct

---

arch/powerpc/include/asm/eeh.h        |   24 +++++--
arch/powerpc/include/asm/pci-bridge.h |    3 +-
arch/powerpc/include/asm/pci.h        |    2 +
arch/powerpc/kernel/eeh.c             |   56 ++++++---------
arch/powerpc/kernel/eeh_driver.c      |  106 ++++++++++++++++++++++++++-
arch/powerpc/kernel/eeh_pe.c          |   43 ++++++-----
arch/powerpc/kernel/pci-common.c      |    8 ++-
arch/powerpc/kernel/pci-hotplug.c     |  129 +++++++++++++++++++++++++++------
arch/powerpc/kernel/pci_of_scan.c     |   43 ++++++++---
drivers/pci/hotplug/rpadlpar_core.c   |    1 -
drivers/pci/probe.c                   |    4 +
drivers/pci/remove.c                  |    2 +
include/linux/pci.h                   |    1 +
13 files changed, 322 insertions(+), 100 deletions(-)

Thanks,
Gavin

[PATCH 1/8] PCI: Add pcibios_stop_dev()

[Gavin Shan]

When stopping and removing one specific PCI device, the platform
might need take some actions. One example is that EEH already had
eeh cache and eeh device attached to the PCI device, and we need
release eeh cache and device during the time. The patch introduces
hook pcibios_stop_dev() for the purpose.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 drivers/pci/probe.c  |    4 ++++
 drivers/pci/remove.c |    2 ++
 include/linux/pci.h  |    1 +
 3 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 70f10fa..7167dc4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1669,6 +1669,10 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
 {
 }
 
+void __weak pcibios_stop_dev(struct pci_dev *dev)
+{
+}
+
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 		struct pci_ops *ops, void *sysdata, struct list_head *resources)
 {
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 8fc54b7..e329efc 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -21,6 +21,8 @@ static void pci_stop_dev(struct pci_dev *dev)
 {
 	pci_pme_active(dev, false);
 
+	pcibios_stop_dev(dev);
+
 	if (dev->is_added) {
 		pci_proc_detach_device(dev);
 		pci_remove_sysfs_dev_files(dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3a24e4f..40df783 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -696,6 +696,7 @@ int no_pci_devices(void);
 void pcibios_resource_survey_bus(struct pci_bus *bus);
 void pcibios_add_bus(struct pci_bus *bus);
 void pcibios_remove_bus(struct pci_bus *bus);
+void pcibios_stop_dev(struct pci_dev *dev);
 void pcibios_fixup_bus(struct pci_bus *);
 int __must_check pcibios_enable_device(struct pci_dev *, int mask);
 /* Architecture specific versions may override this (weak) */
-- 
1.7.5.4

[PATCH 2/8] powerpc/eeh: Export functions for hotplug

[Gavin Shan]

Make some functions public in order to support hotplug on either specific
PCI bus or PCI device in future.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h |    9 +++++++++
 arch/powerpc/kernel/eeh.c      |    6 +++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 09a8743..d9d35c2 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -209,9 +209,12 @@ unsigned long eeh_check_failure(const volatile void __iomem *token,
 				unsigned long val);
 int eeh_dev_check_failure(struct eeh_dev *edev);
 void eeh_addr_cache_build(void);
+void eeh_add_device_early(struct device_node *);
 void eeh_add_device_tree_early(struct device_node *);
+void eeh_add_device_late(struct pci_dev *);
 void eeh_add_device_tree_late(struct pci_bus *);
 void eeh_add_sysfs_files(struct pci_bus *);
+void eeh_remove_device(struct pci_dev *, int);
 void eeh_remove_bus_device(struct pci_dev *, int);
 
 /**
@@ -252,12 +255,18 @@ static inline unsigned long eeh_check_failure(const volatile void __iomem *token
 
 static inline void eeh_addr_cache_build(void) { }
 
+static inline void eeh_add_device_early(struct device_node *dn) { }
+
 static inline void eeh_add_device_tree_early(struct device_node *dn) { }
 
+static inline void eeh_add_device_late(struct pci_dev *dev) { }
+
 static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
 
 static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
 
+static inline void eeh_remove_device(struct pci_dev *dev, int purge_pe) { }
+
 static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
 
 #define EEH_POSSIBLE_ERROR(val, type) (0)
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 39954fe..a186de8 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -838,7 +838,7 @@ core_initcall_sync(eeh_init);
  * on the CEC architecture, type of the device, on earlier boot
  * command-line arguments & etc.
  */
-static void eeh_add_device_early(struct device_node *dn)
+void eeh_add_device_early(struct device_node *dn)
 {
 	struct pci_controller *phb;
 
@@ -886,7 +886,7 @@ EXPORT_SYMBOL_GPL(eeh_add_device_tree_early);
  * This routine must be used to complete EEH initialization for PCI
  * devices that were added after system boot (e.g. hotplug, dlpar).
  */
-static void eeh_add_device_late(struct pci_dev *dev)
+void eeh_add_device_late(struct pci_dev *dev)
 {
 	struct device_node *dn;
 	struct eeh_dev *edev;
@@ -975,7 +975,7 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
  * this device will no longer be detected after this call; thus,
  * i/o errors affecting this slot may leave this device unusable.
  */
-static void eeh_remove_device(struct pci_dev *dev, int purge_pe)
+void eeh_remove_device(struct pci_dev *dev, int purge_pe)
 {
 	struct eeh_dev *edev;
 
-- 
1.7.5.4

[PATCH 3/8] powerpc/pci: Override pcibios_stop_dev()

[Gavin Shan]

The patch overrides the weak function pcibios_stop_dev() to destroy
the EEH device and cache while stopping and removing the corresponding
PCI device.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/pci-hotplug.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 3f60880..617d2df 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -22,6 +22,18 @@
 #include <asm/eeh.h>
 
 /**
+ * pcibios_stop_dev - stop the PCI device
+ * @dev: the indicated PCI device
+ *
+ * Stop the PCI device. The function should be called before
+ * stopping the specified PCI device.
+ */
+void pcibios_stop_dev(struct pci_dev *dev)
+{
+	eeh_remove_device(dev, 1);
+}
+
+/**
  * __pcibios_remove_pci_devices - remove all devices under this bus
  * @bus: the indicated PCI bus
  * @purge_pe: destroy the PE on removal of PCI devices
@@ -45,7 +57,6 @@ void __pcibios_remove_pci_devices(struct pci_bus *bus, int purge_pe)
 		 pci_domain_nr(bus),  bus->number);
 	list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
 		pr_debug("     * Removing %s...\n", pci_name(dev));
-		eeh_remove_bus_device(dev, purge_pe);
 		pci_stop_and_remove_bus_device(dev);
 	}
 }
-- 
1.7.5.4

[PATCH 4/8] PCI/hotplug: Needn't remove EEH cache again

[Gavin Shan]

Since pci_stop_and_remove_bus_device() has removed the EEH cache,
we needn't do that again.

Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 drivers/pci/hotplug/rpadlpar_core.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
index b29e20b..bb7af78 100644
--- a/drivers/pci/hotplug/rpadlpar_core.c
+++ b/drivers/pci/hotplug/rpadlpar_core.c
@@ -388,7 +388,6 @@ int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn)
 	/* Remove the EADS bridge device itself */
 	BUG_ON(!bus->self);
 	pr_debug("PCI: Now removing bridge device %s\n", pci_name(bus->self));
-	eeh_remove_bus_device(bus->self, true);
 	pci_stop_and_remove_bus_device(bus->self);
 
 	return 0;
-- 
1.7.5.4

[PATCH 5/8] powerpc/eeh: Keep PE during hotplug

[Gavin Shan]

When we do normal hotplug, the PE shouldn't be kept. However, we
need the PE if the hotplug caused by EEH errors. Since we remove
EEH device through the PCI hook pcibios_stop_dev(), the flag
"purge_pe" passed to various functions is meaningless. So the patch
removes the meaningless flag and introduce new flag "EEH_PE_KEEP"
to save the PE while doing hotplug during EEH error recovery.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h        |   11 +++++------
 arch/powerpc/include/asm/pci-bridge.h |    1 -
 arch/powerpc/kernel/eeh.c             |   28 ++--------------------------
 arch/powerpc/kernel/eeh_driver.c      |    7 +++++--
 arch/powerpc/kernel/eeh_pe.c          |    7 +++----
 arch/powerpc/kernel/pci-hotplug.c     |   26 +++++---------------------
 6 files changed, 20 insertions(+), 60 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index d9d35c2..2ce22d7 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -55,6 +55,8 @@ struct device_node;
 #define EEH_PE_RECOVERING	(1 << 1)	/* Recovering PE	*/
 #define EEH_PE_PHB_DEAD		(1 << 2)	/* Dead PHB		*/
 
+#define EEH_PE_KEEP		(1 << 8)	/* Keep PE on hotplug	*/
+
 struct eeh_pe {
 	int type;			/* PE type: PHB/Bus/Device	*/
 	int state;			/* PE EEH dependent mode	*/
@@ -193,7 +195,7 @@ int eeh_phb_pe_create(struct pci_controller *phb);
 struct eeh_pe *eeh_phb_pe_get(struct pci_controller *phb);
 struct eeh_pe *eeh_pe_get(struct eeh_dev *edev);
 int eeh_add_to_parent_pe(struct eeh_dev *edev);
-int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe);
+int eeh_rmv_from_parent_pe(struct eeh_dev *edev);
 void eeh_pe_update_time_stamp(struct eeh_pe *pe);
 void *eeh_pe_dev_traverse(struct eeh_pe *root,
 		eeh_traverse_func fn, void *flag);
@@ -214,8 +216,7 @@ void eeh_add_device_tree_early(struct device_node *);
 void eeh_add_device_late(struct pci_dev *);
 void eeh_add_device_tree_late(struct pci_bus *);
 void eeh_add_sysfs_files(struct pci_bus *);
-void eeh_remove_device(struct pci_dev *, int);
-void eeh_remove_bus_device(struct pci_dev *, int);
+void eeh_remove_device(struct pci_dev *);
 
 /**
  * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.
@@ -265,9 +266,7 @@ static inline void eeh_add_device_tree_late(struct pci_bus *bus) { }
 
 static inline void eeh_add_sysfs_files(struct pci_bus *bus) { }
 
-static inline void eeh_remove_device(struct pci_dev *dev, int purge_pe) { }
-
-static inline void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe) { }
+static inline void eeh_remove_device(struct pci_dev *dev) { }
 
 #define EEH_POSSIBLE_ERROR(val, type) (0)
 #define EEH_IO_ERROR_VALUE(size) (-1UL)
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 2c1d8cb..32d0d20 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -209,7 +209,6 @@ static inline struct eeh_dev *of_node_to_eeh_dev(struct device_node *dn)
 extern struct pci_bus *pcibios_find_pci_bus(struct device_node *dn);
 
 /** Remove all of the PCI devices under this bus */
-extern void __pcibios_remove_pci_devices(struct pci_bus *bus, int purge_pe);
 extern void pcibios_remove_pci_devices(struct pci_bus *bus);
 
 /** Discover new pci devices under this bus, and add them */
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index a186de8..b074b2a 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -967,7 +967,6 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
 /**
  * eeh_remove_device - Undo EEH setup for the indicated pci device
  * @dev: pci device to be removed
- * @purge_pe: remove the PE or not
  *
  * This routine should be called when a device is removed from
  * a running system (e.g. by hotplug or dlpar).  It unregisters
@@ -975,7 +974,7 @@ EXPORT_SYMBOL_GPL(eeh_add_sysfs_files);
  * this device will no longer be detected after this call; thus,
  * i/o errors affecting this slot may leave this device unusable.
  */
-void eeh_remove_device(struct pci_dev *dev, int purge_pe)
+void eeh_remove_device(struct pci_dev *dev)
 {
 	struct eeh_dev *edev;
 
@@ -994,34 +993,11 @@ void eeh_remove_device(struct pci_dev *dev, int purge_pe)
 	dev->dev.archdata.edev = NULL;
 	pci_dev_put(dev);
 
-	eeh_rmv_from_parent_pe(edev, purge_pe);
+	eeh_rmv_from_parent_pe(edev);
 	eeh_addr_cache_rmv_dev(dev);
 	eeh_sysfs_remove_device(dev);
 }
 
-/**
- * eeh_remove_bus_device - Undo EEH setup for the indicated PCI device
- * @dev: PCI device
- * @purge_pe: remove the corresponding PE or not
- *
- * This routine must be called when a device is removed from the
- * running system through hotplug or dlpar. The corresponding
- * PCI address cache will be removed.
- */
-void eeh_remove_bus_device(struct pci_dev *dev, int purge_pe)
-{
-	struct pci_bus *bus = dev->subordinate;
-	struct pci_dev *child, *tmp;
-
-	eeh_remove_device(dev, purge_pe);
-
-	if (bus && dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
-		list_for_each_entry_safe(child, tmp, &bus->devices, bus_list)
-			 eeh_remove_bus_device(child, purge_pe);
-	}
-}
-EXPORT_SYMBOL_GPL(eeh_remove_bus_device);
-
 static int proc_eeh_show(struct seq_file *m, void *v)
 {
 	if (0 == eeh_subsystem_enabled) {
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 2b1ce17..9ef3bbb 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -362,8 +362,10 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 	 * devices are expected to be attached soon when calling
 	 * into pcibios_add_pci_devices().
 	 */
-	if (bus)
-		__pcibios_remove_pci_devices(bus, 0);
+	if (bus) {
+		eeh_pe_state_mark(pe, EEH_PE_KEEP);
+		pcibios_remove_pci_devices(bus);
+	}
 
 	/* Reset the pci controller. (Asserts RST#; resets config space).
 	 * Reconfigure bridges and devices. Don't try to bring the system
@@ -386,6 +388,7 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 	if (bus) {
 		ssleep(5);
 		pcibios_add_pci_devices(bus);
+		eeh_pe_state_clear(pe, EEH_PE_KEEP);
 	}
 
 	pe->tstamp = tstamp;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 016588a..32ef409 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -333,7 +333,7 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
 		while (parent) {
 			if (!(parent->type & EEH_PE_INVALID))
 				break;
-			parent->type &= ~EEH_PE_INVALID;
+			parent->type &= ~(EEH_PE_INVALID | EEH_PE_KEEP);
 			parent = parent->parent;
 		}
 		pr_debug("EEH: Add %s to Device PE#%x, Parent PE#%x\n",
@@ -397,14 +397,13 @@ int eeh_add_to_parent_pe(struct eeh_dev *edev)
 /**
  * eeh_rmv_from_parent_pe - Remove one EEH device from the associated PE
  * @edev: EEH device
- * @purge_pe: remove PE or not
  *
  * The PE hierarchy tree might be changed when doing PCI hotplug.
  * Also, the PCI devices or buses could be removed from the system
  * during EEH recovery. So we have to call the function remove the
  * corresponding PE accordingly if necessary.
  */
-int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe)
+int eeh_rmv_from_parent_pe(struct eeh_dev *edev)
 {
 	struct eeh_pe *pe, *parent, *child;
 	int cnt;
@@ -431,7 +430,7 @@ int eeh_rmv_from_parent_pe(struct eeh_dev *edev, int purge_pe)
 		if (pe->type & EEH_PE_PHB)
 			break;
 
-		if (purge_pe) {
+		if (!(pe->state & EEH_PE_KEEP)) {
 			if (list_empty(&pe->edevs) &&
 			    list_empty(&pe->child_list)) {
 				list_del(&pe->child);
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 617d2df..96c9ab8 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -30,48 +30,32 @@
  */
 void pcibios_stop_dev(struct pci_dev *dev)
 {
-	eeh_remove_device(dev, 1);
+	eeh_remove_device(dev);
 }
 
 /**
- * __pcibios_remove_pci_devices - remove all devices under this bus
+ * pcibios_remove_pci_devices - remove all devices under this bus
  * @bus: the indicated PCI bus
- * @purge_pe: destroy the PE on removal of PCI devices
  *
  * Remove all of the PCI devices under this bus both from the
  * linux pci device tree, and from the powerpc EEH address cache.
- * By default, the corresponding PE will be destroied during the
- * normal PCI hotplug path. For PCI hotplug during EEH recovery,
- * the corresponding PE won't be destroied and deallocated.
  */
-void __pcibios_remove_pci_devices(struct pci_bus *bus, int purge_pe)
+void pcibios_remove_pci_devices(struct pci_bus *bus)
 {
 	struct pci_dev *dev, *tmp;
 	struct pci_bus *child_bus;
 
 	/* First go down child busses */
 	list_for_each_entry(child_bus, &bus->children, node)
-		__pcibios_remove_pci_devices(child_bus, purge_pe);
+		pcibios_remove_pci_devices(child_bus);
 
 	pr_debug("PCI: Removing devices on bus %04x:%02x\n",
 		 pci_domain_nr(bus),  bus->number);
 	list_for_each_entry_safe(dev, tmp, &bus->devices, bus_list) {
-		pr_debug("     * Removing %s...\n", pci_name(dev));
+		pr_debug("   Removing %s...\n", pci_name(dev));
 		pci_stop_and_remove_bus_device(dev);
 	}
 }
-
-/**
- * pcibios_remove_pci_devices - remove all devices under this bus
- * @bus: the indicated PCI bus
- *
- * Remove all of the PCI devices under this bus both from the
- * linux pci device tree, and from the powerpc EEH address cache.
- */
-void pcibios_remove_pci_devices(struct pci_bus *bus)
-{
-	__pcibios_remove_pci_devices(bus, 1);
-}
 EXPORT_SYMBOL_GPL(pcibios_remove_pci_devices);
 
 /**
-- 
1.7.5.4

[PATCH 6/8] powerpc/eeh: Tranverse EEH devices with safe mode

[Gavin Shan]

Currently, we're transversing EEH devices by list_for_each_entry().
That's not safe enough because the EEH devices might be removed from
its parent PE while doing iteration. The patch replaces that with
list_for_each_entry_safe().

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h |    4 ++--
 arch/powerpc/kernel/eeh.c      |    4 ++--
 arch/powerpc/kernel/eeh_pe.c   |   10 +++++-----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 2ce22d7..e8c411b 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -74,8 +74,8 @@ struct eeh_pe {
 	struct list_head child;		/* Child PEs			*/
 };
 
-#define eeh_pe_for_each_dev(pe, edev) \
-		list_for_each_entry(edev, &pe->edevs, list)
+#define eeh_pe_for_each_dev(pe, edev, tmp) \
+		list_for_each_entry_safe(edev, tmp, &pe->edevs, list)
 
 /*
  * The struct is used to trace EEH state for the associated
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index b074b2a..b518c49 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -231,7 +231,7 @@ static size_t eeh_gather_pci_data(struct eeh_dev *edev, char * buf, size_t len)
 void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
 {
 	size_t loglen = 0;
-	struct eeh_dev *edev;
+	struct eeh_dev *edev, *tmp;
 	bool valid_cfg_log = true;
 
 	/*
@@ -251,7 +251,7 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
 		eeh_pe_restore_bars(pe);
 
 		pci_regs_buf[0] = 0;
-		eeh_pe_for_each_dev(pe, edev) {
+		eeh_pe_for_each_dev(pe, edev, tmp) {
 			loglen += eeh_gather_pci_data(edev, pci_regs_buf + loglen,
 						      EEH_PCI_REGS_LOG_LEN - loglen);
 		}
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 32ef409..c8b815e 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -176,7 +176,7 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
 		eeh_traverse_func fn, void *flag)
 {
 	struct eeh_pe *pe;
-	struct eeh_dev *edev;
+	struct eeh_dev *edev, *tmp;
 	void *ret;
 
 	if (!root) {
@@ -186,7 +186,7 @@ void *eeh_pe_dev_traverse(struct eeh_pe *root,
 
 	/* Traverse root PE */
 	for (pe = root; pe; pe = eeh_pe_next(pe, root)) {
-		eeh_pe_for_each_dev(pe, edev) {
+		eeh_pe_for_each_dev(pe, edev, tmp) {
 			ret = fn(edev, flag);
 			if (ret)
 				return ret;
@@ -501,7 +501,7 @@ static void *__eeh_pe_state_mark(void *data, void *flag)
 {
 	struct eeh_pe *pe = (struct eeh_pe *)data;
 	int state = *((int *)flag);
-	struct eeh_dev *tmp;
+	struct eeh_dev *edev, *tmp;
 	struct pci_dev *pdev;
 
 	/*
@@ -511,8 +511,8 @@ static void *__eeh_pe_state_mark(void *data, void *flag)
 	 * the PCI device driver.
 	 */
 	pe->state |= state;
-	eeh_pe_for_each_dev(pe, tmp) {
-		pdev = eeh_dev_to_pci_dev(tmp);
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		pdev = eeh_dev_to_pci_dev(edev);
 		if (pdev)
 			pdev->error_state = pci_channel_io_frozen;
 	}
-- 
1.7.5.4

[PATCH 7/8] powerpc/pci: Partial hotplug support

[Gavin Shan]

When EEH error happens to one specific PE, the device drivers
of its attached EEH devices (PCI devices) are checked to see
the further action: reset with complete hotplug, or reset without
hotplug. However, that's not enough for those PCI devices whose
drivers can't support EEH, or those PCI devices without driver.
So we need do so-called "partial hotplug" on basis of PCI devices.
In the situation, part of PCI devices of the specific PE are
unplugged and plugged again after PE reset.

The patch adds functions to support scanning signle PCI device
(function) either based on device-tree or hardware for plugging.
The existing function pci_stop_and_remove_bus_device() is enough
for unplugging. Besides, the patch also fixes the issue that we
need reassign the resources if we had flag PCI_REASSIGN_ALL_RSRC.
Otherwise, to claim the resources of attached devices of the PCI
bus should fail and the newly added devices in "complete" hotplug
can't be enabled.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pci-bridge.h |    2 +-
 arch/powerpc/include/asm/pci.h        |    2 +
 arch/powerpc/kernel/pci-common.c      |    8 ++-
 arch/powerpc/kernel/pci-hotplug.c     |   92 +++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/pci_of_scan.c     |   43 +++++++++++----
 5 files changed, 132 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 32d0d20..070aed3 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -213,7 +213,7 @@ extern void pcibios_remove_pci_devices(struct pci_bus *bus);
 
 /** Discover new pci devices under this bus, and add them */
 extern void pcibios_add_pci_devices(struct pci_bus *bus);
-
+void pcibios_scan_pci_dev(struct pci_bus *bus, struct device_node *dn);
 
 extern void isa_bridge_find_early(struct pci_controller *hose);
 
diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 6653f27..28cfc95 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h
@@ -167,6 +167,8 @@ extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
 extern void of_scan_pci_bridge(struct pci_dev *dev);
+extern struct pci_dev *of_scan_pci_dev(struct pci_bus *bus,
+				       struct device_node *dn);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
 extern void of_rescan_bus(struct device_node *node, struct pci_bus *bus);
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index f46914a..6f3a1cb 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1460,8 +1460,12 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
 		 pci_domain_nr(bus), bus->number);
 
 	/* Allocate bus and devices resources */
-	pcibios_allocate_bus_resources(bus);
-	pcibios_claim_one_bus(bus);
+	if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
+		pci_assign_unassigned_bus_resources(bus);
+	} else {
+		pcibios_allocate_bus_resources(bus);
+		pcibios_claim_one_bus(bus);
+	}
 
 	/* Fixup EEH */
 	eeh_add_device_tree_late(bus);
diff --git a/arch/powerpc/kernel/pci-hotplug.c b/arch/powerpc/kernel/pci-hotplug.c
index 96c9ab8..bd21c40 100644
--- a/arch/powerpc/kernel/pci-hotplug.c
+++ b/arch/powerpc/kernel/pci-hotplug.c
@@ -104,3 +104,95 @@ void pcibios_add_pci_devices(struct pci_bus * bus)
 	pcibios_finish_adding_to_bus(bus);
 }
 EXPORT_SYMBOL_GPL(pcibios_add_pci_devices);
+
+static void pcibios_of_scan_dev(struct pci_bus *bus, struct device_node *dn)
+{
+	struct pci_dev *dev;
+	int ret;
+
+	dev = of_scan_pci_dev(bus, dn);
+	if (!dev)
+		return;
+
+	eeh_add_device_early(dn);
+	pcibios_add_device(dev);
+	eeh_add_device_late(dev);
+
+	ret = pci_bus_add_device(dev);
+	if (ret) {
+		pr_info("%s: Failed to add PCI dev %s\n",
+			__func__, pci_name(dev));
+		return;
+	}
+
+	eeh_sysfs_add_device(dev);
+}
+
+static void pcibios_scan_dev(struct pci_bus *bus, struct device_node *dn)
+{
+	struct pci_dn *pdn = PCI_DN(dn);
+	struct pci_dev *dev;
+	struct resource *r;
+	int i, ret;
+
+	eeh_add_device_early(dn);
+	dev = pci_scan_single_device(bus, pdn->devfn);
+	if (!dev) {
+		pr_warn("%s: Failed to probe %04x:%02x:%2x.%01x\n",
+			__func__, pci_domain_nr(bus), bus->number,
+			PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+		return;
+	}
+
+	/*
+	 * If we already requested to reassign resources, the
+	 * start address of individual resources is zero'ed
+	 * during PCI header fixup time. So we need reassign
+	 * the resource for the case. Otherwise, it's enough
+	 * to claim it.
+	 */
+	pcibios_add_device(dev);
+	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+		r = &dev->resource[i];
+		if (r->parent || !r->flags)
+			continue;
+		if (pci_has_flag(PCI_REASSIGN_ALL_RSRC)) {
+			ret = pci_assign_resource(dev, i);
+		} else {
+			if (!r->start)
+				continue;
+			ret = pci_claim_resource(dev, i);
+		}
+
+		if (ret) {
+			pr_warn("%s: Can't assign %pR for %s\n",
+				__func__, r, pci_name(dev));
+			/* Clear it out */
+			r->start = 0;
+			r->end = 0;
+			r->flags = 0;
+		}
+	}
+
+	eeh_add_device_late(dev);
+	ret = pci_bus_add_device(dev);
+	if (ret) {
+		pr_warn("%s: Failed to add PCI device %s\n",
+			__func__, pci_name(dev));
+		return;
+	}
+	eeh_sysfs_add_device(dev);
+}
+
+void pcibios_scan_pci_dev(struct pci_bus *bus, struct device_node *dn)
+{
+	int mode = PCI_PROBE_NORMAL;
+
+	if (ppc_md.pci_probe_mode)
+		mode = ppc_md.pci_probe_mode(bus);
+
+	if (mode == PCI_PROBE_DEVTREE)
+		pcibios_of_scan_dev(bus, dn);
+	else if (mode == PCI_PROBE_NORMAL)
+		pcibios_scan_dev(bus, dn);
+}
diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 2a67e9b..a06f0db 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c
@@ -294,6 +294,36 @@ void of_scan_pci_bridge(struct pci_dev *dev)
 EXPORT_SYMBOL(of_scan_pci_bridge);
 
 /**
+ * of_scan_pci_dev - given a PCI device node, setup the PCI device
+ * @bus: PCI bus
+ * @dn: device tree node for the PCI device
+ */
+struct pci_dev *of_scan_pci_dev(struct pci_bus *bus,
+			    struct device_node *dn)
+{
+	struct pci_dev *dev = NULL;
+	const u32 *reg;
+	int reglen, devfn;
+
+	pr_debug("  * %s\n", dn->full_name);
+	if (!of_device_is_available(dn))
+		return NULL;
+
+	reg = of_get_property(dn, "reg", &reglen);
+	if (reg == NULL || reglen < 20)
+		return NULL;
+	devfn = (reg[0] >> 8) & 0xff;
+
+	/* create a new pci_dev for this device */
+	dev = of_create_pci_dev(dn, bus, devfn);
+	if (!dev)
+		return NULL;
+
+	pr_debug("  dev header type: %x\n", dev->hdr_type);
+	return dev;
+}
+
+/**
  * __of_scan_bus - given a PCI bus node, setup bus and scan for child devices
  * @node: device tree node for the PCI bus
  * @bus: pci_bus structure for the PCI bus
@@ -303,8 +333,6 @@ static void __of_scan_bus(struct device_node *node, struct pci_bus *bus,
 			  int rescan_existing)
 {
 	struct device_node *child;
-	const u32 *reg;
-	int reglen, devfn;
 	struct pci_dev *dev;
 
 	pr_debug("of_scan_bus(%s) bus no %d...\n",
@@ -312,16 +340,7 @@ static void __of_scan_bus(struct device_node *node, struct pci_bus *bus,
 
 	/* Scan direct children */
 	for_each_child_of_node(node, child) {
-		pr_debug("  * %s\n", child->full_name);
-		if (!of_device_is_available(child))
-			continue;
-		reg = of_get_property(child, "reg", &reglen);
-		if (reg == NULL || reglen < 20)
-			continue;
-		devfn = (reg[0] >> 8) & 0xff;
-
-		/* create a new pci_dev for this device */
-		dev = of_create_pci_dev(child, bus, devfn);
+		dev = of_scan_pci_dev(bus, child);
 		if (!dev)
 			continue;
 		pr_debug("    dev header type: %x\n", dev->hdr_type);
-- 
1.7.5.4

[PATCH 8/8] powerpc/eeh: Support partial hotplug

[Gavin Shan]

When EEH error happens to one specific PE, some devices with drivers
supporting EEH won't except hotplug on the deivce. However, there
might have other deivces without driver, or with driver without EEH
support. For the case, we need do partial hotplug in order to make
sure that the PE becomes absolutely quite during reset. Otherise,
the PE reset might fail and leads to failure of error recovery.

The patch intends to support so-called "partial" hotplug for EEH:
Before we do reset, we stop and remove those PCI devices without
EEH sensitive driver. The corresponding EEH devices are not detached
from its PE, but with special flag. After the reset is done, those
EEH devices with the special flag will be scanned one by one.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h   |    6 ++-
 arch/powerpc/kernel/eeh.c        |   22 ++++++--
 arch/powerpc/kernel/eeh_driver.c |  109 ++++++++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/eeh_pe.c     |   26 +++++----
 4 files changed, 141 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index e8c411b..f54a601 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -84,7 +84,8 @@ struct eeh_pe {
  * another tree except the currently existing tree of PCI
  * buses and PCI devices
  */
-#define EEH_DEV_IRQ_DISABLED	(1<<0)	/* Interrupt disabled		*/
+#define EEH_DEV_IRQ_DISABLED	(1 << 0)	/* Interrupt disabled	*/
+#define EEH_DEV_DISCONNECTED	(1 << 1)	/* Removing from PE	*/
 
 struct eeh_dev {
 	int mode;			/* EEH mode			*/
@@ -97,6 +98,7 @@ struct eeh_dev {
 	struct pci_controller *phb;	/* Associated PHB		*/
 	struct device_node *dn;		/* Associated device node	*/
 	struct pci_dev *pdev;		/* Associated PCI device	*/
+	struct pci_bus *bus;		/* PCI bus for partial hotplug	*/
 };
 
 static inline struct device_node *eeh_dev_to_of_node(struct eeh_dev *edev)
@@ -197,6 +199,8 @@ struct eeh_pe *eeh_pe_get(struct eeh_dev *edev);
 int eeh_add_to_parent_pe(struct eeh_dev *edev);
 int eeh_rmv_from_parent_pe(struct eeh_dev *edev);
 void eeh_pe_update_time_stamp(struct eeh_pe *pe);
+void *eeh_pe_traverse(struct eeh_pe *root,
+		eeh_traverse_func fn, void *flag);
 void *eeh_pe_dev_traverse(struct eeh_pe *root,
 		eeh_traverse_func fn, void *flag);
 void eeh_pe_restore_bars(struct eeh_pe *pe);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index b518c49..8b414b3 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -985,15 +985,27 @@ void eeh_remove_device(struct pci_dev *dev)
 	/* Unregister the device with the EEH/PCI address search system */
 	pr_debug("EEH: Removing device %s\n", pci_name(dev));
 
-	if (!edev || !edev->pdev) {
+	if (!edev || !edev->pdev || !edev->pe) {
 		pr_debug("EEH: Not referenced !\n");
 		return;
 	}
-	edev->pdev = NULL;
-	dev->dev.archdata.edev = NULL;
-	pci_dev_put(dev);
 
-	eeh_rmv_from_parent_pe(edev);
+	/*
+	 * During the hotplug for EEH error recovery, we need the EEH
+	 * device attached to the parent PE in order for BAR restore
+	 * a bit later. So we keep it for BAR restore and remove it
+	 * from the parent PE during the BAR resotre.
+	 */
+	if (!(edev->pe->state & EEH_PE_KEEP)) {
+		edev->pdev = NULL;
+		dev->dev.archdata.edev = NULL;
+		pci_dev_put(dev);
+
+		eeh_rmv_from_parent_pe(edev);
+	} else {
+		edev->mode |= EEH_DEV_DISCONNECTED;
+	}
+
 	eeh_addr_cache_rmv_dev(dev);
 	eeh_sysfs_remove_device(dev);
 }
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 9ef3bbb..807d2bb 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -338,6 +338,92 @@ static void *eeh_report_failure(void *data, void *userdata)
 	return NULL;
 }
 
+static void *eeh_rmv_device(void *data, void *userdata)
+{
+	struct pci_driver *driver;
+	struct eeh_dev *edev = (struct eeh_dev *)data;
+	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
+	int *removed = (int *)userdata;
+
+	/*
+	 * Actually, we should remove the PCI bridges as well.
+	 * However, that's lots of complexity to do that,
+	 * particularly some of devices under the bridge might
+	 * support EEH. So we just care about PCI devices for
+	 * simplicity here.
+	 */
+	if (!dev || (dev->hdr_type & PCI_HEADER_TYPE_BRIDGE))
+		return NULL;
+	driver = eeh_pcid_get(dev);
+	if (driver && driver->err_handler)
+		return NULL;
+
+	/* Remove it from PCI subsystem */
+	pr_debug("EEH: Removing %s without EEH sensitive driver\n",
+		 pci_name(dev));
+	edev->bus = dev->bus;
+	edev->mode |= EEH_DEV_DISCONNECTED;
+	(*removed)++;
+
+	pci_stop_and_remove_bus_device(dev);
+
+	return NULL;
+}
+
+static void *eeh_add_pe_devices(void *data, void *userdata)
+{
+	struct pci_bus *bus;
+	struct eeh_pe *pe = (struct eeh_pe *)data;
+	struct eeh_dev *edev, *tmp;
+	int *removed = (int *)userdata;
+
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		if ((*removed) <= 0)
+			return pe;
+
+		if (!(edev->mode & EEH_DEV_DISCONNECTED))
+			continue;
+
+		pr_debug("EEH: Scanning %04x:%02x:%02x.%01x\n",
+			 pci_domain_nr(edev->bus), edev->bus->number,
+			 PCI_SLOT(edev->config_addr & 0xFF),
+			 PCI_FUNC(edev->config_addr & 0xFF));
+
+		/*
+		 * The EEH device is still connected to PE. It's time
+		 * to remove it from the parent PE.
+		 */
+		bus = edev->bus;
+		edev->mode &= ~(EEH_DEV_DISCONNECTED | EEH_DEV_IRQ_DISABLED);
+		edev->bus = NULL;
+		(*removed)--;
+		eeh_rmv_from_parent_pe(edev);
+
+		pcibios_scan_pci_dev(bus, eeh_dev_to_of_node(edev));
+	}
+
+	return NULL;
+}
+
+static void *eeh_pe_detach_dev(void *data, void *userdata)
+{
+	struct eeh_pe *pe = (struct eeh_pe *)data;
+	struct eeh_dev *edev, *tmp;
+
+	eeh_pe_for_each_dev(pe, edev, tmp) {
+		if (!(edev->mode & EEH_DEV_DISCONNECTED)) {
+			pr_warn("EEH: PHB#%x-PE#%x has bogus EEH device\n",
+				pe->phb->global_number, pe->addr);
+			continue;
+		}
+
+		edev->mode &= ~EEH_DEV_DISCONNECTED;
+		eeh_rmv_from_parent_pe(edev);
+	}
+
+	return NULL;
+}
+
 /**
  * eeh_reset_device - Perform actual reset of a pci slot
  * @pe: EEH PE
@@ -350,7 +436,7 @@ static void *eeh_report_failure(void *data, void *userdata)
 static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 {
 	struct timeval tstamp;
-	int cnt, rc;
+	int cnt, rc, removed = 0;
 
 	/* pcibios will clear the counter; save the value */
 	cnt = pe->freeze_count;
@@ -362,10 +448,11 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 	 * devices are expected to be attached soon when calling
 	 * into pcibios_add_pci_devices().
 	 */
-	if (bus) {
-		eeh_pe_state_mark(pe, EEH_PE_KEEP);
+	eeh_pe_state_mark(pe, EEH_PE_KEEP);
+	if (bus)
 		pcibios_remove_pci_devices(bus);
-	}
+	else
+		eeh_pe_dev_traverse(pe, eeh_rmv_device, &removed);
 
 	/* Reset the pci controller. (Asserts RST#; resets config space).
 	 * Reconfigure bridges and devices. Don't try to bring the system
@@ -386,10 +473,22 @@ static int eeh_reset_device(struct eeh_pe *pe, struct pci_bus *bus)
 	 * potentially weird things happen.
 	 */
 	if (bus) {
+		pr_info("EEH: Sleep 5s ahead of complete hotplug\n");
 		ssleep(5);
+
+		/*
+		 * The EEH device is still connected with its parent
+		 * PE. We should disconnect it so the binding can be
+		 * rebuilt when adding PCI devices.
+		 */
+		eeh_pe_traverse(pe, eeh_pe_detach_dev, NULL);
 		pcibios_add_pci_devices(bus);
-		eeh_pe_state_clear(pe, EEH_PE_KEEP);
+	} else if (removed) {
+		pr_info("EEH: Sleep 5s ahead of partial hotplug\n");
+		ssleep(5);
+		eeh_pe_traverse(pe, eeh_add_pe_devices, &removed);
 	}
+	eeh_pe_state_clear(pe, EEH_PE_KEEP);
 
 	pe->tstamp = tstamp;
 	pe->freeze_count = cnt;
diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index c8b815e..f6bdde7 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -149,8 +149,8 @@ static struct eeh_pe *eeh_pe_next(struct eeh_pe *pe,
  * callback returns something other than NULL, or no more PEs
  * to be traversed.
  */
-static void *eeh_pe_traverse(struct eeh_pe *root,
-			eeh_traverse_func fn, void *flag)
+void *eeh_pe_traverse(struct eeh_pe *root,
+		      eeh_traverse_func fn, void *flag)
 {
 	struct eeh_pe *pe;
 	void *ret;
@@ -728,22 +728,26 @@ static void eeh_restore_device_bars(struct eeh_dev *edev,
  */
 static void *eeh_restore_one_device_bars(void *data, void *flag)
 {
-	struct pci_dev *pdev = NULL;
 	struct eeh_dev *edev = (struct eeh_dev *)data;
+	struct pci_dev *pdev = eeh_dev_to_pci_dev(edev);
 	struct device_node *dn = eeh_dev_to_of_node(edev);
 
-	/* Trace the PCI bridge */
-	if (eeh_probe_mode_dev()) {
-		pdev = eeh_dev_to_pci_dev(edev);
-		if (pdev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
-                        pdev = NULL;
-        }
-
-	if (pdev)
+	/* Do special restore for bridges */
+	if (pdev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
 		eeh_restore_bridge_bars(pdev, edev, dn);
 	else
 		eeh_restore_device_bars(edev, dn);
 
+	/*
+	 * If the PCI device is associated with the EEH
+	 * device, It's time to clear the association.
+	 */
+	if (edev->mode & EEH_DEV_DISCONNECTED) {
+		edev->pdev = NULL;
+		pdev->dev.archdata.edev = NULL;
+		pci_dev_put(pdev);
+	}
+
 	return NULL;
 }
 
-- 
1.7.5.4

Re: [PATCH 1/8] PCI: Add pcibios_stop_dev()

[Benjamin Herrenschmidt]

On Fri, 2013-07-05 at 10:57 +0800, Gavin Shan wrote:
> When stopping and removing one specific PCI device, the platform
> might need take some actions. One example is that EEH already had
> eeh cache and eeh device attached to the PCI device, and we need
> release eeh cache and device during the time. The patch introduces
> hook pcibios_stop_dev() for the purpose.

Bjorn, any objection ? Ack ? Nack ? :-)

I'd like to put that in my tree, it's part of a series that fixes a
number of bugs with our hotplug and EEH code which I'd like to send
to Linus soonish.

Cheers,
Ben.

> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  drivers/pci/probe.c  |    4 ++++
>  drivers/pci/remove.c |    2 ++
>  include/linux/pci.h  |    1 +
>  3 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 70f10fa..7167dc4 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1669,6 +1669,10 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>  {
>  }
>  
> +void __weak pcibios_stop_dev(struct pci_dev *dev)
> +{
> +}
> +
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  		struct pci_ops *ops, void *sysdata, struct list_head *resources)
>  {
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 8fc54b7..e329efc 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -21,6 +21,8 @@ static void pci_stop_dev(struct pci_dev *dev)
>  {
>  	pci_pme_active(dev, false);
>  
> +	pcibios_stop_dev(dev);
> +
>  	if (dev->is_added) {
>  		pci_proc_detach_device(dev);
>  		pci_remove_sysfs_dev_files(dev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3a24e4f..40df783 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -696,6 +696,7 @@ int no_pci_devices(void);
>  void pcibios_resource_survey_bus(struct pci_bus *bus);
>  void pcibios_add_bus(struct pci_bus *bus);
>  void pcibios_remove_bus(struct pci_bus *bus);
> +void pcibios_stop_dev(struct pci_dev *dev);
>  void pcibios_fixup_bus(struct pci_bus *);
>  int __must_check pcibios_enable_device(struct pci_dev *, int mask);
>  /* Architecture specific versions may override this (weak) */

Re: [PATCH 1/8] PCI: Add pcibios_stop_dev()

[Bjorn Helgaas]

On Thu, Jul 4, 2013 at 8:57 PM, Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> When stopping and removing one specific PCI device, the platform
> might need take some actions. One example is that EEH already had
> eeh cache and eeh device attached to the PCI device, and we need
> release eeh cache and device during the time. The patch introduces
> hook pcibios_stop_dev() for the purpose.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  drivers/pci/probe.c  |    4 ++++
>  drivers/pci/remove.c |    2 ++
>  include/linux/pci.h  |    1 +
>  3 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 70f10fa..7167dc4 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1669,6 +1669,10 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>  {
>  }
>
> +void __weak pcibios_stop_dev(struct pci_dev *dev)
> +{
> +}
> +
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>                 struct pci_ops *ops, void *sysdata, struct list_head *resources)
>  {
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 8fc54b7..e329efc 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -21,6 +21,8 @@ static void pci_stop_dev(struct pci_dev *dev)
>  {
>         pci_pme_active(dev, false);
>
> +       pcibios_stop_dev(dev);

We already have pcibios_release_device() (just merged for v3.11).
Would that work for you?  I haven't seen your whole series, so I can't
tell whether you need something different.

>         if (dev->is_added) {
>                 pci_proc_detach_device(dev);
>                 pci_remove_sysfs_dev_files(dev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3a24e4f..40df783 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -696,6 +696,7 @@ int no_pci_devices(void);
>  void pcibios_resource_survey_bus(struct pci_bus *bus);
>  void pcibios_add_bus(struct pci_bus *bus);
>  void pcibios_remove_bus(struct pci_bus *bus);
> +void pcibios_stop_dev(struct pci_dev *dev);
>  void pcibios_fixup_bus(struct pci_bus *);
>  int __must_check pcibios_enable_device(struct pci_dev *, int mask);
>  /* Architecture specific versions may override this (weak) */
> --
> 1.7.5.4
>

Re: [PATCH 4/8] PCI/hotplug: Needn't remove EEH cache again

[Bjorn Helgaas]

On Thu, Jul 4, 2013 at 8:57 PM, Gavin Shan <shangw@linux.vnet.ibm.com> wrote:
> Since pci_stop_and_remove_bus_device() has removed the EEH cache,
> we needn't do that again.
>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>

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

Please merge this along with the rest of your series via whatever tree
you choose.

> ---
>  drivers/pci/hotplug/rpadlpar_core.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c
> index b29e20b..bb7af78 100644
> --- a/drivers/pci/hotplug/rpadlpar_core.c
> +++ b/drivers/pci/hotplug/rpadlpar_core.c
> @@ -388,7 +388,6 @@ int dlpar_remove_pci_slot(char *drc_name, struct device_node *dn)
>         /* Remove the EADS bridge device itself */
>         BUG_ON(!bus->self);
>         pr_debug("PCI: Now removing bridge device %s\n", pci_name(bus->self));
> -       eeh_remove_bus_device(bus->self, true);
>         pci_stop_and_remove_bus_device(bus->self);
>
>         return 0;
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/8] PCI: Add pcibios_stop_dev()

[Benjamin Herrenschmidt]

On Fri, 2013-07-05 at 12:49 -0600, Bjorn Helgaas wrote:
> We already have pcibios_release_device() (just merged for v3.11).
> Would that work for you?  I haven't seen your whole series, so I can't
> tell whether you need something different.

Maybe... Gavin's original hook applies when the device is removed
from the tree, your new hook when the refcount expires and the
structure is about to be freed...

I *suspect* that's fine but I'll need to have a closer look (or wait
for Gavin to be back from vacation :-)

BTW. One thing we do in EEH is that if we get an error and the driver
for the device doesn't implement recovery, we simulate a hotplug.

IE. We unplug the device (currently removing it), reset it (or the
bus it's on, whatever applies, which lifts the fence established by
the EEH hardware), and re-plug it.

The current method really removes the device from the PCI subsystem,
and "plugs" it back. IE. We rescan the slot, and might even end up
re-assigning resources, which I'm not too fan about. It's also unclear
what quirks gets called in that re-plug case, etc...

I'm wondering whether it might be better to keep the pci_dev around,
just mark it blocked for user access, unbind the driver, reset, restore
the BARs to their original values, unblock and re-bind.

What's your take on this ?

Cheers,
Ben.

Re: [PATCH 1/8] PCI: Add pcibios_stop_dev()

[Bjorn Helgaas]

On Fri, Jul 5, 2013 at 4:36 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2013-07-05 at 12:49 -0600, Bjorn Helgaas wrote:
>> We already have pcibios_release_device() (just merged for v3.11).
>> Would that work for you?  I haven't seen your whole series, so I can't
>> tell whether you need something different.
>
> Maybe... Gavin's original hook applies when the device is removed
> from the tree, your new hook when the refcount expires and the
> structure is about to be freed...

Yep, pcibios_release_device() is definitely at a different point.  I
just want to avoid exposing too many points in the device lifetime to
the arch, because it makes it so much harder to refactor things.

> I *suspect* that's fine but I'll need to have a closer look (or wait
> for Gavin to be back from vacation :-)
>
> BTW. One thing we do in EEH is that if we get an error and the driver
> for the device doesn't implement recovery, we simulate a hotplug.
>
> IE. We unplug the device (currently removing it), reset it (or the
> bus it's on, whatever applies, which lifts the fence established by
> the EEH hardware), and re-plug it.
>
> The current method really removes the device from the PCI subsystem,
> and "plugs" it back. IE. We rescan the slot, and might even end up
> re-assigning resources, which I'm not too fan about. It's also unclear
> what quirks gets called in that re-plug case, etc...
>
> I'm wondering whether it might be better to keep the pci_dev around,
> just mark it blocked for user access, unbind the driver, reset, restore
> the BARs to their original values, unblock and re-bind.

Personally, I like the full hotplug approach, even though it will
probably reassign resources, because it uses the standard paths that
should be better-tested, and it's clear what should happen.
Reassigning resources should be OK because we already have to do that
for the non-error handling hot-add case.

I know we have paths where we save config space, do a reset, and
restore config space.  I don't like those because some quirks aren't
applied and I don't believe restoring config space is sufficient to
make the device safe to use.  There could be internal state that we
aren't restoring.  It's also possible that a firmware update means the
device is different than it was before the reset, e.g., BARs could be
different types or sizes.

Bjorn

Re: [PATCH 1/8] PCI: Add pcibios_stop_dev()

[Benjamin Herrenschmidt]

On Fri, 2013-07-05 at 16:49 -0600, Bjorn Helgaas wrote:
> I know we have paths where we save config space, do a reset, and
> restore config space.  I don't like those because some quirks aren't
> applied and I don't believe restoring config space is sufficient to
> make the device safe to use.  There could be internal state that we
> aren't restoring.  It's also possible that a firmware update means the
> device is different than it was before the reset, e.g., BARs could be
> different types or sizes.

True. Ok, I'll stick to the actual hotplug path then, you make
good points.

Cheers,
Ben.