| Message ID | 20230513132136.15021-9-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enhance vfio PCI hot reset for vfio cdev device | expand |
On Sat, 13 May 2023 06:21:34 -0700 Yi Liu <yi.l.liu@intel.com> 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@intel.com> > --- > 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,
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Thursday, May 18, 2023 3:40 AM > > On Sat, 13 May 2023 06:21:34 -0700 > Yi Liu <yi.l.liu@intel.com> 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@intel.com> > > --- > > 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,
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 + * @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. + */ +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,
to check if any device within the given iommu_group has been bound with the iommufd_ctx. This helpful for the checking on device ownership for the devices which have been bound but cannot be bound to any other iommufd as the iommu_group has been bound. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/iommu/iommufd/device.c | 29 +++++++++++++++++++++++++++++ include/linux/iommufd.h | 8 ++++++++ 2 files changed, 37 insertions(+)