On 2024/4/16 11:01, Duan, Zhenzhong wrote:
-----Original Message-----
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Subject: [PATCH v2 02/12] iommu: Introduce a replace API for device pasid
Provide a high-level API to allow replacements of one domain with
another for specific pasid of a device. This is similar to
iommu_group_replace_domain() and it is expected to be used only by
IOMMUFD.
Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
drivers/iommu/iommu-priv.h | 3 ++
drivers/iommu/iommu.c | 92
+++++++++++++++++++++++++++++++++++---
2 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index 5f731d994803..0949c02cee93 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -20,6 +20,9 @@ static inline const struct iommu_ops
*dev_iommu_ops(struct device *dev)
int iommu_group_replace_domain(struct iommu_group *group,
struct iommu_domain *new_domain);
+int iommu_replace_device_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid);
+
int iommu_device_register_bus(struct iommu_device *iommu,
const struct iommu_ops *ops,
const struct bus_type *bus,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 701b02a118db..343683e646e0 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -3315,14 +3315,15 @@ bool
iommu_group_dma_owner_claimed(struct iommu_group *group)
EXPORT_SYMBOL_GPL(iommu_group_dma_owner_claimed);
static int __iommu_set_group_pasid(struct iommu_domain *domain,
- struct iommu_group *group, ioasid_t pasid)
+ struct iommu_group *group, ioasid_t pasid,
+ struct iommu_domain *old)
{
struct group_device *device, *last_gdev;
int ret;
for_each_group_device(group, device) {
ret = domain->ops->set_dev_pasid(domain, device->dev,
- pasid, NULL);
+ pasid, old);
if (ret)
goto err_revert;
}
@@ -3332,11 +3333,34 @@ static int __iommu_set_group_pasid(struct
iommu_domain *domain,
err_revert:
last_gdev = device;
for_each_group_device(group, device) {
- const struct iommu_ops *ops = dev_iommu_ops(device-
dev);
+ /*
+ * If no old domain, just undo all the devices/pasid that
+ * have attached to the new domain.
+ */
+ if (!old) {
+ const struct iommu_ops *ops =
+ dev_iommu_ops(device->dev);
+
+ if (device == last_gdev)
Maybe this check can be moved to beginning of the for loop,
+ break;
+ ops = dev_iommu_ops(device->dev);
+ ops->remove_dev_pasid(device->dev, pasid, domain);
+ continue;
+ }
- if (device == last_gdev)
+ /*
+ * Rollback the devices/pasid that have attached to the new
+ * domain. And it is a driver bug to fail attaching with a
+ * previously good domain.
+ */
+ if (device == last_gdev) {
then this check can be removed.
+ WARN_ON(old->ops->set_dev_pasid(old, device-
dev,
+ pasid, NULL));
Is this call necessary? last_gdev is the first device failed.
It is since we claim replacement failure would leave the pasid to be
attached with the prior attached domain. That's why old is checked
in the above. If no old domain, it would exit the loop if it is the
first failed device. Nothing need to be done for the failed device.
Thanks
Zhenzhong
break;
- ops->remove_dev_pasid(device->dev, pasid, domain);
+ }
+
+ WARN_ON(old->ops->set_dev_pasid(old, device->dev,
+ pasid, domain));
}
return ret;
}
@@ -3395,7 +3419,7 @@ int iommu_attach_device_pasid(struct
iommu_domain *domain,
goto out_unlock;
}
- ret = __iommu_set_group_pasid(domain, group, pasid);
+ ret = __iommu_set_group_pasid(domain, group, pasid, NULL);
if (ret)
xa_erase(&group->pasid_array, pasid);
out_unlock:
@@ -3404,6 +3428,62 @@ int iommu_attach_device_pasid(struct
iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_attach_device_pasid);
+/**
+ * iommu_replace_device_pasid - replace the domain that a pasid is
attached to
+ * @domain: new IOMMU domain to replace with
+ * @dev: the physical device
+ * @pasid: pasid that will be attached to the new domain
+ *
+ * This API allows the pasid to switch domains. Return 0 on success, or an
+ * error. The pasid will roll back to use the old domain if failure. The
+ * caller could call iommu_detach_device_pasid() before free the old
domain
+ * in order to avoid use-after-free case.
+ */
+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)
+ return -EINVAL;
+
+ if (!domain->ops->set_dev_pasid)
+ return -EOPNOTSUPP;
+
+ if (!group)
+ return -ENODEV;
+
+ if (!dev_has_iommu(dev) || dev_iommu_ops(dev) != domain-
owner)
+ return -EINVAL;
+
+ mutex_lock(&group->mutex);
+ 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(xa_err(xa_store(&group->pasid_array, pasid,
+ curr, GFP_KERNEL)));
+out_unlock:
+ mutex_unlock(&group->mutex);
+ return ret;
+}
+EXPORT_SYMBOL_NS_GPL(iommu_replace_device_pasid,
IOMMUFD_INTERNAL);
+
/*
* iommu_detach_device_pasid() - Detach the domain from pasid of device
* @domain: the iommu domain.
--
2.34.1
--
Regards,
Yi Liu
