linux-kernel - Re: [PATCH 1/1] vfio/type1: Add subdev_ioasid callback to vfio_iommu_driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 17 Nov 2020 22:39:55 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     baolu.lu@...ux.intel.com, Cornelia Huck <cohuck@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Robin Murphy <robin.murphy@....com>,
        Jean-Philippe Brucker <jean-philippe@...aro.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Liu Yi L <yi.l.liu@...el.com>, Zeng Xin <xin.zeng@...el.com>,
        iommu@...ts.linux-foundation.org, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org
Subject: Re: [PATCH 1/1] vfio/type1: Add subdev_ioasid callback to
 vfio_iommu_driver_ops

Hi Alex,

On 2020/11/17 3:56, Alex Williamson wrote:
> On Thu, 12 Nov 2020 10:24:07 +0800
> Lu Baolu<baolu.lu@...ux.intel.com>  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