Re: [Intel-gfx] [PATCH v8 23/24] vfio: Compile group optionally
> From: Alex Williamson
> Sent: Thursday, March 30, 2023 5:51 AM
>
> On Mon, 27 Mar 2023 02:40:46 -0700
> Yi Liu wrote:
>
> > 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.
> >
> > Reviewed-by: Kevin Tian
> > Tested-by: Terrence Xu
> > Signed-off-by: Yi Liu
> > ---
> > drivers/iommu/iommufd/Kconfig | 4 +-
> > drivers/vfio/Kconfig | 16 -
> > drivers/vfio/Makefile | 2 +-
> > drivers/vfio/vfio.h | 111 --
> > include/linux/vfio.h | 13 +++-
> > 5 files changed, 134 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
> > index ada693ea51a7..1946eed1826a 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 && VFIO_GROUP && !VFIO_CONTAINER
> > + default VFIO && VFIO_GROUP && !VFIO_CONTAINER
>
> Shouldn't these simply replace VFIO with VFIO_GROUP since VFIO_GROUP
> necessarily depends on VFIO?
looks so.
>
> > 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 e2105b4dac2d..0942a19601a2 100644
> > --- a/drivers/vfio/Kconfig
> > +++ b/drivers/vfio/Kconfig
> > @@ -4,7 +4,9 @@ menuconfig VFIO
> > select IOMMU_API
> > depends on IOMMUFD || !IOMMUFD
> > select INTERVAL_TREE
> > - select VFIO_CONTAINER if IOMMUFD=n
> > + select VFIO_GROUP if SPAPR_TCE_IOMMU || !IOMMUFD
>
> This needs to be IOMMUFD=n or else VFIO_GROUP cannot be unset when
> IOMMUFD=m
yes. btw. does it mean the "depends on IOMMUFD || !IOMMUFD"
also use IOMMUFD=n?
>
> > + select VFIO_DEVICE_CDEV if !VFIO_GROUP
> > + select VFIO_CONTAINER if IOMMUFD=n && VFIO_GROUP
>
> The fact that CONTAINER depends on GROUP seems to be sufficient that we
> don't need GROUP here. Thanks,
right. I added VFIO_GROUP as I saw a time that container code was comipled
when IOMMUFD=n and VFIO_GROUP=n. This encounters compiling issue since
container code refers device->group->xxx. But this should have been fixed by
selecting VFIO_GROUP when IOMMUFD=n.
Thanks,
Yi Liu
>
> Alex
>
> > help
> > VFIO provides a framework for secure userspace device drivers.
> > See Documentation/driver-api/vfio.rst for more details.
> > @@ -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
> > @@ -23,9 +26,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 c199e410db18..9c7a238ec8dd 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_
Re: [Intel-gfx] [PATCH v8 23/24] vfio: Compile group optionally
On Mon, 27 Mar 2023 02:40:46 -0700
Yi Liu wrote:
> 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.
>
> Reviewed-by: Kevin Tian
> Tested-by: Terrence Xu
> Signed-off-by: Yi Liu
> ---
> drivers/iommu/iommufd/Kconfig | 4 +-
> drivers/vfio/Kconfig | 16 -
> drivers/vfio/Makefile | 2 +-
> drivers/vfio/vfio.h | 111 --
> include/linux/vfio.h | 13 +++-
> 5 files changed, 134 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
> index ada693ea51a7..1946eed1826a 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 && VFIO_GROUP && !VFIO_CONTAINER
> + default VFIO && VFIO_GROUP && !VFIO_CONTAINER
Shouldn't these simply replace VFIO with VFIO_GROUP since VFIO_GROUP
necessarily depends on VFIO?
> 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 e2105b4dac2d..0942a19601a2 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -4,7 +4,9 @@ menuconfig VFIO
> select IOMMU_API
> depends on IOMMUFD || !IOMMUFD
> select INTERVAL_TREE
> - select VFIO_CONTAINER if IOMMUFD=n
> + select VFIO_GROUP if SPAPR_TCE_IOMMU || !IOMMUFD
This needs to be IOMMUFD=n or else VFIO_GROUP cannot be unset when
IOMMUFD=m
> + select VFIO_DEVICE_CDEV if !VFIO_GROUP
> + select VFIO_CONTAINER if IOMMUFD=n && VFIO_GROUP
The fact that CONTAINER depends on GROUP seems to be sufficient that we
don't need GROUP here. Thanks,
Alex
> help
> VFIO provides a framework for secure userspace device drivers.
> See Documentation/driver-api/vfio.rst for more details.
> @@ -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
> @@ -23,9 +26,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 c199e410db18..9c7a238ec8dd 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;
> @@ -113,6 +120,104 @@ static inline void vfio_device_set_noiommu(struct
> vfio_device *device)
> device->noiommu = IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> device->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_d
[Intel-gfx] [PATCH v8 23/24] vfio: Compile group optionally
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.
Reviewed-by: Kevin Tian
Tested-by: Terrence Xu
Signed-off-by: Yi Liu
---
drivers/iommu/iommufd/Kconfig | 4 +-
drivers/vfio/Kconfig | 16 -
drivers/vfio/Makefile | 2 +-
drivers/vfio/vfio.h | 111 --
include/linux/vfio.h | 13 +++-
5 files changed, 134 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
index ada693ea51a7..1946eed1826a 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 && VFIO_GROUP && !VFIO_CONTAINER
+ default VFIO && 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 e2105b4dac2d..0942a19601a2 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -4,7 +4,9 @@ menuconfig VFIO
select IOMMU_API
depends on IOMMUFD || !IOMMUFD
select INTERVAL_TREE
- select VFIO_CONTAINER if IOMMUFD=n
+ select VFIO_GROUP if SPAPR_TCE_IOMMU || !IOMMUFD
+ select VFIO_DEVICE_CDEV if !VFIO_GROUP
+ select VFIO_CONTAINER if IOMMUFD=n && VFIO_GROUP
help
VFIO provides a framework for secure userspace device drivers.
See Documentation/driver-api/vfio.rst for more details.
@@ -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
@@ -23,9 +26,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 c199e410db18..9c7a238ec8dd 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;
@@ -113,6 +120,104 @@ static inline void vfio_device_set_noiommu(struct
vfio_device *device)
device->noiommu = IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
device->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 bool vfio_device_gro
