lkml.org 
[lkml]   [2026]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
SubjectRe: [PATCH V4 03/10] iommufd: Allow binding to a noiommu device
From
On 4/15/26 05:14, Jacob Pan wrote:
> From: Jason Gunthorpe <jgg@nvidia.com>
>
> Allow iommufd to bind devices without an IOMMU (noiommu mode) by creating
> a dummy IOMMU group for such devices and skipping hwpt operations.
>
> This enables noiommu devices to operate through the same iommufd API as IOMMU-
> capable devices.
>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
>
> ---
> v4:
> - handle partially initialized idev w/o igroup (Sashiko)
> v3:
> - handle new error cases iommufd_device_bind. (Mostafa)
> ---
> drivers/iommu/iommufd/device.c | 132 +++++++++++++++++++++++----------
> 1 file changed, 93 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 2f30ea069f66..0283ac39be55 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -23,6 +23,11 @@ struct iommufd_attach {
> struct xarray device_array;
> };
>
> +static bool is_vfio_noiommu(struct iommufd_device *idev)

perhaps iommufd_device_is_noiommu().

> +{
> + return !device_iommu_mapped(idev->dev) || !idev->dev->iommu;
> +}
> +
> static void iommufd_group_release(struct kref *kref)
> {
> struct iommufd_group *igroup =
> @@ -30,9 +35,11 @@ static void iommufd_group_release(struct kref *kref)
>
> WARN_ON(!xa_empty(&igroup->pasid_attach));
>
> - xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
> - NULL, GFP_KERNEL);
> - iommu_group_put(igroup->group);
> + if (igroup->group) {
> + xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group),
> + igroup, NULL, GFP_KERNEL);
> + iommu_group_put(igroup->group);
> + }
> mutex_destroy(&igroup->lock);
> kfree(igroup);
> }
> @@ -204,32 +211,19 @@ void iommufd_device_destroy(struct iommufd_object *obj)
> struct iommufd_device *idev =
> container_of(obj, struct iommufd_device, obj);
>
> - iommu_device_release_dma_owner(idev->dev);
> + if (!idev->igroup)
> + return;
> + if (!is_vfio_noiommu(idev))
> + iommu_device_release_dma_owner(idev->dev);
> iommufd_put_group(idev->igroup);
> if (!iommufd_selftest_is_mock_dev(idev->dev))
> iommufd_ctx_put(idev->ictx);
> }
>
> -/**
> - * iommufd_device_bind - Bind a physical device to an iommu fd
> - * @ictx: iommufd file descriptor
> - * @dev: Pointer to a physical device struct
> - * @id: Output ID number to return to userspace for this device
> - *
> - * A successful bind establishes an ownership over the device and returns
> - * struct iommufd_device pointer, otherwise returns error pointer.
> - *
> - * A driver using this API must set driver_managed_dma and must not touch
> - * the device until this routine succeeds and establishes ownership.
> - *
> - * Binding a PCI device places the entire RID under iommufd control.
> - *
> - * The caller must undo this with iommufd_device_unbind()
> - */
> -struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> - struct device *dev, u32 *id)
> +static int iommufd_bind_iommu(struct iommufd_device *idev)
> {
> - struct iommufd_device *idev;
> + struct iommufd_ctx *ictx = idev->ictx;
> + struct device *dev = idev->dev;
> struct iommufd_group *igroup;
> int rc;
>
> @@ -238,11 +232,11 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> * to restore cache coherency.
> */
> if (!device_iommu_capable(dev, IOMMU_CAP_CACHE_COHERENCY))
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> - igroup = iommufd_get_group(ictx, dev);
> + igroup = iommufd_get_group(idev->ictx, dev);

no need as you already have a local ictx.

> if (IS_ERR(igroup))
> - return ERR_CAST(igroup);
> + return PTR_ERR(igroup);
>
> /*
> * For historical compat with VFIO the insecure interrupt path is
> @@ -268,21 +262,69 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> if (rc)
> goto out_group_put;
>
> + /* igroup refcount moves into iommufd_device */
> + idev->igroup = igroup;
> + return 0;
> +
> +out_group_put:
> + iommufd_put_group(igroup);
> + return rc;
> +}
> +
> +/**
> + * iommufd_device_bind - Bind a physical device to an iommu fd
> + * @ictx: iommufd file descriptor
> + * @dev: Pointer to a physical device struct
> + * @id: Output ID number to return to userspace for this device
> + *
> + * A successful bind establishes an ownership over the device and returns
> + * struct iommufd_device pointer, otherwise returns error pointer.
> + *
> + * A driver using this API must set driver_managed_dma and must not touch
> + * the device until this routine succeeds and establishes ownership.
> + *
> + * Binding a PCI device places the entire RID under iommufd control.
> + *
> + * The caller must undo this with iommufd_device_unbind()
> + */
> +struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> + struct device *dev, u32 *id)
> +{
> + struct iommufd_device *idev;
> + int rc;
> +
> idev = iommufd_object_alloc(ictx, idev, IOMMUFD_OBJ_DEVICE);
> - if (IS_ERR(idev)) {
> - rc = PTR_ERR(idev);
> - goto out_release_owner;
> - }
> + if (IS_ERR(idev))
> + return idev;
> +
> idev->ictx = ictx;
> - if (!iommufd_selftest_is_mock_dev(dev))
> - iommufd_ctx_get(ictx);
> idev->dev = dev;
> idev->enforce_cache_coherency =
> device_iommu_capable(dev, IOMMU_CAP_ENFORCE_CACHE_COHERENCY);

Functioanlly, it is harmless since device_iommu_capable() returns false
for noiommu device. But conceptually, it is nice to move it to
iommufd_bind_iommu().

> +
> + if (!is_vfio_noiommu(idev)) {
> + rc = iommufd_bind_iommu(idev);
> + if (rc)
> + goto err_out;
> + } else {
> + struct iommufd_group *igroup;
> +
> + /*
> + * Create a dummy igroup, lots of stuff expects ths igroup to be
> + * present, but a NULL igroup->group is OK
> + */
> + igroup = iommufd_alloc_group(ictx, NULL);
> + if (IS_ERR(igroup)) {
> + rc = PTR_ERR(igroup);
> + goto err_out;
> + }
> + idev->igroup = igroup;
> + }
> +
> + if (!iommufd_selftest_is_mock_dev(dev))
> + iommufd_ctx_get(ictx);
> /* The calling driver is a user until iommufd_device_unbind() */
> refcount_inc(&idev->obj.users);
> - /* igroup refcount moves into iommufd_device */
> - idev->igroup = igroup;
>
> /*
> * If the caller fails after this success it must call
> @@ -294,11 +336,14 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> *id = idev->obj.id;
> return idev;
>
> -out_release_owner:
> - iommu_device_release_dma_owner(dev);
> -out_group_put:
> - iommufd_put_group(igroup);
> +err_out:
> + /*
> + * Be careful that iommufd_device_destroy() can handle partial
> + * initialization.
> + */

no native speaker. This is a bit strange to me. I think the comment
wants to highligh that iommufd_device_destroy() can handle partial
initialization. But the above comment seems to warn something...
how about below?

/*
* iommufd_device_destroy() handles partially initialized idev,
* so iommufd_object_abort_and_destroy() is safe to call here.
*/

> + iommufd_object_abort_and_destroy(ictx, &idev->obj);
> return ERR_PTR(rc);
> +
> }
> EXPORT_SYMBOL_NS_GPL(iommufd_device_bind, "IOMMUFD");
>
> @@ -512,6 +557,9 @@ static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
> struct iommufd_attach_handle *handle;
> int rc;
>
> + if (is_vfio_noiommu(idev))
> + return 0;
> +
> if (!iommufd_hwpt_compatible_device(hwpt, idev))
> return -EINVAL;
>
> @@ -559,6 +607,9 @@ static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,
> {
> struct iommufd_attach_handle *handle;
>
> + if (is_vfio_noiommu(idev))
> + return;
> +
> handle = iommufd_device_get_attach_handle(idev, pasid);
> if (pasid == IOMMU_NO_PASID)
> iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
> @@ -577,6 +628,9 @@ static int iommufd_hwpt_replace_device(struct iommufd_device *idev,
> struct iommufd_attach_handle *handle, *old_handle;
> int rc;
>
> + if (is_vfio_noiommu(idev))
> + return 0;
> +
> if (!iommufd_hwpt_compatible_device(hwpt, idev))
> return -EINVAL;
>
> @@ -652,7 +706,7 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
> goto err_release_devid;
> }
>
> - if (attach_resv) {
> + if (attach_resv && !is_vfio_noiommu(idev)) {

how about the iommufd_hw_pagetable_detach() path? Does it also need to
check noiommu device?

> rc = iommufd_device_attach_reserved_iova(idev, hwpt_paging);
> if (rc)
> goto err_release_devid;

Regards,
Yi Liu

\
 
 \ /
  Last update: 2026-04-28 10:02    [W:0.261 / U:15.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and my Meterkast|Read the blog