Re: [PATCHv3 3/4] pci: remove slot specific lock/unlock and save/restore

[Keith Busch <kbusch@kernel.org>]

On Wed, Feb 11, 2026 at 08:22:29AM -0700, Alex Williamson wrote:
> On Tue, 10 Feb 2026 17:12:13 -0700
> Keith Busch <kbusch@kernel.org> wrote:
> > No, this still calls the slot's specific reset callback via
> > pci_reset_hotplug_slot. The only thing this does is add locking, config
> > state save/restore, and driver reset prepare/complete calls for the
> > entire bus rather than matching "slots" on that bus. In the worst case
> > scenario, this patch has the kernel do a little more work than it needed
> > to.
> 
> But with this change, when vfio-pci calls pci_probe_reset_slot() we
> test pci_bus_resettable() rather than pci_slot_resettable().  The
> former is a superset of the latter.  Another slot on the same bus might
> have a bridge card or quirked device that previously wouldn't have
> affected the ability to reset a separate physical, conventional slot.
> The risk is small, but the semantics are different.

Thanks, good point.
 
> > > I've always
> > > understood these to be a subset of the bus, where we find the devices
> > > sharing the same physical PCI slot by comparing the pci_dev.slot across
> > > the bus, as done in all the code removed here.  pci_create_slot()
> > > certainly is not simply a reflection of PCI_SLOT().    
> > 
> > pci_create_slot assigns devices that match PCI_SLOT() of the bus's
> > devices, so it certainly is a reflection of that, no? The problem is
> > that PCI_SLOT() macro doesn't actually reflect the true nature of what
> > devices belong in the slot: the devices in a particular "slot" does not
> > define the actual blast radius of resetting that "slot".
> 
> struct pci_slot, the thing created by pci_create_slot(), represents a
> physical slot.  PCI_SLOT() is the bus addressing.  Not all pci_devs
> have a struct pci_slot, not all pci_devs within the same physical slot
> share the same B:D.F slot number (with ARI).
> 
> The vfio code does assume that 'slot' defines the blast radius of a
> pci_slot_reset() and I'm confused how it should operate if we're now
> going to conflate slot and bus reset scopes.

Yes, the "struct pci_slot" is that represetation. The problem is that
its current form is missing semantics to bind to functions that don't
match PCI_SLOT(). I don't know why PCI_SLOT is that criteria, but it's
clearly the wrong criteria for pciehp, probably others too if they
support ARI devices. The previous patch in this series creates entirely
new semantics specifically for pciehp and fixes that issue. It also
makes this patch unnecessary, but *only* for pciehp.

> > > Is the error below
> > > really a reflection that pci_dev.slot isn't carried over to the full
> > > set of functions?  
> > 
> > Correct, the pci slot today in a pciehp hotplug slot is not getting
> > assigned to all the functions in that slot, so slot resets completely
> > miss handling those functions, which inevitably break them.
> 
> Isn't this then a pciehp hotplug issue that all children of the bus
> should be linked via the struct pci_slot regardless of PCI_SLOT()?

It's not really a pciehp issue because pci_slot doesn't make it possible
to do anything else. See [PATCH 2/4] from this series.

In light of these concerns, I think the safest way forward is to drop
this patch and just enable something like what patch 2 provides.