[PATCH v3 0/9]

PATCH Superseded Sam Bobroff 3 participants 13 messages Jul 23, 2019 – Jul 25, 2019

Subsystems (2)

PowerPC PCI enhanced error handling (eeh) for PowerPC

lore

Share & tools

lore

Sam Bobroff

author

2019-07-24 03:29 UTC #0 lore raw

v3 cover [PATCH v3 0/9]

Hi all,

Here is v3, addressing feedback from v2 to do with debug logging.

It includes a new patch from Oliver O'Halloran (patch 7) and two additional patches based on it. See those patches for details.

Cover letter:

This patch set adds support for EEH recovery of hot plugged devices on pSeries machines. Specifically, devices discovered by PCI rescanning using /sys/bus/pci/rescan, which includes devices hotplugged by QEMU's device_add command. (Upstream Linux pSeries guests running under QEMU/KVM don't currently use slot power control for hotplugging.)

As a side effect this also provides EEH support for devices removed by /sys/bus/pci/devices/*/remove and re-discovered by writing to /sys/bus/pci/rescan, on all platforms.

The approach I've taken is to use the fact that the existing pcibios_bus_add_device() platform hooks (which are used to set up EEH on Virtual Function devices (VFs)) are actually called for all devices, so I've widened their scope and made other adjustments necessary to allow them to work for hotplugged and boot-time devices as well.

Because some of the changes are in generic PowerPC code, it's possible that I've disturbed something for another PowerPC platform. I've tried to minimize this by leaving that code alone as much as possible and so there are a few cases where eeh_add_device_{early,late}() or eeh_add_sysfs_files() is called more than once. I think these can be looked at later, as duplicate calls are not harmful.

The first patch is a rework of the pcibios_init reordering patch I posted earlier, which I've included here because it's necessary for this set.

I have done some testing for PowerNV on Power9 using a modified pnv_php module and some testing on pSeries with slot power control using a modified rpaphp module, and the EEH-related parts seem to work.

Cheers,
Sam.

Patch set changelog follows:

Patch set v3:
Patch 1/9: powerpc/64: Adjust order in pcibios_init()
Patch 2/9: powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
Patch 3/9: powerpc/eeh: Improve debug messages around device addition Patch 4/9: powerpc/eeh: Initialize EEH address cache earlier
Patch 5/9: powerpc/eeh: EEH for pSeries hot plug
Patch 6/9: powerpc/eeh: Refactor around eeh_probe_devices()
Patch 7/9 (new in this version): powerpc/eeh: Add bdfn field to eeh_dev Patch 8/9 (new in this version): powerpc/eeh: Introduce EEH edev logging macros Patch 9/9 (new in this version): powerpc/eeh: Convert log messages to eeh_edev_* macros

Patch set v2:
Patch 1/6: powerpc/64: Adjust order in pcibios_init()
Patch 2/6: powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
* Also clear EEH_DEV_NO_HANDLER in eeh_handle_special_event().
Patch 3/6 (was 4/8): powerpc/eeh: Improve debug messages around device addition Patch 4/6 (was 6/8): powerpc/eeh: Initialize EEH address cache earlier Patch 5/6 (was 3/8 and 7/8): powerpc/eeh: EEH for pSeries hot plug
- Dropped changes to the PowerNV PHB EEH flag, instead refactor just enough to use the existing flag from multiple places.
- Merge the little remaining work from the above change into the patch where it's used.
Patch 6/6 (was 5/8 and 8/8): powerpc/eeh: Refactor around eeh_probe_devices()
- As it's so small, merged the enablement message patch into this one (where it's used).
- Reworked enablement messages.

Patch set v1:
Patch 1/8: powerpc/64: Adjust order in pcibios_init()
Patch 2/8: powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag
Patch 3/8: powerpc/eeh: Convert PNV_PHB_FLAG_EEH to global flag Patch 4/8: powerpc/eeh: Improve debug messages around device addition Patch 5/8: powerpc/eeh: Add eeh_show_enabled()
Patch 6/8: powerpc/eeh: Initialize EEH address cache earlier Patch 7/8: powerpc/eeh: EEH for pSeries hot plug
Patch 8/8: powerpc/eeh: Remove eeh_probe_devices() and eeh_addr_cache_build()

Oliver O'Halloran (1):
powerpc/eeh: Add bdfn field to eeh_dev

Sam Bobroff (8):
powerpc/64: Adjust order in pcibios_init() powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag powerpc/eeh: Improve debug messages around device addition powerpc/eeh: Initialize EEH address cache earlier powerpc/eeh: EEH for pSeries hot plug powerpc/eeh: Refactor around eeh_probe_devices() powerpc/eeh: Introduce EEH edev logging macros powerpc/eeh: Convert log messages to eeh_edev_* macros

 arch/powerpc/include/asm/eeh.h               | 21 ++++--
 arch/powerpc/include/asm/ppc-pci.h           |  7 +-
 arch/powerpc/kernel/eeh.c                    | 50 ++++++--------
 arch/powerpc/kernel/eeh_cache.c              | 37 ++---------
 arch/powerpc/kernel/eeh_dev.c                |  2 +
 arch/powerpc/kernel/eeh_driver.c             | 32 ++++-----
 arch/powerpc/kernel/eeh_pe.c                 | 51 ++++-----------
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/kernel/pci-common.c             |  4 --
 arch/powerpc/kernel/pci_32.c                 |  4 ++
 arch/powerpc/kernel/pci_64.c                 | 12 +++-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 56 +++++++++-------
 arch/powerpc/platforms/pseries/eeh_pseries.c | 68 ++++++++++----------
 arch/powerpc/platforms/pseries/pci.c         |  3 +-
 14 files changed, 158 insertions(+), 192 deletions(-)

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-23 03:48 UTC #1 lore raw

PATCH v3 1/9 powerpc/64: Adjust order in pcibios_init()

The pcibios_init() function for 64 bit PowerPC currently calls pci_bus_add_devices() before pcibios_resource_survey(), which seems incorrect because it adds devices and attempts to bind their drivers before allocating their resources (although no problems seem to be apparent).

So move the call to pci_bus_add_devices() to after pcibios_resource_survey(), while extracting call to the pcibios_fixup() hook so that it remains in the same location.

This will also allow the ppc_md.pcibios_bus_add_device() hooks to perform actions that depend on PCI resources, both during rescanning (where this is already the case) and at boot time, to support future work.

Signed-off-by Sam Bobroff <[email protected]>

Reviewed-by Alexey Kardashevskiy <[email protected]>

---
 arch/powerpc/kernel/pci-common.c |  4 ----
 arch/powerpc/kernel/pci_32.c     |  4 ++++
 arch/powerpc/kernel/pci_64.c     | 12 +++++++++---
 3 files changed, 13 insertions(+), 7 deletions(-)

arch/powerpc/kernel/pci-common.c +0 -4

index f627e15bb43c..1c448cf25506 100644
@@ -1379,10 +1379,6 @@ void __init pcibios_resource_survey(void)
 		pr_debug("PCI: Assigning unassigned resources...\n");
 		pci_assign_unassigned_resources();
 	}
-
-	/* Call machine dependent fixup */
-	if (ppc_md.pcibios_fixup)
-		ppc_md.pcibios_fixup();
 }
 
 /* This is used by the PCI hotplug driver to allocate resource

arch/powerpc/kernel/pci_32.c +4 -0

index 50942a1d1a5f..b49e1060a3bf 100644
@@ -263,6 +263,10 @@ static int __init pcibios_init(void)
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	/* Call machine dependent post-init code */
 	if (ppc_md.pcibios_after_init)
 		ppc_md.pcibios_after_init();

arch/powerpc/kernel/pci_64.c +9 -3

index b7030b1189d0..f83d1f69b1dd 100644
@@ -54,14 +54,20 @@ static int __init pcibios_init(void)
 	pci_add_flags(PCI_ENABLE_PROC_DOMAINS | PCI_COMPAT_DOMAIN_0);
 
 	/* Scan all of the recorded PCI controllers.  */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		pcibios_scan_phb(hose);
-		pci_bus_add_devices(hose->bus);
-	}
 
 	/* Call common code to handle resource allocation */
 	pcibios_resource_survey();
 
+	/* Add devices. */
+	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
+		pci_bus_add_devices(hose->bus);
+
+	/* Call machine dependent fixup */
+	if (ppc_md.pcibios_fixup)
+		ppc_md.pcibios_fixup();
+
 	printk(KERN_DEBUG "PCI: Probing PCI hardware done\n");
 
 	return 0;

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:26 UTC #2 lore raw

PATCH v3 2/9 powerpc/eeh: Clear stale EEH_DEV_NO_HANDLER flag

In reply to Sam Bobroff

The EEH_DEV_NO_HANDLER flag is used by the EEH system to prevent the use of driver callbacks in drivers that have been bound part way through the recovery process. This is necessary to prevent later stage handlers from being called when the earlier stage handlers haven't, which can be confusing for drivers.

However, the flag is set for all devices that are added after boot time and only cleared at the end of the EEH recovery process. This results in hot plugged devices erroneously having the flag set during the first recovery after they are added (causing their driver's handlers to be incorrectly ignored).

To remedy this, clear the flag at the beginning of recovery processing. The flag is still cleared at the end of recovery processing, although it is no longer really necessary.

Also clear the flag during eeh_handle_special_event(), for the same reasons.

Signed-off-by Sam Bobroff <[email protected]>

---
 arch/powerpc/kernel/eeh_driver.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

arch/powerpc/kernel/eeh_driver.c +10 -1

index 6f3ee30565dd..d6f54840a3a9 100644
@@ -819,6 +819,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
 		result = PCI_ERS_RESULT_DISCONNECT;
 	}
 
+	eeh_for_each_pe(pe, tmp_pe)
+		eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+			edev->mode &= ~EEH_DEV_NO_HANDLER;
+
 	/* Walk the various device drivers attached to this slot through
 	 * a reset sequence, giving each an opportunity to do what it needs
 	 * to accomplish the reset.  Each child gets a report of the

@@ -1009,7 +1013,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
  */
 void eeh_handle_special_event(void)
 {
-	struct eeh_pe *pe, *phb_pe;
+	struct eeh_pe *pe, *phb_pe, *tmp_pe;
+	struct eeh_dev *edev, *tmp_edev;
 	struct pci_bus *bus;
 	struct pci_controller *hose;
 	unsigned long flags;

@@ -1078,6 +1083,10 @@ void eeh_handle_special_event(void)
 				    (phb_pe->state & EEH_PE_RECOVERING))
 					continue;
 
+				eeh_for_each_pe(pe, tmp_pe)
+					eeh_pe_for_each_dev(tmp_pe, edev, tmp_edev)
+						edev->mode &= ~EEH_DEV_NO_HANDLER;
+
 				/* Notify all devices to be down */
 				eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
 				eeh_set_channel_state(pe, pci_channel_io_perm_failure);

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:24 UTC #3 lore raw

PATCH v3 3/9 powerpc/eeh: Improve debug messages around device addition

In reply to Sam Bobroff

Also remove useless comment.

Signed-off-by Sam Bobroff <[email protected]>

Reviewed-by Alexey Kardashevskiy <[email protected]>

---
 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 14 ++++++++----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 23 +++++++++++++++-----
 3 files changed, 28 insertions(+), 11 deletions(-)

arch/powerpc/kernel/eeh.c +1 -1

index d44533bba642..846cc697030c 100644
@@ -1278,7 +1278,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 	edev = pdn_to_eeh_dev(pdn);
 	if (edev->pdev == dev) {
-		pr_debug("EEH: Already referenced !\n");
+		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
 		return;
 	}

arch/powerpc/platforms/powernv/eeh-powernv.c +10 -4

index 1cd5ebd7299c..629f9390d9af 100644
@@ -46,10 +46,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
-	/*
-	 * The following operations will fail if VF's sysfs files
-	 * aren't created or its resources aren't finalized.
-	 */
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
 	eeh_sysfs_add_device(pdev);

@@ -393,6 +390,10 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	int ret;
 	int config_addr = (pdn->busno << 8) | (pdn->devfn);
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, hose->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/*
 	 * When probing the root bridge, which doesn't have any
 	 * subordinate PCI devices. We don't have OF node for

@@ -487,6 +488,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	/* Save memory bars */
 	eeh_save_bars(edev);
 
+	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
+		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
+		edev->pe->addr);
+
 	return NULL;
 }

arch/powerpc/platforms/pseries/eeh_pseries.c +17 -6

index 07e3fc2667aa..31733f6d642c 100644
@@ -52,6 +52,8 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (!pdev->is_virtfn)
 		return;
 
+	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+
 	pdn->device_id  =  pdev->device;
 	pdn->vendor_id  =  pdev->vendor;
 	pdn->class_code =  pdev->class;

@@ -238,6 +240,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 	int enable = 0;
 	int ret;
 
+	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
+		__func__, pdn->phb->global_number, pdn->busno,
+		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+
 	/* Retrieve OF node and eeh device */
 	edev = pdn_to_eeh_dev(pdn);
 	if (!edev || edev->pe)

@@ -281,7 +287,12 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 
 	/* Enable EEH on the device */
 	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
-	if (!ret) {
+	if (ret) {
+		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
+	} else {
 		/* Retrieve PE address */
 		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);
 		pe.addr = edev->pe_config_addr;

@@ -297,11 +308,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 		if (enable) {
 			eeh_add_flag(EEH_ENABLED);
 			eeh_add_to_parent_pe(edev);
-
-			pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
-				__func__, pdn->busno, PCI_SLOT(pdn->devfn),
-				PCI_FUNC(pdn->devfn), pe.phb->global_number,
-				pe.addr);
 		} else if (pdn->parent && pdn_to_eeh_dev(pdn->parent) &&
 			   (pdn_to_eeh_dev(pdn->parent))->pe) {
 			/* This device doesn't support EEH, but it may have an

@@ -310,6 +316,11 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
 			eeh_add_to_parent_pe(edev);
 		}
+		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
+			__func__, (enable ? "enabled" : "unsupported"),
+			pdn->busno, PCI_SLOT(pdn->devfn),
+			PCI_FUNC(pdn->devfn), pe.phb->global_number,
+			pe.addr, ret);
 	}
 
 	/* Save memory bars */

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-23 03:46 UTC #4 lore raw

PATCH v3 4/9 powerpc/eeh: Initialize EEH address cache earlier

In reply to Sam Bobroff

The EEH address cache is currently initialized and populated by a single function: eeh_addr_cache_build(). While the initial population of the cache can only be done once resources are allocated, initialization (just setting up a spinlock) could be done much earlier.

So move the initialization step into a separate function and call it from a core_initcall (rather than a subsys initcall).

This will allow future work to make use of the cache during boot time PCI scanning.

Signed-off-by Sam Bobroff <[email protected]>

Reviewed-by Alexey Kardashevskiy <[email protected]>

---
 arch/powerpc/include/asm/eeh.h  |  3 +++
 arch/powerpc/kernel/eeh.c       |  2 ++
 arch/powerpc/kernel/eeh_cache.c | 13 +++++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

arch/powerpc/include/asm/eeh.h +3 -0

index 45c9b26e3cce..20105964287a 100644
@@ -275,6 +275,7 @@ int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
+void eeh_addr_cache_init(void);
 void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);

@@ -335,6 +336,8 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 #define eeh_dev_check_failure(x) (0)
 
+static inline void eeh_addr_cache_init(void) { }
+
 static inline void eeh_addr_cache_build(void) { }
 
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }

arch/powerpc/kernel/eeh.c +2 -0

index 846cc697030c..ca8b0c58a6a7 100644
@@ -1200,6 +1200,8 @@ static int eeh_init(void)
 	list_for_each_entry_safe(hose, tmp, &hose_list, list_node)
 		eeh_dev_phb_init_dynamic(hose);
 
+	eeh_addr_cache_init();
+
 	/* Initialize EEH event */
 	return eeh_event_init();
 }

arch/powerpc/kernel/eeh_cache.c +11 -2

index 320472373122..a790fa49c62d 100644
@@ -254,6 +254,17 @@ void eeh_addr_cache_rmv_dev(struct pci_dev *dev)
 	spin_unlock_irqrestore(&pci_io_addr_cache_root.piar_lock, flags);
 }
 
+/**
+ * eeh_addr_cache_init - Initialize a cache of I/O addresses
+ *
+ * Initialize a cache of pci i/o addresses.  This cache will be used to
+ * find the pci device that corresponds to a given address.
+ */
+void eeh_addr_cache_init(void)
+{
+	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
+}
+
 /**
  * eeh_addr_cache_build - Build a cache of I/O addresses
  *

@@ -269,8 +280,6 @@ void eeh_addr_cache_build(void)
 	struct eeh_dev *edev;
 	struct pci_dev *dev = NULL;
 
-	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
-
 	for_each_pci_dev(dev) {
 		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
 		if (!pdn)

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:32 UTC #5 lore raw

PATCH v3 5/9 powerpc/eeh: EEH for pSeries hot plug

In reply to Sam Bobroff

On PowerNV and pSeries, devices currently acquire EEH support from several different places: Boot-time devices from eeh_probe_devices() and eeh_addr_cache_build(), Virtual Function devices from the pcibios bus add device hooks and hot plugged devices from pci_hp_add_devices() (with other platforms using other methods as well). Unfortunately, pSeries machines currently discover hot plugged devices using pci_rescan_bus(), not pci_hp_add_devices(), and so those devices do not receive EEH support.

Rather than adding another case for pci_rescan_bus(), this change widens the scope of the pcibios bus add device hooks so that they can handle all devices. As a side effect this also supports devices discovered after manually rescanning via /sys/bus/pci/rescan.

Note that on PowerNV, this change allows the EEH subsystem to become enabled after boot as long as it has not been forced off, which was not previously possible (it was already possible on pSeries).

Signed-off-by Sam Bobroff <[email protected]>

---
 arch/powerpc/kernel/eeh.c                    |  2 +-
 arch/powerpc/kernel/of_platform.c            |  3 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c | 39 +++++++++-----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 54 ++++++++++----------
 4 files changed, 56 insertions(+), 42 deletions(-)

arch/powerpc/kernel/eeh.c +1 -1

index ca8b0c58a6a7..87edac6f2fd9 100644
@@ -1272,7 +1272,7 @@ void eeh_add_device_late(struct pci_dev *dev)
 	struct pci_dn *pdn;
 	struct eeh_dev *edev;
 
-	if (!dev || !eeh_enabled())
+	if (!dev)
 		return;
 
 	pr_debug("EEH: Adding device %s\n", pci_name(dev));

arch/powerpc/kernel/of_platform.c +2 -1

index 427fc22f72b6..11c807468ab5 100644
@@ -81,7 +81,8 @@ static int of_pci_phb_probe(struct platform_device *dev)
 	pcibios_claim_one_bus(phb->bus);
 
 	/* Finish EEH setup */
-	eeh_add_device_tree_late(phb->bus);
+	if (!eeh_has_flag(EEH_FORCE_DISABLED))
+		eeh_add_device_tree_late(phb->bus);
 
 	/* Add probed PCI devices to the device model */
 	pci_bus_add_devices(phb->bus);

arch/powerpc/platforms/powernv/eeh-powernv.c +27 ~~-12~~

index 629f9390d9af..77cc2f51c2ea 100644
@@ -43,7 +43,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 
-	if (!pdev->is_virtfn)
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
 	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));

@@ -222,6 +222,25 @@ static const struct file_operations eeh_tree_state_debugfs_ops = {
 
 #endif /* CONFIG_DEBUG_FS */
 
+void pnv_eeh_enable_phbs(void)
+{
+	struct pci_controller *hose;
+	struct pnv_phb *phb;
+
+	list_for_each_entry(hose, &hose_list, list_node) {
+		phb = hose->private_data;
+		/*
+		 * If EEH is enabled, we're going to rely on that.
+		 * Otherwise, we restore to conventional mechanism
+		 * to clear frozen PE during PCI config access.
+		 */
+		if (eeh_enabled())
+			phb->flags |= PNV_PHB_FLAG_EEH;
+		else
+			phb->flags &= ~PNV_PHB_FLAG_EEH;
+	}
+}
+
 /**
  * pnv_eeh_post_init - EEH platform dependent post initialization
  *

@@ -260,19 +279,11 @@ int pnv_eeh_post_init(void)
 	if (!eeh_enabled())
 		disable_irq(eeh_event_irq);
 
+	pnv_eeh_enable_phbs();
+
 	list_for_each_entry(hose, &hose_list, list_node) {
 		phb = hose->private_data;
 
-		/*
-		 * If EEH is enabled, we're going to rely on that.
-		 * Otherwise, we restore to conventional mechanism
-		 * to clear frozen PE during PCI config access.
-		 */
-		if (eeh_enabled())
-			phb->flags |= PNV_PHB_FLAG_EEH;
-		else
-			phb->flags &= ~PNV_PHB_FLAG_EEH;
-
 		/* Create debugfs entries */
 #ifdef CONFIG_DEBUG_FS
 		if (phb->has_dbgfs || !phb->dbgfs)

@@ -483,7 +494,11 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	 * Enable EEH explicitly so that we will do EEH check
 	 * while accessing I/O stuff
 	 */
-	eeh_add_flag(EEH_ENABLED);
+	if (!eeh_has_flag(EEH_ENABLED)) {
+		enable_irq(eeh_event_irq);
+		pnv_eeh_enable_phbs();
+		eeh_add_flag(EEH_ENABLED);
+	}
 
 	/* Save memory bars */
 	eeh_save_bars(edev);

arch/powerpc/platforms/pseries/eeh_pseries.c +26 ~~-28~~

80 lines not on this page · raw message

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:31 UTC #6 lore raw

PATCH v3 6/9 powerpc/eeh: Refactor around eeh_probe_devices()

In reply to Sam Bobroff

Now that EEH support for all devices (on PowerNV and pSeries) is provided by the pcibios bus add device hooks, eeh_probe_devices() and eeh_addr_cache_build() are redundant and can be removed.

Move the EEH enabled message into it's own function so that it can be called from multiple places.

Note that previously on pSeries, useless EEH sysfs files were created for some devices that did not have EEH support and this change prevents them from being created.

Signed-off-by Sam Bobroff <[email protected]>

---
 arch/powerpc/include/asm/eeh.h               |  7 ++---
 arch/powerpc/kernel/eeh.c                    | 27 ++++++-----------
 arch/powerpc/kernel/eeh_cache.c              | 32 --------------------
 arch/powerpc/platforms/powernv/eeh-powernv.c |  4 +--
 arch/powerpc/platforms/pseries/pci.c         |  3 +-
 5 files changed, 14 insertions(+), 59 deletions(-)

arch/powerpc/include/asm/eeh.h +2 -5

index 20105964287a..7f9404a0c3bb 100644
@@ -270,13 +270,12 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
 
 struct eeh_dev *eeh_dev_init(struct pci_dn *pdn);
 void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
-void eeh_probe_devices(void);
+void eeh_show_enabled(void);
 int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 int eeh_check_failure(const volatile void __iomem *token);
 int eeh_dev_check_failure(struct eeh_dev *edev);
 void eeh_addr_cache_init(void);
-void eeh_addr_cache_build(void);
 void eeh_add_device_early(struct pci_dn *);
 void eeh_add_device_tree_early(struct pci_dn *);
 void eeh_add_device_late(struct pci_dev *);

@@ -320,7 +319,7 @@ static inline bool eeh_enabled(void)
         return false;
 }
 
-static inline void eeh_probe_devices(void) { }
+static inline void eeh_show_enabled(void) { }
 
 static inline void *eeh_dev_init(struct pci_dn *pdn, void *data)
 {

@@ -338,8 +337,6 @@ static inline int eeh_check_failure(const volatile void __iomem *token)
 
 static inline void eeh_addr_cache_init(void) { }
 
-static inline void eeh_addr_cache_build(void) { }
-
 static inline void eeh_add_device_early(struct pci_dn *pdn) { }
 
 static inline void eeh_add_device_tree_early(struct pci_dn *pdn) { }

arch/powerpc/kernel/eeh.c +10 ~~-17~~

index 87edac6f2fd9..c0ec1b6b1e69 100644
@@ -150,6 +150,16 @@ static int __init eeh_setup(char *str)
 }
 __setup("eeh=", eeh_setup);
 
+void eeh_show_enabled(void)
+{
+	if (eeh_has_flag(EEH_FORCE_DISABLED))
+		pr_info("EEH: Recovery disabled by kernel parameter.\n");
+	else if (eeh_has_flag(EEH_ENABLED))
+		pr_info("EEH: Capable adapter found: recovery enabled.\n");
+	else
+		pr_info("EEH: No capable adapters found: recovery disabled.\n");
+}
+
 /*
  * This routine captures assorted PCI configuration space data
  * for the indicated PCI device, and puts them into a buffer

@@ -1143,23 +1153,6 @@ static struct notifier_block eeh_reboot_nb = {
 	.notifier_call = eeh_reboot_notifier,
 };
 
-void eeh_probe_devices(void)
-{
-	struct pci_controller *hose, *tmp;
-	struct pci_dn *pdn;
-
-	/* Enable EEH for all adapters */
-	list_for_each_entry_safe(hose, tmp, &hose_list, list_node) {
-		pdn = hose->pci_data;
-		traverse_pci_dn(pdn, eeh_ops->probe, NULL);
-	}
-	if (eeh_enabled())
-		pr_info("EEH: PCI Enhanced I/O Error Handling Enabled\n");
-	else
-		pr_info("EEH: No capable adapters found\n");
-
-}
-
 /**
  * eeh_init - EEH initialization
  *

arch/powerpc/kernel/eeh_cache.c +0 ~~-32~~

index a790fa49c62d..8c8649172e97 100644
@@ -265,38 +265,6 @@ void eeh_addr_cache_init(void)
 	spin_lock_init(&pci_io_addr_cache_root.piar_lock);
 }
 
-/**
- * eeh_addr_cache_build - Build a cache of I/O addresses
- *
- * Build a cache of pci i/o addresses.  This cache will be used to
- * find the pci device that corresponds to a given address.
- * This routine scans all pci busses to build the cache.
- * Must be run late in boot process, after the pci controllers
- * have been scanned for devices (after all device resources are known).
- */
-void eeh_addr_cache_build(void)
-{
-	struct pci_dn *pdn;
-	struct eeh_dev *edev;
-	struct pci_dev *dev = NULL;
-
-	for_each_pci_dev(dev) {
-		pdn = pci_get_pdn_by_devfn(dev->bus, dev->devfn);
-		if (!pdn)
-			continue;
-
-		edev = pdn_to_eeh_dev(pdn);
-		if (!edev)
-			continue;
-
-		dev->dev.archdata.edev = edev;
-		edev->pdev = dev;
-
-		eeh_addr_cache_insert_dev(dev);
-		eeh_sysfs_add_device(dev);
-	}
-}
-
 static int eeh_addr_cache_show(struct seq_file *s, void *v)
 {
 	struct pci_io_addr_range *piar;

arch/powerpc/platforms/powernv/eeh-powernv.c +1 -3

index 77cc2f51c2ea..7ee0df9ba2c8 100644
@@ -255,9 +255,7 @@ int pnv_eeh_post_init(void)
 	struct pnv_phb *phb;
 	int ret = 0;
 
-	/* Probe devices & build address cache */
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 
 	/* Register OPAL event notifier */
 	eeh_event_irq = opal_event_request(ilog2(OPAL_EVENT_PCI_ERROR));

arch/powerpc/platforms/pseries/pci.c +1 -2

index 1eae1d09980c..722830978639 100644
@@ -229,8 +229,7 @@ void __init pSeries_final_fixup(void)
 
 	pSeries_request_regions();
 
-	eeh_probe_devices();
-	eeh_addr_cache_build();
+	eeh_show_enabled();
 
 #ifdef CONFIG_PCI_IOV
 	ppc_md.pcibios_sriov_enable = pseries_pcibios_sriov_enable;

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:37 UTC #7 lore raw

PATCH v3 7/9 powerpc/eeh: Add bdfn field to eeh_dev

In reply to Sam Bobroff

From: Oliver O'Halloran <[email protected]>

Preparation for removing pci_dn from the powernv EEH code. The only thing we really use pci_dn for is to get the bdfn of the device for config space accesses, so adding that information to eeh_dev reduces the need to carry around the pci_dn.

Signed-off-by Oliver O'Halloran <[email protected]>

[SB: Re-wrapped commit message, fixed whitespace damage.]

Signed-off-by Sam Bobroff <[email protected]>

---
v3 * New in this version.

 arch/powerpc/include/asm/eeh.h     | 2 ++
 arch/powerpc/include/asm/ppc-pci.h | 2 ++
 arch/powerpc/kernel/eeh_dev.c      | 2 ++
 3 files changed, 6 insertions(+)

arch/powerpc/include/asm/eeh.h +2 -0

index 7f9404a0c3bb..bbe0798f6624 100644
@@ -121,6 +121,8 @@ static inline bool eeh_pe_passed(struct eeh_pe *pe)
 struct eeh_dev {
 	int mode;			/* EEH mode			*/
 	int class_code;			/* Class code of the device	*/
+	int bdfn;			/* bdfn of device (for cfg ops) */
+	struct pci_controller *controller;
 	int pe_config_addr;		/* PE config address		*/
 	u32 config_space[16];		/* Saved PCI config space	*/
 	int pcix_cap;			/* Saved PCIx capability	*/

arch/powerpc/include/asm/ppc-pci.h +2 -0

index cec2d6409515..72860de205a0 100644
@@ -74,6 +74,8 @@ static inline const char *eeh_driver_name(struct pci_dev *pdev)
 
 #endif /* CONFIG_EEH */
 
+#define PCI_BUSNO(bdfn) ((bdfn >> 8) & 0xff)
+
 #else /* CONFIG_PCI */
 static inline void init_pci_config_tokens(void) { }
 #endif /* !CONFIG_PCI */

arch/powerpc/kernel/eeh_dev.c +2 -0

index c4317c452d98..7370185c7a05 100644
@@ -47,6 +47,8 @@ struct eeh_dev *eeh_dev_init(struct pci_dn *pdn)
 	/* Associate EEH device with OF node */
 	pdn->edev = edev;
 	edev->pdn = pdn;
+	edev->bdfn = (pdn->busno << 8) | pdn->devfn;
+	edev->controller = pdn->phb;
 
 	return edev;
 }

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-23 03:49 UTC #8 lore raw

PATCH v3 8/9 powerpc/eeh: Introduce EEH edev logging macros

In reply to Sam Bobroff

Now that struct eeh_dev includes the BDFN of it's PCI device, make use of it to replace eeh_edev_info() with a set of dev_dbg()-style macros that only need a struct edev.

With the BDFN available without the struct pci_dev, eeh_pci_name() is now unnecessary, so remove it.

While only the "info" level function is used here, the others will be used in followup work.

Signed-off-by Sam Bobroff <[email protected]>

---
v3 * New in this version.

 arch/powerpc/include/asm/eeh.h   | 11 +++++++++++
 arch/powerpc/kernel/eeh_driver.c | 17 -----------------
 2 files changed, 11 insertions(+), 17 deletions(-)

arch/powerpc/include/asm/eeh.h +11 -0

index bbe0798f6624..e1023a556721 100644
@@ -138,6 +138,17 @@ struct eeh_dev {
 	struct pci_dev *physfn;		/* Associated SRIOV PF		*/
 };
 
+/* "fmt" must be a simple literal string */
+#define EEH_EDEV_PRINT(level, edev, fmt, ...) \
+	pr_##level("PCI %04x:%02x:%02x.%x#%04x: EEH: " fmt, \
+	(edev)->controller->global_number, PCI_BUSNO((edev)->bdfn), \
+	PCI_SLOT((edev)->bdfn), PCI_FUNC((edev)->bdfn), \
+	((edev)->pe ? (edev)->pe_config_addr : 0xffff), ##__VA_ARGS__)
+#define eeh_edev_dbg(edev, fmt, ...) EEH_EDEV_PRINT(debug, (edev), fmt, ##__VA_ARGS__)
+#define eeh_edev_info(edev, fmt, ...) EEH_EDEV_PRINT(info, (edev), fmt, ##__VA_ARGS__)
+#define eeh_edev_warn(edev, fmt, ...) EEH_EDEV_PRINT(warn, (edev), fmt, ##__VA_ARGS__)
+#define eeh_edev_err(edev, fmt, ...) EEH_EDEV_PRINT(err, (edev), fmt, ##__VA_ARGS__)
+
 static inline struct pci_dn *eeh_dev_to_pdn(struct eeh_dev *edev)
 {
 	return edev ? edev->pdn : NULL;

arch/powerpc/kernel/eeh_driver.c +0 ~~-17~~

index d6f54840a3a9..29424d5e5fea 100644
@@ -82,23 +82,6 @@ static const char *pci_ers_result_name(enum pci_ers_result result)
 	}
 };
 
-static __printf(2, 3) void eeh_edev_info(const struct eeh_dev *edev,
-					 const char *fmt, ...)
-{
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	printk(KERN_INFO "EEH: PE#%x (PCI %s): %pV\n", edev->pe_config_addr,
-	       edev->pdev ? dev_name(&edev->pdev->dev) : "none", &vaf);
-
-	va_end(args);
-}
-
 static enum pci_ers_result pci_ers_merge_result(enum pci_ers_result old,
 						enum pci_ers_result new)
 {

Signature · 1 line

2.22.0.216.g00a2a96fc9

Sam Bobroff

author

2019-07-24 03:34 UTC #9 lore raw

PATCH v3 9/9 powerpc/eeh: Convert log messages to eeh_edev_* macros

In reply to Sam Bobroff

Convert existing messages, where appropriate, to use the eeh_edev_* logging macros.

The only effect should be minor adjustments to the log messages, apart from:

- A new message in pseries_eeh_probe() "Probing device" to match the powernv case.
- The "Probing device" message in pnv_eeh_probe() is now generated slightly later, which will mean that it is no longer emitted for devices that aren't probed due to the initial checks.

Signed-off-by Sam Bobroff <[email protected]>

---
v3 * New in this version.

 arch/powerpc/include/asm/ppc-pci.h           |  5 --
 arch/powerpc/kernel/eeh.c                    | 19 +++-----
 arch/powerpc/kernel/eeh_cache.c              |  8 +--
 arch/powerpc/kernel/eeh_driver.c             |  4 +-
 arch/powerpc/kernel/eeh_pe.c                 | 51 +++++---------------
 arch/powerpc/platforms/powernv/eeh-powernv.c | 17 ++-----
 arch/powerpc/platforms/pseries/eeh_pseries.c | 21 +++-----
 7 files changed, 38 insertions(+), 87 deletions(-)

arch/powerpc/include/asm/ppc-pci.h +0 -5

index 72860de205a0..7f4be5a05eb3 100644
@@ -62,11 +62,6 @@ void eeh_pe_dev_mode_mark(struct eeh_pe *pe, int mode);
 void eeh_sysfs_add_device(struct pci_dev *pdev);
 void eeh_sysfs_remove_device(struct pci_dev *pdev);
 
-static inline const char *eeh_pci_name(struct pci_dev *pdev) 
-{ 
-	return pdev ? pci_name(pdev) : "<null>";
-} 
-
 static inline const char *eeh_driver_name(struct pci_dev *pdev)
 {
 	return (pdev && pdev->driver) ? pdev->driver->name : "<null>";

arch/powerpc/kernel/eeh.c +8 ~~-11~~

index c0ec1b6b1e69..b6683f367f7f 100644
@@ -461,8 +461,7 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	/* Access to IO BARs might get this far and still not want checking. */
 	if (!pe) {
 		eeh_stats.ignored_check++;
-		pr_debug("EEH: Ignored check for %s\n",
-			eeh_pci_name(dev));
+		eeh_edev_dbg(edev, "Ignored check\n");
 		return 0;
 	}

@@ -502,12 +501,11 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 			if (dn)
 				location = of_get_property(dn, "ibm,loc-code",
 						NULL);
-			printk(KERN_ERR "EEH: %d reads ignored for recovering device at "
-				"location=%s driver=%s pci addr=%s\n",
+			eeh_edev_err(edev, "%d reads ignored for recovering device at location=%s driver=%s\n",
 				pe->check_count,
 				location ? location : "unknown",
-				eeh_driver_name(dev), eeh_pci_name(dev));
-			printk(KERN_ERR "EEH: Might be infinite loop in %s driver\n",
+				eeh_driver_name(dev));
+			eeh_edev_err(edev, "Might be infinite loop in %s driver\n",
 				eeh_driver_name(dev));
 			dump_stack();
 		}

@@ -1268,12 +1266,11 @@ void eeh_add_device_late(struct pci_dev *dev)
 	if (!dev)
 		return;
 
-	pr_debug("EEH: Adding device %s\n", pci_name(dev));
-
 	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) {
-		pr_debug("EEH: Device %s already referenced!\n", pci_name(dev));
+		eeh_edev_dbg(edev, "Device already referenced!\n");
 		return;
 	}

@@ -1374,10 +1371,10 @@ void eeh_remove_device(struct pci_dev *dev)
 	edev = pci_dev_to_eeh_dev(dev);
 
 	/* Unregister the device with the EEH/PCI address search system */
-	pr_debug("EEH: Removing device %s\n", pci_name(dev));
+	dev_dbg(&dev->dev, "EEH: Removing device\n");
 
 	if (!edev || !edev->pdev || !edev->pe) {
-		pr_debug("EEH: Not referenced !\n");
+		dev_dbg(&dev->dev, "EEH: Device not referenced!\n");
 		return;
 	}

arch/powerpc/kernel/eeh_cache.c +4 -4

index 8c8649172e97..45360b9eab90 100644
@@ -145,8 +145,8 @@ eeh_addr_cache_insert(struct pci_dev *dev, resource_size_t alo,
 	piar->pcidev = dev;
 	piar->flags = flags;
 
-	pr_debug("PIAR: insert range=[%pap:%pap] dev=%s\n",
-		 &alo, &ahi, pci_name(dev));
+	eeh_edev_dbg(piar->edev, "PIAR: insert range=[%pap:%pap]\n",
+		 &alo, &ahi);
 
 	rb_link_node(&piar->rb_node, parent, p);
 	rb_insert_color(&piar->rb_node, &pci_io_addr_cache_root.rb_root);

@@ -226,8 +226,8 @@ static inline void __eeh_addr_cache_rmv_dev(struct pci_dev *dev)
 		piar = rb_entry(n, struct pci_io_addr_range, rb_node);
 
 		if (piar->pcidev == dev) {
-			pr_debug("PIAR: remove range=[%pap:%pap] dev=%s\n",
-				 &piar->addr_lo, &piar->addr_hi, pci_name(dev));
+			eeh_edev_dbg(piar->edev, "PIAR: remove range=[%pap:%pap]\n",
+				 &piar->addr_lo, &piar->addr_hi);
 			rb_erase(n, &pci_io_addr_cache_root.rb_root);
 			kfree(piar);
 			goto restart;

arch/powerpc/kernel/eeh_driver.c +1 -3

index 29424d5e5fea..87e531bafe75 100644
@@ -460,9 +460,7 @@ static void *eeh_add_virt_device(struct eeh_dev *edev)
 	struct pci_dn *pdn = eeh_dev_to_pdn(edev);
 
 	if (!(edev->physfn)) {
-		pr_warn("%s: EEH dev %04x:%02x:%02x.%01x not for VF\n",
-			__func__, pdn->phb->global_number, pdn->busno,
-			PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
+		eeh_edev_warn(edev, "Not for VF\n");
 		return NULL;
 	}

arch/powerpc/kernel/eeh_pe.c +13 ~~-38~~

115 lines not on this page · raw message

arch/powerpc/platforms/powernv/eeh-powernv.c +5 ~~-12~~

index 7ee0df9ba2c8..43c3b7d81794 100644
@@ -46,7 +46,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
-	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+	dev_dbg(&pdev->dev, "EEH: Setting up device\n");
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
 	eeh_sysfs_add_device(pdev);

@@ -399,10 +399,6 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	int ret;
 	int config_addr = (pdn->busno << 8) | (pdn->devfn);
 
-	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
-		__func__, hose->global_number, pdn->busno,
-		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
-
 	/*
 	 * When probing the root bridge, which doesn't have any
 	 * subordinate PCI devices. We don't have OF node for

@@ -416,6 +412,8 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_ISA)
 		return NULL;
 
+	eeh_edev_dbg(edev, "Probing device\n");
+
 	/* Initialize eeh device */
 	edev->class_code = pdn->class_code;
 	edev->mode	&= 0xFFFFFF00;

@@ -441,9 +439,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	/* Create PE */
 	ret = eeh_add_to_parent_pe(edev);
 	if (ret) {
-		pr_warn("%s: Can't add PCI dev %04x:%02x:%02x.%01x to parent PE (%x)\n",
-			__func__, hose->global_number, pdn->busno,
-			PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn), ret);
+		eeh_edev_warn(edev, "Failed to add device to PE (code %d)\n", ret);
 		return NULL;
 	}

@@ -501,10 +497,7 @@ static void *pnv_eeh_probe(struct pci_dn *pdn, void *data)
 	/* Save memory bars */
 	eeh_save_bars(edev);
 
-	pr_debug("%s: EEH enabled on %02x:%02x.%01x PHB#%x-PE#%x\n",
-		__func__, pdn->busno, PCI_SLOT(pdn->devfn),
-		PCI_FUNC(pdn->devfn), edev->pe->phb->global_number,
-		edev->pe->addr);
+	eeh_edev_dbg(edev, "EEH enabled on device\n");
 
 	return NULL;
 }

arch/powerpc/platforms/pseries/eeh_pseries.c +7 ~~-14~~

index 96ad41fbf96b..517982197451 100644
@@ -49,7 +49,7 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	if (eeh_has_flag(EEH_FORCE_DISABLED))
 		return;
 
-	pr_debug("%s: EEH: Setting up device %s.\n", __func__, pci_name(pdev));
+	dev_dbg(&pdev->dev, "EEH: Setting up device\n");
 #ifdef CONFIG_PCI_IOV
 	if (pdev->is_virtfn) {
 		struct pci_dn *physfn_pdn;

@@ -238,10 +238,6 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 	int enable = 0;
 	int ret;
 
-	pr_debug("%s: probing %04x:%02x:%02x.%01x\n",
-		__func__, pdn->phb->global_number, pdn->busno,
-		PCI_SLOT(pdn->devfn), PCI_FUNC(pdn->devfn));
-
 	/* Retrieve OF node and eeh device */
 	edev = pdn_to_eeh_dev(pdn);
 	if (!edev || edev->pe)

@@ -255,6 +251,8 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
         if ((pdn->class_code >> 8) == PCI_CLASS_BRIDGE_ISA)
 		return NULL;
 
+	eeh_edev_dbg(edev, "Probing device\n");
+
 	/*
 	 * Update class code and mode of eeh device. We need
 	 * correctly reflects that current device is root port

@@ -284,12 +282,10 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 	pe.config_addr = (pdn->busno << 16) | (pdn->devfn << 8);
 
 	/* Enable EEH on the device */
+	eeh_edev_dbg(edev, "Enabling EEH on device\n");
 	ret = eeh_ops->set_option(&pe, EEH_OPT_ENABLE);
 	if (ret) {
-		pr_debug("%s: EEH failed to enable on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
-			__func__, pdn->busno, PCI_SLOT(pdn->devfn),
-			PCI_FUNC(pdn->devfn), pe.phb->global_number,
-			pe.addr, ret);
+		eeh_edev_dbg(edev, "EEH failed to enable on device (code %d)\n", ret);
 	} else {
 		/* Retrieve PE address */
 		edev->pe_config_addr = eeh_ops->get_pe_addr(&pe);

@@ -314,11 +310,8 @@ static void *pseries_eeh_probe(struct pci_dn *pdn, void *data)
 			edev->pe_config_addr = pdn_to_eeh_dev(pdn->parent)->pe_config_addr;
 			eeh_add_to_parent_pe(edev);
 		}
-		pr_debug("%s: EEH %s on %02x:%02x.%01x PHB#%x-PE#%x (code %d)\n",
-			__func__, (enable ? "enabled" : "unsupported"),
-			pdn->busno, PCI_SLOT(pdn->devfn),
-			PCI_FUNC(pdn->devfn), pe.phb->global_number,
-			pe.addr, ret);
+		eeh_edev_dbg(edev, "EEH is %s on device (code %d)\n",
+			     (enable ? "enabled" : "unsupported"), ret);
 	}
 
 	/* Save memory bars */

Signature · 1 line

2.22.0.216.g00a2a96fc9

kbuild test robot

build bot

2019-07-24 09:28 UTC #10 lore raw

Re: [PATCH v3 9/9] powerpc/eeh: Convert log messages to eeh_edev_* macros

I love your patch! Yet something to improve:

Hi Sam,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.3-rc1 next-20190724]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Sam-Bobroff/powerpc-64-Adjust-order-in-pcibios_init/20190724-134001
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=powerpc

If you fix the issue, kindly add following tag

Reported-by kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

arch/powerpc/kernel/eeh_driver.c: In function 'eeh_add_virt_device':

>> arch/powerpc/kernel/eeh_driver.c:459:17: error: unused variable 'pdn' [-Werror=unused-variable]
     struct pci_dn *pdn = eeh_dev_to_pdn(edev);
                    ^~~

cc1: all warnings being treated as errors

vim +/pdn +459 arch/powerpc/kernel/eeh_driver.c

77bd7415 arch/powerpc/platforms/pseries/eeh_driver.c Linas Vepstas 2005-11-03  454  
bf773df9 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-09-12  455  static void *eeh_add_virt_device(struct eeh_dev *edev)
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  456  {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  457  	struct pci_driver *driver;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  458  	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04 @459  	struct pci_dn *pdn = eeh_dev_to_pdn(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  460  
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  461  	if (!(edev->physfn)) {
6dad7bbd arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2019-07-23  462  		eeh_edev_warn(edev, "Not for VF\n");
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  463  		return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  464  	}
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  465  
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  466  	driver = eeh_pcid_get(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  467  	if (driver) {
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  468  		if (driver->err_handler) {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  469  			eeh_pcid_put(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  470  			return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  471  		}
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  472  		eeh_pcid_put(dev);
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  473  	}
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  474

:::::: The code at line 459 was first introduced by commit
:::::: 67086e32b56481531ab1292b284e074b1a8d764c powerpc/eeh: powerpc/eeh: Support error recovery for VF PE

:::::: TO: Wei Yang <[email protected]> :::::: CC: Michael Ellerman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Oliver O'Halloran

reviewer

2019-07-24 09:49 UTC #11 lore raw

Re: [PATCH v3 9/9] powerpc/eeh: Convert log messages to eeh_edev_* macros

Quoted 18 lines · kbuild test robot

Hi Sam,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.3-rc1 next-20190724]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sam-Bobroff/powerpc-64-Adjust-order-in-pcibios_init/20190724-134001
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=powerpc

Quoted 8 lines

If you fix the issue, kindly add following tag
Reported-by kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
arch/powerpc/kernel/eeh_driver.c: In function 'eeh_add_virt_device':
Quoted 1 line
Quoted 1 line
arch/powerpc/kernel/eeh_driver.c:459:17: error: unused variable 'pdn' [-Werror=unused-variable]
     struct pci_dn *pdn = eeh_dev_to_pdn(edev);

FYI this happens when CONFIG_IOV isn't set. Adding a __maybe_unused annotation fixes it.

Quoted 36 lines

^~~

cc1: all warnings being treated as errors

vim +/pdn +459 arch/powerpc/kernel/eeh_driver.c

77bd7415 arch/powerpc/platforms/pseries/eeh_driver.c Linas Vepstas 2005-11-03  454
bf773df9 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-09-12  455  static void *eeh_add_virt_device(struct eeh_dev *edev)
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  456  {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  457      struct pci_driver *driver;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  458      struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04 @459      struct pci_dn *pdn = eeh_dev_to_pdn(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  460
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  461      if (!(edev->physfn)) {
6dad7bbd arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2019-07-23  462              eeh_edev_warn(edev, "Not for VF\n");
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  463              return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  464      }
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  465
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  466      driver = eeh_pcid_get(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  467      if (driver) {
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  468              if (driver->err_handler) {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  469                      eeh_pcid_put(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  470                      return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  471              }
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  472              eeh_pcid_put(dev);
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  473      }
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  474

:::::: The code at line 459 was first introduced by commit
:::::: 67086e32b56481531ab1292b284e074b1a8d764c powerpc/eeh: powerpc/eeh: Support error recovery for VF PE

:::::: TO: Wei Yang <[email protected]> :::::: CC: Michael Ellerman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Sam Bobroff

author

2019-07-25 03:35 UTC #12 lore raw

Re: [PATCH v3 9/9] powerpc/eeh: Convert log messages to eeh_edev_* macros

Quoted 22 lines · Oliver O'Halloran

Quoted 21 lines · kbuild test robot
Hi Sam,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.3-rc1 next-20190724]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Sam-Bobroff/powerpc-64-Adjust-order-in-pcibios_init/20190724-134001
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag
Reported-by kbuild test robot <[email protected]>

Quoted 8 lines

Quoted 5 lines
All errors (new ones prefixed by >>):
arch/powerpc/kernel/eeh_driver.c: In function 'eeh_add_virt_device':
Quoted 1 line
Quoted 1 line
arch/powerpc/kernel/eeh_driver.c:459:17: error: unused variable 'pdn' [-Werror=unused-variable]
     struct pci_dn *pdn = eeh_dev_to_pdn(edev);
FYI this happens when CONFIG_IOV isn't set. Adding a __maybe_unused annotation fixes it.

Ah, thanks. This must be in eeh_add_virt_device().

Since there's now only a single use of pdn in that function, maybe we can remove the variable, and the IOV case can do this:

	pci_iov_add_virtfn(edev->physfn, eeh_dev_to_pdn(edev)->vf_index);

Quoted 37 lines

Quoted 36 lines

^~~

cc1: all warnings being treated as errors

vim +/pdn +459 arch/powerpc/kernel/eeh_driver.c

77bd7415 arch/powerpc/platforms/pseries/eeh_driver.c Linas Vepstas 2005-11-03  454
bf773df9 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-09-12  455  static void *eeh_add_virt_device(struct eeh_dev *edev)
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  456  {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  457      struct pci_driver *driver;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  458      struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04 @459      struct pci_dn *pdn = eeh_dev_to_pdn(edev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  460
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  461      if (!(edev->physfn)) {
6dad7bbd arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2019-07-23  462              eeh_edev_warn(edev, "Not for VF\n");
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  463              return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  464      }
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  465
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  466      driver = eeh_pcid_get(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  467      if (driver) {
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  468              if (driver->err_handler) {
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  469                      eeh_pcid_put(dev);
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  470                      return NULL;
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  471              }
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  472              eeh_pcid_put(dev);
46d4be41 arch/powerpc/kernel/eeh_driver.c            Sam Bobroff   2018-05-25  473      }
67086e32 arch/powerpc/kernel/eeh_driver.c            Wei Yang      2016-03-04  474

:::::: The code at line 459 was first introduced by commit
:::::: 67086e32b56481531ab1292b284e074b1a8d764c powerpc/eeh: powerpc/eeh: Support error recovery for VF PE

:::::: TO: Wei Yang <[email protected]> :::::: CC: Michael Ellerman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation