Re: [PATCH rfcv2 02/20] vfio/iommufd: Add properties and handlers to TYPE_HOST_IOMMU_DEVICE_IOMMUFD

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

Hi Zhenzhong,


On 2/19/25 9:22 AM, Zhenzhong Duan wrote:
> New added properties include IOMMUFD handle, devid and hwpt_id.
a property generally has an other meaning in qemu (PROP*).

I would rather say you enhance HostIOMMUDeviceIOMMUFD object with 3 new
members, specific to the iommufd BE + 2 new class functions.


> IOMMUFD handle and devid are used to allocate/free ioas and hwpt.
> hwpt_id is used to re-attach IOMMUFD backed device to its default
> VFIO sub-system created hwpt, i.e., when vIOMMU is disabled by
> guest. These properties are initialized in .realize_late() handler.
realize_late does not exist yet
>
> New added handlers include [at|de]tach_hwpt. They are used to
> attach/detach hwpt. VFIO and VDPA have different way to attach
> and detach, so implementation will be in sub-class instead of
> HostIOMMUDeviceIOMMUFD.
this is tricky to follow ...
>
> Add two wrappers host_iommu_device_iommufd_[at|de]tach_hwpt to
> wrap the two handlers.
>
> This is a prerequisite patch for following ones.
would get rid of that sentence as it does not help much
>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  include/system/iommufd.h | 50 ++++++++++++++++++++++++++++++++++++++++
>  backends/iommufd.c       | 22 ++++++++++++++++++
>  2 files changed, 72 insertions(+)
>
> diff --git a/include/system/iommufd.h b/include/system/iommufd.h
> index 5d02e9d148..a871601df5 100644
> --- a/include/system/iommufd.h
> +++ b/include/system/iommufd.h
> @@ -66,4 +66,54 @@ int iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t hwpt_id,
>                                       uint32_t *entry_num, void *data_ptr);
>  
>  #define TYPE_HOST_IOMMU_DEVICE_IOMMUFD TYPE_HOST_IOMMU_DEVICE "-iommufd"
> +OBJECT_DECLARE_TYPE(HostIOMMUDeviceIOMMUFD, HostIOMMUDeviceIOMMUFDClass,
> +                    HOST_IOMMU_DEVICE_IOMMUFD)
> +
> +/* Abstract of host IOMMU device with iommufd backend */
specialization/overload of the host IOMMU device for the iommufd BE?
> +struct HostIOMMUDeviceIOMMUFD {
> +    HostIOMMUDevice parent_obj;
> +
> +    IOMMUFDBackend *iommufd;
> +    uint32_t devid;
> +    uint32_t hwpt_id;
> +};
> +
> +struct HostIOMMUDeviceIOMMUFDClass {
> +    HostIOMMUDeviceClass parent_class;
> +
> +    /**
> +     * @attach_hwpt: attach host IOMMU device to IOMMUFD hardware page table.
> +     * VFIO and VDPA device can have different implementation.
> +     *
> +     * Mandatory callback.
> +     *
> +     * @idev: host IOMMU device backed by IOMMUFD backend.
> +     *
> +     * @hwpt_id: ID of IOMMUFD hardware page table.
> +     *
> +     * @errp: pass an Error out when attachment fails.
> +     *
> +     * Returns: true on success, false on failure.
> +     */
> +    bool (*attach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, uint32_t hwpt_id,
> +                        Error **errp);
> +    /**
> +     * @detach_hwpt: detach host IOMMU device from IOMMUFD hardware page table.
> +     * VFIO and VDPA device can have different implementation.
> +     *
> +     * Mandatory callback.
> +     *
> +     * @idev: host IOMMU device backed by IOMMUFD backend.
> +     *
> +     * @errp: pass an Error out when attachment fails.
> +     *
> +     * Returns: true on success, false on failure.
> +     */
> +    bool (*detach_hwpt)(HostIOMMUDeviceIOMMUFD *idev, Error **errp);
> +};
> +
> +bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           uint32_t hwpt_id, Error **errp);
> +bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           Error **errp);
>  #endif
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index fc32aad5cb..574f330c27 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -341,6 +341,26 @@ int iommufd_backend_invalidate_cache(IOMMUFDBackend *be, uint32_t hwpt_id,
>      return ret;
>  }
>  
> +bool host_iommu_device_iommufd_attach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           uint32_t hwpt_id, Error **errp)
> +{
> +    HostIOMMUDeviceIOMMUFDClass *idevc =
> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
> +
> +    g_assert(idevc->attach_hwpt);
> +    return idevc->attach_hwpt(idev, hwpt_id, errp);
> +}
> +
> +bool host_iommu_device_iommufd_detach_hwpt(HostIOMMUDeviceIOMMUFD *idev,
> +                                           Error **errp)
> +{
> +    HostIOMMUDeviceIOMMUFDClass *idevc =
> +        HOST_IOMMU_DEVICE_IOMMUFD_GET_CLASS(idev);
> +
> +    g_assert(idevc->detach_hwpt);
> +    return idevc->detach_hwpt(idev, errp);
> +}
> +
>  static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>  {
>      HostIOMMUDeviceCaps *caps = &hiod->caps;
> @@ -379,6 +399,8 @@ static const TypeInfo types[] = {
>      }, {
>          .name = TYPE_HOST_IOMMU_DEVICE_IOMMUFD,
>          .parent = TYPE_HOST_IOMMU_DEVICE,
> +        .instance_size = sizeof(HostIOMMUDeviceIOMMUFD),
> +        .class_size = sizeof(HostIOMMUDeviceIOMMUFDClass),
>          .class_init = hiod_iommufd_class_init,
>          .abstract = true,
>      }
Thanks

Eric