[PATCH AUTOSEL 6.18-5.10] PCI: Stop setting cached power state to 'unknown' on unbind

[Sasha Levin]

From: Lukas Wunner <lukas@xxxxxxxxx>

[ Upstream commit d462c8e89e84bfb6417e6b4c88e0cb7cc747ba41 ]

When a PCI device is unbound from its driver, pci_device_remove() sets the
cached power state in pci_dev->current_state to PCI_UNKNOWN.  This was
introduced by commit 2449e06a5696 ("PCI: reset pci device state to unknown
state for resume") to invalidate the cached power state in case the system
is subsequently put to sleep.

For bound devices, the cached power state is set to PCI_UNKNOWN in
pci_pm_suspend_noirq(), immediately before entering system sleep.

Extend to unbound devices for consistency.

This obviates the need to change the cached power state on unbind, so stop
doing so.

Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Reviewed-by: Mario Limonciello (AMD) <superm1@xxxxxxxxxx>
Link: https://patch.msgid.link/af7d11d3ceb231acc90829f7a5c8400c2446744f.1776415510.git.lukas@xxxxxxxxx
Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[PCI]` `[Stop]` — Stop setting cached power state to
`PCI_UNKNOWN` on driver unbind; move invalidation to the system-suspend
path for driverless devices.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Lukas Wunner `<lukas@xxxxxxxxx>` (author)
- **Signed-off-by:** Bjorn Helgaas `<bhelgaas@xxxxxxxxxx>` (PCI
  maintainer)
- **Reviewed-by:** Mario Limonciello (AMD) `<superm1@xxxxxxxxxx>`
- **Link:** https://patch.msgid.link/af7d11d3ceb231acc90829f7a5c8400c244
  6744f.1776415510.git.lukas@xxxxxxxxx
- **No** Fixes:, Reported-by:, Tested-by:, Cc: stable@xxxxxxxxxxxxxxx,
  or syzbot tags
- Notable: PCI subsystem maintainer sign-off; AMD reviewer found no
  corner cases

### Step 1.3: Body Analysis
**Record:**
- **Bug described:** On driver unbind, `pci_device_remove()` sets
  `pci_dev->current_state = PCI_UNKNOWN` while hardware may still be in
  D0. Bound devices only invalidate the cache in
  `pci_pm_suspend_noirq()` right before sleep. Driverless/unbound
  devices on the `!pm` suspend path skipped that invalidation.
- **Symptom/failure mode:** Stale cached power state after
  suspend/resume can cause `pci_set_power_state(dev, PCI_D0)` to return
  early (believing the device is already in D0) when hardware is
  actually in D3 — the same class of failure as bugzilla #6024 fixed by
  commit 2449e06a5696.
- **Version info:** References 2449e06a5696 (2006); commit d462c8e89e84
  landed in mainline April 2026.
- **Root cause:** PCI_UNKNOWN invalidation was done at unbind time (too
  early) and was missing from the `!pm` branch of
  `pci_pm_suspend_noirq()`.

### Step 1.4: Hidden Bug Fix?
**Record:** Yes. Described as “consistency,” but it closes a real PM
correctness gap: driverless PCI devices on S2RAM suspend never had their
cached state invalidated, and prematurely setting UNKNOWN at unbind left
cache ≠ hardware between unbind and suspend.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `drivers/pci/pci-driver.c` (+2 / −8 lines)
- **Functions:** `pci_device_remove()`, `pci_pm_suspend_noirq()`
- **Scope:** Single-file surgical fix

### Step 2.2: Code Flow Changes
**Record:**
- **Hunk 1 (`pci_device_remove`):** Before — set `PCI_UNKNOWN` on unbind
  if cached state was D0. After — remove that; cached state stays
  accurate while hardware is still D0.
- **Hunk 2 (`pci_pm_suspend_noirq`):** Before — `!pm` path
  (`pci_save_state` then `goto Fixup`) skipped
  `pci_pm_set_unknown_state()`. After — `goto set_unknown` ensures
  driverless devices also invalidate cache immediately before sleep,
  same as bound devices.

### Step 2.3: Bug Mechanism
**Record:** **Logic / correctness fix (PCI PM cache coherency).**
`pci_set_power_state()` early-returns when `dev->current_state == state`
(see `__pci_set_power_state()` at line 1545 in `pci.c`). Stale D0 after
BIOS changes hardware to D3 during suspend prevents powering the device
back up on driver bind — identical mechanism to bugzilla #6024.

### Step 2.4: Fix Quality
**Record:** Minimal, obviously correct, mirrors existing bound-device
behavior. Low regression risk: only moves UNKNOWN invalidation from
unbind to suspend_noirq; hibernate path (`pci_pm_freeze_noirq`) already
called `pci_pm_set_unknown_state()` for all devices.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Unbind-time `PCI_UNKNOWN` logic traces to 2449e06a5696
(“PCI: reset pci device state to unknown state for resume”, 2006). That
commit is present in this tree. `pci_pm_set_unknown_state()` exists at
line 606 in current `pci-driver.c`.

### Step 3.2: Fixes: Tag
**Record:** N/A — no Fixes: tag. Referenced commit 2449e06a5696 is in
this tree and introduced the unbind-time behavior this patch refines.

### Step 3.3: Related Changes
**Record:** Related recent fix: 382895a288515 “PCI/PM: Reinstate
clearing state_saved in legacy and !PM codepaths” (already in 6.18.43).
No patch-series dependency; standalone 1/1 commit (d462c8e89e84).

### Step 3.4: Author Context
**Record:** Lukas Wunner is a regular PCI/PM contributor. Bjorn Helgaas
(PCI maintainer) applied and signed off. No other related commits from
this author in the immediate `pci-driver.c` history of this tree.

### Step 3.5: Dependencies
**Record:** No prerequisites. Requires only existing
`pci_pm_set_unknown_state()` and `pci_pm_suspend_noirq()` `!pm` path —
all present in Linux 6.18.43. Patch applies cleanly (`git apply --check`
passed).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** b4 dig found thread at https://patch.msgid.link/af7d11d3ceb2
31acc90829f7a5c8400c2446744f.1776415510.git.lukas@xxxxxxxxx. Single v1
patch; Bjorn applied to `pci/pm`; Mario Limonciello Reviewed-by with no
corner cases found. No stable nomination or NAKs.

### Step 4.2: Reviewers
**Record:** b4 dig -w: To/Cc included Bjorn Helgaas, Rafael Wysocki,
Mario Limonciello, Alex Williamson, linux-pci@xxxxxxxxxxxxxxx.

### Step 4.3: Bug Reports
**Record:** No new bug report. Commit references historical bugzilla
#6024 class via 2449e06a5696. No syzbot link.

### Step 4.4: Series Context
**Record:** Standalone patch, not part of a multi-patch series.

### Step 4.5: Stable List History
**Record:** lore.kernel.org blocked by bot protection; no stable-list
discussion found via b4 mbox thread.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `pci_device_remove()`, `pci_pm_suspend_noirq()`,
`pci_pm_set_unknown_state()`, `pci_set_power_state()` /
`__pci_set_power_state()`.

### Step 5.2: Callers
**Record:** `pci_device_remove` — PCI bus `.remove` callback (sysfs
unbind, module unload). `pci_pm_suspend_noirq` — PCI bus
`.suspend_noirq` for every PCI device during system suspend.

### Step 5.3: Callees
**Record:** `pci_save_state()`, `pci_pm_set_unknown_state()`,
`pci_fixup_device()`, `pci_prepare_to_sleep()` (bound path only).

### Step 5.4: Reachability
**Record:** Any PCI device without a bound driver (or driver without PM
ops) going through system suspend hits the `!pm` path. Users can trigger
via S3/S2RAM; driver bind after resume via `modprobe` or sysfs is
common. Reachable without privileges for suspend; driver bind typically
requires root.

### Step 5.5: Similar Patterns
**Record:** `pci_pm_freeze_noirq()` already calls
`pci_pm_set_unknown_state()` unconditionally (line 1098). Suspend path
was inconsistent for driverless devices.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** **Yes.** Tree is **Linux 6.18.43** (`git describe HEAD` →
v6.18.43). Buggy code confirmed at lines 497–498 (unbind UNKNOWN) and
line 874 (`goto Fixup` skipping `pci_pm_set_unknown_state` for `!pm`).

### Step 6.2: Backport Complications
**Record:** Patch applies cleanly with no modifications. No significant
refactoring conflicts in this area of 6.18.43.

### Step 6.3: Related Fixes Already Present?
**Record:** No equivalent fix in this tree. Commit d462c8e89e84 is not
in 6.18.43.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem / Criticality
**Record:** **drivers/pci** — CORE subsystem. PCI PM affects virtually
all systems.

### Step 7.2: Activity
**Record:** Actively maintained; recent PM fix 382895a288515 already
backported to this tree.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Systems with PCI devices that are driverless at suspend time
(never probed, intentionally unbound, or drivers without PM ops) that
later get a driver bound after resume.

### Step 8.2: Trigger Conditions
**Record:** System suspend (S2RAM) → resume → driver bind. Uncommon but
realistic for manual sysfs bind/unbind workflows and devices without in-
tree drivers. Not security-relevant; requires suspend cycle.

### Step 8.3: Failure Mode Severity
**Record:** Device fails to power up / probe after resume (stale D0
cache). **Severity: MEDIUM-HIGH** for affected users (hardware non-
functional), but not a kernel crash, deadlock, or data corruption.

### Step 8.4: Risk-Benefit
**Record:** **Benefit:** MEDIUM — fixes latent PM bug for driverless
devices; improves cache accuracy between unbind and suspend. **Risk:**
VERY LOW — 10-line change, maintainer-reviewed, net deletion of code.
**Ratio:** Favorable.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Fixes real PCI PM cache bug for driverless devices on S2RAM suspend
  (same failure class as bugzilla #6024)
- Stops incorrect premature UNKNOWN at unbind while hardware is still D0
- Tiny, surgical, PCI maintainer + AMD reviewed
- Applies cleanly to 6.18.43
- Aligns suspend path with existing hibernate path behavior

**AGAINST backport:**
- No explicit user bug report or syzbot hit
- Failure is device malfunction, not kernel oops/panic
- Described primarily as “consistency” in commit message
- No stable nomination in review thread

**Unresolved:** None material to the decision.

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — minimal diff, maintainer-
   reviewed, corner-case review done
2. Fixes a real bug affecting users? **PASS** — stale power-state cache
   prevents device power-up after suspend+bind
3. Important issue? **PASS (moderate)** — device non-functionality after
   suspend, same class as historical PCI PM bug
4. Small and contained? **PASS** — 1 file, 10 lines
5. No new features or APIs? **PASS**
6. Can apply to local tree? **PASS** — verified clean apply

### Step 9.3: Exception Categories
**Record:** None (not a quirk, device ID, DT, or build fix).

### Step 9.4: Problem Summary for Stable Users

This commit fixes a gap in PCI core suspend handling: devices without a
driver (or without PM callbacks) never had their cached power state
invalidated before sleep, while bound devices did. After suspend/resume,
if the BIOS changes the hardware power state, the kernel can believe the
device is still in D0 and skip the transition back to D0 when a driver
is later bound — leaving hardware dead. The patch also stops marking the
cache as UNKNOWN at unbind time when hardware is still in D0, deferring
invalidation to `pci_pm_suspend_noirq()` where bound devices already do
it.

For Linux 6.18.43, the buggy code is present, prerequisites exist, and
the patch applies cleanly with very low risk.

---

## Verification

- [Phase 1] Parsed subject, tags, body from provided commit message and
  kernel.org (d462c8e89e84)
- [Phase 1] Confirmed no Reported-by/syzbot/Fixes:/Cc: stable tags
- [Phase 2] Diff analysis: 2 insertions, 8 deletions in `pci-driver.c`;
  two functions modified
- [Phase 2] Read `__pci_set_power_state()` early-return at line 1545 in
  `pci.c`
- [Phase 3] `git describe HEAD` → v6.18.43; Makefile VERSION 6.18.43
- [Phase 3] `git show 2449e06a5696` — original unbind UNKNOWN logic and
  bugzilla #6024 context
- [Phase 3] `git blame` lines 493–498 — unbind UNKNOWN code present
- [Phase 3] Confirmed `pci_pm_set_unknown_state()` at line 606 in local
  tree
- [Phase 4] `b4 dig -c d462c8e89e84` — found lore thread URL
- [Phase 4] `b4 dig -w` — reviewers/maintainers CC'd
- [Phase 4] Downloaded mbox: Bjorn applied, Mario Reviewed-by, no stable
  nomination
- [Phase 4] lore.kernel.org direct fetch blocked (403/bot protection)
- [Phase 5] Traced `pci_pm_suspend_noirq` `!pm` → `goto Fixup` at line
  874 (buggy)
- [Phase 5] Confirmed `pci_pm_freeze_noirq` already sets UNKNOWN at line
  1098
- [Phase 6] Buggy code at lines 497–498 and 874 confirmed in 6.18.43
- [Phase 6] `git apply --check` on upstream patch — applies cleanly
- [Phase 6] Commit d462c8e89e84 not in 6.18.43 tree
- [Phase 8] Failure mode: stale D0 → device won't power up on driver
  bind; severity MEDIUM-HIGH for affected cases

**YES**The background searches finished and line up with the earlier
analysis:

- **Local tree (6.18.43):** The fix is **not** present. The buggy paths
  are still there — `PCI_UNKNOWN` is set on unbind, and the `!pm` branch
  in `pci_pm_suspend_noirq()` still skips `pci_pm_set_unknown_state()`.
- **Mainline (kernel.org):** The commit is **d462c8e89e84** (“PCI: Stop
  setting cached power state to 'unknown' on unbind”), merged April 27,
  2026 via the `pci/pm` branch.

That confirms the patch is a valid backport candidate for this tree and
applies cleanly. **Verdict remains YES.**

 drivers/pci/pci-driver.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index f9ead94d5ebb6..1219f39b1c69b 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -490,13 +490,6 @@ static void pci_device_remove(struct device *dev)
 	/* Undo the runtime PM settings in local_pci_probe() */
 	pm_runtime_put_sync(dev);
 
-	/*
-	 * If the device is still on, set the power state as "unknown",
-	 * since it might change by the next time we load the driver.
-	 */
-	if (pci_dev->current_state == PCI_D0)
-		pci_dev->current_state = PCI_UNKNOWN;
-
 	/*
 	 * We would love to complain here if pci_dev->is_enabled is set, that
 	 * the driver should have called pci_disable_device(), but the
@@ -871,7 +864,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 
 	if (!pm) {
 		pci_save_state(pci_dev);
-		goto Fixup;
+		goto set_unknown;
 	}
 
 	if (pm->suspend_noirq) {
@@ -923,6 +916,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 		goto Fixup;
 	}
 
+set_unknown:
 	pci_pm_set_unknown_state(pci_dev);
 
 	/*
-- 
2.53.0