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

[Alex Williamson <alex@shazbot.org>]

On Tue, 10 Feb 2026 17:12:13 -0700
Keith Busch <kbusch@kernel.org> wrote:

> On Tue, Feb 10, 2026 at 04:46:23PM -0700, Alex Williamson wrote:
> > On Thu, 5 Feb 2026 13:25:32 -0800
> > Keith Busch <kbusch@meta.com> wrote:
> >   
> > > From: Keith Busch <kbusch@kernel.org>
> > > 
> > > The Linux pci driver resolves a "slot" to the "D" in the B:D.f (see
> > > PCI_SLOT()). A pcie "slot reset" is a secondary bus reset, which affects
> > > every function on every "D", not just the ones with a matching "slot".
> > > The slot lock/unlock and save/restore functions, however, are only
> > > handling a subset of the functions, breaking the rest.  
> > 
> > So are we deprecating conventional PCI hotplug slots?    
> 
> 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.

> > 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.
 
> > 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()?
Thanks,

Alex