Currently there's no mechanism to protect the global pci_root_buses list
from dynamic change at runtime. That means, PCI root bridge hotplug
operations, which dynamically change the pci_root_buses list, may cause
invalid memory accesses.
So introduce a global lock to serialize accesses to the pci_root_buses
list and serialize PCI host bridge hotplug operations.
Be careful, never try to acquire this global lock from PCI device drivers,
that may cause deadlocks.
Signed-off-by: Jiang Liu <liuj97@xxxxxxxxx>
---
drivers/acpi/pci_root.c | 8 +++++++-
drivers/edac/i7core_edac.c | 16 +++++++---------
drivers/gpu/drm/drm_fops.c | 6 +++++-
drivers/pci/host-bridge.c | 19 +++++++++++++++++++
drivers/pci/hotplug/sgi_hotplug.c | 2 ++
drivers/pci/pci-sysfs.c | 2 ++
drivers/pci/probe.c | 5 ++++-
drivers/pci/search.c | 9 ++++++++-
include/linux/pci.h | 8 ++++++++
9 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 7aff631..6bd0e32 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -463,6 +463,8 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
if (!root)
return -ENOMEM;
+ pci_host_bridge_hotplug_lock();
+
segment = 0;
status = acpi_evaluate_integer(device->handle, METHOD_NAME__SEG, NULL,
&segment);
@@ -516,7 +518,6 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
* TBD: Need PCI interface for enumeration/configuration of roots.
*/
- /* TBD: Locking */
list_add_tail(&root->node, &acpi_pci_roots);
printk(KERN_INFO PREFIX "%s [%s] (domain %04x %pR)\n",
@@ -622,11 +623,14 @@ static int __devinit acpi_pci_root_add(struct acpi_device *device)
if (device->wakeup.flags.run_wake)
device_set_run_wake(root->bus->bridge, true);
+ pci_host_bridge_hotplug_unlock();
+
return 0;
end:
if (!list_empty(&root->node))
list_del(&root->node);
+ pci_host_bridge_hotplug_unlock();
kfree(root);
return result;
}
@@ -643,8 +647,10 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
{
struct acpi_pci_root *root = acpi_driver_data(device);
+ pci_host_bridge_hotplug_lock();
device_set_run_wake(root->bus->bridge, false);
pci_acpi_remove_bus_pm_notifier(device);
+ pci_host_bridge_hotplug_unlock();
kfree(root);
return 0;
diff --git a/drivers/edac/i7core_edac.c b/drivers/edac/i7core_edac.c
index d27778f..8e6f177 100644
--- a/drivers/edac/i7core_edac.c
+++ b/drivers/edac/i7core_edac.c
@@ -1196,6 +1196,7 @@ static void __init i7core_xeon_pci_fixup(const struct pci_id_table *table)
* aren't announced by acpi. So, we need to use a legacy scan probing
* to detect them
*/
+ pci_host_bridge_hotplug_lock();
while (table && table->descr) {
pdev = pci_get_device(PCI_VENDOR_ID_INTEL, table->descr[0].dev_id, NULL);
if (unlikely(!pdev)) {
@@ -1205,19 +1206,16 @@ static void __init i7core_xeon_pci_fixup(const struct pci_id_table *table)
pci_dev_put(pdev);
table++;
}
+ pci_host_bridge_hotplug_unlock();
}
static unsigned i7core_pci_lastbus(void)
{
- int last_bus = 0, bus;
- struct pci_bus *b = NULL;
-
- while ((b = pci_find_next_bus(b)) != NULL) {
- bus = b->number;
- debugf0("Found bus %d\n", bus);
- if (bus > last_bus)
- last_bus = bus;
- }
+ int last_bus = 0;
+
+ for (last_bus = 255; last_bus >= 0; last_bus--)
+ if (pci_find_bus(0, last_bus))
+ break;
debugf0("Last bus %d\n", last_bus);
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 123de28..f559b5b 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -344,9 +344,13 @@ static int drm_open_helper(struct inode *inode, struct file *filp,
pci_dev_put(pci_dev);
}
if (!dev->hose) {
- struct pci_bus *b = pci_bus_b(pci_root_buses.next);
+ struct pci_bus *b;
+
+ pci_host_bridge_hotplug_lock();
+ b = pci_find_next_bus(NULL);
if (b)
dev->hose = b->sysdata;
+ pci_host_bridge_hotplug_unlock();
}
}
#endif
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index a68dc61..28d5557 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -94,3 +94,22 @@ void pcibios_bus_to_resource(struct pci_dev *dev, struct resource *res,
res->end = region->end + offset;
}
EXPORT_SYMBOL(pcibios_bus_to_resource);
+
+static DEFINE_MUTEX(pci_host_bridge_lock);
+
+/*
+ * Get the lock to serialize PCI host bridge hotplug operations.
+ * It can't be called from PCI device drivers, otherwise it may cause
+ * deadlocks when removing a host bridge.
+ */
+void pci_host_bridge_hotplug_lock(void)
+{
+ mutex_lock(&pci_host_bridge_lock);
+}
+EXPORT_SYMBOL(pci_host_bridge_hotplug_lock);
+
+void pci_host_bridge_hotplug_unlock(void)
+{
+ mutex_unlock(&pci_host_bridge_lock);
+}
+EXPORT_SYMBOL(pci_host_bridge_hotplug_unlock);
diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
index f64ca92..e0e5d13 100644
--- a/drivers/pci/hotplug/sgi_hotplug.c
+++ b/drivers/pci/hotplug/sgi_hotplug.c
@@ -690,6 +690,7 @@ static int __init sn_pci_hotplug_init(void)
INIT_LIST_HEAD(&sn_hp_list);
+ pci_host_bridge_hotplug_lock();
while ((pci_bus = pci_find_next_bus(pci_bus))) {
if (!pci_bus->sysdata)
continue;
@@ -709,6 +710,7 @@ static int __init sn_pci_hotplug_init(void)
break;
}
}
+ pci_host_bridge_hotplug_unlock();
return registered == 1 ? 0 : -ENODEV;
}
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 86c63fe..99fefbe 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -295,10 +295,12 @@ static ssize_t bus_rescan_store(struct bus_type *bus, const char *buf,
return -EINVAL;
if (val) {
+ pci_host_bridge_hotplug_lock();
mutex_lock(&pci_remove_rescan_mutex);
while ((b = pci_find_next_bus(b)) != NULL)
pci_rescan_bus(b);
mutex_unlock(&pci_remove_rescan_mutex);
+ pci_host_bridge_hotplug_unlock();
}
return count;
}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ad77ae5..1f64e8d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -23,7 +23,10 @@ struct resource busn_resource = {
.flags = IORESOURCE_BUS,
};
-/* Ugh. Need to stop exporting this to modules. */
+/*
+ * Ugh. Need to stop exporting this to modules.
+ * Protected by pci_host_bridge_hotplug_{lock|unlock}().
+ */
LIST_HEAD(pci_root_buses);
EXPORT_SYMBOL(pci_root_buses);
diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index 993d4a0..f1147a7 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -100,6 +100,13 @@ struct pci_bus * pci_find_bus(int domain, int busnr)
* initiated by passing %NULL as the @from argument. Otherwise if
* @from is not %NULL, searches continue from next device on the
* global list.
+ *
+ * Please don't call this function at rumtime if possible.
+ * It's designed to be called at boot time only because it's unsafe
+ * to PCI root bridge hotplug operations. But some drivers do invoke
+ * it at runtime and it's hard to fix those drivers. In such cases,
+ * use pci_host_bridge_hotplug()_{lock|unlock} to protect the PCI root
+ * bus list, but you need to be really careful to avoid deadlock.
*/
struct pci_bus *
pci_find_next_bus(const struct pci_bus *from)
@@ -115,6 +122,7 @@ pci_find_next_bus(const struct pci_bus *from)
up_read(&pci_bus_sem);
return b;
}
+EXPORT_SYMBOL(pci_find_next_bus);
/**
* pci_get_slot - locate PCI device for a given PCI slot
@@ -353,7 +361,6 @@ EXPORT_SYMBOL(pci_dev_present);
/* For boot time work */
EXPORT_SYMBOL(pci_find_bus);
-EXPORT_SYMBOL(pci_find_next_bus);
/* For everyone */
EXPORT_SYMBOL(pci_get_device);
EXPORT_SYMBOL(pci_get_subsys);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 21fa79e..e02f130 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -388,6 +388,8 @@ struct pci_host_bridge {
void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
void (*release_fn)(struct pci_host_bridge *),
void *release_data);
+void pci_host_bridge_hotplug_lock(void);
+void pci_host_bridge_hotplug_unlock(void);
/*
* The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
@@ -1359,6 +1361,12 @@ static inline void pci_unblock_cfg_access(struct pci_dev *dev)
static inline struct pci_bus *pci_find_next_bus(const struct pci_bus *from)
{ return NULL; }
+static inline void pci_host_bridge_hotplug_lock(void)
+{ }
+
+static inline void pci_host_bridge_hotplug_unlock(void)
+{ }
+
static inline struct pci_dev *pci_get_slot(struct pci_bus *bus,
unsigned int devfn)
{ return NULL; }
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
