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

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Alex]

On Wed, Jun 10, 2026 at 09:51:55AM -0700, Farhan Ali wrote:
> On 6/9/2026 3:33 PM, Bjorn Helgaas wrote:
> > 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:
> ...

> > > > +++ 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()?
> 
> The reason we wanted the check in pcie_reset_flr() was so to be able
> to escalate to bus reset method if we can't do an FLR. I think the
> check could be moved to pcie_flr(). Is that more preferable?

What if we added a preliminary patch that folds the body of pcie_flr()
into pcie_reset_flr() and adds:

  #define pcie_flr(dev)	pcie_reset_flr(dev, PCI_RESET_DO_RESET)

That would mean pcie_flr() users would start paying attention to
PCI_DEV_FLAGS_NO_FLR_RESET and PCI_EXP_DEVCAP_FLR, which seems like a
good thing.