Re: [PATCH v9 2/5] PCI/ERR: Add support for resetting the Root Ports in a platform specific way

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The Root Port configuration space is restored without being saved before the platform-specific reset, leading to permanent loss of runtime configuration changes.
--

--- Patch [2]: [PATCH v9 2/5] PCI/ERR: Add support for resetting the Root Ports in a platform specific way ---
commit 2b1b6bf7dae47cb5b5ce9a84251ceb7b5b08de2c
Author: Manivannan Sadhasivam <mani@kernel.org>
PCI/ERR: Add support for resetting the Root Ports in a platform specific way

This commit introduces a reset_root_port callback in struct pci_host_bridge
to allow platform-specific root port resets during error recovery. It
modifies pcibios_reset_secondary_bus() to invoke this callback if available.
The commit message states that it will save the Root Port config space before
the reset and restore it afterwards.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 77b17b13ee615..12e099489690b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4841,6 +4841,19 @@ void pci_reset_secondary_bus(struct pci_dev *dev)
>  
>  void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
> +	int ret;
> +
> +	if (pci_is_root_bus(dev->bus) && host->reset_root_port) {
> +		ret = host->reset_root_port(host, dev);
> +		if (ret)
> +			pci_err(dev, "Failed to reset Root Port: %d\n", ret);
> +		else
> +			pci_restore_state(dev);

[Severity: High]
The commit message states "save the Root Port config space before reset and
restore it afterwards". However, is a call to pci_save_state(dev) missing
before the call to host->reset_root_port()?

Calling pci_restore_state(dev) here without a preceding save operation could
restore a stale configuration saved at boot or during the last suspend. Can
this lead to the permanent loss of runtime configuration changes on the Root
Port, such as ASPM link states, dynamic AER masks, or Max Payload Size
adjustments?

> +
> +		return;
> +	}
> +
>  	pci_reset_secondary_bus(dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-port-reset-v9-0-53570b92064d@oss.qualcomm.com?part=2