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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Auger <eric.au...@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.d...@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:

--- 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);
>>      /*