PCI: Avoid race while enabling upstream bridges
When we enable a device, we first enable any upstream bridges. If a bridge has multiple downstream devices and we enable them simultaneously, the race to enable the upstream bridge may cause problems. Consider this hierarchy: bridge A --+-- device B +-- device C If drivers for B and C call pci_enable_device() simultaneously, both will attempt to enable A, which involves setting PCI_COMMAND_MASTER via pci_set_master() and PCI_COMMAND_MEMORY via pci_enable_resources(). In the following sequence, B's update to set A's PCI_COMMAND_MEMORY is lost, and neither B nor C will work correctly: B C pci_set_master(A) cmd = read(A, PCI_COMMAND) cmd |= PCI_COMMAND_MASTER pci_set_master(A) cmd = read(A, PCI_COMMAND) cmd |= PCI_COMMAND_MASTER write(A, PCI_COMMAND, cmd) pci_enable_device(A) pci_enable_resources(A) cmd = read(A, PCI_COMMAND) cmd |= PCI_COMMAND_MEMORY write(A, PCI_COMMAND, cmd) write(A, PCI_COMMAND, cmd) Avoid this race by holding a new pci_bridge_mutex while enabling a bridge. This ensures that both PCI_COMMAND_MASTER and PCI_COMMAND_MEMORY will be updated before another thread can start enabling the bridge. Note that although pci_enable_bridge() is recursive, it enables any upstream bridges *before* acquiring the mutex. When it acquires the mutex and calls pci_set_master() and pci_enable_device(), any upstream bridges have already been enabled so pci_enable_device() will not deadlock by calling pci_enable_bridge() again. Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> [bhelgaas: changelog, comment] Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
This commit is contained in:
parent
62ce94a7a5
commit
40f11adc7c
1 changed files with 11 additions and 2 deletions
|
@ -52,6 +52,7 @@ static void pci_pme_list_scan(struct work_struct *work);
|
|||
static LIST_HEAD(pci_pme_list);
|
||||
static DEFINE_MUTEX(pci_pme_list_mutex);
|
||||
static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan);
|
||||
static DEFINE_MUTEX(pci_bridge_mutex);
|
||||
|
||||
struct pci_pme_device {
|
||||
struct list_head list;
|
||||
|
@ -1348,10 +1349,16 @@ static void pci_enable_bridge(struct pci_dev *dev)
|
|||
if (bridge)
|
||||
pci_enable_bridge(bridge);
|
||||
|
||||
/*
|
||||
* Hold pci_bridge_mutex to prevent a race when enabling two
|
||||
* devices below the bridge simultaneously. The race may cause a
|
||||
* PCI_COMMAND_MEMORY update to be lost (see changelog).
|
||||
*/
|
||||
mutex_lock(&pci_bridge_mutex);
|
||||
if (pci_is_enabled(dev)) {
|
||||
if (!dev->is_busmaster)
|
||||
pci_set_master(dev);
|
||||
return;
|
||||
goto end;
|
||||
}
|
||||
|
||||
retval = pci_enable_device(dev);
|
||||
|
@ -1359,6 +1366,8 @@ static void pci_enable_bridge(struct pci_dev *dev)
|
|||
dev_err(&dev->dev, "Error enabling bridge (%d), continuing\n",
|
||||
retval);
|
||||
pci_set_master(dev);
|
||||
end:
|
||||
mutex_unlock(&pci_bridge_mutex);
|
||||
}
|
||||
|
||||
static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
|
||||
|
@ -1383,7 +1392,7 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
|
|||
return 0; /* already enabled */
|
||||
|
||||
bridge = pci_upstream_bridge(dev);
|
||||
if (bridge)
|
||||
if (bridge && !pci_is_enabled(bridge))
|
||||
pci_enable_bridge(bridge);
|
||||
|
||||
/* only skip sriov related */
|
||||
|
|
Loading…
Reference in a new issue