mirrors/linux
1
0
Fork 1
mirror of https://github.com/torvalds/linux.git synced 2025-11-04 18:49:34 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

PCI: Avoid race while enabling upstream bridges

Browse source 
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:
Srinath Mannam 2017-08-18 21:50:48 -05:00 committed by Bjorn Helgaas
parent 62ce94a7a5
commit 40f11adc7c
1 changed files with 11 additions and 2 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

13
drivers/pci/pci.c
View file

@ -52,6 +52,7 @@ static void pci_pme_list_scan(struct work_struct *work);
static LIST_HEAD(pci_pme_list); static LIST_HEAD(pci_pme_list);
static DEFINE_MUTEX(pci_pme_list_mutex); static DEFINE_MUTEX(pci_pme_list_mutex);
static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan); static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan);
static DEFINE_MUTEX(pci_bridge_mutex);
struct pci_pme_device { struct pci_pme_device {
struct list_head list; struct list_head list;
@ -1348,10 +1349,16 @@ static void pci_enable_bridge(struct pci_dev *dev)
if (bridge) if (bridge)
pci_enable_bridge(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 (pci_is_enabled(dev)) {
if (!dev->is_busmaster) if (!dev->is_busmaster)
pci_set_master(dev); pci_set_master(dev);
return; goto end;
} }
retval = pci_enable_device(dev); 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", dev_err(&dev->dev, "Error enabling bridge (%d), continuing\n",
retval); retval);
pci_set_master(dev); pci_set_master(dev);
end:
mutex_unlock(&pci_bridge_mutex);
} }
static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags) 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 */ return 0; /* already enabled */
bridge = pci_upstream_bridge(dev); bridge = pci_upstream_bridge(dev);
if (bridge) if (bridge && !pci_is_enabled(bridge))
pci_enable_bridge(bridge); pci_enable_bridge(bridge);
/* only skip sriov related */ /* only skip sriov related */