[PATCH v2 0/3] powerpc/pci: PCI slot unified reset

[PATCH v2 0/3] powerpc/pci: PCI slot unified reset

[Gavin Shan]

The patchset corresponds to skiboot changes, which manages PCI slots
in a unified way: OPAL APIs used to do slot reset, power management,
presence status retrival. The patchset shouldn't be merged before
the OPAL firmware counterpart is merged.

The kernel changes have been split into 2 parts: (A) Use the unified
PCI slot reset OPAL API - this patchset; (B) powernv-php driver to
support PCI hotplug for PowerNV platform, which will be sent separately.

This patchset affects EEH, PCI reset logics, which is used mainly by
VFIO. A couple of cases have been tested out on P7/P8 boxes and looks
good: EEH on PowerNV, EEH support for guest, PCI passthrou.

Changelog
=========
v1 -> v2
        * Keep opal_pci_reinit(). In case the slot is resetted by kernel,
          instead of skiboot, this API should be called to restore states
          for those affected devices.
        * Reworked slot ID scheme so that old/new kernel can work with
          skiboot with or without unified PCI slot management support.
        * Code cleanup here and there.

Gavin Shan (3):
  powerpc/powernv: Use PCI slot reset infrastructure
  powerpc/powernv: Refactor ioda_eeh_reset()
  powerpc/powernv: Issue fundamental reset if required

 arch/powerpc/include/asm/eeh.h            |   1 +
 arch/powerpc/include/asm/opal.h           |   9 +-
 arch/powerpc/platforms/powernv/eeh-ioda.c | 240 ++++++++++++++++--------------
 3 files changed, 132 insertions(+), 118 deletions(-)

Thanks,
Gavin

[PATCH 1/3] powerpc/powernv: Use PCI slot reset infrastructure

[Gavin Shan]

For PowerPC PowerNV platform, running on top of skiboot, all PE
level reset should be routed to firmware, which exported PCI slot
reset capability with device-node property "ibm,reset-by-firmware".
Otherwise, the kernel still has to simulate hot reset on PCI bridge's
secondary bus. So the code doesn't depend on if the firmware has
corresponding feature supported.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/eeh.h            |   1 +
 arch/powerpc/include/asm/opal.h           |   9 +-
 arch/powerpc/platforms/powernv/eeh-ioda.c | 163 ++++++++++++++++--------------
 3 files changed, 89 insertions(+), 84 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 9c11d1e..5847721 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -192,6 +192,7 @@ enum {
 #define EEH_RESET_DEACTIVATE	0	/* Deactivate the PE reset	*/
 #define EEH_RESET_HOT		1	/* Hot reset			*/
 #define EEH_RESET_FUNDAMENTAL	3	/* Fundamental reset		*/
+#define EEH_RESET_COMPLETE	4	/* PHB complete reset		*/
 #define EEH_LOG_TEMP		1	/* EEH temporary error log	*/
 #define EEH_LOG_PERM		2	/* EEH permanent error log	*/
 
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 9124b0e..edd1993 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -405,11 +405,6 @@ enum OpalPciResetState {
 	OPAL_ASSERT_RESET = 1
 };
 
-enum OpalPciMaskAction {
-	OPAL_UNMASK_ERROR_TYPE = 0,
-	OPAL_MASK_ERROR_TYPE = 1
-};
-
 enum OpalSlotLedType {
 	OPAL_SLOT_LED_ID_TYPE = 0,
 	OPAL_SLOT_LED_FAULT_TYPE = 1
@@ -906,7 +901,7 @@ int64_t opal_pci_map_pe_dma_window(uint64_t phb_id, uint16_t pe_number, uint16_t
 int64_t opal_pci_map_pe_dma_window_real(uint64_t phb_id, uint16_t pe_number,
 					uint16_t dma_window_number, uint64_t pci_start_addr,
 					uint64_t pci_mem_size);
-int64_t opal_pci_reset(uint64_t phb_id, uint8_t reset_scope, uint8_t assert_state);
+int64_t opal_pci_reset(uint64_t id, uint8_t reset_scope, uint8_t assert_state);
 
 int64_t opal_pci_get_hub_diag_data(uint64_t hub_id, void *diag_buffer,
 				   uint64_t diag_buffer_len);
@@ -922,7 +917,7 @@ int64_t opal_get_epow_status(__be64 *status);
 int64_t opal_set_system_attention_led(uint8_t led_action);
 int64_t opal_pci_next_error(uint64_t phb_id, __be64 *first_frozen_pe,
 			    __be16 *pci_error_type, __be16 *severity);
-int64_t opal_pci_poll(uint64_t phb_id);
+int64_t opal_pci_poll(uint64_t id, uint8_t *val);
 int64_t opal_return_cpu(void);
 int64_t opal_check_token(uint64_t token);
 int64_t opal_reinit_cpus(uint64_t flags);
diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 43aba2d..825da60 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -490,12 +490,12 @@ static int ioda_eeh_get_state(struct eeh_pe *pe)
 	return ioda_eeh_get_pe_state(pe);
 }
 
-static s64 ioda_eeh_phb_poll(struct pnv_phb *phb)
+static s64 ioda_eeh_poll(uint64_t id)
 {
 	s64 rc = OPAL_HARDWARE;
 
 	while (1) {
-		rc = opal_pci_poll(phb->opal_id);
+		rc = opal_pci_poll(id, NULL);
 		if (rc <= 0)
 			break;
 
@@ -511,84 +511,38 @@ static s64 ioda_eeh_phb_poll(struct pnv_phb *phb)
 int ioda_eeh_phb_reset(struct pci_controller *hose, int option)
 {
 	struct pnv_phb *phb = hose->private_data;
+	uint8_t scope;
 	s64 rc = OPAL_HARDWARE;
 
 	pr_debug("%s: Reset PHB#%x, option=%d\n",
 		 __func__, hose->global_number, option);
-
-	/* Issue PHB complete reset request */
-	if (option == EEH_RESET_FUNDAMENTAL ||
-	    option == EEH_RESET_HOT)
-		rc = opal_pci_reset(phb->opal_id,
-				OPAL_RESET_PHB_COMPLETE,
-				OPAL_ASSERT_RESET);
-	else if (option == EEH_RESET_DEACTIVATE)
-		rc = opal_pci_reset(phb->opal_id,
-				OPAL_RESET_PHB_COMPLETE,
-				OPAL_DEASSERT_RESET);
-	if (rc < 0)
-		goto out;
-
-	/*
-	 * Poll state of the PHB until the request is done
-	 * successfully. The PHB reset is usually PHB complete
-	 * reset followed by hot reset on root bus. So we also
-	 * need the PCI bus settlement delay.
-	 */
-	rc = ioda_eeh_phb_poll(phb);
-	if (option == EEH_RESET_DEACTIVATE) {
-		if (system_state < SYSTEM_RUNNING)
-			udelay(1000 * EEH_PE_RST_SETTLE_TIME);
-		else
-			msleep(EEH_PE_RST_SETTLE_TIME);
+	switch (option) {
+	case EEH_RESET_HOT:
+		scope = OPAL_RESET_PCI_HOT;
+		break;
+	case EEH_RESET_FUNDAMENTAL:
+		scope = OPAL_RESET_PCI_FUNDAMENTAL;
+		break;
+	case EEH_RESET_COMPLETE:
+		scope = OPAL_RESET_PHB_COMPLETE;
+		break;
+	case EEH_RESET_DEACTIVATE:
+		return 0;
+	default:
+		pr_warn("%s: Unsupported option %d\n",
+			__func__, option);
+		return -EINVAL;
 	}
-out:
-	if (rc != OPAL_SUCCESS)
-		return -EIO;
-
-	return 0;
-}
-
-static int ioda_eeh_root_reset(struct pci_controller *hose, int option)
-{
-	struct pnv_phb *phb = hose->private_data;
-	s64 rc = OPAL_SUCCESS;
 
-	pr_debug("%s: Reset PHB#%x, option=%d\n",
-		 __func__, hose->global_number, option);
+	/* Issue reset and poll until it's completed */
+	rc = opal_pci_reset(phb->opal_id, scope, OPAL_ASSERT_RESET);
+	if (rc > 0)
+		rc = ioda_eeh_poll(phb->opal_id);
 
-	/*
-	 * During the reset deassert time, we needn't care
-	 * the reset scope because the firmware does nothing
-	 * for fundamental or hot reset during deassert phase.
-	 */
-	if (option == EEH_RESET_FUNDAMENTAL)
-		rc = opal_pci_reset(phb->opal_id,
-				OPAL_RESET_PCI_FUNDAMENTAL,
-				OPAL_ASSERT_RESET);
-	else if (option == EEH_RESET_HOT)
-		rc = opal_pci_reset(phb->opal_id,
-				OPAL_RESET_PCI_HOT,
-				OPAL_ASSERT_RESET);
-	else if (option == EEH_RESET_DEACTIVATE)
-		rc = opal_pci_reset(phb->opal_id,
-				OPAL_RESET_PCI_HOT,
-				OPAL_DEASSERT_RESET);
-	if (rc < 0)
-		goto out;
-
-	/* Poll state of the PHB until the request is done */
-	rc = ioda_eeh_phb_poll(phb);
-	if (option == EEH_RESET_DEACTIVATE)
-		msleep(EEH_PE_RST_SETTLE_TIME);
-out:
-	if (rc != OPAL_SUCCESS)
-		return -EIO;
-
-	return 0;
+	return (rc == OPAL_SUCCESS) ? 0 : -EIO;
 }
 
-static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
+static int __ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
 
 {
 	struct device_node *dn = pci_device_to_OF_node(dev);
@@ -639,14 +593,57 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
 	return 0;
 }
 
+static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
+{
+	struct pci_controller *hose;
+	struct pnv_phb *phb;
+	struct device_node *dn = dev ? pci_device_to_OF_node(dev) : NULL;
+	uint64_t id = (0x1ul << 60);
+	uint8_t scope;
+	s64 rc;
+
+	/*
+	 * If the firmware can't handle it, we still need simulate a hot
+	 * reset on the secondary bus. It should be the rare case.
+	 */
+	if (!of_get_property(dn, "ibm,reset-by-firmware", NULL))
+		return __ioda_eeh_bridge_reset(dev, option);
+
+	/* The firmware can handle the request */
+	switch (option) {
+	case EEH_RESET_HOT:
+		scope = OPAL_RESET_PCI_HOT;
+		break;
+	case EEH_RESET_FUNDAMENTAL:
+		scope = OPAL_RESET_PCI_FUNDAMENTAL;
+		break;
+	case EEH_RESET_DEACTIVATE:
+		return 0;
+	case EEH_RESET_COMPLETE:
+	default:
+		pr_warn("%s: Unsupported option %d on device %s\n",
+			__func__, option, pci_name(dev));
+		return -EINVAL;
+	}
+
+	hose = pci_bus_to_host(dev->bus);
+	phb = hose->private_data;
+	id |= (dev->bus->number << 24) | (dev->devfn << 16) | phb->opal_id;
+	rc = opal_pci_reset(id, scope, OPAL_ASSERT_RESET);
+	if (rc > 0)
+		ioda_eeh_poll(id);
+
+	return (rc == OPAL_SUCCESS) ? 0 : -EIO;
+}
+
 void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
 {
 	struct pci_controller *hose;
 
 	if (pci_is_root_bus(dev->bus)) {
 		hose = pci_bus_to_host(dev->bus);
-		ioda_eeh_root_reset(hose, EEH_RESET_HOT);
-		ioda_eeh_root_reset(hose, EEH_RESET_DEACTIVATE);
+		ioda_eeh_phb_reset(hose, EEH_RESET_HOT);
+		ioda_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
 	} else {
 		ioda_eeh_bridge_reset(dev, EEH_RESET_HOT);
 		ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
@@ -686,7 +683,20 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
 	 * state explicitly after BAR restore.
 	 */
 	if (pe->type & EEH_PE_PHB) {
-		ret = ioda_eeh_phb_reset(hose, option);
+		switch (option) {
+		case EEH_RESET_HOT:
+		case EEH_RESET_FUNDAMENTAL:
+		case EEH_RESET_COMPLETE:
+			ret = ioda_eeh_phb_reset(hose, EEH_RESET_COMPLETE);
+			break;
+		case EEH_RESET_DEACTIVATE:
+			ret = 0;
+			break;
+		default:
+			ret = -EINVAL;
+		}
+
+		return ret;
 	} else {
 		struct pnv_phb *phb;
 		s64 rc;
@@ -714,9 +724,8 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
 		}
 
 		bus = eeh_pe_bus_get(pe);
-		if (pci_is_root_bus(bus) ||
-		    pci_is_root_bus(bus->parent))
-			ret = ioda_eeh_root_reset(hose, option);
+		if (pci_is_root_bus(bus))
+			ret = ioda_eeh_phb_reset(hose, option);
 		else
 			ret = ioda_eeh_bridge_reset(bus->self, option);
 	}
-- 
1.8.3.2

[PATCH 2/3] powerpc/powernv: Refactor ioda_eeh_reset()

[Gavin Shan]

The patch refactors ioda_eeh_reset() to eliminate unnecessary
nested if statements to improve code readability. Except printing
the PHB index in the error message when failing to clear PHB error
injection registers, no logic changed.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 65 ++++++++++++++-----------------
 1 file changed, 29 insertions(+), 36 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 825da60..78d94df 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -665,8 +665,8 @@ void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
 static int ioda_eeh_reset(struct eeh_pe *pe, int option)
 {
 	struct pci_controller *hose = pe->phb;
+	struct pnv_phb *phb = hose->private_data;
 	struct pci_bus *bus;
-	int ret;
 
 	/*
 	 * For PHB reset, we always have complete reset. For those PEs whose
@@ -687,50 +687,43 @@ static int ioda_eeh_reset(struct eeh_pe *pe, int option)
 		case EEH_RESET_HOT:
 		case EEH_RESET_FUNDAMENTAL:
 		case EEH_RESET_COMPLETE:
-			ret = ioda_eeh_phb_reset(hose, EEH_RESET_COMPLETE);
-			break;
+			return ioda_eeh_phb_reset(hose, EEH_RESET_COMPLETE);
 		case EEH_RESET_DEACTIVATE:
-			ret = 0;
-			break;
+			return 0;
 		default:
-			ret = -EINVAL;
+			return -EINVAL;
 		}
+	}
 
-		return ret;
-	} else {
-		struct pnv_phb *phb;
+	/*
+	 * The frozen PE might be caused by PAPR error injection
+	 * registers, which are expected to be cleared after hitting
+	 * frozen PE as stated in the hardware spec. Unfortunately,
+	 * that's not true on P7IOC. So we have to clear it manually
+	 * to avoid recursive EEH errors during recovery.
+	 */
+	if (phb->model == PNV_PHB_MODEL_P7IOC &&
+	    (option == EEH_RESET_HOT ||
+	     option == EEH_RESET_FUNDAMENTAL)) {
 		s64 rc;
 
-		/*
-		 * The frozen PE might be caused by PAPR error injection
-		 * registers, which are expected to be cleared after hitting
-		 * frozen PE as stated in the hardware spec. Unfortunately,
-		 * that's not true on P7IOC. So we have to clear it manually
-		 * to avoid recursive EEH errors during recovery.
-		 */
-		phb = hose->private_data;
-		if (phb->model == PNV_PHB_MODEL_P7IOC &&
-		    (option == EEH_RESET_HOT ||
-		    option == EEH_RESET_FUNDAMENTAL)) {
-			rc = opal_pci_reset(phb->opal_id,
-					    OPAL_RESET_PHB_ERROR,
-					    OPAL_ASSERT_RESET);
-			if (rc != OPAL_SUCCESS) {
-				pr_warn("%s: Failure %lld clearing "
-					"error injection registers\n",
-					__func__, rc);
-				return -EIO;
-			}
+		rc = opal_pci_reset(phb->opal_id,
+				    OPAL_RESET_PHB_ERROR,
+				    OPAL_ASSERT_RESET);
+		if (rc != OPAL_SUCCESS) {
+			pr_warn("%s: Failure %lld clearing PHB#%x "
+				"error injection registers\n",
+				__func__, rc, hose->global_number);
+			return -EIO;
 		}
-
-		bus = eeh_pe_bus_get(pe);
-		if (pci_is_root_bus(bus))
-			ret = ioda_eeh_phb_reset(hose, option);
-		else
-			ret = ioda_eeh_bridge_reset(bus->self, option);
 	}
 
-	return ret;
+	/* Route PE reset request */
+	bus = eeh_pe_bus_get(pe);
+	if (pci_is_root_bus(bus))
+		return ioda_eeh_phb_reset(hose, option);
+
+	return ioda_eeh_bridge_reset(bus->self, option);
 }
 
 /**
-- 
1.8.3.2

[PATCH 3/3] powerpc/powernv: Issue fundamental reset if required

[Gavin Shan]

Function pnv_pci_reset_secondary_bus() is used to reset specified
PCI bus, which is leaded by root complex or PCI bridge. That means
the function shouldn't be called on PCI root bus and the patch
removes the logic for the case.

Also, some adapters may require fundamental reset to reload their
firmwares. Otherwise, they will fail to load their firmwares and
those adapters can't work properly after reset, as being reported
in VFIO pass-through scenario. The patch checks the reset type
required by the child adapters of the PCI bus and issue fundamental
reset if necessary.

Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 34 +++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 78d94df..cf38781 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -636,18 +636,34 @@ static int ioda_eeh_bridge_reset(struct pci_dev *dev, int option)
 	return (rc == OPAL_SUCCESS) ? 0 : -EIO;
 }
 
-void pnv_pci_reset_secondary_bus(struct pci_dev *dev)
+static int pnv_pci_dev_reset_type(struct pci_dev *pdev, void *data)
 {
-	struct pci_controller *hose;
+	int *freset = data;
 
-	if (pci_is_root_bus(dev->bus)) {
-		hose = pci_bus_to_host(dev->bus);
-		ioda_eeh_phb_reset(hose, EEH_RESET_HOT);
-		ioda_eeh_phb_reset(hose, EEH_RESET_DEACTIVATE);
-	} else {
-		ioda_eeh_bridge_reset(dev, EEH_RESET_HOT);
-		ioda_eeh_bridge_reset(dev, EEH_RESET_DEACTIVATE);
+	/*
+	 * Stop the iteration immediately if any one PCI
+	 * device requires fundamental reset
+	 */
+	*freset |= pdev->needs_freset;
+	return *freset;
+}
+
+void pnv_pci_reset_secondary_bus(struct pci_dev *pdev)
+{
+	int option = EEH_RESET_HOT;
+	int freset = 0;
+
+	/* Check if we need issue fundamental reset */
+	if (pdev->subordinate) {
+		pci_walk_bus(pdev->subordinate,
+			     pnv_pci_dev_reset_type, &freset);
+		if (freset)
+			option = EEH_RESET_FUNDAMENTAL;
 	}
+
+	/* Issue required reset type */
+	ioda_eeh_bridge_reset(pdev, option);
+	ioda_eeh_bridge_reset(pdev, EEH_RESET_DEACTIVATE);
 }
 
 /**
-- 
1.8.3.2

Re: [PATCH v2 0/3] powerpc/pci: PCI slot unified reset

[Gavin Shan]

On Thu, Dec 04, 2014 at 04:50:50PM +1100, Gavin Shan wrote:

Please ignore this one. I'll rebase and send out a new revision
shortly.

Thanks,
Gavin

>The patchset corresponds to skiboot changes, which manages PCI slots
>in a unified way: OPAL APIs used to do slot reset, power management,
>presence status retrival. The patchset shouldn't be merged before
>the OPAL firmware counterpart is merged.
>
>The kernel changes have been split into 2 parts: (A) Use the unified
>PCI slot reset OPAL API - this patchset; (B) powernv-php driver to
>support PCI hotplug for PowerNV platform, which will be sent separately.
>
>This patchset affects EEH, PCI reset logics, which is used mainly by
>VFIO. A couple of cases have been tested out on P7/P8 boxes and looks
>good: EEH on PowerNV, EEH support for guest, PCI passthrou.
>
>Changelog
>=========
>v1 -> v2
>        * Keep opal_pci_reinit(). In case the slot is resetted by kernel,
>          instead of skiboot, this API should be called to restore states
>          for those affected devices.
>        * Reworked slot ID scheme so that old/new kernel can work with
>          skiboot with or without unified PCI slot management support.
>        * Code cleanup here and there.
>
>Gavin Shan (3):
>  powerpc/powernv: Use PCI slot reset infrastructure
>  powerpc/powernv: Refactor ioda_eeh_reset()
>  powerpc/powernv: Issue fundamental reset if required
>
> arch/powerpc/include/asm/eeh.h            |   1 +
> arch/powerpc/include/asm/opal.h           |   9 +-
> arch/powerpc/platforms/powernv/eeh-ioda.c | 240 ++++++++++++++++--------------
> 3 files changed, 132 insertions(+), 118 deletions(-)
>
>Thanks,
>Gavin
>