Re: [PATCH v3 0/4] iommu aux-domain APIs extensions

Lu Baolu
2020-07-23T13:56:09Z

Hi Joerg and Alex,

Any comments for this series?

Just check to see whether we could make it for v5.9. The first aux-
domain capable device driver has been posted [1].

[1]
https://lore.kernel.org/linux-pci/159534667974.28840.2045034360240786644.stgit@djiang5-desk3.ch.intel.com/

Best regards,
baolu

On 2020/7/14 13:56, Lu Baolu wrote:
> This series aims to extend the IOMMU aux-domain API set so that it
> could be more friendly to vfio/mdev usage. The interactions between
> vfio/mdev and iommu during mdev creation and passthr are:
>
> 1. Create a group for mdev with iommu_group_alloc();
> 2. Add the device to the group with
>
> group = iommu_group_alloc();
> if (IS_ERR(group))
> return PTR_ERR(group);
>
> ret = iommu_group_add_device(group, &mdev->dev);
> if (!ret)
> dev_info(&mdev->dev, "MDEV: group_id = %d\n",
> iommu_group_id(group));
>
> 3. Allocate an aux-domain with iommu_domain_alloc();
> 4. Attach the aux-domain to the iommu_group.
>
> iommu_group_for_each_dev {
> if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
> return iommu_aux_attach_device(domain, iommu_device);
> else
> return iommu_attach_device(domain, iommu_device);
> }
>
> where, iommu_device is the aux-domain-capable device. The mdev's in
> the group are all derived from it.
>
> In the whole process, an iommu group was allocated for the mdev and an
> iommu domain was attached to the group, but the group->domain leaves
> NULL. As the result, iommu_get_domain_for_dev() (or other similar
> interfaces) doesn't work anymore.
>
> The iommu_get_domain_for_dev() is a necessary interface for device
> drivers that want to support vfio/mdev based aux-domain. For example,
>
> unsigned long pasid;
> struct iommu_domain *domain;
> struct device *dev = mdev_dev(mdev);
> struct device *iommu_device = vfio_mdev_get_iommu_device(dev);
>
> domain = iommu_get_domain_for_dev(dev);
> if (!domain)
> return -ENODEV;
>
> pasid = iommu_aux_get_pasid(domain, iommu_device);
> if (pasid <= 0)
> return -EINVAL;
>
> /* Program the device context */
> ....
>
> We tried to address this by extending iommu_aux_at(de)tach_device() so that
> the users could pass in an optional device pointer (for example vfio/mdev).
> (v2 of this series)
>
> https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu.lu@linux.intel.com/
>
> But that will cause a lock issue as group->mutex has been applied in
> iommu_group_for_each_dev(), but has to be reapplied again in the
> iommu_aux_attach_device().
>
> This version tries to address this by introducing two new APIs into the
> aux-domain API set:
>
> /**
> * iommu_aux_attach_group - attach an aux-domain to an iommu_group which
> * contains sub-devices (for example mdevs)
> * derived from @dev.
> * @domain: an aux-domain;
> * @group: an iommu_group which contains sub-devices derived from @dev;
> * @dev: the physical device which supports IOMMU_DEV_FEAT_AUX.
> *
> * Returns 0 on success, or an error value.
> */
> int iommu_aux_attach_group(struct iommu_domain *domain,
> struct iommu_group *group, struct device *dev)
>
> /**
> * iommu_aux_detach_group - detach an aux-domain from an iommu_group
> *
> * @domain: an aux-domain;
> * @group: an iommu_group which contains sub-devices derived from @dev;
> * @dev: the physical device which supports IOMMU_DEV_FEAT_AUX.
> *
> * @domain must have been attached to @group via
> * iommu_aux_attach_group().
> */
> void iommu_aux_detach_group(struct iommu_domain *domain,
> struct iommu_group *group, struct device *dev)
>
> This version is evolved according to feedbacks from Robin(v1) and
> Alex(v2). Your comments are very appreciated.
>
> Best regards,
> baolu
>
> ---
> Change log:
> - v1->v2:
> - https://lore.kernel.org/linux-iommu/20200627031532.28046-1-baolu.lu@linux.intel.com/
> - Suggested by Robin.
>
> - v2->v3:
> - https://lore.kernel.org/linux-iommu/20200707013957.23672-1-baolu.lu@linux.intel.com/
> - Suggested by Alex
>
> Lu Baolu (4):
> iommu: Check IOMMU_DEV_FEAT_AUX feature in aux api's
> iommu: Add iommu_aux_at(de)tach_group()
> iommu: Add iommu_aux_get_domain_for_dev()
> vfio/type1: Use iommu_aux_at(de)tach_group() APIs
>
> drivers/iommu/iommu.c | 92 ++++++++++++++++++++++++++++++---
> drivers/vfio/vfio_iommu_type1.c | 44 +++-------------
> include/linux/iommu.h | 24 +++++++++
> 3 files changed, 116 insertions(+), 44 deletions(-)