[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <yq5awm9ujouz.fsf@kernel.org>
Date: Mon, 02 Jun 2025 18:13:16 +0530
From: Aneesh Kumar K.V <aneesh.kumar@...nel.org>
To: Xu Yilun <yilun.xu@...ux.intel.com>, kvm@...r.kernel.org,
sumit.semwal@...aro.org, christian.koenig@....com,
pbonzini@...hat.com, seanjc@...gle.com, alex.williamson@...hat.com,
jgg@...dia.com, dan.j.williams@...el.com, aik@....com,
linux-coco@...ts.linux.dev
Cc: dri-devel@...ts.freedesktop.org, linux-media@...r.kernel.org,
linaro-mm-sig@...ts.linaro.org, vivek.kasireddy@...el.com,
yilun.xu@...el.com, yilun.xu@...ux.intel.com,
linux-kernel@...r.kernel.org, lukas@...ner.de, yan.y.zhao@...el.com,
daniel.vetter@...ll.ch, leon@...nel.org, baolu.lu@...ux.intel.com,
zhenzhong.duan@...el.com, tao1.su@...el.com,
linux-pci@...r.kernel.org, zhiw@...dia.com, simona.vetter@...ll.ch,
shameerali.kolothum.thodi@...wei.com, iommu@...ts.linux.dev,
kevin.tian@...el.com
Subject: Re: [RFC PATCH 17/30] iommufd/device: Add TSM Bind/Unbind for TIO
support
Xu Yilun <yilun.xu@...ux.intel.com> writes:
....
> +/**
> + * iommufd_device_tsm_bind - Move a device to TSM Bind state
> + * @idev: device to attach
> + * @vdev_id: Input a IOMMUFD_OBJ_VDEVICE
> + *
> + * This configures for device Confidential Computing(CC), and moves the device
> + * to the TSM Bind state. Once this completes the device is locked down (TDISP
> + * CONFIG_LOCKED or RUN), waiting for guest's attestation.
> + *
> + * This function is undone by calling iommufd_device_tsm_unbind().
> + */
> +int iommufd_device_tsm_bind(struct iommufd_device *idev, u32 vdevice_id)
> +{
> + struct iommufd_vdevice *vdev;
> + int rc;
> +
> + if (!dev_is_pci(idev->dev))
> + return -ENODEV;
> +
> + vdev = container_of(iommufd_get_object(idev->ictx, vdevice_id, IOMMUFD_OBJ_VDEVICE),
> + struct iommufd_vdevice, obj);
> + if (IS_ERR(vdev))
> + return PTR_ERR(vdev);
> +
> + if (vdev->dev != idev->dev) {
> + rc = -EINVAL;
> + goto out_put_vdev;
> + }
> +
> + mutex_lock(&idev->igroup->lock);
> + if (idev->vdev) {
> + rc = -EEXIST;
> + goto out_unlock;
> + }
> +
> + rc = iommufd_vdevice_tsm_bind(vdev);
> + if (rc)
> + goto out_unlock;
> +
> + idev->vdev = vdev;
> + refcount_inc(&vdev->obj.users);
> + mutex_unlock(&idev->igroup->lock);
> +
> + /*
> + * Pairs with iommufd_device_tsm_unbind() - catches caller bugs attempting
> + * to destroy a bound device.
> + */
> + refcount_inc(&idev->obj.users);
>
Do we really need this refcount_inc? As I understand it, the objects
aren't being pinned directly. Instead, the reference count seems to be
used more as a way to establish an object hierarchy, ensuring that
objects are freed in the correct order.
In vfio_pci_core_close_device(), you’re decrementing the reference, and
on the iommufd side, we’re covered because the VFIO bind operation takes
a file reference (fget)—so iommufd_fops_release() won’t be called
prematurely.
Wouldn’t it be simpler to skip the reference count increment altogether
and just call tsm_unbind in the virtual device’s destroy callback?
(iommufd_vdevice_destroy())
> + goto out_put_vdev;
> +
> +out_unlock:
> + mutex_unlock(&idev->igroup->lock);
> +out_put_vdev:
> + iommufd_put_object(idev->ictx, &vdev->obj);
> + return rc;
> +}
> +EXPORT_SYMBOL_NS_GPL(iommufd_device_tsm_bind, "IOMMUFD");
-aneesh
Powered by blists - more mailing lists