Skip to content

Commit 5e70d0a

Browse files
ij-intelbjorn-helgaas
authored andcommitted
PCI: Add locking to RMW PCI Express Capability Register accessors
Many places in the kernel write the Link Control and Root Control PCI Express Capability Registers without proper concurrency control and this could result in losing the changes one of the writers intended to make. Add pcie_cap_lock spinlock into the struct pci_dev and use it to protect bit changes made in the RMW capability accessors. Protect only a selected set of registers by differentiating the RMW accessor internally to locked/unlocked variants using a wrapper which has the same signature as pcie_capability_clear_and_set_word(). As the Capability Register (pos) given to the wrapper is always a constant, the compiler should be able to simplify all the dead-code away. So far only the Link Control Register (ASPM, hotplug, link retraining, various drivers) and the Root Control Register (AER & PME) seem to require RMW locking. Suggested-by: Lukas Wunner <lukas@wunner.de> Fixes: c7f4865 ("PCI PM: PCIe PME root port service driver") Fixes: f12eb72 ("PCI/ASPM: Use PCI Express Capability accessors") Fixes: 7d715a6 ("PCI: add PCI Express ASPM support") Fixes: affa48d ("staging/rdma/hfi1: Add support for enabling/disabling PCIe ASPM") Fixes: 849a936 ("misc: rtsx: Add support new chip rts5228 mmc: rtsx: Add support MMC_CAP2_NO_MMC") Fixes: 3d1e7aa ("misc: rtsx: Use pcie_capability_clear_and_set_word() for PCI_EXP_LNKCTL") Fixes: c0e5f4e ("misc: rtsx: Add support for RTS5261") Fixes: 3df4fce ("misc: rtsx: separate aspm mode into MODE_REG and MODE_CFG") Fixes: 121e9c6 ("misc: rtsx: modify and fix init_hw function") Fixes: 19f3bd5 ("mfd: rtsx: Remove LCTLR defination") Fixes: 773ccdf ("mfd: rtsx: Read vendor setting from config space") Fixes: 8275b77 ("mfd: rts5249: Add support for RTS5250S power saving") Fixes: 5da4e04 ("misc: rtsx: Add support for RTS5260") Fixes: 0f49bfb ("tg3: Use PCI Express Capability accessors") Fixes: 5e7dfd0 ("tg3: Prevent corruption at 10 / 100Mbps w CLKREQ") Fixes: b726e49 ("r8169: sync existing 8168 device hardware start sequences with vendor driver") Fixes: e6de30d ("r8169: more 8168dp support.") Fixes: 8a06127 ("Bluetooth: hci_bcm4377: Add new driver for BCM4377 PCIe boards") Fixes: 6f461f6 ("e1000e: enable/disable ASPM L0s and L1 and ERT according to hardware errata") Fixes: 1eae4eb ("e1000e: Disable L1 ASPM power savings for 82573 mobile variants") Fixes: 8060e16 ("ath9k: Enable extended synch for AR9485 to fix L0s recovery issue") Fixes: 69ce674 ("ath9k: do btcoex ASPM disabling at initialization time") Fixes: f37f055 ("mt76: mt76x2e: disable pcie_aspm by default") Link: https://lore.kernel.org/r/20230717120503.15276-2-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: "Rafael J. Wysocki" <rafael@kernel.org>
1 parent 06c2afb commit 5e70d0a

3 files changed

Lines changed: 50 additions & 5 deletions

File tree

drivers/pci/access.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,8 @@ int pcie_capability_write_dword(struct pci_dev *dev, int pos, u32 val)
497497
}
498498
EXPORT_SYMBOL(pcie_capability_write_dword);
499499

500-
int pcie_capability_clear_and_set_word(struct pci_dev *dev, int pos,
501-
u16 clear, u16 set)
500+
int pcie_capability_clear_and_set_word_unlocked(struct pci_dev *dev, int pos,
501+
u16 clear, u16 set)
502502
{
503503
int ret;
504504
u16 val;
@@ -512,7 +512,21 @@ int pcie_capability_clear_and_set_word(struct pci_dev *dev, int pos,
512512

513513
return ret;
514514
}
515-
EXPORT_SYMBOL(pcie_capability_clear_and_set_word);
515+
EXPORT_SYMBOL(pcie_capability_clear_and_set_word_unlocked);
516+
517+
int pcie_capability_clear_and_set_word_locked(struct pci_dev *dev, int pos,
518+
u16 clear, u16 set)
519+
{
520+
unsigned long flags;
521+
int ret;
522+
523+
spin_lock_irqsave(&dev->pcie_cap_lock, flags);
524+
ret = pcie_capability_clear_and_set_word_unlocked(dev, pos, clear, set);
525+
spin_unlock_irqrestore(&dev->pcie_cap_lock, flags);
526+
527+
return ret;
528+
}
529+
EXPORT_SYMBOL(pcie_capability_clear_and_set_word_locked);
516530

517531
int pcie_capability_clear_and_set_dword(struct pci_dev *dev, int pos,
518532
u32 clear, u32 set)

drivers/pci/probe.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2324,6 +2324,7 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
23242324
.end = -1,
23252325
};
23262326

2327+
spin_lock_init(&dev->pcie_cap_lock);
23272328
#ifdef CONFIG_PCI_MSI
23282329
raw_spin_lock_init(&dev->msi_lock);
23292330
#endif

include/linux/pci.h

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ struct pci_dev {
467467
pci_dev_flags_t dev_flags;
468468
atomic_t enable_cnt; /* pci_enable_device has been called */
469469

470+
spinlock_t pcie_cap_lock; /* Protects RMW ops in capability accessors */
470471
u32 saved_config_space[16]; /* Config space saved at suspend time */
471472
struct hlist_head saved_cap_space;
472473
int rom_attr_enabled; /* Display of ROM attribute enabled? */
@@ -1217,11 +1218,40 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val);
12171218
int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val);
12181219
int pcie_capability_write_word(struct pci_dev *dev, int pos, u16 val);
12191220
int pcie_capability_write_dword(struct pci_dev *dev, int pos, u32 val);
1220-
int pcie_capability_clear_and_set_word(struct pci_dev *dev, int pos,
1221-
u16 clear, u16 set);
1221+
int pcie_capability_clear_and_set_word_unlocked(struct pci_dev *dev, int pos,
1222+
u16 clear, u16 set);
1223+
int pcie_capability_clear_and_set_word_locked(struct pci_dev *dev, int pos,
1224+
u16 clear, u16 set);
12221225
int pcie_capability_clear_and_set_dword(struct pci_dev *dev, int pos,
12231226
u32 clear, u32 set);
12241227

1228+
/**
1229+
* pcie_capability_clear_and_set_word - RMW accessor for PCI Express Capability Registers
1230+
* @dev: PCI device structure of the PCI Express device
1231+
* @pos: PCI Express Capability Register
1232+
* @clear: Clear bitmask
1233+
* @set: Set bitmask
1234+
*
1235+
* Perform a Read-Modify-Write (RMW) operation using @clear and @set
1236+
* bitmasks on PCI Express Capability Register at @pos. Certain PCI Express
1237+
* Capability Registers are accessed concurrently in RMW fashion, hence
1238+
* require locking which is handled transparently to the caller.
1239+
*/
1240+
static inline int pcie_capability_clear_and_set_word(struct pci_dev *dev,
1241+
int pos,
1242+
u16 clear, u16 set)
1243+
{
1244+
switch (pos) {
1245+
case PCI_EXP_LNKCTL:
1246+
case PCI_EXP_RTCTL:
1247+
return pcie_capability_clear_and_set_word_locked(dev, pos,
1248+
clear, set);
1249+
default:
1250+
return pcie_capability_clear_and_set_word_unlocked(dev, pos,
1251+
clear, set);
1252+
}
1253+
}
1254+
12251255
static inline int pcie_capability_set_word(struct pci_dev *dev, int pos,
12261256
u16 set)
12271257
{

0 commit comments

Comments
 (0)