Re: [PATCH v1 08/11] vfio/pci: Allocate and initialize HostIOMMUDevice after attachment
On 3/19/24 04:46, Duan, Zhenzhong wrote:
>
>> -Original Message-
>> From: Eric Auger
>> 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
>>> ---
>>> 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, );
>>> 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);
>>> /*
RE: [PATCH v1 08/11] vfio/pci: Allocate and initialize HostIOMMUDevice after attachment
>-Original Message-
>From: Eric Auger
>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
>> ---
>> 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, );
>> 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);
>> /*
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
> ---
> 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
Eric
> vfio_populate_device(vdev, );
> 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);
> g_free(vdev->emulated_config_bits);
> g_free(vdev->rom);
> /*
[PATCH v1 08/11] vfio/pci: Allocate and initialize HostIOMMUDevice after attachment
Signed-off-by: Zhenzhong Duan
---
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 */
+host_iommu_device_create(vbasedev);
+
vfio_populate_device(vdev, );
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);
g_free(vdev->emulated_config_bits);
g_free(vdev->rom);
/*
--
2.34.1
