[PATCH v2 1/3] PCI/PM: Reinstate clearing state_saved in legacy and !pm codepaths

[Rafael J. Wysocki]

On Wed, Nov 19, 2025 at 9:59 AM Lukas Wunner <lukas at wunner.de> wrote:
>
> When a PCI device is suspended, it is normally the PCI core's job to save
> Config Space and put the device into a low power state.  However drivers
> are allowed to assume these responsibilities.  When they do, the PCI core
> can tell by looking at the state_saved flag in struct pci_dev:  The flag
> is cleared before commencing the suspend sequence and it is set when
> pci_save_state() is called.  If the PCI core finds the flag set late in
> the suspend sequence, it refrains from calling pci_save_state() itself.
>
> But there are two corner cases where the PCI core neglects to clear the
> flag before commencing the suspend sequence:
>
> * If a driver has legacy PCI PM callbacks, pci_legacy_suspend() neglects
>   to clear the flag.  The (stale) flag is subsequently queried by
>   pci_legacy_suspend() itself and pci_legacy_suspend_late().
>
> * If a device has no driver or its driver has no PCI PM callbacks,
>   pci_pm_freeze() neglects to clear the flag.  The (stale) flag is
>   subsequently queried by pci_pm_freeze_noirq().
>
> The flag may be set prior to suspend if the device went through error
> recovery:  Drivers commonly invoke pci_restore_state() + pci_save_state()
> to restore Config Space after reset.
>
> The flag may also be set if drivers call pci_save_state() on probe to
> allow for recovery from subsequent errors.
>
> The result is that pci_legacy_suspend_late() and pci_pm_freeze_noirq()
> don't call pci_save_state() and so the state that will be restored on
> resume is the one recorded on last error recovery or on probe, not the one
> that the device had on suspend.  If the two states happen to be identical,
> there's no problem.
>
> Reinstate clearing the flag in pci_legacy_suspend() and pci_pm_freeze().
> The two functions used to do that until commit 4b77b0a2ba27 ("PCI: Clear
> saved_state after the state has been restored") deemed it unnecessary
> because it assumed that it's sufficient to clear the flag on resume in
> pci_restore_state().  The commit seemingly did not take into account that
> pci_save_state() and pci_restore_state() are not only used by power
> management code, but also for error recovery.

That's right, it didn't.

> Devices without driver or whose driver has no PCI PM callbacks may be in
> runtime suspend when pci_pm_freeze() is called.  Their state has already
> been saved, so don't clear the flag to skip a pointless pci_save_state()
> in pci_pm_freeze_noirq().
>
> None of the drivers with legacy PCI PM callbacks seem to use runtime PM,
> so clear the flag unconditionally in their case.
>
> Fixes: 4b77b0a2ba27 ("PCI: Clear saved_state after the state has been restored")
> Signed-off-by: Lukas Wunner <lukas at wunner.de>
> Cc: stable at vger.kernel.org # v2.6.32+

Reviewed-by: Rafael J. Wysocki (Intel) <rafael at kernel.org>

> ---
>  drivers/pci/pci-driver.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 302d61783f6c..327b21c48614 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -629,6 +629,8 @@ static int pci_legacy_suspend(struct device *dev, pm_message_t state)
>         struct pci_dev *pci_dev = to_pci_dev(dev);
>         struct pci_driver *drv = pci_dev->driver;
>
> +       pci_dev->state_saved = false;
> +
>         if (drv && drv->suspend) {
>                 pci_power_t prev = pci_dev->current_state;
>                 int error;
> @@ -1036,6 +1038,8 @@ static int pci_pm_freeze(struct device *dev)
>
>         if (!pm) {
>                 pci_pm_default_suspend(pci_dev);
> +               if (!pm_runtime_suspended(dev))
> +                       pci_dev->state_saved = false;
>                 return 0;
>         }
>
> --
> 2.51.0
>
>