Re: [PATCH v8 02/12] iommu: Introduce a replace API for device pasid

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

On 2/26/25 19:40, Yi Liu wrote:
> +/**
> + * iommu_replace_device_pasid_handle - Replace the domain that a pasid
> + *                                     is attached to
> + * @domain: the new iommu domain
> + * @dev: the attached device.
> + * @pasid: the pasid of the device.
> + * @handle: the attach handle.
> + *
> + * This API allows the pasid to switch domains. The @pasid should have been
> + * attached. Otherwise, this fails.
> + * The pasid will keep the old configuration if replacement failed.
> + * Return 0 on success, or an error.
> + */
> +int iommu_replace_device_pasid_handle(struct iommu_domain *domain,
> +				      struct device *dev, ioasid_t pasid,
> +				      struct iommu_attach_handle *handle)
> +{
> +	/* Caller must be a probed driver on dev */
> +	struct iommu_group *group = dev->iommu_group;
> +	struct iommu_attach_handle *entry;
> +	struct iommu_domain *curr_domain;
> +	void *curr;
> +	int ret;
> +
> +	if (!group)
> +		return -ENODEV;
> +
> +	if (!domain->ops->set_dev_pasid)
> +		return -EOPNOTSUPP;
> +
> +	if (dev_iommu_ops(dev) != domain->owner ||
> +	    pasid == IOMMU_NO_PASID || !handle)
> +		return -EINVAL;
> +
> +	mutex_lock(&group->mutex);
> +	entry = iommu_make_pasid_array_entry(domain, handle);
> +	curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
> +			  XA_ZERO_ENTRY, GFP_KERNEL);
> +	if (xa_is_err(curr)) {
> +		ret = xa_err(curr);
> +		goto out_unlock;
> +	}
> +
> +	/* Not a replace case */
> +	if (!curr) {
> +		xa_release(&group->pasid_array, pasid);
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +
> +	curr_domain = pasid_array_entry_to_domain(curr);
> +	ret = 0;
> +
> +	if (curr_domain != domain) {
> +		ret = __iommu_set_group_pasid(domain, group,
> +					      pasid, curr_domain);
> +		if (ret)
> +			goto out_unlock;
> +	}
> +
> +	if (curr != entry) {
> +		/*
> +		 * The above xa_cmpxchg() reserved the memory, and the
> +		 * group->mutex is held, this cannot fail.
> +		 */
> +		WARN_ON(xa_is_err(xa_store(&group->pasid_array,
> +					   pasid, entry, GFP_KERNEL)));
> +	}
> +
> +out_unlock:
> +	mutex_unlock(&group->mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(iommu_replace_device_pasid_handle, "IOMMUFD_INTERNAL");

Perhaps you can compact your code with guard(muext). Something like
below:

         guard(mutex)(&group->mutex);
         entry = iommu_make_pasid_array_entry(domain, handle);
         curr = xa_cmpxchg(&group->pasid_array, pasid, NULL,
                           XA_ZERO_ENTRY, GFP_KERNEL);
         if (xa_is_err(curr))
                 return xa_err(curr);

         /* Not a replace case */
         if (!curr) {
                 xa_release(&group->pasid_array, pasid);
                 return -EINVAL;
         }

         curr_domain = pasid_array_entry_to_domain(curr);
         if (curr_domain == domain)
                 return 0;

         ret = __iommu_set_group_pasid(domain, group, pasid, curr_domain);
         if (ret)
                 return ret;

         /*
          * The above xa_cmpxchg() reserved the memory, and the
          * group->mutex is held, this cannot fail.
          */
         WARN_ON(xa_is_err(xa_store(&group->pasid_array, pasid, entry, 
GFP_KERNEL)));

         return 0;

Thanks,
baolu