1. Patchew
  2. QEMU
  3. [PATCH v16 00/13] hw/pci: SR-IOV related fixes and improvements
  4. View patch

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

Akihiko Odaki posted 13 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF
Posted by Akihiko Odaki 2 months, 1 week ago 
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) &&
+        bus->devices[PCI_DEVFN(slotnr, 0)] &&
         PCI_FUNC(pdev->devfn) != 0) {
         error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
                    " additional functions can no longer be exposed to guest.",

-- 
2.46.0
Re: [PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF
Posted by Cédric Le Goater 2 months ago 
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) &&
> +        bus->devices[PCI_DEVFN(slotnr, 0)] &&
>           PCI_FUNC(pdev->devfn) != 0) {
>           error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
>                      " additional functions can no longer be exposed to guest.",
>
Re: [PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF
Posted by Shivaprasad G Bhat 1 month, 1 week ago 
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.


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/
Re: [PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF
Posted by Akihiko Odaki 1 month, 1 week ago 
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.

I have no idea why you can't reproduce the issue by writing 
sriov_numvfs, 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/
>
Re: [PATCH v16 03/13] hw/ppc/spapr_pci: Do not reject VFs created after a PF
Posted by Shivaprasad G Bhat 1 month, 1 week ago 
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/
>>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.