Re: [PATCH] PCI: pciehp: Differentiate between surprise and safe removal

[Lukas Wunner <lukas@wunner.de>]

On Wed, Aug 01, 2018 at 07:43:58PM +0300, Mika Westerberg wrote:
> On Tue, Jul 31, 2018 at 07:50:37AM +0200, Lukas Wunner wrote:
> > -static void remove_board(struct slot *p_slot)
> > +static void remove_board(struct slot *p_slot, bool safe_removal)
> >  {
> >  	struct controller *ctrl = p_slot->ctrl;
> >  
> > -	pciehp_unconfigure_device(p_slot);
> > +	pciehp_unconfigure_device(p_slot, safe_removal);
> 
> Below we turn off power to the slot if it has power controller. Even if
> we disable slot from sysfs, I think it ends up being inaccessible after
> power is turned off. I wonder if we should mark the devices disconnected
> in that case as well?
> 
> >  
> >  	if (POWER_CTRL(ctrl)) {
> >  		pciehp_power_off_slot(p_slot);

No, when pciehp_unconfigure_device() returns, the PCI devices below
the hotplug bridge are unbound and removed from the system.  They're
gone, so the bit set in their pci_dev struct would no longer be
accessible anyway.  Unless of course something is holding a ref on
the pci_dev, but that would seem to be a bug.  (Accessing a device
that's already removed from the system, that is.)

Calling pci_dev_set_disconnected() only gives the PCI core and the
driver bound to the device an indication that's it's inaccessible,
so any code paths during unbound and PCI device teardown can skip
accesses.  (Because pci_dev_is_disconnected() is currently scoped
to the PCI core, the disconnected status can only be queried from
drivers that live in the PCI core, such as portdrv and all the
port services drivers.)  After the pci_dev is removed from the
hierarchy, accessing it seems at least questionable.

Does this make things clearer?  Shout if it not. :-)

Thanks,

Lukas