* [PATCH -v3 1/7] PCI: rework pci_enable_ari to support disable ari forwarding
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 2/7] PCI: Rename pci_enable_ari to pci_configure_ari Yijing Wang
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 2/7] PCI: Rename pci_enable_ari to pci_configure_ari
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 1/7] PCI: rework pci_enable_ari to support disable ari forwarding Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 3/7] PCI: introduce pci_next_fn to simplify code Yijing Wang
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 3/7] PCI: introduce pci_next_fn to simplify code
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 1/7] PCI: rework pci_enable_ari to support disable ari forwarding Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 2/7] PCI: Rename pci_enable_ari to pci_configure_ari Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 4/7] PCI,pciehp: use bus->devices list intead of traditional traversal Yijing Wang
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 4/7] PCI,pciehp: use bus->devices list intead of traditional traversal
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
` (2 preceding siblings ...)
2013-01-15 3:12 ` [PATCH -v3 3/7] PCI: introduce pci_next_fn to simplify code Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 5/7] PCI,cpcihp: use bus->devices list instead " Yijing Wang
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 5/7] PCI,cpcihp: use bus->devices list instead of traditional traversal
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
` (3 preceding siblings ...)
2013-01-15 3:12 ` [PATCH -v3 4/7] PCI,pciehp: use bus->devices list intead of traditional traversal Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 6/7] PCI,sgihp: use bus->devices list intead " Yijing Wang
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 6/7] PCI,sgihp: use bus->devices list intead of traditional traversal
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
` (4 preceding siblings ...)
2013-01-15 3:12 ` [PATCH -v3 5/7] PCI,cpcihp: use bus->devices list instead " Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-15 3:12 ` [PATCH -v3 7/7] PCI,shpchp: use bus->devices list instead " Yijing Wang
2013-01-24 22:45 ` [PATCH -v3 0/7] ARI device hotplug support Bjorn Helgaas
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH -v3 7/7] PCI,shpchp: use bus->devices list instead of traditional traversal
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
` (5 preceding siblings ...)
2013-01-15 3:12 ` [PATCH -v3 6/7] PCI,sgihp: use bus->devices list intead " Yijing Wang
@ 2013-01-15 3:12 ` Yijing Wang
2013-01-24 22:45 ` [PATCH -v3 0/7] ARI device hotplug support Bjorn Helgaas
7 siblings, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-15 3:12 UTC (permalink / raw)
To: Bjorn Helgaas, Yu Zhao
Cc: Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci, Hanjun Guo,
jiang.liu, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH -v3 0/7] ARI device hotplug support
2013-01-15 3:12 [PATCH -v3 0/7] ARI device hotplug support Yijing Wang
` (6 preceding siblings ...)
2013-01-15 3:12 ` [PATCH -v3 7/7] PCI,shpchp: use bus->devices list instead " Yijing Wang
@ 2013-01-24 22:45 ` Bjorn Helgaas
2013-01-25 9:02 ` Yijing Wang
7 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2013-01-24 22:45 UTC (permalink / raw)
To: Yijing Wang
Cc: Yu Zhao, Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci,
Hanjun Guo, jiang.liu, Jack Steiner
[+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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH -v3 0/7] ARI device hotplug support
2013-01-24 22:45 ` [PATCH -v3 0/7] ARI device hotplug support Bjorn Helgaas
@ 2013-01-25 9:02 ` Yijing Wang
2013-01-25 16:33 ` Bjorn Helgaas
0 siblings, 1 reply; 13+ messages in thread
From: Yijing Wang @ 2013-01-25 9:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yu Zhao, Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci,
Hanjun Guo, jiang.liu, Jack Steiner
[-- 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH -v3 0/7] ARI device hotplug support
2013-01-25 9:02 ` Yijing Wang
@ 2013-01-25 16:33 ` Bjorn Helgaas
2013-01-26 1:00 ` Yijing Wang
2013-01-26 17:18 ` Jiang Liu
0 siblings, 2 replies; 13+ messages in thread
From: Bjorn Helgaas @ 2013-01-25 16:33 UTC (permalink / raw)
To: Yijing Wang
Cc: Yu Zhao, Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci,
Hanjun Guo, jiang.liu
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH -v3 0/7] ARI device hotplug support
2013-01-25 16:33 ` Bjorn Helgaas
@ 2013-01-26 1:00 ` Yijing Wang
2013-01-26 17:18 ` Jiang Liu
1 sibling, 0 replies; 13+ messages in thread
From: Yijing Wang @ 2013-01-26 1:00 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yu Zhao, Kenji Kaneshige, Scott Murray, Yinghai Lu, linux-pci,
Hanjun Guo, jiang.liu
>>
>> 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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH -v3 0/7] ARI device hotplug support
2013-01-25 16:33 ` Bjorn Helgaas
2013-01-26 1:00 ` Yijing Wang
@ 2013-01-26 17:18 ` Jiang Liu
1 sibling, 0 replies; 13+ messages in thread
From: Jiang Liu @ 2013-01-26 17:18 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yijing Wang, Yu Zhao, Kenji Kaneshige, Scott Murray, Yinghai Lu,
linux-pci, Hanjun Guo, 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
>
^ permalink raw reply [flat|nested] 13+ messages in thread