Nothing but pedantic spelling and grammar nits of the commit log follow. -Tyrel On 11/13/19 1:40 AM, Oliver O'Halloran wrote: > On PowerNV the PCIe topology is (currently) managed the powernv platform s/the powernv/by the powernv > code in cooperation with firmware. The PCIe-native service drivers bypass > both and this can cause problems. > > Historically this hasn't been a big deal since the only port service > driver that saw much use was the AER driver. The AER driver relies > a kernel service to report when errors occur rather than acting autonmously s/a kernel/on a kernel autonomously > so it's fairly easy to ignore. On PowerNV (and pseries) AER events are > handled through EEH, which ignores the AER service, so it's never been > an issue. > > Unfortunately, the hotplug port service driver (pciehp) does act > autonomously and conflicts with the platform specific hotplug > driver (pnv_php). The main issue is that pciehp claims the interrupt > associated with the PCIe capability which in turn prevents pnv_php from > claiming it. > > This results in hotplug events being handled by pciehp which does not > notify firmware when the PCIe topology changes, and does not setup/teardown > the arch specific PCI device structures (pci_dn) when the topology changes. > The end result is that hot-added devices cannot be enabled and hot-removed > devices may not be fully torn-down on removal. > > We can fix these problems by setting the "pcie_ports_disabled" flag during > platform initialisation. The flag indicates the platform owns the PCIe > ports which stops the portbus driver being registered. s/being/from being > > Cc: Sergey Miroshnichenko <s.miroshnichenko@xxxxxxxxx> > Fixes: 66725152fb9f ("PCI/hotplug: PowerPC PowerNV PCI hotplug driver") > Signed-off-by: Oliver O'Halloran <oohall@xxxxxxxxx> > --- > Sergey, just FYI. I'll try sort out the rest of the hotplug > trainwreck in 5.6. > > The Fixes: here is for the patch that added pnv_php in 4.8. It's been > a problem since then, but wasn't noticed until people started testing > it after the EEH fixes in commit 799abe283e51 ("powerpc/eeh: Clean up > EEH PEs after recovery finishes") went in earlier in the 5.4 cycle. > --- > arch/powerpc/platforms/powernv/pci.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c > index 2825d00..ae62583 100644 > --- a/arch/powerpc/platforms/powernv/pci.c > +++ b/arch/powerpc/platforms/powernv/pci.c > @@ -941,6 +941,23 @@ void __init pnv_pci_init(void) > > pci_add_flags(PCI_CAN_SKIP_ISA_ALIGN); > > +#ifdef CONFIG_PCIEPORTBUS > + /* > + * On PowerNV PCIe devices are (currently) managed in cooperation > + * with firmware. This isn't *strictly* required, but there's enough > + * assumptions baked into both firmware and the platform code that > + * it's unwise to allow the portbus services to be used. > + * > + * We need to fix this eventually, but for now set this flag to disable > + * the portbus driver. The AER service isn't required since that AER > + * events are handled via EEH. The pciehp hotplug driver can't work > + * without kernel changes (and portbus binding breaks pnv_php). The > + * other services also require some thinking about how we're going > + * to integrate them. > + */ > + pcie_ports_disabled = true; > +#endif > + > /* If we don't have OPAL, eg. in sim, just skip PCI probe */ > if (!firmware_has_feature(FW_FEATURE_OPAL)) > return; >