Re: [RFC PATCH v2 14/22] iommufd: Add TIO calls

[Xu Yilun <yilun.xu@xxxxxxxxxxxxxxx>]

> +int iommufd_vdevice_tsm_bind_ioctl(struct iommufd_ucmd *ucmd)
> +{
> +	struct iommu_vdevice_tsm_bind *cmd = ucmd->cmd;
> +	struct iommufd_viommu *viommu;
> +	struct iommufd_vdevice *vdev;
> +	struct iommufd_device *idev;
> +	struct tsm_tdi *tdi;
> +	int rc = 0;
> +
> +	viommu = iommufd_get_viommu(ucmd, cmd->viommu_id);

Why need user to input viommu_id? And why get viommu here?
The viommu is always available after vdevice is allocated, is it?

int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd)
{
	...

	vdev->viommu = viommu;
	refcount_inc(&viommu->obj.users);
	...
}

> +	if (IS_ERR(viommu))
> +		return PTR_ERR(viommu);
> +
> +	idev = iommufd_get_device(ucmd, cmd->dev_id);
> +	if (IS_ERR(idev)) {
> +		rc = PTR_ERR(idev);
> +		goto out_put_viommu;
> +	}
> +
> +	vdev = container_of(iommufd_get_object(ucmd->ictx, cmd->vdevice_id,
> +					       IOMMUFD_OBJ_VDEVICE),
> +			    struct iommufd_vdevice, obj);
> +	if (IS_ERR(idev)) {
                   ^
vdev?

> +		rc = PTR_ERR(idev);
> +		goto out_put_dev;
> +	}
> +
> +	tdi = tsm_tdi_get(idev->dev);

And do we still need dev_id for the struct device *? vdevice also has
this info.

int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd)
{
        ...
	vdev->dev = idev->dev;
	get_device(idev->dev);
        ...
}


> +	if (!tdi) {
> +		rc = -ENODEV;
> +		goto out_put_vdev;
> +	}
> +
> +	rc = tsm_tdi_bind(tdi, vdev->id, cmd->kvmfd);
> +	if (rc)
> +		goto out_put_tdi;
> +
> +	vdev->tsm_bound = true;
> +
> +	rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
> +out_put_tdi:
> +	tsm_tdi_put(tdi);
> +out_put_vdev:
> +	iommufd_put_object(ucmd->ictx, &vdev->obj);
> +out_put_dev:
> +	iommufd_put_object(ucmd->ictx, &idev->obj);
> +out_put_viommu:
> +	iommufd_put_object(ucmd->ictx, &viommu->obj);
> +	return rc;
> +}

Another concern is do we need an unbind ioctl? We don't bind on vdevice
create so it seems not symmetrical we only unbind on vdevice destroy.

Thanks,
Yilun