NACK/Cmnt: [SRU][N/O][PATCH 0/1] s390/pci: Fix zpci_bus_is_isolated_vf() for non-VFs

[Hui Wang]

On 5/26/25 17:42, Zixing Liu wrote:
> BugLink: https://bugs.launchpad.net/linux/+bug/2110289

The bug number is not correct, it points to a different bug.

And the format of BugLink should be like this: 
https://bugs.launchpad.net/bugs/nnnnnnn

> SRU Justification:
>
> [Impact]
>
>   * For non-VFs, function zpci_bus_is_isolated_vf() should return false,
>     because they aren't VFs.
>     While zpci_iov_find_parent_pf() specifically checks if a function is a VF,
>     it then simply returns that there is no parent.
>
>   * The simplistic check for a parent then leads to these functions being
>     confused with isolated VFs, and isolating them on their own domain even
>     if sibling PFs should share the domain.
>
> [Fix]
>
>   * This is fixed by explicitly checking if a function is not a VF.
>     (Notice that at this point the case where RIDs are ignored is already
>      handled - and in this case all PCI functions get isolated by being
>      properly detected in zpci_bus_is_multifunction_root().)
>
>   * 8691abd3afaa "s390/pci: Fix zpci_bus_is_isolated_vf() for non-VFs"
>
> [Test Plan]
>
>   * Setup Ubuntu Server (24.04/24.10) for s390x on an IBM z17
>     or LinuxONE 5 LPAR.
>
>   * Have at least two PCIe adapters that support PF/VF (like RoCE Express)
>     available in that LPAR.
>
>   * Attach multiple PFs of a PF access mode device (notice that this is only
>     possible with z17 and L1-5 hardware), such as the two PFs of a NETD
>     to the same LPAR.
>
>   * Observe that they are put into separate PCI domains
>     instead of sharing the same domain as expected by drivers.
>
>   * Due to lack of hardware, the verification will be conducted by IBM.
>
>   * The fix was discussed upstream and flagged a stable kernel update.
>
> [Where problems could occur]
>
>   * The modification is limited to one additional if statement
>     across two lines) in function zpci_bus_is_isolated_vf()
>     in file arch/s390/pci/pci_bus.c.
>
>   * Hence the modification will be limited to the s390x-specific parts of
>     the PCI code in the kernel (sometimes refers to as zPCI),
>     and will NOT impact any other architecture!
>
>   * If add. if-statement is not correct and a wrong bool is returned,
>     function zpci_bus_is_isolated_vf() might report and incorrect zpci_bus
>     status. Either isolated when it's not or not-isolated when it really is.
>
>   * And since the new if statement got inserted before the already existing
>    'if (!pdev)', the latter code in function zpci_bus_is_isolated_vf()
>     might be accidentally skipped.
>
>   * All this might lead to a similar confusion of the functions in regard to
>     isolated VFs status and whether isolating them on their own domain even
>     or not, hence proper testing is needed.
>
> [Other Info]
>
>   * Since the commit got upstream accepted with v6.15-rc1,
>     'Questing' is not affected.
>
>   * Plucky got fixed with
>     https://bugs.launchpad.net/bugs/2108854
>     (cherry-picked as https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/plucky/commit/?id=58412b2aa12aa43c90d5414e22898a041ec05e8a)
>
>   * Hence the only remaining affected releases, that contain the offending
>     commit ("s390/pci: Fix handling of isolated VFs"), are oracular and noble.
>
>   * Since this issue does not happen (thus cannot be recreated nor tested)
>     on the IBM Z hardware we have in Canonical, the verification(s) will be
>     done by IBM on most recent hardware.
>
> Niklas Schnelle (1):
>    s390/pci: Fix zpci_bus_is_isolated_vf() for non-VFs
>
>   arch/s390/pci/pci_bus.c | 3 +++
>   1 file changed, 3 insertions(+)
>