[SRU][F][PATCH 0/12] s390x/pci: enumerate pci functions per physical adapter (LP: 1874056)
Frank Heimes
frank.heimes at canonical.com
Wed May 20 19:10:32 UTC 2020
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 at 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 at canonical.com
> > <mailto:stefan.bader at canonical.com>> wrote:
> >
> > On 18.05.20 20:24, frank.heimes at canonical.com
> > <mailto:frank.heimes at 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200520/39a85250/attachment-0001.html>
More information about the kernel-team
mailing list