On 2024/9/30 15:38, Tian, Kevin wrote:
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.
Actually, this is why I added the above comment. iommufd can ensure
it would pass valid handle to both iommu_attach_device_pasid() and
iommu_replace_device_pasid(), and iommu_replace_device_pasid() is
only used by iommufd, so iommu_replace_device_pasid() can assume
all the pasids have a valid handle stored in the pasid_array.
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.
There is more detail about it in the below link, but is it necessary
to add them in the comment as well, or is it ok to add more explanation
in commit message?
https://lore.kernel.org/linux-iommu/0bf383b7-ed96-49ca-b1da-d1fff48e161a@xxxxxxxxx/
+ */
+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/
got it.
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?
Because the handle stored in pasid_array would be consistent with the
configuration of pasid. So the PRI would be forwarded to the correct
domain.
--
Regards,
Yi Liu
