On Sat, 13 May 2023 06:21:34 -0700
Yi Liu <yi.l.liu@xxxxxxxxx> wrote:
> to check if any device within the given iommu_group has been bound with
Nit, I find these commit logs where the subject line is intended to
flow into the commit log to form a complete sentence difficult to read.
I expect complete thoughts within the commit log itself and the subject
should be a separate summary of the log. Repeating the subject within
the commit log is ok.
> the iommufd_ctx. This helpful for the checking on device ownership for
s/This/This is/
> the devices which have been bound but cannot be bound to any other iommufd
s/have been/have not been/?
> as the iommu_group has been bound.
>
> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> ---
> drivers/iommu/iommufd/device.c | 29 +++++++++++++++++++++++++++++
> include/linux/iommufd.h | 8 ++++++++
> 2 files changed, 37 insertions(+)
>
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index 81466b97023f..5e5f7912807b 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -98,6 +98,35 @@ struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> }
> EXPORT_SYMBOL_NS_GPL(iommufd_device_bind, IOMMUFD);
>
> +/**
> + * iommufd_ctx_has_group - True if the struct device is bound to this ictx
What struct device? Isn't this "True if any device within the group is
bound to the ictx"?
> + * @ictx: iommufd file descriptor
> + * @group: Pointer to a physical iommu_group struct
> + *
> + * True if a iommufd_device_bind() is present for any device within the
> + * group.
How can a function be present for a device? Maybe "True if any device
within the group has been bound to this ictx, ex. via
iommufd_device_bind(), therefore implying ictx ownership of the group." Thanks,
Alex
> + */
> +bool iommufd_ctx_has_group(struct iommufd_ctx *ictx, struct iommu_group *group)
> +{
> + struct iommufd_object *obj;
> + unsigned long index;
> +
> + if (!ictx || !group)
> + return false;
> +
> + xa_lock(&ictx->objects);
> + xa_for_each(&ictx->objects, index, obj) {
> + if (obj->type == IOMMUFD_OBJ_DEVICE &&
> + container_of(obj, struct iommufd_device, obj)->group == group) {
> + xa_unlock(&ictx->objects);
> + return true;
> + }
> + }
> + xa_unlock(&ictx->objects);
> + return false;
> +}
> +EXPORT_SYMBOL_NS_GPL(iommufd_ctx_has_group, IOMMUFD);
> +
> /**
> * iommufd_device_unbind - Undo iommufd_device_bind()
> * @idev: Device returned by iommufd_device_bind()
> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index 68cd65274e28..e49c16cd6831 100644
> --- a/include/linux/iommufd.h
> +++ b/include/linux/iommufd.h
> @@ -16,6 +16,7 @@ struct page;
> struct iommufd_ctx;
> struct iommufd_access;
> struct file;
> +struct iommu_group;
>
> struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx,
> struct device *dev, u32 *id);
> @@ -56,6 +57,7 @@ void iommufd_ctx_get(struct iommufd_ctx *ictx);
> #if IS_ENABLED(CONFIG_IOMMUFD)
> struct iommufd_ctx *iommufd_ctx_from_file(struct file *file);
> void iommufd_ctx_put(struct iommufd_ctx *ictx);
> +bool iommufd_ctx_has_group(struct iommufd_ctx *ictx, struct iommu_group *group);
>
> int iommufd_access_pin_pages(struct iommufd_access *access, unsigned long iova,
> unsigned long length, struct page **out_pages,
> @@ -77,6 +79,12 @@ static inline void iommufd_ctx_put(struct iommufd_ctx *ictx)
> {
> }
>
> +static inline bool iommufd_ctx_has_group(struct iommufd_ctx *ictx,
> + struct iommu_group *group)
> +{
> + return false;
> +}
> +
> static inline int iommufd_access_pin_pages(struct iommufd_access *access,
> unsigned long iova,
> unsigned long length,
