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

[Lu Baolu <baolu.lu@linux.intel.com>]

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(-)
>