Messages in this thread | | | Date | Thu, 24 Feb 2022 13:21:24 +0800 | Subject | Re: [PATCH v6 01/11] iommu: Add dma ownership management interfaces | From | Lu Baolu <> |
| |
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
| |