diff mbox series

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

Message ID 20250219082228.3303163-6-zhenzhong.duan@intel.com (mailing list archive)
State New
Headers show
Series intel_iommu: Enable stage-1 translation for passthrough device | expand

Commit Message

Duan, Zhenzhong Feb. 19, 2025, 8:22 a.m. UTC 
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(+)

Comments

Eric Auger Feb. 20, 2025, 6:13 p.m. UTC | #1 
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

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(HostIOMMUDeviceIOMMUFD *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[] = {
Duan, Zhenzhong Feb. 28, 2025, 8:24 a.m. UTC | #2 
>-----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?

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[] = {
Eric Auger March 6, 2025, 3:56 p.m. UTC | #3 
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[] = {
diff mbox series

Patch

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
+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(HostIOMMUDeviceIOMMUFD *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[] = {