| Message ID | 09e84c7c9099aba07b24b113c70d57d4574aee08.1712978212.git.nicolinc@nvidia.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add Tegra241 (Grace) CMDQV Support (part 2/2) | expand |
On Fri, Apr 12, 2024 at 08:46:58PM -0700, Nicolin Chen wrote: > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h > index ffc3a949f837..a0cb08a4b653 100644 > --- a/include/linux/iommufd.h > +++ b/include/linux/iommufd.h > @@ -9,6 +9,7 @@ > #include <linux/types.h> > #include <linux/errno.h> > #include <linux/err.h> > +#include <linux/refcount.h> > > struct device; > struct iommufd_device; > @@ -18,6 +19,28 @@ struct iommufd_access; > struct file; > struct iommu_group; > > +enum iommufd_object_type { > + IOMMUFD_OBJ_NONE, > + IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE, > + IOMMUFD_OBJ_DEVICE, > + IOMMUFD_OBJ_HWPT_PAGING, > + IOMMUFD_OBJ_HWPT_NESTED, > + IOMMUFD_OBJ_IOAS, > + IOMMUFD_OBJ_ACCESS, > +#ifdef CONFIG_IOMMUFD_TEST > + IOMMUFD_OBJ_SELFTEST, > +#endif > + IOMMUFD_OBJ_MAX, > +}; Can we just forward declare the enum? It would be nice to keep it in the private header Otherwise makes sense Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
On Sun, May 12, 2024 at 10:21:49AM -0300, Jason Gunthorpe wrote: > On Fri, Apr 12, 2024 at 08:46:58PM -0700, Nicolin Chen wrote: > > diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h > > +enum iommufd_object_type { > > + IOMMUFD_OBJ_NONE, > > + IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE, > > + IOMMUFD_OBJ_DEVICE, > > + IOMMUFD_OBJ_HWPT_PAGING, > > + IOMMUFD_OBJ_HWPT_NESTED, > > + IOMMUFD_OBJ_IOAS, > > + IOMMUFD_OBJ_ACCESS, > > +#ifdef CONFIG_IOMMUFD_TEST > > + IOMMUFD_OBJ_SELFTEST, > > +#endif > > + IOMMUFD_OBJ_MAX, > > +}; > > Can we just forward declare the enum? It would be nice to keep it in > the private header It doesn't seem to support that: ./include/linux/iommufd.h:31:34: error: field ‘type’ has incomplete type 31 | enum iommufd_object_type type; By testing the following change on top of the series: =================================== -enum iommufd_object_type { - IOMMUFD_OBJ_NONE, - IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE, - IOMMUFD_OBJ_DEVICE, - IOMMUFD_OBJ_HWPT_PAGING, - IOMMUFD_OBJ_HWPT_NESTED, - IOMMUFD_OBJ_IOAS, - IOMMUFD_OBJ_ACCESS, - IOMMUFD_OBJ_VIOMMU, - IOMMUFD_OBJ_VQUEUE, -#ifdef CONFIG_IOMMUFD_TEST - IOMMUFD_OBJ_SELFTEST, -#endif - IOMMUFD_OBJ_MAX, -}; +enum iommufd_object_type; /* Base struct for all objects with a userspace ID handle. */ struct iommufd_object { refcount_t shortterm_users; refcount_t users; enum iommufd_object_type type; unsigned int id; }; =================================== We could change the "enum iommufd_object_type type" in struct iommufd_object to "unsigned int type" though... Thanks Nicolin
On Sun, May 12, 2024 at 03:40:44PM -0700, Nicolin Chen wrote: > We could change the "enum iommufd_object_type type" in struct > iommufd_object to "unsigned int type" though... That might be the best compromise Jason
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 991f864d1f9b..3ea0a093ee50 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -6,9 +6,9 @@ #include <linux/rwsem.h> #include <linux/xarray.h> -#include <linux/refcount.h> #include <linux/uaccess.h> #include <linux/iommu.h> +#include <linux/iommufd.h> #include <linux/iova_bitmap.h> #include <uapi/linux/iommufd.h> @@ -120,28 +120,6 @@ static inline int iommufd_ucmd_respond(struct iommufd_ucmd *ucmd, return 0; } -enum iommufd_object_type { - IOMMUFD_OBJ_NONE, - IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE, - IOMMUFD_OBJ_DEVICE, - IOMMUFD_OBJ_HWPT_PAGING, - IOMMUFD_OBJ_HWPT_NESTED, - IOMMUFD_OBJ_IOAS, - IOMMUFD_OBJ_ACCESS, -#ifdef CONFIG_IOMMUFD_TEST - IOMMUFD_OBJ_SELFTEST, -#endif - IOMMUFD_OBJ_MAX, -}; - -/* Base struct for all objects with a userspace ID handle. */ -struct iommufd_object { - refcount_t shortterm_users; - refcount_t users; - enum iommufd_object_type type; - unsigned int id; -}; - static inline bool iommufd_lock_obj(struct iommufd_object *obj) { if (!refcount_inc_not_zero(&obj->users)) diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index ffc3a949f837..a0cb08a4b653 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -9,6 +9,7 @@ #include <linux/types.h> #include <linux/errno.h> #include <linux/err.h> +#include <linux/refcount.h> struct device; struct iommufd_device; @@ -18,6 +19,28 @@ struct iommufd_access; struct file; struct iommu_group; +enum iommufd_object_type { + IOMMUFD_OBJ_NONE, + IOMMUFD_OBJ_ANY = IOMMUFD_OBJ_NONE, + IOMMUFD_OBJ_DEVICE, + IOMMUFD_OBJ_HWPT_PAGING, + IOMMUFD_OBJ_HWPT_NESTED, + IOMMUFD_OBJ_IOAS, + IOMMUFD_OBJ_ACCESS, +#ifdef CONFIG_IOMMUFD_TEST + IOMMUFD_OBJ_SELFTEST, +#endif + IOMMUFD_OBJ_MAX, +}; + +/* Base struct for all objects with a userspace ID handle. */ +struct iommufd_object { + refcount_t shortterm_users; + refcount_t users; + enum iommufd_object_type type; + unsigned int id; +}; + struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, struct device *dev, u32 *id); void iommufd_device_unbind(struct iommufd_device *idev);
To prepare for a new public iommufd_viommu structure that will be an embedded object in a driver's viommu structure: // include/linux/iommufd.h struct iommufd_viommu { struct iommufd_object obj; .... }; // Some IOMMU driver struct iommu_driver_viommu { struct iommufd_viommu core; .... }; Move iommufd_object and iommufd_object_type to the public header. Signed-off-by: Nicolin Chen <nicolinc@nvidia.com> --- drivers/iommu/iommufd/iommufd_private.h | 24 +----------------------- include/linux/iommufd.h | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 23 deletions(-)