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

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Jun 03, 2026 at 06:57:36PM +0000, sashiko-bot@kernel.org wrote:
> 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.
> ...

I don't know why sashiko calls this a "pre-existing" issue.  This
patch *adds* the config space accessibility check, so it looks like an
issue with this patch to me.

> > +++ 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?

I can't remember why we have both pcie_reset_flr() and pcie_flr(),
other than the fact that callers don't need to supply a "probe"
argument to pcie_flr().

Is there a reason to call pci_dev_config_accessible() here rather than
in pcie_flr()?