vfio/iommufd: IOMMUFD Dirty Tracking

[PATCH RFCv2 1/8] backends/iommufd: Introduce helper function iommufd_device_get_hw_capabilities()

Posted by Joao Martins 1 year, 12 months ago

The new helper will fetch vendor agnostic IOMMU capabilities supported
both by hardware and software. Right now it is only iommu dirty
tracking.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 backends/iommufd.c       | 25 +++++++++++++++++++++++++
 include/sysemu/iommufd.h |  2 ++
 2 files changed, 27 insertions(+)

diff --git a/backends/iommufd.c b/backends/iommufd.c
index d92791bba935..8486894f1b3f 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -237,3 +237,28 @@ void iommufd_device_init(IOMMUFDDevice *idev)
     host_iommu_base_device_init(&idev->base, HID_IOMMUFD,
                                 sizeof(IOMMUFDDevice));
 }
+
+int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t *caps,
+                                       Error **errp)
+{
+    struct iommu_hw_info info = {
+        .size = sizeof(info),
+        .flags = 0,
+        .dev_id = idev->devid,
+        .data_len = 0,
+        .__reserved = 0,
+        .data_uptr = 0,
+        .out_capabilities = 0,
+    };
+    int ret;
+
+    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
+    if (ret) {
+        error_setg_errno(errp, errno,
+                         "Failed to get hardware info capabilities");
+    } else {
+        *caps = info.out_capabilities;
+    }
+
+    return ret;
+}
diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
index c3f346976039..4afe97307dbe 100644
--- a/include/sysemu/iommufd.h
+++ b/include/sysemu/iommufd.h
@@ -47,4 +47,6 @@ typedef struct IOMMUFDDevice {
 } IOMMUFDDevice;
 
 void iommufd_device_init(IOMMUFDDevice *idev);
+int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t *caps,
+                                       Error **errp);
 #endif
-- 
2.39.3

RE: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper function iommufd_device_get_hw_capabilities()

Posted by Duan, Zhenzhong 1 year, 11 months ago

Hi Joao,

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper function
>iommufd_device_get_hw_capabilities()
>
>The new helper will fetch vendor agnostic IOMMU capabilities supported
>both by hardware and software. Right now it is only iommu dirty
>tracking.
>
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>---
> backends/iommufd.c       | 25 +++++++++++++++++++++++++
> include/sysemu/iommufd.h |  2 ++
> 2 files changed, 27 insertions(+)
>
>diff --git a/backends/iommufd.c b/backends/iommufd.c
>index d92791bba935..8486894f1b3f 100644
>--- a/backends/iommufd.c
>+++ b/backends/iommufd.c
>@@ -237,3 +237,28 @@ void iommufd_device_init(IOMMUFDDevice *idev)
>     host_iommu_base_device_init(&idev->base, HID_IOMMUFD,
>                                 sizeof(IOMMUFDDevice));
> }
>+
>+int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t
>*caps,
>+                                       Error **errp)
>+{
>+    struct iommu_hw_info info = {
>+        .size = sizeof(info),
>+        .flags = 0,
>+        .dev_id = idev->devid,
>+        .data_len = 0,
>+        .__reserved = 0,
>+        .data_uptr = 0,
>+        .out_capabilities = 0,
>+    };
>+    int ret;
>+
>+    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
>+    if (ret) {
>+        error_setg_errno(errp, errno,
>+                         "Failed to get hardware info capabilities");
>+    } else {
>+        *caps = info.out_capabilities;
>+    }
>+
>+    return ret;
>+}

This helper is redundant with https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00031.html
We have to get other elements in info in nesting series, so mind using that helper
Instead to avoid redundancy? I can move that patch ahead for your usage.

Thanks
Zhenzhong

>diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
>index c3f346976039..4afe97307dbe 100644
>--- a/include/sysemu/iommufd.h
>+++ b/include/sysemu/iommufd.h
>@@ -47,4 +47,6 @@ typedef struct IOMMUFDDevice {
> } IOMMUFDDevice;
>
> void iommufd_device_init(IOMMUFDDevice *idev);
>+int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t
>*caps,
>+                                       Error **errp);
> #endif
>--
>2.39.3

Re: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper function iommufd_device_get_hw_capabilities()

Posted by Joao Martins 1 year, 11 months ago

On 26/02/2024 07:29, Duan, Zhenzhong wrote:
> Hi Joao,
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper function
>> iommufd_device_get_hw_capabilities()
>>
>> The new helper will fetch vendor agnostic IOMMU capabilities supported
>> both by hardware and software. Right now it is only iommu dirty
>> tracking.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> backends/iommufd.c       | 25 +++++++++++++++++++++++++
>> include/sysemu/iommufd.h |  2 ++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index d92791bba935..8486894f1b3f 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -237,3 +237,28 @@ void iommufd_device_init(IOMMUFDDevice *idev)
>>     host_iommu_base_device_init(&idev->base, HID_IOMMUFD,
>>                                 sizeof(IOMMUFDDevice));
>> }
>> +
>> +int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t
>> *caps,
>> +                                       Error **errp)
>> +{
>> +    struct iommu_hw_info info = {
>> +        .size = sizeof(info),
>> +        .flags = 0,
>> +        .dev_id = idev->devid,
>> +        .data_len = 0,
>> +        .__reserved = 0,
>> +        .data_uptr = 0,
>> +        .out_capabilities = 0,
>> +    };
>> +    int ret;
>> +
>> +    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
>> +    if (ret) {
>> +        error_setg_errno(errp, errno,
>> +                         "Failed to get hardware info capabilities");
>> +    } else {
>> +        *caps = info.out_capabilities;
>> +    }
>> +
>> +    return ret;
>> +}
> 
> This helper is redundant with https://lists.gnu.org/archive/html/qemu-devel/2024-02/msg00031.html
> We have to get other elements in info in nesting series, so mind using that helper
> Instead to avoid redundancy? I can move that patch ahead for your usage.
> 

Sure.

Btw, speaking of which. You series could actually be split into two. One for
iommufd device abstraction part (patch 00-09) and another for the nesting bits
(10-18). FWIW this series here as submitted was actually just placing it on top
of the iommufd device abstraction

I am still planning on adding this same helper, probably just calling into
yours. Mostly because I disregard the data/data-size as I am only interested in
vendor agnostic capabilities.

> Thanks
> Zhenzhong
> 
>> diff --git a/include/sysemu/iommufd.h b/include/sysemu/iommufd.h
>> index c3f346976039..4afe97307dbe 100644
>> --- a/include/sysemu/iommufd.h
>> +++ b/include/sysemu/iommufd.h
>> @@ -47,4 +47,6 @@ typedef struct IOMMUFDDevice {
>> } IOMMUFDDevice;
>>
>> void iommufd_device_init(IOMMUFDDevice *idev);
>> +int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev, uint64_t
>> *caps,
>> +                                       Error **errp);
>> #endif
>> --
>> 2.39.3
>

RE: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper function iommufd_device_get_hw_capabilities()

Posted by Duan, Zhenzhong 1 year, 11 months ago


>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: Re: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper
>function iommufd_device_get_hw_capabilities()
>
>On 26/02/2024 07:29, Duan, Zhenzhong wrote:
>> Hi Joao,
>>
>>> -----Original Message-----
>>> From: Joao Martins <joao.m.martins@oracle.com>
>>> Subject: [PATCH RFCv2 1/8] backends/iommufd: Introduce helper
>function
>>> iommufd_device_get_hw_capabilities()
>>>
>>> The new helper will fetch vendor agnostic IOMMU capabilities supported
>>> both by hardware and software. Right now it is only iommu dirty
>>> tracking.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> backends/iommufd.c       | 25 +++++++++++++++++++++++++
>>> include/sysemu/iommufd.h |  2 ++
>>> 2 files changed, 27 insertions(+)
>>>
>>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>>> index d92791bba935..8486894f1b3f 100644
>>> --- a/backends/iommufd.c
>>> +++ b/backends/iommufd.c
>>> @@ -237,3 +237,28 @@ void iommufd_device_init(IOMMUFDDevice
>*idev)
>>>     host_iommu_base_device_init(&idev->base, HID_IOMMUFD,
>>>                                 sizeof(IOMMUFDDevice));
>>> }
>>> +
>>> +int iommufd_device_get_hw_capabilities(IOMMUFDDevice *idev,
>uint64_t
>>> *caps,
>>> +                                       Error **errp)
>>> +{
>>> +    struct iommu_hw_info info = {
>>> +        .size = sizeof(info),
>>> +        .flags = 0,
>>> +        .dev_id = idev->devid,
>>> +        .data_len = 0,
>>> +        .__reserved = 0,
>>> +        .data_uptr = 0,
>>> +        .out_capabilities = 0,
>>> +    };
>>> +    int ret;
>>> +
>>> +    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
>>> +    if (ret) {
>>> +        error_setg_errno(errp, errno,
>>> +                         "Failed to get hardware info capabilities");
>>> +    } else {
>>> +        *caps = info.out_capabilities;
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>
>> This helper is redundant with https://lists.gnu.org/archive/html/qemu-
>devel/2024-02/msg00031.html
>> We have to get other elements in info in nesting series, so mind using that
>helper
>> Instead to avoid redundancy? I can move that patch ahead for your usage.
>>
>
>Sure.
>
>Btw, speaking of which. You series could actually be split into two. One for
>iommufd device abstraction part (patch 00-09) and another for the nesting
>bits
>(10-18). FWIW this series here as submitted was actually just placing it on
>top
>of the iommufd device abstraction

I see, will split in next version.

>
>I am still planning on adding this same helper, probably just calling into
>yours. Mostly because I disregard the data/data-size as I am only interested
>in
>vendor agnostic capabilities.

Sounds good.

Thanks
Zhenzhong

[PATCH RFCv2 1/8] backends/iommufd: Introduce helper function iommufd_device_get_hw_capabilities()
[PATCH RFCv2 2/8] vfio/iommufd: Introduce auto domain creation
[PATCH RFCv2 3/8] vfio/iommufd: Probe and request hwpt dirty tracking capability
[PATCH RFCv2 4/8] vfio/iommufd: Implement VFIOIOMMUClass::set_dirty_tracking support
[PATCH RFCv2 5/8] vfio/iommufd: Implement VFIOIOMMUClass::query_dirty_bitmap support
[PATCH RFCv2 6/8] backends/iommufd: Add ability to disable hugepages
[PATCH RFCv2 7/8] vfio/migration: Don't block migration device dirty tracking is unsupported
[PATCH RFCv2 8/8] vfio/common: Allow disabling device dirty page tracking