Re: [PATCH for-9.2 v15 04/11] s390x/pci: Check for multifunction after device realization

[Matthew Rosato <mjrosato@linux.ibm.com>]

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.

>>
>> s390 PCI devices do not have extended capabilities, so the igb device
>> does not expose the SRIOV capability and only the PF is accessible but
>> it doesn't seem to be an issue. (Btw, CONFIG_PCI_IOV is set to y in the
>> default Linux config which is unexpected)

The linux config option makes sense because the s390 kernel now supports SR-IOV/multifunction.

> 
> Doesn't s390x really see extended capabilities? hw/s390x/s390-pci-inst.c has a call pci_config_size() and pci_host_config_write_common(), which means it is exposing the whole PCI Express configuration space. Why can't s390x use extended capabilities then?
> 

So, rather than poking around in config space, s390 (and thus the s390 kernel) has an extra layer of 'capabilities' that it generally relies on to determine device functionality called 'CLP'.  Basically, there are pieces of CLP that are not currently generated (or forwarded from the host in the case of passthrough) by QEMU that would be needed by the guest to recognize the SRIOV/multifunction capability of a device, despite what config space has in it.  I suspect this is exactly why only the PF was available to your igb device then (missing CLP info made the device appear to not have multifunction capability as far as the s390 guest is concerned - fwiw adding CLP emulation to enable that is on our todo list).

Sounds like the short-term solution here would be to continue allowing the PF without multifunction being visible to the guest (so as to not regress prior functionality) and then aim for proper support after with the necessary CLP pieces. 

Thanks,
Matt