Re: [PATCH v4 05/12] vfio/iommufd: Introduce auto domain creation

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

On 17/07/2024 10:28, Cédric Le Goater wrote:
>>>>>>> @@ -224,6 +300,11 @@ static void
>>>> iommufd_cdev_detach_container(VFIODevice *vbasedev,
>>>>>>>   {
>>>>>>>       Error *err = NULL;
>>>>>>>
>>>>>>> +    if (vbasedev->hwpt) {
>>>>>>> +        iommufd_cdev_autodomains_put(vbasedev, container);
>>>>>>> +        return;
>>>>>> Where do we detach the device from the hwpt?
>>>>>>
>>>>> In iommufd_backend_free_id() for auto domains
>>>>>
>>>>
>>>> to clarify here I meant *userspace* auto domains
>>>>
>>>> *kernel* auto domains (mdev) goes via DETACH_IOMMUFD_PT
>>>
>>> If the device is still attached to the hwpt, will iommufd_backend_free_id()
>>> succeed?
>>> Have you tried the hot unplug?
>>>
>>
>> I have but I didn't see any errors. But I will check again for v5 as it could
>> also be my oversight.
>>
>> I was thinking about Eric's remark overnight and I think what I am doing is not
>> correct regardless of the above.
>>
>> I should be calling DETACH_IOMMUFD_PT pairing with ATTACH_IOMMUFD_PT, and the
>> iommufd_backend_free_id() is to drop the final reference pairing with
>> alloc_hwpt() when the device list is empty i.e. when there's no more devices in
>> that vdev::hwpt.
>>
>> DETACH_IOMMUFD_PT decrement the hwpt refcount and it doesn't differentiate
>> between auto domains vs manual domains.
>>
>> The code is already there anyhow it just has the order of
>> iommufd_cdev_autodomains_put vs detach invocation reversed; I'll fix that for
>> next version.
> 
> While at it, could you please move these routines :
> 
>   iommufd_cdev_detach_ioas_hwpt
>   iommufd_cdev_attach_ioas_hwpt
>  
> under backends/iommufd.c ? I think that's where they belong.

OK