[PATCH 0/5] powerpc/pci/hotplug: Discover surprise-hotplugged PCIe devices during rescan

[PATCH 0/5] powerpc/pci/hotplug: Discover surprise-hotplugged PCIe devices during rescan

[Sergey Miroshnichenko]

This patchset allows hotplugged PCIe devices to be enumerated during a bus
rescan being issued via sysfs, when the "Presence Detect Changed" interrupt
is not available.

As a first part of our work on adding support for hotplugging PCIe bridges
full of devices without special requirement such as Hot-Plug Controller,
reservation of bus numbers and memory regions by firmware, etc., this serie
is intended to solve the first two problems from the list below:

 I   PowerNV doesn't discover new hotplugged PCIe devices
 II  EEH is falsely triggered when poking empty slots during PCIe rescan on PowerNV
 III The PCI subsystem is not prepared to runtime changes of BAR addresses
 IV  Device drivers don't track changes of their BAR addresses
 V   Move BARs of working devices to make space for new ones
 VI  Add support for PCIe bridge hotplug

Sergey Miroshnichenko (5):
  powerpc/pci: Access PCI config space directly w/o pci_dn
  powerpc/pci: Create pci_dn on demand
  powerpc/pci: Use DT to create pci_dn for root bridges only
  powerpc/powernv/pci: Enable reassigning the bus numbers
  PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan

 arch/powerpc/include/asm/eeh.h               |  2 +
 arch/powerpc/kernel/eeh.c                    | 14 +++
 arch/powerpc/kernel/pci_dn.c                 | 75 ++++++++++++-----
 arch/powerpc/kernel/rtas_pci.c               | 89 ++++++++++++--------
 arch/powerpc/platforms/powernv/eeh-powernv.c | 20 +++++
 arch/powerpc/platforms/powernv/pci.c         | 55 +++++++-----
 drivers/pci/probe.c                          | 14 +++
 include/linux/pci.h                          |  2 +
 8 files changed, 197 insertions(+), 74 deletions(-)

-- 
2.17.1

[PATCH 1/5] powerpc/pci: Access PCI config space directly w/o pci_dn

[Sergey Miroshnichenko]

The pci_dn structures are retrieved from a DT, but hot-plugged PCIe
devices don't have them. Don't stop PCIe I/O in absence of pci_dn, so
it is now possible to discover new devices.

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

diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index c2b148b1634a..0da2c3732acd 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -55,10 +55,25 @@ 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 +86,7 @@ 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;
 
@@ -91,25 +97,43 @@ static int rtas_pci_read_config(struct pci_bus *bus,
 				unsigned int devfn,
 				int where, int size, u32 *val)
 {
-	struct pci_dn *pdn;
-	int ret;
+	struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
 
 	*val = 0xFFFFFFFF;
 
-	pdn = pci_get_pdn_by_devfn(bus, devfn);
+	if (pdn && eeh_enabled()) {
+		/* Validity of pdn is checked in here */
+		int 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;
+		return ret;
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
+		return rtas_read_raw_config(phb->buid, bus->number, devfn, where, size, val);
+	}
+}
 
-	/* 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)))
+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 ret;
+	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 +146,7 @@ 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 +157,15 @@ 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);
 
-	pdn = pci_get_pdn_by_devfn(bus, devfn);
-
-	/* Validity of pdn is checked in here. */
-	return rtas_write_config(pdn, where, size, val);
+	if (pdn && eeh_enabled()) {
+		/* Validity of pdn is checked in here. */
+		return rtas_write_config(pdn, where, size, val);
+	} else {
+		struct pci_controller *phb = pci_bus_to_host(bus);
+		return rtas_write_raw_config(phb->buid, bus->number, devfn, where, size, val);
+	}
 }
 
 static struct pci_ops rtas_pci_ops = {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 13aef2323bbc..6d4280086a08 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -654,30 +654,28 @@ 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)
+int pnv_pci_cfg_read_raw(uint64_t 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,
+		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;
 	}
@@ -686,27 +684,27 @@ 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)
+int pnv_pci_cfg_write_raw(uint64_t 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;
@@ -715,6 +713,20 @@ 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)
 {
@@ -750,13 +762,14 @@ 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;
@@ -779,12 +792,13 @@ 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.17.1

[PATCH 2/5] powerpc/pci: Create pci_dn on demand

[Sergey Miroshnichenko]

The pci_dn structures can be created not only from DT, but also
directly from newly discovered PCIe devices, so allocate them
dynamically.

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

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index ab147a1909c8..5ce752874827 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -33,6 +33,8 @@
 #include <asm/firmware.h>
 #include <asm/eeh.h>
 
+static struct pci_dn* create_pdn(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
@@ -58,6 +60,10 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
 		pbus = pbus->parent;
 	}
 
+	if (!pbus->self && !pci_is_root_bus(pbus)) {
+		return NULL;
+	}
+
 	/*
 	 * Except virtual bus, all PCI buses should
 	 * have device nodes.
@@ -65,13 +71,16 @@ static 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;
 }
 
 struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 				    int devfn)
 {
-	struct device_node *dn = NULL;
 	struct pci_dn *parent, *pdn;
 	struct pci_dev *pdev = NULL;
 
@@ -80,17 +89,10 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
 		if (pdev->devfn == devfn) {
 			if (pdev->dev.archdata.pci_data)
 				return pdev->dev.archdata.pci_data;
-
-			dn = pci_device_to_OF_node(pdev);
 			break;
 		}
 	}
 
-	/* Fast path: fetch from device node */
-	pdn = dn ? PCI_DN(dn) : NULL;
-	if (pdn)
-		return pdn;
-
 	/* Slow path: fetch from firmware data hierarchy */
 	parent = pci_bus_to_pdn(bus);
 	if (!parent)
@@ -128,16 +130,9 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
 	if (!parent)
 		return NULL;
 
-	list_for_each_entry(pdn, &parent->child_list, list) {
-		if (pdn->busno == pdev->bus->number &&
-		    pdn->devfn == pdev->devfn)
-			return pdn;
-	}
-
-	return NULL;
+	return create_pdn(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)
@@ -164,7 +159,47 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
 
 	return pdn;
 }
-#endif
+
+static struct pci_dn* create_pdn(struct pci_dev *pdev, struct pci_dn *parent)
+{
+	struct pci_dn *pdn = NULL;
+
+	pdn = add_one_dev_pci_data(parent, 0, pdev->bus->number, pdev->devfn);
+	dev_info(&pdev->dev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
+
+	if (pdn)
+	{
+		u32 class_code;
+		u16 device_id;
+		u16 vendor_id;
+
+		struct eeh_dev *edev = eeh_dev_init(pdn);
+		if (!edev) {
+			kfree(pdn);
+			dev_err(&pdev->dev, "%s:%d: Failed to allocate edev\n", __func__, __LINE__);
+			return NULL;
+		}
+
+		pdn->busno = pdev->bus->busn_res.start;
+
+		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;
+
+		pdn->pci_ext_config_space = 0;
+		pdev->dev.archdata.pci_data = pdn;
+	} else {
+		dev_err(&pdev->dev, "%s:%d: Failed to allocate pdn\n", __func__, __LINE__);
+	}
+
+	return pdn;
+}
 
 struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
 {
-- 
2.17.1

Re: [PATCH 2/5] powerpc/pci: Create pci_dn on demand

[Benjamin Herrenschmidt]

On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> The pci_dn structures can be created not only from DT, but also
> directly from newly discovered PCIe devices, so allocate them
> dynamically.

I'd rather we moved towards killing pci_dn completely to be honest :)

> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/kernel/pci_dn.c | 69 +++++++++++++++++++++++++++---------
>  1 file changed, 52 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
> index ab147a1909c8..5ce752874827 100644
> --- a/arch/powerpc/kernel/pci_dn.c
> +++ b/arch/powerpc/kernel/pci_dn.c
> @@ -33,6 +33,8 @@
>  #include <asm/firmware.h>
>  #include <asm/eeh.h>
>  
> +static struct pci_dn* create_pdn(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
> @@ -58,6 +60,10 @@ static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
>  		pbus = pbus->parent;
>  	}
>  
> +	if (!pbus->self && !pci_is_root_bus(pbus)) {
> +		return NULL;
> +	}
> +
>  	/*
>  	 * Except virtual bus, all PCI buses should
>  	 * have device nodes.
> @@ -65,13 +71,16 @@ static 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;
>  }
>  
>  struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>  				    int devfn)
>  {
> -	struct device_node *dn = NULL;
>  	struct pci_dn *parent, *pdn;
>  	struct pci_dev *pdev = NULL;
>  
> @@ -80,17 +89,10 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
>  		if (pdev->devfn == devfn) {
>  			if (pdev->dev.archdata.pci_data)
>  				return pdev->dev.archdata.pci_data;
> -
> -			dn = pci_device_to_OF_node(pdev);
>  			break;
>  		}
>  	}
>  
> -	/* Fast path: fetch from device node */
> -	pdn = dn ? PCI_DN(dn) : NULL;
> -	if (pdn)
> -		return pdn;
> -
>  	/* Slow path: fetch from firmware data hierarchy */
>  	parent = pci_bus_to_pdn(bus);
>  	if (!parent)
> @@ -128,16 +130,9 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
>  	if (!parent)
>  		return NULL;
>  
> -	list_for_each_entry(pdn, &parent->child_list, list) {
> -		if (pdn->busno == pdev->bus->number &&
> -		    pdn->devfn == pdev->devfn)
> -			return pdn;
> -	}
> -
> -	return NULL;
> +	return create_pdn(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)
> @@ -164,7 +159,47 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
>  
>  	return pdn;
>  }
> -#endif
> +
> +static struct pci_dn* create_pdn(struct pci_dev *pdev, struct pci_dn *parent)
> +{
> +	struct pci_dn *pdn = NULL;
> +
> +	pdn = add_one_dev_pci_data(parent, 0, pdev->bus->number, pdev->devfn);
> +	dev_info(&pdev->dev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
> +
> +	if (pdn)
> +	{
> +		u32 class_code;
> +		u16 device_id;
> +		u16 vendor_id;
> +
> +		struct eeh_dev *edev = eeh_dev_init(pdn);
> +		if (!edev) {
> +			kfree(pdn);
> +			dev_err(&pdev->dev, "%s:%d: Failed to allocate edev\n", __func__, __LINE__);
> +			return NULL;
> +		}
> +
> +		pdn->busno = pdev->bus->busn_res.start;
> +
> +		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;
> +
> +		pdn->pci_ext_config_space = 0;
> +		pdev->dev.archdata.pci_data = pdn;
> +	} else {
> +		dev_err(&pdev->dev, "%s:%d: Failed to allocate pdn\n", __func__, __LINE__);
> +	}
> +
> +	return pdn;
> +}
>  
>  struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
>  {

[PATCH 3/5] powerpc/pci: Use DT to create pci_dn for root bridges only

[Sergey Miroshnichenko]

Endpoint's pci_dn can be created dynamically.

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

diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 5ce752874827..1b9f563d9461 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -530,8 +530,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);
+	}
 }
 
 /** 
-- 
2.17.1

[PATCH 4/5] powerpc/powernv/pci: Enable reassigning the bus numbers

[Sergey Miroshnichenko]

PowerNV doesn't depend on PCIe topology info from DT anymore, and now
it is able to enumerate the fabric and assign the bus numbers.

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

diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 6d4280086a08..f6eaca3123cd 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -1104,6 +1104,7 @@ void __init pnv_pci_init(void)
 	struct device_node *np;
 
 	pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
+	pci_add_flags(PCI_REASSIGN_ALL_BUS);
 
 	/* If we don't have OPAL, eg. in sim, just skip PCI probe */
 	if (!firmware_has_feature(FW_FEATURE_OPAL))
-- 
2.17.1

Re: [PATCH 4/5] powerpc/powernv/pci: Enable reassigning the bus numbers

[Benjamin Herrenschmidt]

On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> PowerNV doesn't depend on PCIe topology info from DT anymore, and now
> it is able to enumerate the fabric and assign the bus numbers.

No it's not, at least unless we drop P7 support.

P7 has constraints on the bus ranges being aligned power-of-two for the
PE assignment to work, which is why we have to honor the firmware
provided numbers.

Additionally, this breaks the mapping between the firmware idea of the
bus numbers and Linux idea. This will probably break all of the SR-IOV
stuff.

Now we should probably fix it all by removing the FW bits completely
and doing it all from Linux, though we really need to better handle how
we deal with the segmented MMIO space.

I would also be weary of what other parts of the code depends on that
matching between the FW bdfn and the Linux bdfn.

Cheers,
Ben.

> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/platforms/powernv/pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
> index 6d4280086a08..f6eaca3123cd 100644
> --- a/arch/powerpc/platforms/powernv/pci.c
> +++ b/arch/powerpc/platforms/powernv/pci.c
> @@ -1104,6 +1104,7 @@ void __init pnv_pci_init(void)
>  	struct device_node *np;
>  
>  	pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
> +	pci_add_flags(PCI_REASSIGN_ALL_BUS);
>  
>  	/* If we don't have OPAL, eg. in sim, just skip PCI probe */
>  	if (!firmware_has_feature(FW_FEATURE_OPAL))

Re: [PATCH 4/5] powerpc/powernv/pci: Enable reassigning the bus numbers

[Sergey Miroshnichenko]

Hello Ben,

On 9/12/18 1:35 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
>> PowerNV doesn't depend on PCIe topology info from DT anymore, and now
>> it is able to enumerate the fabric and assign the bus numbers.
> 
> No it's not, at least unless we drop P7 support.
> 
> P7 has constraints on the bus ranges being aligned power-of-two for the
> PE assignment to work, which is why we have to honor the firmware
> provided numbers.
> 
> Additionally, this breaks the mapping between the firmware idea of the
> bus numbers and Linux idea. This will probably break all of the SR-IOV
> stuff.
> 

Oh, I see. To make this more controllable and less intrusive I've bound
the PCI_REASSIGN_ALL_BUS flag to the "pci=realloc" command line argument
(in version 3 of this patchset) instead of the unconditional setting.

> Now we should probably fix it all by removing the FW bits completely
> and doing it all from Linux, though we really need to better handle how
> we deal with the segmented MMIO space.
> 
> I would also be weary of what other parts of the code depends on that
> matching between the FW bdfn and the Linux bdfn.
> 

This approach allows us to use the same in-kernel hotplug mechanisms for
PowerNV+OPAL and other platforms, so we are highly interested. Would you
kindly advice what are the essential parts to start with, maybe point
out some documentation on EEH segmentation and FW/OS sync?

> Cheers,
> Ben.
> 

Best regards,
Serge

>> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
>> ---
>>  arch/powerpc/platforms/powernv/pci.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index 6d4280086a08..f6eaca3123cd 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -1104,6 +1104,7 @@ void __init pnv_pci_init(void)
>>  	struct device_node *np;
>>  
>>  	pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN);
>> +	pci_add_flags(PCI_REASSIGN_ALL_BUS);
>>  
>>  	/* If we don't have OPAL, eg. in sim, just skip PCI probe */
>>  	if (!firmware_has_feature(FW_FEATURE_OPAL))
> 

[PATCH 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan

[Sergey Miroshnichenko]

Reading an empty slot returns all ones, which triggers a false
EEH error event on PowerNV.

New callbacks pcibios_rescan_prepare/done are introduced to
pause/resume the EEH during rescan.

Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
 arch/powerpc/include/asm/eeh.h               |  2 ++
 arch/powerpc/kernel/eeh.c                    | 14 ++++++++++++++
 arch/powerpc/platforms/powernv/eeh-powernv.c | 20 ++++++++++++++++++++
 drivers/pci/probe.c                          | 14 ++++++++++++++
 include/linux/pci.h                          |  2 ++
 5 files changed, 52 insertions(+)

diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
index 219637ea69a1..926c3e31df99 100644
--- a/arch/powerpc/include/asm/eeh.h
+++ b/arch/powerpc/include/asm/eeh.h
@@ -219,6 +219,8 @@ struct eeh_ops {
 	int (*next_error)(struct eeh_pe **pe);
 	int (*restore_config)(struct pci_dn *pdn);
 	int (*notify_resume)(struct pci_dn *pdn);
+	int (*pause)(struct pci_bus *bus);
+	int (*resume)(struct pci_bus *bus);
 };
 
 extern int eeh_subsystem_flags;
diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
index 6ebba3e48b01..dce9b0978cb5 100644
--- a/arch/powerpc/kernel/eeh.c
+++ b/arch/powerpc/kernel/eeh.c
@@ -1831,3 +1831,17 @@ static int __init eeh_init_proc(void)
 	return 0;
 }
 __initcall(eeh_init_proc);
+
+void pcibios_rescan_prepare(struct pci_bus *bus)
+{
+	if (eeh_ops && eeh_ops->pause) {
+		eeh_ops->pause(bus);
+	}
+}
+
+void pcibios_rescan_done(struct pci_bus *bus)
+{
+	if (eeh_ops && eeh_ops->resume) {
+		eeh_ops->resume(bus);
+	}
+}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 3c1beae29f2d..9c9213d92550 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -59,6 +59,24 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
 	eeh_sysfs_add_device(pdev);
 }
 
+static int pnv_eeh_pause(struct pci_bus *bus)
+{
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
+	phb->flags &= ~PNV_PHB_FLAG_EEH;
+	disable_irq(eeh_event_irq);
+	return 0;
+}
+
+static int pnv_eeh_resume(struct pci_bus *bus)
+{
+	struct pci_controller *hose = pci_bus_to_host(bus);
+	struct pnv_phb *phb = hose->private_data;
+	enable_irq(eeh_event_irq);
+	phb->flags |= PNV_PHB_FLAG_EEH;
+	return 0;
+}
+
 static int pnv_eeh_init(void)
 {
 	struct pci_controller *hose;
@@ -1710,6 +1728,8 @@ static struct eeh_ops pnv_eeh_ops = {
 	.write_config           = pnv_eeh_write_config,
 	.next_error		= pnv_eeh_next_error,
 	.restore_config		= pnv_eeh_restore_config,
+	.pause			= pnv_eeh_pause,
+	.resume			= pnv_eeh_resume,
 	.notify_resume		= NULL
 };
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ec784009a36b..203368566896 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2893,6 +2893,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
 {
 }
 
+void __weak pcibios_rescan_prepare(struct pci_bus *bus)
+{
+}
+
+void __weak pcibios_rescan_done(struct pci_bus *bus)
+{
+}
+
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 		struct pci_ops *ops, void *sysdata, struct list_head *resources)
 {
@@ -3147,9 +3155,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 unsigned int pci_rescan_bus(struct pci_bus *bus)
 {
 	unsigned int max;
+	struct pci_bus *root = bus;
+	while (!pci_is_root_bus(root)) {
+		root = root->parent;
+	}
 
+	pcibios_rescan_prepare(root);
 	max = pci_scan_child_bus(bus);
 	pci_assign_unassigned_bus_resources(bus);
+	pcibios_rescan_done(root);
 	pci_bus_add_devices(bus);
 
 	return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e72ca8dd6241..d7fe72aa53b3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1952,6 +1952,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
 int pcibios_alloc_irq(struct pci_dev *dev);
 void pcibios_free_irq(struct pci_dev *dev);
 resource_size_t pcibios_default_alignment(void);
+void pcibios_rescan_prepare(struct pci_bus *bus);
+void pcibios_rescan_done(struct pci_bus *bus);
 
 #ifdef CONFIG_HIBERNATE_CALLBACKS
 extern struct dev_pm_ops pcibios_pm_ops;
-- 
2.17.1

Re: [PATCH 5/5] PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan

[Benjamin Herrenschmidt]

On Wed, 2018-09-05 at 18:40 +0300, Sergey Miroshnichenko wrote:
> Reading an empty slot returns all ones, which triggers a false
> EEH error event on PowerNV.
> 
> New callbacks pcibios_rescan_prepare/done are introduced to
> pause/resume the EEH during rescan.

This freaks me out a bit. EEH will still happen in HW, you will just
drop the notifications. Disabling the interrupt globally is not a great
idea either as it will affect other unrelated devices or busses.

Just marking the specific bus might work if we deal with unfreezing it
properly but you need some locking there, your code doesn't seem to
have any.

Ben.

> 
> Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
> ---
>  arch/powerpc/include/asm/eeh.h               |  2 ++
>  arch/powerpc/kernel/eeh.c                    | 14 ++++++++++++++
>  arch/powerpc/platforms/powernv/eeh-powernv.c | 20 ++++++++++++++++++++
>  drivers/pci/probe.c                          | 14 ++++++++++++++
>  include/linux/pci.h                          |  2 ++
>  5 files changed, 52 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 219637ea69a1..926c3e31df99 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -219,6 +219,8 @@ struct eeh_ops {
>  	int (*next_error)(struct eeh_pe **pe);
>  	int (*restore_config)(struct pci_dn *pdn);
>  	int (*notify_resume)(struct pci_dn *pdn);
> +	int (*pause)(struct pci_bus *bus);
> +	int (*resume)(struct pci_bus *bus);
>  };
>  
>  extern int eeh_subsystem_flags;
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 6ebba3e48b01..dce9b0978cb5 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1831,3 +1831,17 @@ static int __init eeh_init_proc(void)
>  	return 0;
>  }
>  __initcall(eeh_init_proc);
> +
> +void pcibios_rescan_prepare(struct pci_bus *bus)
> +{
> +	if (eeh_ops && eeh_ops->pause) {
> +		eeh_ops->pause(bus);
> +	}
> +}
> +
> +void pcibios_rescan_done(struct pci_bus *bus)
> +{
> +	if (eeh_ops && eeh_ops->resume) {
> +		eeh_ops->resume(bus);
> +	}
> +}
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 3c1beae29f2d..9c9213d92550 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -59,6 +59,24 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
>  	eeh_sysfs_add_device(pdev);
>  }
>  
> +static int pnv_eeh_pause(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	phb->flags &= ~PNV_PHB_FLAG_EEH;
> +	disable_irq(eeh_event_irq);
> +	return 0;
> +}
> +
> +static int pnv_eeh_resume(struct pci_bus *bus)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	enable_irq(eeh_event_irq);
> +	phb->flags |= PNV_PHB_FLAG_EEH;
> +	return 0;
> +}
> +
>  static int pnv_eeh_init(void)
>  {
>  	struct pci_controller *hose;
> @@ -1710,6 +1728,8 @@ static struct eeh_ops pnv_eeh_ops = {
>  	.write_config           = pnv_eeh_write_config,
>  	.next_error		= pnv_eeh_next_error,
>  	.restore_config		= pnv_eeh_restore_config,
> +	.pause			= pnv_eeh_pause,
> +	.resume			= pnv_eeh_resume,
>  	.notify_resume		= NULL
>  };
>  
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ec784009a36b..203368566896 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2893,6 +2893,14 @@ void __weak pcibios_remove_bus(struct pci_bus *bus)
>  {
>  }
>  
> +void __weak pcibios_rescan_prepare(struct pci_bus *bus)
> +{
> +}
> +
> +void __weak pcibios_rescan_done(struct pci_bus *bus)
> +{
> +}
> +
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  		struct pci_ops *ops, void *sysdata, struct list_head *resources)
>  {
> @@ -3147,9 +3155,15 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
>  unsigned int pci_rescan_bus(struct pci_bus *bus)
>  {
>  	unsigned int max;
> +	struct pci_bus *root = bus;
> +	while (!pci_is_root_bus(root)) {
> +		root = root->parent;
> +	}
>  
> +	pcibios_rescan_prepare(root);
>  	max = pci_scan_child_bus(bus);
>  	pci_assign_unassigned_bus_resources(bus);
> +	pcibios_rescan_done(root);
>  	pci_bus_add_devices(bus);
>  
>  	return max;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e72ca8dd6241..d7fe72aa53b3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1952,6 +1952,8 @@ void pcibios_penalize_isa_irq(int irq, int active);
>  int pcibios_alloc_irq(struct pci_dev *dev);
>  void pcibios_free_irq(struct pci_dev *dev);
>  resource_size_t pcibios_default_alignment(void);
> +void pcibios_rescan_prepare(struct pci_bus *bus);
> +void pcibios_rescan_done(struct pci_bus *bus);
>  
>  #ifdef CONFIG_HIBERNATE_CALLBACKS
>  extern struct dev_pm_ops pcibios_pm_ops;