| Message ID | 2-v4-0de2f6c78ed0+9d1-iommufd_jgg@nvidia.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | IOMMUFD Generic interface | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Tuesday, November 8, 2022 8:49 AM > +static int __iommu_take_dma_ownership(struct iommu_group *group, void > *owner) > +{ > + int ret; > + > + if (WARN_ON(!owner)) > + return -EINVAL; move to iommu_device_claim_dma_owner(). just like how it's checked in the group version. > + > + if ((group->domain && group->domain != group->default_domain) || > + !xa_empty(&group->pasid_array)) > + return -EBUSY; the check of pasid_array is a new addition in this version. it's probably worthy a comment here. otherwise, Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Hi, On 11/8/22 01:48, Jason Gunthorpe wrote: > From: Lu Baolu <baolu.lu@linux.intel.com> > > These complement the group interfaces used by VFIO and are for use by > iommufd. The main difference is that multiple devices in the same group > can all share the ownership by passing the same ownership pointer. > > Move the common code into shared functions. > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/iommu.c | 124 +++++++++++++++++++++++++++++++++--------- > include/linux/iommu.h | 12 ++++ > 2 files changed, 110 insertions(+), 26 deletions(-) > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 6ca377f4fbf9e9..4cb14e44e40f83 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -3108,41 +3108,52 @@ static int __iommu_group_alloc_blocking_domain(struct iommu_group *group) > return 0; > } > > +static int __iommu_take_dma_ownership(struct iommu_group *group, void *owner) > +{ > + int ret; > + > + if (WARN_ON(!owner)) > + return -EINVAL; > + > + if ((group->domain && group->domain != group->default_domain) || > + !xa_empty(&group->pasid_array)) > + return -EBUSY; > + > + ret = __iommu_group_alloc_blocking_domain(group); > + if (ret) > + return ret; > + ret = __iommu_group_set_domain(group, group->blocking_domain); > + if (ret) > + return ret; > + > + group->owner = owner; > + group->owner_cnt++; > + return 0; > +} > + > /** > * 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. > + * 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. Only a single owner may exist for a group. > */ > int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner) > { > int ret = 0; > > + if (WARN_ON(!owner)) > + return -EINVAL; > + > mutex_lock(&group->mutex); > if (group->owner_cnt) { > ret = -EPERM; > goto unlock_out; > - } else { > - if ((group->domain && group->domain != group->default_domain) || > - !xa_empty(&group->pasid_array)) { > - ret = -EBUSY; > - goto unlock_out; > - } > - > - ret = __iommu_group_alloc_blocking_domain(group); > - if (ret) > - goto unlock_out; > - > - ret = __iommu_group_set_domain(group, group->blocking_domain); > - if (ret) > - goto unlock_out; > - group->owner = owner; > } > > - group->owner_cnt++; > + ret = __iommu_take_dma_ownership(group, owner); > unlock_out: > mutex_unlock(&group->mutex); > > @@ -3151,30 +3162,91 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner) > EXPORT_SYMBOL_GPL(iommu_group_claim_dma_owner); > > /** > - * iommu_group_release_dma_owner() - Release DMA ownership of a group > - * @group: The group. > + * iommu_device_claim_dma_owner() - Set DMA ownership of a device > + * @dev: The device. > + * @owner: Caller specified pointer. Used for exclusive ownership. > * > - * Release the DMA ownership claimed by iommu_group_claim_dma_owner(). > + * Claim the DMA ownership of a device. Multiple devices in the same group may > + * concurrently claim ownership if they present the same owner value. Returns 0 > + * on success and error code on failure > */ > -void iommu_group_release_dma_owner(struct iommu_group *group) > +int iommu_device_claim_dma_owner(struct device *dev, void *owner) > { > - int ret; > + struct iommu_group *group = iommu_group_get(dev); > + int ret = 0; > + > + if (!group) > + return -ENODEV; > > mutex_lock(&group->mutex); > + if (group->owner_cnt) { > + if (group->owner != owner) { > + ret = -EPERM; > + goto unlock_out; > + } > + group->owner_cnt++; > + goto unlock_out; > + } > + > + ret = __iommu_take_dma_ownership(group, owner); > +unlock_out: > + mutex_unlock(&group->mutex); > + iommu_group_put(group); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(iommu_device_claim_dma_owner); > + > +static void __iommu_release_dma_ownership(struct iommu_group *group) > +{ > + int ret; > + > if (WARN_ON(!group->owner_cnt || !group->owner || > !xa_empty(&group->pasid_array))) > - goto unlock_out; > + return; > > group->owner_cnt = 0; > group->owner = NULL; > ret = __iommu_group_set_domain(group, group->default_domain); > WARN(ret, "iommu driver failed to attach the default domain"); > +} > > -unlock_out: > +/** > + * 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); > + __iommu_release_dma_ownership(group); > mutex_unlock(&group->mutex); > } > EXPORT_SYMBOL_GPL(iommu_group_release_dma_owner); > > +/** > + * iommu_device_release_dma_owner() - Release DMA ownership of a device > + * @group: The device. @dev: the device > + * > + * Release the DMA ownership claimed by iommu_device_claim_dma_owner(). > + */ > +void iommu_device_release_dma_owner(struct device *dev) > +{ > + struct iommu_group *group = iommu_group_get(dev); > + > + mutex_lock(&group->mutex); > + if (group->owner_cnt > 1) { > + group->owner_cnt--; > + goto unlock_out; > + } > + __iommu_release_dma_ownership(group); > +unlock_out: > + mutex_unlock(&group->mutex); if (group->owner_cnt > 1) group->owner_cnt--; else __iommu_release_dma_ownership(group); mutex_unlock(&group->mutex); iommu_group_put(group); > + iommu_group_put(group); > +} > +EXPORT_SYMBOL_GPL(iommu_device_release_dma_owner); > + > /** > * iommu_group_dma_owner_claimed() - Query group dma ownership status > * @group: The group. > diff --git a/include/linux/iommu.h b/include/linux/iommu.h > index a09fd32d8cc273..1690c334e51631 100644 > --- a/include/linux/iommu.h > +++ b/include/linux/iommu.h > @@ -707,6 +707,9 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner); > void iommu_group_release_dma_owner(struct iommu_group *group); > bool iommu_group_dma_owner_claimed(struct iommu_group *group); > > +int iommu_device_claim_dma_owner(struct device *dev, void *owner); > +void iommu_device_release_dma_owner(struct device *dev); > + > struct iommu_domain *iommu_sva_domain_alloc(struct device *dev, > struct mm_struct *mm); > int iommu_attach_device_pasid(struct iommu_domain *domain, > @@ -1064,6 +1067,15 @@ static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group) > return false; > } > > +static inline void iommu_device_release_dma_owner(struct device *dev) > +{ > +} > + > +static inline int iommu_device_claim_dma_owner(struct device *dev, void *owner) > +{ > + return -ENODEV; > +} > + > static inline struct iommu_domain * > iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm) > { Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
On Fri, Nov 11, 2022 at 05:37:36AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Tuesday, November 8, 2022 8:49 AM > > +static int __iommu_take_dma_ownership(struct iommu_group *group, void > > *owner) > > +{ > > + int ret; > > + > > + if (WARN_ON(!owner)) > > + return -EINVAL; > > move to iommu_device_claim_dma_owner(). just like how it's checked > in the group version. Ok, like this: @@ -3112,9 +3112,6 @@ static int __iommu_take_dma_ownership(struct iommu_group *group, void *owner) { int ret; - if (WARN_ON(!owner)) - return -EINVAL; - if ((group->domain && group->domain != group->default_domain) || !xa_empty(&group->pasid_array)) return -EBUSY; @@ -3177,6 +3174,8 @@ int iommu_device_claim_dma_owner(struct device *dev, void *owner) if (!group) return -ENODEV; + if (WARN_ON(!owner)) + return -EINVAL; mutex_lock(&group->mutex); if (group->owner_cnt) { > > + if ((group->domain && group->domain != group->default_domain) || > > + !xa_empty(&group->pasid_array)) > > + return -EBUSY; > > the check of pasid_array is a new addition in this version. it's probably > worthy a comment here. It is just the merge resolution with the SVA series, the entire if is being copied from someplace else Thanks, Jason
On Mon, Nov 14, 2022 at 02:33:41PM +0100, Eric Auger wrote: > > +/** > > + * iommu_device_release_dma_owner() - Release DMA ownership of a device > > + * @group: The device. > @dev: the device > > + * > > + * Release the DMA ownership claimed by iommu_device_claim_dma_owner(). > > + */ > > +void iommu_device_release_dma_owner(struct device *dev) > > +{ > > + struct iommu_group *group = iommu_group_get(dev); > > + > > + mutex_lock(&group->mutex); > > + if (group->owner_cnt > 1) { > > + group->owner_cnt--; > > + goto unlock_out; > > + } > > + __iommu_release_dma_ownership(group); > > +unlock_out: > > + mutex_unlock(&group->mutex); > > if (group->owner_cnt > 1) > > group->owner_cnt--; > else > __iommu_release_dma_ownership(group); > > mutex_unlock(&group->mutex); > > iommu_group_put(group); Sure, thanks Jason
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index 6ca377f4fbf9e9..4cb14e44e40f83 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -3108,41 +3108,52 @@ static int __iommu_group_alloc_blocking_domain(struct iommu_group *group) return 0; } +static int __iommu_take_dma_ownership(struct iommu_group *group, void *owner) +{ + int ret; + + if (WARN_ON(!owner)) + return -EINVAL; + + if ((group->domain && group->domain != group->default_domain) || + !xa_empty(&group->pasid_array)) + return -EBUSY; + + ret = __iommu_group_alloc_blocking_domain(group); + if (ret) + return ret; + ret = __iommu_group_set_domain(group, group->blocking_domain); + if (ret) + return ret; + + group->owner = owner; + group->owner_cnt++; + return 0; +} + /** * 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. + * 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. Only a single owner may exist for a group. */ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner) { int ret = 0; + if (WARN_ON(!owner)) + return -EINVAL; + mutex_lock(&group->mutex); if (group->owner_cnt) { ret = -EPERM; goto unlock_out; - } else { - if ((group->domain && group->domain != group->default_domain) || - !xa_empty(&group->pasid_array)) { - ret = -EBUSY; - goto unlock_out; - } - - ret = __iommu_group_alloc_blocking_domain(group); - if (ret) - goto unlock_out; - - ret = __iommu_group_set_domain(group, group->blocking_domain); - if (ret) - goto unlock_out; - group->owner = owner; } - group->owner_cnt++; + ret = __iommu_take_dma_ownership(group, owner); unlock_out: mutex_unlock(&group->mutex); @@ -3151,30 +3162,91 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner) EXPORT_SYMBOL_GPL(iommu_group_claim_dma_owner); /** - * iommu_group_release_dma_owner() - Release DMA ownership of a group - * @group: The group. + * iommu_device_claim_dma_owner() - Set DMA ownership of a device + * @dev: The device. + * @owner: Caller specified pointer. Used for exclusive ownership. * - * Release the DMA ownership claimed by iommu_group_claim_dma_owner(). + * Claim the DMA ownership of a device. Multiple devices in the same group may + * concurrently claim ownership if they present the same owner value. Returns 0 + * on success and error code on failure */ -void iommu_group_release_dma_owner(struct iommu_group *group) +int iommu_device_claim_dma_owner(struct device *dev, void *owner) { - int ret; + struct iommu_group *group = iommu_group_get(dev); + int ret = 0; + + if (!group) + return -ENODEV; mutex_lock(&group->mutex); + if (group->owner_cnt) { + if (group->owner != owner) { + ret = -EPERM; + goto unlock_out; + } + group->owner_cnt++; + goto unlock_out; + } + + ret = __iommu_take_dma_ownership(group, owner); +unlock_out: + mutex_unlock(&group->mutex); + iommu_group_put(group); + + return ret; +} +EXPORT_SYMBOL_GPL(iommu_device_claim_dma_owner); + +static void __iommu_release_dma_ownership(struct iommu_group *group) +{ + int ret; + if (WARN_ON(!group->owner_cnt || !group->owner || !xa_empty(&group->pasid_array))) - goto unlock_out; + return; group->owner_cnt = 0; group->owner = NULL; ret = __iommu_group_set_domain(group, group->default_domain); WARN(ret, "iommu driver failed to attach the default domain"); +} -unlock_out: +/** + * 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); + __iommu_release_dma_ownership(group); mutex_unlock(&group->mutex); } EXPORT_SYMBOL_GPL(iommu_group_release_dma_owner); +/** + * iommu_device_release_dma_owner() - Release DMA ownership of a device + * @group: The device. + * + * Release the DMA ownership claimed by iommu_device_claim_dma_owner(). + */ +void iommu_device_release_dma_owner(struct device *dev) +{ + struct iommu_group *group = iommu_group_get(dev); + + mutex_lock(&group->mutex); + if (group->owner_cnt > 1) { + group->owner_cnt--; + goto unlock_out; + } + __iommu_release_dma_ownership(group); +unlock_out: + mutex_unlock(&group->mutex); + iommu_group_put(group); +} +EXPORT_SYMBOL_GPL(iommu_device_release_dma_owner); + /** * iommu_group_dma_owner_claimed() - Query group dma ownership status * @group: The group. diff --git a/include/linux/iommu.h b/include/linux/iommu.h index a09fd32d8cc273..1690c334e51631 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -707,6 +707,9 @@ int iommu_group_claim_dma_owner(struct iommu_group *group, void *owner); void iommu_group_release_dma_owner(struct iommu_group *group); bool iommu_group_dma_owner_claimed(struct iommu_group *group); +int iommu_device_claim_dma_owner(struct device *dev, void *owner); +void iommu_device_release_dma_owner(struct device *dev); + struct iommu_domain *iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm); int iommu_attach_device_pasid(struct iommu_domain *domain, @@ -1064,6 +1067,15 @@ static inline bool iommu_group_dma_owner_claimed(struct iommu_group *group) return false; } +static inline void iommu_device_release_dma_owner(struct device *dev) +{ +} + +static inline int iommu_device_claim_dma_owner(struct device *dev, void *owner) +{ + return -ENODEV; +} + static inline struct iommu_domain * iommu_sva_domain_alloc(struct device *dev, struct mm_struct *mm) {