Re: [PATCH] pcie: Release references of virtual functions

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

On 2023/04/11 19:11, Philippe Mathieu-Daudé wrote:
> On 11/4/23 11:04, Akihiko Odaki wrote:
>> pci_new() automatically retains a reference to a virtual function when
>> registering it so we need to release the reference when unregistering.
>>
>> Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O 
>> Virtualization (SR/IOV)")
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   hw/pci/pcie_sriov.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
>> index aa5a757b11..76a3b6917e 100644
>> --- a/hw/pci/pcie_sriov.c
>> +++ b/hw/pci/pcie_sriov.c
>> @@ -211,6 +211,7 @@ static void unregister_vfs(PCIDevice *dev)
>>               error_free(local_err);
>>           }
>>           object_unparent(OBJECT(vf));
>> +        object_unref(OBJECT(vf));
>>       }
>>       g_free(dev->exp.sriov_pf.vf);
>>       dev->exp.sriov_pf.vf = NULL;
> 
> It feels the issue is at the device creation.

I added object_unref() to unregister_vfs() because dev->exp.sriov_pf.vf 
actually holds the reference. Practically there should be no difference 
as the parent bus also keeps the reference until unregister_vfs() calls 
object_unparent(), but I think it is semantically more correct to 
object_unref() in unregister_vfs().

> 
> [/me looking at the code]
> 
> What about:
> 
> -- >8 --
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index aa5a757b11..fca3bf6e72 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -150,7 +150,7 @@ static PCIDevice *register_vf(PCIDevice *pf, int 
> devfn, const char *name,
>       PCIBus *bus = pci_get_bus(pf);
>       Error *local_err = NULL;
> 
> -    qdev_realize(&dev->qdev, &bus->qbus, &local_err);
> +    pci_realize_and_unref(dev, bus, &local_err);
>       if (local_err) {
>           error_report_err(local_err);
>           return NULL;
> ---