On Fri, Feb 7, 2020 at 2:35 PM Oliver O'Halloran <ooh...@gmail.com> wrote: > > On Fri, Feb 7, 2020 at 1:24 PM Sam Bobroff <sbobr...@linux.ibm.com> wrote: > > > > On Mon, Feb 03, 2020 at 07:35:20PM +1100, Oliver O'Halloran wrote: > > > The eeh_ops->probe() function is called from two different contexts: > > > > > > 1. On pseries, where set set EEH_PROBE_MODE_DEVTREE, it's called in > > "set set" -> "we set" > > > eeh_add_device_early() which is supposed to run before we create > > > a pci_dev. > > > > > > 2. On PowerNV, where we set EEH_PROBE_MODE_DEV, it's called in > > > eeh_device_add_late() which is supposed to run *after* the > > > pci_dev is created. > > > > > > The "early" probe is required because PAPR requires that we perform an > > > RTAS > > > call to enable EEH support on a device before we start interacting with it > > > via config space or MMIO. This requirement doesn't exist on PowerNV and > > > shoehorning two completely separate initialisation paths into a common > > > interface just results in a convoluted code everywhere. > > > > > > Additionally the early probe requires the probe function to take an pci_dn > > > rather than a pci_dev argument. We'd like to make pci_dn a pseries > > > specific > > > data structure since there's no real requirement for them on PowerNV. To > > > help both goals move the early probe into the pseries containment zone > > > so the platform depedence is more explicit. > > > > > I had a look around near your comment: > > > + // XXX: uh, do we have the rescan lock held here? > > And we definitely don't have the lock when it gets called via the module > > init path (as rpaphp is loaded) -- I tried it and there was no deadlock. > > I don't think we have the lock in other situations but I haven't > > unravelled it all enough yet to tell, either. > > The other hotplug drivers seem to be taking the lock manually in their > enable_slot() callback. So I guess we need to be doing it there too. > I'll fix it in another patch since this one is a bit big.
On closer inspection I think we'll need to have a deeper look at this. This function isn't used for operations on the hotplug slot. Instead it's used for DLPAR operations including adding / removing whole PHBs. There doesn't appear to be any code in the DLPAR add / remove paths which takes the PCI rescan / remove lock so I think we'll need to have a careful look at what's going on there. Great stuff...
