Re: [PATCH v5 03/18] PCI: Propagate FLR return values to callers
From: Nicolin Chen
Date: Fri Aug 28 2026 - 00:00:27 EST
On Thu, Aug 27, 2026 at 09:02:51PM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 27, 2026 at 02:18:08PM -0700, Nicolin Chen wrote:
> > On Thu, Aug 27, 2026 at 05:31:31PM -0300, Jason Gunthorpe wrote:
> > > > [ ... 24 lines skipped ... ]
> > > > Since these quirks have always returned success, escalating
> > > > would be a new and harmful behavior: the next method is typically the plain
> > > > "flr" method, which would re-run the same FLR without the extra steps the
> > > > quirk wraps around its pcie_flr() call. Add quirk_flr_err() converting that
> > > > -ENOTTY to -ETIMEDOUT, so the cascade still stops at the quirk as before.
> > > > The early "not applicable" -ENOTTY returns are left intact, and they still
> > > > ask the caller to try the next method.
> > >
> > > This seems like a very suspect AI conclusion to me.
> > >
> > > I think I'd drop this patch
> > >
> > > > Convert it at the quirk level rather than in pcie_flr() or pci_dev_wait(),
> > > > because those also serve the native "flr"/"af_flr"/"pm" methods, where the
> > > > -ENOTTY on timeout is deliberate per commit 91295d79d658 ("PCI: Handle FLR
> > > > failure and allow other reset types"): a timed-out native FLR escalates to
> > > > a stronger reset (e.g. bus reset), which skips no workaround there and may
> > > > recover the device. Converting in the core would also change the value seen
> > > > by every direct pcie_flr() caller.
> > > >
> > > > This is not a bug fix, since these functions have always returned success
> > > > and the propagated values are only consumed by incoming work.
> > >
> > > In what way? The quirks flows always succeed and it can keep always
> > > succeeding. If anyone finds this problematic they can properly
> > > propogate the error.
> > >
> > > Squashing the error just for quirks makes little sense.
> >
> > PATCH-5 forwards the return values to pci_dev_reset_iommu_done(),
> > so IOMMU can keep a device blocked on a nonzero result. Dropping
> > this patch would leave the three VF quirks here always reporting
> > success, reopening the stale-ATC issue this series is fixing for
> > them.
>
> Sure its fine,
Hmm, that paragraph was a justification for having this patch. But
given the context here, I assume you replied "sure" to my question:
"Or do you think it's okay?"
> I bet this HW doesn't even have ATC and if it does
> someone can properly fix the quirk and test it that way if they ever
> hit a problem.
OK. I can drop it, since we don't want to fix any return result in
the PCI reset layer.
Thanks
Nicolin