Re: [PATCH v6 01/11] iommu: Add dma ownership management interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/24/22 2:00 AM, Robin Murphy wrote:
On 2022-02-18 00:55, Lu Baolu wrote:
[...]
+/**
+ * iommu_group_claim_dma_owner() - Set DMA ownership of a group
+ * @group: The group.
+ * @owner: Caller specified pointer. Used for exclusive ownership.
+ *
+ * This is to support backward compatibility for vfio which manages
+ * the dma ownership in iommu_group level. New invocations on this
+ * interface should be prohibited.
+ */
+int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner)
+{
+    int ret = 0;
+
+    mutex_lock(&group->mutex);
+    if (group->owner_cnt) {

To clarify the comment buried in the other thread, I really think we should just unconditionally flag the error here...

+        if (group->owner != owner) {
+            ret = -EPERM;
+            goto unlock_out;
+        }
+    } else {
+        if (group->domain && group->domain != group->default_domain) {
+            ret = -EBUSY;
+            goto unlock_out;
+        }
+
+        group->owner = owner;
+        if (group->domain)
+            __iommu_detach_group(group->domain, group);
+    }
+
+    group->owner_cnt++;
+unlock_out:
+    mutex_unlock(&group->mutex);
+
+    return ret;
+}
+EXPORT_SYMBOL_GPL(iommu_group_claim_dma_owner);
+
+/**
+ * iommu_group_release_dma_owner() - Release DMA ownership of a group
+ * @group: The group.
+ *
+ * Release the DMA ownership claimed by iommu_group_claim_dma_owner().
+ */
+void iommu_group_release_dma_owner(struct iommu_group *group)
+{
+    mutex_lock(&group->mutex);
+    if (WARN_ON(!group->owner_cnt || !group->owner))
+        goto unlock_out;
+
+    if (--group->owner_cnt > 0)
+        goto unlock_out;

...and equivalently just set owner_cnt directly to 0 here. I don't see a realistic use-case for any driver to claim the same group more than once, and allowing it in the API just feels like opening up various potential corners for things to get out of sync.

Yeah! Both make sense to me. I will also drop the owner token in the API
as it's unnecessary anymore after the change.

I think that's the only significant concern I have left with the series as a whole - you can consider my other grumbles non-blocking :)

Thank you and very appreciated for your time!

Best regards,
baolu



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux