Re: [PATCH v4 25/27] backends/iommufd: Add a callback helper to retrieve PASID support

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

Hi Shameer,

On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> Subsequent patch will make use of this to add a PASID CAP for assigned devices.
>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  backends/iommufd.c                 |  9 +++++++++
>  include/system/host_iommu_device.h | 12 ++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 023e67bc46..0ff46a5747 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -523,6 +523,14 @@ bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
>      return idevc->detach_hwpt(idev, errp);
>  }
>  
> +static uint8_t hiod_iommufd_get_pasid(HostIOMMUDevice *hiod, uint64_t *hw_caps)
I would call it hiod_iommufd_get_max_pasid_log2 instead.
> +{
> +    HostIOMMUDeviceCaps *caps = &hiod->caps;
> +
> +    *hw_caps = caps->hw_caps;
> +    return caps->max_pasid_log2;
> +}
> +
>  static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>  {
>      HostIOMMUDeviceCaps *caps = &hiod->caps;
> @@ -543,6 +551,7 @@ static void hiod_iommufd_class_init(ObjectClass *oc, const void *data)
>      HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc);
>  
>      hioc->get_cap = hiod_iommufd_get_cap;
> +    hioc->get_pasid = hiod_iommufd_get_pasid;
same for the cb name
>  };
>  
>  static const TypeInfo types[] = {
> diff --git a/include/system/host_iommu_device.h b/include/system/host_iommu_device.h
> index c6a2a3899a..3773c54977 100644
> --- a/include/system/host_iommu_device.h
> +++ b/include/system/host_iommu_device.h
> @@ -115,6 +115,18 @@ struct HostIOMMUDeviceClass {
>       * @hiod: handle to the host IOMMU device
>       */
>      uint64_t (*get_page_size_mask)(HostIOMMUDevice *hiod);
> +    /**
> +     * @get_pasid: Get PASID support information along this
> +     *             @hiod Host IOMMU device
> +     * Optional callback. If not implemented, PASID not supported
> +     *
> +     * @hiod: handle to the host IOMMU device
> +     *
> +     * @out_hw_caps: Output the generic iommu capability info which includes
> +     *               device PASID CAP info

I would precise this corresponds to kernel bitmap of enum iommufd_hw_capabilities

> +     * Returns the width of PASIDs. Zero means no PASID support
> +     */
> +     uint8_t (*get_pasid)(HostIOMMUDevice *hiod, uint64_t *out_hw_caps);
ditto. But actually even get_pasid_max_pasid_log2 is not very well
suited as it returns both iommu_hw_capabilities and max_pasid_log2.
and this can now be easily mixed up with the other get_cap() callback.

Why can't we use hiod_iommufd_get_cap() cb instead that would return an
uint64_t instead. we could easily add new cap value for max_pasid_log2
and output_hw_capabilities.

Or do I miss something?

Thanks

Eric

>  };
>  
>  /*