Re: [PATCH v18 3/3] PCI: Fail FLR when config space is inaccessible

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The config space accessibility check is placed in pcie_reset_flr() instead of pcie_flr(), leaving direct callers of pcie_flr() exposed to 60-second hangs.
- [High] The PCI reset quirk delay_250ms_after_flr ignores the return value of pcie_reset_flr(), which masks reset failures and completely breaks the PCI reset fallback mechanism.
--

commit 4e372ed957b10d82d85382f347de3e515a680bbf
Author: Farhan Ali <alifm@linux.ibm.com>

PCI: Fail FLR when config space is inaccessible

This commit adds an accessibility check for the PCI configuration space before
attempting an FLR (Function Level Reset). This is intended to avoid potential
hangs on devices that are in an error state.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e71b814153921..c50418f5e3181 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4396,6 +4396,9 @@ int pcie_reset_flr(struct pci_dev *dev, bool probe)
>  	if (!(dev->devcap & PCI_EXP_DEVCAP_FLR))
>  		return -ENOTTY;
>  
> +	if (!pci_dev_config_accessible(dev, "FLR"))
> +		return -ENOTTY;
> +

[Severity: High]
This isn't a bug introduced by this patch, but does placing this check in
pcie_reset_flr() leave direct callers of pcie_flr() exposed to 60-second
hangs?

Direct callers of pcie_flr() (like network and storage drivers during error
recovery) bypass this wrapper. They will still experience a stall in
pci_dev_wait() when attempting to reset a device with an inaccessible config
space. Should this accessibility check be moved into pcie_flr() instead to
protect all callers?

[Severity: High]
This is a pre-existing issue, but does adding this new dynamic failure path
break the fallback reset mechanism in delay_250ms_after_flr()?

Looking at delay_250ms_after_flr() in drivers/pci/quirks.c, it executes
pcie_reset_flr() but discards its return value:

drivers/pci/quirks.c:delay_250ms_after_flr() {
    ...
    pcie_reset_flr(dev, PCI_RESET_DO_RESET);

    msleep(250);

    return 0;
}

With this patch returning -ENOTTY when the config space is inaccessible,
the quirk will falsely report success. This causes
__pci_reset_function_locked() to abort early and completely prevents it from
attempting fallback reset methods, leaving the device in an unrecoverable
state.

>  	if (probe)
>  		return 0;
>

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