[PATCH -v3 0/7] ARI device hotplug support

[PATCH -v3 0/7] ARI device hotplug support

[Yijing Wang]

This patchset mainly to fix pcie ari device hotplug bug.
The last four based on the comment Bjorn suggested at
http://marc.info/?l=linux-pci&m=135766710910208&w=2

v1: Oct 9  2012	add disable ari forwarding in pci hotplug drivers
v2: Oct 16 2012 update ari forwarding in pci_init_capabilities() and 
				rework pciehp_configure_device(),pciehp_unconfigure_device 
				to traverse all ari function devices.
v3: Jan 15 2013 use bus->devices list instead of pci_next_fn to traverse
				all pci fun devices.

Commit 58c3a727cb(PCI: support PCIe ARI capability) introduced PCIe ARI 
capability support. pci_enable_ari() was introduced to enable ari forwarding
bit in pcie port device when a connected pcie ari device was found in pci scan
path. But system never clear ari forwarding bit regardless of a new non-ari pci
device hot-inserted in the slot.

PCIe Spec 2.0(6.13/441) recommends:
"Following a hot-plug event below a Downstream Port, it is strongly recommended
that software Clear the ARI Forwarding Enable bit in the Downstream Port until
software determines that a newly added component is in fact an ARI Device"

Following log described The ari device hotplug bug:
Intel 82576 is PCIe ARI device and Qlogic HBA is non-ari device.
"hot-remove Intel 82576(slot 201) NIC and Qlogic HBA card(slot 212)"
pciehp 0000:08:14.0:pcie24: Button pressed on Slot(212)
pciehp 0000:08:14.0:pcie24: PCI slot #212 - powering off due to button press.
pciehp 0000:08:09.0:pcie24: Button pressed on Slot(201)
pciehp 0000:08:09.0:pcie24: PCI slot #201 - powering off due to button press.
igb 0000:0b:00.0: removed PHC on eth0
igb 0000:0b:00.1: removed PHC on eth1
GSI 39 (level, low) -> CPU 16 (0x0200) vector 85 unregistered
pciehp 0000:08:14.0:pcie24: Latch open on Slot(212)
pciehp 0000:08:14.0:pcie24: Latch close on Slot(212)
pciehp 0000:08:14.0:pcie24: Latch open on Slot(212)
pciehp 0000:08:09.0:pcie24: Latch open on Slot(201)
pciehp 0000:08:14.0:pcie24: Latch close on Slot(212)
pciehp 0000:08:09.0:pcie24: Latch close on Slot(201)

"Inserted Intel 82576 in slot 212(originally HBA slot) and insert HBA in slot 201(insertd 82576)"
pciehp 0000:08:14.0:pcie24: Button pressed on Slot(212)
pciehp 0000:08:14.0:pcie24: PCI slot #212 - powering on due to button press.
pciehp 0000:08:09.0:pcie24: Button pressed on Slot(201)
pciehp 0000:08:09.0:pcie24: PCI slot #201 - powering on due to button press.
pci 0000:0e:00.0: [8086:10c9] type 00 class 0x020000
pci 0000:0e:00.0: reg 10: [mem 0x00000000-0x0001ffff]
pci 0000:0e:00.0: reg 14: [mem 0x00000000-0x0001ffff]
pci 0000:0e:00.0: reg 18: [io  0x0000-0x001f]
pci 0000:0e:00.0: reg 1c: [mem 0x00000000-0x00003fff]
pci 0000:0e:00.0: reg 30: [mem 0x00000000-0x0001ffff pref]
pci 0000:0e:00.0: calling pci_fixup_video+0x0/0x300
pci 0000:0e:00.0: PME# supported from D0 D3hot D3cold
pci 0000:0e:00.0: PME# disabled
pci 0000:0e:00.1: [8086:10c9] type 00 class 0x020000
pci 0000:0e:00.1: reg 10: [mem 0x00000000-0x0001ffff]
pci 0000:0e:00.1: reg 14: [mem 0x00000000-0x0001ffff]
pci 0000:0e:00.1: reg 18: [io  0x0000-0x001f]
pci 0000:0e:00.1: reg 1c: [mem 0x00000000-0x00003fff]
pci 0000:0e:00.1: reg 30: [mem 0x00000000-0x0001ffff pref]
pci 0000:0e:00.1: calling pci_fixup_video+0x0/0x300
pci 0000:0e:00.1: PME# supported from D0 D3hot D3cold
pci 0000:0e:00.1: PME# disabled
pcieport 0000:08:14.0: bridge window [mem 0x00100000-0x001fffff pref] to [bus 0e] add_size 200000
pcieport 0000:08:14.0: res[9]=[mem 0x00100000-0x001fffff pref] get_res_add_size add_size 200000
pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x300000)
pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x100000)
pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
pci 0000:0e:00.0: BAR 0: assigned [mem 0x56c00000-0x56c1ffff]
pci 0000:0e:00.0: BAR 0: set to [mem 0x56c00000-0x56c1ffff] (PCI address [0x56c00000-0x56c1ffff])
pci 0000:0e:00.0: BAR 1: assigned [mem 0x56c20000-0x56c3ffff]
pci 0000:0e:00.0: BAR 1: set to [mem 0x56c20000-0x56c3ffff] (PCI address [0x56c20000-0x56c3ffff])
pci 0000:0e:00.0: BAR 6: assigned [mem 0x56c40000-0x56c5ffff pref]
pci 0000:0e:00.1: BAR 0: assigned [mem 0x56c60000-0x56c7ffff]
pci 0000:0e:00.1: BAR 0: set to [mem 0x56c60000-0x56c7ffff] (PCI address [0x56c60000-0x56c7ffff])
pci 0000:0e:00.1: BAR 1: assigned [mem 0x56c80000-0x56c9ffff]
pci 0000:0e:00.1: BAR 1: set to [mem 0x56c80000-0x56c9ffff] (PCI address [0x56c80000-0x56c9ffff])
pci 0000:0e:00.1: BAR 6: assigned [mem 0x56ca0000-0x56cbffff pref]
pci 0000:0e:00.0: BAR 3: assigned [mem 0x56cc0000-0x56cc3fff]
pci 0000:0e:00.0: BAR 3: set to [mem 0x56cc0000-0x56cc3fff] (PCI address [0x56cc0000-0x56cc3fff])
pci 0000:0e:00.1: BAR 3: assigned [mem 0x56cc4000-0x56cc7fff]
pci 0000:0e:00.1: BAR 3: set to [mem 0x56cc4000-0x56cc7fff] (PCI address [0x56cc4000-0x56cc7fff])
pci 0000:0e:00.0: BAR 2: can't assign io (size 0x20)
pci 0000:0e:00.1: BAR 2: can't assign io (size 0x20)
pcieport 0000:08:14.0: PCI bridge to [bus 0e]
pcieport 0000:08:14.0:   bridge window [mem 0x56c00000-0x56efffff]
PCI: No. 2 try to assign unassigned res
pcieport 0000:08:14.0: bridge window [mem 0x00100000-0x001fffff 64bit pref] to [bus 0e] add_size 200000
pcieport 0000:08:14.0: res[9]=[mem 0x00100000-0x001fffff 64bit pref] get_res_add_size add_size 200000
pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x300000)
pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x100000)
pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
pci 0000:0e:00.0: BAR 2: can't assign io (size 0x20)
pci 0000:0e:00.1: BAR 2: can't assign io (size 0x20)
pcieport 0000:08:14.0: PCI bridge to [bus 0e]
pcieport 0000:08:14.0:   bridge window [mem 0x56c00000-0x56efffff]
pci 0000:0e:00.0: no hotplug settings from platform
pci 0000:0e:00.1: no hotplug settings from platform
pci 0000:0e:00.0: calling quirk_e100_interrupt+0x0/0x500
igb 0000:0e:00.0: enabling device (0000 -> 0002)
igb 0000:0e:00.0: enabling bus mastering
igb 0000:0e:00.0: added PHC on eth0
igb 0000:0e:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:0e:00.0: eth0: (PCIe:2.5Gb/s:Width x4) 90:e2:ba:1e:59:8c
igb 0000:0e:00.0: eth0: PBA No: FFFFFF-0FF
igb 0000:0e:00.0: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
pci 0000:0e:00.1: calling quirk_e100_interrupt+0x0/0x500
igb 0000:0e:00.1: enabling device (0000 -> 0002)
igb 0000:0e:00.1: enabling bus mastering
igb 0000:0e:00.1: added PHC on eth1
igb 0000:0e:00.1: Intel(R) Gigabit Ethernet Network Connection
igb 0000:0e:00.1: eth1: (PCIe:2.5Gb/s:Width x4) 90:e2:ba:1e:59:8d
igb 0000:0e:00.1: eth1: PBA No: FFFFFF-0FF
igb 0000:0e:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
pci 0000:0b:00.0: [1077:2532] type 00 class 0x0c0400
pci 0000:0b:00.0: reg 10: [io  0x0000-0x00ff]
pci 0000:0b:00.0: reg 14: [mem 0x00000000-0x00003fff 64bit]
pci 0000:0b:00.0: reg 30: [mem 0x00000000-0x0003ffff pref]
pci 0000:0b:00.0: calling pci_fixup_video+0x0/0x300
pcieport 0000:08:09.0: bridge window [mem 0x00100000-0x001fffff pref] to [bus 0b] add_size 200000
pcieport 0000:08:09.0: res[9]=[mem 0x00100000-0x001fffff pref] get_res_add_size add_size 200000
pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x300000)
pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x100000)
pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
pci 0000:0b:00.0: BAR 6: assigned [mem 0x57300000-0x5733ffff pref]
pci 0000:0b:00.0: BAR 1: assigned [mem 0x57340000-0x57343fff 64bit]
pci 0000:0b:00.0: BAR 1: set to [mem 0x57340000-0x57343fff 64bit] (PCI address [0x57340000-0x57343fff])
pci 0000:0b:00.0: BAR 0: can't assign io (size 0x100)
pcieport 0000:08:09.0: PCI bridge to [bus 0b]
pcieport 0000:08:09.0:   bridge window [mem 0x57300000-0x575fffff]
PCI: No. 2 try to assign unassigned res
pcieport 0000:08:09.0: bridge window [mem 0x00100000-0x001fffff 64bit pref] to [bus 0b] add_size 200000
pcieport 0000:08:09.0: res[9]=[mem 0x00100000-0x001fffff 64bit pref] get_res_add_size add_size 200000
pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x300000)
pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x100000)
pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
pci 0000:0b:00.0: BAR 0: can't assign io (size 0x100)
pcieport 0000:08:09.0: PCI bridge to [bus 0b]
pcieport 0000:08:09.0:   bridge window [mem 0x57300000-0x575fffff]
pci 0000:0b:00.0: no hotplug settings from platform
qla2xxx 0000:0b:00.0: enabling device (0000 -> 0002)
qla2xxx [0000:0b:00.0]-001d: : Found an ISP2532 irq 68 iobase 0xc000000057340000.
qla2xxx 0000:0b:00.0: enabling bus mastering
qla2xxx 0000:0b:00.0: enabling Mem-Wr-Inval
scsi6 : qla2xxx
qla2xxx [0000:0b:00.0]-00fb:6: QLogic QLE2562 - PCI-Express Dual Channel 8Gb Fibre Channel HBA.
qla2xxx [0000:0b:00.0]-00fc:6: ISP2532: PCIe (5.0GT/s x8) @ 0000:0b:00.0 hdma+ host#=6 fw=5.03.02 (d5).
qla2xxx [0000:0b:00.0]-8038:6: Cable is unplugged...

>From the dmesg log, Qlogic HBA card can only find function device 0 after inserted this card in 
Intel 82576 slot, because ARI device set the ari forwarding of the pcie port device 0000:08:09.0.

The first four patches have been tested in IA64 hotplug machine. The last three patches have not been tested
because my hotplug machine don't support cpcihp,sgihp and shpchp. 

Bjorn Helgaas (4):
  PCI,pciehp: use bus->devices list intead of traditional traversal
  PCI,cpcihp: use bus->devices list instead of traditional traversal
  PCI,sgihp: use bus->devices list intead of traditional traversal
  PCI,shpchp: use bus->devices list instead of traditional traversal

Yijing Wang (3):
  PCI: rework pci_enable_ari to support disable ari forwarding
  PCI: Rename pci_enable_ari to pci_configure_ari
  PCI: introduce pci_next_fn to simplify code

 drivers/pci/hotplug/cpci_hotplug_pci.c |   13 +-----
 drivers/pci/hotplug/pciehp_pci.c       |   46 ++++++++--------------
 drivers/pci/hotplug/sgi_hotplug.c      |   40 ++++++++-----------
 drivers/pci/hotplug/shpchp_pci.c       |   16 +------
 drivers/pci/pci.c                      |   20 ++++++----
 drivers/pci/pci.h                      |    2 +-
 drivers/pci/probe.c                    |   67 +++++++++++++++++---------------
 7 files changed, 89 insertions(+), 115 deletions(-)

[PATCH -v3 1/7] PCI: rework pci_enable_ari to support disable ari forwarding

[Yijing Wang]

pci_enable_ari will set the pci bridge ARI Forwarding Enable bit in Device
Control 2 Register when a ARI pcie device connected found. But the bridge ARI
Forwarding Enable bit will never be cleared when the ARI device hot removed.

As PCIe Spec 2.0(6.13/441) recommends:
"Following a hot-plug event below a Downstream Port, it is strongly recommended
that software Clear the ARI Forwarding Enable bit in the Downstream Port until
software determines that a newly added component is in fact an ARI Device"

This patch rework pci_enable_ari to support disable ARI Forwarding whenever found
the new pci device is a non-ari device.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/pci.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 5cb5820..a17129f 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2078,9 +2078,6 @@ void pci_enable_ari(struct pci_dev *dev)
 	if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
 		return;
 
-	if (!pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI))
-		return;
-
 	bridge = dev->bus->self;
 	if (!bridge)
 		return;
@@ -2088,9 +2085,16 @@ void pci_enable_ari(struct pci_dev *dev)
 	pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
 	if (!(cap & PCI_EXP_DEVCAP2_ARI))
 		return;
-
-	pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_ARI);
-	bridge->ari_enabled = 1;
+
+	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) {
+		pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2,
+				PCI_EXP_DEVCTL2_ARI);
+		bridge->ari_enabled = 1;
+	} else {
+		pcie_capability_clear_word(bridge, PCI_EXP_DEVCTL2,
+				PCI_EXP_DEVCTL2_ARI);
+		bridge->ari_enabled = 0;
+	}
 }
 
 /**
-- 
1.7.1

[PATCH -v3 2/7] PCI: Rename pci_enable_ari to pci_configure_ari

[Yijing Wang]

pci_enable_ari now support enable or disable ARI forwarding.
So rename pci_enable_ari() to pci_configure_ari for easy
understanding.

No functional change.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/pci.c   |    4 ++--
 drivers/pci/pci.h   |    2 +-
 drivers/pci/probe.c |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a17129f..d921900 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2067,10 +2067,10 @@ void pci_free_cap_save_buffers(struct pci_dev *dev)
 }
 
 /**
- * pci_enable_ari - enable ARI forwarding if hardware support it
+ * pci_configure_ari - enable or disable ARI forwarding
  * @dev: the PCI device
  */
-void pci_enable_ari(struct pci_dev *dev)
+void pci_configure_ari(struct pci_dev *dev)
 {
 	u32 cap;
 	struct pci_dev *bridge;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index e851829..19043cb 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -209,7 +209,7 @@ extern int __pci_read_base(struct pci_dev *dev, enum pci_bar_type type,
 extern int pci_resource_bar(struct pci_dev *dev, int resno,
 			    enum pci_bar_type *type);
 extern int pci_bus_add_child(struct pci_bus *bus);
-extern void pci_enable_ari(struct pci_dev *dev);
+extern void pci_configure_ari(struct pci_dev *dev);
 /**
  * pci_ari_enabled - query ARI forwarding status
  * @bus: the PCI bus
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 6186f03..7b9e691 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1286,7 +1286,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_vpd_pci22_init(dev);
 
 	/* Alternative Routing-ID Forwarding */
-	pci_enable_ari(dev);
+	pci_configure_ari(dev);
 
 	/* Single Root I/O Virtualization */
 	pci_iov_init(dev);
-- 
1.7.1

[PATCH -v3 3/7] PCI: introduce pci_next_fn to simplify code

[Yijing Wang]

There are several next_fn functions(no_next_fn,next_trad_fn,next_ari_fn)
now, introduce pci_next_fun to simplify the code.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
---
 drivers/pci/probe.c |   65 +++++++++++++++++++++++++++-----------------------
 1 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7b9e691..abc6e31 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1349,34 +1349,44 @@ struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_scan_single_device);
 
-static unsigned next_ari_fn(struct pci_dev *dev, unsigned fn)
+/**
+ * pci_next_fn - get next fn number for device
+ * @bus: PCI bus on which desired PCI Function device resides
+ * @dev: pci device
+ * @fn: fn number of pci device
+ *
+ * return next_fn if success, if can not find next fn or fail,
+ * 0 is returned.
+ */
+int pci_next_fn(struct pci_bus *bus, struct pci_dev *dev,
+		unsigned fn)
 {
 	u16 cap;
-	unsigned pos, next_fn;
-
-	if (!dev)
-		return 0;
-
-	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
-	if (!pos)
-		return 0;
-	pci_read_config_word(dev, pos + 4, &cap);
-	next_fn = cap >> 8;
-	if (next_fn <= fn)
-		return 0;
+	int pos, next_fn = 0;
+
+	if (!bus)
+		goto out;
+
+	if (!pci_ari_enabled(bus)) {
+		if (dev && !dev->multifunction)
+			goto out;
+		else
+			next_fn = (fn + 1) % 8;
+	} else {
+		if (!dev)
+			goto out;
+		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
+		if (!pos)
+			goto out;
+		pci_read_config_word(dev, pos + 4, &cap);
+		next_fn = cap >> 8;
+		if (next_fn <= fn)
+			next_fn = 0;
+	}
+out:
 	return next_fn;
 }
 
-static unsigned next_trad_fn(struct pci_dev *dev, unsigned fn)
-{
-	return (fn + 1) % 8;
-}
-
-static unsigned no_next_fn(struct pci_dev *dev, unsigned fn)
-{
-	return 0;
-}
-
 static int only_one_child(struct pci_bus *bus)
 {
 	struct pci_dev *parent = bus->self;
@@ -1406,7 +1416,6 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 {
 	unsigned fn, nr = 0;
 	struct pci_dev *dev;
-	unsigned (*next_fn)(struct pci_dev *, unsigned) = no_next_fn;
 
 	if (only_one_child(bus) && (devfn > 0))
 		return 0; /* Already scanned the entire slot */
@@ -1417,12 +1426,8 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 	if (!dev->is_added)
 		nr++;
 
-	if (pci_ari_enabled(bus))
-		next_fn = next_ari_fn;
-	else if (dev->multifunction)
-		next_fn = next_trad_fn;
-
-	for (fn = next_fn(dev, 0); fn > 0; fn = next_fn(dev, fn)) {
+	for (fn = pci_next_fn(bus, dev, 0); fn > 0;
+			fn = pci_next_fn(bus, dev, fn)) {
 		dev = pci_scan_single_device(bus, devfn + fn);
 		if (dev) {
 			if (!dev->is_added)
-- 
1.7.1

[PATCH -v3 4/7] PCI,pciehp: use bus->devices list intead of traditional traversal

[Yijing Wang]

From: Bjorn Helgaas <bhelgaas@google.com>

pciehp driver traverses pci function deivces by for(fn = 0; fn < 8; fn++) to
configure every pci device when enable or disable slot.
If the pci device to be add/remove is an ARI device, this traversal may
miss some function device configuration, because ari device function
number is up to 255. This patch fix this problem by traversing bus->devices
list to configure pci device.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/pciehp_pci.c |   46 ++++++++++++++------------------------
 1 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index 09cecaf..0cc1977 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -39,7 +39,7 @@ int pciehp_configure_device(struct slot *p_slot)
 	struct pci_dev *dev;
 	struct pci_dev *bridge = p_slot->ctrl->pcie->port;
 	struct pci_bus *parent = bridge->subordinate;
-	int num, fn;
+	int num;
 	struct controller *ctrl = p_slot->ctrl;
 
 	dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
@@ -57,28 +57,18 @@ int pciehp_configure_device(struct slot *p_slot)
 		return -ENODEV;
 	}
 
-	for (fn = 0; fn < 8; fn++) {
-		dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
-		if (!dev)
-			continue;
+	list_for_each_entry(dev, &parent->devices, bus_list)
 		if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
 				(dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
 			pci_hp_add_bridge(dev);
-		pci_dev_put(dev);
-	}
-
+
 	pci_assign_unassigned_bridge_resources(bridge);
 
-	for (fn = 0; fn < 8; fn++) {
-		dev = pci_get_slot(parent, PCI_DEVFN(0, fn));
-		if (!dev)
-			continue;
-		if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY) {
-			pci_dev_put(dev);
+	list_for_each_entry(dev, &parent->devices, bus_list) {
+		if ((dev->class >> 16) == PCI_BASE_CLASS_DISPLAY)
 			continue;
-		}
+
 		pci_configure_slot(dev);
-		pci_dev_put(dev);
 	}
 
 	pci_bus_add_devices(parent);
@@ -89,9 +79,9 @@ int pciehp_configure_device(struct slot *p_slot)
 int pciehp_unconfigure_device(struct slot *p_slot)
 {
 	int ret, rc = 0;
-	int j;
 	u8 bctl = 0;
 	u8 presence = 0;
+	struct pci_dev *dev, *temp;
 	struct pci_bus *parent = p_slot->ctrl->pcie->port->subordinate;
 	u16 command;
 	struct controller *ctrl = p_slot->ctrl;
@@ -102,33 +92,31 @@ int pciehp_unconfigure_device(struct slot *p_slot)
 	if (ret)
 		presence = 0;
 
-	for (j = 0; j < 8; j++) {
-		struct pci_dev *temp = pci_get_slot(parent, PCI_DEVFN(0, j));
-		if (!temp)
-			continue;
-		if (temp->hdr_type == PCI_HEADER_TYPE_BRIDGE && presence) {
-			pci_read_config_byte(temp, PCI_BRIDGE_CONTROL, &bctl);
+	list_for_each_entry_safe(dev, temp, &parent->devices, bus_list) {
+		pci_dev_get(dev);
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE && presence) {
+			pci_read_config_byte(dev, PCI_BRIDGE_CONTROL, &bctl);
 			if (bctl & PCI_BRIDGE_CTL_VGA) {
 				ctrl_err(ctrl,
 					 "Cannot remove display device %s\n",
-					 pci_name(temp));
-				pci_dev_put(temp);
+					 pci_name(dev));
+				pci_dev_put(dev);
 				rc = -EINVAL;
 				break;
 			}
 		}
-		pci_stop_and_remove_bus_device(temp);
+		pci_stop_and_remove_bus_device(dev);
 		/*
 		 * Ensure that no new Requests will be generated from
 		 * the device.
 		 */
 		if (presence) {
-			pci_read_config_word(temp, PCI_COMMAND, &command);
+			pci_read_config_word(dev, PCI_COMMAND, &command);
 			command &= ~(PCI_COMMAND_MASTER | PCI_COMMAND_SERR);
 			command |= PCI_COMMAND_INTX_DISABLE;
-			pci_write_config_word(temp, PCI_COMMAND, command);
+			pci_write_config_word(dev, PCI_COMMAND, command);
 		}
-		pci_dev_put(temp);
+		pci_dev_put(dev);
 	}
 
 	return rc;
-- 
1.7.1

[PATCH -v3 5/7] PCI,cpcihp: use bus->devices list instead of traditional traversal

[Yijing Wang]

From: Bjorn Helgaas <bhelgaas@google.com>

Use bus->devices list to traverse the pci function devices when
configure or unconfigure the slot like other pci hotplug drivers.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/cpci_hotplug_pci.c |   13 +++----------
 1 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/hotplug/cpci_hotplug_pci.c b/drivers/pci/hotplug/cpci_hotplug_pci.c
index dcc75c7..9329642 100644
--- a/drivers/pci/hotplug/cpci_hotplug_pci.c
+++ b/drivers/pci/hotplug/cpci_hotplug_pci.c
@@ -252,8 +252,8 @@ int cpci_led_off(struct slot* slot)
 
 int __ref cpci_configure_slot(struct slot *slot)
 {
+	struct pci_dev *dev;
 	struct pci_bus *parent;
-	int fn;
 
 	dbg("%s - enter", __func__);
 
@@ -282,18 +282,11 @@ int __ref cpci_configure_slot(struct slot *slot)
 	}
 	parent = slot->dev->bus;
 
-	for (fn = 0; fn < 8; fn++) {
-		struct pci_dev *dev;
-
-		dev = pci_get_slot(parent,
-				   PCI_DEVFN(PCI_SLOT(slot->devfn), fn));
-		if (!dev)
-			continue;
+	list_for_each_entry(dev, &parent->devices, bus_list)
 		if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
 		    (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
 			pci_hp_add_bridge(dev);
-		pci_dev_put(dev);
-	}
+
 
 	pci_assign_unassigned_bridge_resources(parent->self);
 
-- 
1.7.1

[PATCH -v3 6/7] PCI,sgihp: use bus->devices list intead of traditional traversal

[Yijing Wang]

From: Bjorn Helgaas <bhelgaas@google.com>

Use bus->devices list to traverse the pci function devices when
configure or unconfigure the slot like other pci hotplug drivers.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/sgi_hotplug.c |   40 +++++++++++++++---------------------
 1 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index f64ca92..45966b8 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -334,7 +334,7 @@ static int enable_slot(struct hotplug_slot *bss_hotplug_slot)
 	struct slot *slot = bss_hotplug_slot->private;
 	struct pci_bus *new_bus = NULL;
 	struct pci_dev *dev;
-	int func, num_funcs;
+	int num_funcs;
 	int new_ppb = 0;
 	int rc;
 	char *ssdt = NULL;
@@ -381,29 +381,23 @@ static int enable_slot(struct hotplug_slot *bss_hotplug_slot)
 	 * to the Linux PCI interface and tell the drivers
 	 * about them.
 	 */
-	for (func = 0; func < num_funcs;  func++) {
-		dev = pci_get_slot(slot->pci_bus,
-				   PCI_DEVFN(slot->device_num + 1,
-					     PCI_FUNC(func)));
-		if (dev) {
-			/* Need to do slot fixup on PPB before fixup of children
-			 * (PPB's pcidev_info needs to be in pcidev_info list
-			 * before child's SN_PCIDEV_INFO() call to setup
-			 * pdi_host_pcidev_info).
-			 */
-			pcibios_fixup_device_resources(dev);
-			if (SN_ACPI_BASE_SUPPORT())
-				sn_acpi_slot_fixup(dev);
-			else
-				sn_io_slot_fixup(dev);
-			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
-				pci_hp_add_bridge(dev);
-				if (dev->subordinate) {
-					new_bus = dev->subordinate;
-					new_ppb = 1;
-				}
+	list_for_each_entry(dev, &slot->pci_bus->devices, bus_list) {
+		/* Need to do slot fixup on PPB before fixup of children
+		 * (PPB's pcidev_info needs to be in pcidev_info list
+		 * before child's SN_PCIDEV_INFO() call to setup
+		 * pdi_host_pcidev_info).
+	 */
+		pcibios_fixup_device_resources(dev);
+		if (SN_ACPI_BASE_SUPPORT())
+			sn_acpi_slot_fixup(dev);
+		else
+			sn_io_slot_fixup(dev);
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+			pci_hp_add_bridge(dev);
+			if (dev->subordinate) {
+				new_bus = dev->subordinate;
+				new_ppb = 1;
 			}
-			pci_dev_put(dev);
 		}
 	}
 
-- 
1.7.1

[PATCH -v3 7/7] PCI,shpchp: use bus->devices list instead of traditional traversal

[Yijing Wang]

From: Bjorn Helgaas <bhelgaas@google.com>

Use bus->devices list to traverse the pci function devices when
configure or unconfigure the slot like other pci hotplug drivers.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/pci/hotplug/shpchp_pci.c |   16 +++-------------
 1 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/hotplug/shpchp_pci.c b/drivers/pci/hotplug/shpchp_pci.c
index c627ed9..0eda985 100644
--- a/drivers/pci/hotplug/shpchp_pci.c
+++ b/drivers/pci/hotplug/shpchp_pci.c
@@ -40,7 +40,7 @@ int __ref shpchp_configure_device(struct slot *p_slot)
 	struct controller *ctrl = p_slot->ctrl;
 	struct pci_dev *bridge = ctrl->pci_dev;
 	struct pci_bus *parent = bridge->subordinate;
-	int num, fn;
+	int num;
 
 	dev = pci_get_slot(parent, PCI_DEVFN(p_slot->device, 0));
 	if (dev) {
@@ -57,25 +57,15 @@ int __ref shpchp_configure_device(struct slot *p_slot)
 		return -ENODEV;
 	}
 
-	for (fn = 0; fn < 8; fn++) {
-		dev = pci_get_slot(parent, PCI_DEVFN(p_slot->device, fn));
-		if (!dev)
-			continue;
+	list_for_each_entry(dev, &parent->devices, bus_list)
 		if ((dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) ||
 		    (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS))
 			pci_hp_add_bridge(dev);
-		pci_dev_put(dev);
-	}
 
 	pci_assign_unassigned_bridge_resources(bridge);
 
-	for (fn = 0; fn < 8; fn++) {
-		dev = pci_get_slot(parent, PCI_DEVFN(p_slot->device, fn));
-		if (!dev)
-			continue;
+	list_for_each_entry(dev, &parent->devices, bus_list)
 		pci_configure_slot(dev);
-		pci_dev_put(dev);
-	}
 
 	pci_bus_add_devices(parent);
 
-- 
1.7.1

Re: [PATCH -v3 0/7] ARI device hotplug support

[Bjorn Helgaas]

[+cc Jack]

On Mon, Jan 14, 2013 at 8:12 PM, Yijing Wang <wangyijing@huawei.com> wrote:
> This patchset mainly to fix pcie ari device hotplug bug.
> The last four based on the comment Bjorn suggested at
> http://marc.info/?l=linux-pci&m=135766710910208&w=2
>
> v1: Oct 9  2012 add disable ari forwarding in pci hotplug drivers
> v2: Oct 16 2012 update ari forwarding in pci_init_capabilities() and
>                                 rework pciehp_configure_device(),pciehp_unconfigure_device
>                                 to traverse all ari function devices.
> v3: Jan 15 2013 use bus->devices list instead of pci_next_fn to traverse
>                                 all pci fun devices.
>
> Commit 58c3a727cb(PCI: support PCIe ARI capability) introduced PCIe ARI
> capability support. pci_enable_ari() was introduced to enable ari forwarding
> bit in pcie port device when a connected pcie ari device was found in pci scan
> path. But system never clear ari forwarding bit regardless of a new non-ari pci
> device hot-inserted in the slot.
>
> PCIe Spec 2.0(6.13/441) recommends:
> "Following a hot-plug event below a Downstream Port, it is strongly recommended
> that software Clear the ARI Forwarding Enable bit in the Downstream Port until
> software determines that a newly added component is in fact an ARI Device"
>
> Following log described The ari device hotplug bug:
> Intel 82576 is PCIe ARI device and Qlogic HBA is non-ari device.
> "hot-remove Intel 82576(slot 201) NIC and Qlogic HBA card(slot 212)"
> pciehp 0000:08:14.0:pcie24: Button pressed on Slot(212)
> pciehp 0000:08:14.0:pcie24: PCI slot #212 - powering off due to button press.
> pciehp 0000:08:09.0:pcie24: Button pressed on Slot(201)
> pciehp 0000:08:09.0:pcie24: PCI slot #201 - powering off due to button press.
> igb 0000:0b:00.0: removed PHC on eth0
> igb 0000:0b:00.1: removed PHC on eth1
> GSI 39 (level, low) -> CPU 16 (0x0200) vector 85 unregistered
> pciehp 0000:08:14.0:pcie24: Latch open on Slot(212)
> pciehp 0000:08:14.0:pcie24: Latch close on Slot(212)
> pciehp 0000:08:14.0:pcie24: Latch open on Slot(212)
> pciehp 0000:08:09.0:pcie24: Latch open on Slot(201)
> pciehp 0000:08:14.0:pcie24: Latch close on Slot(212)
> pciehp 0000:08:09.0:pcie24: Latch close on Slot(201)
>
> "Inserted Intel 82576 in slot 212(originally HBA slot) and insert HBA in slot 201(insertd 82576)"
> pciehp 0000:08:14.0:pcie24: Button pressed on Slot(212)
> pciehp 0000:08:14.0:pcie24: PCI slot #212 - powering on due to button press.
> pciehp 0000:08:09.0:pcie24: Button pressed on Slot(201)
> pciehp 0000:08:09.0:pcie24: PCI slot #201 - powering on due to button press.
> pci 0000:0e:00.0: [8086:10c9] type 00 class 0x020000
> pci 0000:0e:00.0: reg 10: [mem 0x00000000-0x0001ffff]
> pci 0000:0e:00.0: reg 14: [mem 0x00000000-0x0001ffff]
> pci 0000:0e:00.0: reg 18: [io  0x0000-0x001f]
> pci 0000:0e:00.0: reg 1c: [mem 0x00000000-0x00003fff]
> pci 0000:0e:00.0: reg 30: [mem 0x00000000-0x0001ffff pref]
> pci 0000:0e:00.0: calling pci_fixup_video+0x0/0x300
> pci 0000:0e:00.0: PME# supported from D0 D3hot D3cold
> pci 0000:0e:00.0: PME# disabled
> pci 0000:0e:00.1: [8086:10c9] type 00 class 0x020000
> pci 0000:0e:00.1: reg 10: [mem 0x00000000-0x0001ffff]
> pci 0000:0e:00.1: reg 14: [mem 0x00000000-0x0001ffff]
> pci 0000:0e:00.1: reg 18: [io  0x0000-0x001f]
> pci 0000:0e:00.1: reg 1c: [mem 0x00000000-0x00003fff]
> pci 0000:0e:00.1: reg 30: [mem 0x00000000-0x0001ffff pref]
> pci 0000:0e:00.1: calling pci_fixup_video+0x0/0x300
> pci 0000:0e:00.1: PME# supported from D0 D3hot D3cold
> pci 0000:0e:00.1: PME# disabled
> pcieport 0000:08:14.0: bridge window [mem 0x00100000-0x001fffff pref] to [bus 0e] add_size 200000
> pcieport 0000:08:14.0: res[9]=[mem 0x00100000-0x001fffff pref] get_res_add_size add_size 200000
> pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x300000)
> pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
> pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x100000)
> pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
> pci 0000:0e:00.0: BAR 0: assigned [mem 0x56c00000-0x56c1ffff]
> pci 0000:0e:00.0: BAR 0: set to [mem 0x56c00000-0x56c1ffff] (PCI address [0x56c00000-0x56c1ffff])
> pci 0000:0e:00.0: BAR 1: assigned [mem 0x56c20000-0x56c3ffff]
> pci 0000:0e:00.0: BAR 1: set to [mem 0x56c20000-0x56c3ffff] (PCI address [0x56c20000-0x56c3ffff])
> pci 0000:0e:00.0: BAR 6: assigned [mem 0x56c40000-0x56c5ffff pref]
> pci 0000:0e:00.1: BAR 0: assigned [mem 0x56c60000-0x56c7ffff]
> pci 0000:0e:00.1: BAR 0: set to [mem 0x56c60000-0x56c7ffff] (PCI address [0x56c60000-0x56c7ffff])
> pci 0000:0e:00.1: BAR 1: assigned [mem 0x56c80000-0x56c9ffff]
> pci 0000:0e:00.1: BAR 1: set to [mem 0x56c80000-0x56c9ffff] (PCI address [0x56c80000-0x56c9ffff])
> pci 0000:0e:00.1: BAR 6: assigned [mem 0x56ca0000-0x56cbffff pref]
> pci 0000:0e:00.0: BAR 3: assigned [mem 0x56cc0000-0x56cc3fff]
> pci 0000:0e:00.0: BAR 3: set to [mem 0x56cc0000-0x56cc3fff] (PCI address [0x56cc0000-0x56cc3fff])
> pci 0000:0e:00.1: BAR 3: assigned [mem 0x56cc4000-0x56cc7fff]
> pci 0000:0e:00.1: BAR 3: set to [mem 0x56cc4000-0x56cc7fff] (PCI address [0x56cc4000-0x56cc7fff])
> pci 0000:0e:00.0: BAR 2: can't assign io (size 0x20)
> pci 0000:0e:00.1: BAR 2: can't assign io (size 0x20)
> pcieport 0000:08:14.0: PCI bridge to [bus 0e]
> pcieport 0000:08:14.0:   bridge window [mem 0x56c00000-0x56efffff]
> PCI: No. 2 try to assign unassigned res
> pcieport 0000:08:14.0: bridge window [mem 0x00100000-0x001fffff 64bit pref] to [bus 0e] add_size 200000
> pcieport 0000:08:14.0: res[9]=[mem 0x00100000-0x001fffff 64bit pref] get_res_add_size add_size 200000
> pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x300000)
> pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
> pcieport 0000:08:14.0: BAR 9: can't assign mem pref (size 0x100000)
> pcieport 0000:08:14.0: BAR 7: can't assign io (size 0x1000)
> pci 0000:0e:00.0: BAR 2: can't assign io (size 0x20)
> pci 0000:0e:00.1: BAR 2: can't assign io (size 0x20)
> pcieport 0000:08:14.0: PCI bridge to [bus 0e]
> pcieport 0000:08:14.0:   bridge window [mem 0x56c00000-0x56efffff]
> pci 0000:0e:00.0: no hotplug settings from platform
> pci 0000:0e:00.1: no hotplug settings from platform
> pci 0000:0e:00.0: calling quirk_e100_interrupt+0x0/0x500
> igb 0000:0e:00.0: enabling device (0000 -> 0002)
> igb 0000:0e:00.0: enabling bus mastering
> igb 0000:0e:00.0: added PHC on eth0
> igb 0000:0e:00.0: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:0e:00.0: eth0: (PCIe:2.5Gb/s:Width x4) 90:e2:ba:1e:59:8c
> igb 0000:0e:00.0: eth0: PBA No: FFFFFF-0FF
> igb 0000:0e:00.0: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
> pci 0000:0e:00.1: calling quirk_e100_interrupt+0x0/0x500
> igb 0000:0e:00.1: enabling device (0000 -> 0002)
> igb 0000:0e:00.1: enabling bus mastering
> igb 0000:0e:00.1: added PHC on eth1
> igb 0000:0e:00.1: Intel(R) Gigabit Ethernet Network Connection
> igb 0000:0e:00.1: eth1: (PCIe:2.5Gb/s:Width x4) 90:e2:ba:1e:59:8d
> igb 0000:0e:00.1: eth1: PBA No: FFFFFF-0FF
> igb 0000:0e:00.1: Using MSI-X interrupts. 8 rx queue(s), 8 tx queue(s)
> pci 0000:0b:00.0: [1077:2532] type 00 class 0x0c0400
> pci 0000:0b:00.0: reg 10: [io  0x0000-0x00ff]
> pci 0000:0b:00.0: reg 14: [mem 0x00000000-0x00003fff 64bit]
> pci 0000:0b:00.0: reg 30: [mem 0x00000000-0x0003ffff pref]
> pci 0000:0b:00.0: calling pci_fixup_video+0x0/0x300
> pcieport 0000:08:09.0: bridge window [mem 0x00100000-0x001fffff pref] to [bus 0b] add_size 200000
> pcieport 0000:08:09.0: res[9]=[mem 0x00100000-0x001fffff pref] get_res_add_size add_size 200000
> pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x300000)
> pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
> pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x100000)
> pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
> pci 0000:0b:00.0: BAR 6: assigned [mem 0x57300000-0x5733ffff pref]
> pci 0000:0b:00.0: BAR 1: assigned [mem 0x57340000-0x57343fff 64bit]
> pci 0000:0b:00.0: BAR 1: set to [mem 0x57340000-0x57343fff 64bit] (PCI address [0x57340000-0x57343fff])
> pci 0000:0b:00.0: BAR 0: can't assign io (size 0x100)
> pcieport 0000:08:09.0: PCI bridge to [bus 0b]
> pcieport 0000:08:09.0:   bridge window [mem 0x57300000-0x575fffff]
> PCI: No. 2 try to assign unassigned res
> pcieport 0000:08:09.0: bridge window [mem 0x00100000-0x001fffff 64bit pref] to [bus 0b] add_size 200000
> pcieport 0000:08:09.0: res[9]=[mem 0x00100000-0x001fffff 64bit pref] get_res_add_size add_size 200000
> pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x300000)
> pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
> pcieport 0000:08:09.0: BAR 9: can't assign mem pref (size 0x100000)
> pcieport 0000:08:09.0: BAR 7: can't assign io (size 0x1000)
> pci 0000:0b:00.0: BAR 0: can't assign io (size 0x100)
> pcieport 0000:08:09.0: PCI bridge to [bus 0b]
> pcieport 0000:08:09.0:   bridge window [mem 0x57300000-0x575fffff]
> pci 0000:0b:00.0: no hotplug settings from platform
> qla2xxx 0000:0b:00.0: enabling device (0000 -> 0002)
> qla2xxx [0000:0b:00.0]-001d: : Found an ISP2532 irq 68 iobase 0xc000000057340000.
> qla2xxx 0000:0b:00.0: enabling bus mastering
> qla2xxx 0000:0b:00.0: enabling Mem-Wr-Inval
> scsi6 : qla2xxx
> qla2xxx [0000:0b:00.0]-00fb:6: QLogic QLE2562 - PCI-Express Dual Channel 8Gb Fibre Channel HBA.
> qla2xxx [0000:0b:00.0]-00fc:6: ISP2532: PCIe (5.0GT/s x8) @ 0000:0b:00.0 hdma+ host#=6 fw=5.03.02 (d5).
> qla2xxx [0000:0b:00.0]-8038:6: Cable is unplugged...
>
> From the dmesg log, Qlogic HBA card can only find function device 0 after inserted this card in
> Intel 82576 slot, because ARI device set the ari forwarding of the pcie port device 0000:08:09.0.
>
> The first four patches have been tested in IA64 hotplug machine. The last three patches have not been tested
> because my hotplug machine don't support cpcihp,sgihp and shpchp.
>
> Bjorn Helgaas (4):
>   PCI,pciehp: use bus->devices list intead of traditional traversal
>   PCI,cpcihp: use bus->devices list instead of traditional traversal
>   PCI,sgihp: use bus->devices list intead of traditional traversal
>   PCI,shpchp: use bus->devices list instead of traditional traversal
>
> Yijing Wang (3):
>   PCI: rework pci_enable_ari to support disable ari forwarding
>   PCI: Rename pci_enable_ari to pci_configure_ari
>   PCI: introduce pci_next_fn to simplify code
>
>  drivers/pci/hotplug/cpci_hotplug_pci.c |   13 +-----
>  drivers/pci/hotplug/pciehp_pci.c       |   46 ++++++++--------------
>  drivers/pci/hotplug/sgi_hotplug.c      |   40 ++++++++-----------
>  drivers/pci/hotplug/shpchp_pci.c       |   16 +------
>  drivers/pci/pci.c                      |   20 ++++++----
>  drivers/pci/pci.h                      |    2 +-
>  drivers/pci/probe.c                    |   67 +++++++++++++++++---------------
>  7 files changed, 89 insertions(+), 115 deletions(-)

I applied this series on the pci/yijing-ari branch in my git tree,
with the following changes:

  - Updated changelogs for readability
  - Reworked next_fn() and made it static
  - Updated the unconfigure/disable paths for cpcihp, sgihp, shpchp
  - Check PCI_SLOT for non-PCIe drivers in case a bus has several slots
  - Reset "Author:" to Yijing (since you wrote the original patches)

Please review the changes I made and test the parts you can.  I need
your acknowledgement before putting these in "next" with  your
Signed-off-by because I changed them so much.

I think there are really two defects you're fixing here:

  (1) If you hot-remove an ARI device and replace it with a non-ARI
multi-function device, we find only function 0 of the new device
because the upstream bridge still has ARI enabled, and next_ari_fn()
only returns function 0 for non-ARI devices.  Patch [1/7] fixes this.
I think this is the issue shown by your dmesg quotes above.

  (2) If you hot-add an ARI device, the PCI core enumerates all the
functions, but pciehp only initializes functions 0-7, and other
functions don't work correctly.  Additionally, if you hot-remove the
device, pciehp only removes functions 0-7, leaving stale pci_dev
structures around.  Patch [4/7] fixes this.

If my understanding is correct, I'll update the commit logs to mention
these scenarios explicitly.

Bjorn

Re: [PATCH -v3 0/7] ARI device hotplug support

[Yijing Wang]

[-- Attachment #1: Type: text/plain, Size: 3900 bytes --]

On 2013/1/25 6:45, Bjorn Helgaas wrote:
Hi Bjorn,
   Thanks for your review and great work for this series patches!
> 
> I applied this series on the pci/yijing-ari branch in my git tree,
> with the following changes:
> 
>   - Updated changelogs for readability
>   - Reworked next_fn() and made it static
>   - Updated the unconfigure/disable paths for cpcihp, sgihp, shpchp
>   - Check PCI_SLOT for non-PCIe drivers in case a bus has several slots
>   - Reset "Author:" to Yijing (since you wrote the original patches)
> 
> Please review the changes I made and test the parts you can.  I need
> your acknowledgement before putting these in "next" with  your
> Signed-off-by because I changed them so much.

I reviewed the changes and tested this series again in my hotplug machine.
Most of the changes looks good to me except [PATCH 3/7] PCI: Consolidate "next-function" functions.

Currently, if parameter "dev" is passed as NULL (pci_scan_single_device() return NULL), next_fn
will return 0, so pci_scan_slot() will stop scanning rest function devices in this slot.
According to PCI 3.0 Spec "Multi-function devices require to always implement function 0
in the device. Implementing other functions is optional and maybe assigned in any order".
So we will miss some function devices when scanning pci slot. I tested this patch in my machine and found it boot failed,
because some usb devices can not found.

+static unsigned next_fn(struct pci_bus *bus, struct pci_dev *dev, unsigned fn)
 {
-	u16 cap;
-	unsigned pos, next_fn;
+	int pos;
+	u16 cap = 0;
+	unsigned next_fn;

 	if (!dev)
 		return 0;

lspci info:
00:16.0 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.1 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.2 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.3 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.4 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.5 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.6 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:16.7 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
00:1a.0 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4
00:1a.1 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5
00:1a.2 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6
00:1a.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2

I fixed this problem with [PATCH 3/7] PCI: Consolidate "next-function" functions.
And attach this refreshed patch at the end. This patch has been tested, and result is ok in my machine.

> 
> I think there are really two defects you're fixing here:
> 
>   (1) If you hot-remove an ARI device and replace it with a non-ARI
> multi-function device, we find only function 0 of the new device
> because the upstream bridge still has ARI enabled, and next_ari_fn()
> only returns function 0 for non-ARI devices.  Patch [1/7] fixes this.
> I think this is the issue shown by your dmesg quotes above.
> 
>   (2) If you hot-add an ARI device, the PCI core enumerates all the
> functions, but pciehp only initializes functions 0-7, and other
> functions don't work correctly.  Additionally, if you hot-remove the
> device, pciehp only removes functions 0-7, leaving stale pci_dev
> structures around.  Patch [4/7] fixes this.
> 
> If my understanding is correct, I'll update the commit logs to mention
> these scenarios explicitly.

Yes, exactly.

Thanks!
Yijing.

> 
> Bjorn
> 
> .
> 


-- 
Thanks!
Yijing

[-- Attachment #2: 0001-PCI-Consolidate-next-function-functions.patch --]
[-- Type: text/x-patch, Size: 2738 bytes --]

>From 440b930c73d41328c2e355ce989d0e26ee69a50e Mon Sep 17 00:00:00 2001
From: Yijing Wang <wangyijing@huawei.com>
Date: Tue, 15 Jan 2013 11:12:18 +0800
Subject: [PATCH] PCI: Consolidate "next-function" functions

There are several next_fn functions (no_next_fn, next_trad_fn,
next_ari_fn); consolidate them in next_fn() to simplify the code.

[bhelgaas: rework code, make next_fn() static, remove NULL checks]
Signed-off-by: Yijing Wang <wangyijing@huawei.com>
Signed-off-by: Jiang Liu <jiang.liu@huawei.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/probe.c |   47 ++++++++++++++++++++---------------------------
 1 files changed, 20 insertions(+), 27 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7b9e691..75721a2 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1349,31 +1349,30 @@ struct pci_dev *__ref pci_scan_single_device(struct pci_bus *bus, int devfn)
 }
 EXPORT_SYMBOL(pci_scan_single_device);
 
-static unsigned next_ari_fn(struct pci_dev *dev, unsigned fn)
+static unsigned next_fn(struct pci_bus *bus, struct pci_dev *dev, unsigned fn)
 {
-	u16 cap;
-	unsigned pos, next_fn;
+	int pos;
+	u16 cap = 0;
+	unsigned next_fn;
 
-	if (!dev)
-		return 0;
+	if (pci_ari_enabled(bus)) {
+		if (!dev)
+			return 0;
+		pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
+		if (!pos)
+			return 0;
 
-	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI);
-	if (!pos)
-		return 0;
-	pci_read_config_word(dev, pos + 4, &cap);
-	next_fn = cap >> 8;
-	if (next_fn <= fn)
-		return 0;
-	return next_fn;
-}
+		pci_read_config_word(dev, pos + PCI_ARI_CAP, &cap);
+		next_fn = PCI_ARI_CAP_NFN(cap);
+		if (next_fn <= fn)
+			return 0;	/* protect against malformed list */
 
-static unsigned next_trad_fn(struct pci_dev *dev, unsigned fn)
-{
-	return (fn + 1) % 8;
-}
+		return next_fn;
+	}
 
-static unsigned no_next_fn(struct pci_dev *dev, unsigned fn)
-{
+	if (!dev || dev->multifunction)
+		return (fn + 1) % 8;
+
 	return 0;
 }
 
@@ -1406,7 +1405,6 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 {
 	unsigned fn, nr = 0;
 	struct pci_dev *dev;
-	unsigned (*next_fn)(struct pci_dev *, unsigned) = no_next_fn;
 
 	if (only_one_child(bus) && (devfn > 0))
 		return 0; /* Already scanned the entire slot */
@@ -1417,12 +1415,7 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
 	if (!dev->is_added)
 		nr++;
 
-	if (pci_ari_enabled(bus))
-		next_fn = next_ari_fn;
-	else if (dev->multifunction)
-		next_fn = next_trad_fn;
-
-	for (fn = next_fn(dev, 0); fn > 0; fn = next_fn(dev, fn)) {
+	for (fn = next_fn(bus, dev, 0); fn > 0; fn = next_fn(bus, dev, fn)) {
 		dev = pci_scan_single_device(bus, devfn + fn);
 		if (dev) {
 			if (!dev->is_added)
-- 
1.7.1

Re: [PATCH -v3 0/7] ARI device hotplug support

[Bjorn Helgaas]

On Fri, Jan 25, 2013 at 2:02 AM, Yijing Wang <wangyijing@huawei.com> wrote:
> On 2013/1/25 6:45, Bjorn Helgaas wrote:
> Hi Bjorn,
>    Thanks for your review and great work for this series patches!
>>
>> I applied this series on the pci/yijing-ari branch in my git tree,
>> with the following changes:
>>
>>   - Updated changelogs for readability
>>   - Reworked next_fn() and made it static
>>   - Updated the unconfigure/disable paths for cpcihp, sgihp, shpchp
>>   - Check PCI_SLOT for non-PCIe drivers in case a bus has several slots
>>   - Reset "Author:" to Yijing (since you wrote the original patches)
>>
>> Please review the changes I made and test the parts you can.  I need
>> your acknowledgement before putting these in "next" with  your
>> Signed-off-by because I changed them so much.
>
> I reviewed the changes and tested this series again in my hotplug machine.
> Most of the changes looks good to me except [PATCH 3/7] PCI: Consolidate "next-function" functions.
>
> Currently, if parameter "dev" is passed as NULL (pci_scan_single_device() return NULL), next_fn
> will return 0, so pci_scan_slot() will stop scanning rest function devices in this slot.
> According to PCI 3.0 Spec "Multi-function devices require to always implement function 0
> in the device. Implementing other functions is optional and maybe assigned in any order".
> So we will miss some function devices when scanning pci slot. I tested this patch in my machine and found it boot failed,
> because some usb devices can not found.

Oh, right.  Thanks for catching this!

> +static unsigned next_fn(struct pci_bus *bus, struct pci_dev *dev, unsigned fn)
>  {
> -       u16 cap;
> -       unsigned pos, next_fn;
> +       int pos;
> +       u16 cap = 0;
> +       unsigned next_fn;
>
>         if (!dev)
>                 return 0;
>
> lspci info:
> 00:16.0 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.1 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.2 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.3 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.4 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.5 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.6 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:16.7 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
> 00:1a.0 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4
> 00:1a.1 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5
> 00:1a.2 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6
> 00:1a.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2
>
> I fixed this problem with [PATCH 3/7] PCI: Consolidate "next-function" functions.
> And attach this refreshed patch at the end. This patch has been tested, and result is ok in my machine.

I replaced [3/7] with yours.

>> I think there are really two defects you're fixing here:
>>
>>   (1) If you hot-remove an ARI device and replace it with a non-ARI
>> multi-function device, we find only function 0 of the new device
>> because the upstream bridge still has ARI enabled, and next_ari_fn()
>> only returns function 0 for non-ARI devices.  Patch [1/7] fixes this.
>> I think this is the issue shown by your dmesg quotes above.
>>
>>   (2) If you hot-add an ARI device, the PCI core enumerates all the
>> functions, but pciehp only initializes functions 0-7, and other
>> functions don't work correctly.  Additionally, if you hot-remove the
>> device, pciehp only removes functions 0-7, leaving stale pci_dev
>> structures around.  Patch [4/7] fixes this.
>>
>> If my understanding is correct, I'll update the commit logs to mention
>> these scenarios explicitly.
>
> Yes, exactly.

OK, I made these tweaks and updated my pci/yijing-ari branch.

I'll merge that branch into "next" as soon as Jiang confirms that his
Signed-off-by is still valid, given that I made significant changes
since your first posting.

Bjorn

Re: [PATCH -v3 0/7] ARI device hotplug support

[Yijing Wang]

>>
>> Yes, exactly.
> 
> OK, I made these tweaks and updated my pci/yijing-ari branch.
> 
> I'll merge that branch into "next" as soon as Jiang confirms that his
> Signed-off-by is still valid, given that I made significant changes
> since your first posting.

Ok, thanks!

> 
> Bjorn
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH -v3 0/7] ARI device hotplug support

[Jiang Liu]

On 01/26/2013 12:33 AM, Bjorn Helgaas wrote:
> On Fri, Jan 25, 2013 at 2:02 AM, Yijing Wang <wangyijing@huawei.com> wrote:
>> On 2013/1/25 6:45, Bjorn Helgaas wrote:
>> Hi Bjorn,
>>    Thanks for your review and great work for this series patches!
>>>
>>> I applied this series on the pci/yijing-ari branch in my git tree,
>>> with the following changes:
>>>
>>>   - Updated changelogs for readability
>>>   - Reworked next_fn() and made it static
>>>   - Updated the unconfigure/disable paths for cpcihp, sgihp, shpchp
>>>   - Check PCI_SLOT for non-PCIe drivers in case a bus has several slots
>>>   - Reset "Author:" to Yijing (since you wrote the original patches)
>>>
>>> Please review the changes I made and test the parts you can.  I need
>>> your acknowledgement before putting these in "next" with  your
>>> Signed-off-by because I changed them so much.
>>
>> I reviewed the changes and tested this series again in my hotplug machine.
>> Most of the changes looks good to me except [PATCH 3/7] PCI: Consolidate "next-function" functions.
>>
>> Currently, if parameter "dev" is passed as NULL (pci_scan_single_device() return NULL), next_fn
>> will return 0, so pci_scan_slot() will stop scanning rest function devices in this slot.
>> According to PCI 3.0 Spec "Multi-function devices require to always implement function 0
>> in the device. Implementing other functions is optional and maybe assigned in any order".
>> So we will miss some function devices when scanning pci slot. I tested this patch in my machine and found it boot failed,
>> because some usb devices can not found.
> 
> Oh, right.  Thanks for catching this!
> 
>> +static unsigned next_fn(struct pci_bus *bus, struct pci_dev *dev, unsigned fn)
>>  {
>> -       u16 cap;
>> -       unsigned pos, next_fn;
>> +       int pos;
>> +       u16 cap = 0;
>> +       unsigned next_fn;
>>
>>         if (!dev)
>>                 return 0;
>>
>> lspci info:
>> 00:16.0 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.1 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.2 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.3 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.4 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.5 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.6 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:16.7 System peripheral: Intel Corporation 5520/5500/X58 Chipset QuickData Technology Device (rev 22)
>> 00:1a.0 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #4
>> 00:1a.1 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #5
>> 00:1a.2 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB UHCI Controller #6
>> 00:1a.7 USB Controller: Intel Corporation 82801JI (ICH10 Family) USB2 EHCI Controller #2
>>
>> I fixed this problem with [PATCH 3/7] PCI: Consolidate "next-function" functions.
>> And attach this refreshed patch at the end. This patch has been tested, and result is ok in my machine.
> 
> I replaced [3/7] with yours.
> 
>>> I think there are really two defects you're fixing here:
>>>
>>>   (1) If you hot-remove an ARI device and replace it with a non-ARI
>>> multi-function device, we find only function 0 of the new device
>>> because the upstream bridge still has ARI enabled, and next_ari_fn()
>>> only returns function 0 for non-ARI devices.  Patch [1/7] fixes this.
>>> I think this is the issue shown by your dmesg quotes above.
>>>
>>>   (2) If you hot-add an ARI device, the PCI core enumerates all the
>>> functions, but pciehp only initializes functions 0-7, and other
>>> functions don't work correctly.  Additionally, if you hot-remove the
>>> device, pciehp only removes functions 0-7, leaving stale pci_dev
>>> structures around.  Patch [4/7] fixes this.
>>>
>>> If my understanding is correct, I'll update the commit logs to mention
>>> these scenarios explicitly.
>>
>> Yes, exactly.
> 
> OK, I made these tweaks and updated my pci/yijing-ari branch.
> 
> I'll merge that branch into "next" as soon as Jiang confirms that his
> Signed-off-by is still valid, given that I made significant changes
> since your first posting.
Hi Bjorn,
	I'm OK with the new patch, thanks for your help to improve it.

> 
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>