On 2024/09/12 6:11, Matthew Rosato wrote:
On 9/11/24 11:15 AM, Akihiko Odaki wrote:
On 2024/09/11 22:53, Matthew Rosato wrote:
On 9/11/24 6:58 AM, Akihiko Odaki wrote:
On 2024/09/11 18:38, Cédric Le Goater wrote:
+Matthew +Eric
Side note for the maintainers :
Before this change, the igb device, which is multifunction, was working
fine under Linux.
Was there a fix in Linux since :
57da367b9ec4 ("s390x/pci: forbid multifunction pci device")
6069bcdeacee ("s390x/pci: Move some hotplug checks to the pre_plug
handler")
?
The timing of those particular commits predates the linux s390 kernel support
of multifunction/SR-IOV. At that time it was simply not possible on s390.
Is it OK to remove this check of multifunction now?
Yes, I think removing the check (which AFAIU was broken since 6069bcdeacee)
will get us back to the behavior Cedric was seeing, where a device that reports
multifunction in the config space is still allowed through but only the PF will
be usable in the guest.
Commit 6069bcdeacee predates the introduction of SR-IOV (commit
7c0fa8dff811 ["pcie: Add support for Single Root I/O Virtualization
(SR/IOV)"]) so it did not break anything, strictly speaking. Ideally, we
should have taken care of the check when introducing SR-IOV.
This code is not working properly with SR-IOV and misleading. It is better to
remove the code if it does no good.
It would be nice if anyone confirms multifunction works for s390x with the code
removed.
Even if you remove the check, multifunction itself won't work in the s390x
guest without these missing CLP pieces too. When I have some time I'll hack
something together to fabricate some CLP data and try it out, but it sounds
like Cedric could use his setup to right now at least verify that the PF itself
should remain usable in the guest (current behavior).
Well, it seems better to keep the check for non-SR-IOV multifunction
devices while not enforcing the restriction for SR-IOV.
Multifunction devices without SR-IOV are created with explicit requests
by specifying multifunction=on for PCI devices. Such requests cannot be
fulfilled without multifunction CLP so we should reject them.
The situation is different for SR-IOV. SR-IOV is a feature inherent to
specific type of devices and gets automatically enabled for these
devices. It may make more sense to ignore just the SR-IOV part of such
devices and keep the other functionalities working.
The current code implements such a behavior, but it is accidental and
semantically wrong. I will correct the code to explicitly allow
multifunction for SR-IOV.
Regards,
Akihiko Odaki
