> From: Liu, Yi L <yi.l....@intel.com>
> Sent: Friday, June 28, 2024 5:06 PM
>
> @@ -3289,7 +3290,20 @@ static int __iommu_set_group_pasid(struct
> iommu_domain *domain,
>
> if (device == last_gdev)
> break;
> - ops->remove_dev_pasid(device->dev, pasid, domain);
> + /* If no old domain, undo the succeeded devices/pasid */
> + if (!old) {
> + ops->remove_dev_pasid(device->dev, pasid, domain);
> + continue;
> + }
> +
> + /*
> + * Rollback the succeeded devices/pasid to the old domain.
> + * And it is a driver bug to fail attaching with a previously
> + * good domain.
> + */
> + if (WARN_ON(old->ops->set_dev_pasid(old, device->dev,
> + pasid, domain)))
> + ops->remove_dev_pasid(device->dev, pasid, domain);
I wonder whether @remove_dev_pasid() can be replaced by having
blocking_domain support @set_dev_pasid?
> +int iommu_replace_device_pasid(struct iommu_domain *domain,
> + struct device *dev, ioasid_t pasid)
> +{
> + /* Caller must be a probed driver on dev */
> + struct iommu_group *group = dev->iommu_group;
> + void *curr;
> + int ret;
> +
> + if (!domain->ops->set_dev_pasid)
> + return -EOPNOTSUPP;
> +
> + if (!group)
> + return -ENODEV;
> +
> + if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain-
> >owner ||
> + pasid == IOMMU_NO_PASID)
> + return -EINVAL;
> +
> + mutex_lock(&group->mutex);
> + /*
> + * The recorded domain is inconsistent with the domain pasid is
> + * actually attached until pasid is attached to the new domain.
> + * This has race condition with the paths that do not hold
> + * group->mutex. E.g. the Page Request forwarding.
> + */
so?
> + curr = xa_store(&group->pasid_array, pasid, domain, GFP_KERNEL);
> + if (!curr) {
> + xa_erase(&group->pasid_array, pasid);
> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + ret = xa_err(curr);
> + if (ret)
> + goto out_unlock;
> +
> + if (curr == domain)
> + goto out_unlock;
> +
> + ret = __iommu_set_group_pasid(domain, group, pasid, curr);
> + if (ret)
> + WARN_ON(domain != xa_store(&group->pasid_array, pasid,
> + curr, GFP_KERNEL));
above can follow Jason's suggestion to iommu_group_replace_domain ()
in Baolu's series, i.e. doing a xa_reserve() first.
