| Message ID | 20200518182438.392269-1-frank.heimes@canonical.com |
|---|---|
| Headers | show |
| Series | s390x/pci: enumerate pci functions per physical adapter (LP: 1874056) | expand |
On 18.05.20 20:24, frank.heimes@canonical.com wrote: > Buglink: https://bugs.launchpad.net/bugs/1874056 > > SRU Justification: > > [Impact] Somehow the impact section should give a quick overview about the issue that one attempts to fix. Right now it sounds more like a nice to have which would not really be a reason to adapt code that much. Maybe if it were preventing to reliably pass virtual functions of physical adapters into VM guests. And that was part of the release and now is found not to work as promised... And if say that gets said in simple words in the bug reports justification, maybe this would raise enough interest in even looking at the patches... -Stefan > > * On s390x the enumeration of PCI functions does not reflect which functions belongs to which physical adapter. > > * Layout of a PCI function address on Linux: > 0000:00:00.0 > <root complex>:<bus>:<device>.<function> > > * On s390x, each function is presented as individual root complex today, e.g.: > PCHID 0100 VF1 0000:00:00.0 > PCHID 0100 VF23 0001:00:00.0 > PCHID 0200 VF1 0002:00:00.0 > OCHID 0100 VF17 0003:00:00.0 > > * On other platforms, the addresses correctly reflect the actual HW configuration. > > * Some device drivers (like mlx5, for Mellanox adapters) group functions of one physical adapter by checking which PCI functions have identical values for <root complex>:<bus>:<device>. > > * We need to use the same enumeration scheme to achieve this functionality on s390x. > > * In this case, the two physical functions of a Mellanox adapter need to get function number 0 and 1, > and all virtual functions need to use the same <root complex>:<bus> numbers with function/device numbers counting up. > > * Required result (example with 4 VFs per PF): > PCHID 0100 PF 0 0000:00:00.0 > PCHID 0100 PF 1 0000:00:00.1 > PCHID 0100 PF 0 VF 0 0000:00:00.2 > PCHID 0100 PF 0 VF 1 0000:00:00.3 > PCHID 0100 PF 0 VF 2 0000:00:00.4 > PCHID 0100 PF 0 VF 3 0000:00:00.5 > PCHID 0100 PF 1 VF 0 0000:00:00.6 > PCHID 0100 PF 1 VF 1 0000:00:00.7 > PCHID 0100 PF 1 VF 2 0000:00:00.8 > PCHID 0100 PF 1 VF 3 0000:00:00.9 > PCHID 0200 PF 0 0001:00:00.0 > > [Fix] > > * Backport 1: https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > * Backport 2: https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > * Backport 3: https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > * Backport 4: https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > * Backport 5: https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > * Backport 6: https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > * Backport 7: https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > * Backport 8: https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > * Backport 9: https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > * Backport 10: https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > * Backport 11: https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > * Backport 12: https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > [Test Case] > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more RoCE Express PCI 2(.1) adapters. > > * Assign the adapters (and it's virtual functions) to an LPAR. > > * Verify whether the physical and virtual functions are grouped in arbitrary order or in consecutive order - physical first (for example with lspci -t ...) > > [Regression Potential] > > * The regression potential can be considered as moderate, since: > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > * It largely affects zPCI, the s390x specific PCI code layer. > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not very wide-spread. > > * The situation described above affects the RoCE adapters only (Mellanox based). > > * The patches are also upstream accepted and available via linux-next, but to apply them to focal kernel 5.4 the above backports are needed. > > * However, the code is modified by several patches (12), hence there is a chance to break zPCI with them. > > * For upfront testing a PPA got created with a focal (master-next) kernel that incl. all the above patches. > > Alexander Schmidt (1): > s390/pci: Expose new port attribute for PCIe functions > > Niklas Schnelle (1): > s390/pci: Improve handling of unset UID > > Pierre Morel (10): > s390/pci: embedding hotplug_slot in zdev > s390/pci: adaptation of iommu to multifunction > s390/pci: define kernel parameters for PCI multifunction > s390/pci: define RID and RID available > s390/pci: create zPCI bus > s390/pci: adapt events for zbus > s390/pci: Handling multifunctions > s390/pci: Do not disable PF when VFs exist > s390/pci: Documentation for zPCI > s390/pci: removes wrong PCI multifunction assignment > > .../admin-guide/kernel-parameters.txt | 2 + > Documentation/s390/index.rst | 1 + > Documentation/s390/pci.rst | 126 +++++++++ > MAINTAINERS | 1 + > arch/s390/include/asm/pci.h | 45 +++- > arch/s390/include/asm/pci_clp.h | 12 +- > arch/s390/pci/Makefile | 3 +- > arch/s390/pci/pci.c | 198 ++++++-------- > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > arch/s390/pci/pci_bus.h | 31 +++ > arch/s390/pci/pci_clp.c | 6 +- > arch/s390/pci/pci_event.c | 39 ++- > arch/s390/pci/pci_sysfs.c | 4 +- > drivers/iommu/s390-iommu.c | 8 +- > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > 15 files changed, 618 insertions(+), 218 deletions(-) > create mode 100644 Documentation/s390/pci.rst > create mode 100644 arch/s390/pci/pci_bus.c > create mode 100644 arch/s390/pci/pci_bus.h >
After reaching out to the initial bug reporter and condensing the (probably too detailed and too long) description, I think this is a brief summary: I hope that this better fit's to the Impact section of the SRU Justification - I also updated the justification in the LP bug description. [Impact] * Mellanox CX5 port multi-pathing is broken on s390x due to non-standard topology of PCI IDs (phys. and virtual): * The Mellanox Connect-X 5 PCI driver (mlx5) implements multi-path that can be used to combine multiple networking ports to improve performance and reliability. * For that purpose, the mlx5 driver combines PCI functions based on topology information (the function number) as determined by their PCI ID. * Currently the Linux on Z PCI bus does not reflect PCI topology information in the PCI ID. As a result, the mlx5 multi-path function is broken and cannot be activated. On Tue, May 19, 2020 at 10:04 AM Stefan Bader <stefan.bader@canonical.com> wrote: > On 18.05.20 20:24, frank.heimes@canonical.com wrote: > > Buglink: https://bugs.launchpad.net/bugs/1874056 > > > > SRU Justification: > > > > [Impact] > > Somehow the impact section should give a quick overview about the issue > that one > attempts to fix. Right now it sounds more like a nice to have which would > not > really be a reason to adapt code that much. > > Maybe if it were preventing to reliably pass virtual functions of physical > adapters into VM guests. And that was part of the release and now is found > not > to work as promised... > > And if say that gets said in simple words in the bug reports justification, > maybe this would raise enough interest in even looking at the patches... > > -Stefan > > > > > * On s390x the enumeration of PCI functions does not reflect which > functions belongs to which physical adapter. > > > > * Layout of a PCI function address on Linux: > > 0000:00:00.0 > > <root complex>:<bus>:<device>.<function> > > > > * On s390x, each function is presented as individual root complex today, > e.g.: > > PCHID 0100 VF1 0000:00:00.0 > > PCHID 0100 VF23 0001:00:00.0 > > PCHID 0200 VF1 0002:00:00.0 > > OCHID 0100 VF17 0003:00:00.0 > > > > * On other platforms, the addresses correctly reflect the actual HW > configuration. > > > > * Some device drivers (like mlx5, for Mellanox adapters) group functions > of one physical adapter by checking which PCI functions have identical > values for <root complex>:<bus>:<device>. > > > > * We need to use the same enumeration scheme to achieve this > functionality on s390x. > > > > * In this case, the two physical functions of a Mellanox adapter need to > get function number 0 and 1, > > and all virtual functions need to use the same <root complex>:<bus> > numbers with function/device numbers counting up. > > > > * Required result (example with 4 VFs per PF): > > PCHID 0100 PF 0 0000:00:00.0 > > PCHID 0100 PF 1 0000:00:00.1 > > PCHID 0100 PF 0 VF 0 0000:00:00.2 > > PCHID 0100 PF 0 VF 1 0000:00:00.3 > > PCHID 0100 PF 0 VF 2 0000:00:00.4 > > PCHID 0100 PF 0 VF 3 0000:00:00.5 > > PCHID 0100 PF 1 VF 0 0000:00:00.6 > > PCHID 0100 PF 1 VF 1 0000:00:00.7 > > PCHID 0100 PF 1 VF 2 0000:00:00.8 > > PCHID 0100 PF 1 VF 3 0000:00:00.9 > > PCHID 0200 PF 0 0001:00:00.0 > > > > [Fix] > > > > * Backport 1: > https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > > > * Backport 2: > https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > > > * Backport 3: > https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > > > * Backport 4: > https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > > > * Backport 5: > https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > > > * Backport 6: > https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > > > * Backport 7: > https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > > > * Backport 8: > https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > > > * Backport 9: > https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > > > * Backport 10: > https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > > > * Backport 11: > https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > > > * Backport 12: > https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > > > [Test Case] > > > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more > RoCE Express PCI 2(.1) adapters. > > > > * Assign the adapters (and it's virtual functions) to an LPAR. > > > > * Verify whether the physical and virtual functions are grouped in > arbitrary order or in consecutive order - physical first (for example with > lspci -t ...) > > > > [Regression Potential] > > > > * The regression potential can be considered as moderate, since: > > > > * It is purely s390x specific code (arch/s390/* > drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and > some doc adjustments, too). > > > > * It largely affects zPCI, the s390x specific PCI code layer. > > > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and > not very wide-spread. > > > > * The situation described above affects the RoCE adapters only (Mellanox > based). > > > > * The patches are also upstream accepted and available via linux-next, > but to apply them to focal kernel 5.4 the above backports are needed. > > > > * However, the code is modified by several patches (12), hence there is > a chance to break zPCI with them. > > > > * For upfront testing a PPA got created with a focal (master-next) > kernel that incl. all the above patches. > > > > Alexander Schmidt (1): > > s390/pci: Expose new port attribute for PCIe functions > > > > Niklas Schnelle (1): > > s390/pci: Improve handling of unset UID > > > > Pierre Morel (10): > > s390/pci: embedding hotplug_slot in zdev > > s390/pci: adaptation of iommu to multifunction > > s390/pci: define kernel parameters for PCI multifunction > > s390/pci: define RID and RID available > > s390/pci: create zPCI bus > > s390/pci: adapt events for zbus > > s390/pci: Handling multifunctions > > s390/pci: Do not disable PF when VFs exist > > s390/pci: Documentation for zPCI > > s390/pci: removes wrong PCI multifunction assignment > > > > .../admin-guide/kernel-parameters.txt | 2 + > > Documentation/s390/index.rst | 1 + > > Documentation/s390/pci.rst | 126 +++++++++ > > MAINTAINERS | 1 + > > arch/s390/include/asm/pci.h | 45 +++- > > arch/s390/include/asm/pci_clp.h | 12 +- > > arch/s390/pci/Makefile | 3 +- > > arch/s390/pci/pci.c | 198 ++++++-------- > > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > > arch/s390/pci/pci_bus.h | 31 +++ > > arch/s390/pci/pci_clp.c | 6 +- > > arch/s390/pci/pci_event.c | 39 ++- > > arch/s390/pci/pci_sysfs.c | 4 +- > > drivers/iommu/s390-iommu.c | 8 +- > > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > > 15 files changed, 618 insertions(+), 218 deletions(-) > > create mode 100644 Documentation/s390/pci.rst > > create mode 100644 arch/s390/pci/pci_bus.c > > create mode 100644 arch/s390/pci/pci_bus.h > > > > >
On 19.05.20 11:55, Frank Heimes wrote: > After reaching out to the initial bug reporter and condensing the (probably too > detailed and too long) description, I think this is a brief summary: > I hope that this better fit's to the Impact section of the SRU Justification - I > also updated the justification in the LP bug description. > > [Impact] > > * Mellanox CX5 port multi-pathing is broken on s390x due to non-standard > topology of PCI IDs (phys. and virtual): > > * The Mellanox Connect-X 5 PCI driver (mlx5) implements multi-path that can be > used to combine multiple networking ports to improve performance and reliability. > > * For that purpose, the mlx5 driver combines PCI functions based on topology > information (the function number) as determined by their PCI ID. > > * Currently the Linux on Z PCI bus does not reflect PCI topology information in > the PCI ID. As a result, the mlx5 multi-path function is broken and cannot be > activated. > > > > On Tue, May 19, 2020 at 10:04 AM Stefan Bader <stefan.bader@canonical.com > <mailto:stefan.bader@canonical.com>> wrote: > > On 18.05.20 20:24, frank.heimes@canonical.com > <mailto:frank.heimes@canonical.com> wrote: > > Buglink: https://bugs.launchpad.net/bugs/1874056 > > > > SRU Justification: > > > > [Impact] > > Somehow the impact section should give a quick overview about the issue that one > attempts to fix. Right now it sounds more like a nice to have which would not > really be a reason to adapt code that much. > > Maybe if it were preventing to reliably pass virtual functions of physical > adapters into VM guests. And that was part of the release and now is found not > to work as promised... > > And if say that gets said in simple words in the bug reports justification, > maybe this would raise enough interest in even looking at the patches... > > -Stefan > > > > > * On s390x the enumeration of PCI functions does not reflect which > functions belongs to which physical adapter. > > > > * Layout of a PCI function address on Linux: > > 0000:00:00.0 > > <root complex>:<bus>:<device>.<function> > > > > * On s390x, each function is presented as individual root complex today, e.g.: > > PCHID 0100 VF1 0000:00:00.0 > > PCHID 0100 VF23 0001:00:00.0 > > PCHID 0200 VF1 0002:00:00.0 > > OCHID 0100 VF17 0003:00:00.0 > > > > * On other platforms, the addresses correctly reflect the actual HW > configuration. > > > > * Some device drivers (like mlx5, for Mellanox adapters) group functions > of one physical adapter by checking which PCI functions have identical > values for <root complex>:<bus>:<device>. > > > > * We need to use the same enumeration scheme to achieve this functionality > on s390x. > > > > * In this case, the two physical functions of a Mellanox adapter need to > get function number 0 and 1, > > and all virtual functions need to use the same <root complex>:<bus> > numbers with function/device numbers counting up. > > > > * Required result (example with 4 VFs per PF): > > PCHID 0100 PF 0 0000:00:00.0 > > PCHID 0100 PF 1 0000:00:00.1 > > PCHID 0100 PF 0 VF 0 0000:00:00.2 > > PCHID 0100 PF 0 VF 1 0000:00:00.3 > > PCHID 0100 PF 0 VF 2 0000:00:00.4 > > PCHID 0100 PF 0 VF 3 0000:00:00.5 > > PCHID 0100 PF 1 VF 0 0000:00:00.6 > > PCHID 0100 PF 1 VF 1 0000:00:00.7 > > PCHID 0100 PF 1 VF 2 0000:00:00.8 > > PCHID 0100 PF 1 VF 3 0000:00:00.9 > > PCHID 0200 PF 0 0001:00:00.0 > > > > [Fix] > > > > * Backport 1: > https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > > > * Backport 2: > https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > > > * Backport 3: > https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > > > * Backport 4: > https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > > > * Backport 5: > https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > > > * Backport 6: > https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > > > * Backport 7: > https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > > > * Backport 8: > https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > > > * Backport 9: > https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > > > * Backport 10: > https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > > > * Backport 11: > https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > > > * Backport 12: > https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > > > [Test Case] > > > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more > RoCE Express PCI 2(.1) adapters. > > > > * Assign the adapters (and it's virtual functions) to an LPAR. > > > > * Verify whether the physical and virtual functions are grouped in > arbitrary order or in consecutive order - physical first (for example with > lspci -t ...) > > > > [Regression Potential] > > > > * The regression potential can be considered as moderate, since: > > > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c > and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > > > * It largely affects zPCI, the s390x specific PCI code layer. > > > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not > very wide-spread. > > > > * The situation described above affects the RoCE adapters only (Mellanox > based). > > > > * The patches are also upstream accepted and available via linux-next, but > to apply them to focal kernel 5.4 the above backports are needed. > > > > * However, the code is modified by several patches (12), hence there is a > chance to break zPCI with them. > > > > * For upfront testing a PPA got created with a focal (master-next) kernel > that incl. all the above patches. > > > > Alexander Schmidt (1): > > s390/pci: Expose new port attribute for PCIe functions > > > > Niklas Schnelle (1): > > s390/pci: Improve handling of unset UID > > > > Pierre Morel (10): > > s390/pci: embedding hotplug_slot in zdev > > s390/pci: adaptation of iommu to multifunction > > s390/pci: define kernel parameters for PCI multifunction > > s390/pci: define RID and RID available > > s390/pci: create zPCI bus > > s390/pci: adapt events for zbus > > s390/pci: Handling multifunctions > > s390/pci: Do not disable PF when VFs exist > > s390/pci: Documentation for zPCI > > s390/pci: removes wrong PCI multifunction assignment > > > > .../admin-guide/kernel-parameters.txt | 2 + > > Documentation/s390/index.rst | 1 + > > Documentation/s390/pci.rst | 126 +++++++++ > > MAINTAINERS | 1 + > > arch/s390/include/asm/pci.h | 45 +++- > > arch/s390/include/asm/pci_clp.h | 12 +- > > arch/s390/pci/Makefile | 3 +- > > arch/s390/pci/pci.c | 198 ++++++-------- > > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > > arch/s390/pci/pci_bus.h | 31 +++ > > arch/s390/pci/pci_clp.c | 6 +- > > arch/s390/pci/pci_event.c | 39 ++- > > arch/s390/pci/pci_sysfs.c | 4 +- > > drivers/iommu/s390-iommu.c | 8 +- > > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > > 15 files changed, 618 insertions(+), 218 deletions(-) > > create mode 100644 Documentation/s390/pci.rst > > create mode 100644 arch/s390/pci/pci_bus.c > > create mode 100644 arch/s390/pci/pci_bus.h > > > > Ok, the justification looks better now. I did glance over the patches but more than checking for changes being contains in the arch specific code is not really doable. I saw that a test kernel was provided, so I would wait for feedback there before considering further. One ask for you: if you could provide a git branch we can pull the set from, this would help a lot. Generally I would say if a set consists of more than say 5 patches, a pull request is preferred. Thanks, Stefan
Okay, the backports cam now also be pulled from: lp:~fheimes/+git/lp1874056 (https://code.launchpad.net/~fheimes/+git/lp1874056) the last 12 commit from 94b950790675 to head ee26de93bdf7 (both including) $ git log --pretty=oneline --abbrev-commit --decorate -n 12 ee26de93bdf7 (HEAD -> master-next, origin/master-next) s390/pci: removes wrong PCI multifunction assignment 59ba5a8b7686 s390/pci: Documentation for zPCI 408c98b74cf3 s390/pci: Do not disable PF when VFs exist 4f50a99d3b34 s390/pci: Handling multifunctions aeaa922a01a9 s390/pci: adapt events for zbus 377ba01b5397 s390/pci: create zPCI bus 90d84dc73178 s390/pci: define RID and RID available 45f454285a80 s390/pci: define kernel parameters for PCI multifunction 0ce1bf788d55 s390/pci: adaptation of iommu to multifunction fb5462e5425c s390/pci: Expose new port attribute for PCIe functions 3f8cde5caa2f s390/pci: embedding hotplug_slot in zdev 94b950790675 s390/pci: Improve handling of unset UID In between IBM successfully verified the modifications based on the PPA that I've created. Thx, Frank On Wed, May 20, 2020 at 9:39 AM Stefan Bader <stefan.bader@canonical.com> wrote: > On 19.05.20 11:55, Frank Heimes wrote: > > After reaching out to the initial bug reporter and condensing the > (probably too > > detailed and too long) description, I think this is a brief summary: > > I hope that this better fit's to the Impact section of the SRU > Justification - I > > also updated the justification in the LP bug description. > > > > [Impact] > > > > * Mellanox CX5 port multi-pathing is broken on s390x due to non-standard > > topology of PCI IDs (phys. and virtual): > > > > * The Mellanox Connect-X 5 PCI driver (mlx5) implements multi-path that > can be > > used to combine multiple networking ports to improve performance and > reliability. > > > > * For that purpose, the mlx5 driver combines PCI functions based on > topology > > information (the function number) as determined by their PCI ID. > > > > * Currently the Linux on Z PCI bus does not reflect PCI topology > information in > > the PCI ID. As a result, the mlx5 multi-path function is broken and > cannot be > > activated. > > > > > > > > On Tue, May 19, 2020 at 10:04 AM Stefan Bader < > stefan.bader@canonical.com > > <mailto:stefan.bader@canonical.com>> wrote: > > > > On 18.05.20 20:24, frank.heimes@canonical.com > > <mailto:frank.heimes@canonical.com> wrote: > > > Buglink: https://bugs.launchpad.net/bugs/1874056 > > > > > > SRU Justification: > > > > > > [Impact] > > > > Somehow the impact section should give a quick overview about the > issue that one > > attempts to fix. Right now it sounds more like a nice to have which > would not > > really be a reason to adapt code that much. > > > > Maybe if it were preventing to reliably pass virtual functions of > physical > > adapters into VM guests. And that was part of the release and now is > found not > > to work as promised... > > > > And if say that gets said in simple words in the bug reports > justification, > > maybe this would raise enough interest in even looking at the > patches... > > > > -Stefan > > > > > > > > * On s390x the enumeration of PCI functions does not reflect which > > functions belongs to which physical adapter. > > > > > > * Layout of a PCI function address on Linux: > > > 0000:00:00.0 > > > <root complex>:<bus>:<device>.<function> > > > > > > * On s390x, each function is presented as individual root complex > today, e.g.: > > > PCHID 0100 VF1 0000:00:00.0 > > > PCHID 0100 VF23 0001:00:00.0 > > > PCHID 0200 VF1 0002:00:00.0 > > > OCHID 0100 VF17 0003:00:00.0 > > > > > > * On other platforms, the addresses correctly reflect the actual HW > > configuration. > > > > > > * Some device drivers (like mlx5, for Mellanox adapters) group > functions > > of one physical adapter by checking which PCI functions have > identical > > values for <root complex>:<bus>:<device>. > > > > > > * We need to use the same enumeration scheme to achieve this > functionality > > on s390x. > > > > > > * In this case, the two physical functions of a Mellanox adapter > need to > > get function number 0 and 1, > > > and all virtual functions need to use the same <root > complex>:<bus> > > numbers with function/device numbers counting up. > > > > > > * Required result (example with 4 VFs per PF): > > > PCHID 0100 PF 0 0000:00:00.0 > > > PCHID 0100 PF 1 0000:00:00.1 > > > PCHID 0100 PF 0 VF 0 0000:00:00.2 > > > PCHID 0100 PF 0 VF 1 0000:00:00.3 > > > PCHID 0100 PF 0 VF 2 0000:00:00.4 > > > PCHID 0100 PF 0 VF 3 0000:00:00.5 > > > PCHID 0100 PF 1 VF 0 0000:00:00.6 > > > PCHID 0100 PF 1 VF 1 0000:00:00.7 > > > PCHID 0100 PF 1 VF 2 0000:00:00.8 > > > PCHID 0100 PF 1 VF 3 0000:00:00.9 > > > PCHID 0200 PF 0 0001:00:00.0 > > > > > > [Fix] > > > > > > * Backport 1: > > > https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > > > > > * Backport 2: > > > https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > > > > > * Backport 3: > > > https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > > > > > * Backport 4: > > > https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > > > > > * Backport 5: > > > https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > > > > > * Backport 6: > > > https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > > > > > * Backport 7: > > > https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > > > > > * Backport 8: > > > https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > > > > > * Backport 9: > > > https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > > > > > * Backport 10: > > > https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > > > > > * Backport 11: > > > https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > > > > > * Backport 12: > > > https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > > > > > [Test Case] > > > > > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or > more > > RoCE Express PCI 2(.1) adapters. > > > > > > * Assign the adapters (and it's virtual functions) to an LPAR. > > > > > > * Verify whether the physical and virtual functions are grouped in > > arbitrary order or in consecutive order - physical first (for > example with > > lspci -t ...) > > > > > > [Regression Potential] > > > > > > * The regression potential can be considered as moderate, since: > > > > > > * It is purely s390x specific code (arch/s390/* > drivers/iommu/s390-iommu.c > > and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, > too). > > > > > > * It largely affects zPCI, the s390x specific PCI code layer. > > > > > > * PCI cards available for s390x are optional cards (RoCE and zEDC) > and not > > very wide-spread. > > > > > > * The situation described above affects the RoCE adapters only > (Mellanox > > based). > > > > > > * The patches are also upstream accepted and available via > linux-next, but > > to apply them to focal kernel 5.4 the above backports are needed. > > > > > > * However, the code is modified by several patches (12), hence > there is a > > chance to break zPCI with them. > > > > > > * For upfront testing a PPA got created with a focal (master-next) > kernel > > that incl. all the above patches. > > > > > > Alexander Schmidt (1): > > > s390/pci: Expose new port attribute for PCIe functions > > > > > > Niklas Schnelle (1): > > > s390/pci: Improve handling of unset UID > > > > > > Pierre Morel (10): > > > s390/pci: embedding hotplug_slot in zdev > > > s390/pci: adaptation of iommu to multifunction > > > s390/pci: define kernel parameters for PCI multifunction > > > s390/pci: define RID and RID available > > > s390/pci: create zPCI bus > > > s390/pci: adapt events for zbus > > > s390/pci: Handling multifunctions > > > s390/pci: Do not disable PF when VFs exist > > > s390/pci: Documentation for zPCI > > > s390/pci: removes wrong PCI multifunction assignment > > > > > > .../admin-guide/kernel-parameters.txt | 2 + > > > Documentation/s390/index.rst | 1 + > > > Documentation/s390/pci.rst | 126 +++++++++ > > > MAINTAINERS | 1 + > > > arch/s390/include/asm/pci.h | 45 +++- > > > arch/s390/include/asm/pci_clp.h | 12 +- > > > arch/s390/pci/Makefile | 3 +- > > > arch/s390/pci/pci.c | 198 ++++++-------- > > > arch/s390/pci/pci_bus.c | 255 > ++++++++++++++++++ > > > arch/s390/pci/pci_bus.h | 31 +++ > > > arch/s390/pci/pci_clp.c | 6 +- > > > arch/s390/pci/pci_event.c | 39 ++- > > > arch/s390/pci/pci_sysfs.c | 4 +- > > > drivers/iommu/s390-iommu.c | 8 +- > > > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > > > 15 files changed, 618 insertions(+), 218 deletions(-) > > > create mode 100644 Documentation/s390/pci.rst > > > create mode 100644 arch/s390/pci/pci_bus.c > > > create mode 100644 arch/s390/pci/pci_bus.h > > > > > > > > Ok, the justification looks better now. I did glance over the patches but > more > than checking for changes being contains in the arch specific code is not > really > doable. I saw that a test kernel was provided, so I would wait for feedback > there before considering further. > One ask for you: if you could provide a git branch we can pull the set > from, > this would help a lot. Generally I would say if a set consists of more > than say > 5 patches, a pull request is preferred. > > Thanks, > Stefan > > >
On 18.05.20 20:24, frank.heimes@canonical.com wrote: > Buglink: https://bugs.launchpad.net/bugs/1874056 > > SRU Justification: > > [Impact] > > * On s390x the enumeration of PCI functions does not reflect which functions belongs to which physical adapter. > > * Layout of a PCI function address on Linux: > 0000:00:00.0 > <root complex>:<bus>:<device>.<function> > > * On s390x, each function is presented as individual root complex today, e.g.: > PCHID 0100 VF1 0000:00:00.0 > PCHID 0100 VF23 0001:00:00.0 > PCHID 0200 VF1 0002:00:00.0 > OCHID 0100 VF17 0003:00:00.0 > > * On other platforms, the addresses correctly reflect the actual HW configuration. > > * Some device drivers (like mlx5, for Mellanox adapters) group functions of one physical adapter by checking which PCI functions have identical values for <root complex>:<bus>:<device>. > > * We need to use the same enumeration scheme to achieve this functionality on s390x. > > * In this case, the two physical functions of a Mellanox adapter need to get function number 0 and 1, > and all virtual functions need to use the same <root complex>:<bus> numbers with function/device numbers counting up. > > * Required result (example with 4 VFs per PF): > PCHID 0100 PF 0 0000:00:00.0 > PCHID 0100 PF 1 0000:00:00.1 > PCHID 0100 PF 0 VF 0 0000:00:00.2 > PCHID 0100 PF 0 VF 1 0000:00:00.3 > PCHID 0100 PF 0 VF 2 0000:00:00.4 > PCHID 0100 PF 0 VF 3 0000:00:00.5 > PCHID 0100 PF 1 VF 0 0000:00:00.6 > PCHID 0100 PF 1 VF 1 0000:00:00.7 > PCHID 0100 PF 1 VF 2 0000:00:00.8 > PCHID 0100 PF 1 VF 3 0000:00:00.9 > PCHID 0200 PF 0 0001:00:00.0 > > [Fix] > > * Backport 1: https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > * Backport 2: https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > * Backport 3: https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > * Backport 4: https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > * Backport 5: https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > * Backport 6: https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > * Backport 7: https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > * Backport 8: https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > * Backport 9: https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > * Backport 10: https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > * Backport 11: https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > * Backport 12: https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > [Test Case] > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more RoCE Express PCI 2(.1) adapters. > > * Assign the adapters (and it's virtual functions) to an LPAR. > > * Verify whether the physical and virtual functions are grouped in arbitrary order or in consecutive order - physical first (for example with lspci -t ...) > > [Regression Potential] > > * The regression potential can be considered as moderate, since: > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > * It largely affects zPCI, the s390x specific PCI code layer. > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not very wide-spread. > > * The situation described above affects the RoCE adapters only (Mellanox based). > > * The patches are also upstream accepted and available via linux-next, but to apply them to focal kernel 5.4 the above backports are needed. > > * However, the code is modified by several patches (12), hence there is a chance to break zPCI with them. > > * For upfront testing a PPA got created with a focal (master-next) kernel that incl. all the above patches. > > Alexander Schmidt (1): > s390/pci: Expose new port attribute for PCIe functions > > Niklas Schnelle (1): > s390/pci: Improve handling of unset UID > > Pierre Morel (10): > s390/pci: embedding hotplug_slot in zdev > s390/pci: adaptation of iommu to multifunction > s390/pci: define kernel parameters for PCI multifunction > s390/pci: define RID and RID available > s390/pci: create zPCI bus > s390/pci: adapt events for zbus > s390/pci: Handling multifunctions > s390/pci: Do not disable PF when VFs exist > s390/pci: Documentation for zPCI > s390/pci: removes wrong PCI multifunction assignment > > .../admin-guide/kernel-parameters.txt | 2 + > Documentation/s390/index.rst | 1 + > Documentation/s390/pci.rst | 126 +++++++++ > MAINTAINERS | 1 + > arch/s390/include/asm/pci.h | 45 +++- > arch/s390/include/asm/pci_clp.h | 12 +- > arch/s390/pci/Makefile | 3 +- > arch/s390/pci/pci.c | 198 ++++++-------- > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > arch/s390/pci/pci_bus.h | 31 +++ > arch/s390/pci/pci_clp.c | 6 +- > arch/s390/pci/pci_event.c | 39 ++- > arch/s390/pci/pci_sysfs.c | 4 +- > drivers/iommu/s390-iommu.c | 8 +- > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > 15 files changed, 618 insertions(+), 218 deletions(-) > create mode 100644 Documentation/s390/pci.rst > create mode 100644 arch/s390/pci/pci_bus.c > create mode 100644 arch/s390/pci/pci_bus.h > Based on architecture only code and positive test results in the bug report. Acked-by: Stefan Bader <stefan.bader@canonical.com>
On 2020-05-18 20:24, frank.heimes@canonical.com wrote: > Buglink: https://bugs.launchpad.net/bugs/1874056 > > SRU Justification: > > [Impact] > > * On s390x the enumeration of PCI functions does not reflect which functions belongs to which physical adapter. > > * Layout of a PCI function address on Linux: > 0000:00:00.0 > <root complex>:<bus>:<device>.<function> > > * On s390x, each function is presented as individual root complex today, e.g.: > PCHID 0100 VF1 0000:00:00.0 > PCHID 0100 VF23 0001:00:00.0 > PCHID 0200 VF1 0002:00:00.0 > OCHID 0100 VF17 0003:00:00.0 > > * On other platforms, the addresses correctly reflect the actual HW configuration. > > * Some device drivers (like mlx5, for Mellanox adapters) group functions of one physical adapter by checking which PCI functions have identical values for <root complex>:<bus>:<device>. > > * We need to use the same enumeration scheme to achieve this functionality on s390x. > > * In this case, the two physical functions of a Mellanox adapter need to get function number 0 and 1, > and all virtual functions need to use the same <root complex>:<bus> numbers with function/device numbers counting up. > > * Required result (example with 4 VFs per PF): > PCHID 0100 PF 0 0000:00:00.0 > PCHID 0100 PF 1 0000:00:00.1 > PCHID 0100 PF 0 VF 0 0000:00:00.2 > PCHID 0100 PF 0 VF 1 0000:00:00.3 > PCHID 0100 PF 0 VF 2 0000:00:00.4 > PCHID 0100 PF 0 VF 3 0000:00:00.5 > PCHID 0100 PF 1 VF 0 0000:00:00.6 > PCHID 0100 PF 1 VF 1 0000:00:00.7 > PCHID 0100 PF 1 VF 2 0000:00:00.8 > PCHID 0100 PF 1 VF 3 0000:00:00.9 > PCHID 0200 PF 0 0001:00:00.0 > > [Fix] > > * Backport 1: https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > * Backport 2: https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > * Backport 3: https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > * Backport 4: https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > * Backport 5: https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > * Backport 6: https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > * Backport 7: https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > * Backport 8: https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > * Backport 9: https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > * Backport 10: https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > * Backport 11: https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > * Backport 12: https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > [Test Case] > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more RoCE Express PCI 2(.1) adapters. > > * Assign the adapters (and it's virtual functions) to an LPAR. > > * Verify whether the physical and virtual functions are grouped in arbitrary order or in consecutive order - physical first (for example with lspci -t ...) > > [Regression Potential] > > * The regression potential can be considered as moderate, since: > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > * It largely affects zPCI, the s390x specific PCI code layer. > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not very wide-spread. > > * The situation described above affects the RoCE adapters only (Mellanox based). > > * The patches are also upstream accepted and available via linux-next, but to apply them to focal kernel 5.4 the above backports are needed. > > * However, the code is modified by several patches (12), hence there is a chance to break zPCI with them. > > * For upfront testing a PPA got created with a focal (master-next) kernel that incl. all the above patches. Limited to a single architecture, positive test results. Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > > Alexander Schmidt (1): > s390/pci: Expose new port attribute for PCIe functions > > Niklas Schnelle (1): > s390/pci: Improve handling of unset UID > > Pierre Morel (10): > s390/pci: embedding hotplug_slot in zdev > s390/pci: adaptation of iommu to multifunction > s390/pci: define kernel parameters for PCI multifunction > s390/pci: define RID and RID available > s390/pci: create zPCI bus > s390/pci: adapt events for zbus > s390/pci: Handling multifunctions > s390/pci: Do not disable PF when VFs exist > s390/pci: Documentation for zPCI > s390/pci: removes wrong PCI multifunction assignment > > .../admin-guide/kernel-parameters.txt | 2 + > Documentation/s390/index.rst | 1 + > Documentation/s390/pci.rst | 126 +++++++++ > MAINTAINERS | 1 + > arch/s390/include/asm/pci.h | 45 +++- > arch/s390/include/asm/pci_clp.h | 12 +- > arch/s390/pci/Makefile | 3 +- > arch/s390/pci/pci.c | 198 ++++++-------- > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > arch/s390/pci/pci_bus.h | 31 +++ > arch/s390/pci/pci_clp.c | 6 +- > arch/s390/pci/pci_event.c | 39 ++- > arch/s390/pci/pci_sysfs.c | 4 +- > drivers/iommu/s390-iommu.c | 8 +- > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > 15 files changed, 618 insertions(+), 218 deletions(-) > create mode 100644 Documentation/s390/pci.rst > create mode 100644 arch/s390/pci/pci_bus.c > create mode 100644 arch/s390/pci/pci_bus.h >
On Mon, May 18, 2020 at 08:24:26PM +0200, frank.heimes@canonical.com wrote: > Buglink: https://bugs.launchpad.net/bugs/1874056 > > SRU Justification: > > [Impact] > > * On s390x the enumeration of PCI functions does not reflect which functions belongs to which physical adapter. > > * Layout of a PCI function address on Linux: > 0000:00:00.0 > <root complex>:<bus>:<device>.<function> > > * On s390x, each function is presented as individual root complex today, e.g.: > PCHID 0100 VF1 0000:00:00.0 > PCHID 0100 VF23 0001:00:00.0 > PCHID 0200 VF1 0002:00:00.0 > OCHID 0100 VF17 0003:00:00.0 > > * On other platforms, the addresses correctly reflect the actual HW configuration. > > * Some device drivers (like mlx5, for Mellanox adapters) group functions of one physical adapter by checking which PCI functions have identical values for <root complex>:<bus>:<device>. > > * We need to use the same enumeration scheme to achieve this functionality on s390x. > > * In this case, the two physical functions of a Mellanox adapter need to get function number 0 and 1, > and all virtual functions need to use the same <root complex>:<bus> numbers with function/device numbers counting up. > > * Required result (example with 4 VFs per PF): > PCHID 0100 PF 0 0000:00:00.0 > PCHID 0100 PF 1 0000:00:00.1 > PCHID 0100 PF 0 VF 0 0000:00:00.2 > PCHID 0100 PF 0 VF 1 0000:00:00.3 > PCHID 0100 PF 0 VF 2 0000:00:00.4 > PCHID 0100 PF 0 VF 3 0000:00:00.5 > PCHID 0100 PF 1 VF 0 0000:00:00.6 > PCHID 0100 PF 1 VF 1 0000:00:00.7 > PCHID 0100 PF 1 VF 2 0000:00:00.8 > PCHID 0100 PF 1 VF 3 0000:00:00.9 > PCHID 0200 PF 0 0001:00:00.0 > > [Fix] > > * Backport 1: https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > * Backport 2: https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > * Backport 3: https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > * Backport 4: https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > * Backport 5: https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > * Backport 6: https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > * Backport 7: https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > * Backport 8: https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > * Backport 9: https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > * Backport 10: https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > * Backport 11: https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > * Backport 12: https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > [Test Case] > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more RoCE Express PCI 2(.1) adapters. > > * Assign the adapters (and it's virtual functions) to an LPAR. > > * Verify whether the physical and virtual functions are grouped in arbitrary order or in consecutive order - physical first (for example with lspci -t ...) > > [Regression Potential] > > * The regression potential can be considered as moderate, since: > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > * It largely affects zPCI, the s390x specific PCI code layer. > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not very wide-spread. > > * The situation described above affects the RoCE adapters only (Mellanox based). > > * The patches are also upstream accepted and available via linux-next, but to apply them to focal kernel 5.4 the above backports are needed. > > * However, the code is modified by several patches (12), hence there is a chance to break zPCI with them. > > * For upfront testing a PPA got created with a focal (master-next) kernel that incl. all the above patches. Several of these patches are from linux-next, which needs to be indicated in the provenance, e.g. "(backported from commit xxx linux-next)". Those patches should also be submitted for the development release. I went ahead and cherry-picked the following patches from linux-next to unstable/master. s390/pci: Expose new port attribute for PCIe functions s390/pci: adaptation of iommu to multifunction s390/pci: define kernel parameters for PCI multifunction s390/pci: define RID and RID available s390/pci: create zPCI bus s390/pci: adapt events for zbus s390/pci: Handling multifunctions s390/pci: Do not disable PF when VFs exist s390/pci: Documentation for zPCI s390/pci: removes wrong PCI multifunction assignment
On 2020-05-18 20:24:26 , frank.heimes@canonical.com wrote: > Buglink: https://bugs.launchpad.net/bugs/1874056 > > SRU Justification: > > [Impact] > > * On s390x the enumeration of PCI functions does not reflect which functions belongs to which physical adapter. > > * Layout of a PCI function address on Linux: > 0000:00:00.0 > <root complex>:<bus>:<device>.<function> > > * On s390x, each function is presented as individual root complex today, e.g.: > PCHID 0100 VF1 0000:00:00.0 > PCHID 0100 VF23 0001:00:00.0 > PCHID 0200 VF1 0002:00:00.0 > OCHID 0100 VF17 0003:00:00.0 > > * On other platforms, the addresses correctly reflect the actual HW configuration. > > * Some device drivers (like mlx5, for Mellanox adapters) group functions of one physical adapter by checking which PCI functions have identical values for <root complex>:<bus>:<device>. > > * We need to use the same enumeration scheme to achieve this functionality on s390x. > > * In this case, the two physical functions of a Mellanox adapter need to get function number 0 and 1, > and all virtual functions need to use the same <root complex>:<bus> numbers with function/device numbers counting up. > > * Required result (example with 4 VFs per PF): > PCHID 0100 PF 0 0000:00:00.0 > PCHID 0100 PF 1 0000:00:00.1 > PCHID 0100 PF 0 VF 0 0000:00:00.2 > PCHID 0100 PF 0 VF 1 0000:00:00.3 > PCHID 0100 PF 0 VF 2 0000:00:00.4 > PCHID 0100 PF 0 VF 3 0000:00:00.5 > PCHID 0100 PF 1 VF 0 0000:00:00.6 > PCHID 0100 PF 1 VF 1 0000:00:00.7 > PCHID 0100 PF 1 VF 2 0000:00:00.8 > PCHID 0100 PF 1 VF 3 0000:00:00.9 > PCHID 0200 PF 0 0001:00:00.0 > > [Fix] > > * Backport 1: https://launchpadlibrarian.net/479699471/0001-s390-pci-Improve-handling-of-unset-UID.patch > > * Backport 2: https://launchpadlibrarian.net/479699482/0002-s390-pci-embedding-hotplug_slot-in-zdev.patch > > * Backport 3: https://launchpadlibrarian.net/479699492/0003-s390-pci-Expose-new-port-attribute-for-PCIe-function.patch > > * Backport 4: https://launchpadlibrarian.net/479699497/0004-s390-pci-adaptation-of-iommu-to-multifunction.patch > > * Backport 5: https://launchpadlibrarian.net/479700706/0005-s390-pci-define-kernel-parameters-for-PCI-multifunct.patch > > * Backport 6: https://launchpadlibrarian.net/479700712/0006-s390-pci-define-RID-and-RID-available.patch > > * Backport 7: https://launchpadlibrarian.net/479700739/0007-s390-pci-create-zPCI-bus.patch > > * Backport 8: https://launchpadlibrarian.net/479700769/0008-s390-pci-adapt-events-for-zbus.patch > > * Backport 9: https://launchpadlibrarian.net/479700786/0009-s390-pci-Handling-multifunctions.patch > > * Backport 10: https://launchpadlibrarian.net/479700794/0010-s390-pci-Do-not-disable-PF-when-VFs-exist.patch > > * Backport 11: https://launchpadlibrarian.net/479700798/0011-s390-pci-Documentation-for-zPCI.patch > > * Backport 12: https://launchpadlibrarian.net/479700799/0012-s390-pci-removes-wrong-PCI-multifunction-assignment.patch > > [Test Case] > > * Prepare an IBM z13 or LinuxONE III (or newer) system with two or more RoCE Express PCI 2(.1) adapters. > > * Assign the adapters (and it's virtual functions) to an LPAR. > > * Verify whether the physical and virtual functions are grouped in arbitrary order or in consecutive order - physical first (for example with lspci -t ...) > > [Regression Potential] > > * The regression potential can be considered as moderate, since: > > * It is purely s390x specific code (arch/s390/* drivers/iommu/s390-iommu.c and drivers/pci/hotplug/s390_pci_hpc.c - and some doc adjustments, too). > > * It largely affects zPCI, the s390x specific PCI code layer. > > * PCI cards available for s390x are optional cards (RoCE and zEDC) and not very wide-spread. > > * The situation described above affects the RoCE adapters only (Mellanox based). > > * The patches are also upstream accepted and available via linux-next, but to apply them to focal kernel 5.4 the above backports are needed. > > * However, the code is modified by several patches (12), hence there is a chance to break zPCI with them. > > * For upfront testing a PPA got created with a focal (master-next) kernel that incl. all the above patches. > > Alexander Schmidt (1): > s390/pci: Expose new port attribute for PCIe functions > > Niklas Schnelle (1): > s390/pci: Improve handling of unset UID > > Pierre Morel (10): > s390/pci: embedding hotplug_slot in zdev > s390/pci: adaptation of iommu to multifunction > s390/pci: define kernel parameters for PCI multifunction > s390/pci: define RID and RID available > s390/pci: create zPCI bus > s390/pci: adapt events for zbus > s390/pci: Handling multifunctions > s390/pci: Do not disable PF when VFs exist > s390/pci: Documentation for zPCI > s390/pci: removes wrong PCI multifunction assignment > > .../admin-guide/kernel-parameters.txt | 2 + > Documentation/s390/index.rst | 1 + > Documentation/s390/pci.rst | 126 +++++++++ > MAINTAINERS | 1 + > arch/s390/include/asm/pci.h | 45 +++- > arch/s390/include/asm/pci_clp.h | 12 +- > arch/s390/pci/Makefile | 3 +- > arch/s390/pci/pci.c | 198 ++++++-------- > arch/s390/pci/pci_bus.c | 255 ++++++++++++++++++ > arch/s390/pci/pci_bus.h | 31 +++ > arch/s390/pci/pci_clp.c | 6 +- > arch/s390/pci/pci_event.c | 39 ++- > arch/s390/pci/pci_sysfs.c | 4 +- > drivers/iommu/s390-iommu.c | 8 +- > drivers/pci/hotplug/s390_pci_hpc.c | 105 +++----- > 15 files changed, 618 insertions(+), 218 deletions(-) > create mode 100644 Documentation/s390/pci.rst > create mode 100644 arch/s390/pci/pci_bus.c > create mode 100644 arch/s390/pci/pci_bus.h > > -- > 2.25.1 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team