Re: [PATCH v1 08/11] vfio/pci: Allocate and initialize HostIOMMUDevice after attachment

[Eric Auger]

On 3/19/24 04:46, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH v1 08/11] vfio/pci: Allocate and initialize
>> HostIOMMUDevice after attachment
>>
>>
>>
>> On 2/28/24 04:58, Zhenzhong Duan wrote:
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>  hw/vfio/pci.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>>> index 4fa387f043..6cc7de5d10 100644
>>> --- a/hw/vfio/pci.c
>>> +++ b/hw/vfio/pci.c
>>> @@ -3006,6 +3006,9 @@ static void vfio_realize(PCIDevice *pdev, Error
>> **errp)
>>>          goto error;
>>>      }
>>>
>>> +    /* Allocate and initialize HostIOMMUDevice after attachment succeed
>> */
>> after successful attachment?
>>> +    host_iommu_device_create(vbasedev);
>>> +
>> you shall free on error: as well
> I free it in vfio_instance_finalize().
> Vfio-pci's design is special, it didn't free all allocated resources in 
> realize's error path,
> They are freed in _finalize(). e.g., vdev->emulated_config_bits, vdev->rom, 
> devices and group resources(vfio_detach_device).
> I'm following the same way. I'm fine to free it as you suggested something 
> like below:

Oh yes I remember now. I had exactly the same question some months ago
:-/ So that's fine then

Eric
>
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3246,6 +3246,7 @@ out_teardown:
>      vfio_teardown_msi(vdev);
>      vfio_bars_exit(vdev);
>  error:
> +    g_free(vdev->vbasedev.base_hdev);
>      error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->name);
>  }
>
> @@ -3288,6 +3289,7 @@ static void vfio_exitfn(PCIDevice *pdev)
>      vfio_bars_exit(vdev);
>      vfio_migration_exit(vbasedev);
>      pci_device_unset_iommu_device(pdev);
> +    g_free(vdev->vbasedev.base_hdev);
>  }
>
>> Eric
>>>      vfio_populate_device(vdev, &err);
>>>      if (err) {
>>>          error_propagate(errp, err);
>>> @@ -3244,6 +3247,7 @@ static void vfio_instance_finalize(Object *obj)
>>>
>>>      vfio_display_finalize(vdev);
>>>      vfio_bars_finalize(vdev);
>>> +    g_free(vdev->vbasedev.base_hdev);
> I free it here.
>
> Thanks
> Zhenzhong
>
>>>      g_free(vdev->emulated_config_bits);
>>>      g_free(vdev->rom);
>>>      /*