[PATCH] PCI/PM: Serialize PMCSR updates against PME handling

[Giuseppe Piscitelli <ooonea@gmail.com>]

PME handling and native power transitions independently read and update
PMCSR. Config access locking protects individual transactions, so a PME
worker can read D0, another CPU can request D3hot, and the worker can write
its stale D0 state back while acknowledging PME status. This was observed
during normal runtime PM of an NVIDIA UCSI controller, followed by a
D0-to-D3hot refusal message.

Serialize native PCI-core PMCSR writers with the existing capability
lock, releasing it before transition delays and readiness waits.
State-only updates write zero to the RW1C PME_Status bit to preserve
pending wakeups. Read fresh PMCSR for each PM-reset transition instead of
keeping a stale word across the cooldown. Keep the literal-zero power-up
write.

Fixes: 58ff463396ad ("PCI PM: Add function for checking PME status of devices")
Signed-off-by: Giuseppe Piscitelli <ooonea@gmail.com>
---
OpenAI Codex analyzed the passive trace and wrote this fix and its local
tests without changing the running system.

Earlier validation used a source-derived RW1C model and four KUnit tests
on Linux 7.2.11 in QEMU with lockdep. The tests use a fake PCI bus; no
physical hardware, PREEMPT_RT or other architectures were runtime-tested.
This base was checked for applicability; no new build or runtime test
was run.

 drivers/pci/pci.c | 44 +++++++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index d79d2ee..3628886 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1315,6 +1315,22 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
 	return 0;
 }
 
+static int pci_pm_set_state(struct pci_dev *dev, pci_power_t state)
+{
+	u16 pmcsr;
+
+	guard(spinlock_irqsave)(&dev->pcie_cap_lock);
+	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
+	if (PCI_POSSIBLE_ERROR(pmcsr))
+		return -EIO;
+
+	/* PME_Status is RW1C; leave pending wakeups for PME handling. */
+	pmcsr &= ~(PCI_PM_CTRL_STATE_MASK | PCI_PM_CTRL_PME_STATUS);
+	pmcsr |= state;
+	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
+	return 0;
+}
+
 /**
  * pci_power_up - Put the given device into D0
  * @dev: PCI device to power up
@@ -1380,7 +1396,8 @@ int pci_power_up(struct pci_dev *dev)
 	 * Force the entire word to 0. This doesn't affect PME_Status, disables
 	 * PME_En, and sets PowerState to 0.
 	 */
-	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, 0);
+	scoped_guard(spinlock_irqsave, &dev->pcie_cap_lock)
+		pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, 0);
 
 	/* Mandatory transition delays; see PCI PM 1.2. */
 	if (state == PCI_D3hot) {
@@ -1537,8 +1554,7 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state, bool
 	if (dev->current_state == state)
 		return 0;
 
-	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
-	if (PCI_POSSIBLE_ERROR(pmcsr)) {
+	if (pci_pm_set_state(dev, state)) {
 		pci_err(dev, "Unable to change power state from %s to %s, device inaccessible\n",
 			pci_power_name(dev->current_state),
 			pci_power_name(state));
@@ -1546,12 +1562,6 @@ static int pci_set_low_power_state(struct pci_dev *dev, pci_power_t state, bool
 		return -EIO;
 	}
 
-	pmcsr &= ~PCI_PM_CTRL_STATE_MASK;
-	pmcsr |= state;
-
-	/* Enter specified state */
-	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, pmcsr);
-
 	/* Mandatory power management transition delays; see PCI PM 1.2. */
 	if (state == PCI_D3hot)
 		pci_dev_d3_sleep(dev);
@@ -2311,6 +2321,7 @@ bool pci_check_pme_status(struct pci_dev *dev)
 	if (!dev->pm_cap)
 		return false;
 
+	guard(spinlock_irqsave)(&dev->pcie_cap_lock);
 	pmcsr_pos = dev->pm_cap + PCI_PM_CTRL;
 	pci_read_config_word(dev, pmcsr_pos, &pmcsr);
 	if (!(pmcsr & PCI_PM_CTRL_PME_STATUS))
@@ -2433,6 +2444,7 @@ static void __pci_pme_active(struct pci_dev *dev, bool enable)
 	if (!dev->pme_support)
 		return;
 
+	guard(spinlock_irqsave)(&dev->pcie_cap_lock);
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	/* Clear PME_Status by writing 1 to it and enable PME# */
 	pmcsr |= PCI_PM_CTRL_PME_STATUS | PCI_PM_CTRL_PME_ENABLE;
@@ -2453,6 +2465,7 @@ void pci_pme_restore(struct pci_dev *dev)
 	if (!dev->pme_support)
 		return;
 
+	guard(spinlock_irqsave)(&dev->pcie_cap_lock);
 	pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr);
 	if (dev->wakeup_prepared) {
 		pmcsr |= PCI_PM_CTRL_PME_ENABLE;
@@ -4564,17 +4577,18 @@ static int pci_pm_reset(struct pci_dev *dev, bool probe)
 		return ret;
 	}
 
-	csr &= ~PCI_PM_CTRL_STATE_MASK;
-	csr |= PCI_D3hot;
-	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, csr);
+	ret = pci_pm_set_state(dev, PCI_D3hot);
+	if (ret)
+		goto out_iommu;
 	pci_dev_d3_sleep(dev);
 
-	csr &= ~PCI_PM_CTRL_STATE_MASK;
-	csr |= PCI_D0;
-	pci_write_config_word(dev, dev->pm_cap + PCI_PM_CTRL, csr);
+	ret = pci_pm_set_state(dev, PCI_D0);
+	if (ret)
+		goto out_iommu;
 	pci_dev_d3_sleep(dev);
 
 	ret = pci_dev_wait(dev, "PM D3hot->D0", PCIE_RESET_READY_POLL_MS);
+out_iommu:
 	pci_dev_reset_iommu_done(dev);
 	return ret;
 }
base-commit: f4319f548f7d8cb595ac1c91bfd459876399eeb3
-- 
2.54.0