Re: [PATCH v2 4/4] vfio/iommufd: Save vendor specific device info

[Eric Auger <eric.auger@redhat.com>]

Hi Zhenzhong,

On 5/30/25 11:35 AM, Zhenzhong Duan wrote:
> Some device information returned by ioctl(IOMMU_GET_HW_INFO) are vendor
> specific. Save them as raw data in a union supporting different vendors,
> then vendor IOMMU can query the raw data with its fixed format for
> capability directly.
>
> Because IOMMU_GET_HW_INFO is only supported in linux, so declare those
> capability related structures with CONFIG_LINUX.
>
> 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 | 11 +++++++++++
>  hw/vfio/iommufd.c                  |  8 +++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index 809cced4ba..10fccc10be 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -14,6 +14,13 @@
>  
>  #include "qom/object.h"
>  #include "qapi/error.h"
> +#ifdef CONFIG_LINUX
> +#include "linux/iommufd.h"
> +
> +typedef union VendorCaps {
> +    struct iommu_hw_info_vtd vtd;
> +    struct iommu_hw_info_arm_smmuv3 smmuv3;
> +} VendorCaps;
>  
>  /**
>   * struct HostIOMMUDeviceCaps - Define host IOMMU device capabilities.
> @@ -26,7 +33,9 @@
>  typedef struct HostIOMMUDeviceCaps {
>      uint32_t type;
>      uint64_t hw_caps;
> +    VendorCaps vendor_caps;
missing the doc comment update for new field vendor_caps

Otherwise

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric


>  } HostIOMMUDeviceCaps;
> +#endif
>  
>  #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
>  OBJECT_DECLARE_TYPE(HostIOMMUDevice, HostIOMMUDeviceClass, HOST_IOMMU_DEVICE)
> @@ -38,7 +47,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/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index d661737c17..fbf47cab09 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -834,16 +834,14 @@ 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;
> -    } 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;
>      }