RE: [PATCH v5 18/19] vfio: Compile group optionally

Liu, Yi L
2023-02-28T06:00:25Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Monday, February 27, 2023 7:12 PM
>
> group code is not needed for vfio device cdev, so with vfio device cdev
> introduced, the group infrastructures can be compiled out if only cdev
> is needed.
>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/vfio/Kconfig | 14 +++++++++
> drivers/vfio/Makefile | 2 +-
> drivers/vfio/vfio.h | 72
> +++++++++++++++++++++++++++++++++++++++++++
> include/linux/vfio.h | 24 ++++++++++++++-
> 4 files changed, 110 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 169762316513..c3ab06c314ea 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
> + select VFIO_DEVICE_CDEV if !VFIO_GROUP && (X86 || S390 || ARM || ARM64)

Got below warning when IOMMUFD=n, VFIO_GROUP=n. so may remove
this select or needs to let VFIO_DEVICE_CDEV select IOMMUFD instead of
depends on IOMMUFD.

WARNING: unmet direct dependencies detected for VFIO_DEVICE_CDEV
Depends on [n]: VFIO [=m] && IOMMUFD [=n]
Selected by [m]:
- VFIO [=m] && (IOMMUFD [=n] || !IOMMUFD [=n]) && !VFIO_GROUP [=n]

> select VFIO_CONTAINER if IOMMUFD=n

Needs to be if IOMMUFD=n && VFIO_GROUP, otherwise vfio container
is compiled even VFIO_GROUP=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 && (X86 || S390 || ARM || ARM64)

depends on IOMMUFD have warning when IOMMUFD=n and VFIO_GROUP=n.

> + 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
> 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 5a1ceb014779..a7b88521bf48 100644
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -62,6 +62,7 @@ enum vfio_group_type {
> VFIO_NO_IOMMU,
> };
>
> +#if IS_ENABLED(CONFIG_VFIO_GROUP)
> struct vfio_group {
> struct device dev;
> struct cdev cdev;
> @@ -107,6 +108,77 @@ void vfio_group_set_kvm(struct vfio_group *group,
> struct kvm *kvm);
> bool vfio_device_has_container(struct vfio_device *device);
> int __init vfio_group_init(void);
> void vfio_group_cleanup(void);
> +#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_device_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)
> +{
> +}
> +#endif /* CONFIG_VFIO_GROUP */
>
> #if IS_ENABLED(CONFIG_VFIO_CONTAINER)
> /**
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index ce390533cb30..d12384824656 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -43,7 +43,9 @@ 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;
> +#endif
> struct vfio_device_set *dev_set;
> struct list_head dev_set_list;
> unsigned int migration_flags;
> @@ -58,8 +60,10 @@ struct vfio_device {
> refcount_t refcount; /* user count on registered device*/
> unsigned int open_count;
> struct completion comp;
> +#if IS_ENABLED(CONFIG_VFIO_GROUP)
> struct list_head group_next;
> struct list_head iommu_entry;
> +#endif
> struct iommufd_access *iommufd_access;
> void (*put_kvm)(struct kvm *kvm);
> #if IS_ENABLED(CONFIG_IOMMUFD)
> @@ -257,12 +261,30 @@ 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