Tyrel Datwyler <tyreld@xxxxxxxxxxxxx> writes:
> Nothing but pedantic spelling and grammar nits of the commit log follow.
Thanks, they were annoying me :)
cheers
> 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;
>>
