[PATCH v1 0/7] SR-IOV Enablement on PowerVM

STALE3141d

Revision v1 of 4 in this series.

15 messages, 5 authors, 2017-12-29 · open the first message on its own page

[PATCH v1 0/7] SR-IOV Enablement on PowerVM

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:32

This patch series will enable SR-IOV on PowerVM. A specific set of
lids for PFW/PHYP is required. They are planned to release with
920 at the moment.

For IBM internal testers let me know of a system you want to test on
and we can put on the lids required or we can provide a system to run
the tests.

This patch depends on the three patches:
988fc3ba5653278a8c14d6ccf687371775930d2b
dae7253f9f78a731755ca20c66b2d2c40b86baea
608c0d8804ef3ca4cda8ec6ad914e47deb283d7b

v1 - Initial Patch
v2 - Addressed Alexey and Russell's comments

Bryant G. Ly (7):
  platform/pseries: Update VF config space after EEH
  powerpc/kernel: Add uevents in EEH error/resume
  platforms/pseries: Set eeh_pe of EEH_PE_VF type
  powerpc/kernel Add EEH operations to notify resume
  powerpc/kernel: Add EEH notify resume sysfs
  pseries/pci: Associate PEs to VFs in configure SR-IOV
  pseries/setup: Add Initialization of VF Bars

 arch/powerpc/include/asm/eeh.h               |   1 +
 arch/powerpc/include/asm/pci-bridge.h        |   5 +-
 arch/powerpc/include/asm/pci.h               |   2 +
 arch/powerpc/kernel/eeh_driver.c             |   9 +-
 arch/powerpc/kernel/eeh_sysfs.c              |  46 ++++++-
 arch/powerpc/kernel/pci_of_scan.c            |   2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c |   3 +-
 arch/powerpc/platforms/pseries/eeh_pseries.c | 192 ++++++++++++++++++++++++++-
 arch/powerpc/platforms/pseries/pci.c         | 156 +++++++++++++++++++++-
 arch/powerpc/platforms/pseries/setup.c       | 183 +++++++++++++++++++++++++
 10 files changed, 589 insertions(+), 10 deletions(-)

-- 
2.14.3 (Apple Git-98)

[PATCH v2 3/7] platforms/pseries: Set eeh_pe of EEH_PE_VF type

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:39

To correctly use EEH code one has to make
sure that the EEH_PE_VF is set for dynamic created
VFs. Therefore this patch allocates an eeh_pe of
eeh type EEH_PE_VF and associates PE with parent.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/include/asm/pci-bridge.h        |  5 ++++-
 arch/powerpc/platforms/pseries/eeh_pseries.c | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index 9f66ddebb799..16d70740a76f 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h

@@ -211,7 +211,10 @@ struct pci_dn {
 	unsigned int *pe_num_map;	/* PE# for the first VF PE or array */
 	bool    m64_single_mode;	/* Use M64 BAR in Single Mode */
 #define IODA_INVALID_M64        (-1)
-	int     (*m64_map)[PCI_SRIOV_NUM_BARS];
+	union {
+		int     (*m64_map)[PCI_SRIOV_NUM_BARS]; /*Only used in powernv */
+		int     last_allow_rc;	/* Only used in pSeries */
+	};
 #endif /* CONFIG_PCI_IOV */
 	int	mps;			/* Maximum Payload Size */
 	struct list_head child_list;

diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index a671ef4f57f5..c4bc5dd0c6ec 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c

@@ -58,6 +58,8 @@ static int ibm_configure_pe;
 void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
+	struct pci_dn *physfn_pdn;
+	struct eeh_dev *edev;
 
 	if (!pdev->is_virtfn)
 		return;

@@ -65,6 +67,14 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	pdn->device_id  =  pdev->device;
 	pdn->vendor_id  =  pdev->vendor;
 	pdn->class_code =  pdev->class;
+	/* Last allow unfreeze return code used for retrieval
+	 * by user space in eeh-sysfs to show the last command
+	 * completion from platform
+	 */
+	pdn->last_allow_rc =  0;
+	physfn_pdn      =  pci_get_pdn(pdev->physfn);
+	pdn->pe_number  =  physfn_pdn->pe_num_map[pdn->vf_index];
+	edev = pdn_to_eeh_dev(pdn);
 
 	/*
 	 * The following operations will fail if VF's sysfs files

@@ -72,8 +82,9 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
 	 */
 	eeh_add_device_early(pdn);
 	eeh_add_device_late(pdev);
+	edev->pe_config_addr =  (pdn->busno << 16) | (pdn->devfn << 8);
+	eeh_add_to_parent_pe(edev);
 	eeh_sysfs_add_device(pdev);
-
 }
 
 /*

-- 
2.14.3 (Apple Git-98)

[PATCH v2 4/7] powerpc/kernel Add EEH operations to notify resume

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:39

When pseries SR-IOV is enabled and after a PF driver
has resumed from EEH, platform has to be notified
of the event so the child VFs can be allowed to
resume their normal recovery path.

This patch makes the EEH operation allow unfreeze
platform dependent code and adds the call to
pseries EEH code.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/include/asm/eeh.h               |  1 +
 arch/powerpc/kernel/eeh_driver.c             |  3 +
 arch/powerpc/platforms/powernv/eeh-powernv.c |  3 +-
 arch/powerpc/platforms/pseries/eeh_pseries.c | 90 +++++++++++++++++++++++++++-
 4 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 82829c65f31a..fd37cc101f4f 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h

@@ -214,6 +214,7 @@ struct eeh_ops {
 	int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
 	int (*next_error)(struct eeh_pe **pe);
 	int (*restore_config)(struct pci_dn *pdn);
+	int (*notify_resume)(struct pci_dn *pdn);
 };
 
 extern int eeh_subsystem_flags;

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 9d4e8177c2e0..ed710076b042 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c

@@ -385,6 +385,9 @@ static void *eeh_report_resume(void *data, void *userdata)
 
 	eeh_pcid_put(dev);
 	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
+#ifdef CONFIG_PCI_IOV
+	eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
+#endif
 	return NULL;
 }

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 0665b6d03cb3..33c86c1a1720 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c

@@ -1704,7 +1704,8 @@ static struct eeh_ops pnv_eeh_ops = {
 	.read_config            = pnv_eeh_read_config,
 	.write_config           = pnv_eeh_write_config,
 	.next_error		= pnv_eeh_next_error,
-	.restore_config		= pnv_eeh_restore_config
+	.restore_config		= pnv_eeh_restore_config,
+	.notify_resume		= NULL
 };
 
 #ifdef CONFIG_PCI_IOV

diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index c4bc5dd0c6ec..1bae44f30b73 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c

@@ -743,6 +743,93 @@ static int pseries_eeh_restore_config(struct pci_dn *pdn)
 	return ret;
 }
 
+#ifdef CONFIG_PCI_IOV
+int pseries_send_allow_unfreeze(struct pci_dn *pdn,
+				u16 *vf_pe_array, int cur_vfs)
+{
+	int rc;
+	int ibm_allow_unfreeze = rtas_token("ibm,open-sriov-allow-unfreeze");
+	unsigned long buid, addr;
+
+	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
+	buid = pdn->phb->buid;
+	spin_lock(&rtas_data_buf_lock);
+	memcpy(rtas_data_buf, vf_pe_array, RTAS_DATA_BUF_SIZE);
+	rc = rtas_call(ibm_allow_unfreeze, 5, 1, NULL,
+		       addr,
+		       BUID_HI(buid),
+		       BUID_LO(buid),
+		       rtas_data_buf, cur_vfs * sizeof(u16));
+	spin_unlock(&rtas_data_buf_lock);
+	if (rc)
+		pr_warn("%s: Failed to allow unfreeze for PHB#%x-PE#%lx, rc=%x\n",
+			__func__,
+			pdn->phb->global_number, addr, rc);
+	return rc;
+}
+
+static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
+{
+	struct pci_dn *pdn, *tmp, *parent, *physfn_pdn;
+	int cur_vfs = 0, rc = 0, vf_index, bus, devfn;
+	u16 *vf_pe_array;
+
+	vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
+	if (!vf_pe_array)
+		return -ENOMEM;
+
+	if (edev->pdev->is_physfn) {
+		cur_vfs = pci_num_vf(edev->pdev);
+		pdn = eeh_dev_to_pdn(edev);
+		parent = pdn->parent;
+
+		for (vf_index = 0; vf_index < cur_vfs; vf_index++)
+			vf_pe_array[vf_index] =
+				cpu_to_be16(pdn->pe_num_map[vf_index]);
+		rc = pseries_send_allow_unfreeze(pdn, vf_pe_array, cur_vfs);
+		pdn->last_allow_rc = rc;
+		for (vf_index = 0; vf_index < cur_vfs; vf_index++) {
+			list_for_each_entry_safe(pdn, tmp, &parent->child_list,
+						 list) {
+				bus = pci_iov_virtfn_bus(edev->pdev, vf_index);
+				devfn = pci_iov_virtfn_devfn(edev->pdev,
+							     vf_index);
+				if (pdn->busno != bus ||
+				    pdn->devfn != devfn)
+					continue;
+				pdn->last_allow_rc = rc;
+			}
+		}
+	} else {
+		pdn = pci_get_pdn(edev->pdev);
+		vf_pe_array[0] = cpu_to_be16(pdn->pe_number);
+		physfn_pdn = pci_get_pdn(edev->physfn);
+		rc = pseries_send_allow_unfreeze(physfn_pdn, vf_pe_array, 1);
+		pdn->last_allow_rc = rc;
+	}
+
+	kfree(vf_pe_array);
+	return rc;
+}
+
+static int pseries_notify_resume(struct pci_dn *pdn)
+{
+	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+
+	if (!edev)
+		return -EEXIST;
+
+	if (rtas_token("ibm,open-sriov-allow-unfreeze")
+	    == RTAS_UNKNOWN_SERVICE)
+		return -EINVAL;
+
+	if (edev->pdev->is_physfn || edev->pdev->is_virtfn)
+		return pseries_call_allow_unfreeze(edev);
+
+	return 0;
+}
+#endif
+
 static struct eeh_ops pseries_eeh_ops = {
 	.name			= "pseries",
 	.init			= pseries_eeh_init,

@@ -758,7 +845,8 @@ static struct eeh_ops pseries_eeh_ops = {
 	.read_config		= pseries_eeh_read_config,
 	.write_config		= pseries_eeh_write_config,
 	.next_error		= NULL,
-	.restore_config		= pseries_eeh_restore_config
+	.restore_config		= pseries_eeh_restore_config,
+	.notify_resume		= pseries_notify_resume
 };
 
 /**

-- 
2.14.3 (Apple Git-98)

[PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:42

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/kernel/eeh_driver.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3c0fa99c5533..9d4e8177c2e0 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c

@@ -204,6 +204,7 @@ static void *eeh_report_error(void *data, void *userdata)
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	enum pci_ers_result rc, *res = userdata;
 	struct pci_driver *driver;
+	char *envp[] = {"EVENT=EEH_ERROR", "ONLINE=0", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -228,6 +229,7 @@ static void *eeh_report_error(void *data, void *userdata)
 
 	edev->in_error = true;
 	eeh_pcid_put(dev);
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	return NULL;
 }

@@ -358,6 +360,7 @@ static void *eeh_report_resume(void *data, void *userdata)
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	bool was_in_error;
 	struct pci_driver *driver;
+	char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -381,6 +384,7 @@ static void *eeh_report_resume(void *data, void *userdata)
 	driver->err_handler->resume(dev);
 
 	eeh_pcid_put(dev);
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	return NULL;
 }

@@ -397,6 +401,7 @@ static void *eeh_report_failure(void *data, void *userdata)
 	struct eeh_dev *edev = (struct eeh_dev *)data;
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	struct pci_driver *driver;
+	char * envp[] = {"EVENT=EEH_PERMANENT_FAILURE", "ONLINE=0", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -415,6 +420,7 @@ static void *eeh_report_failure(void *data, void *userdata)
 
 	driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);
 
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	eeh_pcid_put(dev);
 	return NULL;
 }

-- 
2.14.3 (Apple Git-98)

[PATCH v2 5/7] powerpc/kernel: Add EEH notify resume sysfs

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:43

Introduce a method for notify resume to be
called from sysfs. In this patch one can
now call notify resume from sysfs when
is supported by platform.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/kernel/eeh_sysfs.c | 46 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/eeh_sysfs.c b/arch/powerpc/kernel/eeh_sysfs.c
index 797549289798..344e7607f2e0 100644
--- a/arch/powerpc/kernel/eeh_sysfs.c
+++ b/arch/powerpc/kernel/eeh_sysfs.c

@@ -90,6 +90,38 @@ static ssize_t eeh_pe_state_store(struct device *dev,
 
 static DEVICE_ATTR_RW(eeh_pe_state);
 
+#ifdef CONFIG_PCI_IOV
+static ssize_t eeh_notify_resume_show(struct device *dev,
+				      struct device_attribute *attr, char *buf)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
+	struct pci_dn *pdn = pci_get_pdn(pdev);
+
+	if (!edev || !edev->pe)
+		return -ENODEV;
+
+	pdn = pci_get_pdn(pdev);
+	return sprintf(buf, "%d\n", pdn->last_allow_rc);
+}
+
+static ssize_t eeh_notify_resume_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
+
+	if (!edev || !edev->pe)
+		return -ENODEV;
+
+	if (eeh_ops->notify_resume(pci_get_pdn(pdev)))
+		return -EIO;
+	return count;
+}
+static DEVICE_ATTR_RW(eeh_notify_resume);
+#endif
+
 void eeh_sysfs_add_device(struct pci_dev *pdev)
 {
 	struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);

@@ -104,7 +136,13 @@ void eeh_sysfs_add_device(struct pci_dev *pdev)
 	rc += device_create_file(&pdev->dev, &dev_attr_eeh_mode);
 	rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
 	rc += device_create_file(&pdev->dev, &dev_attr_eeh_pe_state);
-
+#ifdef CONFIG_PCI_IOV
+	if (of_get_property(pci_device_to_OF_node
+			    ((pdev->is_physfn ? pdev : pdev->physfn)),
+			    "ibm,is-open-sriov-pf", NULL))
+		rc += device_create_file(&pdev->dev,
+					 &dev_attr_eeh_notify_resume);
+#endif
 	if (rc)
 		pr_warn("EEH: Unable to create sysfs entries\n");
 	else if (edev)

@@ -128,6 +166,12 @@ void eeh_sysfs_remove_device(struct pci_dev *pdev)
 	device_remove_file(&pdev->dev, &dev_attr_eeh_mode);
 	device_remove_file(&pdev->dev, &dev_attr_eeh_pe_config_addr);
 	device_remove_file(&pdev->dev, &dev_attr_eeh_pe_state);
+#ifdef CONFIG_PCI_IOV
+	if (of_get_property(pci_device_to_OF_node
+			    ((pdev->is_physfn ? pdev : pdev->physfn)),
+			    "ibm,is-open-sriov-pf", NULL))
+		device_remove_file(&pdev->dev, &dev_attr_eeh_notify_resume);
+#endif
 
 	if (edev)
 		edev->mode &= ~EEH_DEV_SYSFS;

-- 
2.14.3 (Apple Git-98)

[PATCH v2 1/7] platform/pseries: Update VF config space after EEH

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:43

Add EEH platform operations for pseries to update VF
config space. With this change after EEH, the VF
will have updated config space for pseries platform.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/include/asm/eeh.h               |  1 +
 arch/powerpc/kernel/eeh.c                    | 59 +++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/eeh-powernv.c | 65 ++--------------------------
 arch/powerpc/platforms/pseries/eeh_pseries.c | 26 ++++++++++-
 4 files changed, 88 insertions(+), 63 deletions(-)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 5161c37dd039..82829c65f31a 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h

@@ -297,6 +297,7 @@ int eeh_pe_reset(struct eeh_pe *pe, int option);
 int eeh_pe_configure(struct eeh_pe *pe);
 int eeh_pe_inject_err(struct eeh_pe *pe, int type, int func,
 		      unsigned long addr, unsigned long mask);
+int eeh_restore_vf_config(struct pci_dn *pdn);
 
 /**
  * EEH_POSSIBLE_ERROR() -- test for possible MMIO failure.

diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index cbca0a667682..cc649809885e 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c

@@ -740,6 +740,65 @@ static void *eeh_restore_dev_state(void *data, void *userdata)
 	return NULL;
 }
 
+int eeh_restore_vf_config(struct pci_dn *pdn)
+{
+	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+	u32 devctl, cmd, cap2, aer_capctl;
+	int old_mps;
+
+	if (edev->pcie_cap) {
+		/* Restore MPS */
+		old_mps = (ffs(pdn->mps) - 8) << 5;
+		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+				     2, &devctl);
+		devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
+		devctl |= old_mps;
+		eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+				      2, devctl);
+
+		/* Disable Completion Timeout */
+		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP2,
+				     4, &cap2);
+		if (cap2 & 0x10) {
+			eeh_ops->read_config(pdn,
+					     edev->pcie_cap + PCI_EXP_DEVCTL2,
+					     4, &cap2);
+			cap2 |= 0x10;
+			eeh_ops->write_config(pdn,
+					      edev->pcie_cap + PCI_EXP_DEVCTL2,
+					      4, cap2);
+		}
+	}
+
+	/* Enable SERR and parity checking */
+	eeh_ops->read_config(pdn, PCI_COMMAND, 2, &cmd);
+	cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
+	eeh_ops->write_config(pdn, PCI_COMMAND, 2, cmd);
+
+	/* Enable report various errors */
+	if (edev->pcie_cap) {
+		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+				     2, &devctl);
+		devctl &= ~PCI_EXP_DEVCTL_CERE;
+		devctl |= (PCI_EXP_DEVCTL_NFERE |
+			   PCI_EXP_DEVCTL_FERE |
+			   PCI_EXP_DEVCTL_URRE);
+		eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
+				      2, devctl);
+	}
+
+	/* Enable ECRC generation and check */
+	if (edev->pcie_cap && edev->aer_cap) {
+		eeh_ops->read_config(pdn, edev->aer_cap + PCI_ERR_CAP,
+				     4, &aer_capctl);
+		aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
+		eeh_ops->write_config(pdn, edev->aer_cap + PCI_ERR_CAP,
+				      4, aer_capctl);
+	}
+
+	return 0;
+}
+
 /**
  * pcibios_set_pcie_reset_state - Set PCI-E reset state
  * @dev: pci device struct

diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 961e64115d92..0665b6d03cb3 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c

@@ -1655,70 +1655,11 @@ static int pnv_eeh_next_error(struct eeh_pe **pe)
 	return ret;
 }
 
-static int pnv_eeh_restore_vf_config(struct pci_dn *pdn)
-{
-	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
-	u32 devctl, cmd, cap2, aer_capctl;
-	int old_mps;
-
-	if (edev->pcie_cap) {
-		/* Restore MPS */
-		old_mps = (ffs(pdn->mps) - 8) << 5;
-		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
-				     2, &devctl);
-		devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
-		devctl |= old_mps;
-		eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
-				      2, devctl);
-
-		/* Disable Completion Timeout */
-		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP2,
-				     4, &cap2);
-		if (cap2 & 0x10) {
-			eeh_ops->read_config(pdn,
-					     edev->pcie_cap + PCI_EXP_DEVCTL2,
-					     4, &cap2);
-			cap2 |= 0x10;
-			eeh_ops->write_config(pdn,
-					      edev->pcie_cap + PCI_EXP_DEVCTL2,
-					      4, cap2);
-		}
-	}
-
-	/* Enable SERR and parity checking */
-	eeh_ops->read_config(pdn, PCI_COMMAND, 2, &cmd);
-	cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
-	eeh_ops->write_config(pdn, PCI_COMMAND, 2, cmd);
-
-	/* Enable report various errors */
-	if (edev->pcie_cap) {
-		eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
-				     2, &devctl);
-		devctl &= ~PCI_EXP_DEVCTL_CERE;
-		devctl |= (PCI_EXP_DEVCTL_NFERE |
-			   PCI_EXP_DEVCTL_FERE |
-			   PCI_EXP_DEVCTL_URRE);
-		eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
-				      2, devctl);
-	}
-
-	/* Enable ECRC generation and check */
-	if (edev->pcie_cap && edev->aer_cap) {
-		eeh_ops->read_config(pdn, edev->aer_cap + PCI_ERR_CAP,
-				     4, &aer_capctl);
-		aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
-		eeh_ops->write_config(pdn, edev->aer_cap + PCI_ERR_CAP,
-				      4, aer_capctl);
-	}
-
-	return 0;
-}
-
 static int pnv_eeh_restore_config(struct pci_dn *pdn)
 {
 	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
 	struct pnv_phb *phb;
-	s64 ret;
+	s64 ret = 0;
 	int config_addr = (pdn->busno << 8) | (pdn->devfn);
 
 	if (!edev)

@@ -1732,7 +1673,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
 	 * to be exported by firmware in extendible way.
 	 */
 	if (edev->physfn) {
-		ret = pnv_eeh_restore_vf_config(pdn);
+		ret = eeh_restore_vf_config(pdn);
 	} else {
 		phb = pdn->phb->private_data;
 		ret = opal_pci_reinit(phb->opal_id,

@@ -1745,7 +1686,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
 		return -EIO;
 	}
 
-	return 0;
+	return ret;
 }
 
 static struct eeh_ops pnv_eeh_ops = {

diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 2295f117e2d3..a671ef4f57f5 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c

@@ -708,6 +708,30 @@ static int pseries_eeh_write_config(struct pci_dn *pdn, int where, int size, u32
 	return rtas_write_config(pdn, where, size, val);
 }
 
+static int pseries_eeh_restore_config(struct pci_dn *pdn)
+{
+	struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
+	s64 ret = 0;
+
+	if (!edev)
+		return -EEXIST;
+
+	/*
+	 * FIXME: The MPS, error routing rules, timeout setting are worthy
+	 * to be exported by firmware in extendible way.
+	 */
+	if (edev->physfn)
+		ret = eeh_restore_vf_config(pdn);
+
+	if (ret) {
+		pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
+			__func__, edev->pe_config_addr, ret);
+		return -EIO;
+	}
+
+	return ret;
+}
+
 static struct eeh_ops pseries_eeh_ops = {
 	.name			= "pseries",
 	.init			= pseries_eeh_init,

@@ -723,7 +747,7 @@ static struct eeh_ops pseries_eeh_ops = {
 	.read_config		= pseries_eeh_read_config,
 	.write_config		= pseries_eeh_write_config,
 	.next_error		= NULL,
-	.restore_config		= NULL
+	.restore_config		= pseries_eeh_restore_config
 };
 
 /**

-- 
2.14.3 (Apple Git-98)

[PATCH v2 6/7] pseries/pci: Associate PEs to VFs in configure SR-IOV

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:45

After initial validation of SR-IOV resources, firmware will
associate PEs to the dynamic VFs created within this call. This
patch adds the association of PEs to the PF array of PE numbers
indexed by VF.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/platforms/pseries/pci.c | 151 ++++++++++++++++++++++++++++++++++-
 1 file changed, 148 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 48d3af026f90..00e6475004ff 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c

@@ -57,18 +57,163 @@ void pcibios_name_device(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_name_device);
 #endif
-
 #ifdef CONFIG_PCI_IOV
+#define MAX_VFS_FOR_MAP_PE 256
+struct pe_map_bar_entry {
+	__be64     bar;       /* Input:  Virtual Function BAR */
+	__be16     rid;       /* Input:  Virtual Function Router ID */
+	__be16     pe_num;    /* Output: Virtual Function PE Number */
+	__be32     reserved;  /* Reserved Space */
+};
+
+int pseries_send_map_pe(struct pci_dev *pdev,
+			u16 num_vfs,
+			struct pe_map_bar_entry *vf_pe_array)
+{
+	struct pci_dn *pdn;
+	int rc;
+	unsigned long buid, addr;
+	int ibm_map_pes = rtas_token("ibm,open-sriov-map-pe-number");
+
+	if (ibm_map_pes == RTAS_UNKNOWN_SERVICE)
+		return -EINVAL;
+
+	pdn = pci_get_pdn(pdev);
+	addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
+	buid = pdn->phb->buid;
+	spin_lock(&rtas_data_buf_lock);
+	memcpy(rtas_data_buf, vf_pe_array,
+	       RTAS_DATA_BUF_SIZE);
+	rc = rtas_call(ibm_map_pes, 5, 1, NULL, addr,
+		       BUID_HI(buid), BUID_LO(buid),
+		       rtas_data_buf,
+		       num_vfs * sizeof(struct pe_map_bar_entry));
+	memcpy(vf_pe_array, rtas_data_buf, RTAS_DATA_BUF_SIZE);
+	spin_unlock(&rtas_data_buf_lock);
+
+	if (rc)
+		dev_err(&pdev->dev,
+			"%s: Failed to associate pes PE#%lx, rc=%x\n",
+			__func__,  addr, rc);
+
+	return rc;
+}
+
+void pseries_set_pe_num(struct pci_dev *pdev, u16 vf_index, __be16 pe_num)
+{
+	struct pci_dn *pdn;
+
+	pdn = pci_get_pdn(pdev);
+	pdn->pe_num_map[vf_index] = be16_to_cpu(pe_num);
+	dev_dbg(&pdev->dev, "VF %04x:%02x:%02x.%x associated with PE#%x\n",
+		pci_domain_nr(pdev->bus),
+		pdev->bus->number,
+		PCI_SLOT(pci_iov_virtfn_devfn(pdev, vf_index)),
+		PCI_FUNC(pci_iov_virtfn_devfn(pdev, vf_index)),
+		pdn->pe_num_map[vf_index]);
+}
+
+int pseries_associate_pes(struct pci_dev *pdev, u16 num_vfs)
+{
+	struct pci_dn *pdn;
+	int i, rc, vf_index;
+	struct pe_map_bar_entry *vf_pe_array;
+	struct resource *res;
+	u64 size;
+
+	vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
+	if (!vf_pe_array)
+		return -ENOMEM;
+
+	pdn = pci_get_pdn(pdev);
+	/* create firmware structure to associate pes */
+	for (vf_index = 0; vf_index < num_vfs; vf_index++) {
+		pdn->pe_num_map[vf_index] = IODA_INVALID_PE;
+		for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
+			res = &pdev->resource[i + PCI_IOV_RESOURCES];
+			if (!res->parent)
+				continue;
+			size = pcibios_iov_resource_alignment(pdev, i +
+					PCI_IOV_RESOURCES);
+			vf_pe_array[vf_index].bar =
+				cpu_to_be64(res->start + size * vf_index);
+			vf_pe_array[vf_index].rid =
+				cpu_to_be16((pci_iov_virtfn_bus(pdev, vf_index)
+					    << 8) | pci_iov_virtfn_devfn(pdev,
+					    vf_index));
+			vf_pe_array[vf_index].pe_num =
+				cpu_to_be16(IODA_INVALID_PE);
+		}
+	}
+
+	rc = pseries_send_map_pe(pdev, num_vfs, vf_pe_array);
+	/* Only zero is success */
+	if (!rc)
+		for (vf_index = 0; vf_index < num_vfs; vf_index++)
+			pseries_set_pe_num(pdev, vf_index,
+					   vf_pe_array[vf_index].pe_num);
+
+	kfree(vf_pe_array);
+	return rc;
+}
+
+int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
+{
+	struct pci_dn         *pdn;
+	int                    rc;
+	const int *max_vfs;
+	int max_config_vfs;
+	struct device_node *dn = pci_device_to_OF_node(pdev);
+
+	max_vfs = of_get_property(dn, "ibm,number-of-configurable-vfs", NULL);
+
+	if (!max_vfs)
+		return -EINVAL;
+
+	/* First integer stores max config */
+	max_config_vfs = of_read_number(&max_vfs[0], 1);
+	if (max_config_vfs < num_vfs && num_vfs > MAX_VFS_FOR_MAP_PE) {
+		dev_err(&pdev->dev,
+			"Num VFs %x > %x Configurable VFs\n",
+			num_vfs, (num_vfs > MAX_VFS_FOR_MAP_PE) ?
+			MAX_VFS_FOR_MAP_PE : max_config_vfs);
+		return -EINVAL;
+	}
+
+	pdn = pci_get_pdn(pdev);
+	pdn->pe_num_map = kmalloc_array(num_vfs,
+					sizeof(*pdn->pe_num_map),
+					GFP_KERNEL);
+	if (!pdn->pe_num_map)
+		return -ENOMEM;
+
+	rc = pseries_associate_pes(pdev, num_vfs);
+
+	/* Anything other than zero is failure */
+	if (rc) {
+		dev_err(&pdev->dev, "Failure to enable sriov: %x\n", rc);
+		kfree(pdn->pe_num_map);
+	} else {
+		pci_vf_drivers_autoprobe(pdev, false);
+	}
+
+	return rc;
+}
+
 int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 {
 	/* Allocate PCI data */
 	add_dev_pci_data(pdev);
-	pci_vf_drivers_autoprobe(pdev, false);
-	return 0;
+	return pseries_pci_sriov_enable(pdev, num_vfs);
 }
 
 int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
 {
+	struct pci_dn         *pdn;
+
+	pdn = pci_get_pdn(pdev);
+	/* Releasing pe_num_map */
+	kfree(pdn->pe_num_map);
 	/* Release PCI data */
 	remove_dev_pci_data(pdev);
 	pci_vf_drivers_autoprobe(pdev, true);

-- 
2.14.3 (Apple Git-98)

[PATCH v2 7/7] pseries/setup: Add Initialization of VF Bars

From: Bryant G. Ly <hidden>
Date: 2017-12-18 22:38:46

When enabling SR-IOV in pseries platform,
the VF bar properties for a PF are reported on
the device node in the device tree.

This patch adds the IOV Bar resources to Linux
structures from the device tree for later use
when configuring SR-IOV by PF driver.

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/include/asm/pci.h         |   2 +
 arch/powerpc/kernel/pci_of_scan.c      |   2 +-
 arch/powerpc/platforms/pseries/setup.c | 167 +++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
index 8dc32eacc97c..d82802ff5088 100644
--- a/arch/powerpc/include/asm/pci.h
+++ b/arch/powerpc/include/asm/pci.h

@@ -121,6 +121,8 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
 extern struct pci_dev *of_create_pci_dev(struct device_node *node,
 					struct pci_bus *bus, int devfn);
 
+extern unsigned int pci_parse_of_flags(u32 addr0, int bridge);
+
 extern void of_scan_pci_bridge(struct pci_dev *dev);
 
 extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);

diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c
index 0d790f8432d2..20ceec4a5f5e 100644
--- a/arch/powerpc/kernel/pci_of_scan.c
+++ b/arch/powerpc/kernel/pci_of_scan.c

@@ -38,7 +38,7 @@ static u32 get_int_prop(struct device_node *np, const char *name, u32 def)
  * @addr0: value of 1st cell of a device tree PCI address.
  * @bridge: Set this flag if the address is from a bridge 'ranges' property
  */
-static unsigned int pci_parse_of_flags(u32 addr0, int bridge)
+unsigned int pci_parse_of_flags(u32 addr0, int bridge)
 {
 	unsigned int flags = 0;

diff --git a/arch/powerpc/platforms/pseries/setup.c b/arch/powerpc/platforms/pseries/setup.c
index 5f1beb8367ac..cbb716315070 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c

@@ -459,6 +459,165 @@ static void __init find_and_init_phbs(void)
 	of_pci_check_probe_only();
 }
 
+#ifdef CONFIG_PCI_IOV
+enum rtas_iov_fw_value_map {
+	NUM_RES_PROPERTY  = 0, ///< Number of Resources
+	LOW_INT           = 1, ///< Lowest 32 bits of Address
+	START_OF_ENTRIES  = 2, ///< Always start of entry
+	APERTURE_PROPERTY = 2, ///< Start of entry+ to  Aperture Size
+	WDW_SIZE_PROPERTY = 4, ///< Start of entry+ to Window Size
+	NEXT_ENTRY        = 7  ///< Go to next entry on array
+};
+
+enum get_iov_fw_value_index {
+	BAR_ADDRS     = 1,    ///<  Get Bar Address
+	APERTURE_SIZE = 2,    ///<  Get Aperture Size
+	WDW_SIZE      = 3     ///<  Get Window Size
+};
+
+resource_size_t pseries_get_iov_fw_value(struct pci_dev *dev, int resno,
+					 enum get_iov_fw_value_index value)
+{
+	const int *indexes;
+	struct device_node *dn = pci_device_to_OF_node(dev);
+	int i, num_res, ret = 0;
+
+	indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
+	if (!indexes)
+		return  0;
+
+	/*
+	 * First element in the array is the number of Bars
+	 * returned.  Search through the list to find the matching
+	 * bar
+	 */
+	num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
+	if (resno >= num_res)
+		return 0; /* or an errror */
+
+	i = START_OF_ENTRIES + NEXT_ENTRY * resno;
+	switch (value) {
+	case BAR_ADDRS:
+		ret = of_read_number(&indexes[i], 2);
+		break;
+	case APERTURE_SIZE:
+		ret = of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
+		break;
+	case WDW_SIZE:
+		ret = of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
+		break;
+	}
+
+	return ret;
+}
+
+void of_pci_set_vf_bar_size(struct pci_dev *dev, const int *indexes)
+{
+	struct resource *res;
+	resource_size_t base, size;
+	int i, r, num_res;
+
+	num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
+	num_res = min_t(int, num_res, PCI_SRIOV_NUM_BARS);
+	for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PCI_SRIOV_NUM_BARS;
+	     i += NEXT_ENTRY, r++) {
+		res = &dev->resource[r + PCI_IOV_RESOURCES];
+		base = of_read_number(&indexes[i], 2);
+		size = of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
+		res->flags = pci_parse_of_flags(of_read_number
+						(&indexes[i + LOW_INT], 1), 0);
+		res->flags |= (IORESOURCE_MEM_64 | IORESOURCE_PCI_FIXED);
+		res->name = pci_name(dev);
+		res->start = base;
+		res->end = base + size - 1;
+	}
+}
+
+void of_pci_parse_iov_addrs(struct pci_dev *dev, const int *indexes)
+{
+	struct resource *res, *root, *conflict;
+	resource_size_t base, size;
+	int i, r, num_res;
+
+	/*
+	 * First element in the array is the number of Bars
+	 * returned.  Search through the list to find the matching
+	 * bars assign them from firmware into resources structure.
+	 */
+	num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
+	for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PCI_SRIOV_NUM_BARS;
+	     i += NEXT_ENTRY, r++) {
+		res = &dev->resource[r + PCI_IOV_RESOURCES];
+		base = of_read_number(&indexes[i], 2);
+		size = of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
+		res->name = pci_name(dev);
+		res->start = base;
+		res->end = base + size - 1;
+		root = pci_find_parent_resource(dev, res);
+
+		if (!root)
+			root = &iomem_resource;
+		dev_dbg(&dev->dev,
+			"pSeries IOV BAR %d: trying firmware assignment %pR\n",
+			 r + PCI_IOV_RESOURCES, res);
+		conflict = request_resource_conflict(root, res);
+		if (conflict) {
+			dev_info(&dev->dev,
+				 "BAR %d: %pR conflicts with %s %pR\n",
+				 r + PCI_IOV_RESOURCES, res,
+				 conflict->name, conflict);
+			res->flags |= IORESOURCE_UNSET;
+		}
+	}
+}
+
+static void pseries_pci_fixup_resources(struct pci_dev *pdev)
+{
+	const int *indexes;
+	struct device_node *dn = pci_device_to_OF_node(pdev);
+
+	/*Firmware must support open sriov otherwise dont configure*/
+	indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
+	if (!indexes)
+		return;
+	/* Assign the addresses from device tree*/
+	of_pci_set_vf_bar_size(pdev, indexes);
+}
+
+static void pseries_pci_fixup_iov_resources(struct pci_dev *pdev)
+{
+	const int *indexes;
+	struct device_node *dn = pci_device_to_OF_node(pdev);
+
+	if (!pdev->is_physfn || pdev->is_added)
+		return;
+	/*Firmware must support open sriov otherwise dont configure*/
+	indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
+	if (!indexes)
+		return;
+	/* Assign the addresses from device tree*/
+	of_pci_parse_iov_addrs(pdev, indexes);
+}
+
+static resource_size_t pseries_pci_iov_resource_alignment(struct pci_dev *pdev,
+							  int resno)
+{
+	const __be32 *reg;
+	struct device_node *dn = pci_device_to_OF_node(pdev);
+
+	/*Firmware must support open sriov otherwise report regular alignment*/
+	reg = of_get_property(dn, "ibm,is-open-sriov-pf", NULL);
+	if (!reg)
+		return pci_iov_resource_size(pdev, resno);
+
+	if (!pdev->is_physfn)
+		return 0;
+	return pseries_get_iov_fw_value(pdev,
+					resno - PCI_IOV_RESOURCES,
+					APERTURE_SIZE);
+}
+#endif
+
 static void __init pSeries_setup_arch(void)
 {
 	set_arch_panic_timeout(10, ARCH_PANIC_TIMEOUT);

@@ -490,6 +649,14 @@ static void __init pSeries_setup_arch(void)
 		vpa_init(boot_cpuid);
 		ppc_md.power_save = pseries_lpar_idle;
 		ppc_md.enable_pmcs = pseries_lpar_enable_pmcs;
+#ifdef CONFIG_PCI_IOV
+		ppc_md.pcibios_fixup_resources =
+			pseries_pci_fixup_resources;
+		ppc_md.pcibios_fixup_sriov =
+			pseries_pci_fixup_iov_resources;
+		ppc_md.pcibios_iov_resource_alignment =
+			pseries_pci_iov_resource_alignment;
+#endif
 	} else {
 		/* No special idle routine */
 		ppc_md.enable_pmcs = power4_enable_pmcs;

-- 
2.14.3 (Apple Git-98)

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2017-12-19 04:50:14

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

From a very quick look, I only see a few uevents even mentioned in

drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents in
the PCI core.  That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

quoted hunk

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>
---
 arch/powerpc/kernel/eeh_driver.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3c0fa99c5533..9d4e8177c2e0 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c

@@ -204,6 +204,7 @@ static void *eeh_report_error(void *data, void *userdata)
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	enum pci_ers_result rc, *res = userdata;
 	struct pci_driver *driver;
+	char *envp[] = {"EVENT=EEH_ERROR", "ONLINE=0", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -228,6 +229,7 @@ static void *eeh_report_error(void *data, void *userdata)
 
 	edev->in_error = true;
 	eeh_pcid_put(dev);
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	return NULL;
 }

@@ -358,6 +360,7 @@ static void *eeh_report_resume(void *data, void *userdata)
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	bool was_in_error;
 	struct pci_driver *driver;
+	char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -381,6 +384,7 @@ static void *eeh_report_resume(void *data, void *userdata)
 	driver->err_handler->resume(dev);
 
 	eeh_pcid_put(dev);
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	return NULL;
 }

@@ -397,6 +401,7 @@ static void *eeh_report_failure(void *data, void *userdata)
 	struct eeh_dev *edev = (struct eeh_dev *)data;
 	struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
 	struct pci_driver *driver;
+	char * envp[] = {"EVENT=EEH_PERMANENT_FAILURE", "ONLINE=0", NULL};
 
 	if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
 		return NULL;

@@ -415,6 +420,7 @@ static void *eeh_report_failure(void *data, void *userdata)
 
 	driver->err_handler->error_detected(dev, pci_channel_io_perm_failure);
 
+	kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
 	eeh_pcid_put(dev);
 	return NULL;
 }

-- 
2.14.3 (Apple Git-98)

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Russell Currey <hidden>
Date: 2017-12-19 04:59:49

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

quoted

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

From a very quick look, I only see a few uevents even mentioned in
drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents
in
the PCI core.  That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

I don't think this needs to be particularly complex, could we get away
with events for when devices do the following?

- begin recovery
- successfully recover
- fail recovery

It might be worthwhile sorting out some consistent, non-EEH-specific
naming, and then other device error recovery systems can do the same
later.

- Russell

quoted

Signed-off-by: Bryant G. Ly <redacted>
Signed-off-by: Juan J. Alvarez <redacted>

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2017-12-19 06:28:26

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

quoted

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

quoted

From a very quick look, I only see a few uevents even mentioned in

drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents in
the PCI core.  That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

We also need to be careful about what specific EEH activity we are
talking about, and if we bring into the picture things like DPDK, it
gets even more murky...

The basic way EEH is supposed to work for recovery (minus all sort of
implementation nasties which hopefully Russell and Sam are trying to
cleanup and fix) is that either:

	- The driver of the device has recovery callbacks, in which
case the driver participates in the recovery process, the device
doesn't "go away" (though it shouldn't be accessed during that process
by other entities, userspace originated config space could be a problem
and needs to be blocked...). The recovery typically involves a reset of
the device but in sync with the driver.

	- The driver doesn't have the callbacks. In this case, we
simulate an unplug, reset the device, and replug.

So it makes sense for the second case to emit the same uevents as a
normal PCI(e) hotplug.

For the former case I'm less sure.... Do we really need userspace to be
notified ? If yes, what for precisely ?

Cheers,
Ben.

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Juan Alvarez <hidden>
Date: 2017-12-21 03:04:23

On 12/18/17 10:59 PM, Russell Currey wrote:

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:

quoted

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

quoted

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

From a very quick look, I only see a few uevents even mentioned in
drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents
in
the PCI core.

The only place where I see the KOBJ_REMOVE being used is when the device is 
removed in pci_destroy_dev -> device_del whic will be called implicitly
in permanent failure path of EEH code

quoted

That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

I don't think this needs to be particularly complex, could we get away
with events for when devices do the following?

- begin recovery
- successfully recover
- fail recovery

If there are no objections in the on going review of this patch 
I can change them to these names:

  - BEGIN_RECOVERY
  - SUCCESSFUL_RECOVERY
  - FAILED_RECOVERY

It might be worthwhile sorting out some consistent, non-EEH-specific
naming, and then other device error recovery systems can do the same
later.

Do you have a more consistent naming in mind for these events? 

- Juan

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Juan Alvarez <hidden>
Date: 2017-12-21 03:04:39

On 12/19/17 12:27 AM, Benjamin Herrenschmidt wrote:

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:

quoted

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

quoted

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

quoted

From a very quick look, I only see a few uevents even mentioned in

drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents in
the PCI core.  That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

We also need to be careful about what specific EEH activity we are
talking about, and if we bring into the picture things like DPDK, it
gets even more murky...

The basic way EEH is supposed to work for recovery (minus all sort of
implementation nasties which hopefully Russell and Sam are trying to
cleanup and fix) is that either:

	- The driver of the device has recovery callbacks, in which
case the driver participates in the recovery process, the device
doesn't "go away" (though it shouldn't be accessed during that process
by other entities, userspace originated config space could be a problem
and needs to be blocked...). The recovery typically involves a reset of
the device but in sync with the driver.

	- The driver doesn't have the callbacks. In this case, we
simulate an unplug, reset the device, and replug.

So it makes sense for the second case to emit the same uevents as a
normal PCI(e) hotplug.

For the former case I'm less sure.... Do we really need userspace to be
notified ? If yes, what for precisely ?

In pSeries SR-IOV environment the management console might need to apply
certain configuration changes to the PF driver after it has been recovered
and before the VF drivers are allowed to resume their recovery path.
I could not think of another way to notify user space of these events.
I made this assumption because I saw there were no uevents added when 
the device goes offline and come back online in EEH code. It was my 
intention to make the event as generic as possible in EEH component,
therefore, making this change independent of pSeries SR-IOV.

- Juan

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Bjorn Helgaas <helgaas@kernel.org>
Date: 2017-12-28 23:23:00

On Wed, Dec 20, 2017 at 09:04:27PM -0600, Juan Alvarez wrote:

On 12/19/17 12:27 AM, Benjamin Herrenschmidt wrote:

quoted

On Mon, 2017-12-18 at 22:50 -0600, Bjorn Helgaas wrote:

quoted

[+cc Keith, Gabriele, Dongdong]

On Mon, Dec 18, 2017 at 04:38:03PM -0600, Bryant G. Ly wrote:

quoted

Devices can go offline when EEH is reported. This patch adds
a change to the kernel object and lets udev know of error.
When device resumes a change is also set reporting device as
online. Therefore, EEH events are better propagated to user
space for devices in powerpc arch.

I'm on vacation and can't review this in detail, but I wonder if you
can compare this with the uevents we emit for DPC, AER, and hotplug
events (if any).  I hope we don't end up with userspace having to be
aware of the differences between EEH, DPC, AER, etc.

quoted

From a very quick look, I only see a few uevents even mentioned in

drivers/pci: KOBJ_ADD in __pci_hp_register() and KOBJ_CHANGE in the
SR-IOV code.  I'm worried that we're missing some important uevents in
the PCI core.  That's not an argument against what you're doing here;
it just would be nice to fill in any missing pieces in the core also,
and hopefully make them consistent with these EEH events.

We also need to be careful about what specific EEH activity we are
talking about, and if we bring into the picture things like DPDK, it
gets even more murky...

The basic way EEH is supposed to work for recovery (minus all sort of
implementation nasties which hopefully Russell and Sam are trying to
cleanup and fix) is that either:

	- The driver of the device has recovery callbacks, in which
case the driver participates in the recovery process, the device
doesn't "go away" (though it shouldn't be accessed during that process
by other entities, userspace originated config space could be a problem
and needs to be blocked...). The recovery typically involves a reset of
the device but in sync with the driver.

	- The driver doesn't have the callbacks. In this case, we
simulate an unplug, reset the device, and replug.

So it makes sense for the second case to emit the same uevents as a
normal PCI(e) hotplug.

For the former case I'm less sure.... Do we really need userspace to be
notified ? If yes, what for precisely ?

In pSeries SR-IOV environment the management console might need to apply
certain configuration changes to the PF driver after it has been recovered
and before the VF drivers are allowed to resume their recovery path.
I could not think of another way to notify user space of these events.
I made this assumption because I saw there were no uevents added when 
the device goes offline and come back online in EEH code. It was my 
intention to make the event as generic as possible in EEH component,
therefore, making this change independent of pSeries SR-IOV.

I don't know what your plan for this is, but we do have two different
paths that use the struct pci_error_handlers hooks that drivers may
supply.  There's this AER path that may be used on all arches:

  aer_isr
    get_e_source              # remove from rpc->e_sources[] queue
    aer_isr_one_error
      aer_process_err_devices
        handle_error_source   # or aer_recover_work_func
          do_recovery         # for uncorrectable (fatal/nonfatal) only
            broadcast_error_message(dev, ..., report_error_detected)
              pci_walk_bus(..., report_error_detected)
                report_error_detected
                  dev->driver->err_handler->error_detected

And there's this powerpc path where you're adding a uevent:

  eeh_event_handler
    eeh_handle_event
      eeh_handle_normal_event
        eeh_pe_dev_traverse(pe, eeh_report_error, &result)
          eeh_report_error
            driver->err_handler->error_detected(dev, pci_channel_io_frozen)
 +          kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);

Both paths end up calling the pci_error_handlers.error_detected()
hook.

Drivers are not supposed to care what arch they're running on.  If the
driver supplies an .error_detected() entry point, it's up to the PCI
core and powerpc code to use it consistently across arches.  That
means the same uevents (if any) should be emitted from both paths.

The best way would be to unify the call of .error_detected() so the
AER path and the powerpc path do it via the same function.  The AER
report_error_detected() and the powerpc eeh_report_error() do look
fairly similar, so this seems possible in principle, but I'm not
holding my breath.

Bjorn

Re: [PATCH v2 2/7] powerpc/kernel: Add uevents in EEH error/resume

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2017-12-29 00:03:18

On Thu, 2017-12-28 at 17:22 -0600, Bjorn Helgaas wrote:

Both paths end up calling the pci_error_handlers.error_detected()
hook.

Drivers are not supposed to care what arch they're running on.  If the
driver supplies an .error_detected() entry point, it's up to the PCI
core and powerpc code to use it consistently across arches.  That
means the same uevents (if any) should be emitted from both paths.

The best way would be to unify the call of .error_detected() so the
AER path and the powerpc path do it via the same function.  The AER
report_error_detected() and the powerpc eeh_report_error() do look
fairly similar, so this seems possible in principle, but I'm not
holding my breath.

Factoring these callers into a common function that can then do the
uevent for errors makes a lot of sense.

The "resume" path might be trickier, but even then, rather than calling
directly the driver op, it would be easy to have a little wrapper that
does it, which can then also do the uevent.

Ben.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help