Re: [PATCH v15 2/7] PCI: Avoid saving config space state if inaccessible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Early return when config space is inaccessible skips device DMA disablement.
- [Low] The 'msg' parameter in pci_dev_config_accessible() lacks a 'const' qualifier.
--

commit 18b574199dc0470f2784ade2f4883a656c60245e
Author: Farhan Ali <alifm@linux.ibm.com>

PCI: Avoid saving config space state if inaccessible

The commit updates the reset process to check if a device's config space
is accessible before attempting to save its state. This prevents saving
invalid values if the device is in an error state like DPC.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index d0c9f0166af5c..e71b814153921 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -722,6 +722,27 @@ u16 pci_find_dvsec_capability(struct pci_dev *dev, u16 vendor, u16 dvsec)
>  }
>  EXPORT_SYMBOL_GPL(pci_find_dvsec_capability);
>  
> +static bool pci_dev_config_accessible(struct pci_dev *dev, char *msg)

This isn't a bug, but should the msg parameter be declared as a const
char pointer?

The function is later called with the string literal "save state".

[ ... ]

> @@ -5027,6 +5048,9 @@ 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"))
> +		return;
> +
>  	pci_save_state(dev);
>  	/*
>  	 * Disable the device by clearing the Command register, except for
>  	 * INTx-disable which is set.  This not only disables MMIO and I/O port
>  	 * BARs, but also prevents the device from being Bus Master, preventing
>  	 * DMA from the device including MSI/MSI-X interrupts.  For PCI 2.3
>  	 * compliant devices, INTx-disable prevents legacy interrupts.
>  	 */
>  	pci_write_config_word(dev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);

Does returning early here skip disabling the device DMA engine?

If the configuration space read fails due to a transient error like a
completion timeout instead of the link being fully down, the device might
still be active.

Skipping the write to PCI_COMMAND leaves Bus Master enabled, which could
allow the device to continue performing DMA or raising interrupts during the
reset process.

Could the code conditionally skip pci_save_state() but still attempt the
write to PCI_COMMAND to ensure a best-effort disablement?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260505200510.2954-1-alifm@linux.ibm.com?part=2