Re: [PATCH v3 0/6] powerpc/powernv/pci: Discover surprise-hotplugged PCIe devices during rescan

Hello Oliver,

On 9/12/18 12:49 PM, Oliver wrote:

On Tue, Sep 11, 2018 at 9:56 PM, Sergey Miroshnichenko
[off-list ref] wrote:

quoted

This patchset allows hotplugged PCIe devices to be enumerated during a bus
rescan being issued via sysfs on PowerNV platforms, when the "Presence
Detect Changed" interrupt is not available.

Seems to be on par with the sysfs slot power hack that pnv_php uses.

Yes, ours is just for manual initiation of rescan, which helps us with
reliable detection of a bridge hotplug in our particular config.

quoted

As a first part of our work on adding support for hotplugging PCIe bridges
full of devices (without special requirement such as Hot-Plug Controller,
reservation of bus numbers and memory regions by firmware, etc.), this
serie is intended to solve the first two problems of the listed below:

I   PowerNV doesn't discover new hotplugged PCIe devices
II  EEH is falsely triggered when poking empty slots during the PCIe rescan

We avoid this problem in pnv_php by having OPAL to do the rescan and
Linux requests
a FDT fragment of everything under the slot. I'm don't think it's a
great system, but
it keeps firmware and the OS on the same page.

So if we re-enumerate the PCIe topology from the Linux, we must then
synchronize with the firmware? How would you recommend to approach that
for PowerNV and OPAL? Can we can find somewhere a list of criteria to
ensure that they are properly synced?

quoted

III The PCI subsystem is not prepared to runtime changes of BAR addresses
IV  Device drivers don't track changes of their BAR addresses
V   BARs of working devices don't move to make space for new ones

I'm having a really hard to figuring out what would make this
necessary. Keep in mind
that each PHB has it's own set of bus numbers and it's own MMIO space,
so it's not
like you're short on either.

How are you planning on making this sort of live-device-migration work? And what
are you trying to do that makes the added complexity worth it?

With the "pci=realloc" command line argument and with the
PCI_REASSIGN_ALL_BUS flag the kernel doesn't rely on values of bus
numbers and BAR addresses provided by a firmware (OPAL via FDT in our
case, BIOS/UEFI/Coreboot for x86_64), but re-enumerates the PCIe
topology by its own means, and it arranges BARs quite compactly.

Let's say we have two bridges plugged into neighboring ports of the
root/PHB, each of them have a few NVME drives inserted and several empty
slots, when the system boots. Linux makes their bridge windows adjacent,
so if we plug in a new NVME into the first of them, there will be just
no free space to put its BARs.

Without considering memory pre-allocation, the only way we see to free
some space for new BARs is to move existing BARs of the second bridge
(in this example).

We've implemented a "firmware-independent" proof-of-concept (not
flawless, though, as you and Ben pointed out) and verified on
PowerNV+OPAL and x86_64 that a running NVME with an ongoing "fio"
benchmark always survives BAR movement during hotplug - of course after
applying a patch that pauses the NVME Linux driver during rescan. The
only visible effect is a bandwidth temporary drops to 0 for a second or
two, until NVME restarts. The same for a network adapter - an SSH
connection just freezes for a while.

This patchset is a first part of our work, and we've just published [1]
a second part (on BAR movement and pausing the drivers) for the
community to review, discuss and validate.

[1] https://www.spinics.net/lists/linux-pci/msg76211.html

Best regards,
Serge

quoted

Tested on:
 - POWER8 PowerNV+OPAL ppc64le (our Vesnin server) w/ and w/o pci=realloc;
 - POWER8 IBM 8247-42L (pSeries);
 - POWER8 IBM 8247-42L (PowerNV+OPAL) w/ and w/o pci=realloc.

Changes since v2:
 - Don't reassign bus numbers on PowerNV by default (to retain the default
   behavior), but only when pci=realloc is passed;
 - Less code affected;
 - pci_add_device_node_info is refactored with add_one_dev_pci_data;
 - Minor code cleanup.

Changes since v1:
 - Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
 - Fixed build for ppc64e when CONFIG_EEH is disabled;
 - Fixed code style warnings.

Sergey Miroshnichenko (6):
  powerpc/pci: Access PCI config space directly w/o pci_dn
  powerpc/pci: Create pci_dn on demand
  powerpc/pci: Use DT to create pci_dn for root bridges only
  powerpc/powernv/pci: Enable reassigning the bus numbers
  PCI/powerpc/eeh: Add pcibios hooks for preparing to rescan
  powerpc/pci: Reduce code duplication in pci_add_device_node_info

 arch/powerpc/include/asm/eeh.h               |   2 +
 arch/powerpc/kernel/eeh.c                    |  12 ++
 arch/powerpc/kernel/pci_dn.c                 | 119 ++++++++++++++-----
 arch/powerpc/kernel/rtas_pci.c               |  97 ++++++++++-----
 arch/powerpc/platforms/powernv/eeh-powernv.c |  22 ++++
 arch/powerpc/platforms/powernv/pci.c         |  64 ++++++----
 drivers/pci/probe.c                          |  14 +++
 include/linux/pci.h                          |   2 +
 8 files changed, 253 insertions(+), 79 deletions(-)

--
2.17.1