| Message ID | 20240212135643.5858-2-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/iommufd: IOMMUFD Dirty Tracking | expand |
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
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 >
>-----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
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
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(+)