Xu Yilun <yilun...@linux.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

Reply via email to