Re: [PATCH v10 06/12] pcie_sriov: Reuse SR-IOV VF device instances

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/07/10 19:52, Michael S. Tsirkin wrote:
> On Wed, Jul 10, 2024 at 08:37:27AM +0200, Cédric Le Goater wrote:
>> Hello,
>>
>> This change introduced a regression on s390x. I could have spotted it
>> earlier. Sorry about that. Here is the scenario,
>>
>> QEMU now creates automatically the PCI device objects representing the
>> VFs when the PF device is realized in pcie_sriov_pf_init(). This is
>> good to report errors early but it has an important drawback.
>>
>> On s390x, PCI devices have a dual S390PCIBusDevice object. This device
>> model has 'uid' and 'fid' properties which can be either set by the VMM
>> or, if not, auto-generated by the S390PCIBusDevice realize handler. In
>> the VF case, these ids are auto-generated by QEMU and they can possibly
>> conflict with the uid number space of libvirt. The conflict is detected
>> when the machine is created and the start is aborted with a message :
>>
>>    2024-07-08T12:51:42.876883Z qemu-system-s390x: -device {"driver":"zpci","uid":17,"fid":16,"target":"hostdev0","id":"zpci17"}: uid 17 already in use
>>
>> This problem can occur today with a s390x VM using an IGB device.
>>
>> It worked fine when the VFs were created at OS runtime because the initial
>> topology of the machine was in place. Adding VFs was more or less like
>> hotplug. AIUI, libvirt should have full control on the machine topology
>> and so, creating VFs in QEMU at init time in the back of libvirt seems
>> like a violation of this rule.
>>
>> That said, the s390x case is specific and could perhaps be handled in a
>> special way.
>>
>> Thanks,
>>
>> C.
> 
> 
> Thanks for reporting this Cédric. Akihiko what's your
> plan to handle this? Do you have the time to address this issue?

Creating VFs at initialization time only makes problems apparent early. 
Even without this change, hot-plugging another PCI device after 
realizing a VF results in a similar situation.

A proper way to handle this is to add new properties to igb and nvme to 
let libvirt specify the VF ids. However I wonder if it is a worthwhile 
addition (i.e., if igb and nvme's SR-IOV emulation will be used with 
s390x and libvirt).

Regards,
Akihiko Odaki