Re: [PATCH v12 07/17] iommu: Try to allocate blocking domain when probing device

Baolu Lu
2022-09-01T10:44:38Z

On 2022/8/31 22:10, Jason Gunthorpe wrote:
> On Wed, Aug 31, 2022 at 09:49:44AM +0800, Baolu Lu wrote:
>>> Maybe all of this is just the good reason to go to a simple
>>> device->ops->remove_dev_pasid() callback and forget about blocking
>>> domain here.
>>
>> Do you mean rolling back to what we did in v10?
>
> Yeah, but it shouldn't be a domain_op, removing a pasid is a device op
>
> Just
>
> remove_dev_pasid(struct device *dev, ioasid_t pasid)

It's clear now. Thanks!

How about below iommu_attach/detach_device_pasid() code?

By the way, how about naming it "block_dev_pasid(dev, pasid)"?

+static int __iommu_set_group_pasid(struct iommu_domain *domain,
+ struct iommu_group *group, ioasid_t pasid)
+{
+ struct group_device *device;
+ int ret = 0;
+
+ list_for_each_entry(device, &group->devices, list) {
+ ret = domain->ops->set_dev_pasid(domain, device->dev, pasid);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+static void __iommu_remove_group_pasid(struct iommu_group *group,
+ ioasid_t pasid)
+{
+ struct group_device *device;
+ const struct iommu_ops *ops;
+
+ list_for_each_entry(device, &group->devices, list) {
+ ops = dev_iommu_ops(device->dev);
+ ops->remove_dev_pasid(device->dev, pasid);
+ }
+}
+
+/*
+ * iommu_attach_device_pasid() - Attach a domain to pasid of device
+ * @domain: the iommu domain.
+ * @dev: the attached device.
+ * @pasid: the pasid of the device.
+ *
+ * Return: 0 on success, or an error.
+ */
+int iommu_attach_device_pasid(struct iommu_domain *domain,
+ struct device *dev, ioasid_t pasid)
+{
+ struct iommu_group *group;
+ void *curr;
+ int ret;
+
+ if (!domain->ops->set_dev_pasid)
+ return -EOPNOTSUPP;
+
+ group = iommu_group_get(dev);
+ if (!group)
+ return -ENODEV;
+
+ mutex_lock(&group->mutex);
+ curr = xa_cmpxchg(&group->pasid_array, pasid, NULL, domain, GFP_KERNEL);
+ if (curr) {
+ ret = xa_err(curr) ? : -EBUSY;
+ goto out_unlock;
+ }
+
+ ret = __iommu_set_group_pasid(domain, group, pasid);
+ if (ret) {
+ __iommu_remove_group_pasid(group, pasid);
+ xa_erase(&group->pasid_array, pasid);
+ }
+out_unlock:
+ mutex_unlock(&group->mutex);
+ iommu_group_put(group);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_attach_device_pasid);
+
+/*
+ * iommu_detach_device_pasid() - Detach the domain from pasid of device
+ * @domain: the iommu domain.
+ * @dev: the attached device.
+ * @pasid: the pasid of the device.
+ *
+ * The @domain must have been attached to @pasid of the @dev with
+ * iommu_attach_device_pasid().
+ */
+void iommu_detach_device_pasid(struct iommu_domain *domain, struct
device *dev,
+ ioasid_t pasid)
+{
+ struct iommu_group *group = iommu_group_get(dev);
+
+ mutex_lock(&group->mutex);
+ __iommu_remove_group_pasid(group, pasid);
+ WARN_ON(xa_erase(&group->pasid_array, pasid) != domain);
+ mutex_unlock(&group->mutex);
+
+ iommu_group_put(group);
+}
+EXPORT_SYMBOL_GPL(iommu_detach_device_pasid);

+ * @remove_dev_pasid: Remove any translation configurations of a specific
+ * pasid, so that any DMA transactions with this pasid
+ * will be blocked by the hardware.
* @pgsize_bitmap: bitmap of all possible supported page sizes
* @owner: Driver module providing these ops
*/
@@ -256,6 +259,7 @@ struct iommu_ops {
struct iommu_page_response *msg);

int (*def_domain_type)(struct device *dev);
+ void (*remove_dev_pasid)(struct device *dev, ioasid_t pasid);

Best regards,
baolu