| Message ID | 20230616093946.68711-22-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add vfio_device cdev for iommufd support | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Friday, June 16, 2023 5:40 PM > > vfio_group is not needed for vfio device cdev, so with vfio device cdev > introduced, the vfio_group infrastructures can be compiled out if only > cdev is needed. > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > Tested-by: Terrence Xu <terrence.xu@intel.com> > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > drivers/iommu/iommufd/Kconfig | 4 +- > drivers/vfio/Kconfig | 15 ++++++ > drivers/vfio/Makefile | 2 +- > drivers/vfio/vfio.h | 89 ++++++++++++++++++++++++++++++++--- > include/linux/vfio.h | 25 ++++++++-- > 5 files changed, 123 insertions(+), 12 deletions(-) > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > index ada693ea51a7..99d4b075df49 100644 > --- a/drivers/iommu/iommufd/Kconfig > +++ b/drivers/iommu/iommufd/Kconfig > @@ -14,8 +14,8 @@ config IOMMUFD > if IOMMUFD > config IOMMUFD_VFIO_CONTAINER > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > - depends on VFIO && !VFIO_CONTAINER > - default VFIO && !VFIO_CONTAINER > + depends on VFIO_GROUP && !VFIO_CONTAINER > + default VFIO_GROUP && !VFIO_CONTAINER Hi Alex, Jason, I found a minor nit on the kconfig. The below configuration is valid. But user cannot use vfio directly as there is no /dev/vfio/vfio. Although user can open /dev/iommu instead. This is not good. CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEv=n CONFIG_VFIO_GROUP=y CONFIG_VFIO_CONTAINER=n CONFIG_IOMMUFD_VFIO_CONTAINER=n So need to have the below change. I'll incorporate this change in this series after your ack. diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig index 99d4b075df49..d675c96c2bbb 100644 --- a/drivers/iommu/iommufd/Kconfig +++ b/drivers/iommu/iommufd/Kconfig @@ -14,8 +14,8 @@ config IOMMUFD if IOMMUFD config IOMMUFD_VFIO_CONTAINER bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" - depends on VFIO_GROUP && !VFIO_CONTAINER - default VFIO_GROUP && !VFIO_CONTAINER + depends on VFIO_GROUP + default n help IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on IOMMUFD providing compatibility emulation to give the same ioctls. diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 6bda6dbb4878..ee3bbad6beb8 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -6,7 +6,7 @@ menuconfig VFIO select INTERVAL_TREE select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n select VFIO_DEVICE_CDEV if !VFIO_GROUP - select VFIO_CONTAINER if IOMMUFD=n + select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n help VFIO provides a framework for secure userspace device drivers. See Documentation/driver-api/vfio.rst for more details. > help > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > IOMMUFD providing compatibility emulation to give the same ioctls. > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 1cab8e4729de..35ab8ab87688 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -4,6 +4,8 @@ menuconfig VFIO > select IOMMU_API > depends on IOMMUFD || !IOMMUFD > select INTERVAL_TREE > + select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > + select VFIO_DEVICE_CDEV if !VFIO_GROUP > select VFIO_CONTAINER if IOMMUFD=n This should be " select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n" Regards, Yi Liu > help > VFIO provides a framework for secure userspace device drivers. > @@ -15,6 +17,7 @@ if VFIO > config VFIO_DEVICE_CDEV > bool "Support for the VFIO cdev /dev/vfio/devices/vfioX" > depends on IOMMUFD > + default !VFIO_GROUP > help > The VFIO device cdev is another way for userspace to get device > access. Userspace gets device fd by opening device cdev under > @@ -24,9 +27,20 @@ config VFIO_DEVICE_CDEV > > If you don't know what to do here, say N. > > +config VFIO_GROUP > + bool "Support for the VFIO group /dev/vfio/$group_id" > + default y > + help > + VFIO group support provides the traditional model for accessing > + devices through VFIO and is used by the majority of userspace > + applications and drivers making use of VFIO. > + > + If you don't know what to do here, say Y. > + > config VFIO_CONTAINER > bool "Support for the VFIO container /dev/vfio/vfio" > select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64) > + depends on VFIO_GROUP > default y > help > The VFIO container is the classic interface to VFIO for establishing > @@ -48,6 +62,7 @@ endif > > config VFIO_NOIOMMU > bool "VFIO No-IOMMU support" > + depends on VFIO_GROUP > help > VFIO is built on the ability to isolate devices using the IOMMU. > Only with an IOMMU can userspace access to DMA capable devices be > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > index 245394aeb94b..57c3515af606 100644 > --- a/drivers/vfio/Makefile > +++ b/drivers/vfio/Makefile > @@ -2,9 +2,9 @@ > obj-$(CONFIG_VFIO) += vfio.o > > vfio-y += vfio_main.o \ > - group.o \ > iova_bitmap.o > vfio-$(CONFIG_VFIO_DEVICE_CDEV) += device_cdev.o > +vfio-$(CONFIG_VFIO_GROUP) += group.o > vfio-$(CONFIG_IOMMUFD) += iommufd.o > vfio-$(CONFIG_VFIO_CONTAINER) += container.o > vfio-$(CONFIG_VFIO_VIRQFD) += virqfd.o > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > index e7a3fe093362..b27a3915e6c9 100644 > --- a/drivers/vfio/vfio.h > +++ b/drivers/vfio/vfio.h > @@ -36,6 +36,12 @@ vfio_allocate_device_file(struct vfio_device *device); > > extern const struct file_operations vfio_device_fops; > > +#ifdef CONFIG_VFIO_NOIOMMU > +extern bool vfio_noiommu __read_mostly; > +#else > +enum { vfio_noiommu = false }; > +#endif > + > enum vfio_group_type { > /* > * Physical device with IOMMU backing. > @@ -60,6 +66,7 @@ enum vfio_group_type { > VFIO_NO_IOMMU, > }; > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > struct vfio_group { > struct device dev; > struct cdev cdev; > @@ -111,6 +118,82 @@ static inline bool vfio_device_is_noiommu(struct vfio_device > *vdev) > return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && > vdev->group->type == VFIO_NO_IOMMU; > } > +#else > +struct vfio_group; > + > +static inline int vfio_device_block_group(struct vfio_device *device) > +{ > + return 0; > +} > + > +static inline void vfio_device_unblock_group(struct vfio_device *device) > +{ > +} > + > +static inline int vfio_device_set_group(struct vfio_device *device, > + enum vfio_group_type type) > +{ > + return 0; > +} > + > +static inline void vfio_device_remove_group(struct vfio_device *device) > +{ > +} > + > +static inline void vfio_device_group_register(struct vfio_device *device) > +{ > +} > + > +static inline void vfio_device_group_unregister(struct vfio_device *device) > +{ > +} > + > +static inline int vfio_device_group_use_iommu(struct vfio_device *device) > +{ > + return -EOPNOTSUPP; > +} > + > +static inline void vfio_device_group_unuse_iommu(struct vfio_device *device) > +{ > +} > + > +static inline void vfio_df_group_close(struct vfio_device_file *df) > +{ > +} > + > +static inline struct vfio_group *vfio_group_from_file(struct file *file) > +{ > + return NULL; > +} > + > +static inline bool vfio_group_enforced_coherent(struct vfio_group *group) > +{ > + return true; > +} > + > +static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > +{ > +} > + > +static inline bool vfio_device_has_container(struct vfio_device *device) > +{ > + return false; > +} > + > +static inline int __init vfio_group_init(void) > +{ > + return 0; > +} > + > +static inline void vfio_group_cleanup(void) > +{ > +} > + > +static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) > +{ > + return false; > +} > +#endif /* CONFIG_VFIO_GROUP */ > > #if IS_ENABLED(CONFIG_VFIO_CONTAINER) > /** > @@ -362,12 +445,6 @@ static inline void vfio_virqfd_exit(void) > } > #endif > > -#ifdef CONFIG_VFIO_NOIOMMU > -extern bool vfio_noiommu __read_mostly; > -#else > -enum { vfio_noiommu = false }; > -#endif > - > #ifdef CONFIG_HAVE_KVM > void _vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm); > void vfio_device_put_kvm(struct vfio_device *device); > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index d6228c839c44..5a1dee983f17 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -43,7 +43,11 @@ struct vfio_device { > */ > const struct vfio_migration_ops *mig_ops; > const struct vfio_log_ops *log_ops; > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > struct vfio_group *group; > + struct list_head group_next; > + struct list_head iommu_entry; > +#endif > struct vfio_device_set *dev_set; > struct list_head dev_set_list; > unsigned int migration_flags; > @@ -58,8 +62,6 @@ struct vfio_device { > refcount_t refcount; /* user count on registered device*/ > unsigned int open_count; > struct completion comp; > - struct list_head group_next; > - struct list_head iommu_entry; > struct iommufd_access *iommufd_access; > void (*put_kvm)(struct kvm *kvm); > #if IS_ENABLED(CONFIG_IOMMUFD) > @@ -284,12 +286,29 @@ int vfio_mig_get_next_state(struct vfio_device *device, > /* > * External user API > */ > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > struct iommu_group *vfio_file_iommu_group(struct file *file); > bool vfio_file_is_group(struct file *file); > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > +#else > +static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > +{ > + return NULL; > +} > + > +static inline bool vfio_file_is_group(struct file *file) > +{ > + return false; > +} > + > +static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > +{ > + return false; > +} > +#endif > bool vfio_file_is_valid(struct file *file); > bool vfio_file_enforced_coherent(struct file *file); > void vfio_file_set_kvm(struct file *file, struct kvm *kvm); > -bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long)) > > -- > 2.34.1
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Monday, July 17, 2023 2:36 PM > > > From: Liu, Yi L <yi.l.liu@intel.com> > > Sent: Friday, June 16, 2023 5:40 PM > > > > vfio_group is not needed for vfio device cdev, so with vfio device cdev > > introduced, the vfio_group infrastructures can be compiled out if only > > cdev is needed. > > > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > Tested-by: Terrence Xu <terrence.xu@intel.com> > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > --- > > drivers/iommu/iommufd/Kconfig | 4 +- > > drivers/vfio/Kconfig | 15 ++++++ > > drivers/vfio/Makefile | 2 +- > > drivers/vfio/vfio.h | 89 ++++++++++++++++++++++++++++++++--- > > include/linux/vfio.h | 25 ++++++++-- > > 5 files changed, 123 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > > index ada693ea51a7..99d4b075df49 100644 > > --- a/drivers/iommu/iommufd/Kconfig > > +++ b/drivers/iommu/iommufd/Kconfig > > @@ -14,8 +14,8 @@ config IOMMUFD > > if IOMMUFD > > config IOMMUFD_VFIO_CONTAINER > > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > > - depends on VFIO && !VFIO_CONTAINER > > - default VFIO && !VFIO_CONTAINER > > + depends on VFIO_GROUP && !VFIO_CONTAINER > > + default VFIO_GROUP && !VFIO_CONTAINER > > Hi Alex, Jason, > > I found a minor nit on the kconfig. The below configuration is valid. > But user cannot use vfio directly as there is no /dev/vfio/vfio. Although > user can open /dev/iommu instead. This is not good. > > CONFIG_IOMMUFD=y > CONFIG_VFIO_DEVICE_CDEv=n > CONFIG_VFIO_GROUP=y > CONFIG_VFIO_CONTAINER=n > CONFIG_IOMMUFD_VFIO_CONTAINER=n > > So need to have the below change. I'll incorporate this change in > this series after your ack. > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > index 99d4b075df49..d675c96c2bbb 100644 > --- a/drivers/iommu/iommufd/Kconfig > +++ b/drivers/iommu/iommufd/Kconfig > @@ -14,8 +14,8 @@ config IOMMUFD > if IOMMUFD > config IOMMUFD_VFIO_CONTAINER > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > - depends on VFIO_GROUP && !VFIO_CONTAINER > - default VFIO_GROUP && !VFIO_CONTAINER > + depends on VFIO_GROUP > + default n > help > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > IOMMUFD providing compatibility emulation to give the same ioctls. > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > index 6bda6dbb4878..ee3bbad6beb8 100644 > --- a/drivers/vfio/Kconfig > +++ b/drivers/vfio/Kconfig > @@ -6,7 +6,7 @@ menuconfig VFIO > select INTERVAL_TREE > select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > select VFIO_DEVICE_CDEV if !VFIO_GROUP > - select VFIO_CONTAINER if IOMMUFD=n > + select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n > help > VFIO provides a framework for secure userspace device drivers. > See Documentation/driver-api/vfio.rst for more details. > Just realized that it is possible to config both VFIO_CONTAINER and IOMMUFD_VFIO_CONTAINER to "y". Then there will be a conflict when registering /dev/vfio/vfio. Any suggestion? Regards, Yi Liu > > help > > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > > IOMMUFD providing compatibility emulation to give the same ioctls. > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > index 1cab8e4729de..35ab8ab87688 100644 > > --- a/drivers/vfio/Kconfig > > +++ b/drivers/vfio/Kconfig > > @@ -4,6 +4,8 @@ menuconfig VFIO > > select IOMMU_API > > depends on IOMMUFD || !IOMMUFD > > select INTERVAL_TREE > > + select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > > + select VFIO_DEVICE_CDEV if !VFIO_GROUP > > select VFIO_CONTAINER if IOMMUFD=n > > This should be " select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n" > > Regards, > Yi Liu > > > help > > VFIO provides a framework for secure userspace device drivers. > > @@ -15,6 +17,7 @@ if VFIO > > config VFIO_DEVICE_CDEV > > bool "Support for the VFIO cdev /dev/vfio/devices/vfioX" > > depends on IOMMUFD > > + default !VFIO_GROUP > > help > > The VFIO device cdev is another way for userspace to get device > > access. Userspace gets device fd by opening device cdev under > > @@ -24,9 +27,20 @@ config VFIO_DEVICE_CDEV > > > > If you don't know what to do here, say N. > > > > +config VFIO_GROUP > > + bool "Support for the VFIO group /dev/vfio/$group_id" > > + default y > > + help > > + VFIO group support provides the traditional model for accessing > > + devices through VFIO and is used by the majority of userspace > > + applications and drivers making use of VFIO. > > + > > + If you don't know what to do here, say Y. > > + > > config VFIO_CONTAINER > > bool "Support for the VFIO container /dev/vfio/vfio" > > select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64) > > + depends on VFIO_GROUP > > default y > > help > > The VFIO container is the classic interface to VFIO for establishing > > @@ -48,6 +62,7 @@ endif > > > > config VFIO_NOIOMMU > > bool "VFIO No-IOMMU support" > > + depends on VFIO_GROUP > > help > > VFIO is built on the ability to isolate devices using the IOMMU. > > Only with an IOMMU can userspace access to DMA capable devices be > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > index 245394aeb94b..57c3515af606 100644 > > --- a/drivers/vfio/Makefile > > +++ b/drivers/vfio/Makefile > > @@ -2,9 +2,9 @@ > > obj-$(CONFIG_VFIO) += vfio.o > > > > vfio-y += vfio_main.o \ > > - group.o \ > > iova_bitmap.o > > vfio-$(CONFIG_VFIO_DEVICE_CDEV) += device_cdev.o > > +vfio-$(CONFIG_VFIO_GROUP) += group.o > > vfio-$(CONFIG_IOMMUFD) += iommufd.o > > vfio-$(CONFIG_VFIO_CONTAINER) += container.o > > vfio-$(CONFIG_VFIO_VIRQFD) += virqfd.o > > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > > index e7a3fe093362..b27a3915e6c9 100644 > > --- a/drivers/vfio/vfio.h > > +++ b/drivers/vfio/vfio.h > > @@ -36,6 +36,12 @@ vfio_allocate_device_file(struct vfio_device *device); > > > > extern const struct file_operations vfio_device_fops; > > > > +#ifdef CONFIG_VFIO_NOIOMMU > > +extern bool vfio_noiommu __read_mostly; > > +#else > > +enum { vfio_noiommu = false }; > > +#endif > > + > > enum vfio_group_type { > > /* > > * Physical device with IOMMU backing. > > @@ -60,6 +66,7 @@ enum vfio_group_type { > > VFIO_NO_IOMMU, > > }; > > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > struct vfio_group { > > struct device dev; > > struct cdev cdev; > > @@ -111,6 +118,82 @@ static inline bool vfio_device_is_noiommu(struct vfio_device > > *vdev) > > return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && > > vdev->group->type == VFIO_NO_IOMMU; > > } > > +#else > > +struct vfio_group; > > + > > +static inline int vfio_device_block_group(struct vfio_device *device) > > +{ > > + return 0; > > +} > > + > > +static inline void vfio_device_unblock_group(struct vfio_device *device) > > +{ > > +} > > + > > +static inline int vfio_device_set_group(struct vfio_device *device, > > + enum vfio_group_type type) > > +{ > > + return 0; > > +} > > + > > +static inline void vfio_device_remove_group(struct vfio_device *device) > > +{ > > +} > > + > > +static inline void vfio_device_group_register(struct vfio_device *device) > > +{ > > +} > > + > > +static inline void vfio_device_group_unregister(struct vfio_device *device) > > +{ > > +} > > + > > +static inline int vfio_device_group_use_iommu(struct vfio_device *device) > > +{ > > + return -EOPNOTSUPP; > > +} > > + > > +static inline void vfio_device_group_unuse_iommu(struct vfio_device *device) > > +{ > > +} > > + > > +static inline void vfio_df_group_close(struct vfio_device_file *df) > > +{ > > +} > > + > > +static inline struct vfio_group *vfio_group_from_file(struct file *file) > > +{ > > + return NULL; > > +} > > + > > +static inline bool vfio_group_enforced_coherent(struct vfio_group *group) > > +{ > > + return true; > > +} > > + > > +static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > > +{ > > +} > > + > > +static inline bool vfio_device_has_container(struct vfio_device *device) > > +{ > > + return false; > > +} > > + > > +static inline int __init vfio_group_init(void) > > +{ > > + return 0; > > +} > > + > > +static inline void vfio_group_cleanup(void) > > +{ > > +} > > + > > +static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) > > +{ > > + return false; > > +} > > +#endif /* CONFIG_VFIO_GROUP */ > > > > #if IS_ENABLED(CONFIG_VFIO_CONTAINER) > > /** > > @@ -362,12 +445,6 @@ static inline void vfio_virqfd_exit(void) > > } > > #endif > > > > -#ifdef CONFIG_VFIO_NOIOMMU > > -extern bool vfio_noiommu __read_mostly; > > -#else > > -enum { vfio_noiommu = false }; > > -#endif > > - > > #ifdef CONFIG_HAVE_KVM > > void _vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm); > > void vfio_device_put_kvm(struct vfio_device *device); > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > index d6228c839c44..5a1dee983f17 100644 > > --- a/include/linux/vfio.h > > +++ b/include/linux/vfio.h > > @@ -43,7 +43,11 @@ struct vfio_device { > > */ > > const struct vfio_migration_ops *mig_ops; > > const struct vfio_log_ops *log_ops; > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > struct vfio_group *group; > > + struct list_head group_next; > > + struct list_head iommu_entry; > > +#endif > > struct vfio_device_set *dev_set; > > struct list_head dev_set_list; > > unsigned int migration_flags; > > @@ -58,8 +62,6 @@ struct vfio_device { > > refcount_t refcount; /* user count on registered device*/ > > unsigned int open_count; > > struct completion comp; > > - struct list_head group_next; > > - struct list_head iommu_entry; > > struct iommufd_access *iommufd_access; > > void (*put_kvm)(struct kvm *kvm); > > #if IS_ENABLED(CONFIG_IOMMUFD) > > @@ -284,12 +286,29 @@ int vfio_mig_get_next_state(struct vfio_device *device, > > /* > > * External user API > > */ > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > bool vfio_file_is_group(struct file *file); > > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > +#else > > +static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > > +{ > > + return NULL; > > +} > > + > > +static inline bool vfio_file_is_group(struct file *file) > > +{ > > + return false; > > +} > > + > > +static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > > +{ > > + return false; > > +} > > +#endif > > bool vfio_file_is_valid(struct file *file); > > bool vfio_file_enforced_coherent(struct file *file); > > void vfio_file_set_kvm(struct file *file, struct kvm *kvm); > > -bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > > #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long)) > > > > -- > > 2.34.1
On Mon, Jul 17, 2023 at 06:36:19AM +0000, Liu, Yi L wrote: > Hi Alex, Jason, > > I found a minor nit on the kconfig. The below configuration is valid. > But user cannot use vfio directly as there is no /dev/vfio/vfio. Although > user can open /dev/iommu instead. This is not good. > > CONFIG_IOMMUFD=y > CONFIG_VFIO_DEVICE_CDEv=n > CONFIG_VFIO_GROUP=y > CONFIG_VFIO_CONTAINER=n > CONFIG_IOMMUFD_VFIO_CONTAINER=n > > So need to have the below change. I'll incorporate this change in > this series after your ack. This is fine, we decided to allow this in the original vfio series. It is usable in that you have to use iommufd natively with the group interface. It won't be backwards compatible with current userspace. Jason
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Monday, July 17, 2023 8:34 PM > > On Mon, Jul 17, 2023 at 06:36:19AM +0000, Liu, Yi L wrote: > > Hi Alex, Jason, > > > > I found a minor nit on the kconfig. The below configuration is valid. > > But user cannot use vfio directly as there is no /dev/vfio/vfio. Although > > user can open /dev/iommu instead. This is not good. > > > > CONFIG_IOMMUFD=y > > CONFIG_VFIO_DEVICE_CDEv=n > > CONFIG_VFIO_GROUP=y > > CONFIG_VFIO_CONTAINER=n > > CONFIG_IOMMUFD_VFIO_CONTAINER=n > > > > So need to have the below change. I'll incorporate this change in > > this series after your ack. > > This is fine, we decided to allow this in the original vfio series. It > is usable in that you have to use iommufd natively with the group > interface. It won't be backwards compatible with current userspace. Sure. I'll keep the current code. Regards, Yi Liu
On Mon, 17 Jul 2023 08:08:59 +0000 "Liu, Yi L" <yi.l.liu@intel.com> wrote: > > From: Liu, Yi L <yi.l.liu@intel.com> > > Sent: Monday, July 17, 2023 2:36 PM > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > Sent: Friday, June 16, 2023 5:40 PM > > > > > > vfio_group is not needed for vfio device cdev, so with vfio device cdev > > > introduced, the vfio_group infrastructures can be compiled out if only > > > cdev is needed. > > > > > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > > > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > > > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > > > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > Tested-by: Terrence Xu <terrence.xu@intel.com> > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > > --- > > > drivers/iommu/iommufd/Kconfig | 4 +- > > > drivers/vfio/Kconfig | 15 ++++++ > > > drivers/vfio/Makefile | 2 +- > > > drivers/vfio/vfio.h | 89 ++++++++++++++++++++++++++++++++--- > > > include/linux/vfio.h | 25 ++++++++-- > > > 5 files changed, 123 insertions(+), 12 deletions(-) > > > > > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > > > index ada693ea51a7..99d4b075df49 100644 > > > --- a/drivers/iommu/iommufd/Kconfig > > > +++ b/drivers/iommu/iommufd/Kconfig > > > @@ -14,8 +14,8 @@ config IOMMUFD > > > if IOMMUFD > > > config IOMMUFD_VFIO_CONTAINER > > > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > > > - depends on VFIO && !VFIO_CONTAINER > > > - default VFIO && !VFIO_CONTAINER > > > + depends on VFIO_GROUP && !VFIO_CONTAINER > > > + default VFIO_GROUP && !VFIO_CONTAINER > > > > Hi Alex, Jason, > > > > I found a minor nit on the kconfig. The below configuration is valid. > > But user cannot use vfio directly as there is no /dev/vfio/vfio. Although > > user can open /dev/iommu instead. This is not good. > > > > CONFIG_IOMMUFD=y > > CONFIG_VFIO_DEVICE_CDEv=n > > CONFIG_VFIO_GROUP=y > > CONFIG_VFIO_CONTAINER=n > > CONFIG_IOMMUFD_VFIO_CONTAINER=n > > > > So need to have the below change. I'll incorporate this change in > > this series after your ack. > > > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > > index 99d4b075df49..d675c96c2bbb 100644 > > --- a/drivers/iommu/iommufd/Kconfig > > +++ b/drivers/iommu/iommufd/Kconfig > > @@ -14,8 +14,8 @@ config IOMMUFD > > if IOMMUFD > > config IOMMUFD_VFIO_CONTAINER > > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > > - depends on VFIO_GROUP && !VFIO_CONTAINER > > - default VFIO_GROUP && !VFIO_CONTAINER > > + depends on VFIO_GROUP > > + default n > > help > > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > > IOMMUFD providing compatibility emulation to give the same ioctls. > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > index 6bda6dbb4878..ee3bbad6beb8 100644 > > --- a/drivers/vfio/Kconfig > > +++ b/drivers/vfio/Kconfig > > @@ -6,7 +6,7 @@ menuconfig VFIO > > select INTERVAL_TREE > > select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > > select VFIO_DEVICE_CDEV if !VFIO_GROUP > > - select VFIO_CONTAINER if IOMMUFD=n > > + select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n > > help > > VFIO provides a framework for secure userspace device drivers. > > See Documentation/driver-api/vfio.rst for more details. > > > > Just realized that it is possible to config both VFIO_CONTAINER and > IOMMUFD_VFIO_CONTAINER to "y". Then there will be a conflict when > registering /dev/vfio/vfio. Any suggestion? This is only an issue with the proposed change, right? I agree with Jason, removing /dev/vfio/vfio entirely should be possible. That's actually our ultimate goal, but obviously it breaks current userspace depending on vfio container compatibility. It's a configuration error, not a Kconfig error if someone finds themselves without /dev/vfio/vfio currently. Thanks, Alex > > > help > > > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > > > IOMMUFD providing compatibility emulation to give the same ioctls. > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > > index 1cab8e4729de..35ab8ab87688 100644 > > > --- a/drivers/vfio/Kconfig > > > +++ b/drivers/vfio/Kconfig > > > @@ -4,6 +4,8 @@ menuconfig VFIO > > > select IOMMU_API > > > depends on IOMMUFD || !IOMMUFD > > > select INTERVAL_TREE > > > + select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > > > + select VFIO_DEVICE_CDEV if !VFIO_GROUP > > > select VFIO_CONTAINER if IOMMUFD=n > > > > This should be " select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n" > > > > Regards, > > Yi Liu > > > > > help > > > VFIO provides a framework for secure userspace device drivers. > > > @@ -15,6 +17,7 @@ if VFIO > > > config VFIO_DEVICE_CDEV > > > bool "Support for the VFIO cdev /dev/vfio/devices/vfioX" > > > depends on IOMMUFD > > > + default !VFIO_GROUP > > > help > > > The VFIO device cdev is another way for userspace to get device > > > access. Userspace gets device fd by opening device cdev under > > > @@ -24,9 +27,20 @@ config VFIO_DEVICE_CDEV > > > > > > If you don't know what to do here, say N. > > > > > > +config VFIO_GROUP > > > + bool "Support for the VFIO group /dev/vfio/$group_id" > > > + default y > > > + help > > > + VFIO group support provides the traditional model for accessing > > > + devices through VFIO and is used by the majority of userspace > > > + applications and drivers making use of VFIO. > > > + > > > + If you don't know what to do here, say Y. > > > + > > > config VFIO_CONTAINER > > > bool "Support for the VFIO container /dev/vfio/vfio" > > > select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64) > > > + depends on VFIO_GROUP > > > default y > > > help > > > The VFIO container is the classic interface to VFIO for establishing > > > @@ -48,6 +62,7 @@ endif > > > > > > config VFIO_NOIOMMU > > > bool "VFIO No-IOMMU support" > > > + depends on VFIO_GROUP > > > help > > > VFIO is built on the ability to isolate devices using the IOMMU. > > > Only with an IOMMU can userspace access to DMA capable devices be > > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > > index 245394aeb94b..57c3515af606 100644 > > > --- a/drivers/vfio/Makefile > > > +++ b/drivers/vfio/Makefile > > > @@ -2,9 +2,9 @@ > > > obj-$(CONFIG_VFIO) += vfio.o > > > > > > vfio-y += vfio_main.o \ > > > - group.o \ > > > iova_bitmap.o > > > vfio-$(CONFIG_VFIO_DEVICE_CDEV) += device_cdev.o > > > +vfio-$(CONFIG_VFIO_GROUP) += group.o > > > vfio-$(CONFIG_IOMMUFD) += iommufd.o > > > vfio-$(CONFIG_VFIO_CONTAINER) += container.o > > > vfio-$(CONFIG_VFIO_VIRQFD) += virqfd.o > > > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > > > index e7a3fe093362..b27a3915e6c9 100644 > > > --- a/drivers/vfio/vfio.h > > > +++ b/drivers/vfio/vfio.h > > > @@ -36,6 +36,12 @@ vfio_allocate_device_file(struct vfio_device *device); > > > > > > extern const struct file_operations vfio_device_fops; > > > > > > +#ifdef CONFIG_VFIO_NOIOMMU > > > +extern bool vfio_noiommu __read_mostly; > > > +#else > > > +enum { vfio_noiommu = false }; > > > +#endif > > > + > > > enum vfio_group_type { > > > /* > > > * Physical device with IOMMU backing. > > > @@ -60,6 +66,7 @@ enum vfio_group_type { > > > VFIO_NO_IOMMU, > > > }; > > > > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > struct vfio_group { > > > struct device dev; > > > struct cdev cdev; > > > @@ -111,6 +118,82 @@ static inline bool vfio_device_is_noiommu(struct vfio_device > > > *vdev) > > > return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && > > > vdev->group->type == VFIO_NO_IOMMU; > > > } > > > +#else > > > +struct vfio_group; > > > + > > > +static inline int vfio_device_block_group(struct vfio_device *device) > > > +{ > > > + return 0; > > > +} > > > + > > > +static inline void vfio_device_unblock_group(struct vfio_device *device) > > > +{ > > > +} > > > + > > > +static inline int vfio_device_set_group(struct vfio_device *device, > > > + enum vfio_group_type type) > > > +{ > > > + return 0; > > > +} > > > + > > > +static inline void vfio_device_remove_group(struct vfio_device *device) > > > +{ > > > +} > > > + > > > +static inline void vfio_device_group_register(struct vfio_device *device) > > > +{ > > > +} > > > + > > > +static inline void vfio_device_group_unregister(struct vfio_device *device) > > > +{ > > > +} > > > + > > > +static inline int vfio_device_group_use_iommu(struct vfio_device *device) > > > +{ > > > + return -EOPNOTSUPP; > > > +} > > > + > > > +static inline void vfio_device_group_unuse_iommu(struct vfio_device *device) > > > +{ > > > +} > > > + > > > +static inline void vfio_df_group_close(struct vfio_device_file *df) > > > +{ > > > +} > > > + > > > +static inline struct vfio_group *vfio_group_from_file(struct file *file) > > > +{ > > > + return NULL; > > > +} > > > + > > > +static inline bool vfio_group_enforced_coherent(struct vfio_group *group) > > > +{ > > > + return true; > > > +} > > > + > > > +static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > > > +{ > > > +} > > > + > > > +static inline bool vfio_device_has_container(struct vfio_device *device) > > > +{ > > > + return false; > > > +} > > > + > > > +static inline int __init vfio_group_init(void) > > > +{ > > > + return 0; > > > +} > > > + > > > +static inline void vfio_group_cleanup(void) > > > +{ > > > +} > > > + > > > +static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) > > > +{ > > > + return false; > > > +} > > > +#endif /* CONFIG_VFIO_GROUP */ > > > > > > #if IS_ENABLED(CONFIG_VFIO_CONTAINER) > > > /** > > > @@ -362,12 +445,6 @@ static inline void vfio_virqfd_exit(void) > > > } > > > #endif > > > > > > -#ifdef CONFIG_VFIO_NOIOMMU > > > -extern bool vfio_noiommu __read_mostly; > > > -#else > > > -enum { vfio_noiommu = false }; > > > -#endif > > > - > > > #ifdef CONFIG_HAVE_KVM > > > void _vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm); > > > void vfio_device_put_kvm(struct vfio_device *device); > > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > > index d6228c839c44..5a1dee983f17 100644 > > > --- a/include/linux/vfio.h > > > +++ b/include/linux/vfio.h > > > @@ -43,7 +43,11 @@ struct vfio_device { > > > */ > > > const struct vfio_migration_ops *mig_ops; > > > const struct vfio_log_ops *log_ops; > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > struct vfio_group *group; > > > + struct list_head group_next; > > > + struct list_head iommu_entry; > > > +#endif > > > struct vfio_device_set *dev_set; > > > struct list_head dev_set_list; > > > unsigned int migration_flags; > > > @@ -58,8 +62,6 @@ struct vfio_device { > > > refcount_t refcount; /* user count on registered device*/ > > > unsigned int open_count; > > > struct completion comp; > > > - struct list_head group_next; > > > - struct list_head iommu_entry; > > > struct iommufd_access *iommufd_access; > > > void (*put_kvm)(struct kvm *kvm); > > > #if IS_ENABLED(CONFIG_IOMMUFD) > > > @@ -284,12 +286,29 @@ int vfio_mig_get_next_state(struct vfio_device *device, > > > /* > > > * External user API > > > */ > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > > bool vfio_file_is_group(struct file *file); > > > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > +#else > > > +static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > > > +{ > > > + return NULL; > > > +} > > > + > > > +static inline bool vfio_file_is_group(struct file *file) > > > +{ > > > + return false; > > > +} > > > + > > > +static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > > > +{ > > > + return false; > > > +} > > > +#endif > > > bool vfio_file_is_valid(struct file *file); > > > bool vfio_file_enforced_coherent(struct file *file); > > > void vfio_file_set_kvm(struct file *file, struct kvm *kvm); > > > -bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > > > > #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long)) > > > > > > -- > > > 2.34.1 >
> From: Alex Williamson <alex.williamson@redhat.com> > Sent: Tuesday, July 18, 2023 2:46 AM > > On Mon, 17 Jul 2023 08:08:59 +0000 > "Liu, Yi L" <yi.l.liu@intel.com> wrote: > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > Sent: Monday, July 17, 2023 2:36 PM > > > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > > Sent: Friday, June 16, 2023 5:40 PM > > > > > > > > vfio_group is not needed for vfio device cdev, so with vfio device cdev > > > > introduced, the vfio_group infrastructures can be compiled out if only > > > > cdev is needed. > > > > > > > > Tested-by: Nicolin Chen <nicolinc@nvidia.com> > > > > Tested-by: Matthew Rosato <mjrosato@linux.ibm.com> > > > > Tested-by: Yanting Jiang <yanting.jiang@intel.com> > > > > Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > > > Tested-by: Terrence Xu <terrence.xu@intel.com> > > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > > > --- > > > > drivers/iommu/iommufd/Kconfig | 4 +- > > > > drivers/vfio/Kconfig | 15 ++++++ > > > > drivers/vfio/Makefile | 2 +- > > > > drivers/vfio/vfio.h | 89 ++++++++++++++++++++++++++++++++--- > > > > include/linux/vfio.h | 25 ++++++++-- > > > > 5 files changed, 123 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > > > > index ada693ea51a7..99d4b075df49 100644 > > > > --- a/drivers/iommu/iommufd/Kconfig > > > > +++ b/drivers/iommu/iommufd/Kconfig > > > > @@ -14,8 +14,8 @@ config IOMMUFD > > > > if IOMMUFD > > > > config IOMMUFD_VFIO_CONTAINER > > > > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > > > > - depends on VFIO && !VFIO_CONTAINER > > > > - default VFIO && !VFIO_CONTAINER > > > > + depends on VFIO_GROUP && !VFIO_CONTAINER > > > > + default VFIO_GROUP && !VFIO_CONTAINER > > > > > > Hi Alex, Jason, > > > > > > I found a minor nit on the kconfig. The below configuration is valid. > > > But user cannot use vfio directly as there is no /dev/vfio/vfio. Although > > > user can open /dev/iommu instead. This is not good. > > > > > > CONFIG_IOMMUFD=y > > > CONFIG_VFIO_DEVICE_CDEv=n > > > CONFIG_VFIO_GROUP=y > > > CONFIG_VFIO_CONTAINER=n > > > CONFIG_IOMMUFD_VFIO_CONTAINER=n > > > > > > So need to have the below change. I'll incorporate this change in > > > this series after your ack. > > > > > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > > > index 99d4b075df49..d675c96c2bbb 100644 > > > --- a/drivers/iommu/iommufd/Kconfig > > > +++ b/drivers/iommu/iommufd/Kconfig > > > @@ -14,8 +14,8 @@ config IOMMUFD > > > if IOMMUFD > > > config IOMMUFD_VFIO_CONTAINER > > > bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" > > > - depends on VFIO_GROUP && !VFIO_CONTAINER > > > - default VFIO_GROUP && !VFIO_CONTAINER > > > + depends on VFIO_GROUP > > > + default n > > > help > > > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > > > IOMMUFD providing compatibility emulation to give the same ioctls. > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > > index 6bda6dbb4878..ee3bbad6beb8 100644 > > > --- a/drivers/vfio/Kconfig > > > +++ b/drivers/vfio/Kconfig > > > @@ -6,7 +6,7 @@ menuconfig VFIO > > > select INTERVAL_TREE > > > select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > > > select VFIO_DEVICE_CDEV if !VFIO_GROUP > > > - select VFIO_CONTAINER if IOMMUFD=n > > > + select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n > > > help > > > VFIO provides a framework for secure userspace device drivers. > > > See Documentation/driver-api/vfio.rst for more details. > > > > > > > Just realized that it is possible to config both VFIO_CONTAINER and > > IOMMUFD_VFIO_CONTAINER to "y". Then there will be a conflict when > > registering /dev/vfio/vfio. Any suggestion? > > This is only an issue with the proposed change, right? Yes. > I agree with > Jason, removing /dev/vfio/vfio entirely should be possible. That's > actually our ultimate goal, but obviously it breaks current userspace > depending on vfio container compatibility. It's a configuration error, > not a Kconfig error if someone finds themselves without /dev/vfio/vfio > currently. Thanks, Sure. Let me post a new version then. I've addressed other comments from Jason. Regards, Yi Liu > > Alex > > > > > help > > > > IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on > > > > IOMMUFD providing compatibility emulation to give the same ioctls. > > > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig > > > > index 1cab8e4729de..35ab8ab87688 100644 > > > > --- a/drivers/vfio/Kconfig > > > > +++ b/drivers/vfio/Kconfig > > > > @@ -4,6 +4,8 @@ menuconfig VFIO > > > > select IOMMU_API > > > > depends on IOMMUFD || !IOMMUFD > > > > select INTERVAL_TREE > > > > + select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n > > > > + select VFIO_DEVICE_CDEV if !VFIO_GROUP > > > > select VFIO_CONTAINER if IOMMUFD=n > > > > > > This should be " select VFIO_CONTAINER if IOMMUFD_VFIO_CONTAINER=n" > > > > > > Regards, > > > Yi Liu > > > > > > > help > > > > VFIO provides a framework for secure userspace device drivers. > > > > @@ -15,6 +17,7 @@ if VFIO > > > > config VFIO_DEVICE_CDEV > > > > bool "Support for the VFIO cdev /dev/vfio/devices/vfioX" > > > > depends on IOMMUFD > > > > + default !VFIO_GROUP > > > > help > > > > The VFIO device cdev is another way for userspace to get device > > > > access. Userspace gets device fd by opening device cdev under > > > > @@ -24,9 +27,20 @@ config VFIO_DEVICE_CDEV > > > > > > > > If you don't know what to do here, say N. > > > > > > > > +config VFIO_GROUP > > > > + bool "Support for the VFIO group /dev/vfio/$group_id" > > > > + default y > > > > + help > > > > + VFIO group support provides the traditional model for accessing > > > > + devices through VFIO and is used by the majority of userspace > > > > + applications and drivers making use of VFIO. > > > > + > > > > + If you don't know what to do here, say Y. > > > > + > > > > config VFIO_CONTAINER > > > > bool "Support for the VFIO container /dev/vfio/vfio" > > > > select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64) > > > > + depends on VFIO_GROUP > > > > default y > > > > help > > > > The VFIO container is the classic interface to VFIO for establishing > > > > @@ -48,6 +62,7 @@ endif > > > > > > > > config VFIO_NOIOMMU > > > > bool "VFIO No-IOMMU support" > > > > + depends on VFIO_GROUP > > > > help > > > > VFIO is built on the ability to isolate devices using the IOMMU. > > > > Only with an IOMMU can userspace access to DMA capable devices be > > > > diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile > > > > index 245394aeb94b..57c3515af606 100644 > > > > --- a/drivers/vfio/Makefile > > > > +++ b/drivers/vfio/Makefile > > > > @@ -2,9 +2,9 @@ > > > > obj-$(CONFIG_VFIO) += vfio.o > > > > > > > > vfio-y += vfio_main.o \ > > > > - group.o \ > > > > iova_bitmap.o > > > > vfio-$(CONFIG_VFIO_DEVICE_CDEV) += device_cdev.o > > > > +vfio-$(CONFIG_VFIO_GROUP) += group.o > > > > vfio-$(CONFIG_IOMMUFD) += iommufd.o > > > > vfio-$(CONFIG_VFIO_CONTAINER) += container.o > > > > vfio-$(CONFIG_VFIO_VIRQFD) += virqfd.o > > > > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > > > > index e7a3fe093362..b27a3915e6c9 100644 > > > > --- a/drivers/vfio/vfio.h > > > > +++ b/drivers/vfio/vfio.h > > > > @@ -36,6 +36,12 @@ vfio_allocate_device_file(struct vfio_device *device); > > > > > > > > extern const struct file_operations vfio_device_fops; > > > > > > > > +#ifdef CONFIG_VFIO_NOIOMMU > > > > +extern bool vfio_noiommu __read_mostly; > > > > +#else > > > > +enum { vfio_noiommu = false }; > > > > +#endif > > > > + > > > > enum vfio_group_type { > > > > /* > > > > * Physical device with IOMMU backing. > > > > @@ -60,6 +66,7 @@ enum vfio_group_type { > > > > VFIO_NO_IOMMU, > > > > }; > > > > > > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > > struct vfio_group { > > > > struct device dev; > > > > struct cdev cdev; > > > > @@ -111,6 +118,82 @@ static inline bool vfio_device_is_noiommu(struct > vfio_device > > > > *vdev) > > > > return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && > > > > vdev->group->type == VFIO_NO_IOMMU; > > > > } > > > > +#else > > > > +struct vfio_group; > > > > + > > > > +static inline int vfio_device_block_group(struct vfio_device *device) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > +static inline void vfio_device_unblock_group(struct vfio_device *device) > > > > +{ > > > > +} > > > > + > > > > +static inline int vfio_device_set_group(struct vfio_device *device, > > > > + enum vfio_group_type type) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > +static inline void vfio_device_remove_group(struct vfio_device *device) > > > > +{ > > > > +} > > > > + > > > > +static inline void vfio_device_group_register(struct vfio_device *device) > > > > +{ > > > > +} > > > > + > > > > +static inline void vfio_device_group_unregister(struct vfio_device *device) > > > > +{ > > > > +} > > > > + > > > > +static inline int vfio_device_group_use_iommu(struct vfio_device *device) > > > > +{ > > > > + return -EOPNOTSUPP; > > > > +} > > > > + > > > > +static inline void vfio_device_group_unuse_iommu(struct vfio_device *device) > > > > +{ > > > > +} > > > > + > > > > +static inline void vfio_df_group_close(struct vfio_device_file *df) > > > > +{ > > > > +} > > > > + > > > > +static inline struct vfio_group *vfio_group_from_file(struct file *file) > > > > +{ > > > > + return NULL; > > > > +} > > > > + > > > > +static inline bool vfio_group_enforced_coherent(struct vfio_group *group) > > > > +{ > > > > + return true; > > > > +} > > > > + > > > > +static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) > > > > +{ > > > > +} > > > > + > > > > +static inline bool vfio_device_has_container(struct vfio_device *device) > > > > +{ > > > > + return false; > > > > +} > > > > + > > > > +static inline int __init vfio_group_init(void) > > > > +{ > > > > + return 0; > > > > +} > > > > + > > > > +static inline void vfio_group_cleanup(void) > > > > +{ > > > > +} > > > > + > > > > +static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) > > > > +{ > > > > + return false; > > > > +} > > > > +#endif /* CONFIG_VFIO_GROUP */ > > > > > > > > #if IS_ENABLED(CONFIG_VFIO_CONTAINER) > > > > /** > > > > @@ -362,12 +445,6 @@ static inline void vfio_virqfd_exit(void) > > > > } > > > > #endif > > > > > > > > -#ifdef CONFIG_VFIO_NOIOMMU > > > > -extern bool vfio_noiommu __read_mostly; > > > > -#else > > > > -enum { vfio_noiommu = false }; > > > > -#endif > > > > - > > > > #ifdef CONFIG_HAVE_KVM > > > > void _vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm); > > > > void vfio_device_put_kvm(struct vfio_device *device); > > > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > > > index d6228c839c44..5a1dee983f17 100644 > > > > --- a/include/linux/vfio.h > > > > +++ b/include/linux/vfio.h > > > > @@ -43,7 +43,11 @@ struct vfio_device { > > > > */ > > > > const struct vfio_migration_ops *mig_ops; > > > > const struct vfio_log_ops *log_ops; > > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > > struct vfio_group *group; > > > > + struct list_head group_next; > > > > + struct list_head iommu_entry; > > > > +#endif > > > > struct vfio_device_set *dev_set; > > > > struct list_head dev_set_list; > > > > unsigned int migration_flags; > > > > @@ -58,8 +62,6 @@ struct vfio_device { > > > > refcount_t refcount; /* user count on registered device*/ > > > > unsigned int open_count; > > > > struct completion comp; > > > > - struct list_head group_next; > > > > - struct list_head iommu_entry; > > > > struct iommufd_access *iommufd_access; > > > > void (*put_kvm)(struct kvm *kvm); > > > > #if IS_ENABLED(CONFIG_IOMMUFD) > > > > @@ -284,12 +286,29 @@ int vfio_mig_get_next_state(struct vfio_device *device, > > > > /* > > > > * External user API > > > > */ > > > > +#if IS_ENABLED(CONFIG_VFIO_GROUP) > > > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > > > bool vfio_file_is_group(struct file *file); > > > > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > > +#else > > > > +static inline struct iommu_group *vfio_file_iommu_group(struct file *file) > > > > +{ > > > > + return NULL; > > > > +} > > > > + > > > > +static inline bool vfio_file_is_group(struct file *file) > > > > +{ > > > > + return false; > > > > +} > > > > + > > > > +static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > > > > +{ > > > > + return false; > > > > +} > > > > +#endif > > > > bool vfio_file_is_valid(struct file *file); > > > > bool vfio_file_enforced_coherent(struct file *file); > > > > void vfio_file_set_kvm(struct file *file, struct kvm *kvm); > > > > -bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > > > > > > > > #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long)) > > > > > > > > -- > > > > 2.34.1 > >
diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig index ada693ea51a7..99d4b075df49 100644 --- a/drivers/iommu/iommufd/Kconfig +++ b/drivers/iommu/iommufd/Kconfig @@ -14,8 +14,8 @@ config IOMMUFD if IOMMUFD config IOMMUFD_VFIO_CONTAINER bool "IOMMUFD provides the VFIO container /dev/vfio/vfio" - depends on VFIO && !VFIO_CONTAINER - default VFIO && !VFIO_CONTAINER + depends on VFIO_GROUP && !VFIO_CONTAINER + default VFIO_GROUP && !VFIO_CONTAINER help IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on IOMMUFD providing compatibility emulation to give the same ioctls. diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1cab8e4729de..35ab8ab87688 100644 --- a/drivers/vfio/Kconfig +++ b/drivers/vfio/Kconfig @@ -4,6 +4,8 @@ menuconfig VFIO select IOMMU_API depends on IOMMUFD || !IOMMUFD select INTERVAL_TREE + select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n + select VFIO_DEVICE_CDEV if !VFIO_GROUP select VFIO_CONTAINER if IOMMUFD=n help VFIO provides a framework for secure userspace device drivers. @@ -15,6 +17,7 @@ if VFIO config VFIO_DEVICE_CDEV bool "Support for the VFIO cdev /dev/vfio/devices/vfioX" depends on IOMMUFD + default !VFIO_GROUP help The VFIO device cdev is another way for userspace to get device access. Userspace gets device fd by opening device cdev under @@ -24,9 +27,20 @@ config VFIO_DEVICE_CDEV If you don't know what to do here, say N. +config VFIO_GROUP + bool "Support for the VFIO group /dev/vfio/$group_id" + default y + help + VFIO group support provides the traditional model for accessing + devices through VFIO and is used by the majority of userspace + applications and drivers making use of VFIO. + + If you don't know what to do here, say Y. + config VFIO_CONTAINER bool "Support for the VFIO container /dev/vfio/vfio" select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64) + depends on VFIO_GROUP default y help The VFIO container is the classic interface to VFIO for establishing @@ -48,6 +62,7 @@ endif config VFIO_NOIOMMU bool "VFIO No-IOMMU support" + depends on VFIO_GROUP help VFIO is built on the ability to isolate devices using the IOMMU. Only with an IOMMU can userspace access to DMA capable devices be diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile index 245394aeb94b..57c3515af606 100644 --- a/drivers/vfio/Makefile +++ b/drivers/vfio/Makefile @@ -2,9 +2,9 @@ obj-$(CONFIG_VFIO) += vfio.o vfio-y += vfio_main.o \ - group.o \ iova_bitmap.o vfio-$(CONFIG_VFIO_DEVICE_CDEV) += device_cdev.o +vfio-$(CONFIG_VFIO_GROUP) += group.o vfio-$(CONFIG_IOMMUFD) += iommufd.o vfio-$(CONFIG_VFIO_CONTAINER) += container.o vfio-$(CONFIG_VFIO_VIRQFD) += virqfd.o diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h index e7a3fe093362..b27a3915e6c9 100644 --- a/drivers/vfio/vfio.h +++ b/drivers/vfio/vfio.h @@ -36,6 +36,12 @@ vfio_allocate_device_file(struct vfio_device *device); extern const struct file_operations vfio_device_fops; +#ifdef CONFIG_VFIO_NOIOMMU +extern bool vfio_noiommu __read_mostly; +#else +enum { vfio_noiommu = false }; +#endif + enum vfio_group_type { /* * Physical device with IOMMU backing. @@ -60,6 +66,7 @@ enum vfio_group_type { VFIO_NO_IOMMU, }; +#if IS_ENABLED(CONFIG_VFIO_GROUP) struct vfio_group { struct device dev; struct cdev cdev; @@ -111,6 +118,82 @@ static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) return IS_ENABLED(CONFIG_VFIO_NOIOMMU) && vdev->group->type == VFIO_NO_IOMMU; } +#else +struct vfio_group; + +static inline int vfio_device_block_group(struct vfio_device *device) +{ + return 0; +} + +static inline void vfio_device_unblock_group(struct vfio_device *device) +{ +} + +static inline int vfio_device_set_group(struct vfio_device *device, + enum vfio_group_type type) +{ + return 0; +} + +static inline void vfio_device_remove_group(struct vfio_device *device) +{ +} + +static inline void vfio_device_group_register(struct vfio_device *device) +{ +} + +static inline void vfio_device_group_unregister(struct vfio_device *device) +{ +} + +static inline int vfio_device_group_use_iommu(struct vfio_device *device) +{ + return -EOPNOTSUPP; +} + +static inline void vfio_device_group_unuse_iommu(struct vfio_device *device) +{ +} + +static inline void vfio_df_group_close(struct vfio_device_file *df) +{ +} + +static inline struct vfio_group *vfio_group_from_file(struct file *file) +{ + return NULL; +} + +static inline bool vfio_group_enforced_coherent(struct vfio_group *group) +{ + return true; +} + +static inline void vfio_group_set_kvm(struct vfio_group *group, struct kvm *kvm) +{ +} + +static inline bool vfio_device_has_container(struct vfio_device *device) +{ + return false; +} + +static inline int __init vfio_group_init(void) +{ + return 0; +} + +static inline void vfio_group_cleanup(void) +{ +} + +static inline bool vfio_device_is_noiommu(struct vfio_device *vdev) +{ + return false; +} +#endif /* CONFIG_VFIO_GROUP */ #if IS_ENABLED(CONFIG_VFIO_CONTAINER) /** @@ -362,12 +445,6 @@ static inline void vfio_virqfd_exit(void) } #endif -#ifdef CONFIG_VFIO_NOIOMMU -extern bool vfio_noiommu __read_mostly; -#else -enum { vfio_noiommu = false }; -#endif - #ifdef CONFIG_HAVE_KVM void _vfio_device_get_kvm_safe(struct vfio_device *device, struct kvm *kvm); void vfio_device_put_kvm(struct vfio_device *device); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index d6228c839c44..5a1dee983f17 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -43,7 +43,11 @@ struct vfio_device { */ const struct vfio_migration_ops *mig_ops; const struct vfio_log_ops *log_ops; +#if IS_ENABLED(CONFIG_VFIO_GROUP) struct vfio_group *group; + struct list_head group_next; + struct list_head iommu_entry; +#endif struct vfio_device_set *dev_set; struct list_head dev_set_list; unsigned int migration_flags; @@ -58,8 +62,6 @@ struct vfio_device { refcount_t refcount; /* user count on registered device*/ unsigned int open_count; struct completion comp; - struct list_head group_next; - struct list_head iommu_entry; struct iommufd_access *iommufd_access; void (*put_kvm)(struct kvm *kvm); #if IS_ENABLED(CONFIG_IOMMUFD) @@ -284,12 +286,29 @@ int vfio_mig_get_next_state(struct vfio_device *device, /* * External user API */ +#if IS_ENABLED(CONFIG_VFIO_GROUP) struct iommu_group *vfio_file_iommu_group(struct file *file); bool vfio_file_is_group(struct file *file); +bool vfio_file_has_dev(struct file *file, struct vfio_device *device); +#else +static inline struct iommu_group *vfio_file_iommu_group(struct file *file) +{ + return NULL; +} + +static inline bool vfio_file_is_group(struct file *file) +{ + return false; +} + +static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *device) +{ + return false; +} +#endif bool vfio_file_is_valid(struct file *file); bool vfio_file_enforced_coherent(struct file *file); void vfio_file_set_kvm(struct file *file, struct kvm *kvm); -bool vfio_file_has_dev(struct file *file, struct vfio_device *device); #define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long))