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

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

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
>