Re: [PATCH rfcv1 4/6] vfio: initialize IOMMUFDDevice and pass to vIOMMU

[Joao Martins <joao.m.martins@oracle.com>]

On 18/01/2024 10:17, Yi Liu wrote:
> On 2024/1/18 16:17, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Joao Martins <joao.m.martins@oracle.com>
>>> Subject: Re: [PATCH rfcv1 4/6] vfio: initialize IOMMUFDDevice and pass to
>>> vIOMMU
>>>
>>> On 15/01/2024 10:13, Zhenzhong Duan wrote:
>>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>>> index 9bfddc1360..cbd035f148 100644
>>>> --- a/hw/vfio/iommufd.c
>>>> +++ b/hw/vfio/iommufd.c
>>>> @@ -309,6 +309,7 @@ static int iommufd_cdev_attach(const char *name,
>>> VFIODevice *vbasedev,
>>>>       VFIOContainerBase *bcontainer;
>>>>       VFIOIOMMUFDContainer *container;
>>>>       VFIOAddressSpace *space;
>>>> +    IOMMUFDDevice *idev = &vbasedev->idev;
>>>>       struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
>>>>       int ret, devfd;
>>>>       uint32_t ioas_id;
>>>> @@ -428,6 +429,7 @@ found_container:
>>>>       QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev,
>>> container_next);
>>>>       QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
>>>>
>>>> +    iommufd_device_init(idev, sizeof(*idev), container->be, vbasedev-
>>>> devid);
>>>>       trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev-
>>>> num_irqs,
>>>>                                      vbasedev->num_regions, vbasedev->flags);
>>>>       return 0;
>>>
>>> In the dirty tracking series, I'll need to fetch out_capabilities from device
>>> and do a bunch of stuff that is used when allocating hwpt to ask for dirty
>>> tracking. And this means having iommufd_device_init() be called before we
>>> call
>>> iommufd_cdev_attach_container().
>>>
>>> Here's what it looks based on an earlier version of your patch:
>>>
>>> https://github.com/jpemartins/qemu/commit/433f97a05e0cdd8e3b8563a
>>> a20e4f22d107219b5
>>>
>>> I can move the call earlier in my series, unless there's something specifically
>>> when you call it here?
>>
>> I think it's safe to move it earlier, just remember to do the same for existing
>> container.
> 
> yes, as long as the input of iommufd_device_init() are available in the new
> place. And remember to destroy it if the code failed after initializing
> iommufd_device.
> 
In the way I am using I don't think there's any teardown as no new resources or
things are done. We essentially just initialize @idev and fetch some
capabilities thus nothing needs teardown. But I am not sure is the nesting
needs; perhaps destruction of resources is related to something to that?