Skip to content

Commit b025dea

Browse files
yiliu1765joergroedel
authored andcommitted
iommu: Undo pasid attachment only for the devices that have succeeded
There is no error handling now in __iommu_set_group_pasid(), it relies on its caller to loop all the devices to undo the pasid attachment. This is not self-contained and has drawbacks. It would result in unnecessary remove_dev_pasid() calls on the devices that have not been attached to the new domain. But the remove_dev_pasid() callback would get the new domain from the group->pasid_array. So for such devices, the iommu driver won't find the attachment under the domain, hence unable to do cleanup. This may not be a real problem today. But it depends on the implementation of the underlying iommu driver. e.g. the intel iommu driver would warn for such devices. Such warnings are unnecessary. To solve the above problem, it is necessary to handle the error within __iommu_set_group_pasid(). It only loops the devices that have attached to the new domain, and undo it. Fixes: 1660370 ("iommu: Add attach/detach_dev_pasid iommu interfaces") Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> Signed-off-by: Yi Liu <yi.l.liu@intel.com> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com> Link: https://lore.kernel.org/r/20240328122958.83332-2-yi.l.liu@intel.com Signed-off-by: Joerg Roedel <jroedel@suse.de>
1 parent fec50db commit b025dea

File tree

1 file changed

+15
-6
lines changed

1 file changed

+15
-6
lines changed

drivers/iommu/iommu.c

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3317,15 +3317,26 @@ EXPORT_SYMBOL_GPL(iommu_group_dma_owner_claimed);
33173317
static int __iommu_set_group_pasid(struct iommu_domain *domain,
33183318
struct iommu_group *group, ioasid_t pasid)
33193319
{
3320-
struct group_device *device;
3321-
int ret = 0;
3320+
struct group_device *device, *last_gdev;
3321+
int ret;
33223322

33233323
for_each_group_device(group, device) {
33243324
ret = domain->ops->set_dev_pasid(domain, device->dev, pasid);
33253325
if (ret)
3326-
break;
3326+
goto err_revert;
33273327
}
33283328

3329+
return 0;
3330+
3331+
err_revert:
3332+
last_gdev = device;
3333+
for_each_group_device(group, device) {
3334+
const struct iommu_ops *ops = dev_iommu_ops(device->dev);
3335+
3336+
if (device == last_gdev)
3337+
break;
3338+
ops->remove_dev_pasid(device->dev, pasid);
3339+
}
33293340
return ret;
33303341
}
33313342

@@ -3383,10 +3394,8 @@ int iommu_attach_device_pasid(struct iommu_domain *domain,
33833394
}
33843395

33853396
ret = __iommu_set_group_pasid(domain, group, pasid);
3386-
if (ret) {
3387-
__iommu_remove_group_pasid(group, pasid);
3397+
if (ret)
33883398
xa_erase(&group->pasid_array, pasid);
3389-
}
33903399
out_unlock:
33913400
mutex_unlock(&group->mutex);
33923401
return ret;

0 commit comments

Comments
 (0)