All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
@ 2019-03-11 11:52 ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

This patchset allows switching from the pnv_php module to the standard
pciehp driver for PCIe hotplug functionality, if the platform supports it:
PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
the changes of bdfns between the firmware and the OS" [1] patch serie
applied.

The feature is activated by the "pci=realloc" command line argument.

The goal is ability to hotplug bridges full of devices in the future. The
"Movable BARs" [2] is a platform-independent part of our work in this. The
final part will be movable bus numbers to support inserting a bridge in the
middle of an existing PCIe tree.

Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
 - the pciehp driver active;
 - the pnv_php driver disabled;
 - The "pci=realloc" argument is passed;
 - surprise hotplug of an NVME disk works;
 - controlled hotplug of a network card with SR-IOV works;
 - activating of SR-IOV on a network card works;
 - [with extra patches] manually initiated (via sysfs) rescan has found
   and turned on a hotplugged bridge;
 - Without "pci=realloc" works just as before.

Changes since v4:
 - Fixed failing build when EEH is disabled in a kernel config;
 - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
 - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
 - Renamed create_pdn() -> pci_create_pdn_from_dev();
 - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
 - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
 - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
 - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
   commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
   VFs")

Changes since v3 [3]:
 - Subject changed;
 - Don't disable EEH during rescan anymore - instead just unfreeze the
   target buses deliberately;
 - Add synchronization with the firmware when changing the PCIe topology;
 - Fixed for VFs;
 - Code cleanup.

Changes since v2:
 - Don't reassign bus numbers on PowerNV by default (to retain the default
   behavior), but only when pci=realloc is passed;
 - Less code affected;
 - pci_add_device_node_info is refactored with add_one_dev_pci_data;
 - Minor code cleanup.

Changes since v1:
 - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
 - Fixed build for ppc64e when CONFIG_EEH is disabled;
 - Fixed code style warnings.

[1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
[2] https://www.spinics.net/lists/linux-pci/msg79995.html
[3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html

Sergey Miroshnichenko (8):
  powerpc/pci: Access PCI config space directly w/o pci_dn
  powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
  powerpc/pci: Create pci_dn on demand
  powerpc/pci: Reduce code duplication in pci_add_device_node_info
  powerpc/pci/IOV: Add support for runtime enabling the VFs
  powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
  powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
  powerpc/powernv/pci: Enable reassigning the bus numbers

 arch/powerpc/include/asm/pci-bridge.h        |   4 +-
 arch/powerpc/include/asm/ppc-pci.h           |   1 +
 arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
 arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
 arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
 arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
 arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
 arch/powerpc/platforms/pseries/pci.c         |   4 +-
 8 files changed, 379 insertions(+), 108 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
@ 2019-03-11 11:52 ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

This patchset allows switching from the pnv_php module to the standard
pciehp driver for PCIe hotplug functionality, if the platform supports it:
PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
the changes of bdfns between the firmware and the OS" [1] patch serie
applied.

The feature is activated by the "pci=realloc" command line argument.

The goal is ability to hotplug bridges full of devices in the future. The
"Movable BARs" [2] is a platform-independent part of our work in this. The
final part will be movable bus numbers to support inserting a bridge in the
middle of an existing PCIe tree.

Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
 - the pciehp driver active;
 - the pnv_php driver disabled;
 - The "pci=realloc" argument is passed;
 - surprise hotplug of an NVME disk works;
 - controlled hotplug of a network card with SR-IOV works;
 - activating of SR-IOV on a network card works;
 - [with extra patches] manually initiated (via sysfs) rescan has found
   and turned on a hotplugged bridge;
 - Without "pci=realloc" works just as before.

Changes since v4:
 - Fixed failing build when EEH is disabled in a kernel config;
 - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
 - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
 - Renamed create_pdn() -> pci_create_pdn_from_dev();
 - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
 - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
 - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
 - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
   commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
   VFs")

Changes since v3 [3]:
 - Subject changed;
 - Don't disable EEH during rescan anymore - instead just unfreeze the
   target buses deliberately;
 - Add synchronization with the firmware when changing the PCIe topology;
 - Fixed for VFs;
 - Code cleanup.

Changes since v2:
 - Don't reassign bus numbers on PowerNV by default (to retain the default
   behavior), but only when pci=realloc is passed;
 - Less code affected;
 - pci_add_device_node_info is refactored with add_one_dev_pci_data;
 - Minor code cleanup.

Changes since v1:
 - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
 - Fixed build for ppc64e when CONFIG_EEH is disabled;
 - Fixed code style warnings.

[1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
[2] https://www.spinics.net/lists/linux-pci/msg79995.html
[3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html

Sergey Miroshnichenko (8):
  powerpc/pci: Access PCI config space directly w/o pci_dn
  powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
  powerpc/pci: Create pci_dn on demand
  powerpc/pci: Reduce code duplication in pci_add_device_node_info
  powerpc/pci/IOV: Add support for runtime enabling the VFs
  powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
  powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
  powerpc/powernv/pci: Enable reassigning the bus numbers

 arch/powerpc/include/asm/pci-bridge.h        |   4 +-
 arch/powerpc/include/asm/ppc-pci.h           |   1 +
 arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
 arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
 arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
 arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
 arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
 arch/powerpc/platforms/pseries/pci.c         |   4 +-
 8 files changed, 379 insertions(+), 108 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 30+ messages in thread
* [PATCH v5 1/8] powerpc/pci: Access PCI config space directly w/o pci_dn
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

To fetch an updated DT for the newly hotplugged device, OS must explicitly
request it from the firmware via the pnv_php driver.

If pnv_php wasn't triggered/loaded, it is still possible to discover new
devices if PCIe I/O will not stop in absence of the pci_dn structure.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/rtas_pci.c       | 97 +++++++++++++++++++---------
 arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
 2 files changed, 109 insertions(+), 52 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index c2b148b1634a..f675b5ecb5bc 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
 	return 0;
 }
 
-int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
+				int where, int size, u32 *val)
 {
 	int returnval = -1;
-	unsigned long buid, addr;
+	unsigned long addr = rtas_config_addr(busno, devfn, where);
+	int ret;
+
+	if (buid) {
+		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
+				addr, BUID_HI(buid), BUID_LO(buid), size);
+	} else {
+		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
+	}
+	*val = returnval;
+
+	return ret;
+}
+
+int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+{
 	int ret;
 
 	if (!pdn)
@@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
 		return PCIBIOS_SET_FAILED;
 #endif
 
-	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
-	buid = pdn->phb->buid;
-	if (buid) {
-		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
-				addr, BUID_HI(buid), BUID_LO(buid), size);
-	} else {
-		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
-	}
-	*val = returnval;
-
+	ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+				   where, size, val);
 	if (ret)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
@@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
 
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 
-	/* Validity of pdn is checked in here */
-	ret = rtas_read_config(pdn, where, size, val);
-	if (*val == EEH_IO_ERROR_VALUE(size) &&
-	    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
-		return PCIBIOS_DEVICE_NOT_FOUND;
+	if (pdn) {
+		/* Validity of pdn is checked in here */
+		ret = rtas_read_config(pdn, where, size, val);
+
+		if (*val == EEH_IO_ERROR_VALUE(size) &&
+		    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
+			ret = PCIBIOS_DEVICE_NOT_FOUND;
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
+
+		ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
+					   where, size, val);
+	}
 
 	return ret;
 }
 
+static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
+				 int where, int size, u32 val)
+{
+	unsigned long addr = rtas_config_addr(busno, devfn, where);
+	int ret;
+
+	if (buid) {
+		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
+				BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
+	} else {
+		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
+	}
+
+	if (ret)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
 int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
 {
-	unsigned long buid, addr;
 	int ret;
 
 	if (!pdn)
@@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
 		return PCIBIOS_SET_FAILED;
 #endif
 
-	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
-	buid = pdn->phb->buid;
-	if (buid) {
-		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
-			BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
-	} else {
-		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
-	}
-
+	ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+				    where, size, val);
 	if (ret)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
@@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
 				 unsigned int devfn,
 				 int where, int size, u32 val)
 {
-	struct pci_dn *pdn;
+	struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
+	int ret;
 
-	pdn = pci_get_pdn_by_devfn(bus, devfn);
+	if (pdn) {
+		/* Validity of pdn is checked in here. */
+		ret = rtas_write_config(pdn, where, size, val);
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
 
-	/* Validity of pdn is checked in here. */
-	return rtas_write_config(pdn, where, size, val);
+		ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
+					    where, size, val);
+	}
+
+	return ret;
 }
 
 static struct pci_ops rtas_pci_ops = {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index ef9448a907c6..41a381dfc2a1 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -652,30 +652,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
 	}
 }
 
-int pnv_pci_cfg_read(struct pci_dn *pdn,
-		     int where, int size, u32 *val)
+static int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
+				int where, int size, u32 *val)
 {
-	struct pnv_phb *phb = pdn->phb->private_data;
-	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+	u32 bdfn = (busno << 8) | devfn;
 	s64 rc;
 
 	switch (size) {
 	case 1: {
 		u8 v8;
-		rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
+		rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
 		*val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
 		break;
 	}
 	case 2: {
 		__be16 v16;
-		rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
-						   &v16);
+		rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
+						    &v16);
 		*val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
 		break;
 	}
 	case 4: {
 		__be32 v32;
-		rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
+		rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
 		*val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
 		break;
 	}
@@ -684,27 +683,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
 	}
 
 	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
-		 __func__, pdn->busno, pdn->devfn, where, size, *val);
+		 __func__, busno, devfn, where, size, *val);
+
 	return PCIBIOS_SUCCESSFUL;
 }
 
-int pnv_pci_cfg_write(struct pci_dn *pdn,
-		      int where, int size, u32 val)
+static int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
+				 int where, int size, u32 val)
 {
-	struct pnv_phb *phb = pdn->phb->private_data;
-	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+	u32 bdfn = (busno << 8) | devfn;
 
 	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
-		 __func__, pdn->busno, pdn->devfn, where, size, val);
+		 __func__, busno, devfn, where, size, val);
+
 	switch (size) {
 	case 1:
-		opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_byte(phb_id, bdfn, where, val);
 		break;
 	case 2:
-		opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_half_word(phb_id, bdfn, where, val);
 		break;
 	case 4:
-		opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_word(phb_id, bdfn, where, val);
 		break;
 	default:
 		return PCIBIOS_FUNC_NOT_SUPPORTED;
@@ -713,6 +713,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+int pnv_pci_cfg_read(struct pci_dn *pdn,
+		     int where, int size, u32 *val)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+
+	return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				    where, size, val);
+}
+
+int pnv_pci_cfg_write(struct pci_dn *pdn,
+		      int where, int size, u32 val)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+
+	return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				     where, size, val);
+}
+
 #if CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
@@ -748,13 +766,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 			       int where, int size, u32 *val)
 {
 	struct pci_dn *pdn;
-	struct pnv_phb *phb;
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
 	*val = 0xFFFFFFFF;
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 	if (!pdn)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+					    where, size, val);
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
@@ -777,12 +797,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
 				int where, int size, u32 val)
 {
 	struct pci_dn *pdn;
-	struct pnv_phb *phb;
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 	if (!pdn)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
+					     where, size, val);
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 1/8] powerpc/pci: Access PCI config space directly w/o pci_dn
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

To fetch an updated DT for the newly hotplugged device, OS must explicitly
request it from the firmware via the pnv_php driver.

If pnv_php wasn't triggered/loaded, it is still possible to discover new
devices if PCIe I/O will not stop in absence of the pci_dn structure.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/rtas_pci.c       | 97 +++++++++++++++++++---------
 arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
 2 files changed, 109 insertions(+), 52 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index c2b148b1634a..f675b5ecb5bc 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
 	return 0;
 }
 
-int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
+				int where, int size, u32 *val)
 {
 	int returnval = -1;
-	unsigned long buid, addr;
+	unsigned long addr = rtas_config_addr(busno, devfn, where);
+	int ret;
+
+	if (buid) {
+		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
+				addr, BUID_HI(buid), BUID_LO(buid), size);
+	} else {
+		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
+	}
+	*val = returnval;
+
+	return ret;
+}
+
+int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+{
 	int ret;
 
 	if (!pdn)
@@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
 		return PCIBIOS_SET_FAILED;
 #endif
 
-	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
-	buid = pdn->phb->buid;
-	if (buid) {
-		ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
-				addr, BUID_HI(buid), BUID_LO(buid), size);
-	} else {
-		ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
-	}
-	*val = returnval;
-
+	ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+				   where, size, val);
 	if (ret)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
@@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
 
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 
-	/* Validity of pdn is checked in here */
-	ret = rtas_read_config(pdn, where, size, val);
-	if (*val == EEH_IO_ERROR_VALUE(size) &&
-	    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
-		return PCIBIOS_DEVICE_NOT_FOUND;
+	if (pdn) {
+		/* Validity of pdn is checked in here */
+		ret = rtas_read_config(pdn, where, size, val);
+
+		if (*val == EEH_IO_ERROR_VALUE(size) &&
+		    eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
+			ret = PCIBIOS_DEVICE_NOT_FOUND;
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
+
+		ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
+					   where, size, val);
+	}
 
 	return ret;
 }
 
+static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
+				 int where, int size, u32 val)
+{
+	unsigned long addr = rtas_config_addr(busno, devfn, where);
+	int ret;
+
+	if (buid) {
+		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
+				BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
+	} else {
+		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
+	}
+
+	if (ret)
+		return PCIBIOS_DEVICE_NOT_FOUND;
+
+	return PCIBIOS_SUCCESSFUL;
+}
+
 int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
 {
-	unsigned long buid, addr;
 	int ret;
 
 	if (!pdn)
@@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
 		return PCIBIOS_SET_FAILED;
 #endif
 
-	addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
-	buid = pdn->phb->buid;
-	if (buid) {
-		ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
-			BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
-	} else {
-		ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
-	}
-
+	ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+				    where, size, val);
 	if (ret)
 		return PCIBIOS_DEVICE_NOT_FOUND;
 
@@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
 				 unsigned int devfn,
 				 int where, int size, u32 val)
 {
-	struct pci_dn *pdn;
+	struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
+	int ret;
 
-	pdn = pci_get_pdn_by_devfn(bus, devfn);
+	if (pdn) {
+		/* Validity of pdn is checked in here. */
+		ret = rtas_write_config(pdn, where, size, val);
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
 
-	/* Validity of pdn is checked in here. */
-	return rtas_write_config(pdn, where, size, val);
+		ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
+					    where, size, val);
+	}
+
+	return ret;
 }
 
 static struct pci_ops rtas_pci_ops = {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index ef9448a907c6..41a381dfc2a1 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -652,30 +652,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
 	}
 }
 
-int pnv_pci_cfg_read(struct pci_dn *pdn,
-		     int where, int size, u32 *val)
+static int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
+				int where, int size, u32 *val)
 {
-	struct pnv_phb *phb = pdn->phb->private_data;
-	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+	u32 bdfn = (busno << 8) | devfn;
 	s64 rc;
 
 	switch (size) {
 	case 1: {
 		u8 v8;
-		rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
+		rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
 		*val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
 		break;
 	}
 	case 2: {
 		__be16 v16;
-		rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
-						   &v16);
+		rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
+						    &v16);
 		*val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
 		break;
 	}
 	case 4: {
 		__be32 v32;
-		rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
+		rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
 		*val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
 		break;
 	}
@@ -684,27 +683,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
 	}
 
 	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
-		 __func__, pdn->busno, pdn->devfn, where, size, *val);
+		 __func__, busno, devfn, where, size, *val);
+
 	return PCIBIOS_SUCCESSFUL;
 }
 
-int pnv_pci_cfg_write(struct pci_dn *pdn,
-		      int where, int size, u32 val)
+static int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
+				 int where, int size, u32 val)
 {
-	struct pnv_phb *phb = pdn->phb->private_data;
-	u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+	u32 bdfn = (busno << 8) | devfn;
 
 	pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
-		 __func__, pdn->busno, pdn->devfn, where, size, val);
+		 __func__, busno, devfn, where, size, val);
+
 	switch (size) {
 	case 1:
-		opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_byte(phb_id, bdfn, where, val);
 		break;
 	case 2:
-		opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_half_word(phb_id, bdfn, where, val);
 		break;
 	case 4:
-		opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
+		opal_pci_config_write_word(phb_id, bdfn, where, val);
 		break;
 	default:
 		return PCIBIOS_FUNC_NOT_SUPPORTED;
@@ -713,6 +713,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
 	return PCIBIOS_SUCCESSFUL;
 }
 
+int pnv_pci_cfg_read(struct pci_dn *pdn,
+		     int where, int size, u32 *val)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+
+	return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				    where, size, val);
+}
+
+int pnv_pci_cfg_write(struct pci_dn *pdn,
+		      int where, int size, u32 val)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+
+	return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				     where, size, val);
+}
+
 #if CONFIG_EEH
 static bool pnv_pci_cfg_check(struct pci_dn *pdn)
 {
@@ -748,13 +766,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 			       int where, int size, u32 *val)
 {
 	struct pci_dn *pdn;
-	struct pnv_phb *phb;
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
 	*val = 0xFFFFFFFF;
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 	if (!pdn)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+					    where, size, val);
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
@@ -777,12 +797,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
 				int where, int size, u32 val)
 {
 	struct pci_dn *pdn;
-	struct pnv_phb *phb;
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
 	int ret;
 
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
 	if (!pdn)
-		return PCIBIOS_DEVICE_NOT_FOUND;
+		return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
+					     where, size, val);
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 2/8] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

Reading an empty slot returns all ones, which triggers a false
EEH error event on PowerNV. This patch unfreezes the bus where
it has happened.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/include/asm/ppc-pci.h   |  1 +
 arch/powerpc/kernel/pci_dn.c         |  2 +-
 arch/powerpc/platforms/powernv/pci.c | 31 +++++++++++++++++++++++++---
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index f191ef0d2a0a..a22d52a9bb1f 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -40,6 +40,7 @@ void *traverse_pci_dn(struct pci_dn *root,
 		      void *(*fn)(struct pci_dn *, void *),
 		      void *data);
 extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus);
 
 /* From rtas_pci.h */
 extern void init_pci_config_tokens (void);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index ab147a1909c8..341ed71250f1 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -40,7 +40,7 @@
  * one of PF's bridge. For other devices, their firmware
  * data is linked to that of their bridge.
  */
-static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
 {
 	struct pci_bus *pbus;
 	struct device_node *dn;
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 41a381dfc2a1..8cc6661781e2 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -761,6 +761,21 @@ static inline pnv_pci_cfg_check(struct pci_dn *pdn)
 }
 #endif /* CONFIG_EEH */
 
+static int get_bus_pe_number(struct pci_bus *bus)
+{
+	struct pci_dn *pdn = pci_bus_to_pdn(bus);
+	struct pci_dn *child;
+
+	if (!pdn)
+		return IODA_INVALID_PE;
+
+	list_for_each_entry(child, &pdn->child_list, list)
+		if (child->pe_number != IODA_INVALID_PE)
+			return child->pe_number;
+
+	return IODA_INVALID_PE;
+}
+
 static int pnv_pci_read_config(struct pci_bus *bus,
 			       unsigned int devfn,
 			       int where, int size, u32 *val)
@@ -772,9 +787,19 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 
 	*val = 0xFFFFFFFF;
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
-	if (!pdn)
-		return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
-					    where, size, val);
+	if (!pdn) {
+		int pe_number = get_bus_pe_number(bus);
+
+		ret = pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+					   where, size, val);
+
+		if (!ret && (*val == EEH_IO_ERROR_VALUE(size)) && phb->unfreeze_pe)
+			phb->unfreeze_pe(phb, (pe_number == IODA_INVALID_PE) ?
+					 phb->ioda.reserved_pe_idx : pe_number,
+					 OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
+
+		return ret;
+	}
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 2/8] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

Reading an empty slot returns all ones, which triggers a false
EEH error event on PowerNV. This patch unfreezes the bus where
it has happened.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/include/asm/ppc-pci.h   |  1 +
 arch/powerpc/kernel/pci_dn.c         |  2 +-
 arch/powerpc/platforms/powernv/pci.c | 31 +++++++++++++++++++++++++---
 3 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index f191ef0d2a0a..a22d52a9bb1f 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -40,6 +40,7 @@ void *traverse_pci_dn(struct pci_dn *root,
 		      void *(*fn)(struct pci_dn *, void *),
 		      void *data);
 extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus);
 
 /* From rtas_pci.h */
 extern void init_pci_config_tokens (void);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index ab147a1909c8..341ed71250f1 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -40,7 +40,7 @@
  * one of PF's bridge. For other devices, their firmware
  * data is linked to that of their bridge.
  */
-static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
 {
 	struct pci_bus *pbus;
 	struct device_node *dn;
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 41a381dfc2a1..8cc6661781e2 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -761,6 +761,21 @@ static inline pnv_pci_cfg_check(struct pci_dn *pdn)
 }
 #endif /* CONFIG_EEH */
 
+static int get_bus_pe_number(struct pci_bus *bus)
+{
+	struct pci_dn *pdn = pci_bus_to_pdn(bus);
+	struct pci_dn *child;
+
+	if (!pdn)
+		return IODA_INVALID_PE;
+
+	list_for_each_entry(child, &pdn->child_list, list)
+		if (child->pe_number != IODA_INVALID_PE)
+			return child->pe_number;
+
+	return IODA_INVALID_PE;
+}
+
 static int pnv_pci_read_config(struct pci_bus *bus,
 			       unsigned int devfn,
 			       int where, int size, u32 *val)
@@ -772,9 +787,19 @@ static int pnv_pci_read_config(struct pci_bus *bus,
 
 	*val = 0xFFFFFFFF;
 	pdn = pci_get_pdn_by_devfn(bus, devfn);
-	if (!pdn)
-		return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
-					    where, size, val);
+	if (!pdn) {
+		int pe_number = get_bus_pe_number(bus);
+
+		ret = pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+					   where, size, val);
+
+		if (!ret && (*val == EEH_IO_ERROR_VALUE(size)) && phb->unfreeze_pe)
+			phb->unfreeze_pe(phb, (pe_number == IODA_INVALID_PE) ?
+					 phb->ioda.reserved_pe_idx : pe_number,
+					 OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
+
+		return ret;
+	}
 
 	if (!pnv_pci_cfg_check(pdn))
 		return PCIBIOS_DEVICE_NOT_FOUND;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 3/8] powerpc/pci: Create pci_dn on demand
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

If a struct pci_dn hasn't yet been created for the PCIe device (there was
no DT node for it), allocate this structure and fill with info read from
the device directly.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 88 ++++++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 341ed71250f1..b594b055b2cf 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -33,6 +33,9 @@
 #include <asm/firmware.h>
 #include <asm/eeh.h>
 
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+					      struct pci_dn *parent);
+
 /*
  * The function is used to find the firmware data of one
  * specific PCI device, which is attached to the indicated
@@ -65,6 +68,9 @@ struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
 	dn = pci_bus_to_OF_node(pbus);
 	pdn = dn ? PCI_DN(dn) : NULL;
 
+	if (!pdn && pbus->self)
+		pdn = pbus->self->dev.archdata.pci_data;
+
 	return pdn;
 }
 
@@ -74,10 +80,13 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 	struct device_node *dn = NULL;
 	struct pci_dn *parent, *pdn;
 	struct pci_dev *pdev = NULL;
+	bool pdev_found = false;
 
 	/* Fast path: fetch from PCI device */
 	list_for_each_entry(pdev, &bus->devices, bus_list) {
 		if (pdev->devfn == devfn) {
+			pdev_found = true;
+
 			if (pdev->dev.archdata.pci_data)
 				return pdev->dev.archdata.pci_data;
 
@@ -86,6 +95,9 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 		}
 	}
 
+	if (!pdev_found)
+		pdev = NULL;
+
 	/* Fast path: fetch from device node */
 	pdn = dn ? PCI_DN(dn) : NULL;
 	if (pdn)
@@ -98,9 +110,12 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 
 	list_for_each_entry(pdn, &parent->child_list, list) {
 		if (pdn->busno == bus->number &&
-                    pdn->devfn == devfn)
-                        return pdn;
-        }
+		    pdn->devfn == devfn) {
+			if (pdev)
+				pdev->dev.archdata.pci_data = pdn;
+			return pdn;
+		}
+	}
 
 	return NULL;
 }
@@ -130,17 +145,17 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
 
 	list_for_each_entry(pdn, &parent->child_list, list) {
 		if (pdn->busno == pdev->bus->number &&
-		    pdn->devfn == pdev->devfn)
+		    pdn->devfn == pdev->devfn) {
+			pdev->dev.archdata.pci_data = pdn;
 			return pdn;
+		}
 	}
 
-	return NULL;
+	return pci_create_pdn_from_dev(pdev, parent);
 }
 
-#ifdef CONFIG_PCI_IOV
-static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
-					   int vf_index,
-					   int busno, int devfn)
+static struct pci_dn *pci_alloc_pdn(struct pci_dn *parent,
+				    int busno, int devfn)
 {
 	struct pci_dn *pdn;
 
@@ -156,7 +171,6 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 	pdn->parent = parent;
 	pdn->busno = busno;
 	pdn->devfn = devfn;
-	pdn->vf_index = vf_index;
 	pdn->pe_number = IODA_INVALID_PE;
 	INIT_LIST_HEAD(&pdn->child_list);
 	INIT_LIST_HEAD(&pdn->list);
@@ -164,7 +178,51 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 
 	return pdn;
 }
-#endif
+
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+					      struct pci_dn *parent)
+{
+	struct pci_dn *pdn = NULL;
+	u32 class_code;
+	u16 device_id;
+	u16 vendor_id;
+
+	if (!parent)
+		return NULL;
+
+	pdn = pci_alloc_pdn(parent, pdev->bus->busn_res.start, pdev->devfn);
+	pci_info(pdev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
+
+	if (!pdn) {
+		pci_err(pdev, "%s: Failed to allocate pdn\n", __func__);
+		return NULL;
+	}
+
+	#ifdef CONFIG_EEH
+	if (!eeh_dev_init(pdn)) {
+		kfree(pdn);
+		pci_err(pdev, "%s: Failed to allocate edev\n", __func__);
+		return NULL;
+	}
+	#endif /* CONFIG_EEH */
+
+	pci_bus_read_config_word(pdev->bus, pdev->devfn,
+				 PCI_VENDOR_ID, &vendor_id);
+	pdn->vendor_id = vendor_id;
+
+	pci_bus_read_config_word(pdev->bus, pdev->devfn,
+				 PCI_DEVICE_ID, &device_id);
+	pdn->device_id = device_id;
+
+	pci_bus_read_config_dword(pdev->bus, pdev->devfn,
+				  PCI_CLASS_REVISION, &class_code);
+	class_code >>= 8;
+	pdn->class_code = class_code;
+
+	pdev->dev.archdata.pci_data = pdn;
+
+	return pdn;
+}
 
 struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 {
@@ -189,15 +247,17 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
 		struct eeh_dev *edev __maybe_unused;
 
-		pdn = add_one_dev_pci_data(parent, i,
-					   pci_iov_virtfn_bus(pdev, i),
-					   pci_iov_virtfn_devfn(pdev, i));
+		pdn = pci_alloc_pdn(parent,
+				    pci_iov_virtfn_bus(pdev, i),
+				    pci_iov_virtfn_devfn(pdev, i));
 		if (!pdn) {
 			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
 				 __func__, i);
 			return NULL;
 		}
 
+		pdn->vf_index = i;
+
 #ifdef CONFIG_EEH
 		/* Create the EEH device for the VF */
 		edev = eeh_dev_init(pdn);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 3/8] powerpc/pci: Create pci_dn on demand
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

If a struct pci_dn hasn't yet been created for the PCIe device (there was
no DT node for it), allocate this structure and fill with info read from
the device directly.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 88 ++++++++++++++++++++++++++++++------
 1 file changed, 74 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 341ed71250f1..b594b055b2cf 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -33,6 +33,9 @@
 #include <asm/firmware.h>
 #include <asm/eeh.h>
 
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+					      struct pci_dn *parent);
+
 /*
  * The function is used to find the firmware data of one
  * specific PCI device, which is attached to the indicated
@@ -65,6 +68,9 @@ struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
 	dn = pci_bus_to_OF_node(pbus);
 	pdn = dn ? PCI_DN(dn) : NULL;
 
+	if (!pdn && pbus->self)
+		pdn = pbus->self->dev.archdata.pci_data;
+
 	return pdn;
 }
 
@@ -74,10 +80,13 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 	struct device_node *dn = NULL;
 	struct pci_dn *parent, *pdn;
 	struct pci_dev *pdev = NULL;
+	bool pdev_found = false;
 
 	/* Fast path: fetch from PCI device */
 	list_for_each_entry(pdev, &bus->devices, bus_list) {
 		if (pdev->devfn == devfn) {
+			pdev_found = true;
+
 			if (pdev->dev.archdata.pci_data)
 				return pdev->dev.archdata.pci_data;
 
@@ -86,6 +95,9 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 		}
 	}
 
+	if (!pdev_found)
+		pdev = NULL;
+
 	/* Fast path: fetch from device node */
 	pdn = dn ? PCI_DN(dn) : NULL;
 	if (pdn)
@@ -98,9 +110,12 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 
 	list_for_each_entry(pdn, &parent->child_list, list) {
 		if (pdn->busno == bus->number &&
-                    pdn->devfn == devfn)
-                        return pdn;
-        }
+		    pdn->devfn == devfn) {
+			if (pdev)
+				pdev->dev.archdata.pci_data = pdn;
+			return pdn;
+		}
+	}
 
 	return NULL;
 }
@@ -130,17 +145,17 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
 
 	list_for_each_entry(pdn, &parent->child_list, list) {
 		if (pdn->busno == pdev->bus->number &&
-		    pdn->devfn == pdev->devfn)
+		    pdn->devfn == pdev->devfn) {
+			pdev->dev.archdata.pci_data = pdn;
 			return pdn;
+		}
 	}
 
-	return NULL;
+	return pci_create_pdn_from_dev(pdev, parent);
 }
 
-#ifdef CONFIG_PCI_IOV
-static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
-					   int vf_index,
-					   int busno, int devfn)
+static struct pci_dn *pci_alloc_pdn(struct pci_dn *parent,
+				    int busno, int devfn)
 {
 	struct pci_dn *pdn;
 
@@ -156,7 +171,6 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 	pdn->parent = parent;
 	pdn->busno = busno;
 	pdn->devfn = devfn;
-	pdn->vf_index = vf_index;
 	pdn->pe_number = IODA_INVALID_PE;
 	INIT_LIST_HEAD(&pdn->child_list);
 	INIT_LIST_HEAD(&pdn->list);
@@ -164,7 +178,51 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 
 	return pdn;
 }
-#endif
+
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+					      struct pci_dn *parent)
+{
+	struct pci_dn *pdn = NULL;
+	u32 class_code;
+	u16 device_id;
+	u16 vendor_id;
+
+	if (!parent)
+		return NULL;
+
+	pdn = pci_alloc_pdn(parent, pdev->bus->busn_res.start, pdev->devfn);
+	pci_info(pdev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
+
+	if (!pdn) {
+		pci_err(pdev, "%s: Failed to allocate pdn\n", __func__);
+		return NULL;
+	}
+
+	#ifdef CONFIG_EEH
+	if (!eeh_dev_init(pdn)) {
+		kfree(pdn);
+		pci_err(pdev, "%s: Failed to allocate edev\n", __func__);
+		return NULL;
+	}
+	#endif /* CONFIG_EEH */
+
+	pci_bus_read_config_word(pdev->bus, pdev->devfn,
+				 PCI_VENDOR_ID, &vendor_id);
+	pdn->vendor_id = vendor_id;
+
+	pci_bus_read_config_word(pdev->bus, pdev->devfn,
+				 PCI_DEVICE_ID, &device_id);
+	pdn->device_id = device_id;
+
+	pci_bus_read_config_dword(pdev->bus, pdev->devfn,
+				  PCI_CLASS_REVISION, &class_code);
+	class_code >>= 8;
+	pdn->class_code = class_code;
+
+	pdev->dev.archdata.pci_data = pdn;
+
+	return pdn;
+}
 
 struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 {
@@ -189,15 +247,17 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
 		struct eeh_dev *edev __maybe_unused;
 
-		pdn = add_one_dev_pci_data(parent, i,
-					   pci_iov_virtfn_bus(pdev, i),
-					   pci_iov_virtfn_devfn(pdev, i));
+		pdn = pci_alloc_pdn(parent,
+				    pci_iov_virtfn_bus(pdev, i),
+				    pci_iov_virtfn_devfn(pdev, i));
 		if (!pdn) {
 			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
 				 __func__, i);
 			return NULL;
 		}
 
+		pdn->vf_index = i;
+
 #ifdef CONFIG_EEH
 		/* Create the EEH device for the VF */
 		edev = eeh_dev_init(pdn);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 4/8] powerpc/pci: Reduce code duplication in pci_add_device_node_info
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

It is possible now to allocate and fill a new pdn with add_one_dev_pci_data

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 38 +++++++++++++++---------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index b594b055b2cf..7f12882d8882 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -159,22 +159,20 @@ static struct pci_dn *pci_alloc_pdn(struct pci_dn *parent,
 {
 	struct pci_dn *pdn;
 
-	/* Except PHB, we always have the parent */
-	if (!parent)
-		return NULL;
-
 	pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
 	if (!pdn)
 		return NULL;
 
-	pdn->phb = parent->phb;
 	pdn->parent = parent;
 	pdn->busno = busno;
 	pdn->devfn = devfn;
 	pdn->pe_number = IODA_INVALID_PE;
 	INIT_LIST_HEAD(&pdn->child_list);
 	INIT_LIST_HEAD(&pdn->list);
-	list_add_tail(&pdn->list, &parent->child_list);
+	if (parent) {
+		pdn->phb = parent->phb;
+		list_add_tail(&pdn->list, &parent->child_list);
+	}
 
 	return pdn;
 }
@@ -341,25 +339,29 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 	const __be32 *regs;
 	struct device_node *parent;
 	struct pci_dn *pdn;
+	int busno = 0, devfn = 0;
 #ifdef CONFIG_EEH
 	struct eeh_dev *edev;
 #endif
 
-	pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
-	if (pdn == NULL)
-		return NULL;
-	dn->data = pdn;
-	pdn->phb = hose;
-	pdn->pe_number = IODA_INVALID_PE;
 	regs = of_get_property(dn, "reg", NULL);
 	if (regs) {
 		u32 addr = of_read_number(regs, 1);
 
 		/* First register entry is addr (00BBSS00)  */
-		pdn->busno = (addr >> 16) & 0xff;
-		pdn->devfn = (addr >> 8) & 0xff;
+		busno = (addr >> 16) & 0xff;
+		devfn = (addr >> 8) & 0xff;
 	}
 
+	parent = of_get_parent(dn);
+	pdn = pci_alloc_pdn(parent ? PCI_DN(parent) : NULL,
+			    busno, devfn);
+	if (!pdn)
+		return NULL;
+
+	dn->data = pdn;
+	pdn->phb = hose;
+
 	/* vendor/device IDs and class code */
 	regs = of_get_property(dn, "vendor-id", NULL);
 	pdn->vendor_id = regs ? of_read_number(regs, 1) : 0;
@@ -380,14 +382,6 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 	}
 #endif
 
-	/* Attach to parent node */
-	INIT_LIST_HEAD(&pdn->child_list);
-	INIT_LIST_HEAD(&pdn->list);
-	parent = of_get_parent(dn);
-	pdn->parent = parent ? PCI_DN(parent) : NULL;
-	if (pdn->parent)
-		list_add_tail(&pdn->list, &pdn->parent->child_list);
-
 	return pdn;
 }
 EXPORT_SYMBOL_GPL(pci_add_device_node_info);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 4/8] powerpc/pci: Reduce code duplication in pci_add_device_node_info
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

It is possible now to allocate and fill a new pdn with add_one_dev_pci_data

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 38 +++++++++++++++---------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index b594b055b2cf..7f12882d8882 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -159,22 +159,20 @@ static struct pci_dn *pci_alloc_pdn(struct pci_dn *parent,
 {
 	struct pci_dn *pdn;
 
-	/* Except PHB, we always have the parent */
-	if (!parent)
-		return NULL;
-
 	pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
 	if (!pdn)
 		return NULL;
 
-	pdn->phb = parent->phb;
 	pdn->parent = parent;
 	pdn->busno = busno;
 	pdn->devfn = devfn;
 	pdn->pe_number = IODA_INVALID_PE;
 	INIT_LIST_HEAD(&pdn->child_list);
 	INIT_LIST_HEAD(&pdn->list);
-	list_add_tail(&pdn->list, &parent->child_list);
+	if (parent) {
+		pdn->phb = parent->phb;
+		list_add_tail(&pdn->list, &parent->child_list);
+	}
 
 	return pdn;
 }
@@ -341,25 +339,29 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 	const __be32 *regs;
 	struct device_node *parent;
 	struct pci_dn *pdn;
+	int busno = 0, devfn = 0;
 #ifdef CONFIG_EEH
 	struct eeh_dev *edev;
 #endif
 
-	pdn = kzalloc(sizeof(*pdn), GFP_KERNEL);
-	if (pdn == NULL)
-		return NULL;
-	dn->data = pdn;
-	pdn->phb = hose;
-	pdn->pe_number = IODA_INVALID_PE;
 	regs = of_get_property(dn, "reg", NULL);
 	if (regs) {
 		u32 addr = of_read_number(regs, 1);
 
 		/* First register entry is addr (00BBSS00)  */
-		pdn->busno = (addr >> 16) & 0xff;
-		pdn->devfn = (addr >> 8) & 0xff;
+		busno = (addr >> 16) & 0xff;
+		devfn = (addr >> 8) & 0xff;
 	}
 
+	parent = of_get_parent(dn);
+	pdn = pci_alloc_pdn(parent ? PCI_DN(parent) : NULL,
+			    busno, devfn);
+	if (!pdn)
+		return NULL;
+
+	dn->data = pdn;
+	pdn->phb = hose;
+
 	/* vendor/device IDs and class code */
 	regs = of_get_property(dn, "vendor-id", NULL);
 	pdn->vendor_id = regs ? of_read_number(regs, 1) : 0;
@@ -380,14 +382,6 @@ struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 	}
 #endif
 
-	/* Attach to parent node */
-	INIT_LIST_HEAD(&pdn->child_list);
-	INIT_LIST_HEAD(&pdn->list);
-	parent = of_get_parent(dn);
-	pdn->parent = parent ? PCI_DN(parent) : NULL;
-	if (pdn->parent)
-		list_add_tail(&pdn->list, &pdn->parent->child_list);
-
 	return pdn;
 }
 EXPORT_SYMBOL_GPL(pci_add_device_node_info);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
returns zero, because the device is yet preparing to enable the VFs.

With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
on PowerNV.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/include/asm/pci-bridge.h     |  4 +--
 arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
 arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
 arch/powerpc/platforms/pseries/pci.c      |  4 +--
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index fc188e0e9179..6479bc96e0b6 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -225,8 +225,8 @@ struct pci_dn {
 extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 					   int devfn);
 extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
-extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
-extern void remove_dev_pci_data(struct pci_dev *pdev);
+extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
+extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
 extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 					       struct device_node *dn);
 extern void pci_remove_device_node_info(struct device_node *dn);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 7f12882d8882..7fa362f8038d 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
 	return pdn;
 }
 
-struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
+struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
 {
+	struct pci_dn *pdn = pci_get_pdn(pdev);
+
 #ifdef CONFIG_PCI_IOV
-	struct pci_dn *parent, *pdn;
+	struct pci_dn *parent;
 	int i;
 
 	/* Only support IOV for now */
 	if (!pdev->is_physfn)
-		return pci_get_pdn(pdev);
+		return pdn;
 
 	/* Check if VFs have been populated */
-	pdn = pci_get_pdn(pdev);
 	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
 		return NULL;
 
@@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 	if (!parent)
 		return NULL;
 
-	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
+	for (i = 0; i < num_vfs; i++) {
 		struct eeh_dev *edev __maybe_unused;
+		struct pci_dn *vpdn;
 
-		pdn = pci_alloc_pdn(parent,
-				    pci_iov_virtfn_bus(pdev, i),
-				    pci_iov_virtfn_devfn(pdev, i));
-		if (!pdn) {
+		vpdn = pci_alloc_pdn(parent,
+				     pci_iov_virtfn_bus(pdev, i),
+				     pci_iov_virtfn_devfn(pdev, i));
+		if (!vpdn) {
 			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
 				 __func__, i);
 			return NULL;
 		}
 
-		pdn->vf_index = i;
+		vpdn->vf_index = i;
+		vpdn->vendor_id = pdn->vendor_id;
+		vpdn->device_id = pdn->device_id;
+		vpdn->class_code = pdn->class_code;
+		vpdn->pci_ext_config_space = 0;
 
 #ifdef CONFIG_EEH
 		/* Create the EEH device for the VF */
-		edev = eeh_dev_init(pdn);
+		edev = eeh_dev_init(vpdn);
 		BUG_ON(!edev);
 		edev->physfn = pdev;
 #endif /* CONFIG_EEH */
 	}
 #endif /* CONFIG_PCI_IOV */
 
-	return pci_get_pdn(pdev);
+	return pdn;
 }
 
-void remove_dev_pci_data(struct pci_dev *pdev)
+void pci_destroy_vf_pdns(struct pci_dev *pdev)
 {
 #ifdef CONFIG_PCI_IOV
 	struct pci_dn *parent;
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index ed500f51d449..979c901535f2 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
 	pnv_pci_sriov_disable(pdev);
 
 	/* Release PCI data */
-	remove_dev_pci_data(pdev);
+	pci_destroy_vf_pdns(pdev);
 	return 0;
 }
 
 int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 {
 	/* Allocate PCI data */
-	add_dev_pci_data(pdev);
+	pci_create_vf_pdns(pdev, num_vfs);
 
 	return pnv_pci_sriov_enable(pdev, num_vfs);
 }
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 37a77e57893e..5e87596903a6 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 {
 	/* Allocate PCI data */
-	add_dev_pci_data(pdev);
+	pci_create_vf_pdns(pdev, num_vfs);
 	return pseries_pci_sriov_enable(pdev, num_vfs);
 }
 
@@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
 	/* Releasing pe_num_map */
 	kfree(pdn->pe_num_map);
 	/* Release PCI data */
-	remove_dev_pci_data(pdev);
+	pci_destroy_vf_pdns(pdev);
 	pci_vf_drivers_autoprobe(pdev, true);
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
returns zero, because the device is yet preparing to enable the VFs.

With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
on PowerNV.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/include/asm/pci-bridge.h     |  4 +--
 arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
 arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
 arch/powerpc/platforms/pseries/pci.c      |  4 +--
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
index fc188e0e9179..6479bc96e0b6 100644
--- a/arch/powerpc/include/asm/pci-bridge.h
+++ b/arch/powerpc/include/asm/pci-bridge.h
@@ -225,8 +225,8 @@ struct pci_dn {
 extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 					   int devfn);
 extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
-extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
-extern void remove_dev_pci_data(struct pci_dev *pdev);
+extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
+extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
 extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
 					       struct device_node *dn);
 extern void pci_remove_device_node_info(struct device_node *dn);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 7f12882d8882..7fa362f8038d 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
 	return pdn;
 }
 
-struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
+struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
 {
+	struct pci_dn *pdn = pci_get_pdn(pdev);
+
 #ifdef CONFIG_PCI_IOV
-	struct pci_dn *parent, *pdn;
+	struct pci_dn *parent;
 	int i;
 
 	/* Only support IOV for now */
 	if (!pdev->is_physfn)
-		return pci_get_pdn(pdev);
+		return pdn;
 
 	/* Check if VFs have been populated */
-	pdn = pci_get_pdn(pdev);
 	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
 		return NULL;
 
@@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 	if (!parent)
 		return NULL;
 
-	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
+	for (i = 0; i < num_vfs; i++) {
 		struct eeh_dev *edev __maybe_unused;
+		struct pci_dn *vpdn;
 
-		pdn = pci_alloc_pdn(parent,
-				    pci_iov_virtfn_bus(pdev, i),
-				    pci_iov_virtfn_devfn(pdev, i));
-		if (!pdn) {
+		vpdn = pci_alloc_pdn(parent,
+				     pci_iov_virtfn_bus(pdev, i),
+				     pci_iov_virtfn_devfn(pdev, i));
+		if (!vpdn) {
 			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
 				 __func__, i);
 			return NULL;
 		}
 
-		pdn->vf_index = i;
+		vpdn->vf_index = i;
+		vpdn->vendor_id = pdn->vendor_id;
+		vpdn->device_id = pdn->device_id;
+		vpdn->class_code = pdn->class_code;
+		vpdn->pci_ext_config_space = 0;
 
 #ifdef CONFIG_EEH
 		/* Create the EEH device for the VF */
-		edev = eeh_dev_init(pdn);
+		edev = eeh_dev_init(vpdn);
 		BUG_ON(!edev);
 		edev->physfn = pdev;
 #endif /* CONFIG_EEH */
 	}
 #endif /* CONFIG_PCI_IOV */
 
-	return pci_get_pdn(pdev);
+	return pdn;
 }
 
-void remove_dev_pci_data(struct pci_dev *pdev)
+void pci_destroy_vf_pdns(struct pci_dev *pdev)
 {
 #ifdef CONFIG_PCI_IOV
 	struct pci_dn *parent;
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index ed500f51d449..979c901535f2 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
 	pnv_pci_sriov_disable(pdev);
 
 	/* Release PCI data */
-	remove_dev_pci_data(pdev);
+	pci_destroy_vf_pdns(pdev);
 	return 0;
 }
 
 int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 {
 	/* Allocate PCI data */
-	add_dev_pci_data(pdev);
+	pci_create_vf_pdns(pdev, num_vfs);
 
 	return pnv_pci_sriov_enable(pdev, num_vfs);
 }
diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
index 37a77e57893e..5e87596903a6 100644
--- a/arch/powerpc/platforms/pseries/pci.c
+++ b/arch/powerpc/platforms/pseries/pci.c
@@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
 {
 	/* Allocate PCI data */
-	add_dev_pci_data(pdev);
+	pci_create_vf_pdns(pdev, num_vfs);
 	return pseries_pci_sriov_enable(pdev, num_vfs);
 }
 
@@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
 	/* Releasing pe_num_map */
 	kfree(pdn->pe_num_map);
 	/* Release PCI data */
-	remove_dev_pci_data(pdev);
+	pci_destroy_vf_pdns(pdev);
 	pci_vf_drivers_autoprobe(pdev, true);
 	return 0;
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 6/8] powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

If supported by the platform, endpoint's pci_dn can be created dynamically,
without need to wait for DT updates from the firmware.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c                 | 6 ++++--
 arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 7fa362f8038d..17362a9b4678 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -555,8 +555,10 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)
 		phb->pci_data = pdn;
 	}
 
-	/* Update dn->phb ptrs for new phb and children devices */
-	pci_traverse_device_nodes(dn, add_pdn, phb);
+	if (!pci_has_flag(PCI_REASSIGN_ALL_BUS)) {
+		/* Update dn->phb ptrs for new phb and children devices */
+		pci_traverse_device_nodes(dn, add_pdn, phb);
+	}
 }
 
 /** 
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index f38078976c5d..40feff2653a0 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 
-	if (!pdev->is_virtfn)
+	if (!pci_has_flag(PCI_REASSIGN_ALL_BUS) && !pdev->is_virtfn)
 		return;
 
 	/*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 6/8] powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

If supported by the platform, endpoint's pci_dn can be created dynamically,
without need to wait for DT updates from the firmware.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c                 | 6 ++++--
 arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 7fa362f8038d..17362a9b4678 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -555,8 +555,10 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)
 		phb->pci_data = pdn;
 	}
 
-	/* Update dn->phb ptrs for new phb and children devices */
-	pci_traverse_device_nodes(dn, add_pdn, phb);
+	if (!pci_has_flag(PCI_REASSIGN_ALL_BUS)) {
+		/* Update dn->phb ptrs for new phb and children devices */
+		pci_traverse_device_nodes(dn, add_pdn, phb);
+	}
 }
 
 /** 
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index f38078976c5d..40feff2653a0 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -47,7 +47,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 {
 	struct pci_dn *pdn = pci_get_pdn(pdev);
 
-	if (!pdev->is_virtfn)
+	if (!pci_has_flag(PCI_REASSIGN_ALL_BUS) && !pdev->is_virtfn)
 		return;
 
 	/*
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 7/8] powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

Writing a new value to the PCI_SECONDARY_BUS register of the bridge means
that its children will become addressable on another address (new B in BDF)
or even un-addressable if the secondary bus is set to zero.

On PowerNV, device PEs are heavily BDF-dependent, so they must be updated
on every such change of its address.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/platforms/powernv/pci.c | 118 ++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 8cc6661781e2..40f68955f34f 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -722,13 +722,127 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
 				    where, size, val);
 }
 
+static void invalidate_children_pes(struct pci_dn *pdn)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+	struct pci_dn *child;
+	bool found_pe = false;
+	int pe_num;
+	int pe_bus;
+
+	list_for_each_entry(child, &pdn->child_list, list) {
+		struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+			&phb->ioda.pe_array[child->pe_number] :
+			NULL;
+
+		if (!child->busno)
+			continue;
+
+		if ((child->class_code >> 8) == PCI_CLASS_BRIDGE_PCI)
+			invalidate_children_pes(child);
+
+		if (pe) {
+			u8 rid_bus = (pe->rid >> 8) & 0xff;
+
+			if (rid_bus) {
+				pe_num = child->pe_number;
+				pe_bus = rid_bus;
+				found_pe = true;
+			}
+
+			pe->rid &= 0xff;
+		}
+
+		child->busno = 0;
+	}
+
+	if (found_pe) {
+		u16 rid = pe_bus << 8;
+
+		opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_UNMAP_PE);
+	}
+}
+
+static u8 pre_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	u32 old_secondary_bus = 0;
+
+	if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+		return 0;
+
+	pnv_pci_cfg_read(pdn, PCI_SECONDARY_BUS, 1, &old_secondary_bus);
+	old_secondary_bus &= 0xff;
+
+	if (old_secondary_bus != new_secondary_bus)
+		invalidate_children_pes(pdn);
+
+	return old_secondary_bus;
+}
+
+static void update_children_pes(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+	struct pci_dn *child;
+	bool found_pe = false;
+	int pe_num;
+
+	if (!new_secondary_bus)
+		return;
+
+	list_for_each_entry(child, &pdn->child_list, list) {
+		struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+			&phb->ioda.pe_array[child->pe_number] :
+			NULL;
+
+		if (child->busno)
+			continue;
+
+		child->busno = new_secondary_bus;
+
+		if (pe) {
+			pe->rid |= (child->busno << 8);
+			pe_num = child->pe_number;
+			found_pe = true;
+		}
+	}
+
+	if (found_pe) {
+		u16 rid = new_secondary_bus << 8;
+
+		opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_MAP_PE);
+	}
+}
+
+static void post_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+		return;
+
+	update_children_pes(pdn, new_secondary_bus);
+}
+
 int pnv_pci_cfg_write(struct pci_dn *pdn,
 		      int where, int size, u32 val)
 {
 	struct pnv_phb *phb = pdn->phb->private_data;
+	u8 old_secondary_bus = 0, new_secondary_bus = 0;
+	int rc;
+
+	if (where == PCI_SECONDARY_BUS) {
+		new_secondary_bus = val & 0xff;
+		old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+	} else if (where == PCI_PRIMARY_BUS && size > 1) {
+		new_secondary_bus = (val >> 8) & 0xff;
+		old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+	}
 
-	return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
-				     where, size, val);
+	rc = pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				   where, size, val);
+
+	if (new_secondary_bus && old_secondary_bus != new_secondary_bus)
+		post_hook_new_sec_bus(pdn, new_secondary_bus);
+
+	return rc;
 }
 
 #if CONFIG_EEH
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 7/8] powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

Writing a new value to the PCI_SECONDARY_BUS register of the bridge means
that its children will become addressable on another address (new B in BDF)
or even un-addressable if the secondary bus is set to zero.

On PowerNV, device PEs are heavily BDF-dependent, so they must be updated
on every such change of its address.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/platforms/powernv/pci.c | 118 ++++++++++++++++++++++++++-
 1 file changed, 116 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 8cc6661781e2..40f68955f34f 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -722,13 +722,127 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
 				    where, size, val);
 }
 
+static void invalidate_children_pes(struct pci_dn *pdn)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+	struct pci_dn *child;
+	bool found_pe = false;
+	int pe_num;
+	int pe_bus;
+
+	list_for_each_entry(child, &pdn->child_list, list) {
+		struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+			&phb->ioda.pe_array[child->pe_number] :
+			NULL;
+
+		if (!child->busno)
+			continue;
+
+		if ((child->class_code >> 8) == PCI_CLASS_BRIDGE_PCI)
+			invalidate_children_pes(child);
+
+		if (pe) {
+			u8 rid_bus = (pe->rid >> 8) & 0xff;
+
+			if (rid_bus) {
+				pe_num = child->pe_number;
+				pe_bus = rid_bus;
+				found_pe = true;
+			}
+
+			pe->rid &= 0xff;
+		}
+
+		child->busno = 0;
+	}
+
+	if (found_pe) {
+		u16 rid = pe_bus << 8;
+
+		opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_UNMAP_PE);
+	}
+}
+
+static u8 pre_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	u32 old_secondary_bus = 0;
+
+	if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+		return 0;
+
+	pnv_pci_cfg_read(pdn, PCI_SECONDARY_BUS, 1, &old_secondary_bus);
+	old_secondary_bus &= 0xff;
+
+	if (old_secondary_bus != new_secondary_bus)
+		invalidate_children_pes(pdn);
+
+	return old_secondary_bus;
+}
+
+static void update_children_pes(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	struct pnv_phb *phb = pdn->phb->private_data;
+	struct pci_dn *child;
+	bool found_pe = false;
+	int pe_num;
+
+	if (!new_secondary_bus)
+		return;
+
+	list_for_each_entry(child, &pdn->child_list, list) {
+		struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+			&phb->ioda.pe_array[child->pe_number] :
+			NULL;
+
+		if (child->busno)
+			continue;
+
+		child->busno = new_secondary_bus;
+
+		if (pe) {
+			pe->rid |= (child->busno << 8);
+			pe_num = child->pe_number;
+			found_pe = true;
+		}
+	}
+
+	if (found_pe) {
+		u16 rid = new_secondary_bus << 8;
+
+		opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_MAP_PE);
+	}
+}
+
+static void post_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+	if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+		return;
+
+	update_children_pes(pdn, new_secondary_bus);
+}
+
 int pnv_pci_cfg_write(struct pci_dn *pdn,
 		      int where, int size, u32 val)
 {
 	struct pnv_phb *phb = pdn->phb->private_data;
+	u8 old_secondary_bus = 0, new_secondary_bus = 0;
+	int rc;
+
+	if (where == PCI_SECONDARY_BUS) {
+		new_secondary_bus = val & 0xff;
+		old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+	} else if (where == PCI_PRIMARY_BUS && size > 1) {
+		new_secondary_bus = (val >> 8) & 0xff;
+		old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+	}
 
-	return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
-				     where, size, val);
+	rc = pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+				   where, size, val);
+
+	if (new_secondary_bus && old_secondary_bus != new_secondary_bus)
+		post_hook_new_sec_bus(pdn, new_secondary_bus);
+
+	return rc;
 }
 
 #if CONFIG_EEH
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 8/8] powerpc/powernv/pci: Enable reassigning the bus numbers
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Oliver O'Halloran, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux,
	Sergey Miroshnichenko

When the pci=realloc command line switch is enabled (which should only be
set when working on on top of the skiboot with the "core/pci: Sync VFs and
the changes of bdfns between the firmware and the OS" patch serie applied),
PowerNV will not depend on PCIe topology info from DT anymore.

This makes possible to re-enumerate the fabric, assign the new bus numbers
and switch from the pnv_php module to the standard pciehp driver for PCIe
hotplug functionality.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 17362a9b4678..9437af1a3b20 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -595,3 +595,15 @@ static void pci_dev_pdn_setup(struct pci_dev *pdev)
 	pdev->dev.archdata.pci_data = pdn;
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, pci_dev_pdn_setup);
+
+char * __init pcibios_setup(char *str)
+{
+	if (!strncmp(str, "realloc=", 8)) {
+		if (!strncmp(str + 8, "on", 2))
+			pci_add_flags(PCI_REASSIGN_ALL_BUS);
+	} else if (!strncmp(str, "realloc", 7)) {
+		pci_add_flags(PCI_REASSIGN_ALL_BUS);
+	}
+
+	return str;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* [PATCH v5 8/8] powerpc/powernv/pci: Enable reassigning the bus numbers
@ 2019-03-11 11:52   ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-03-11 11:52 UTC (permalink / raw)
  To: linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, Oliver O'Halloran

When the pci=realloc command line switch is enabled (which should only be
set when working on on top of the skiboot with the "core/pci: Sync VFs and
the changes of bdfns between the firmware and the OS" patch serie applied),
PowerNV will not depend on PCIe topology info from DT anymore.

This makes possible to re-enumerate the fabric, assign the new bus numbers
and switch from the pnv_php module to the standard pciehp driver for PCIe
hotplug functionality.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/kernel/pci_dn.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 17362a9b4678..9437af1a3b20 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -595,3 +595,15 @@ static void pci_dev_pdn_setup(struct pci_dev *pdev)
 	pdev->dev.archdata.pci_data = pdn;
 }
 DECLARE_PCI_FIXUP_EARLY(PCI_ANY_ID, PCI_ANY_ID, pci_dev_pdn_setup);
+
+char * __init pcibios_setup(char *str)
+{
+	if (!strncmp(str, "realloc=", 8)) {
+		if (!strncmp(str + 8, "on", 2))
+			pci_add_flags(PCI_REASSIGN_ALL_BUS);
+	} else if (!strncmp(str, "realloc", 7)) {
+		pci_add_flags(PCI_REASSIGN_ALL_BUS);
+	}
+
+	return str;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
  2019-03-11 11:52 ` Sergey Miroshnichenko
@ 2019-03-27 14:10   ` Bjorn Helgaas
  -1 siblings, 0 replies; 30+ messages in thread
From: Bjorn Helgaas @ 2019-03-27 14:10 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: linuxppc-dev, linux-pci, Oliver O'Halloran, Stewart Smith,
	Alexey Kardashevskiy, Benjamin Herrenschmidt, Russell Currey,
	linux

Hi Sergey,

Since this doesn't touch drivers/pci, I assume powerpc folks will
handle this series.  Let me know if otherwise.

On Mon, Mar 11, 2019 at 02:52:25PM +0300, Sergey Miroshnichenko wrote:
> This patchset allows switching from the pnv_php module to the standard
> pciehp driver for PCIe hotplug functionality, if the platform supports it:
> PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
> the changes of bdfns between the firmware and the OS" [1] patch serie
> applied.

s/bdfns/BDFs/  Maybe?  I see this is a reference to another patch
  series, but if it hasn't been merged yet, "BDFs" would be consistent
  with "VFs" and give a hint that "bdfns" is not itself a word.

s/serie/series/

> The feature is activated by the "pci=realloc" command line argument.

From a user point of view, it doesn't seem intuitive that
"pci=realloc" also means "switch from pnv_php to pciehp".

The only direct effect of "pci=realloc" is to set pci_realloc_enable.
I haven't read the patches, but is there really something in
arch/powerpc/ that does something different based on
pci_realloc_enable?

> The goal is ability to hotplug bridges full of devices in the future. The
> "Movable BARs" [2] is a platform-independent part of our work in this. The
> final part will be movable bus numbers to support inserting a bridge in the
> middle of an existing PCIe tree.
> 
> Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
>  - the pciehp driver active;
>  - the pnv_php driver disabled;
>  - The "pci=realloc" argument is passed;
>  - surprise hotplug of an NVME disk works;
>  - controlled hotplug of a network card with SR-IOV works;
>  - activating of SR-IOV on a network card works;
>  - [with extra patches] manually initiated (via sysfs) rescan has found
>    and turned on a hotplugged bridge;
>  - Without "pci=realloc" works just as before.
> 
> Changes since v4:
>  - Fixed failing build when EEH is disabled in a kernel config;
>  - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
>  - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
>  - Renamed create_pdn() -> pci_create_pdn_from_dev();
>  - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
>  - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
>  - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
>  - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
>    commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
>    VFs")
> 
> Changes since v3 [3]:
>  - Subject changed;
>  - Don't disable EEH during rescan anymore - instead just unfreeze the
>    target buses deliberately;
>  - Add synchronization with the firmware when changing the PCIe topology;
>  - Fixed for VFs;
>  - Code cleanup.
> 
> Changes since v2:
>  - Don't reassign bus numbers on PowerNV by default (to retain the default
>    behavior), but only when pci=realloc is passed;
>  - Less code affected;
>  - pci_add_device_node_info is refactored with add_one_dev_pci_data;
>  - Minor code cleanup.
> 
> Changes since v1:
>  - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
>  - Fixed build for ppc64e when CONFIG_EEH is disabled;
>  - Fixed code style warnings.
> 
> [1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
> [2] https://www.spinics.net/lists/linux-pci/msg79995.html
> [3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html
> 
> Sergey Miroshnichenko (8):
>   powerpc/pci: Access PCI config space directly w/o pci_dn
>   powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
>   powerpc/pci: Create pci_dn on demand
>   powerpc/pci: Reduce code duplication in pci_add_device_node_info
>   powerpc/pci/IOV: Add support for runtime enabling the VFs
>   powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
>   powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
>   powerpc/powernv/pci: Enable reassigning the bus numbers
> 
>  arch/powerpc/include/asm/pci-bridge.h        |   4 +-
>  arch/powerpc/include/asm/ppc-pci.h           |   1 +
>  arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
>  arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
>  arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
>  arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
>  arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
>  arch/powerpc/platforms/pseries/pci.c         |   4 +-
>  8 files changed, 379 insertions(+), 108 deletions(-)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
@ 2019-03-27 14:10   ` Bjorn Helgaas
  0 siblings, 0 replies; 30+ messages in thread
From: Bjorn Helgaas @ 2019-03-27 14:10 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: Stewart Smith, Alexey Kardashevskiy, linux-pci, linux,
	Oliver O'Halloran, linuxppc-dev

Hi Sergey,

Since this doesn't touch drivers/pci, I assume powerpc folks will
handle this series.  Let me know if otherwise.

On Mon, Mar 11, 2019 at 02:52:25PM +0300, Sergey Miroshnichenko wrote:
> This patchset allows switching from the pnv_php module to the standard
> pciehp driver for PCIe hotplug functionality, if the platform supports it:
> PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
> the changes of bdfns between the firmware and the OS" [1] patch serie
> applied.

s/bdfns/BDFs/  Maybe?  I see this is a reference to another patch
  series, but if it hasn't been merged yet, "BDFs" would be consistent
  with "VFs" and give a hint that "bdfns" is not itself a word.

s/serie/series/

> The feature is activated by the "pci=realloc" command line argument.

From a user point of view, it doesn't seem intuitive that
"pci=realloc" also means "switch from pnv_php to pciehp".

The only direct effect of "pci=realloc" is to set pci_realloc_enable.
I haven't read the patches, but is there really something in
arch/powerpc/ that does something different based on
pci_realloc_enable?

> The goal is ability to hotplug bridges full of devices in the future. The
> "Movable BARs" [2] is a platform-independent part of our work in this. The
> final part will be movable bus numbers to support inserting a bridge in the
> middle of an existing PCIe tree.
> 
> Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
>  - the pciehp driver active;
>  - the pnv_php driver disabled;
>  - The "pci=realloc" argument is passed;
>  - surprise hotplug of an NVME disk works;
>  - controlled hotplug of a network card with SR-IOV works;
>  - activating of SR-IOV on a network card works;
>  - [with extra patches] manually initiated (via sysfs) rescan has found
>    and turned on a hotplugged bridge;
>  - Without "pci=realloc" works just as before.
> 
> Changes since v4:
>  - Fixed failing build when EEH is disabled in a kernel config;
>  - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
>  - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
>  - Renamed create_pdn() -> pci_create_pdn_from_dev();
>  - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
>  - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
>  - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
>  - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
>    commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
>    VFs")
> 
> Changes since v3 [3]:
>  - Subject changed;
>  - Don't disable EEH during rescan anymore - instead just unfreeze the
>    target buses deliberately;
>  - Add synchronization with the firmware when changing the PCIe topology;
>  - Fixed for VFs;
>  - Code cleanup.
> 
> Changes since v2:
>  - Don't reassign bus numbers on PowerNV by default (to retain the default
>    behavior), but only when pci=realloc is passed;
>  - Less code affected;
>  - pci_add_device_node_info is refactored with add_one_dev_pci_data;
>  - Minor code cleanup.
> 
> Changes since v1:
>  - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
>  - Fixed build for ppc64e when CONFIG_EEH is disabled;
>  - Fixed code style warnings.
> 
> [1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
> [2] https://www.spinics.net/lists/linux-pci/msg79995.html
> [3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html
> 
> Sergey Miroshnichenko (8):
>   powerpc/pci: Access PCI config space directly w/o pci_dn
>   powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
>   powerpc/pci: Create pci_dn on demand
>   powerpc/pci: Reduce code duplication in pci_add_device_node_info
>   powerpc/pci/IOV: Add support for runtime enabling the VFs
>   powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
>   powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
>   powerpc/powernv/pci: Enable reassigning the bus numbers
> 
>  arch/powerpc/include/asm/pci-bridge.h        |   4 +-
>  arch/powerpc/include/asm/ppc-pci.h           |   1 +
>  arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
>  arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
>  arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
>  arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
>  arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
>  arch/powerpc/platforms/pseries/pci.c         |   4 +-
>  8 files changed, 379 insertions(+), 108 deletions(-)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
  2019-03-27 14:10   ` Bjorn Helgaas
@ 2019-03-28 12:44     ` Oliver
  -1 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-03-28 12:44 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Sergey Miroshnichenko, linuxppc-dev, linux-pci, Stewart Smith,
	Alexey Kardashevskiy, Benjamin Herrenschmidt, Russell Currey,
	linux

On Thu, Mar 28, 2019 at 1:10 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> Hi Sergey,
>
> Since this doesn't touch drivers/pci, I assume powerpc folks will
> handle this series.  Let me know if otherwise.

I've been looking at it and reviewed the last spin. I'll have another
look next week.

> On Mon, Mar 11, 2019 at 02:52:25PM +0300, Sergey Miroshnichenko wrote:
> > This patchset allows switching from the pnv_php module to the standard
> > pciehp driver for PCIe hotplug functionality, if the platform supports it:
> > PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
> > the changes of bdfns between the firmware and the OS" [1] patch serie
> > applied.
>
> s/bdfns/BDFs/  Maybe?  I see this is a reference to another patch
>   series, but if it hasn't been merged yet, "BDFs" would be consistent
>   with "VFs" and give a hint that "bdfns" is not itself a word.
>
> s/serie/series/
>
> > The feature is activated by the "pci=realloc" command line argument.
>
> From a user point of view, it doesn't seem intuitive that
> "pci=realloc" also means "switch from pnv_php to pciehp".

I think he means something more along the lines of "allows pciehp to
be used instead of pnv_php." Currently pnv_php is the only way to
hotplug devices on PowerNV because of:

a) Legacy assumptions from pseries about PCI devices always having a
corresponding DT node,
b) Firmware being responsible for assigning bus numbers on PowerNV, and
c) Our root ports not implementing most of the PCIe slot capabilities.

There's no real reason why a) needs to be the case and part of this
series addresses that. It's a similar story for b) which is a
side-effect of supporting Power7 hardware which used a fixed mapping
between bus numbers and EEH error domains (PEs). Power8 and Power9 use
a different method for mapping devices to PEs so there's no real
reason to enforce the restriction on modern hardware. c) is still a
problem, but it's a non-issue for switch ports. Fixing a) is the only
real requirement to allow pciehp to be used, but IIRC Sergey is
interested in hotplugging entire racks of NVMe drives so he needs b)
fixed too.

I don't think passing pci=realloc is the best way to handle enabling
bus number re-assignments. Fundamentally being able to re-assign bus
numbers depends on the system/firmware supporting it so I think it
would make more sense for firmware to advertise the capability in the
DT and have the kernel enable it automatically when it can. That said,
it's worth pointing out that everyone's favourite init system will use
the bus number in it's "persistent" network device names so changing
the bus number assignment policy can cause a bit of grief. Making it a
per-PHB flag might help there.

> The only direct effect of "pci=realloc" is to set pci_realloc_enable.
> I haven't read the patches, but is there really something in
> arch/powerpc/ that does something different based on
> pci_realloc_enable?

I don't think we use that flag at all. Patch 8/8 of this series adds a
pcibios_setup() hook that sets the PCI_REASSIGN_ALL_BUS flag when
pci=realloc is in the command line. I need to have a closer look into
what that actually does though.





> > The goal is ability to hotplug bridges full of devices in the future. The
> > "Movable BARs" [2] is a platform-independent part of our work in this. The
> > final part will be movable bus numbers to support inserting a bridge in the
> > middle of an existing PCIe tree.
> >
> > Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
> >  - the pciehp driver active;
> >  - the pnv_php driver disabled;
> >  - The "pci=realloc" argument is passed;
> >  - surprise hotplug of an NVME disk works;
> >  - controlled hotplug of a network card with SR-IOV works;
> >  - activating of SR-IOV on a network card works;
> >  - [with extra patches] manually initiated (via sysfs) rescan has found
> >    and turned on a hotplugged bridge;
> >  - Without "pci=realloc" works just as before.
> >
> > Changes since v4:
> >  - Fixed failing build when EEH is disabled in a kernel config;
> >  - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
> >  - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
> >  - Renamed create_pdn() -> pci_create_pdn_from_dev();
> >  - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
> >  - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
> >  - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
> >  - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
> >    commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
> >    VFs")
> >
> > Changes since v3 [3]:
> >  - Subject changed;
> >  - Don't disable EEH during rescan anymore - instead just unfreeze the
> >    target buses deliberately;
> >  - Add synchronization with the firmware when changing the PCIe topology;
> >  - Fixed for VFs;
> >  - Code cleanup.
> >
> > Changes since v2:
> >  - Don't reassign bus numbers on PowerNV by default (to retain the default
> >    behavior), but only when pci=realloc is passed;
> >  - Less code affected;
> >  - pci_add_device_node_info is refactored with add_one_dev_pci_data;
> >  - Minor code cleanup.
> >
> > Changes since v1:
> >  - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
> >  - Fixed build for ppc64e when CONFIG_EEH is disabled;
> >  - Fixed code style warnings.
> >
> > [1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
> > [2] https://www.spinics.net/lists/linux-pci/msg79995.html
> > [3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html
> >
> > Sergey Miroshnichenko (8):
> >   powerpc/pci: Access PCI config space directly w/o pci_dn
> >   powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
> >   powerpc/pci: Create pci_dn on demand
> >   powerpc/pci: Reduce code duplication in pci_add_device_node_info
> >   powerpc/pci/IOV: Add support for runtime enabling the VFs
> >   powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
> >   powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
> >   powerpc/powernv/pci: Enable reassigning the bus numbers
> >
> >  arch/powerpc/include/asm/pci-bridge.h        |   4 +-
> >  arch/powerpc/include/asm/ppc-pci.h           |   1 +
> >  arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
> >  arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
> >  arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
> >  arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
> >  arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
> >  arch/powerpc/platforms/pseries/pci.c         |   4 +-
> >  8 files changed, 379 insertions(+), 108 deletions(-)
> >
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient,  independent of FW and DT
@ 2019-03-28 12:44     ` Oliver
  0 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-03-28 12:44 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Stewart Smith, Alexey Kardashevskiy, Sergey Miroshnichenko,
	linux, linux-pci, linuxppc-dev

On Thu, Mar 28, 2019 at 1:10 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> Hi Sergey,
>
> Since this doesn't touch drivers/pci, I assume powerpc folks will
> handle this series.  Let me know if otherwise.

I've been looking at it and reviewed the last spin. I'll have another
look next week.

> On Mon, Mar 11, 2019 at 02:52:25PM +0300, Sergey Miroshnichenko wrote:
> > This patchset allows switching from the pnv_php module to the standard
> > pciehp driver for PCIe hotplug functionality, if the platform supports it:
> > PowerNV working on on top of the skiboot with the "core/pci: Sync VFs and
> > the changes of bdfns between the firmware and the OS" [1] patch serie
> > applied.
>
> s/bdfns/BDFs/  Maybe?  I see this is a reference to another patch
>   series, but if it hasn't been merged yet, "BDFs" would be consistent
>   with "VFs" and give a hint that "bdfns" is not itself a word.
>
> s/serie/series/
>
> > The feature is activated by the "pci=realloc" command line argument.
>
> From a user point of view, it doesn't seem intuitive that
> "pci=realloc" also means "switch from pnv_php to pciehp".

I think he means something more along the lines of "allows pciehp to
be used instead of pnv_php." Currently pnv_php is the only way to
hotplug devices on PowerNV because of:

a) Legacy assumptions from pseries about PCI devices always having a
corresponding DT node,
b) Firmware being responsible for assigning bus numbers on PowerNV, and
c) Our root ports not implementing most of the PCIe slot capabilities.

There's no real reason why a) needs to be the case and part of this
series addresses that. It's a similar story for b) which is a
side-effect of supporting Power7 hardware which used a fixed mapping
between bus numbers and EEH error domains (PEs). Power8 and Power9 use
a different method for mapping devices to PEs so there's no real
reason to enforce the restriction on modern hardware. c) is still a
problem, but it's a non-issue for switch ports. Fixing a) is the only
real requirement to allow pciehp to be used, but IIRC Sergey is
interested in hotplugging entire racks of NVMe drives so he needs b)
fixed too.

I don't think passing pci=realloc is the best way to handle enabling
bus number re-assignments. Fundamentally being able to re-assign bus
numbers depends on the system/firmware supporting it so I think it
would make more sense for firmware to advertise the capability in the
DT and have the kernel enable it automatically when it can. That said,
it's worth pointing out that everyone's favourite init system will use
the bus number in it's "persistent" network device names so changing
the bus number assignment policy can cause a bit of grief. Making it a
per-PHB flag might help there.

> The only direct effect of "pci=realloc" is to set pci_realloc_enable.
> I haven't read the patches, but is there really something in
> arch/powerpc/ that does something different based on
> pci_realloc_enable?

I don't think we use that flag at all. Patch 8/8 of this series adds a
pcibios_setup() hook that sets the PCI_REASSIGN_ALL_BUS flag when
pci=realloc is in the command line. I need to have a closer look into
what that actually does though.





> > The goal is ability to hotplug bridges full of devices in the future. The
> > "Movable BARs" [2] is a platform-independent part of our work in this. The
> > final part will be movable bus numbers to support inserting a bridge in the
> > middle of an existing PCIe tree.
> >
> > Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
> >  - the pciehp driver active;
> >  - the pnv_php driver disabled;
> >  - The "pci=realloc" argument is passed;
> >  - surprise hotplug of an NVME disk works;
> >  - controlled hotplug of a network card with SR-IOV works;
> >  - activating of SR-IOV on a network card works;
> >  - [with extra patches] manually initiated (via sysfs) rescan has found
> >    and turned on a hotplugged bridge;
> >  - Without "pci=realloc" works just as before.
> >
> > Changes since v4:
> >  - Fixed failing build when EEH is disabled in a kernel config;
> >  - Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
> >  - Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
> >  - Renamed create_pdn() -> pci_create_pdn_from_dev();
> >  - Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
> >  - Renamed add_dev_pci_data() -> pci_create_vf_pdns();
> >  - Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
> >  - Removed the patch fixing uninitialized IOMMU group - now it is fixed in
> >    commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
> >    VFs")
> >
> > Changes since v3 [3]:
> >  - Subject changed;
> >  - Don't disable EEH during rescan anymore - instead just unfreeze the
> >    target buses deliberately;
> >  - Add synchronization with the firmware when changing the PCIe topology;
> >  - Fixed for VFs;
> >  - Code cleanup.
> >
> > Changes since v2:
> >  - Don't reassign bus numbers on PowerNV by default (to retain the default
> >    behavior), but only when pci=realloc is passed;
> >  - Less code affected;
> >  - pci_add_device_node_info is refactored with add_one_dev_pci_data;
> >  - Minor code cleanup.
> >
> > Changes since v1:
> >  - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
> >  - Fixed build for ppc64e when CONFIG_EEH is disabled;
> >  - Fixed code style warnings.
> >
> > [1] https://lists.ozlabs.org/pipermail/skiboot/2019-March/013571.html
> > [2] https://www.spinics.net/lists/linux-pci/msg79995.html
> > [3] https://lists.ozlabs.org/pipermail/linuxppc-dev/2018-September/178053.html
> >
> > Sergey Miroshnichenko (8):
> >   powerpc/pci: Access PCI config space directly w/o pci_dn
> >   powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
> >   powerpc/pci: Create pci_dn on demand
> >   powerpc/pci: Reduce code duplication in pci_add_device_node_info
> >   powerpc/pci/IOV: Add support for runtime enabling the VFs
> >   powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set
> >   powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
> >   powerpc/powernv/pci: Enable reassigning the bus numbers
> >
> >  arch/powerpc/include/asm/pci-bridge.h        |   4 +-
> >  arch/powerpc/include/asm/ppc-pci.h           |   1 +
> >  arch/powerpc/kernel/pci_dn.c                 | 170 ++++++++++-----
> >  arch/powerpc/kernel/rtas_pci.c               |  97 ++++++---
> >  arch/powerpc/platforms/powernv/eeh-powernv.c |   2 +-
> >  arch/powerpc/platforms/powernv/pci-ioda.c    |   4 +-
> >  arch/powerpc/platforms/powernv/pci.c         | 205 +++++++++++++++++--
> >  arch/powerpc/platforms/pseries/pci.c         |   4 +-
> >  8 files changed, 379 insertions(+), 108 deletions(-)
> >
> > --
> > 2.20.1
> >

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/8] powerpc/pci: Access PCI config space directly w/o pci_dn
  2019-03-11 11:52   ` Sergey Miroshnichenko
@ 2019-04-30  4:22     ` Oliver
  -1 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-04-30  4:22 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: linuxppc-dev, linux-pci, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux

On Mon, Mar 11, 2019 at 10:52 PM Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
>
> To fetch an updated DT for the newly hotplugged device, OS must explicitly
> request it from the firmware via the pnv_php driver.
>
> If pnv_php wasn't triggered/loaded, it is still possible to discover new
> devices if PCIe I/O will not stop in absence of the pci_dn structure.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/kernel/rtas_pci.c       | 97 +++++++++++++++++++---------
>  arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
>  2 files changed, 109 insertions(+), 52 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
> index c2b148b1634a..f675b5ecb5bc 100644
> --- a/arch/powerpc/kernel/rtas_pci.c
> +++ b/arch/powerpc/kernel/rtas_pci.c
> @@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
>         return 0;
>  }
>
> -int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
> +static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
> +                               int where, int size, u32 *val)
>  {
>         int returnval = -1;
> -       unsigned long buid, addr;
> +       unsigned long addr = rtas_config_addr(busno, devfn, where);
> +       int ret;
> +
> +       if (buid) {
> +               ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
> +                               addr, BUID_HI(buid), BUID_LO(buid), size);
> +       } else {
> +               ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
> +       }
> +       *val = returnval;
> +
> +       return ret;
> +}
> +
> +int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
> +{
>         int ret;
>
>         if (!pdn)
> @@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>                 return PCIBIOS_SET_FAILED;
>  #endif
>
> -       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> -       buid = pdn->phb->buid;
> -       if (buid) {
> -               ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
> -                               addr, BUID_HI(buid), BUID_LO(buid), size);
> -       } else {
> -               ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
> -       }
> -       *val = returnval;
> -
> +       ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
> +                                  where, size, val);
>         if (ret)
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>
> @@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
>
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>
> -       /* Validity of pdn is checked in here */
> -       ret = rtas_read_config(pdn, where, size, val);
> -       if (*val == EEH_IO_ERROR_VALUE(size) &&
> -           eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +       if (pdn) {
> +               /* Validity of pdn is checked in here */
> +               ret = rtas_read_config(pdn, where, size, val);
> +
> +               if (*val == EEH_IO_ERROR_VALUE(size) &&
> +                   eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
> +                       ret = PCIBIOS_DEVICE_NOT_FOUND;
> +       } else {
> +               struct pci_controller *phb = pci_bus_to_host(bus);
> +
> +               ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
> +                                          where, size, val);
> +       }
>
>         return ret;
>  }
>
> +static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
> +                                int where, int size, u32 val)
> +{
> +       unsigned long addr = rtas_config_addr(busno, devfn, where);
> +       int ret;
> +
> +       if (buid) {
> +               ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
> +                               BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
> +       } else {
> +               ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
> +       }
> +
> +       if (ret)
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
>  int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>  {
> -       unsigned long buid, addr;
>         int ret;
>
>         if (!pdn)
> @@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>                 return PCIBIOS_SET_FAILED;
>  #endif
>
> -       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> -       buid = pdn->phb->buid;
> -       if (buid) {
> -               ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
> -                       BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
> -       } else {
> -               ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
> -       }
> -
> +       ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
> +                                   where, size, val);
>         if (ret)
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>
> @@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
>                                  unsigned int devfn,
>                                  int where, int size, u32 val)
>  {
> -       struct pci_dn *pdn;
> +       struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
> +       int ret;
>
> -       pdn = pci_get_pdn_by_devfn(bus, devfn);
> +       if (pdn) {
> +               /* Validity of pdn is checked in here. */
> +               ret = rtas_write_config(pdn, where, size, val);
> +       } else {
> +               struct pci_controller *phb = pci_bus_to_host(bus);
>
> -       /* Validity of pdn is checked in here. */
> -       return rtas_write_config(pdn, where, size, val);
> +               ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
> +                                           where, size, val);
> +       }
> +
> +       return ret;
>  }
>
>  static struct pci_ops rtas_pci_ops = {
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index ef9448a907c6..41a381dfc2a1 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -652,30 +652,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
>         }
>  }
>
> -int pnv_pci_cfg_read(struct pci_dn *pdn,
> -                    int where, int size, u32 *val)
> +static int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
> +                               int where, int size, u32 *val)
>  {
> -       struct pnv_phb *phb = pdn->phb->private_data;
> -       u32 bdfn = (pdn->busno << 8) | pdn->devfn;
> +       u32 bdfn = (busno << 8) | devfn;
>         s64 rc;
>
>         switch (size) {
>         case 1: {
>                 u8 v8;
> -               rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
> +               rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
>                 *val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
>                 break;
>         }
>         case 2: {
>                 __be16 v16;
> -               rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
> -                                                  &v16);
> +               rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
> +                                                   &v16);
>                 *val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
>                 break;
>         }
>         case 4: {
>                 __be32 v32;
> -               rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
> +               rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
>                 *val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
>                 break;
>         }
> @@ -684,27 +683,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
>         }
>
>         pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
> -                __func__, pdn->busno, pdn->devfn, where, size, *val);
> +                __func__, busno, devfn, where, size, *val);
> +
>         return PCIBIOS_SUCCESSFUL;
>  }
>
> -int pnv_pci_cfg_write(struct pci_dn *pdn,
> -                     int where, int size, u32 val)
> +static int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
> +                                int where, int size, u32 val)
>  {
> -       struct pnv_phb *phb = pdn->phb->private_data;
> -       u32 bdfn = (pdn->busno << 8) | pdn->devfn;
> +       u32 bdfn = (busno << 8) | devfn;
>
>         pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
> -                __func__, pdn->busno, pdn->devfn, where, size, val);
> +                __func__, busno, devfn, where, size, val);
> +
>         switch (size) {
>         case 1:
> -               opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_byte(phb_id, bdfn, where, val);
>                 break;
>         case 2:
> -               opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_half_word(phb_id, bdfn, where, val);
>                 break;
>         case 4:
> -               opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_word(phb_id, bdfn, where, val);
>                 break;
>         default:
>                 return PCIBIOS_FUNC_NOT_SUPPORTED;
> @@ -713,6 +713,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
>         return PCIBIOS_SUCCESSFUL;
>  }
>
> +int pnv_pci_cfg_read(struct pci_dn *pdn,
> +                    int where, int size, u32 *val)
> +{
> +       struct pnv_phb *phb = pdn->phb->private_data;
> +
> +       return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
> +                                   where, size, val);
> +}
> +
> +int pnv_pci_cfg_write(struct pci_dn *pdn,
> +                     int where, int size, u32 val)
> +{
> +       struct pnv_phb *phb = pdn->phb->private_data;
> +
> +       return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
> +                                    where, size, val);
> +}
> +
>  #if CONFIG_EEH
>  static bool pnv_pci_cfg_check(struct pci_dn *pdn)
>  {
> @@ -748,13 +766,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>                                int where, int size, u32 *val)
>  {
>         struct pci_dn *pdn;
> -       struct pnv_phb *phb;
> +       struct pci_controller *hose = pci_bus_to_host(bus);
> +       struct pnv_phb *phb = hose->private_data;
>         int ret;
>
>         *val = 0xFFFFFFFF;
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>         if (!pdn)
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +               return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> +                                           where, size, val);
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> @@ -777,12 +797,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
>                                 int where, int size, u32 val)
>  {
>         struct pci_dn *pdn;
> -       struct pnv_phb *phb;
> +       struct pci_controller *hose = pci_bus_to_host(bus);
> +       struct pnv_phb *phb = hose->private_data;
>         int ret;
>
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>         if (!pdn)
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +               return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
> +                                            where, size, val);
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> --
> 2.20.1
>

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 1/8] powerpc/pci: Access PCI config space directly w/o pci_dn
@ 2019-04-30  4:22     ` Oliver
  0 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-04-30  4:22 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: Stewart Smith, Alexey Kardashevskiy, linux-pci, linux, linuxppc-dev

On Mon, Mar 11, 2019 at 10:52 PM Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
>
> To fetch an updated DT for the newly hotplugged device, OS must explicitly
> request it from the firmware via the pnv_php driver.
>
> If pnv_php wasn't triggered/loaded, it is still possible to discover new
> devices if PCIe I/O will not stop in absence of the pci_dn structure.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/kernel/rtas_pci.c       | 97 +++++++++++++++++++---------
>  arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
>  2 files changed, 109 insertions(+), 52 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
> index c2b148b1634a..f675b5ecb5bc 100644
> --- a/arch/powerpc/kernel/rtas_pci.c
> +++ b/arch/powerpc/kernel/rtas_pci.c
> @@ -55,10 +55,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
>         return 0;
>  }
>
> -int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
> +static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
> +                               int where, int size, u32 *val)
>  {
>         int returnval = -1;
> -       unsigned long buid, addr;
> +       unsigned long addr = rtas_config_addr(busno, devfn, where);
> +       int ret;
> +
> +       if (buid) {
> +               ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
> +                               addr, BUID_HI(buid), BUID_LO(buid), size);
> +       } else {
> +               ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
> +       }
> +       *val = returnval;
> +
> +       return ret;
> +}
> +
> +int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
> +{
>         int ret;
>
>         if (!pdn)
> @@ -71,16 +87,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
>                 return PCIBIOS_SET_FAILED;
>  #endif
>
> -       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> -       buid = pdn->phb->buid;
> -       if (buid) {
> -               ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
> -                               addr, BUID_HI(buid), BUID_LO(buid), size);
> -       } else {
> -               ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
> -       }
> -       *val = returnval;
> -
> +       ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
> +                                  where, size, val);
>         if (ret)
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>
> @@ -98,18 +106,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
>
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>
> -       /* Validity of pdn is checked in here */
> -       ret = rtas_read_config(pdn, where, size, val);
> -       if (*val == EEH_IO_ERROR_VALUE(size) &&
> -           eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +       if (pdn) {
> +               /* Validity of pdn is checked in here */
> +               ret = rtas_read_config(pdn, where, size, val);
> +
> +               if (*val == EEH_IO_ERROR_VALUE(size) &&
> +                   eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
> +                       ret = PCIBIOS_DEVICE_NOT_FOUND;
> +       } else {
> +               struct pci_controller *phb = pci_bus_to_host(bus);
> +
> +               ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
> +                                          where, size, val);
> +       }
>
>         return ret;
>  }
>
> +static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
> +                                int where, int size, u32 val)
> +{
> +       unsigned long addr = rtas_config_addr(busno, devfn, where);
> +       int ret;
> +
> +       if (buid) {
> +               ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
> +                               BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
> +       } else {
> +               ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
> +       }
> +
> +       if (ret)
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +       return PCIBIOS_SUCCESSFUL;
> +}
> +
>  int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>  {
> -       unsigned long buid, addr;
>         int ret;
>
>         if (!pdn)
> @@ -122,15 +156,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
>                 return PCIBIOS_SET_FAILED;
>  #endif
>
> -       addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
> -       buid = pdn->phb->buid;
> -       if (buid) {
> -               ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
> -                       BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
> -       } else {
> -               ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
> -       }
> -
> +       ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
> +                                   where, size, val);
>         if (ret)
>                 return PCIBIOS_DEVICE_NOT_FOUND;
>
> @@ -141,12 +168,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
>                                  unsigned int devfn,
>                                  int where, int size, u32 val)
>  {
> -       struct pci_dn *pdn;
> +       struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
> +       int ret;
>
> -       pdn = pci_get_pdn_by_devfn(bus, devfn);
> +       if (pdn) {
> +               /* Validity of pdn is checked in here. */
> +               ret = rtas_write_config(pdn, where, size, val);
> +       } else {
> +               struct pci_controller *phb = pci_bus_to_host(bus);
>
> -       /* Validity of pdn is checked in here. */
> -       return rtas_write_config(pdn, where, size, val);
> +               ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
> +                                           where, size, val);
> +       }
> +
> +       return ret;
>  }
>
>  static struct pci_ops rtas_pci_ops = {
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index ef9448a907c6..41a381dfc2a1 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -652,30 +652,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
>         }
>  }
>
> -int pnv_pci_cfg_read(struct pci_dn *pdn,
> -                    int where, int size, u32 *val)
> +static int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
> +                               int where, int size, u32 *val)
>  {
> -       struct pnv_phb *phb = pdn->phb->private_data;
> -       u32 bdfn = (pdn->busno << 8) | pdn->devfn;
> +       u32 bdfn = (busno << 8) | devfn;
>         s64 rc;
>
>         switch (size) {
>         case 1: {
>                 u8 v8;
> -               rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
> +               rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
>                 *val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
>                 break;
>         }
>         case 2: {
>                 __be16 v16;
> -               rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
> -                                                  &v16);
> +               rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
> +                                                   &v16);
>                 *val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
>                 break;
>         }
>         case 4: {
>                 __be32 v32;
> -               rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
> +               rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
>                 *val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
>                 break;
>         }
> @@ -684,27 +683,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
>         }
>
>         pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
> -                __func__, pdn->busno, pdn->devfn, where, size, *val);
> +                __func__, busno, devfn, where, size, *val);
> +
>         return PCIBIOS_SUCCESSFUL;
>  }
>
> -int pnv_pci_cfg_write(struct pci_dn *pdn,
> -                     int where, int size, u32 val)
> +static int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
> +                                int where, int size, u32 val)
>  {
> -       struct pnv_phb *phb = pdn->phb->private_data;
> -       u32 bdfn = (pdn->busno << 8) | pdn->devfn;
> +       u32 bdfn = (busno << 8) | devfn;
>
>         pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
> -                __func__, pdn->busno, pdn->devfn, where, size, val);
> +                __func__, busno, devfn, where, size, val);
> +
>         switch (size) {
>         case 1:
> -               opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_byte(phb_id, bdfn, where, val);
>                 break;
>         case 2:
> -               opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_half_word(phb_id, bdfn, where, val);
>                 break;
>         case 4:
> -               opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
> +               opal_pci_config_write_word(phb_id, bdfn, where, val);
>                 break;
>         default:
>                 return PCIBIOS_FUNC_NOT_SUPPORTED;
> @@ -713,6 +713,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
>         return PCIBIOS_SUCCESSFUL;
>  }
>
> +int pnv_pci_cfg_read(struct pci_dn *pdn,
> +                    int where, int size, u32 *val)
> +{
> +       struct pnv_phb *phb = pdn->phb->private_data;
> +
> +       return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
> +                                   where, size, val);
> +}
> +
> +int pnv_pci_cfg_write(struct pci_dn *pdn,
> +                     int where, int size, u32 val)
> +{
> +       struct pnv_phb *phb = pdn->phb->private_data;
> +
> +       return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
> +                                    where, size, val);
> +}
> +
>  #if CONFIG_EEH
>  static bool pnv_pci_cfg_check(struct pci_dn *pdn)
>  {
> @@ -748,13 +766,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>                                int where, int size, u32 *val)
>  {
>         struct pci_dn *pdn;
> -       struct pnv_phb *phb;
> +       struct pci_controller *hose = pci_bus_to_host(bus);
> +       struct pnv_phb *phb = hose->private_data;
>         int ret;
>
>         *val = 0xFFFFFFFF;
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>         if (!pdn)
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +               return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> +                                           where, size, val);
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> @@ -777,12 +797,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
>                                 int where, int size, u32 val)
>  {
>         struct pci_dn *pdn;
> -       struct pnv_phb *phb;
> +       struct pci_controller *hose = pci_bus_to_host(bus);
> +       struct pnv_phb *phb = hose->private_data;
>         int ret;
>
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
>         if (!pdn)
> -               return PCIBIOS_DEVICE_NOT_FOUND;
> +               return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
> +                                            where, size, val);
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> --
> 2.20.1
>

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/8] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
  2019-03-11 11:52   ` Sergey Miroshnichenko
@ 2019-04-30  4:26     ` Oliver
  -1 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-04-30  4:26 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: linuxppc-dev, linux-pci, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux

On Mon, Mar 11, 2019 at 10:52 PM Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
>
> Reading an empty slot returns all ones, which triggers a false
> EEH error event on PowerNV. This patch unfreezes the bus where
> it has happened.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/include/asm/ppc-pci.h   |  1 +
>  arch/powerpc/kernel/pci_dn.c         |  2 +-
>  arch/powerpc/platforms/powernv/pci.c | 31 +++++++++++++++++++++++++---
>  3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
> index f191ef0d2a0a..a22d52a9bb1f 100644
> --- a/arch/powerpc/include/asm/ppc-pci.h
> +++ b/arch/powerpc/include/asm/ppc-pci.h
> @@ -40,6 +40,7 @@ void *traverse_pci_dn(struct pci_dn *root,
>                       void *(*fn)(struct pci_dn *, void *),
>                       void *data);
>  extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
> +struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus);
>
>  /* From rtas_pci.h */
>  extern void init_pci_config_tokens (void);
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index ab147a1909c8..341ed71250f1 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -40,7 +40,7 @@
>   * one of PF's bridge. For other devices, their firmware
>   * data is linked to that of their bridge.
>   */
> -static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
> +struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
>  {
>         struct pci_bus *pbus;
>         struct device_node *dn;
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 41a381dfc2a1..8cc6661781e2 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -761,6 +761,21 @@ static inline pnv_pci_cfg_check(struct pci_dn *pdn)
>  }
>  #endif /* CONFIG_EEH */
>
> +static int get_bus_pe_number(struct pci_bus *bus)
> +{
> +       struct pci_dn *pdn = pci_bus_to_pdn(bus);
> +       struct pci_dn *child;
> +
> +       if (!pdn)
> +               return IODA_INVALID_PE;
> +
> +       list_for_each_entry(child, &pdn->child_list, list)
> +               if (child->pe_number != IODA_INVALID_PE)
> +                       return child->pe_number;
> +
> +       return IODA_INVALID_PE;
> +}
> +
>  static int pnv_pci_read_config(struct pci_bus *bus,
>                                unsigned int devfn,
>                                int where, int size, u32 *val)
> @@ -772,9 +787,19 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>
>         *val = 0xFFFFFFFF;
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
> -       if (!pdn)
> -               return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> -                                           where, size, val);
> +       if (!pdn) {
> +               int pe_number = get_bus_pe_number(bus);
> +
> +               ret = pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> +                                          where, size, val);
> +
> +               if (!ret && (*val == EEH_IO_ERROR_VALUE(size)) && phb->unfreeze_pe)
> +                       phb->unfreeze_pe(phb, (pe_number == IODA_INVALID_PE) ?
> +                                        phb->ioda.reserved_pe_idx : pe_number,
> +                                        OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
> +
> +               return ret;
> +       }
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> --
> 2.20.1
>

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 2/8] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
@ 2019-04-30  4:26     ` Oliver
  0 siblings, 0 replies; 30+ messages in thread
From: Oliver @ 2019-04-30  4:26 UTC (permalink / raw)
  To: Sergey Miroshnichenko
  Cc: Stewart Smith, Alexey Kardashevskiy, linux-pci, linux, linuxppc-dev

On Mon, Mar 11, 2019 at 10:52 PM Sergey Miroshnichenko
<s.miroshnichenko@yadro.com> wrote:
>
> Reading an empty slot returns all ones, which triggers a false
> EEH error event on PowerNV. This patch unfreezes the bus where
> it has happened.
>
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/include/asm/ppc-pci.h   |  1 +
>  arch/powerpc/kernel/pci_dn.c         |  2 +-
>  arch/powerpc/platforms/powernv/pci.c | 31 +++++++++++++++++++++++++---
>  3 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
> index f191ef0d2a0a..a22d52a9bb1f 100644
> --- a/arch/powerpc/include/asm/ppc-pci.h
> +++ b/arch/powerpc/include/asm/ppc-pci.h
> @@ -40,6 +40,7 @@ void *traverse_pci_dn(struct pci_dn *root,
>                       void *(*fn)(struct pci_dn *, void *),
>                       void *data);
>  extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
> +struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus);
>
>  /* From rtas_pci.h */
>  extern void init_pci_config_tokens (void);
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index ab147a1909c8..341ed71250f1 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -40,7 +40,7 @@
>   * one of PF's bridge. For other devices, their firmware
>   * data is linked to that of their bridge.
>   */
> -static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
> +struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
>  {
>         struct pci_bus *pbus;
>         struct device_node *dn;
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 41a381dfc2a1..8cc6661781e2 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -761,6 +761,21 @@ static inline pnv_pci_cfg_check(struct pci_dn *pdn)
>  }
>  #endif /* CONFIG_EEH */
>
> +static int get_bus_pe_number(struct pci_bus *bus)
> +{
> +       struct pci_dn *pdn = pci_bus_to_pdn(bus);
> +       struct pci_dn *child;
> +
> +       if (!pdn)
> +               return IODA_INVALID_PE;
> +
> +       list_for_each_entry(child, &pdn->child_list, list)
> +               if (child->pe_number != IODA_INVALID_PE)
> +                       return child->pe_number;
> +
> +       return IODA_INVALID_PE;
> +}
> +
>  static int pnv_pci_read_config(struct pci_bus *bus,
>                                unsigned int devfn,
>                                int where, int size, u32 *val)
> @@ -772,9 +787,19 @@ static int pnv_pci_read_config(struct pci_bus *bus,
>
>         *val = 0xFFFFFFFF;
>         pdn = pci_get_pdn_by_devfn(bus, devfn);
> -       if (!pdn)
> -               return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> -                                           where, size, val);
> +       if (!pdn) {
> +               int pe_number = get_bus_pe_number(bus);
> +
> +               ret = pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
> +                                          where, size, val);
> +
> +               if (!ret && (*val == EEH_IO_ERROR_VALUE(size)) && phb->unfreeze_pe)
> +                       phb->unfreeze_pe(phb, (pe_number == IODA_INVALID_PE) ?
> +                                        phb->ioda.reserved_pe_idx : pe_number,
> +                                        OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
> +
> +               return ret;
> +       }
>
>         if (!pnv_pci_cfg_check(pdn))
>                 return PCIBIOS_DEVICE_NOT_FOUND;
> --
> 2.20.1
>

Reviewed-by: Oliver O'Halloran <oohall@gmail.com>

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
  2019-03-11 11:52   ` Sergey Miroshnichenko
@ 2019-04-30  6:00     ` Oliver O'Halloran
  -1 siblings, 0 replies; 30+ messages in thread
From: Oliver O'Halloran @ 2019-04-30  6:00 UTC (permalink / raw)
  To: Sergey Miroshnichenko, linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, Benjamin Herrenschmidt,
	Russell Currey, linux

On Mon, 2019-03-11 at 14:52 +0300, Sergey Miroshnichenko wrote:

> When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
> returns zero, because the device is yet preparing to enable the VFs.

I don't think this is correct. The earliest pcibios_sriov_enable() can
be called is during a driver probe function. The totalvfs field is
initialised by pci_iov_init() which is called before the device has
been added to the bus. If it's returning zero then maybe the driver
limited the number of VFs to zero?

That said, you need to reset numvfs to zero before changing the value. 
So limiting the number of pci_dns that are created to the number
actually required rather than totalvfs doesn't hurt.

> With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
> on PowerNV.

I tested on a few of our lab systems with random kernel versions
spanning from 4.15 to 5.0 and sriov_numvfs seemed to work fine on all
of them. Is there a specific configuration you're testing that needed
this change?

> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/include/asm/pci-bridge.h     |  4 +--
>  arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
>  arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
>  arch/powerpc/platforms/pseries/pci.c      |  4 +--
>  4 files changed, 25 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index fc188e0e9179..6479bc96e0b6 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -225,8 +225,8 @@ struct pci_dn {
>  extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>  					   int devfn);
>  extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
> -extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
> -extern void remove_dev_pci_data(struct pci_dev *pdev);
> +extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
> +extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
>  extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>  					       struct device_node *dn);
>  extern void pci_remove_device_node_info(struct device_node *dn);
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 7f12882d8882..7fa362f8038d 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
>  	return pdn;
>  }
>  
> -struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> +struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
>  {
> +	struct pci_dn *pdn = pci_get_pdn(pdev);
> +
>  #ifdef CONFIG_PCI_IOV
> -	struct pci_dn *parent, *pdn;
> +	struct pci_dn *parent;
>  	int i;
>  
>  	/* Only support IOV for now */
>  	if (!pdev->is_physfn)
> -		return pci_get_pdn(pdev);
> +		return pdn;
>  
>  	/* Check if VFs have been populated */
> -	pdn = pci_get_pdn(pdev);
>  	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>  		return NULL;
>  
> @@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>  	if (!parent)
>  		return NULL;
>  
> -	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
> +	for (i = 0; i < num_vfs; i++) {
>  		struct eeh_dev *edev __maybe_unused;
> +		struct pci_dn *vpdn;
>  
> -		pdn = pci_alloc_pdn(parent,
> -				    pci_iov_virtfn_bus(pdev, i),
> -				    pci_iov_virtfn_devfn(pdev, i));
> -		if (!pdn) {
> +		vpdn = pci_alloc_pdn(parent,
> +				     pci_iov_virtfn_bus(pdev, i),
> +				     pci_iov_virtfn_devfn(pdev, i));
> +		if (!vpdn) {
>  			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
>  				 __func__, i);
>  			return NULL;
>  		}
>  
> -		pdn->vf_index = i;
> +		vpdn->vf_index = i;
> +		vpdn->vendor_id = pdn->vendor_id;
> +		vpdn->device_id = pdn->device_id;
> +		vpdn->class_code = pdn->class_code;
> +		vpdn->pci_ext_config_space = 0;
>  
>  #ifdef CONFIG_EEH
>  		/* Create the EEH device for the VF */
> -		edev = eeh_dev_init(pdn);
> +		edev = eeh_dev_init(vpdn);
>  		BUG_ON(!edev);
>  		edev->physfn = pdev;
>  #endif /* CONFIG_EEH */
>  	}
>  #endif /* CONFIG_PCI_IOV */
>  
> -	return pci_get_pdn(pdev);
> +	return pdn;
>  }
>  
> -void remove_dev_pci_data(struct pci_dev *pdev)
> +void pci_destroy_vf_pdns(struct pci_dev *pdev)
>  {
>  #ifdef CONFIG_PCI_IOV
>  	struct pci_dn *parent;
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index ed500f51d449..979c901535f2 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
>  	pnv_pci_sriov_disable(pdev);
>  
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	pci_destroy_vf_pdns(pdev);
>  	return 0;
>  }
>  
>  int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	pci_create_vf_pdns(pdev, num_vfs);
>  
>  	return pnv_pci_sriov_enable(pdev, num_vfs);
>  }
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 37a77e57893e..5e87596903a6 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	pci_create_vf_pdns(pdev, num_vfs);
>  	return pseries_pci_sriov_enable(pdev, num_vfs);
>  }
>  
> @@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
>  	/* Releasing pe_num_map */
>  	kfree(pdn->pe_num_map);
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	pci_destroy_vf_pdns(pdev);
>  	pci_vf_drivers_autoprobe(pdev, true);
>  	return 0;
>  }


^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
@ 2019-04-30  6:00     ` Oliver O'Halloran
  0 siblings, 0 replies; 30+ messages in thread
From: Oliver O'Halloran @ 2019-04-30  6:00 UTC (permalink / raw)
  To: Sergey Miroshnichenko, linuxppc-dev, linux-pci
  Cc: Stewart Smith, Alexey Kardashevskiy, linux

On Mon, 2019-03-11 at 14:52 +0300, Sergey Miroshnichenko wrote:

> When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
> returns zero, because the device is yet preparing to enable the VFs.

I don't think this is correct. The earliest pcibios_sriov_enable() can
be called is during a driver probe function. The totalvfs field is
initialised by pci_iov_init() which is called before the device has
been added to the bus. If it's returning zero then maybe the driver
limited the number of VFs to zero?

That said, you need to reset numvfs to zero before changing the value. 
So limiting the number of pci_dns that are created to the number
actually required rather than totalvfs doesn't hurt.

> With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
> on PowerNV.

I tested on a few of our lab systems with random kernel versions
spanning from 4.15 to 5.0 and sriov_numvfs seemed to work fine on all
of them. Is there a specific configuration you're testing that needed
this change?

> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/include/asm/pci-bridge.h     |  4 +--
>  arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
>  arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
>  arch/powerpc/platforms/pseries/pci.c      |  4 +--
>  4 files changed, 25 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
> index fc188e0e9179..6479bc96e0b6 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -225,8 +225,8 @@ struct pci_dn {
>  extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>  					   int devfn);
>  extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
> -extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
> -extern void remove_dev_pci_data(struct pci_dev *pdev);
> +extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
> +extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
>  extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>  					       struct device_node *dn);
>  extern void pci_remove_device_node_info(struct device_node *dn);
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index 7f12882d8882..7fa362f8038d 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
>  	return pdn;
>  }
>  
> -struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
> +struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
>  {
> +	struct pci_dn *pdn = pci_get_pdn(pdev);
> +
>  #ifdef CONFIG_PCI_IOV
> -	struct pci_dn *parent, *pdn;
> +	struct pci_dn *parent;
>  	int i;
>  
>  	/* Only support IOV for now */
>  	if (!pdev->is_physfn)
> -		return pci_get_pdn(pdev);
> +		return pdn;
>  
>  	/* Check if VFs have been populated */
> -	pdn = pci_get_pdn(pdev);
>  	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>  		return NULL;
>  
> @@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>  	if (!parent)
>  		return NULL;
>  
> -	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
> +	for (i = 0; i < num_vfs; i++) {
>  		struct eeh_dev *edev __maybe_unused;
> +		struct pci_dn *vpdn;
>  
> -		pdn = pci_alloc_pdn(parent,
> -				    pci_iov_virtfn_bus(pdev, i),
> -				    pci_iov_virtfn_devfn(pdev, i));
> -		if (!pdn) {
> +		vpdn = pci_alloc_pdn(parent,
> +				     pci_iov_virtfn_bus(pdev, i),
> +				     pci_iov_virtfn_devfn(pdev, i));
> +		if (!vpdn) {
>  			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
>  				 __func__, i);
>  			return NULL;
>  		}
>  
> -		pdn->vf_index = i;
> +		vpdn->vf_index = i;
> +		vpdn->vendor_id = pdn->vendor_id;
> +		vpdn->device_id = pdn->device_id;
> +		vpdn->class_code = pdn->class_code;
> +		vpdn->pci_ext_config_space = 0;
>  
>  #ifdef CONFIG_EEH
>  		/* Create the EEH device for the VF */
> -		edev = eeh_dev_init(pdn);
> +		edev = eeh_dev_init(vpdn);
>  		BUG_ON(!edev);
>  		edev->physfn = pdev;
>  #endif /* CONFIG_EEH */
>  	}
>  #endif /* CONFIG_PCI_IOV */
>  
> -	return pci_get_pdn(pdev);
> +	return pdn;
>  }
>  
> -void remove_dev_pci_data(struct pci_dev *pdev)
> +void pci_destroy_vf_pdns(struct pci_dev *pdev)
>  {
>  #ifdef CONFIG_PCI_IOV
>  	struct pci_dn *parent;
> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
> index ed500f51d449..979c901535f2 100644
> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
> @@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
>  	pnv_pci_sriov_disable(pdev);
>  
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	pci_destroy_vf_pdns(pdev);
>  	return 0;
>  }
>  
>  int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	pci_create_vf_pdns(pdev, num_vfs);
>  
>  	return pnv_pci_sriov_enable(pdev, num_vfs);
>  }
> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
> index 37a77e57893e..5e87596903a6 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>  {
>  	/* Allocate PCI data */
> -	add_dev_pci_data(pdev);
> +	pci_create_vf_pdns(pdev, num_vfs);
>  	return pseries_pci_sriov_enable(pdev, num_vfs);
>  }
>  
> @@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
>  	/* Releasing pe_num_map */
>  	kfree(pdn->pe_num_map);
>  	/* Release PCI data */
> -	remove_dev_pci_data(pdev);
> +	pci_destroy_vf_pdns(pdev);
>  	pci_vf_drivers_autoprobe(pdev, true);
>  	return 0;
>  }


^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
  2019-04-30  6:00     ` Oliver O'Halloran
@ 2019-05-14 14:44       ` Sergey Miroshnichenko
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-05-14 14:44 UTC (permalink / raw)
  To: Oliver O'Halloran
  Cc: linuxppc-dev, linux-pci, Stewart Smith, Alexey Kardashevskiy,
	Benjamin Herrenschmidt, Russell Currey, linux

On 4/30/19 9:00 AM, Oliver O'Halloran wrote:
> On Mon, 2019-03-11 at 14:52 +0300, Sergey Miroshnichenko wrote:
> 
>> When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
>> returns zero, because the device is yet preparing to enable the VFs.
> 
> I don't think this is correct. The earliest pcibios_sriov_enable() can
> be called is during a driver probe function. The totalvfs field is
> initialised by pci_iov_init() which is called before the device has
> been added to the bus. If it's returning zero then maybe the driver
> limited the number of VFs to zero?
> 
> That said, you need to reset numvfs to zero before changing the value. 
> So limiting the number of pci_dns that are created to the number
> actually required rather than totalvfs doesn't hurt.
> 
>> With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
>> on PowerNV.
> 
> I tested on a few of our lab systems with random kernel versions
> spanning from 4.15 to 5.0 and sriov_numvfs seemed to work fine on all
> of them. Is there a specific configuration you're testing that needed
> this change?
> 

Thanks a lot for the review and testing!

I've just received back the hardware (Mellanox ConnectX-4 -
drivers/net/ethernet/mellanox/mlx5), and got surprised: the issue with the
pci_sriov_get_totalvfs(pdev) returning zero can't be reproduced anymore :/ I've rechecked
the code and don't know how could this even happen. I'm sorry about that; if it will
happen again, I have to investigate deeper.

The PCI subsystem doesn't let the number of VFs to be changed from non-zero value to
another non-zero value: it needs to sriov_disable() first. I guess we can rely on that and
don't reset the numvfs to zero explicitly.

I'll change the patch description and resend it in v6 with other fixes of this patchset.

Best regards,
Serge

>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/include/asm/pci-bridge.h     |  4 +--
>>  arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
>>  arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
>>  arch/powerpc/platforms/pseries/pci.c      |  4 +--
>>  4 files changed, 25 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>> index fc188e0e9179..6479bc96e0b6 100644
>> --- a/arch/powerpc/include/asm/pci-bridge.h
>> +++ b/arch/powerpc/include/asm/pci-bridge.h
>> @@ -225,8 +225,8 @@ struct pci_dn {
>>  extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>>  					   int devfn);
>>  extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
>> -extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
>> -extern void remove_dev_pci_data(struct pci_dev *pdev);
>> +extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
>> +extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
>>  extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>>  					       struct device_node *dn);
>>  extern void pci_remove_device_node_info(struct device_node *dn);
>> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>> index 7f12882d8882..7fa362f8038d 100644
>> --- a/arch/powerpc/kernel/pci_dn.c
>> +++ b/arch/powerpc/kernel/pci_dn.c
>> @@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
>>  	return pdn;
>>  }
>>  
>> -struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>> +struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
>>  {
>> +	struct pci_dn *pdn = pci_get_pdn(pdev);
>> +
>>  #ifdef CONFIG_PCI_IOV
>> -	struct pci_dn *parent, *pdn;
>> +	struct pci_dn *parent;
>>  	int i;
>>  
>>  	/* Only support IOV for now */
>>  	if (!pdev->is_physfn)
>> -		return pci_get_pdn(pdev);
>> +		return pdn;
>>  
>>  	/* Check if VFs have been populated */
>> -	pdn = pci_get_pdn(pdev);
>>  	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>>  		return NULL;
>>  
>> @@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>  	if (!parent)
>>  		return NULL;
>>  
>> -	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>> +	for (i = 0; i < num_vfs; i++) {
>>  		struct eeh_dev *edev __maybe_unused;
>> +		struct pci_dn *vpdn;
>>  
>> -		pdn = pci_alloc_pdn(parent,
>> -				    pci_iov_virtfn_bus(pdev, i),
>> -				    pci_iov_virtfn_devfn(pdev, i));
>> -		if (!pdn) {
>> +		vpdn = pci_alloc_pdn(parent,
>> +				     pci_iov_virtfn_bus(pdev, i),
>> +				     pci_iov_virtfn_devfn(pdev, i));
>> +		if (!vpdn) {
>>  			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
>>  				 __func__, i);
>>  			return NULL;
>>  		}
>>  
>> -		pdn->vf_index = i;
>> +		vpdn->vf_index = i;
>> +		vpdn->vendor_id = pdn->vendor_id;
>> +		vpdn->device_id = pdn->device_id;
>> +		vpdn->class_code = pdn->class_code;
>> +		vpdn->pci_ext_config_space = 0;
>>  
>>  #ifdef CONFIG_EEH
>>  		/* Create the EEH device for the VF */
>> -		edev = eeh_dev_init(pdn);
>> +		edev = eeh_dev_init(vpdn);
>>  		BUG_ON(!edev);
>>  		edev->physfn = pdev;
>>  #endif /* CONFIG_EEH */
>>  	}
>>  #endif /* CONFIG_PCI_IOV */
>>  
>> -	return pci_get_pdn(pdev);
>> +	return pdn;
>>  }
>>  
>> -void remove_dev_pci_data(struct pci_dev *pdev)
>> +void pci_destroy_vf_pdns(struct pci_dev *pdev)
>>  {
>>  #ifdef CONFIG_PCI_IOV
>>  	struct pci_dn *parent;
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index ed500f51d449..979c901535f2 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
>>  	pnv_pci_sriov_disable(pdev);
>>  
>>  	/* Release PCI data */
>> -	remove_dev_pci_data(pdev);
>> +	pci_destroy_vf_pdns(pdev);
>>  	return 0;
>>  }
>>  
>>  int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  {
>>  	/* Allocate PCI data */
>> -	add_dev_pci_data(pdev);
>> +	pci_create_vf_pdns(pdev, num_vfs);
>>  
>>  	return pnv_pci_sriov_enable(pdev, num_vfs);
>>  }
>> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
>> index 37a77e57893e..5e87596903a6 100644
>> --- a/arch/powerpc/platforms/pseries/pci.c
>> +++ b/arch/powerpc/platforms/pseries/pci.c
>> @@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  {
>>  	/* Allocate PCI data */
>> -	add_dev_pci_data(pdev);
>> +	pci_create_vf_pdns(pdev, num_vfs);
>>  	return pseries_pci_sriov_enable(pdev, num_vfs);
>>  }
>>  
>> @@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
>>  	/* Releasing pe_num_map */
>>  	kfree(pdn->pe_num_map);
>>  	/* Release PCI data */
>> -	remove_dev_pci_data(pdev);
>> +	pci_destroy_vf_pdns(pdev);
>>  	pci_vf_drivers_autoprobe(pdev, true);
>>  	return 0;
>>  }
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
* Re: [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs
@ 2019-05-14 14:44       ` Sergey Miroshnichenko
  0 siblings, 0 replies; 30+ messages in thread
From: Sergey Miroshnichenko @ 2019-05-14 14:44 UTC (permalink / raw)
  To: Oliver O'Halloran
  Cc: Stewart Smith, Alexey Kardashevskiy, linux-pci, linux, linuxppc-dev

On 4/30/19 9:00 AM, Oliver O'Halloran wrote:
> On Mon, 2019-03-11 at 14:52 +0300, Sergey Miroshnichenko wrote:
> 
>> When called within pcibios_sriov_enable(), the pci_sriov_get_totalvfs(pdev)
>> returns zero, because the device is yet preparing to enable the VFs.
> 
> I don't think this is correct. The earliest pcibios_sriov_enable() can
> be called is during a driver probe function. The totalvfs field is
> initialised by pci_iov_init() which is called before the device has
> been added to the bus. If it's returning zero then maybe the driver
> limited the number of VFs to zero?
> 
> That said, you need to reset numvfs to zero before changing the value. 
> So limiting the number of pci_dns that are created to the number
> actually required rather than totalvfs doesn't hurt.
> 
>> With this patch it becomes possible to enable VFs via sysfs "sriov_numvfs"
>> on PowerNV.
> 
> I tested on a few of our lab systems with random kernel versions
> spanning from 4.15 to 5.0 and sriov_numvfs seemed to work fine on all
> of them. Is there a specific configuration you're testing that needed
> this change?
> 

Thanks a lot for the review and testing!

I've just received back the hardware (Mellanox ConnectX-4 -
drivers/net/ethernet/mellanox/mlx5), and got surprised: the issue with the
pci_sriov_get_totalvfs(pdev) returning zero can't be reproduced anymore :/ I've rechecked
the code and don't know how could this even happen. I'm sorry about that; if it will
happen again, I have to investigate deeper.

The PCI subsystem doesn't let the number of VFs to be changed from non-zero value to
another non-zero value: it needs to sriov_disable() first. I guess we can rely on that and
don't reset the numvfs to zero explicitly.

I'll change the patch description and resend it in v6 with other fixes of this patchset.

Best regards,
Serge

>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/include/asm/pci-bridge.h     |  4 +--
>>  arch/powerpc/kernel/pci_dn.c              | 32 ++++++++++++++---------
>>  arch/powerpc/platforms/powernv/pci-ioda.c |  4 +--
>>  arch/powerpc/platforms/pseries/pci.c      |  4 +--
>>  4 files changed, 25 insertions(+), 19 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>> index fc188e0e9179..6479bc96e0b6 100644
>> --- a/arch/powerpc/include/asm/pci-bridge.h
>> +++ b/arch/powerpc/include/asm/pci-bridge.h
>> @@ -225,8 +225,8 @@ struct pci_dn {
>>  extern struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>>  					   int devfn);
>>  extern struct pci_dn *pci_get_pdn(struct pci_dev *pdev);
>> -extern struct pci_dn *add_dev_pci_data(struct pci_dev *pdev);
>> -extern void remove_dev_pci_data(struct pci_dev *pdev);
>> +extern struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs);
>> +extern void pci_destroy_vf_pdns(struct pci_dev *pdev);
>>  extern struct pci_dn *pci_add_device_node_info(struct pci_controller *hose,
>>  					       struct device_node *dn);
>>  extern void pci_remove_device_node_info(struct device_node *dn);
>> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
>> index 7f12882d8882..7fa362f8038d 100644
>> --- a/arch/powerpc/kernel/pci_dn.c
>> +++ b/arch/powerpc/kernel/pci_dn.c
>> @@ -222,18 +222,19 @@ static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
>>  	return pdn;
>>  }
>>  
>> -struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>> +struct pci_dn *pci_create_vf_pdns(struct pci_dev *pdev, int num_vfs)
>>  {
>> +	struct pci_dn *pdn = pci_get_pdn(pdev);
>> +
>>  #ifdef CONFIG_PCI_IOV
>> -	struct pci_dn *parent, *pdn;
>> +	struct pci_dn *parent;
>>  	int i;
>>  
>>  	/* Only support IOV for now */
>>  	if (!pdev->is_physfn)
>> -		return pci_get_pdn(pdev);
>> +		return pdn;
>>  
>>  	/* Check if VFs have been populated */
>> -	pdn = pci_get_pdn(pdev);
>>  	if (!pdn || (pdn->flags & PCI_DN_FLAG_IOV_VF))
>>  		return NULL;
>>  
>> @@ -242,33 +243,38 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>>  	if (!parent)
>>  		return NULL;
>>  
>> -	for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
>> +	for (i = 0; i < num_vfs; i++) {
>>  		struct eeh_dev *edev __maybe_unused;
>> +		struct pci_dn *vpdn;
>>  
>> -		pdn = pci_alloc_pdn(parent,
>> -				    pci_iov_virtfn_bus(pdev, i),
>> -				    pci_iov_virtfn_devfn(pdev, i));
>> -		if (!pdn) {
>> +		vpdn = pci_alloc_pdn(parent,
>> +				     pci_iov_virtfn_bus(pdev, i),
>> +				     pci_iov_virtfn_devfn(pdev, i));
>> +		if (!vpdn) {
>>  			dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
>>  				 __func__, i);
>>  			return NULL;
>>  		}
>>  
>> -		pdn->vf_index = i;
>> +		vpdn->vf_index = i;
>> +		vpdn->vendor_id = pdn->vendor_id;
>> +		vpdn->device_id = pdn->device_id;
>> +		vpdn->class_code = pdn->class_code;
>> +		vpdn->pci_ext_config_space = 0;
>>  
>>  #ifdef CONFIG_EEH
>>  		/* Create the EEH device for the VF */
>> -		edev = eeh_dev_init(pdn);
>> +		edev = eeh_dev_init(vpdn);
>>  		BUG_ON(!edev);
>>  		edev->physfn = pdev;
>>  #endif /* CONFIG_EEH */
>>  	}
>>  #endif /* CONFIG_PCI_IOV */
>>  
>> -	return pci_get_pdn(pdev);
>> +	return pdn;
>>  }
>>  
>> -void remove_dev_pci_data(struct pci_dev *pdev)
>> +void pci_destroy_vf_pdns(struct pci_dev *pdev)
>>  {
>>  #ifdef CONFIG_PCI_IOV
>>  	struct pci_dn *parent;
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index ed500f51d449..979c901535f2 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -1720,14 +1720,14 @@ int pnv_pcibios_sriov_disable(struct pci_dev *pdev)
>>  	pnv_pci_sriov_disable(pdev);
>>  
>>  	/* Release PCI data */
>> -	remove_dev_pci_data(pdev);
>> +	pci_destroy_vf_pdns(pdev);
>>  	return 0;
>>  }
>>  
>>  int pnv_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  {
>>  	/* Allocate PCI data */
>> -	add_dev_pci_data(pdev);
>> +	pci_create_vf_pdns(pdev, num_vfs);
>>  
>>  	return pnv_pci_sriov_enable(pdev, num_vfs);
>>  }
>> diff --git a/arch/powerpc/platforms/pseries/pci.c b/arch/powerpc/platforms/pseries/pci.c
>> index 37a77e57893e..5e87596903a6 100644
>> --- a/arch/powerpc/platforms/pseries/pci.c
>> +++ b/arch/powerpc/platforms/pseries/pci.c
>> @@ -205,7 +205,7 @@ int pseries_pci_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  int pseries_pcibios_sriov_enable(struct pci_dev *pdev, u16 num_vfs)
>>  {
>>  	/* Allocate PCI data */
>> -	add_dev_pci_data(pdev);
>> +	pci_create_vf_pdns(pdev, num_vfs);
>>  	return pseries_pci_sriov_enable(pdev, num_vfs);
>>  }
>>  
>> @@ -217,7 +217,7 @@ int pseries_pcibios_sriov_disable(struct pci_dev *pdev)
>>  	/* Releasing pe_num_map */
>>  	kfree(pdn->pe_num_map);
>>  	/* Release PCI data */
>> -	remove_dev_pci_data(pdev);
>> +	pci_destroy_vf_pdns(pdev);
>>  	pci_vf_drivers_autoprobe(pdev, true);
>>  	return 0;
>>  }
> 

^ permalink raw reply	[flat|nested] 30+ messages in thread
end of thread, other threads:[~2019-05-14 14:52 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 11:52 [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT Sergey Miroshnichenko
2019-03-11 11:52 ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 1/8] powerpc/pci: Access PCI config space directly w/o pci_dn Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-04-30  4:22   ` Oliver
2019-04-30  4:22     ` Oliver
2019-03-11 11:52 ` [PATCH v5 2/8] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-04-30  4:26   ` Oliver
2019-04-30  4:26     ` Oliver
2019-03-11 11:52 ` [PATCH v5 3/8] powerpc/pci: Create pci_dn on demand Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 4/8] powerpc/pci: Reduce code duplication in pci_add_device_node_info Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 5/8] powerpc/pci/IOV: Add support for runtime enabling the VFs Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-04-30  6:00   ` Oliver O'Halloran
2019-04-30  6:00     ` Oliver O'Halloran
2019-05-14 14:44     ` Sergey Miroshnichenko
2019-05-14 14:44       ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 6/8] powerpc/pci: Don't rely on DT is the PCI_REASSIGN_ALL_BUS is set Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 7/8] powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-03-11 11:52 ` [PATCH v5 8/8] powerpc/powernv/pci: Enable reassigning the bus numbers Sergey Miroshnichenko
2019-03-11 11:52   ` Sergey Miroshnichenko
2019-03-27 14:10 ` [PATCH v5 0/8] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT Bjorn Helgaas
2019-03-27 14:10   ` Bjorn Helgaas
2019-03-28 12:44   ` Oliver
2019-03-28 12:44     ` Oliver

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.