[PATCH] PCI: Only restore state that was successfully saved around a reset

[PATCH] PCI: Only restore state that was successfully saved around a reset

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

pci_dev_save_and_disable() returns early when config space is inaccessible,
without saving anything, but pci_dev_restore() calls pci_restore_state()
unconditionally and pci_restore_state() does not look at state_saved. After
the reset makes config space accessible again, the all-zero or stale
saved_config_space[] array is written back to the device, clearing Command,
BARs and, for bridges, the bus number registers.

Clear state_saved when the save is skipped and only restore when a state
was saved.

Fixes: e18d1abc3bff ("PCI: Avoid saving config space state if inaccessible")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/pci/pci.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 67fe565..ddb86dd 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5156,8 +5156,10 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
 	 */
 	pci_set_power_state(dev, PCI_D0);
 
-	if (!pci_dev_config_accessible(dev, "save state"))
+	if (!pci_dev_config_accessible(dev, "save state")) {
+		dev->state_saved = false;
 		return;
+	}
 
 	pci_save_state(dev);
 	/*
@@ -5175,7 +5177,8 @@ static void pci_dev_restore(struct pci_dev *dev)
 	const struct pci_error_handlers *err_handler =
 			dev->driver ? dev->driver->err_handler : NULL;
 
-	pci_restore_state(dev);
+	if (dev->state_saved)
+		pci_restore_state(dev);
 
 	/*
 	 * dev->driver->err_handler->reset_done() is protected against
-- 
2.9.4

[PATCH] PCI: Save the Root Port state before a platform-specific reset

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

pcibios_reset_secondary_bus() restores the Root Port after
host->reset_root_port() without ever saving it. pci_bus_reset() only locks
the devices below the bridge, so the bridge itself is not in the saved set
either. Save the Root Port before the platform reset.

Fixes: 3fc686d550f6 ("PCI/ERR: Add support for resetting the Root Ports in a platform-specific way")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/pci/pci.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ddb86dd..eaccd89 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4929,6 +4929,7 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
 	int ret;
 
 	if (pci_is_root_bus(dev->bus) && host->reset_root_port) {
+		pci_save_state(dev);
 		ret = host->reset_root_port(host, dev);
 		if (ret)
 			pci_err(dev, "Failed to reset Root Port: %d\n", ret);
-- 
2.9.4

Re: [PATCH] PCI: Save the Root Port state before a platform-specific reset

[Lukas Wunner]

On Tue, Nov 10, 2026 at 01:33:48PM +0800, lirongqing wrote:
> pcibios_reset_secondary_bus() restores the Root Port after
> host->reset_root_port() without ever saving it. pci_bus_reset() only locks
> the devices below the bridge, so the bridge itself is not in the saved set
> either. Save the Root Port before the platform reset.

The state is saved on enumeration in pci_bus_add_device(),
see commit a2f1e22390ac.  Please explain why that is not
sufficient for you.

Thanks,

Lukas

Re: [PATCH] PCI: Only restore state that was successfully saved around a reset

[Lukas Wunner]

On Tue, Nov 10, 2026 at 01:33:47PM +0800, lirongqing wrote:
> pci_dev_save_and_disable() returns early when config space is inaccessible,
> without saving anything, but pci_dev_restore() calls pci_restore_state()
> unconditionally and pci_restore_state() does not look at state_saved. After
> the reset makes config space accessible again, the all-zero or stale
> saved_config_space[] array is written back to the device, clearing Command,
> BARs and, for bridges, the bus number registers.
> 
> Clear state_saved when the save is skipped and only restore when a state
> was saved.

The state_saved flag is used internally by the PCI core for
suspend/resume.  If a driver calls pci_save_state() on suspend,
the flag is set and that tells the PCI core that the driver already
put the device into a lower power state, obviating the need for the
PCI core to do that.

Please do not use the flag for a completely different purpose.

Thanks,

Lukas

Re: [PATCH] PCI: Only restore state that was successfully saved around a reset

[Farhan Ali]

Hi Li,

On 11/9/2026 9:33 PM, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
>
> pci_dev_save_and_disable() returns early when config space is inaccessible,
> without saving anything, but pci_dev_restore() calls pci_restore_state()
> unconditionally and pci_restore_state() does not look at state_saved. After
> the reset makes config space accessible again, the all-zero or stale
> saved_config_space[] array is written back to the device, clearing Command,
> BARs and, for bridges, the bus number registers.
>
> Clear state_saved when the save is skipped and only restore when a state
> was saved.

We should always have a valid state of the device saved at enumeration 
due to commit a2f1e22390ac ("PCI/ERR: Ensure error recoverability at all 
times"). In pci_dev_save_disable(), if the device is inaccessible then 
we want to avoid storing a state that can corrupt the device on restore. 
Since we always have a valid state, we would restore to the last good state.

May I ask what device or driver do you hit this issue?

Thanks

Farhan


>
> Fixes: e18d1abc3bff ("PCI: Avoid saving config space state if inaccessible")
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> ---
>   drivers/pci/pci.c | 7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 67fe565..ddb86dd 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5156,8 +5156,10 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
>   	 */
>   	pci_set_power_state(dev, PCI_D0);
>   
> -	if (!pci_dev_config_accessible(dev, "save state"))
> +	if (!pci_dev_config_accessible(dev, "save state")) {
> +		dev->state_saved = false;
>   		return;
> +	}
>   
>   	pci_save_state(dev);
>   	/*
> @@ -5175,7 +5177,8 @@ static void pci_dev_restore(struct pci_dev *dev)
>   	const struct pci_error_handlers *err_handler =
>   			dev->driver ? dev->driver->err_handler : NULL;
>   
> -	pci_restore_state(dev);
> +	if (dev->state_saved)
> +		pci_restore_state(dev);
>   
>   	/*
>   	 * dev->driver->err_handler->reset_done() is protected against