[PATCH v3 0/3] Unfiy PCI suspend and hibernate paths

[PATCH v3 0/3] Unfiy PCI suspend and hibernate paths

[Mario Limonciello]

Many cycles ago I sent out a kernel series for using the S4 paths when
the system goes to S5.  Some parts of it got merged, and Rafael suggested
to split the other parts into smaller pieces across multiple kernel cycles
to make bisecting easier.

This series attempts to unify the PCI suspend and hibernate paths so that
PCIe bridges with downstream devices are put into D3 at hibernate the same
way they already are at S3 suspend.  I have been carrying it, rebasing it
and personally using it for a while now.

Since v2, the two changes that had major push back on have been dropped
(see the changelog below), and the remaining patches have been reworked to
minimize churn and to explain the concrete motivation behind them.

v3:
 - Drop "PCI/PM: Disable device wakeups when halting or powering off
   system" (was 2/6).  It would break Wake-on-LAN / Remote Start from S5,
   has a user-visible sysfs side effect if the poweroff transition is
   rolled back, and runs counter to ACPI r6.5 sec 16.1.5 ("OSPM does not
   disable wake events before setting the SLP_EN bit when entering the S5
   system state"). (Rafael, Bjorn) 

 - Drop "PCI/PM: Use pci_power_manageable() in pci_pm_poweroff_noirq()" 
   (was 5/6).  It only touched the line that the final patch replaces, and 
   the shared helper already selects bridges via pci_power_manageable(), so
   it was superseded churn. (Rafael) 

 - Drop "PCI: Stop setting cached power state to unknown on unbind"
   (was 1/6) from the series; it has been merged.
   (Reviewed-by: Rafael J. Wysocki) 

 - Split-out-helper patch: keep pci_pm_set_unknown_state() in the callers
   so pci_pm_suspend_noirq()'s set_unknown: label is preserved and no label
   churn is needed; fix the kernel-doc comment for the new helper. (Rafael,
   Sashiko) 

 - Rewrite the changelog of "Run bridge power up actions as part of restore
   phase" to explain why it is needed: it is the restore-side counterpart
   to putting bridges into D3cold at hibernate, mirroring
   pci_pm_resume_noirq(). (Rafael) 

 - Rewrite the changelog of "Put PCIe bridges with downstream devices into
   D3 at hibernate" to describe the concrete problem (firmware not powering
   the bridges down at S4 on affected platforms, and the resulting power
   draw) and to argue safety (it mirrors the shipping S3 suspend_noirq path
   and is guarded identically). (Rafael) 

Mario Limonciello (AMD) (3):
  PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
  PCI/PM: Run bridge power up actions as part of restore phase
  PCI: Put PCIe bridges with downstream devices into D3 at hibernate

 drivers/pci/pci-driver.c | 89 ++++++++++++++++++++++++++--------------
 1 file changed, 59 insertions(+), 30 deletions(-)

-- 
2.43.0

[PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper

[Mario Limonciello]

From: "Mario Limonciello (AMD)" <superm1@kernel.org>

In order to unify suspend and hibernate codepaths without code duplication
the common code should be in common helpers.  Move it from
pci_pm_suspend_noirq() into a helper.  No intended functional changes.

Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/pci/pci-driver.c | 75 +++++++++++++++++++++++++---------------
 1 file changed, 47 insertions(+), 28 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e16aa59dd7ac8..e3d352c7b31d4 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -818,6 +818,50 @@ static void pci_pm_complete(struct device *dev)
 
 #endif /* !CONFIG_PM_SLEEP */
 
+#if defined(CONFIG_SUSPEND)
+/**
+ * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
+ * @pci_dev: pci device
+ * @skip_bus_pm: pointer to a boolean indicating whether to skip bus PM
+ *
+ * Save the device state and decide whether bus-level power management should
+ * be skipped, reporting the decision back to the caller via @skip_bus_pm.
+ */
+static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_pm)
+{
+	if (!pci_dev->state_saved) {
+		pci_save_state(pci_dev);
+
+		/*
+		 * If the device is a bridge with a child in D0 below it,
+		 * it needs to stay in D0, so check skip_bus_pm to avoid
+		 * putting it into a low-power state in that case.
+		 */
+		if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
+			pci_prepare_to_sleep(pci_dev);
+	}
+
+	pci_dbg(pci_dev, "PCI PM: Sleep power state: %s\n",
+		pci_power_name(pci_dev->current_state));
+
+	if (pci_dev->current_state == PCI_D0) {
+		pci_dev->skip_bus_pm = true;
+		/*
+		 * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
+		 * downstream device is in D0, so avoid changing the power state
+		 * of the parent bridge by setting the skip_bus_pm flag for it.
+		 */
+		if (pci_dev->bus->self)
+			pci_dev->bus->self->skip_bus_pm = true;
+	}
+
+	if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
+		pci_dbg(pci_dev, "PCI PM: Skipped\n");
+		*skip_bus_pm = true;
+	}
+}
+#endif /* CONFIG_SUSPEND */
+
 #ifdef CONFIG_SUSPEND
 static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
 {
@@ -907,6 +951,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+	bool skip_bus_pm = false;
 
 	if (dev_pm_skip_suspend(dev))
 		return 0;
@@ -937,36 +982,10 @@ static int pci_pm_suspend_noirq(struct device *dev)
 		}
 	}
 
-	if (!pci_dev->state_saved) {
-		pci_save_state(pci_dev);
-
-		/*
-		 * If the device is a bridge with a child in D0 below it,
-		 * it needs to stay in D0, so check skip_bus_pm to avoid
-		 * putting it into a low-power state in that case.
-		 */
-		if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
-			pci_prepare_to_sleep(pci_dev);
-	}
-
-	pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
-		pci_power_name(pci_dev->current_state));
+	pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
 
-	if (pci_dev->current_state == PCI_D0) {
-		pci_dev->skip_bus_pm = true;
-		/*
-		 * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
-		 * downstream device is in D0, so avoid changing the power state
-		 * of the parent bridge by setting the skip_bus_pm flag for it.
-		 */
-		if (pci_dev->bus->self)
-			pci_dev->bus->self->skip_bus_pm = true;
-	}
-
-	if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
-		pci_dbg(pci_dev, "PCI PM: Skipped\n");
+	if (skip_bus_pm)
 		goto Fixup;
-	}
 
 set_unknown:
 	pci_pm_set_unknown_state(pci_dev);
-- 
2.43.0

Re: [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper

[sashiko-bot]

> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
> 
> In order to unify suspend and hibernate codepaths without code duplication
> the common code should be in common helpers.  Move it from
> pci_pm_suspend_noirq() into a helper.  No intended functional changes.
> 
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831205533.2325747-1-mario.limonciello@amd.com?part=1

Re: [PATCH v3 1/3] PCI/PM: Split out code from pci_pm_suspend_noirq() into helper

[Rafael J. Wysocki (Intel)]

On Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> In order to unify suspend and hibernate codepaths without code duplication
> the common code should be in common helpers.  Move it from
> pci_pm_suspend_noirq() into a helper.  No intended functional changes.
>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  drivers/pci/pci-driver.c | 75 +++++++++++++++++++++++++---------------
>  1 file changed, 47 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e16aa59dd7ac8..e3d352c7b31d4 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -818,6 +818,50 @@ static void pci_pm_complete(struct device *dev)
>
>  #endif /* !CONFIG_PM_SLEEP */
>
> +#if defined(CONFIG_SUSPEND)
> +/**
> + * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
> + * @pci_dev: pci device
> + * @skip_bus_pm: pointer to a boolean indicating whether to skip bus PM
> + *
> + * Save the device state and decide whether bus-level power management should
> + * be skipped, reporting the decision back to the caller via @skip_bus_pm.
> + */
> +static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_pm)

Change it to bool.

> +{
> +       if (!pci_dev->state_saved) {
> +               pci_save_state(pci_dev);
> +
> +               /*
> +                * If the device is a bridge with a child in D0 below it,
> +                * it needs to stay in D0, so check skip_bus_pm to avoid
> +                * putting it into a low-power state in that case.
> +                */
> +               if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> +                       pci_prepare_to_sleep(pci_dev);
> +       }
> +
> +       pci_dbg(pci_dev, "PCI PM: Sleep power state: %s\n",
> +               pci_power_name(pci_dev->current_state));
> +
> +       if (pci_dev->current_state == PCI_D0) {
> +               pci_dev->skip_bus_pm = true;
> +               /*
> +                * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
> +                * downstream device is in D0, so avoid changing the power state
> +                * of the parent bridge by setting the skip_bus_pm flag for it.
> +                */
> +               if (pci_dev->bus->self)
> +                       pci_dev->bus->self->skip_bus_pm = true;
> +       }
> +
> +       if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> +               pci_dbg(pci_dev, "PCI PM: Skipped\n");
> +               *skip_bus_pm = true;

And return true here.

> +       }

And false here.

Then you won't need the second argument.

> +}
> +#endif /* CONFIG_SUSPEND */
> +
>  #ifdef CONFIG_SUSPEND
>  static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
>  {
> @@ -907,6 +951,7 @@ static int pci_pm_suspend_noirq(struct device *dev)
>  {
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +       bool skip_bus_pm = false;
>
>         if (dev_pm_skip_suspend(dev))
>                 return 0;
> @@ -937,36 +982,10 @@ static int pci_pm_suspend_noirq(struct device *dev)
>                 }
>         }
>
> -       if (!pci_dev->state_saved) {
> -               pci_save_state(pci_dev);
> -
> -               /*
> -                * If the device is a bridge with a child in D0 below it,
> -                * it needs to stay in D0, so check skip_bus_pm to avoid
> -                * putting it into a low-power state in that case.
> -                */
> -               if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
> -                       pci_prepare_to_sleep(pci_dev);
> -       }
> -
> -       pci_dbg(pci_dev, "PCI PM: Suspend power state: %s\n",
> -               pci_power_name(pci_dev->current_state));
> +       pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
>
> -       if (pci_dev->current_state == PCI_D0) {
> -               pci_dev->skip_bus_pm = true;
> -               /*
> -                * Per PCI PM r1.2, table 6-1, a bridge must be in D0 if any
> -                * downstream device is in D0, so avoid changing the power state
> -                * of the parent bridge by setting the skip_bus_pm flag for it.
> -                */
> -               if (pci_dev->bus->self)
> -                       pci_dev->bus->self->skip_bus_pm = true;
> -       }
> -
> -       if (pci_dev->skip_bus_pm && pm_suspend_no_platform()) {
> -               pci_dbg(pci_dev, "PCI PM: Skipped\n");
> +       if (skip_bus_pm)
>                 goto Fixup;
> -       }
>
>  set_unknown:
>         pci_pm_set_unknown_state(pci_dev);
> --
> 2.43.0
>

[PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase

[Mario Limonciello]

From: "Mario Limonciello (AMD)" <superm1@kernel.org>

pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
bridge is coming back from D3cold and bus PM was not skipped, so that
the mandatory post-power-on link training delays are observed before
any downstream device is accessed.

The last patch in this series makes the hibernate poweroff_noirq path
put bridges with downstream devices into D3 (including D3cold),
mirroring what the S3 suspend path already does.  For that to be safe,
the hibernate image restore path has to bring those bridges back up the
same way suspend resume does; otherwise a downstream device may be
accessed before its parent bridge's link is retrained.

Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
the same conditions as the resume path (bus PM not skipped and the
previous state being D3cold), so that hibernate restore and suspend
resume behave identically for bridges.

Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/pci/pci-driver.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index e3d352c7b31d4..d15b3eb55ec53 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1297,10 +1297,15 @@ static int pci_pm_restore_noirq(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+	pci_power_t prev_state = pci_dev->current_state;
+	bool skip_bus_pm = pci_dev->skip_bus_pm;
 
 	pci_pm_default_resume_early(pci_dev);
 	pci_fixup_device(pci_fixup_resume_early, pci_dev);
 
+	if (!skip_bus_pm && prev_state == PCI_D3cold)
+		pci_pm_bridge_power_up_actions(pci_dev);
+
 	if (pci_has_legacy_pm_support(pci_dev))
 		return 0;
 
-- 
2.43.0

Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The condition `prev_state == PCI_D3cold` in `pci_pm_restore_noirq()` will always evaluate to false for devices active before hibernation, causing bridge power-up actions to be incorrectly skipped.
- [High] `skip_bus_pm` evaluates to stale or uninitialized values during hibernation restore.
--

commit bf2b47aa05ce71a85384cc813156b2a5ff4653d5
Author: Mario Limonciello (AMD) <superm1@kernel.org>

PCI/PM: Run bridge power up actions as part of restore phase

This commit adds bridge power up actions to the hibernate restore phase in
pci_pm_restore_noirq(), gated by the previous power state and whether bus PM
was skipped. This aims to ensure mandatory link training delays are observed
before downstream devices are accessed, mirroring the S3 suspend path.

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e3d352c7b31d4..d15b3eb55ec53 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1297,10 +1297,15 @@ static int pci_pm_restore_noirq(struct device *dev)
>  {
>  	struct pci_dev *pci_dev = to_pci_dev(dev);
>  	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +	pci_power_t prev_state = pci_dev->current_state;

[Severity: High]
Will prev_state always evaluate to PCI_UNKNOWN instead of PCI_D3cold?

During hibernation, the memory snapshot is taken in the FREEZE phase.
pci_pm_freeze() resumes devices to PCI_D0, and pci_pm_freeze_noirq()
subsequently sets their current_state to PCI_UNKNOWN.

This PCI_UNKNOWN state is what gets saved in the memory snapshot and restored
here. Although devices are put in PCI_D3cold during the POWEROFF phase, this
occurs after the snapshot has already been captured and written to disk.

> +	bool skip_bus_pm = pci_dev->skip_bus_pm;

[Severity: High]
Could pci_dev->skip_bus_pm evaluate to a stale or uninitialized value here?

The hibernation snapshot is captured during the FREEZE phase, where neither
pci_pm_freeze() nor pci_pm_freeze_noirq() initialize or update skip_bus_pm.
The snapshot merely captures whatever stale value it had, such as from a prior
S3 suspend.

While skip_bus_pm is initialized in pci_pm_poweroff(), that function runs in
the POWEROFF phase after the hibernation snapshot has been completed, so the
initialized value is never saved to the image.

>  
>  	pci_pm_default_resume_early(pci_dev);
>  	pci_fixup_device(pci_fixup_resume_early, pci_dev);
>  
> +	if (!skip_bus_pm && prev_state == PCI_D3cold)
> +		pci_pm_bridge_power_up_actions(pci_dev);

[Severity: High]
Since prev_state is PCI_UNKNOWN and skip_bus_pm may be stale, is it possible
that this condition will always fail and cause the required bridge retraining
to be skipped?

This could lead to accessing downstream devices before the bridge link is
retrained, resulting in PCIe errors or system crashes on hibernation resume.

> +
>  	if (pci_has_legacy_pm_support(pci_dev))
>  		return 0;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831205533.2325747-1-mario.limonciello@amd.com?part=2

Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase

[Mario Limonciello]

Based on the feedback (the robot was correct!) I'm going to drop this 
patch from the series and adjust patch 3/3 slightly.

On 8/31/26 17:06, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The condition `prev_state == PCI_D3cold` in `pci_pm_restore_noirq()` will always evaluate to false for devices active before hibernation, causing bridge power-up actions to be incorrectly skipped.
> - [High] `skip_bus_pm` evaluates to stale or uninitialized values during hibernation restore.
> --
> 
> commit bf2b47aa05ce71a85384cc813156b2a5ff4653d5
> Author: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> PCI/PM: Run bridge power up actions as part of restore phase
> 
> This commit adds bridge power up actions to the hibernate restore phase in
> pci_pm_restore_noirq(), gated by the previous power state and whether bus PM
> was skipped. This aims to ensure mandatory link training delays are observed
> before downstream devices are accessed, mirroring the S3 suspend path.
> 
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> index e3d352c7b31d4..d15b3eb55ec53 100644
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
>> @@ -1297,10 +1297,15 @@ static int pci_pm_restore_noirq(struct device *dev)
>>   {
>>   	struct pci_dev *pci_dev = to_pci_dev(dev);
>>   	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
>> +	pci_power_t prev_state = pci_dev->current_state;
> 
> [Severity: High]
> Will prev_state always evaluate to PCI_UNKNOWN instead of PCI_D3cold?
> 
> During hibernation, the memory snapshot is taken in the FREEZE phase.
> pci_pm_freeze() resumes devices to PCI_D0, and pci_pm_freeze_noirq()
> subsequently sets their current_state to PCI_UNKNOWN.
> 
> This PCI_UNKNOWN state is what gets saved in the memory snapshot and restored
> here. Although devices are put in PCI_D3cold during the POWEROFF phase, this
> occurs after the snapshot has already been captured and written to disk.
> 

The robot looks right here.

>> +	bool skip_bus_pm = pci_dev->skip_bus_pm;
> 
> [Severity: High]
> Could pci_dev->skip_bus_pm evaluate to a stale or uninitialized value here?
> 
> The hibernation snapshot is captured during the FREEZE phase, where neither
> pci_pm_freeze() nor pci_pm_freeze_noirq() initialize or update skip_bus_pm.
> The snapshot merely captures whatever stale value it had, such as from a prior
> S3 suspend.
> 
> While skip_bus_pm is initialized in pci_pm_poweroff(), that function runs in
> the POWEROFF phase after the hibernation snapshot has been completed, so the
> initialized value is never saved to the image.


The robot looks right here too.

> 
>>   
>>   	pci_pm_default_resume_early(pci_dev);
>>   	pci_fixup_device(pci_fixup_resume_early, pci_dev);
>>   
>> +	if (!skip_bus_pm && prev_state == PCI_D3cold)
>> +		pci_pm_bridge_power_up_actions(pci_dev);
> 
> [Severity: High]
> Since prev_state is PCI_UNKNOWN and skip_bus_pm may be stale, is it possible
> that this condition will always fail and cause the required bridge retraining
> to be skipped?
> 
> This could lead to accessing downstream devices before the bridge link is
> retrained, resulting in PCIe errors or system crashes on hibernation resume.
> 
>> +
>>   	if (pci_has_legacy_pm_support(pci_dev))
>>   		return 0;
>>
> 

Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase

[Rafael J. Wysocki (Intel)]

On Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> From: "Mario Limonciello (AMD)" <superm1@kernel.org>
>
> pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
> bridge is coming back from D3cold and bus PM was not skipped, so that
> the mandatory post-power-on link training delays are observed before
> any downstream device is accessed.
>
> The last patch in this series makes the hibernate poweroff_noirq path
> put bridges with downstream devices into D3 (including D3cold),
> mirroring what the S3 suspend path already does.  For that to be safe,
> the hibernate image restore path has to bring those bridges back up the
> same way suspend resume does; otherwise a downstream device may be
> accessed before its parent bridge's link is retrained.

Note though that pci_pm_restore_noirq() runs after the boot kernel has
initialized the PCI bus, so all of the links should be retrained by
that time.

It then carries out a "freeze" transition before passing control to
the image kernel, so "restore" doesn't really get devices in
"pristine" states.

Moreover, BIOS code runs before "poweroff_noirq" and the boot kernel
and it also messes up with devices in general.

Is there a specific issue that is avoided by this change?

> Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
> the same conditions as the resume path (bus PM not skipped and the
> previous state being D3cold), so that hibernate restore and suspend
> resume behave identically for bridges.
>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
>  drivers/pci/pci-driver.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index e3d352c7b31d4..d15b3eb55ec53 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -1297,10 +1297,15 @@ static int pci_pm_restore_noirq(struct device *dev)
>  {
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> +       pci_power_t prev_state = pci_dev->current_state;
> +       bool skip_bus_pm = pci_dev->skip_bus_pm;
>
>         pci_pm_default_resume_early(pci_dev);
>         pci_fixup_device(pci_fixup_resume_early, pci_dev);
>
> +       if (!skip_bus_pm && prev_state == PCI_D3cold)
> +               pci_pm_bridge_power_up_actions(pci_dev);
> +
>         if (pci_has_legacy_pm_support(pci_dev))
>                 return 0;
>
> --
> 2.43.0
>

Re: [PATCH v3 2/3] PCI/PM: Run bridge power up actions as part of restore phase

[Rafael J. Wysocki (Intel)]

On Fri, Sep 4, 2026 at 3:09 PM Rafael J. Wysocki (Intel)
<rafael@kernel.org> wrote:
>
> On Mon, Aug 31, 2026 at 10:56 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> >
> > From: "Mario Limonciello (AMD)" <superm1@kernel.org>
> >
> > pci_pm_resume_noirq() calls pci_pm_bridge_power_up_actions() when a
> > bridge is coming back from D3cold and bus PM was not skipped, so that
> > the mandatory post-power-on link training delays are observed before
> > any downstream device is accessed.
> >
> > The last patch in this series makes the hibernate poweroff_noirq path
> > put bridges with downstream devices into D3 (including D3cold),
> > mirroring what the S3 suspend path already does.  For that to be safe,
> > the hibernate image restore path has to bring those bridges back up the
> > same way suspend resume does; otherwise a downstream device may be
> > accessed before its parent bridge's link is retrained.
>
> Note though that pci_pm_restore_noirq() runs after the boot kernel has
> initialized the PCI bus, so all of the links should be retrained by
> that time.
>
> It then carries out a "freeze" transition before passing control to

s/It then/The boot kernel/

> the image kernel, so "restore" doesn't really get devices in
> "pristine" states.
>
> Moreover, BIOS code runs before "poweroff_noirq" and the boot kernel

s/before/between/

> and it also messes up with devices in general.

Sent too early, sorry about that, but the question below is still relevant.

> Is there a specific issue that is avoided by this change?
>
> > Add the same bridge power up actions to pci_pm_restore_noirq(), gated on
> > the same conditions as the resume path (bus PM not skipped and the
> > previous state being D3cold), so that hibernate restore and suspend
> > resume behave identically for bridges.
> >
> > Tested-by: Eric Naim <dnaim@cachyos.org>
> > Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> > ---
> >  drivers/pci/pci-driver.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index e3d352c7b31d4..d15b3eb55ec53 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -1297,10 +1297,15 @@ static int pci_pm_restore_noirq(struct device *dev)
> >  {
> >         struct pci_dev *pci_dev = to_pci_dev(dev);
> >         const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> > +       pci_power_t prev_state = pci_dev->current_state;
> > +       bool skip_bus_pm = pci_dev->skip_bus_pm;
> >
> >         pci_pm_default_resume_early(pci_dev);
> >         pci_fixup_device(pci_fixup_resume_early, pci_dev);
> >
> > +       if (!skip_bus_pm && prev_state == PCI_D3cold)
> > +               pci_pm_bridge_power_up_actions(pci_dev);
> > +
> >         if (pci_has_legacy_pm_support(pci_dev))
> >                 return 0;
> >
> > --
> > 2.43.0
> >

[PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate

[Mario Limonciello]

From: "Mario Limonciello (AMD)" <superm1@kernel.org>

During S3 suspend, pci_pm_suspend_noirq() puts PCIe bridges with
downstream devices into a low-power state (D3hot or D3cold) when the
platform allows it.  The hibernate poweroff_noirq path never did this:
it only called pci_prepare_to_sleep() for devices with no subordinate,
so bridges with active children were left in D0.

The platform firmware leaves bridges alone when the system enters S4.
This prevents being able to meet various energy certification criteria
for different parts of the world, particularly in designs with a dGPU.

Align the hibernate flow with suspend by making pci_pm_poweroff_noirq()
use pci_pm_suspend_noirq_common() instead of the open-coded
pci_prepare_to_sleep() call.  This reuses exactly the logic the S3
suspend path uses, including the skip_bus_pm handling that keeps a
bridge in D0 when a downstream device must stay in D0 (e.g. a
configured wakeup source) and the pm_suspend_no_platform() bus-PM skip.
The restore side is handled by the preceding patch, which runs the
bridge power up actions when a bridge comes back from D3cold.

Because the poweroff_noirq path now mirrors the already-shipping
suspend_noirq path and is guarded identically, bridges that must remain
in D0 are unaffected; only bridges that S3 suspend would have powered
down are now also powered down at hibernate.

Cc: AceLan Kao <acelan.kao@canonical.com>
Cc: Kai-Heng Feng <kaihengf@nvidia.com>
Cc: Mark Pearson <mpearson-lenovo@squebb.ca>
Cc: Denis Benato <benato.denis96@gmail.com>
Cc: Merthan Karakaş <m3rthn.k@gmail.com>
Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
 drivers/pci/pci-driver.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index d15b3eb55ec53..a6fb731378d9c 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -818,7 +818,7 @@ static void pci_pm_complete(struct device *dev)
 
 #endif /* !CONFIG_PM_SLEEP */
 
-#if defined(CONFIG_SUSPEND)
+#if defined(CONFIG_SUSPEND) || defined(CONFIG_HIBERNATE_CALLBACKS)
 /**
  * pci_pm_suspend_noirq_common - prepare a device to enter a low-power state
  * @pci_dev: pci device
@@ -860,7 +860,7 @@ static void pci_pm_suspend_noirq_common(struct pci_dev *pci_dev, bool *skip_bus_
 		*skip_bus_pm = true;
 	}
 }
-#endif /* CONFIG_SUSPEND */
+#endif /* CONFIG_SUSPEND || CONFIG_HIBERNATE_CALLBACKS */
 
 #ifdef CONFIG_SUSPEND
 static void pcie_pme_root_status_cleanup(struct pci_dev *pci_dev)
@@ -1215,6 +1215,8 @@ static int pci_pm_poweroff(struct device *dev)
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
 
+	pci_dev->skip_bus_pm = false;
+
 	if (pci_has_legacy_pm_support(pci_dev))
 		return pci_legacy_suspend(dev, PMSG_HIBERNATE);
 
@@ -1257,6 +1259,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
+	bool skip_bus_pm = false;
 
 	if (dev_pm_skip_suspend(dev))
 		return 0;
@@ -1278,8 +1281,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 			return error;
 	}
 
-	if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
-		pci_prepare_to_sleep(pci_dev);
+	pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
+	if (skip_bus_pm)
+		goto Fixup;
 
 	/*
 	 * The reason for doing this here is the same as for the analogous code
@@ -1288,6 +1292,7 @@ static int pci_pm_poweroff_noirq(struct device *dev)
 	if (pci_dev->class == PCI_CLASS_SERIAL_USB_EHCI)
 		pci_write_config_word(pci_dev, PCI_COMMAND, 0);
 
+Fixup:
 	pci_fixup_device(pci_fixup_suspend_late, pci_dev);
 
 	return 0;
-- 
2.43.0

Re: [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hibernation restore skips mandatory PCIe bridge link retraining delays due to snapshot state mismatch, leading to fatal PCIe errors when downstream devices are accessed.
--

commit daecc28d1bb864ce78fb9c9df2c8ecd53a30fb12
Author: Mario Limonciello (AMD) <superm1@kernel.org>

PCI: Put PCIe bridges with downstream devices into D3 at hibernate

This commit aligns the hibernation path with the S3 suspend path by making
pci_pm_poweroff_noirq() use pci_pm_suspend_noirq_common(). This allows PCIe
bridges with downstream devices to enter low-power states like D3cold during
hibernation.

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index d15b3eb55ec53..a6fb731378d9c 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c

[ ... ]

> @@ -1278,8 +1281,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
>  			return error;
>  	}
>  
> -	if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
> -		pci_prepare_to_sleep(pci_dev);
> +	pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
> +	if (skip_bus_pm)
> +		goto Fixup;

[Severity: High]
Does this cause mandatory PCIe bridge link retraining delays to be skipped on
resume from hibernation?

During hibernation, pci_pm_freeze_noirq() sets the device state to PCI_UNKNOWN
via pci_pm_set_unknown_state() just before the memory snapshot is taken. The
snapshot therefore records the state as PCI_UNKNOWN.

Later in the hibernation process, pci_pm_suspend_noirq_common() powers the
bridge down to D3cold.

On system resume, the boot kernel loads the memory snapshot, reverting the
in-memory state back to PCI_UNKNOWN.

When pci_pm_restore_noirq() executes, it skips the bridge power up actions
because the restored state doesn't match PCI_D3cold:

    if (!skip_bus_pm && prev_state == PCI_D3cold)
        pci_pm_bridge_power_up_actions(pci_dev);

Can this state mismatch lead to downstream device accesses failing with fatal
PCIe errors on resume?

>  
>  	/*
>  	 * The reason for doing this here is the same as for the analogous code

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831205533.2325747-1-mario.limonciello@amd.com?part=3

Re: [PATCH v3 3/3] PCI: Put PCIe bridges with downstream devices into D3 at hibernate

[Mario Limonciello]

I think this one is a false positive based upon an incorrect premise 
because hibernate-to-disk power-cycles the entire machine.

Only after all that does the image load, atomic-restore, and 
restore_noirq run — against hardware whose links are already up and trained.

I'll adjust commit message with v4 for patch 3/3 (and drop patch 2/3).

On 8/31/26 17:20, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Hibernation restore skips mandatory PCIe bridge link retraining delays due to snapshot state mismatch, leading to fatal PCIe errors when downstream devices are accessed.
> --
> 
> commit daecc28d1bb864ce78fb9c9df2c8ecd53a30fb12
> Author: Mario Limonciello (AMD) <superm1@kernel.org>
> 
> PCI: Put PCIe bridges with downstream devices into D3 at hibernate
> 
> This commit aligns the hibernation path with the S3 suspend path by making
> pci_pm_poweroff_noirq() use pci_pm_suspend_noirq_common(). This allows PCIe
> bridges with downstream devices to enter low-power states like D3cold during
> hibernation.
> 
>> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
>> index d15b3eb55ec53..a6fb731378d9c 100644
>> --- a/drivers/pci/pci-driver.c
>> +++ b/drivers/pci/pci-driver.c
> 
> [ ... ]
> 
>> @@ -1278,8 +1281,9 @@ static int pci_pm_poweroff_noirq(struct device *dev)
>>   			return error;
>>   	}
>>   
>> -	if (!pci_dev->state_saved && !pci_has_subordinate(pci_dev))
>> -		pci_prepare_to_sleep(pci_dev);
>> +	pci_pm_suspend_noirq_common(pci_dev, &skip_bus_pm);
>> +	if (skip_bus_pm)
>> +		goto Fixup;
> 
> [Severity: High]
> Does this cause mandatory PCIe bridge link retraining delays to be skipped on
> resume from hibernation?
> 
> During hibernation, pci_pm_freeze_noirq() sets the device state to PCI_UNKNOWN
> via pci_pm_set_unknown_state() just before the memory snapshot is taken. The
> snapshot therefore records the state as PCI_UNKNOWN.
> 
> Later in the hibernation process, pci_pm_suspend_noirq_common() powers the
> bridge down to D3cold.
> 
> On system resume, the boot kernel loads the memory snapshot, reverting the
> in-memory state back to PCI_UNKNOWN.
> 
> When pci_pm_restore_noirq() executes, it skips the bridge power up actions
> because the restored state doesn't match PCI_D3cold:
> 
>      if (!skip_bus_pm && prev_state == PCI_D3cold)
>          pci_pm_bridge_power_up_actions(pci_dev);
> 
> Can this state mismatch lead to downstream device accesses failing with fatal
> PCIe errors on resume?
> 
>>   
>>   	/*
>>   	 * The reason for doing this here is the same as for the analogous code
>