Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
specific. Save them all in a new defined structure mirroring that vendor
IOMMU's structure, then get_cap() can query those information for
capability.

We can't use the vendor IOMMU's structure directly because they are in
linux/iommufd.h which breaks build on windows.

Suggested-by: Eric Auger <eric.auger@redhat.com>
Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/system/host_iommu_device.h | 12 ++++++++++++
 hw/vfio/iommufd.c                  | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 809cced4ba..908bfe32c7 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -15,6 +15,17 @@
 #include "qom/object.h"
 #include "qapi/error.h"
 
+/* This is mirror of struct iommu_hw_info_vtd */
+typedef struct Vtd_Caps {
+    uint32_t flags;
+    uint64_t cap_reg;
+    uint64_t ecap_reg;
+} Vtd_Caps;
+
+typedef union VendorCaps {
+    Vtd_Caps vtd;
+} VendorCaps;
+
 /**
  * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities.
  *
@@ -26,6 +37,7 @@
 typedef struct HostIOMMUDeviceCaps {
     uint32_t type;
     uint64_t hw_caps;
+    VendorCaps vendor_caps;
 } HostIOMMUDeviceCaps;
 
 #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index d661737c17..5c740222e5 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -834,6 +834,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     VFIODevice *vdev = opaque;
     HostIOMMUDeviceIOMMUFD *idev;
     HostIOMMUDeviceCaps *caps = &hiod->caps;
+    VendorCaps *vendor_caps = &caps->vendor_caps;
     enum iommu_hw_info_type type;
     union {
         struct iommu_hw_info_vtd vtd;
@@ -852,6 +853,17 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     caps->type = type;
     caps->hw_caps = hw_caps;
 
+    switch (type) {
+    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
+        vendor_caps->vtd.flags = data.vtd.flags;
+        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
+        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
+        break;
+    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
+    case IOMMU_HW_INFO_TYPE_NONE:
+        break;
+    }
+
     idev = HOST_IOMMU_DEVICE_IOMMUFD(hiod);
     idev->iommufd = vdev->iommufd;
     idev->devid = vdev->devid;
-- 
2.34.1

Hello Zhenzhong,

On 5/21/25 13:14, Zhenzhong Duan wrote:
> Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
> specific. Save them all in a new defined structure mirroring that vendor
> IOMMU's structure, then get_cap() can query those information for
> capability.
> 
> We can't use the vendor IOMMU's structure directly because they are in
> linux/iommufd.h which breaks build on windows.
> 
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/system/host_iommu_device.h | 12 ++++++++++++
>   hw/vfio/iommufd.c                  | 12 ++++++++++++
>   2 files changed, 24 insertions(+)
> 
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 809cced4ba..908bfe32c7 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -15,6 +15,17 @@
>   #include "qom/object.h"
>   #include "qapi/error.h"
>   
> +/* This is mirror of struct iommu_hw_info_vtd */
> +typedef struct Vtd_Caps {

please name the struct VtdCaps instead.


Thanks,

C.



> +    uint32_t flags;
> +    uint64_t cap_reg;
> +    uint64_t ecap_reg;
> +} Vtd_Caps;
> +
> +typedef union VendorCaps {
> +    Vtd_Caps vtd;
> +} VendorCaps;
> +
>   /**
>    * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities.
>    *
> @@ -26,6 +37,7 @@
>   typedef struct HostIOMMUDeviceCaps {
>       uint32_t type;
>       uint64_t hw_caps;
> +    VendorCaps vendor_caps;
>   } HostIOMMUDeviceCaps;
>   
>   #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index d661737c17..5c740222e5 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -834,6 +834,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>       VFIODevice *vdev = opaque;
>       HostIOMMUDeviceIOMMUFD *idev;
>       HostIOMMUDeviceCaps *caps = &hiod->caps;
> +    VendorCaps *vendor_caps = &caps->vendor_caps;
>       enum iommu_hw_info_type type;
>       union {
>           struct iommu_hw_info_vtd vtd;
> @@ -852,6 +853,17 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>       caps->type = type;
>       caps->hw_caps = hw_caps;
>   
> +    switch (type) {
> +    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
> +        vendor_caps->vtd.flags = data.vtd.flags;
> +        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
> +        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
> +        break;
> +    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
> +    case IOMMU_HW_INFO_TYPE_NONE:
> +        break;
> +    }
> +
>       idev = HOST_IOMMU_DEVICE_IOMMUFD(hiod);
>       idev->iommufd = vdev->iommufd;
>       idev->devid = vdev->devid;

Hi Cédric,

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH rfcv3 05/21] vfio/iommufd: Save vendor specific device info
>
>Hello Zhenzhong,
>
>On 5/21/25 13:14, Zhenzhong Duan wrote:
>> Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
>> specific. Save them all in a new defined structure mirroring that vendor
>> IOMMU's structure, then get_cap() can query those information for
>> capability.
>>
>> We can't use the vendor IOMMU's structure directly because they are in
>> linux/iommufd.h which breaks build on windows.
>>
>> Suggested-by: Eric Auger <eric.auger@redhat.com>
>> Suggested-by: Nicolin Chen <nicolinc@nvidia.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/system/host_iommu_device.h | 12 ++++++++++++
>>   hw/vfio/iommufd.c                  | 12 ++++++++++++
>>   2 files changed, 24 insertions(+)
>>
>> diff --git a/include/system/host_iommu_device.h
>b/include/system/host_iommu_device.h
>> index 809cced4ba..908bfe32c7 100644
>> --- a/include/system/host_iommu_device.h
>> +++ b/include/system/host_iommu_device.h
>> @@ -15,6 +15,17 @@
>>   #include "qom/object.h"
>>   #include "qapi/error.h"
>>
>> +/* This is mirror of struct iommu_hw_info_vtd */
>> +typedef struct Vtd_Caps {
>
>please name the struct VtdCaps instead.

Will do.

Thanks
Zhenzhong

On Wed, May 21, 2025 at 07:14:35PM +0800, Zhenzhong Duan wrote:
> @@ -852,6 +853,17 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>      caps->type = type;
>      caps->hw_caps = hw_caps;
>  
> +    switch (type) {
> +    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
> +        vendor_caps->vtd.flags = data.vtd.flags;
> +        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
> +        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
> +        break;
> +    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
> +    case IOMMU_HW_INFO_TYPE_NONE:

Should this be a part of hiod_iommufd_get_vendor_cap() in backends?

Thanks
Nicolin

>-----Original Message-----
>From: Nicolin Chen <nicolinc@nvidia.com>
>Subject: Re: [PATCH rfcv3 05/21] vfio/iommufd: Save vendor specific device info
>
>On Wed, May 21, 2025 at 07:14:35PM +0800, Zhenzhong Duan wrote:
>> @@ -852,6 +853,17 @@ static bool
>hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>      caps->type = type;
>>      caps->hw_caps = hw_caps;
>>
>> +    switch (type) {
>> +    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
>> +        vendor_caps->vtd.flags = data.vtd.flags;
>> +        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
>> +        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
>> +        break;
>> +    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
>> +    case IOMMU_HW_INFO_TYPE_NONE:
>
>Should this be a part of hiod_iommufd_get_vendor_cap() in backends?

Made following	adjustments which save raw data in VendorCaps,
let me know if it matches your thought.

diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
index 38070aff09..14cda4fdc3 100644
--- a/include/system/host_iommu_device.h
+++ b/include/system/host_iommu_device.h
@@ -14,16 +14,12 @@

 #include "qom/object.h"
 #include "qapi/error.h"
-
-/* This is mirror of struct iommu_hw_info_vtd */
-typedef struct Vtd_Caps {
-    uint32_t flags;
-    uint64_t cap_reg;
-    uint64_t ecap_reg;
-} Vtd_Caps;
+#ifdef CONFIG_LINUX
+#include "linux/iommufd.h"

 typedef union VendorCaps {
-    Vtd_Caps vtd;
+    struct iommu_hw_info_vtd vtd;
+    struct iommu_hw_info_arm_smmuv3 smmuv3;
 } VendorCaps;

 /**
@@ -43,6 +39,7 @@ typedef struct HostIOMMUDeviceCaps {
     uint64_t hw_caps;
     VendorCaps vendor_caps;
 } HostIOMMUDeviceCaps;
+#endif

 #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
 OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE)
@@ -54,7 +51,9 @@ struct HostIOMMUDevice {
     void *agent; /* pointer to agent device, ie. VFIO or VDPA device */
     PCIBus *aliased_bus;
     int aliased_devfn;
+#ifdef CONFIG_LINUX
     HostIOMMUDeviceCaps caps;
+#endif
 };

 /**
diff --git a/backends/iommufd.c b/backends/iommufd.c
index d91c1eb8b8..63209659f3 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -368,7 +368,7 @@ bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
 static int hiod_iommufd_get_vtd_cap(HostIOMMUDevice *hiod, int cap,
                                     Error **errp)
 {
-    Vtd_Caps *caps = &hiod->caps.vendor_caps.vtd;
+    struct iommu_hw_info_vtd *caps = &hiod->caps.vendor_caps.vtd;

     switch (cap) {
     case HOST_IOMMU_DEVICE_CAP_NESTING:
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 5c740222e5..fbf47cab09 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -836,15 +836,12 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     HostIOMMUDeviceCaps *caps = &hiod->caps;
     VendorCaps *vendor_caps = &caps->vendor_caps;
     enum iommu_hw_info_type type;
-    union {
-        struct iommu_hw_info_vtd vtd;
-    } data;
     uint64_t hw_caps;

     hiod->agent = opaque;

-    if (!iommufd_backend_get_device_info(vdev->iommufd, vdev->devid,
-                                         &type, &data, sizeof(data),
+    if (!iommufd_backend_get_device_info(vdev->iommufd, vdev->devid, &type,
+                                         vendor_caps, sizeof(*vendor_caps),
                                          &hw_caps, errp)) {
         return false;
     }
@@ -853,17 +850,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
     caps->type = type;
     caps->hw_caps = hw_caps;

-    switch (type) {
-    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
-        vendor_caps->vtd.flags = data.vtd.flags;
-        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
-        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
-        break;
-    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
-    case IOMMU_HW_INFO_TYPE_NONE:
-        break;
-    }
-
     idev = HOST_IOMMU_DEVICE_IOMMUFD(hiod);
     idev->iommufd = vdev->iommufd;
     idev->devid = vdev->devid;

On Thu, May 22, 2025 at 09:21:04AM +0000, Duan, Zhenzhong wrote:
> 
> 
> >-----Original Message-----
> >From: Nicolin Chen <nicolinc@nvidia.com>
> >Subject: Re: [PATCH rfcv3 05/21] vfio/iommufd: Save vendor specific device info
> >
> >On Wed, May 21, 2025 at 07:14:35PM +0800, Zhenzhong Duan wrote:
> >> @@ -852,6 +853,17 @@ static bool
> >hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
> >>      caps->type = type;
> >>      caps->hw_caps = hw_caps;
> >>
> >> +    switch (type) {
> >> +    case IOMMU_HW_INFO_TYPE_INTEL_VTD:
> >> +        vendor_caps->vtd.flags = data.vtd.flags;
> >> +        vendor_caps->vtd.cap_reg = data.vtd.cap_reg;
> >> +        vendor_caps->vtd.ecap_reg = data.vtd.ecap_reg;
> >> +        break;
> >> +    case IOMMU_HW_INFO_TYPE_ARM_SMMUV3:
> >> +    case IOMMU_HW_INFO_TYPE_NONE:
> >
> >Should this be a part of hiod_iommufd_get_vendor_cap() in backends?
> 
> Made following	adjustments which save raw data in VendorCaps,
> let me know if it matches your thought.

Yea, LGTM. Point is that we keep all vendor structure decoding
inside the backend, so VFIO wouldn't need to care about types
nor what's inside the data.

Thanks
Nicolin