Re: [PATCH 1/1] vfio/type1: Add subdev_ioasid callback to vfio_iommu_driver_ops

[Lu Baolu]

Hi Alex,

On 2020/11/17 3:56, Alex Williamson wrote:
> On Thu, 12 Nov 2020 10:24:07 +0800
> Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>  wrote:
>
> > Add API for getting the ioasid of a subdevice (vfio/mdev). This calls
> > into the backend IOMMU module to get the actual value or error number
> > if ioasid for subdevice is not supported. The physical device driver
> > implementations which rely on the vfio/mdev framework for mediated
> > device user level access could typically consume this interface like
> > below:
> >
> > 	struct device *dev = mdev_dev(mdev);
> > 	unsigned int pasid;
> > 	int ret;
> >
> > 	ret = vfio_subdev_ioasid(dev, &pasid);
> > 	if (ret < 0)
> > 		return ret;
> >
> >           /* Program device context with pasid value. */
> >           ....
>
> Seems like an overly specific callback.  We already export means for
> you to get a vfio_group, test that a device is an mdev, and get the
> iommu device from an mdev.  So you can already test whether a given
> device is an mdev with an iommu backing device that supports aux
> domains.  The only missing piece seems to be that you can't get the
> domain for a group in order to retrieve the pasid.  So why aren't we
> exporting a callback that given a vfio_group provides the iommu domain?

Make sense! Thanks for your guidance. :-)

So what we want to export in vfio.c is

struct iommu_domain *vfio_group_get_domain(struct vfio_group *group)

What the callers need to do are:

	unsigned int pasid;
	struct vfio_group *vfio_group;
	struct iommu_domain *iommu_domain;
	struct device *dev = mdev_dev(mdev);
	struct device *iommu_device = mdev_get_iommu_device(dev);

	if (!iommu_device ||
	    !iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
		return -EINVAL;

	vfio_group = vfio_group_get_external_user_from_dev(dev);
	if (IS_ERR_OR_NULL(vfio_group))
		return -EFAULT;

	iommu_domain = vfio_group_get_domain(vfio_group);
	if (IS_ERR_OR_NULL(iommu_domain)) {
		vfio_group_put_external_user(vfio_group);
		return -EFAULT;
	}

	pasid = iommu_aux_get_pasid(iommu_domain, iommu_device);
	if (pasid < 0) {
		vfio_group_put_external_user(vfio_group);
		return -EFAULT;
	}

	/* Program device context with pasid value. */
	...

	/* After use of this pasid */

	/* Clear the pasid value in device context */
	...

	vfio_group_put_external_user(vfio_group);

Do I understand your points correctly?

Best regards,
baolu