[PATCH v2 00/6] Follow-up fixes for EEH on PowerNV

[PATCH v2 00/6] Follow-up fixes for EEH on PowerNV

[Gavin Shan]

The series of patches are follow-up in order to make EEH workable for PowerNV
platform on Juno-IOC-L machine. Couple of issues have been fixed with help of
Ben:

	- Check PCIe link after PHB complete reset
	- Restore config space for bridges
	- The EEH address cache wasn't built successfully
	- Misc cleanup on output messages
	- Misc cleanup on EEH flags maintained by "struct pnv_phb"
	- Misc cleanup on properties of functions to avoid build warnings
 
The series of patches have been verified on Juno-IOC-L machine:

Trigger frozen PE:

        echo 0x0000000002000000 > /sys/kernel/debug/powerpc/PCI0000/err_injct
        sleep 1
        echo 0x0 > /sys/kernel/debug/powerpc/PCI0000/err_injct

Trigger fenced PHB:

	echo 0x8000000000000000 > /sys/kernel/debug/powerpc/PCI0000/err_injct


Changelog:

v1 -> v2:
	* Remove the mechanism to block PCI-CFG and MMIO.
	* Add one patch to do cleanup on output messages.
	* Add one patch to avoid build warnings.
	* Split functions to restore BARs for PCI devices and bridges separately.

---

arch/powerpc/include/asm/eeh.h            |    4 +-
arch/powerpc/kernel/eeh.c                 |   43 ++++++--
arch/powerpc/kernel/eeh_cache.c           |    4 +-
arch/powerpc/kernel/eeh_pe.c              |  157 ++++++++++++++++++++++++++---
arch/powerpc/platforms/powernv/eeh-ioda.c |   33 ++++---
arch/powerpc/platforms/powernv/pci-ioda.c |    1 +
arch/powerpc/platforms/powernv/pci.c      |    4 +-
arch/powerpc/platforms/powernv/pci.h      |    7 +-
8 files changed, 207 insertions(+), 46 deletions(-)

Thanks,
Gavin

[PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Gavin Shan]

When the PHB is fenced or dead, it's pointless to collect the data
from PCI config space of subordinate PCI devices since it should
return 0xFF's. It also has potential risk to incur additional errors.
The patch avoids collecting PCI-CFG data while PHB is in fenced or
dead state.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh.c |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 951a632..60deb42 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -232,16 +232,30 @@ void eeh_slot_error_detail(struct eeh_pe *pe, int severity)
 {
 	size_t loglen = 0;
 	struct eeh_dev *edev;
+	bool valid_cfg_log = true;
 
-	eeh_pci_enable(pe, EEH_OPT_THAW_MMIO);
-	eeh_ops->configure_bridge(pe);
-	eeh_pe_restore_bars(pe);
-
-	pci_regs_buf[0] = 0;
-	eeh_pe_for_each_dev(pe, edev) {
-		loglen += eeh_gather_pci_data(edev, pci_regs_buf,
-				EEH_PCI_REGS_LOG_LEN);
-        }
+	/*
+	 * When the PHB is fenced or dead, it's pointless to collect
+	 * the data from PCI config space because it should return
+	 * 0xFF's. For ER, we still retrieve the data from the PCI
+	 * config space.
+	 */
+	if (eeh_probe_mode_dev() &&
+	    (pe->type & EEH_PE_PHB) &&
+	    (pe->state & (EEH_PE_ISOLATED | EEH_PE_PHB_DEAD)))
+		valid_cfg_log = false;
+
+	if (valid_cfg_log) {
+		eeh_pci_enable(pe, EEH_OPT_THAW_MMIO);
+		eeh_ops->configure_bridge(pe);
+		eeh_pe_restore_bars(pe);
+
+		pci_regs_buf[0] = 0;
+		eeh_pe_for_each_dev(pe, edev) {
+			loglen += eeh_gather_pci_data(edev, pci_regs_buf,
+						      EEH_PCI_REGS_LOG_LEN);
+		}
+	}
 
 	eeh_ops->get_log(pe, severity, pci_regs_buf, loglen);
 }
-- 
1.7.5.4

[PATCH 2/6] powerpc/eeh: Check PCIe link after reset

[Gavin Shan]

After reset (e.g. complete reset) in order to bring the fenced PHB
back, the PCIe link might not be ready yet. The patch intends to
make sure the PCIe link is ready before accessing its subordinate
PCI devices. The patch also fixes that wrong values restored to
PCI_COMMAND register for PCI bridges.

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

diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 55943fc..016588a 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -22,6 +22,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/gfp.h>
 #include <linux/init.h>
@@ -567,30 +568,132 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state)
 	eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
 }
 
-/**
- * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
- * @data: EEH device
- * @flag: Unused
+/*
+ * Some PCI bridges (e.g. PLX bridges) have primary/secondary
+ * buses assigned explicitly by firmware, and we probably have
+ * lost that after reset. So we have to delay the check until
+ * the PCI-CFG registers have been restored for the parent
+ * bridge.
  *
- * Loads the PCI configuration space base address registers,
- * the expansion ROM base address, the latency timer, and etc.
- * from the saved values in the device node.
+ * Don't use normal PCI-CFG accessors, which probably has been
+ * blocked on normal path during the stage. So we need utilize
+ * eeh operations, which is always permitted.
  */
-static void *eeh_restore_one_device_bars(void *data, void *flag)
+static void eeh_bridge_check_link(struct pci_dev *pdev,
+				  struct device_node *dn)
+{
+	int cap;
+	uint32_t val;
+	int timeout = 0;
+
+	/*
+	 * We only check root port and downstream ports of
+	 * PCIe switches
+	 */
+	if (!pci_is_pcie(pdev) ||
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
+	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
+		return;
+
+	pr_debug("%s: Check PCIe link for %s ...\n",
+		 __func__, pci_name(pdev));
+
+	/* Check slot status */
+	cap = pdev->pcie_cap;
+	eeh_ops->read_config(dn, cap + PCI_EXP_SLTSTA, 2, &val);
+	if (!(val & PCI_EXP_SLTSTA_PDS)) {
+		pr_debug("  No card in the slot (0x%04x) !\n", val);
+		return;
+	}
+
+	/* Check power status if we have the capability */
+	eeh_ops->read_config(dn, cap + PCI_EXP_SLTCAP, 2, &val);
+	if (val & PCI_EXP_SLTCAP_PCP) {
+		eeh_ops->read_config(dn, cap + PCI_EXP_SLTCTL, 2, &val);
+		if (val & PCI_EXP_SLTCTL_PCC) {
+			pr_debug("  In power-off state, power it on ...\n");
+			val &= ~(PCI_EXP_SLTCTL_PCC | PCI_EXP_SLTCTL_PIC);
+			val |= (0x0100 & PCI_EXP_SLTCTL_PIC);
+			eeh_ops->write_config(dn, cap + PCI_EXP_SLTCTL, 2, val);
+			msleep(2 * 1000);
+		}
+	}
+
+	/* Enable link */
+	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCTL, 2, &val);
+	val &= ~PCI_EXP_LNKCTL_LD;
+	eeh_ops->write_config(dn, cap + PCI_EXP_LNKCTL, 2, val);
+
+	/* Check link */
+	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCAP, 4, &val);
+	if (!(val & PCI_EXP_LNKCAP_DLLLARC)) {
+		pr_debug("  No link reporting capability (0x%08x) \n", val);
+		msleep(1000);
+		return;
+	}
+
+	/* Wait the link is up until timeout (5s) */
+	timeout = 0;
+	while (timeout < 5000) {
+		msleep(20);
+		timeout += 20;
+
+		eeh_ops->read_config(dn, cap + PCI_EXP_LNKSTA, 2, &val);
+		if (val & PCI_EXP_LNKSTA_DLLLA)
+			break;
+	}
+
+	if (val & PCI_EXP_LNKSTA_DLLLA)
+		pr_debug("  Link up (%s)\n",
+			 (val & PCI_EXP_LNKSTA_CLS_2_5GB) ? "2.5GB" : "5GB");
+	else
+		pr_debug("  Link not ready (0x%04x)\n", val);
+}
+
+#define BYTE_SWAP(OFF)	(8*((OFF)/4)+3-(OFF))
+#define SAVED_BYTE(OFF)	(((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
+
+static void eeh_restore_bridge_bars(struct pci_dev *pdev,
+				    struct eeh_dev *edev,
+				    struct device_node *dn)
+{
+	int i;
+
+	/*
+	 * Device BARs: 0x10 - 0x18
+	 * Bus numbers and windows: 0x18 - 0x30
+	 */
+	for (i = 4; i < 13; i++)
+		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
+	/* Rom: 0x38 */
+	eeh_ops->write_config(dn, 14*4, 4, edev->config_space[14]);
+
+	/* Cache line & Latency timer: 0xC 0xD */
+	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
+                SAVED_BYTE(PCI_CACHE_LINE_SIZE));
+        eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
+                SAVED_BYTE(PCI_LATENCY_TIMER));
+	/* Max latency, min grant, interrupt ping and line: 0x3C */
+	eeh_ops->write_config(dn, 15*4, 4, edev->config_space[15]);
+
+	/* PCI Command: 0x4 */
+	eeh_ops->write_config(dn, PCI_COMMAND, 4, edev->config_space[1]);
+
+	/* Check the PCIe link is ready */
+	eeh_bridge_check_link(pdev, dn);
+}
+
+static void eeh_restore_device_bars(struct eeh_dev *edev,
+				    struct device_node *dn)
 {
 	int i;
 	u32 cmd;
-	struct eeh_dev *edev = (struct eeh_dev *)data;
-	struct device_node *dn = eeh_dev_to_of_node(edev);
 
 	for (i = 4; i < 10; i++)
 		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
 	/* 12 == Expansion ROM Address */
 	eeh_ops->write_config(dn, 12*4, 4, edev->config_space[12]);
 
-#define BYTE_SWAP(OFF) (8*((OFF)/4)+3-(OFF))
-#define SAVED_BYTE(OFF) (((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
-
 	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
 		SAVED_BYTE(PCI_CACHE_LINE_SIZE));
 	eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
@@ -613,6 +716,34 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
 	else
 		cmd &= ~PCI_COMMAND_SERR;
 	eeh_ops->write_config(dn, PCI_COMMAND, 4, cmd);
+}
+
+/**
+ * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
+ * @data: EEH device
+ * @flag: Unused
+ *
+ * Loads the PCI configuration space base address registers,
+ * the expansion ROM base address, the latency timer, and etc.
+ * from the saved values in the device node.
+ */
+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 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)
+		eeh_restore_bridge_bars(pdev, edev, dn);
+	else
+		eeh_restore_device_bars(edev, dn);
 
 	return NULL;
 }
-- 
1.7.5.4

[PATCH 3/6] powerpc/powernv: Replace variables with flags

[Gavin Shan]

We have 2 fields in "struct pnv_phb" to trace the states. The patch
replace the fields with one and introduces flags for that. The patch
doesn't impact the logic.

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c |    8 ++++----
 arch/powerpc/platforms/powernv/pci.c      |    4 ++--
 arch/powerpc/platforms/powernv/pci.h      |    7 +++++--
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 84f3036..85025d7 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -132,7 +132,7 @@ static int ioda_eeh_post_init(struct pci_controller *hose)
 					    &ioda_eeh_dbgfs_ops);
 #endif
 
-		phb->eeh_enabled = 1;
+		phb->eeh_state |= PNV_EEH_STATE_ENABLED;
 	}
 
 	return 0;
@@ -815,7 +815,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 		 * removed, we needn't take care of it any more.
 		 */
 		phb = hose->private_data;
-		if (phb->removed)
+		if (phb->eeh_state & PNV_EEH_STATE_REMOVED)
 			continue;
 
 		rc = opal_pci_next_error(phb->opal_id,
@@ -850,7 +850,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 				list_for_each_entry_safe(hose, tmp,
 						&hose_list, list_node) {
 					phb = hose->private_data;
-					phb->removed = 1;
+					phb->eeh_state |= PNV_EEH_STATE_REMOVED;
 				}
 
 				WARN(1, "EEH: dead IOC detected\n");
@@ -867,7 +867,7 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 
 				WARN(1, "EEH: dead PHB#%x detected\n",
 				     hose->global_number);
-				phb->removed = 1;
+				phb->eeh_state |= PNV_EEH_STATE_REMOVED;
 				ret = 3;
 				goto out;
 			} else if (severity == OPAL_EEH_SEV_PHB_FENCED) {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 6d9a506..1f31826 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -308,7 +308,7 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 	if (phb_pe && (phb_pe->state & EEH_PE_ISOLATED))
 		return PCIBIOS_SUCCESSFUL;
 
-	if (phb->eeh_enabled) {
+	if (phb->eeh_state & PNV_EEH_STATE_ENABLED) {
 		if (*val == EEH_IO_ERROR_VALUE(size)) {
 			busdn = pci_bus_to_OF_node(bus);
 			for (dn = busdn->child; dn; dn = dn->sibling) {
@@ -358,7 +358,7 @@ static int pnv_pci_write_config(struct pci_bus *bus,
 
 	/* Check if the PHB got frozen due to an error (no response) */
 #ifdef CONFIG_EEH
-	if (!phb->eeh_enabled)
+	if (!(phb->eeh_state & PNV_EEH_STATE_ENABLED))
 		pnv_pci_config_check_eeh(phb, bus, bdfn);
 #else
 	pnv_pci_config_check_eeh(phb, bus, bdfn);
diff --git a/arch/powerpc/platforms/powernv/pci.h b/arch/powerpc/platforms/powernv/pci.h
index 43906e3..40bdf02 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -78,6 +78,10 @@ struct pnv_eeh_ops {
 	int (*configure_bridge)(struct eeh_pe *pe);
 	int (*next_error)(struct eeh_pe **pe);
 };
+
+#define PNV_EEH_STATE_ENABLED	(1 << 0)	/* EEH enabled	*/
+#define PNV_EEH_STATE_REMOVED	(1 << 1)	/* PHB removed	*/
+
 #endif /* CONFIG_EEH */
 
 struct pnv_phb {
@@ -92,8 +96,7 @@ struct pnv_phb {
 
 #ifdef CONFIG_EEH
 	struct pnv_eeh_ops	*eeh_ops;
-	int			eeh_enabled;
-	int			removed;
+	int			eeh_state;
 #endif
 
 #ifdef CONFIG_DEBUG_FS
-- 
1.7.5.4

[PATCH 4/6] powerpc/eeh: Fix address catch for PowerNV

[Gavin Shan]

On the PowerNV platform, the EEH address cache isn't built correctly
because we skipped the EEH devices without binding PE. The patch
fixes that.

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

diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index 1d5d9a6..858ebea 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -194,7 +194,7 @@ static void __eeh_addr_cache_insert_dev(struct pci_dev *dev)
 	}
 
 	/* Skip any devices for which EEH is not enabled. */
-	if (!edev->pe) {
+	if (!eeh_probe_mode_dev() && !edev->pe) {
 #ifdef DEBUG
 		pr_info("PCI: skip building address cache for=%s - %s\n",
 			pci_name(dev), dn->full_name);
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index 3e5c3d5..0ff9a3a 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -998,6 +998,7 @@ static void pnv_pci_ioda_fixup(void)
 	pnv_pci_ioda_create_dbgfs();
 
 #ifdef CONFIG_EEH
+	eeh_probe_mode_set(EEH_PROBE_MODE_DEV);
 	eeh_addr_cache_build();
 	eeh_init();
 #endif
-- 
1.7.5.4

[PATCH 5/6] powerpc/eeh: Refactor the output message

[Gavin Shan]

We needn't the the whole backtrace other than one-line message in
the error reporting interrupt handler. For errors triggered by
access PCI config space or MMIO, we replace "WARN(1, ...)" with
pr_err() and dump_stack().

Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/eeh.c                 |    9 +++++++--
 arch/powerpc/platforms/powernv/eeh-ioda.c |   25 ++++++++++++++++---------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 60deb42..38e4b40 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -324,7 +324,9 @@ static int eeh_phb_check_failure(struct eeh_pe *pe)
 	eeh_serialize_unlock(flags);
 	eeh_send_failure_event(phb_pe);
 
-	WARN(1, "EEH: PHB failure detected\n");
+	pr_err("EEH: PHB#%x failure detected\n",
+		phb_pe->phb->global_number);
+	dump_stack();
 
 	return 1;
 out:
@@ -453,7 +455,10 @@ int eeh_dev_check_failure(struct eeh_dev *edev)
 	 * a stack trace will help the device-driver authors figure
 	 * out what happened.  So print that out.
 	 */
-	WARN(1, "EEH: failure detected\n");
+	pr_err("EEH: Frozen PE#%x detected on PHB#%x\n",
+		pe->addr, pe->phb->global_number);
+	dump_stack();
+
 	return 1;
 
 dn_unlock:
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 85025d7..0cd1c4a 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -853,11 +853,14 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 					phb->eeh_state |= PNV_EEH_STATE_REMOVED;
 				}
 
-				WARN(1, "EEH: dead IOC detected\n");
+				pr_err("EEH: dead IOC detected\n");
 				ret = 4;
 				goto out;
-			} else if (severity == OPAL_EEH_SEV_INF)
+			} else if (severity == OPAL_EEH_SEV_INF) {
+				pr_info("EEH: IOC informative error "
+					"detected\n");
 				ioda_eeh_hub_diag(hose);
+			}
 
 			break;
 		case OPAL_EEH_PHB_ERROR:
@@ -865,8 +868,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 				if (ioda_eeh_get_phb_pe(hose, pe))
 					break;
 
-				WARN(1, "EEH: dead PHB#%x detected\n",
-				     hose->global_number);
+				pr_err("EEH: dead PHB#%x detected\n",
+					hose->global_number);
 				phb->eeh_state |= PNV_EEH_STATE_REMOVED;
 				ret = 3;
 				goto out;
@@ -874,20 +877,24 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
 				if (ioda_eeh_get_phb_pe(hose, pe))
 					break;
 
-				WARN(1, "EEH: fenced PHB#%x detected\n",
-				     hose->global_number);
+				pr_err("EEH: fenced PHB#%x detected\n",
+					hose->global_number);
 				ret = 2;
 				goto out;
-			} else if (severity == OPAL_EEH_SEV_INF)
+			} else if (severity == OPAL_EEH_SEV_INF) {
+				pr_info("EEH: PHB#%x informative error "
+					"detected\n",
+					hose->global_number);
 				ioda_eeh_phb_diag(hose);
+			}
 
 			break;
 		case OPAL_EEH_PE_ERROR:
 			if (ioda_eeh_get_pe(hose, frozen_pe_no, pe))
 				break;
 
-			WARN(1, "EEH: Frozen PE#%x on PHB#%x detected\n",
-			     (*pe)->addr, (*pe)->phb->global_number);
+			pr_err("EEH: Frozen PE#%x on PHB#%x detected\n",
+				(*pe)->addr, (*pe)->phb->global_number);
 			ret = 1;
 			goto out;
 		}
-- 
1.7.5.4

[PATCH 6/6] powerpc/eeh: Avoid build warnings

[Gavin Shan]

The patch is for avoiding following build warnings:

   The function .pnv_pci_ioda_fixup() references
   the function __init .eeh_init().
   This is often because .pnv_pci_ioda_fixup lacks a __init

   The function .pnv_pci_ioda_fixup() references
   the function __init .eeh_addr_cache_build().
   This is often because .pnv_pci_ioda_fixup lacks a __init

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

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index dd65e31..09a8743 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -202,13 +202,13 @@ struct pci_bus *eeh_pe_bus_get(struct eeh_pe *pe);
 
 void *eeh_dev_init(struct device_node *dn, void *data);
 void eeh_dev_phb_init_dynamic(struct pci_controller *phb);
-int __init eeh_init(void);
+int eeh_init(void);
 int __init eeh_ops_register(struct eeh_ops *ops);
 int __exit eeh_ops_unregister(const char *name);
 unsigned long eeh_check_failure(const volatile void __iomem *token,
 				unsigned long val);
 int eeh_dev_check_failure(struct eeh_dev *edev);
-void __init eeh_addr_cache_build(void);
+void eeh_addr_cache_build(void);
 void eeh_add_device_tree_early(struct device_node *);
 void eeh_add_device_tree_late(struct pci_bus *);
 void eeh_add_sysfs_files(struct pci_bus *);
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 38e4b40..f055e6f 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -751,7 +751,7 @@ int __exit eeh_ops_unregister(const char *name)
  * Even if force-off is set, the EEH hardware is still enabled, so that
  * newer systems can boot.
  */
-int __init eeh_init(void)
+int eeh_init(void)
 {
 	struct pci_controller *hose, *tmp;
 	struct device_node *phb;
diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
index 858ebea..ea9a94c 100644
--- a/arch/powerpc/kernel/eeh_cache.c
+++ b/arch/powerpc/kernel/eeh_cache.c
@@ -285,7 +285,7 @@ void eeh_addr_cache_rmv_dev(struct pci_dev *dev)
  * Must be run late in boot process, after the pci controllers
  * have been scanned for devices (after all device resources are known).
  */
-void __init eeh_addr_cache_build(void)
+void eeh_addr_cache_build(void)
 {
 	struct device_node *dn;
 	struct eeh_dev *edev;
-- 
1.7.5.4

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Benjamin Herrenschmidt]

On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
> +       /*
> +        * When the PHB is fenced or dead, it's pointless to collect
> +        * the data from PCI config space because it should return
> +        * 0xFF's. For ER, we still retrieve the data from the PCI
> +        * config space.
> +        */
> +       if (eeh_probe_mode_dev() &&
> +           (pe->type & EEH_PE_PHB) &&
> +           (pe->state & (EEH_PE_ISOLATED | EEH_PE_PHB_DEAD)))
> +               valid_cfg_log = false;
> +

I'm still unsure about that one. EEH_PE_ISOLATED could be the result
of a normal ER of PE#0 (which can happen for various reasons other
than a fence) in which case the config space is available and
interesting.

I would either not bother and collect the FF's, or make this specific
to fence and only fence.

Cheers,
Ben.

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Benjamin Herrenschmidt]

On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
> +               pci_regs_buf[0] = 0;
> +               eeh_pe_for_each_dev(pe, edev) {
> +                       loglen += eeh_gather_pci_data(edev, pci_regs_buf,
> +                                                     EEH_PCI_REGS_LOG_LEN);
> +               }
> +       }

Unless I'm mistaken, this is buggy and will overwrite the content of
pci_regs_buf for every device (they will all write over the same
portion of the log).

Ben.

Re: [PATCH 2/6] powerpc/eeh: Check PCIe link after reset

[Benjamin Herrenschmidt]

On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
> After reset (e.g. complete reset) in order to bring the fenced PHB
> back, the PCIe link might not be ready yet. The patch intends to
> make sure the PCIe link is ready before accessing its subordinate
> PCI devices. The patch also fixes that wrong values restored to
> PCI_COMMAND register for PCI bridges.

This should also help if we end up doing a full reset for ER cases
right ?

IE, in a setup with PHB -> device (no switch), if the device driver
requests a fundamental reset, we should do a PERST at the PHB level (are
we ?) and thus restore things in a similar way.

> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/eeh_pe.c |  157 ++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 144 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
> index 55943fc..016588a 100644
> --- a/arch/powerpc/kernel/eeh_pe.c
> +++ b/arch/powerpc/kernel/eeh_pe.c
> @@ -22,6 +22,7 @@
>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/export.h>
>  #include <linux/gfp.h>
>  #include <linux/init.h>
> @@ -567,30 +568,132 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state)
>  	eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
>  }
>  
> -/**
> - * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
> - * @data: EEH device
> - * @flag: Unused
> +/*
> + * Some PCI bridges (e.g. PLX bridges) have primary/secondary
> + * buses assigned explicitly by firmware, and we probably have
> + * lost that after reset. So we have to delay the check until
> + * the PCI-CFG registers have been restored for the parent
> + * bridge.
>   *
> - * Loads the PCI configuration space base address registers,
> - * the expansion ROM base address, the latency timer, and etc.
> - * from the saved values in the device node.
> + * Don't use normal PCI-CFG accessors, which probably has been
> + * blocked on normal path during the stage. So we need utilize
> + * eeh operations, which is always permitted.
>   */
> -static void *eeh_restore_one_device_bars(void *data, void *flag)
> +static void eeh_bridge_check_link(struct pci_dev *pdev,
> +				  struct device_node *dn)
> +{
> +	int cap;
> +	uint32_t val;
> +	int timeout = 0;
> +
> +	/*
> +	 * We only check root port and downstream ports of
> +	 * PCIe switches
> +	 */
> +	if (!pci_is_pcie(pdev) ||
> +	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
> +	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
> +		return;
> +
> +	pr_debug("%s: Check PCIe link for %s ...\n",
> +		 __func__, pci_name(pdev));
> +
> +	/* Check slot status */
> +	cap = pdev->pcie_cap;
> +	eeh_ops->read_config(dn, cap + PCI_EXP_SLTSTA, 2, &val);
> +	if (!(val & PCI_EXP_SLTSTA_PDS)) {
> +		pr_debug("  No card in the slot (0x%04x) !\n", val);
> +		return;
> +	}
> +
> +	/* Check power status if we have the capability */
> +	eeh_ops->read_config(dn, cap + PCI_EXP_SLTCAP, 2, &val);
> +	if (val & PCI_EXP_SLTCAP_PCP) {
> +		eeh_ops->read_config(dn, cap + PCI_EXP_SLTCTL, 2, &val);
> +		if (val & PCI_EXP_SLTCTL_PCC) {
> +			pr_debug("  In power-off state, power it on ...\n");
> +			val &= ~(PCI_EXP_SLTCTL_PCC | PCI_EXP_SLTCTL_PIC);
> +			val |= (0x0100 & PCI_EXP_SLTCTL_PIC);
> +			eeh_ops->write_config(dn, cap + PCI_EXP_SLTCTL, 2, val);
> +			msleep(2 * 1000);
> +		}
> +	}
> +
> +	/* Enable link */
> +	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCTL, 2, &val);
> +	val &= ~PCI_EXP_LNKCTL_LD;
> +	eeh_ops->write_config(dn, cap + PCI_EXP_LNKCTL, 2, val);
> +
> +	/* Check link */
> +	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCAP, 4, &val);
> +	if (!(val & PCI_EXP_LNKCAP_DLLLARC)) {
> +		pr_debug("  No link reporting capability (0x%08x) \n", val);
> +		msleep(1000);
> +		return;
> +	}
> +
> +	/* Wait the link is up until timeout (5s) */
> +	timeout = 0;
> +	while (timeout < 5000) {
> +		msleep(20);
> +		timeout += 20;
> +
> +		eeh_ops->read_config(dn, cap + PCI_EXP_LNKSTA, 2, &val);
> +		if (val & PCI_EXP_LNKSTA_DLLLA)
> +			break;
> +	}
> +
> +	if (val & PCI_EXP_LNKSTA_DLLLA)
> +		pr_debug("  Link up (%s)\n",
> +			 (val & PCI_EXP_LNKSTA_CLS_2_5GB) ? "2.5GB" : "5GB");
> +	else
> +		pr_debug("  Link not ready (0x%04x)\n", val);
> +}
> +
> +#define BYTE_SWAP(OFF)	(8*((OFF)/4)+3-(OFF))
> +#define SAVED_BYTE(OFF)	(((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
> +
> +static void eeh_restore_bridge_bars(struct pci_dev *pdev,
> +				    struct eeh_dev *edev,
> +				    struct device_node *dn)
> +{
> +	int i;
> +
> +	/*
> +	 * Device BARs: 0x10 - 0x18
> +	 * Bus numbers and windows: 0x18 - 0x30
> +	 */
> +	for (i = 4; i < 13; i++)
> +		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
> +	/* Rom: 0x38 */
> +	eeh_ops->write_config(dn, 14*4, 4, edev->config_space[14]);
> +
> +	/* Cache line & Latency timer: 0xC 0xD */
> +	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
> +                SAVED_BYTE(PCI_CACHE_LINE_SIZE));
> +        eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
> +                SAVED_BYTE(PCI_LATENCY_TIMER));
> +	/* Max latency, min grant, interrupt ping and line: 0x3C */
> +	eeh_ops->write_config(dn, 15*4, 4, edev->config_space[15]);
> +
> +	/* PCI Command: 0x4 */
> +	eeh_ops->write_config(dn, PCI_COMMAND, 4, edev->config_space[1]);
> +
> +	/* Check the PCIe link is ready */
> +	eeh_bridge_check_link(pdev, dn);
> +}
> +
> +static void eeh_restore_device_bars(struct eeh_dev *edev,
> +				    struct device_node *dn)
>  {
>  	int i;
>  	u32 cmd;
> -	struct eeh_dev *edev = (struct eeh_dev *)data;
> -	struct device_node *dn = eeh_dev_to_of_node(edev);
>  
>  	for (i = 4; i < 10; i++)
>  		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
>  	/* 12 == Expansion ROM Address */
>  	eeh_ops->write_config(dn, 12*4, 4, edev->config_space[12]);
>  
> -#define BYTE_SWAP(OFF) (8*((OFF)/4)+3-(OFF))
> -#define SAVED_BYTE(OFF) (((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
> -
>  	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
>  		SAVED_BYTE(PCI_CACHE_LINE_SIZE));
>  	eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
> @@ -613,6 +716,34 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
>  	else
>  		cmd &= ~PCI_COMMAND_SERR;
>  	eeh_ops->write_config(dn, PCI_COMMAND, 4, cmd);
> +}
> +
> +/**
> + * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
> + * @data: EEH device
> + * @flag: Unused
> + *
> + * Loads the PCI configuration space base address registers,
> + * the expansion ROM base address, the latency timer, and etc.
> + * from the saved values in the device node.
> + */
> +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 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)
> +		eeh_restore_bridge_bars(pdev, edev, dn);
> +	else
> +		eeh_restore_device_bars(edev, dn);
>  
>  	return NULL;
>  }

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Gavin Shan]

On Tue, Jun 25, 2013 at 09:55:15PM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
>> +       /*
>> +        * When the PHB is fenced or dead, it's pointless to collect
>> +        * the data from PCI config space because it should return
>> +        * 0xFF's. For ER, we still retrieve the data from the PCI
>> +        * config space.
>> +        */
>> +       if (eeh_probe_mode_dev() &&
>> +           (pe->type & EEH_PE_PHB) &&
>> +           (pe->state & (EEH_PE_ISOLATED | EEH_PE_PHB_DEAD)))
>> +               valid_cfg_log = false;
>> +
>
>I'm still unsure about that one. EEH_PE_ISOLATED could be the result
>of a normal ER of PE#0 (which can happen for various reasons other
>than a fence) in which case the config space is available and
>interesting.
>

It's something like the followings. For ER on PE#0, we will have
PE with type of EEH_PE_BUS marked as isolated, instead of the
one with EEH_PE_PHB.


	[ EEH_PE_PHB] <---> [ EEH_PE_PHB] <---> [ EEH_PE_PHB]
				  |
			    [ EEH_PE_BUS ] PE#0
				  |
		        -------------------------
			|			|
		   [ EEH_PE_BUS ] PE#1	   [ EEH_PE_BUS] PE#2

>I would either not bother and collect the FF's, or make this specific
>to fence and only fence.
>

I'd like to keep it specific to fenced PHB and it's already be that :-)

Thanks,
Gavin

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Gavin Shan]

On Tue, Jun 25, 2013 at 09:56:06PM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
>> +               pci_regs_buf[0] = 0;
>> +               eeh_pe_for_each_dev(pe, edev) {
>> +                       loglen += eeh_gather_pci_data(edev, pci_regs_buf,
>> +                                                     EEH_PCI_REGS_LOG_LEN);
>> +               }
>> +       }
>
>Unless I'm mistaken, this is buggy and will overwrite the content of
>pci_regs_buf for every device (they will all write over the same
>portion of the log).
>

No, you're right. I'm going to fix it in next revision :-)

Thanks,
Gavin

Re: [PATCH 2/6] powerpc/eeh: Check PCIe link after reset

[Gavin Shan]

On Tue, Jun 25, 2013 at 09:58:40PM +1000, Benjamin Herrenschmidt wrote:
>On Tue, 2013-06-25 at 18:00 +0800, Gavin Shan wrote:
>> After reset (e.g. complete reset) in order to bring the fenced PHB
>> back, the PCIe link might not be ready yet. The patch intends to
>> make sure the PCIe link is ready before accessing its subordinate
>> PCI devices. The patch also fixes that wrong values restored to
>> PCI_COMMAND register for PCI bridges.
>
>This should also help if we end up doing a full reset for ER cases
>right ?
>
>IE, in a setup with PHB -> device (no switch), if the device driver
>requests a fundamental reset, we should do a PERST at the PHB level (are
>we ?) and thus restore things in a similar way.
>

Yes, you're right. We will do PERST if the device driver requests
fundamental reset on the root port. Otherwise, no matter which
type of reset (fundamental or hot) the device driver requests,
the upstream bridge got hot reset. Anyway, the piece of code
(checking PCIe link) is possiblly applied to ER case.

Thanks,
Gavin

>> Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/eeh_pe.c |  157 ++++++++++++++++++++++++++++++++++++++----
>>  1 files changed, 144 insertions(+), 13 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
>> index 55943fc..016588a 100644
>> --- a/arch/powerpc/kernel/eeh_pe.c
>> +++ b/arch/powerpc/kernel/eeh_pe.c
>> @@ -22,6 +22,7 @@
>>   * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
>>   */
>>  
>> +#include <linux/delay.h>
>>  #include <linux/export.h>
>>  #include <linux/gfp.h>
>>  #include <linux/init.h>
>> @@ -567,30 +568,132 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state)
>>  	eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
>>  }
>>  
>> -/**
>> - * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
>> - * @data: EEH device
>> - * @flag: Unused
>> +/*
>> + * Some PCI bridges (e.g. PLX bridges) have primary/secondary
>> + * buses assigned explicitly by firmware, and we probably have
>> + * lost that after reset. So we have to delay the check until
>> + * the PCI-CFG registers have been restored for the parent
>> + * bridge.
>>   *
>> - * Loads the PCI configuration space base address registers,
>> - * the expansion ROM base address, the latency timer, and etc.
>> - * from the saved values in the device node.
>> + * Don't use normal PCI-CFG accessors, which probably has been
>> + * blocked on normal path during the stage. So we need utilize
>> + * eeh operations, which is always permitted.
>>   */
>> -static void *eeh_restore_one_device_bars(void *data, void *flag)
>> +static void eeh_bridge_check_link(struct pci_dev *pdev,
>> +				  struct device_node *dn)
>> +{
>> +	int cap;
>> +	uint32_t val;
>> +	int timeout = 0;
>> +
>> +	/*
>> +	 * We only check root port and downstream ports of
>> +	 * PCIe switches
>> +	 */
>> +	if (!pci_is_pcie(pdev) ||
>> +	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
>> +	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
>> +		return;
>> +
>> +	pr_debug("%s: Check PCIe link for %s ...\n",
>> +		 __func__, pci_name(pdev));
>> +
>> +	/* Check slot status */
>> +	cap = pdev->pcie_cap;
>> +	eeh_ops->read_config(dn, cap + PCI_EXP_SLTSTA, 2, &val);
>> +	if (!(val & PCI_EXP_SLTSTA_PDS)) {
>> +		pr_debug("  No card in the slot (0x%04x) !\n", val);
>> +		return;
>> +	}
>> +
>> +	/* Check power status if we have the capability */
>> +	eeh_ops->read_config(dn, cap + PCI_EXP_SLTCAP, 2, &val);
>> +	if (val & PCI_EXP_SLTCAP_PCP) {
>> +		eeh_ops->read_config(dn, cap + PCI_EXP_SLTCTL, 2, &val);
>> +		if (val & PCI_EXP_SLTCTL_PCC) {
>> +			pr_debug("  In power-off state, power it on ...\n");
>> +			val &= ~(PCI_EXP_SLTCTL_PCC | PCI_EXP_SLTCTL_PIC);
>> +			val |= (0x0100 & PCI_EXP_SLTCTL_PIC);
>> +			eeh_ops->write_config(dn, cap + PCI_EXP_SLTCTL, 2, val);
>> +			msleep(2 * 1000);
>> +		}
>> +	}
>> +
>> +	/* Enable link */
>> +	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCTL, 2, &val);
>> +	val &= ~PCI_EXP_LNKCTL_LD;
>> +	eeh_ops->write_config(dn, cap + PCI_EXP_LNKCTL, 2, val);
>> +
>> +	/* Check link */
>> +	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCAP, 4, &val);
>> +	if (!(val & PCI_EXP_LNKCAP_DLLLARC)) {
>> +		pr_debug("  No link reporting capability (0x%08x) \n", val);
>> +		msleep(1000);
>> +		return;
>> +	}
>> +
>> +	/* Wait the link is up until timeout (5s) */
>> +	timeout = 0;
>> +	while (timeout < 5000) {
>> +		msleep(20);
>> +		timeout += 20;
>> +
>> +		eeh_ops->read_config(dn, cap + PCI_EXP_LNKSTA, 2, &val);
>> +		if (val & PCI_EXP_LNKSTA_DLLLA)
>> +			break;
>> +	}
>> +
>> +	if (val & PCI_EXP_LNKSTA_DLLLA)
>> +		pr_debug("  Link up (%s)\n",
>> +			 (val & PCI_EXP_LNKSTA_CLS_2_5GB) ? "2.5GB" : "5GB");
>> +	else
>> +		pr_debug("  Link not ready (0x%04x)\n", val);
>> +}
>> +
>> +#define BYTE_SWAP(OFF)	(8*((OFF)/4)+3-(OFF))
>> +#define SAVED_BYTE(OFF)	(((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
>> +
>> +static void eeh_restore_bridge_bars(struct pci_dev *pdev,
>> +				    struct eeh_dev *edev,
>> +				    struct device_node *dn)
>> +{
>> +	int i;
>> +
>> +	/*
>> +	 * Device BARs: 0x10 - 0x18
>> +	 * Bus numbers and windows: 0x18 - 0x30
>> +	 */
>> +	for (i = 4; i < 13; i++)
>> +		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
>> +	/* Rom: 0x38 */
>> +	eeh_ops->write_config(dn, 14*4, 4, edev->config_space[14]);
>> +
>> +	/* Cache line & Latency timer: 0xC 0xD */
>> +	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
>> +                SAVED_BYTE(PCI_CACHE_LINE_SIZE));
>> +        eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
>> +                SAVED_BYTE(PCI_LATENCY_TIMER));
>> +	/* Max latency, min grant, interrupt ping and line: 0x3C */
>> +	eeh_ops->write_config(dn, 15*4, 4, edev->config_space[15]);
>> +
>> +	/* PCI Command: 0x4 */
>> +	eeh_ops->write_config(dn, PCI_COMMAND, 4, edev->config_space[1]);
>> +
>> +	/* Check the PCIe link is ready */
>> +	eeh_bridge_check_link(pdev, dn);
>> +}
>> +
>> +static void eeh_restore_device_bars(struct eeh_dev *edev,
>> +				    struct device_node *dn)
>>  {
>>  	int i;
>>  	u32 cmd;
>> -	struct eeh_dev *edev = (struct eeh_dev *)data;
>> -	struct device_node *dn = eeh_dev_to_of_node(edev);
>>  
>>  	for (i = 4; i < 10; i++)
>>  		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
>>  	/* 12 == Expansion ROM Address */
>>  	eeh_ops->write_config(dn, 12*4, 4, edev->config_space[12]);
>>  
>> -#define BYTE_SWAP(OFF) (8*((OFF)/4)+3-(OFF))
>> -#define SAVED_BYTE(OFF) (((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
>> -
>>  	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
>>  		SAVED_BYTE(PCI_CACHE_LINE_SIZE));
>>  	eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
>> @@ -613,6 +716,34 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
>>  	else
>>  		cmd &= ~PCI_COMMAND_SERR;
>>  	eeh_ops->write_config(dn, PCI_COMMAND, 4, cmd);
>> +}
>> +
>> +/**
>> + * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
>> + * @data: EEH device
>> + * @flag: Unused
>> + *
>> + * Loads the PCI configuration space base address registers,
>> + * the expansion ROM base address, the latency timer, and etc.
>> + * from the saved values in the device node.
>> + */
>> +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 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)
>> +		eeh_restore_bridge_bars(pdev, edev, dn);
>> +	else
>> +		eeh_restore_device_bars(edev, dn);
>>  
>>  	return NULL;
>>  }
>
>

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Benjamin Herrenschmidt]

On Wed, 2013-06-26 at 07:49 +0800, Gavin Shan wrote:
> It's something like the followings. For ER on PE#0, we will have
> PE with type of EEH_PE_BUS marked as isolated, instead of the
> one with EEH_PE_PHB.
> 
> 
>         [ EEH_PE_PHB] <---> [ EEH_PE_PHB] <---> [ EEH_PE_PHB]
>                                   |
>                             [ EEH_PE_BUS ] PE#0
>                                   |

So we actually have two PEs here ? One real (PE#0) and one imaginary
(PHB PE) with no PE# associated ?

>                         -------------------------
>                         |                       |
>                    [ EEH_PE_BUS ] PE#1     [ EEH_PE_BUS] PE#2
> 
> >I would either not bother and collect the FF's, or make this specific
> >to fence and only fence.
> >
> 
> I'd like to keep it specific to fenced PHB and it's already be
> that :-)

Cheers,
Ben.

Re: [PATCH 1/6] powerpc/eeh: Don't collect PCI-CFG data on PHB

[Gavin Shan]

On Wed, Jun 26, 2013 at 09:57:26AM +1000, Benjamin Herrenschmidt wrote:
>On Wed, 2013-06-26 at 07:49 +0800, Gavin Shan wrote:
>> It's something like the followings. For ER on PE#0, we will have
>> PE with type of EEH_PE_BUS marked as isolated, instead of the
>> one with EEH_PE_PHB.
>> 
>> 
>>         [ EEH_PE_PHB] <---> [ EEH_PE_PHB] <---> [ EEH_PE_PHB]
>>                                   |
>>                             [ EEH_PE_BUS ] PE#0
>>                                   |
>
>So we actually have two PEs here ? One real (PE#0) and one imaginary
>(PHB PE) with no PE# associated ?
>

Yes, The (PHB PE) is actually a container to all PEs under the
PHB ;-)

>>                         -------------------------
>>                         |                       |
>>                    [ EEH_PE_BUS ] PE#1     [ EEH_PE_BUS] PE#2
>> 
>> >I would either not bother and collect the FF's, or make this specific
>> >to fence and only fence.
>> >
>> 
>> I'd like to keep it specific to fenced PHB and it's already be
>> that :-)

Thanks,
Gavin

[PATCH 2/6] powerpc/eeh: Check PCIe link after reset

[Gavin Shan]

After reset (e.g. complete reset) in order to bring the fenced PHB
back, the PCIe link might not be ready yet. The patch intends to
make sure the PCIe link is ready before accessing its subordinate
PCI devices. The patch also fixes that wrong values restored to
PCI_COMMAND register for PCI bridges.

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

diff --git a/arch/powerpc/kernel/eeh_pe.c b/arch/powerpc/kernel/eeh_pe.c
index 55943fc..016588a 100644
--- a/arch/powerpc/kernel/eeh_pe.c
+++ b/arch/powerpc/kernel/eeh_pe.c
@@ -22,6 +22,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
  */
 
+#include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/gfp.h>
 #include <linux/init.h>
@@ -567,30 +568,132 @@ void eeh_pe_state_clear(struct eeh_pe *pe, int state)
 	eeh_pe_traverse(pe, __eeh_pe_state_clear, &state);
 }
 
-/**
- * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
- * @data: EEH device
- * @flag: Unused
+/*
+ * Some PCI bridges (e.g. PLX bridges) have primary/secondary
+ * buses assigned explicitly by firmware, and we probably have
+ * lost that after reset. So we have to delay the check until
+ * the PCI-CFG registers have been restored for the parent
+ * bridge.
  *
- * Loads the PCI configuration space base address registers,
- * the expansion ROM base address, the latency timer, and etc.
- * from the saved values in the device node.
+ * Don't use normal PCI-CFG accessors, which probably has been
+ * blocked on normal path during the stage. So we need utilize
+ * eeh operations, which is always permitted.
  */
-static void *eeh_restore_one_device_bars(void *data, void *flag)
+static void eeh_bridge_check_link(struct pci_dev *pdev,
+				  struct device_node *dn)
+{
+	int cap;
+	uint32_t val;
+	int timeout = 0;
+
+	/*
+	 * We only check root port and downstream ports of
+	 * PCIe switches
+	 */
+	if (!pci_is_pcie(pdev) ||
+	    (pci_pcie_type(pdev) != PCI_EXP_TYPE_ROOT_PORT &&
+	     pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM))
+		return;
+
+	pr_debug("%s: Check PCIe link for %s ...\n",
+		 __func__, pci_name(pdev));
+
+	/* Check slot status */
+	cap = pdev->pcie_cap;
+	eeh_ops->read_config(dn, cap + PCI_EXP_SLTSTA, 2, &val);
+	if (!(val & PCI_EXP_SLTSTA_PDS)) {
+		pr_debug("  No card in the slot (0x%04x) !\n", val);
+		return;
+	}
+
+	/* Check power status if we have the capability */
+	eeh_ops->read_config(dn, cap + PCI_EXP_SLTCAP, 2, &val);
+	if (val & PCI_EXP_SLTCAP_PCP) {
+		eeh_ops->read_config(dn, cap + PCI_EXP_SLTCTL, 2, &val);
+		if (val & PCI_EXP_SLTCTL_PCC) {
+			pr_debug("  In power-off state, power it on ...\n");
+			val &= ~(PCI_EXP_SLTCTL_PCC | PCI_EXP_SLTCTL_PIC);
+			val |= (0x0100 & PCI_EXP_SLTCTL_PIC);
+			eeh_ops->write_config(dn, cap + PCI_EXP_SLTCTL, 2, val);
+			msleep(2 * 1000);
+		}
+	}
+
+	/* Enable link */
+	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCTL, 2, &val);
+	val &= ~PCI_EXP_LNKCTL_LD;
+	eeh_ops->write_config(dn, cap + PCI_EXP_LNKCTL, 2, val);
+
+	/* Check link */
+	eeh_ops->read_config(dn, cap + PCI_EXP_LNKCAP, 4, &val);
+	if (!(val & PCI_EXP_LNKCAP_DLLLARC)) {
+		pr_debug("  No link reporting capability (0x%08x) \n", val);
+		msleep(1000);
+		return;
+	}
+
+	/* Wait the link is up until timeout (5s) */
+	timeout = 0;
+	while (timeout < 5000) {
+		msleep(20);
+		timeout += 20;
+
+		eeh_ops->read_config(dn, cap + PCI_EXP_LNKSTA, 2, &val);
+		if (val & PCI_EXP_LNKSTA_DLLLA)
+			break;
+	}
+
+	if (val & PCI_EXP_LNKSTA_DLLLA)
+		pr_debug("  Link up (%s)\n",
+			 (val & PCI_EXP_LNKSTA_CLS_2_5GB) ? "2.5GB" : "5GB");
+	else
+		pr_debug("  Link not ready (0x%04x)\n", val);
+}
+
+#define BYTE_SWAP(OFF)	(8*((OFF)/4)+3-(OFF))
+#define SAVED_BYTE(OFF)	(((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
+
+static void eeh_restore_bridge_bars(struct pci_dev *pdev,
+				    struct eeh_dev *edev,
+				    struct device_node *dn)
+{
+	int i;
+
+	/*
+	 * Device BARs: 0x10 - 0x18
+	 * Bus numbers and windows: 0x18 - 0x30
+	 */
+	for (i = 4; i < 13; i++)
+		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
+	/* Rom: 0x38 */
+	eeh_ops->write_config(dn, 14*4, 4, edev->config_space[14]);
+
+	/* Cache line & Latency timer: 0xC 0xD */
+	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
+                SAVED_BYTE(PCI_CACHE_LINE_SIZE));
+        eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
+                SAVED_BYTE(PCI_LATENCY_TIMER));
+	/* Max latency, min grant, interrupt ping and line: 0x3C */
+	eeh_ops->write_config(dn, 15*4, 4, edev->config_space[15]);
+
+	/* PCI Command: 0x4 */
+	eeh_ops->write_config(dn, PCI_COMMAND, 4, edev->config_space[1]);
+
+	/* Check the PCIe link is ready */
+	eeh_bridge_check_link(pdev, dn);
+}
+
+static void eeh_restore_device_bars(struct eeh_dev *edev,
+				    struct device_node *dn)
 {
 	int i;
 	u32 cmd;
-	struct eeh_dev *edev = (struct eeh_dev *)data;
-	struct device_node *dn = eeh_dev_to_of_node(edev);
 
 	for (i = 4; i < 10; i++)
 		eeh_ops->write_config(dn, i*4, 4, edev->config_space[i]);
 	/* 12 == Expansion ROM Address */
 	eeh_ops->write_config(dn, 12*4, 4, edev->config_space[12]);
 
-#define BYTE_SWAP(OFF) (8*((OFF)/4)+3-(OFF))
-#define SAVED_BYTE(OFF) (((u8 *)(edev->config_space))[BYTE_SWAP(OFF)])
-
 	eeh_ops->write_config(dn, PCI_CACHE_LINE_SIZE, 1,
 		SAVED_BYTE(PCI_CACHE_LINE_SIZE));
 	eeh_ops->write_config(dn, PCI_LATENCY_TIMER, 1,
@@ -613,6 +716,34 @@ static void *eeh_restore_one_device_bars(void *data, void *flag)
 	else
 		cmd &= ~PCI_COMMAND_SERR;
 	eeh_ops->write_config(dn, PCI_COMMAND, 4, cmd);
+}
+
+/**
+ * eeh_restore_one_device_bars - Restore the Base Address Registers for one device
+ * @data: EEH device
+ * @flag: Unused
+ *
+ * Loads the PCI configuration space base address registers,
+ * the expansion ROM base address, the latency timer, and etc.
+ * from the saved values in the device node.
+ */
+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 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)
+		eeh_restore_bridge_bars(pdev, edev, dn);
+	else
+		eeh_restore_device_bars(edev, dn);
 
 	return NULL;
 }
-- 
1.7.5.4