Hi Shameer,

On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <[email protected]>

I would suggest to be more explicit in the commit title like:
Convey input type in iommufd_backend_get_device_info()

>
> The updated IOMMUFD uAPI introduces the ability for userspace to request
> a specific hardware info data type via IOMMU_GET_HW_INFO. Update
> iommufd_backend_get_device_info() to set IOMMU_HW_INFO_FLAG_INPUT_TYPE
> when a non-zero type is supplied, and adjust all callers to pass an
> explicitly initialised type value.
>
> Signed-off-by: Nicolin Chen <[email protected]>
> Signed-off-by: Shameer Kolothum <[email protected]>
> ---
>  backends/iommufd.c    | 7 +++++++
>  hw/arm/smmuv3-accel.c | 2 +-
>  hw/vfio/iommufd.c     | 6 ++----
>  3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 633aecd525..938c8fe669 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -386,16 +386,23 @@ bool iommufd_backend_get_dirty_bitmap(IOMMUFDBackend 
> *be,
>      return true;
>  }
>  
> +/*
> + * @type can carry a desired HW info type defined in the uapi headers. If 
> caller
> + * doesn't have one, indicating it wants the default type, then @type should 
> be
> + * zeroed (i.e. IOMMU_HW_INFO_TYPE_DEFAULT).
the kernel uapi doc is a bit ambiguous as it is said:
 * @flags: Must be 0
while a flag value does exist. 
> + */
>  bool iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
>                                       uint32_t *type, void *data, uint32_t 
> len,
>                                       uint64_t *caps, uint8_t *max_pasid_log2,
>                                       Error **errp)
>  {
>      struct iommu_hw_info info = {
> +        .flags = (*type) ? IOMMU_HW_INFO_FLAG_INPUT_TYPE : 0,
>          .size = sizeof(info),
>          .dev_id = devid,
>          .data_len = len,
>          .data_uptr = (uintptr_t)data,
> +        .in_data_type = *type,
>      };
>  
>      if (ioctl(be->fd, IOMMU_GET_HW_INFO, &info)) {
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index d320c62b04..300c35ccb5 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -115,7 +115,7 @@ smmuv3_accel_hw_compatible(SMMUv3State *s, 
> HostIOMMUDeviceIOMMUFD *idev,
>                             Error **errp)
>  {
>      struct iommu_hw_info_arm_smmuv3 info;
> -    uint32_t data_type;
> +    uint32_t data_type = 0;
>      uint64_t caps;
>  
>      if (!iommufd_backend_get_device_info(idev->iommufd, idev->devid, 
> &data_type,
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index bbe944d7cc..670bdfc53b 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -306,7 +306,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice 
> *vbasedev,
>      ERRP_GUARD();
>      IOMMUFDBackend *iommufd = vbasedev->iommufd;
>      VFIOContainer *bcontainer = VFIO_IOMMU(container);
> -    uint32_t type, flags = 0;
> +    uint32_t type = 0, flags = 0;
>      uint64_t hw_caps;
>      VFIOIOASHwpt *hwpt;
>      uint32_t hwpt_id;
> @@ -631,8 +631,6 @@ skip_ioas_alloc:
>      bcontainer->initialized = true;
>  
>  found_container:
> -    vbasedev->cpr.ioas_id = container->ioas_id;
> -
indeed looks unrelated to that patch
>      ret = ioctl(devfd, VFIO_DEVICE_GET_INFO, &dev_info);
>      if (ret) {
>          error_setg_errno(errp, errno, "error getting device info");
> @@ -889,7 +887,7 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice 
> *hiod, void *opaque,
>      HostIOMMUDeviceIOMMUFD *idev;
>      HostIOMMUDeviceCaps *caps = &hiod->caps;
>      VendorCaps *vendor_caps = &caps->vendor_caps;
> -    enum iommu_hw_info_type type;
> +    enum iommu_hw_info_type type = 0;
>      uint8_t max_pasid_log2;
>      uint64_t hw_caps;
>  
Besides
Reviewed-by: Eric Auger <[email protected]>
Thanks

Eric


Reply via email to