> From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Sent: Thursday, May 18, 2023 3:40 AM
>
> 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.
Sure. I'll go through the commit messages.
>
> > 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"?
Yes, yes. a poor copy from a prior version..
>
> > + * @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,
Yes, this is the meaning of it. will fix it.
Regards,
Yi Liu
>
> > + */
> > +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,
