RE: [PATCH v4 01/10] iommu: Introduce a replace API for device pasid

From: Liu, Yi L yi.l.liu@intel.com 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?