On Tue, 14 Jul 2020 13:57:03 +0800 Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> wrote: > Replace iommu_aux_at(de)tach_device() with iommu_aux_at(de)tach_group(). > It also saves the IOMMU_DEV_FEAT_AUX-capable physcail device in the > vfio_group data structure so that it could be reused in other places. > > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx> > --- > drivers/vfio/vfio_iommu_type1.c | 44 ++++++--------------------------- > 1 file changed, 7 insertions(+), 37 deletions(-) > > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c > index 5e556ac9102a..f8812e68de77 100644 > --- a/drivers/vfio/vfio_iommu_type1.c > +++ b/drivers/vfio/vfio_iommu_type1.c > @@ -100,6 +100,7 @@ struct vfio_dma { > struct vfio_group { > struct iommu_group *iommu_group; > struct list_head next; > + struct device *iommu_device; > bool mdev_group; /* An mdev group */ > bool pinned_page_dirty_scope; > }; > @@ -1627,45 +1628,13 @@ static struct device *vfio_mdev_get_iommu_device(struct device *dev) > return NULL; > } > > -static int vfio_mdev_attach_domain(struct device *dev, void *data) > -{ > - struct iommu_domain *domain = data; > - struct device *iommu_device; > - > - iommu_device = vfio_mdev_get_iommu_device(dev); > - if (iommu_device) { > - 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); > - } > - > - return -EINVAL; > -} > - > -static int vfio_mdev_detach_domain(struct device *dev, void *data) > -{ > - struct iommu_domain *domain = data; > - struct device *iommu_device; > - > - iommu_device = vfio_mdev_get_iommu_device(dev); > - if (iommu_device) { > - if (iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX)) > - iommu_aux_detach_device(domain, iommu_device); > - else > - iommu_detach_device(domain, iommu_device); > - } > - > - return 0; > -} > - > static int vfio_iommu_attach_group(struct vfio_domain *domain, > struct vfio_group *group) > { > if (group->mdev_group) > - return iommu_group_for_each_dev(group->iommu_group, > - domain->domain, > - vfio_mdev_attach_domain); > + return iommu_aux_attach_group(domain->domain, > + group->iommu_group, > + group->iommu_device); No, we previously iterated all devices in the group and used the aux interface only when we have an iommu_device supporting aux. If we simply assume an mdev group only uses an aux domain we break existing users, ex. SR-IOV VF backed mdevs. Thanks, Alex > else > return iommu_attach_group(domain->domain, group->iommu_group); > } > @@ -1674,8 +1643,8 @@ static void vfio_iommu_detach_group(struct vfio_domain *domain, > struct vfio_group *group) > { > if (group->mdev_group) > - iommu_group_for_each_dev(group->iommu_group, domain->domain, > - vfio_mdev_detach_domain); > + iommu_aux_detach_group(domain->domain, group->iommu_group, > + group->iommu_device); > else > iommu_detach_group(domain->domain, group->iommu_group); > } > @@ -2007,6 +1976,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, > return 0; > } > > + group->iommu_device = iommu_device; > bus = iommu_device->bus; > } >