| Message ID | 20210824144649.1488190-12-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/14] vfio: Move vfio_iommu_group_get() to vfio_register_group_dev() | expand |
On Tue, Aug 24, 2021 at 04:46:46PM +0200, Christoph Hellwig wrote: > Pass the group flags to ->attach_group and remove the messy check for > the bus type. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > drivers/vfio/vfio.c | 11 +++++------ > drivers/vfio/vfio.h | 7 ++++++- > drivers/vfio/vfio_iommu_spapr_tce.c | 2 +- > drivers/vfio/vfio_iommu_type1.c | 19 ++----------------- > 4 files changed, 14 insertions(+), 25 deletions(-) Every caller is doing group->iommu_group, maybe change the signature to (*attach_group)(struct vfio_iommu *iommu, struct vfio_iommu_group *group) ? Then the flags don't need to be another arg, just group->flags Happy to see the symbol_get removed! Jason
On Tue, Aug 24, 2021 at 09:28:50PM -0300, Jason Gunthorpe wrote: > On Tue, Aug 24, 2021 at 04:46:46PM +0200, Christoph Hellwig wrote: > > Pass the group flags to ->attach_group and remove the messy check for > > the bus type. > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > --- > > drivers/vfio/vfio.c | 11 +++++------ > > drivers/vfio/vfio.h | 7 ++++++- > > drivers/vfio/vfio_iommu_spapr_tce.c | 2 +- > > drivers/vfio/vfio_iommu_type1.c | 19 ++----------------- > > 4 files changed, 14 insertions(+), 25 deletions(-) > > Every caller is doing group->iommu_group, maybe change the signature > to > > (*attach_group)(struct vfio_iommu *iommu, struct vfio_iommu_group *group) > > ? s/vfio_iommu/vfio_container/, but yes. I actually have a series that does this (also for a few other methods), but this requires exposing vfio_container and vfio_iommu_group outside of vfio.c. Given that this series is big enough I decided to skip it for now, but that plus a few other changes will eventually simplify the group lookups in vfio_iommu_type1.c.
On Wed, Aug 25, 2021 at 07:34:27AM +0200, Christoph Hellwig wrote: > On Tue, Aug 24, 2021 at 09:28:50PM -0300, Jason Gunthorpe wrote: > > On Tue, Aug 24, 2021 at 04:46:46PM +0200, Christoph Hellwig wrote: > > > Pass the group flags to ->attach_group and remove the messy check for > > > the bus type. > > > > > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > > drivers/vfio/vfio.c | 11 +++++------ > > > drivers/vfio/vfio.h | 7 ++++++- > > > drivers/vfio/vfio_iommu_spapr_tce.c | 2 +- > > > drivers/vfio/vfio_iommu_type1.c | 19 ++----------------- > > > 4 files changed, 14 insertions(+), 25 deletions(-) > > > > Every caller is doing group->iommu_group, maybe change the signature > > to > > > > (*attach_group)(struct vfio_iommu *iommu, struct vfio_iommu_group *group) > > > > ? > > s/vfio_iommu/vfio_container/, but yes. I actually have a series that > does this (also for a few other methods), but this requires exposing > vfio_container and vfio_iommu_group outside of vfio.c. Given that this > series is big enough I decided to skip it for now, but that plus a few > other changes will eventually simplify the group lookups in > vfio_iommu_type1.c. Fair enough Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 26663dfa19e70d..a140ce3b15196d 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -68,9 +68,6 @@ struct vfio_unbound_dev { struct list_head unbound_next; }; -#define VFIO_MEDIATED (1 << 0) -#define VFIO_NOIOMMU (1 << 1) - struct vfio_group { struct kref kref; int minor; @@ -198,7 +195,7 @@ static long vfio_noiommu_ioctl(void *iommu_data, } static int vfio_noiommu_attach_group(void *iommu_data, - struct iommu_group *iommu_group) + struct iommu_group *iommu_group, unsigned int flags) { return 0; } @@ -1100,7 +1097,8 @@ static int __vfio_container_attach_groups(struct vfio_container *container, int ret = -ENODEV; list_for_each_entry(group, &container->group_list, container_next) { - ret = driver->ops->attach_group(data, group->iommu_group); + ret = driver->ops->attach_group(data, group->iommu_group, + group->flags); if (ret) goto unwind; } @@ -1358,7 +1356,8 @@ static int vfio_group_set_container(struct vfio_group *group, int container_fd) driver = container->iommu_driver; if (driver) { ret = driver->ops->attach_group(container->iommu_data, - group->iommu_group); + group->iommu_group, + group->flags); if (ret) goto unlock_out; } diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h index a78de649eb2f16..0c1a88fa991545 100644 --- a/drivers/vfio/vfio.h +++ b/drivers/vfio/vfio.h @@ -9,6 +9,10 @@ enum vfio_iommu_notify_type { VFIO_IOMMU_CONTAINER_CLOSE = 0, }; +/* flags for group->flags and ->attach_group */ +#define VFIO_MEDIATED (1 << 0) +#define VFIO_NOIOMMU (1 << 1) + /** * struct vfio_iommu_driver_ops - VFIO IOMMU driver callbacks */ @@ -20,7 +24,8 @@ struct vfio_iommu_driver_ops { long (*ioctl)(void *iommu_data, unsigned int cmd, unsigned long arg); int (*attach_group)(void *iommu_data, - struct iommu_group *group); + struct iommu_group *group, + unsigned int flags); void (*detach_group)(void *iommu_data, struct iommu_group *group); int (*pin_pages)(void *iommu_data, diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c index 3efd09faeca4a8..7567328d347d25 100644 --- a/drivers/vfio/vfio_iommu_spapr_tce.c +++ b/drivers/vfio/vfio_iommu_spapr_tce.c @@ -1239,7 +1239,7 @@ static long tce_iommu_take_ownership_ddw(struct tce_container *container, } static int tce_iommu_attach_group(void *iommu_data, - struct iommu_group *iommu_group) + struct iommu_group *iommu_group, unsigned int flags) { int ret = 0; struct tce_container *container = iommu_data; diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index 39e2706d0b3f34..44a3abdca580a0 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -36,7 +36,6 @@ #include <linux/uaccess.h> #include <linux/vfio.h> #include <linux/workqueue.h> -#include <linux/mdev.h> #include <linux/notifier.h> #include <linux/dma-iommu.h> #include <linux/irqdomain.h> @@ -1934,20 +1933,6 @@ static bool vfio_iommu_has_sw_msi(struct list_head *group_resv_regions, return ret; } -static bool vfio_bus_is_mdev(struct bus_type *bus) -{ - struct bus_type *mdev_bus; - bool ret = false; - - mdev_bus = symbol_get(mdev_bus_type); - if (mdev_bus) { - ret = (bus == mdev_bus); - symbol_put(mdev_bus_type); - } - - return ret; -} - /* * This is a helper function to insert an address range to iova list. * The list is initially created with a single entry corresponding to @@ -2172,7 +2157,7 @@ static void vfio_iommu_iova_insert_copy(struct vfio_iommu *iommu, } static int vfio_iommu_type1_attach_group(void *iommu_data, - struct iommu_group *iommu_group) + struct iommu_group *iommu_group, unsigned int flags) { struct vfio_iommu *iommu = iommu_data; struct vfio_iommu_group *group; @@ -2207,7 +2192,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data, if (ret) goto out_free; - if (vfio_bus_is_mdev(bus)) { + if (flags & VFIO_MEDIATED) { if (!iommu->external_domain) { INIT_LIST_HEAD(&domain->group_list); iommu->external_domain = domain;
Pass the group flags to ->attach_group and remove the messy check for the bus type. Signed-off-by: Christoph Hellwig <hch@lst.de> --- drivers/vfio/vfio.c | 11 +++++------ drivers/vfio/vfio.h | 7 ++++++- drivers/vfio/vfio_iommu_spapr_tce.c | 2 +- drivers/vfio/vfio_iommu_type1.c | 19 ++----------------- 4 files changed, 14 insertions(+), 25 deletions(-)