Re: [PATCH v21 2/4] 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:
- [High] pci_dev_save_and_disable() skips disabling the device if config space is momentarily inaccessible, potentially leaving DMA and interrupts enabled.
- [Low] String literal passed to a non-const `char *` pointer in `pci_dev_config_accessible()`.
--

commit 067c10f20c7b3c97acf8c9886f6586c07bea7142
Author: Farhan Ali <alifm@linux.ibm.com>

PCI: Avoid saving config space state if inaccessible

This commit avoids saving the device's config space state during a reset
if the device config space is temporarily inaccessible, preventing invalid
values from being written back later.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -713,6 +713,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)

[Severity: Low]
This isn't a bug, but should the msg parameter be marked const?

Since a string literal ("save state") is passed to this function, keeping
it as a non-const pointer violates const-correctness and might trigger
compiler warnings.

> +{
> +	u32 val;
> +

[ ... ]

> @@ -5059,6 +5080,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;
> +

[Severity: High]
Does this early return skip disabling the device entirely if the config
space is momentarily inaccessible?

If the device is only transiently inaccessible and recovers immediately
after the pci_dev_config_accessible() check, this early return skips the
subsequent pci_write_config_word() that clears the Command register.

Could this leave the device with Bus Master enabled, potentially allowing
it to generate unexpected DMA requests or interrupt storms during the
reset sequence?

>  	pci_save_state(dev);
>  	/*
>  	 * Disable the device by clearing the Command register, except for

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