> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Sent: Thursday, September 12, 2024 9:13 PM
>
> +/**
> + * iommu_replace_device_pasid - 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. Return 0 on success, or an
> + * error. The pasid will keep the old configuration if replacement failed.
> + * This is supposed to be used by iommufd, and iommufd can guarantee
> that
> + * both iommu_attach_device_pasid() and iommu_replace_device_pasid()
> would
> + * pass in a valid @handle.
this function assumes handle is always valid. So above comment
makes it clear that iommufd is the only user and it will always
pass in a valid handle.
but the code in iommu_attach_device_pasid() allows handle to
be NULL. Then that comment is meaningless for it.
Also following patches are built on iommufd always passing in
a valid handle as it's required by pasid operations but there is
no detail explanation why it's mandatory or any alternative
option exists. More explanation is welcomed.
> + */
> +int iommu_replace_device_pasid(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 *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 || !handle)
> + return -EINVAL;
> +
> + handle->domain = domain;
> +
> + mutex_lock(&group->mutex);
> + /*
> + * The iommu_attach_handle of the pasid becomes inconsistent with
> the
> + * actual handle per the below operation. The concurrent PRI path
> will
> + * deliver the PRQs per the new handle, this does not have a function
> + * impact. The PRI path would eventually become consistent when
s/function/functional/
> the
> + * replacement is done.
> + */
> + curr = (struct iommu_attach_handle *)xa_store(&group->pasid_array,
> + pasid, handle,
> + GFP_KERNEL);
Could you elaborate why the PRI path will eventually becomes
consistent with this path?
