On Fri, Jul 27, 2018 at 03:52:04PM +0000, Alex_Gagniuc@xxxxxxxxxxxx wrote: > On 07/27/2018 02:18 AM, Lukas Wunner wrote: > > On Thu, Jul 26, 2018 at 05:38:50PM -0500, Alex G. wrote: > >> I was under the impression that a DLLSC or PDSC would trigger the > >> PCI_DEV_DISCONNECTED bit to be set, blocking any further config access. > > > > Only if the Presence Detect State bit in the Slot Status register is > > not set. > > > > I think the idea was that if the card is still in the slot, its driver can > > be unbound orderly because the device is still accessible. So there's no > > need to set PCI_DEV_DISCONNECTED. > > This sounds counter to what I had intuited. I mentally associate > DISCONNECTED with "the link is down". I don't understand the idea of a > driver doing an orderly removal when the link is down. Device registers > wouldn't be accessible in that case. That's basically what I meant, sorry for not being clear. > > However if there is a already a new card in the slot, PCI_DEV_DISCONNECTED > > will erroneously not be set. Also, if card removal was triggered by the > > link going down but the card is still in the slot, PCI_DEV_DISCONNECTED > > will also erroneously not be set even though it's inaccessible. > > > > The only situations when PCI_DEV_DISCONNECTED should not be set is if the > > card is being removed by sysfs or by the user pressing the Attention Button. > > Anything else is a surprise removal. What we need to do is pass down a > > flag to pciehp_unconfigure_device() to indicate whether one of those two > > situations is at hand or not, and PCI_DEV_DISCONNECTED should be set > > depending on that flag. > > That makes a little more sense. Is someone working on this? Me, but not for 4.19, we're too late in the cycle, I'm going to post a small number of fixup patches for Bjorn's pci/hotplug branch shortly, to be included in 4.19, and that's it. I posted a patch as part of the series that's now on Bjorn's pci/hotplug branch which touched PCI_DEV_DISCONNECTED code in pciehp_pci.c, but had to withdraw that particular patch: https://patchwork.ozlabs.org/patch/930403/ The first problem with that patch was that I hadn't fully understood yet when to set PCI_DEV_DISCONNECTED and when not to set it. The second problem was that it fixed a deadlock on unplug in a way that wasn't generic enough. The same deadlock can occur in other situations. The real fix is to unbind the driver lockless in pci_stop_dev(). That's non-trivial to achieve, but doable. I don't think there's a way around that to fix the problem: https://www.spinics.net/lists/linux-pci/msg73652.html There's (still) a lot that can be improved in pciehp, I hope to keep working on that as time permits. Thanks, Lukas
