Re: [PATCH v5 02/18] PCI/CXL: Probe the underlying bus reset in cxl_reset_bus_function()

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Aug 27, 2026 at 09:00:51PM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 27, 2026 at 01:49:05PM -0700, Nicolin Chen wrote:
> > On Thu, Aug 27, 2026 at 05:31:31PM -0300, Jason Gunthorpe wrote:
> > > > [ ... 27 lines skipped ... ]
> > > > @@ -4979,8 +4979,16 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
> > > >  	if (!dvsec)
> > > >  		return -ENOTTY;
> > > >  
> > > > -	if (probe)
> > > > -		return 0;
> > > > +	/*
> > > > +	 * Do not probe via pci_reset_bus_function(), which would reject a
> > > > +	 * masked SBR that the do-reset path below unmasks before resetting.
> > > > +	 */
> > > > +	if (probe) {
> > > > +		rc = pci_dev_reset_slot_function(dev, PCI_RESET_PROBE);
> > > > +		if (rc != -ENOTTY)
> > > > +			return rc;
> > > > +		return pci_parent_bus_reset(dev, PCI_RESET_PROBE);
> > > > +	}
> > > 
> > > Why not put this a little lower after the:
> > > 
> > > >
> > > >  	rc = pci_read_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL, &reg);
> > > >  	if (rc)
> > > 
> > > ?
> > 
> > Hmm, the "if (probe)" was checked before pci_read_config_word, so I
> > kept it there. I don't quite get the reason of putting it behind.
> > 
> > Mind elaborating?
> 
> I'm looking at the ENOTTY:
> 
> 	rc = pci_read_config_word(bridge, dvsec + PCI_DVSEC_CXL_PORT_CTL, &reg);
> 	if (rc)
> 		return -ENOTTY;
> 
> If the general point with these fixes was probe should capture all the
> ENOTTY cases under probe there should not be an ENOTTY return outside
> probe.
> 
> So maybe that should be ENODEV, or maybe probe should check it. I'm
> not really sure how config read can fail, so maybe ENODEV is the right
> direction

I think returning -ENOTTY is somewhat correct here.

-ENOTTY has two slightly different meanings in the PCI reset layer:
 1. This reset method does not support the device, try next method
 2. This reset method failed, but no actual HW reset was asserted

Either 1 or 2 seems a fit for this case, although worth mentioning
for 1, changing to -ENODEV doesn't change the behavior for now, as
cxl_reset_bus_function is the last function in the method[] array.
But it will be different once some new method is appended to the
reset array.

On the other hand, I see the probe-passing reset functions define:
 * @probe: if true, return 0 if the device can be reset this way.
And read failure can say that the device cannot be reset this way.

So, though part of me still lean towards leaving it as-is, I think
moving is probably the conservative correct thing here. It aligns
the probe path closer to the !probe path by a harmless read.

Thanks
Nicolin