Re: [PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF

[Shivaprasad G Bhat <sbhat@linux.ibm.com>]

On 10/12/24 5:40 PM, Akihiko Odaki wrote:
> On 2024/10/12 2:22, Shivaprasad G Bhat wrote:
>> On 9/18/24 7:57 PM, Cédric Le Goater wrote:
>>> Adding :
>>>
>>>   Harsh for QEMU/PPC pseries machine,
>>>   Shivaprasad for KVM/PPC VFIO and IOMMU support.
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>>
>>> On 9/13/24 05:44, Akihiko Odaki wrote:
>>>> A PF may automatically create VFs and the PF may be function 0.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>   hw/ppc/spapr_pci.c | 4 +++-
>>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>>>> index f63182a03c41..ed4454bbf79e 100644
>>>> --- a/hw/ppc/spapr_pci.c
>>>> +++ b/hw/ppc/spapr_pci.c
>>>> @@ -1573,7 +1573,9 @@ static void spapr_pci_pre_plug(HotplugHandler 
>>>> *plug_handler,
>>>>        * hotplug, we do not allow functions to be hotplugged to a
>>>>        * slot that already has function 0 present
>>>>        */
>>>> -    if (plugged_dev->hotplugged && bus->devices[PCI_DEVFN(slotnr, 
>>>> 0)] &&
>>>> +    if (plugged_dev->hotplugged &&
>>>> +        !pci_is_vf(pdev) &&
>>
>> I see there is history to this change. The reverted[1] virtio-net-pci 
>> SRIOV emulation support
>>
>> needed this as the VFs were explicitly specified with -device virtio- 
>> net-pci,sriov-pf=X
>>
>> property. I see the pre_plug handlers for the VFs cant be reached now 
>> with the reverted
>>
>> code base for the other devices(nvme and igb) supporting the SRIOV 
>> emulation.
>>
>>
>> Do the VFs really reach this path in today's code base ? Other than 
>> the above
>>
>> workflow, the pre_plug() handlers wont be called for VFs when the
>>
>> echo X > /<sys-fs-pf-path>/sriov_numvfs inside the guest too. I don't 
>> see the
>>
>> workflow(PF automatically creating VFs) to hit this path. Could you 
>> clarify how?
>>
>>
>> I see before the revert of virito-net-pci sriov use-case, the out of 
>> order VF hot|cold
>>
>> plug post PF are prevented here. Even if we allowed VFs to continue 
>> here, PFs were
>>
>> prevented in pcie_sriov_register_device() which is followed 
>> sequentially anyway. Now,
>>
>> as the pcie_sriov_register_device() is no longer there, this check 
>> actually makes
>>
>> sense as this would be the only place we avoid the out of order 
>> plugging.
>
> VFs are always plugged after its paired PF. Currently, VFs are plugged 
> when the guest writes sriov_numvfs. With "[PATCH v16 08/13] 
> pcie_sriov: Reuse SR-IOV VF device instances", which follows this 
> patch, VFs will plug while the PF is being realized.
>
Thanks, I was juggling between the reverted patches and the current ones

and missed this patch. I see things fine now.


Reviewed-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>

Tested-by: Shivaprasad G Bhat <sbhat@linux.ibm.com>


> I have no idea why you can't reproduce the issue by writing sriov_numvfs, 

On PPC64, IOV resources are disabled and the sriov attributes in sysfs are

not accessible without open-sriov support [2](not yet added on qemu).

However, the config space is showing the IOV capabilities and the values

fine with the patches.


> but it is easy to reproduce it with "[PATCH v16 08/13] pcie_sriov: 
> Reuse SR-IOV VF device instances" applied. You can use the following 
> command:
> qemu-system-ppc64 -nographic -monitor stdio -serial none <<< 
> 'device_add igb'
>
> It should say:
> Error: PCI: slot 18 function 0 already occupied by igbvf, additional 
> functions can no longer be exposed to guest.
>
> Regards,
> Akihiko Odaki
>
>>
>>
>> On a side note, for testing this fulky on PPC, we need more work on 
>> Qemu today as the
>>
>> open-sriov[2] is supported only on PowerVM.
>>
>>
>> Thanks,
>>
>> Shivaprasad
>>
>>
>> Reference :
>>
>> [1] - Atleast till commit b0fdaee5d1
>>
>> [2] - https://lore.kernel.org/linuxppc-dev/20180105164552.36371-1- 
>> bryantly@linux.vnet.ibm.com/
>>
>