Re: [PATCH rfcv2 05/20] vfio/iommufd: Implement [at|de]tach_hwpt handlers

[Eric Auger <eric.auger@redhat.com>]

On 2/28/25 9:24 AM, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH rfcv2 05/20] vfio/iommufd: Implement [at|de]tach_hwpt
>> handlers
>>
>>
>>
>>
>> On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
>>> Implement [at|de]tach_hwpt handlers in VFIO subsystem. vIOMMU
>>> utilizes them to attach to or detach from hwpt on host side.
>>>
>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>  hw/vfio/iommufd.c | 22 ++++++++++++++++++++++
>>>  1 file changed, 22 insertions(+)
>>>
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index 53639bf88b..175c4fe1f4 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -802,6 +802,24 @@ static void
>> vfio_iommu_iommufd_class_init(ObjectClass *klass, void *data)
>>>      vioc->query_dirty_bitmap = iommufd_query_dirty_bitmap;
>>>  };
>>>
>>> +static bool
>> can't we return an integer instead. This looks more standard to me
> I can do that, but I remember VFIO honors bool return value
> whenever possible. We had ever cleanup patches to make all functions
> return bool when possible. Do we really want to return int for only these
> two functions?
I now remember those patches from Cédric. As I mentionned realier I have
not found in the errp doc that this was a requirement but nevertheless
ignore this comment then ;-)

Eric
>
> Thanks
> Zhenzhong
>
>> Eric
>> +host_iommu_device_iommufd_vfio_attach_hwpt(HostIOMMUDeviceIOMMUFD
>> *idev,
>>> +                                           uint32_t hwpt_id, Error **errp)
>>> +{
>>> +    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
>>> +
>>> +    return !iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt_id, errp);
>>> +}
>>> +
>>> +static bool
>>>
>> +host_iommu_device_iommufd_vfio_detach_hwpt(HostIOMMUDeviceIOMMUF
>> D *idev,
>>> +                                           Error **errp)
>>> +{
>>> +    VFIODevice *vbasedev = HOST_IOMMU_DEVICE(idev)->agent;
>>> +
>>> +    return iommufd_cdev_detach_ioas_hwpt(vbasedev, errp);
>>> +}
>>> +
>>>  static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void
>> *opaque,
>>>                                        Error **errp)
>>>  {
>>> @@ -863,11 +881,15 @@
>> hiod_iommufd_vfio_get_page_size_mask(HostIOMMUDevice *hiod)
>>>  static void hiod_iommufd_vfio_class_init(ObjectClass *oc, void *data)
>>>  {
>>>      HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_CLASS(oc);
>>> +    HostIOMMUDeviceIOMMUFDClass *idevc =
>> HOST_IOMMU_DEVICE_IOMMUFD_CLASS(oc);
>>>      hiodc->realize = hiod_iommufd_vfio_realize;
>>>      hiodc->realize_late = hiod_iommufd_vfio_realize_late;
>>>      hiodc->get_iova_ranges = hiod_iommufd_vfio_get_iova_ranges;
>>>      hiodc->get_page_size_mask = hiod_iommufd_vfio_get_page_size_mask;
>>> +
>>> +    idevc->attach_hwpt = host_iommu_device_iommufd_vfio_attach_hwpt;
>>> +    idevc->detach_hwpt = host_iommu_device_iommufd_vfio_detach_hwpt;
>>>  };
>>>
>>>  static const TypeInfo types[] = {