diff mbox series

[v2,11/11] iommufd: Allow iommufd to supply /dev/vfio/vfio

Message ID 11-v2-65016290f146+33e-vfio_iommufd_jgg@nvidia.com (mailing list archive)
State New, archived
Headers show
Series Connect VFIO to IOMMUFD | expand

Commit Message

Jason Gunthorpe Nov. 8, 2022, 12:52 a.m. UTC
If the VFIO container is compiled out, give a kconfig option for iommufd
to provide the miscdev node with the same name and permissions as vfio
uses.

The compatibility node supports the same ioctls as VFIO and automatically
enables the VFIO compatible pinned page accounting mode.

Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/iommufd/Kconfig | 12 ++++++++++++
 drivers/iommu/iommufd/main.c  | 36 +++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+)

Comments

Tian, Kevin Nov. 10, 2022, 7:01 a.m. UTC | #1
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 8, 2022 8:53 AM
> 
> If the VFIO container is compiled out, give a kconfig option for iommufd
> to provide the miscdev node with the same name and permissions as vfio
> uses.
> 
> The compatibility node supports the same ioctls as VFIO and automatically
> enables the VFIO compatible pinned page accounting mode.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Yi Liu Nov. 11, 2022, 4:16 a.m. UTC | #2
On 2022/11/8 08:52, Jason Gunthorpe wrote:
> If the VFIO container is compiled out, give a kconfig option for iommufd
> to provide the miscdev node with the same name and permissions as vfio
> uses.
> 
> The compatibility node supports the same ioctls as VFIO and automatically
> enables the VFIO compatible pinned page accounting mode.
> 
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>   drivers/iommu/iommufd/Kconfig | 12 ++++++++++++
>   drivers/iommu/iommufd/main.c  | 36 +++++++++++++++++++++++++++++++++++
>   2 files changed, 48 insertions(+)
> 
> diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
> index 399a2edeaef6de..f387f803dc6f7f 100644
> --- a/drivers/iommu/iommufd/Kconfig
> +++ b/drivers/iommu/iommufd/Kconfig
> @@ -12,6 +12,18 @@ config IOMMUFD
>   	  If you don't know what to do here, say N.
>   
>   if IOMMUFD
> +config IOMMUFD_VFIO_CONTAINER
> +	bool "IOMMUFD provides the VFIO container /dev/vfio/vfio"
> +	depends on VFIO && !VFIO_CONTAINER
> +	default VFIO && !VFIO_CONTAINER
> +	help
> +	  IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on
> +	  IOMMUFD providing compatibility emulation to give the same ioctls.
> +	  It provides an option to build a kernel with legacy VFIO components
> +	  removed.
> +
> +	  Unless testing IOMMUFD say N here.
> +
>   config IOMMUFD_TEST
>   	bool "IOMMU Userspace API Test support"
>   	depends on RUNTIME_TESTING_MENU
> diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
> index ab3fa05f38505d..1eeb326f74f005 100644
> --- a/drivers/iommu/iommufd/main.c
> +++ b/drivers/iommu/iommufd/main.c
> @@ -18,6 +18,7 @@
>   #include <uapi/linux/iommufd.h>
>   #include <linux/iommufd.h>
>   
> +#include "io_pagetable.h"
>   #include "iommufd_private.h"
>   #include "iommufd_test.h"
>   
> @@ -25,6 +26,7 @@ struct iommufd_object_ops {
>   	void (*destroy)(struct iommufd_object *obj);
>   };
>   static const struct iommufd_object_ops iommufd_object_ops[];
> +static struct miscdevice vfio_misc_dev;
>   
>   struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
>   					     size_t size,
> @@ -170,6 +172,16 @@ static int iommufd_fops_open(struct inode *inode, struct file *filp)
>   	if (!ictx)
>   		return -ENOMEM;
>   
> +	/*
> +	 * For compatibility with VFIO when /dev/vfio/vfio is opened we default
> +	 * to the same rlimit accounting as vfio uses.
> +	 */
> +	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER) &&
> +	    filp->private_data == &vfio_misc_dev) {
> +		ictx->account_mode = IOPT_PAGES_ACCOUNT_MM;
> +		pr_info_once("IOMMUFD is providing /dev/vfio/vfio, not VFIO.\n");
> +	}
> +
>   	xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT);
>   	ictx->file = filp;
>   	filp->private_data = ictx;
> @@ -395,6 +407,15 @@ static struct miscdevice iommu_misc_dev = {
>   	.mode = 0660,
>   };
>   
> +
> +static struct miscdevice vfio_misc_dev = {
> +	.minor = VFIO_MINOR,
> +	.name = "vfio",
> +	.fops = &iommufd_fops,
> +	.nodename = "vfio/vfio",
> +	.mode = 0666,
> +};
> +
>   static int __init iommufd_init(void)
>   {
>   	int ret;
> @@ -402,18 +423,33 @@ static int __init iommufd_init(void)
>   	ret = misc_register(&iommu_misc_dev);
>   	if (ret)
>   		return ret;
> +
> +	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)) {
> +		ret = misc_register(&vfio_misc_dev);
> +		if (ret)
> +			goto err_misc;
> +	}
>   	iommufd_test_init();
>   	return 0;
> +err_misc:
> +	misc_deregister(&iommu_misc_dev);
> +	return ret;
>   }
>   
>   static void __exit iommufd_exit(void)
>   {
>   	iommufd_test_exit();
> +	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER))
> +		misc_deregister(&vfio_misc_dev);
>   	misc_deregister(&iommu_misc_dev);
>   }
>   
>   module_init(iommufd_init);
>   module_exit(iommufd_exit);
>   
> +#if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)
> +MODULE_ALIAS_MISCDEV(VFIO_MINOR);
> +MODULE_ALIAS("devname:vfio/vfio");

will this line also result in systemd to create this devnodes at boot
based on the module info even if the IOMMUFD_VFIO_CONTAINER is not
configured?

> +#endif
>   MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices");
>   MODULE_LICENSE("GPL");
Yi Liu Nov. 11, 2022, 6:38 a.m. UTC | #3
On 2022/11/11 12:16, Yi Liu wrote:
> 
> 
> On 2022/11/8 08:52, Jason Gunthorpe wrote:
>> If the VFIO container is compiled out, give a kconfig option for iommufd
>> to provide the miscdev node with the same name and permissions as vfio
>> uses.
>>
>> The compatibility node supports the same ioctls as VFIO and automatically
>> enables the VFIO compatible pinned page accounting mode.
>>
>> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>> ---
>>   drivers/iommu/iommufd/Kconfig | 12 ++++++++++++
>>   drivers/iommu/iommufd/main.c  | 36 +++++++++++++++++++++++++++++++++++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
>> index 399a2edeaef6de..f387f803dc6f7f 100644
>> --- a/drivers/iommu/iommufd/Kconfig
>> +++ b/drivers/iommu/iommufd/Kconfig
>> @@ -12,6 +12,18 @@ config IOMMUFD
>>         If you don't know what to do here, say N.
>>   if IOMMUFD
>> +config IOMMUFD_VFIO_CONTAINER
>> +    bool "IOMMUFD provides the VFIO container /dev/vfio/vfio"
>> +    depends on VFIO && !VFIO_CONTAINER
>> +    default VFIO && !VFIO_CONTAINER
>> +    help
>> +      IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on
>> +      IOMMUFD providing compatibility emulation to give the same ioctls.
>> +      It provides an option to build a kernel with legacy VFIO components
>> +      removed.
>> +
>> +      Unless testing IOMMUFD say N here.
>> +
>>   config IOMMUFD_TEST
>>       bool "IOMMU Userspace API Test support"
>>       depends on RUNTIME_TESTING_MENU
>> diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
>> index ab3fa05f38505d..1eeb326f74f005 100644
>> --- a/drivers/iommu/iommufd/main.c
>> +++ b/drivers/iommu/iommufd/main.c
>> @@ -18,6 +18,7 @@
>>   #include <uapi/linux/iommufd.h>
>>   #include <linux/iommufd.h>
>> +#include "io_pagetable.h"
>>   #include "iommufd_private.h"
>>   #include "iommufd_test.h"
>> @@ -25,6 +26,7 @@ struct iommufd_object_ops {
>>       void (*destroy)(struct iommufd_object *obj);
>>   };
>>   static const struct iommufd_object_ops iommufd_object_ops[];
>> +static struct miscdevice vfio_misc_dev;
>>   struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
>>                            size_t size,
>> @@ -170,6 +172,16 @@ static int iommufd_fops_open(struct inode *inode, 
>> struct file *filp)
>>       if (!ictx)
>>           return -ENOMEM;
>> +    /*
>> +     * For compatibility with VFIO when /dev/vfio/vfio is opened we default
>> +     * to the same rlimit accounting as vfio uses.
>> +     */
>> +    if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER) &&
>> +        filp->private_data == &vfio_misc_dev) {
>> +        ictx->account_mode = IOPT_PAGES_ACCOUNT_MM;
>> +        pr_info_once("IOMMUFD is providing /dev/vfio/vfio, not VFIO.\n");
>> +    }
>> +
>>       xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT);
>>       ictx->file = filp;
>>       filp->private_data = ictx;
>> @@ -395,6 +407,15 @@ static struct miscdevice iommu_misc_dev = {
>>       .mode = 0660,
>>   };
>> +
>> +static struct miscdevice vfio_misc_dev = {
>> +    .minor = VFIO_MINOR,
>> +    .name = "vfio",
>> +    .fops = &iommufd_fops,
>> +    .nodename = "vfio/vfio",
>> +    .mode = 0666,
>> +};
>> +
>>   static int __init iommufd_init(void)
>>   {
>>       int ret;
>> @@ -402,18 +423,33 @@ static int __init iommufd_init(void)
>>       ret = misc_register(&iommu_misc_dev);
>>       if (ret)
>>           return ret;
>> +
>> +    if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)) {
>> +        ret = misc_register(&vfio_misc_dev);
>> +        if (ret)
>> +            goto err_misc;
>> +    }
>>       iommufd_test_init();
>>       return 0;
>> +err_misc:
>> +    misc_deregister(&iommu_misc_dev);
>> +    return ret;
>>   }
>>   static void __exit iommufd_exit(void)
>>   {
>>       iommufd_test_exit();
>> +    if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER))
>> +        misc_deregister(&vfio_misc_dev);
>>       misc_deregister(&iommu_misc_dev);
>>   }
>>   module_init(iommufd_init);
>>   module_exit(iommufd_exit);
>> +#if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)
>> +MODULE_ALIAS_MISCDEV(VFIO_MINOR);
>> +MODULE_ALIAS("devname:vfio/vfio");
> 
> will this line also result in systemd to create this devnodes at boot
> based on the module info even if the IOMMUFD_VFIO_CONTAINER is not
> configured?

stale comment. it's already under if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER).

looks good to me.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

>> +#endif
>>   MODULE_DESCRIPTION("I/O Address Space Management for passthrough 
>> devices");
>>   MODULE_LICENSE("GPL");
>
Jason Gunthorpe Nov. 14, 2022, 2:50 p.m. UTC | #4
On Fri, Nov 11, 2022 at 12:16:02PM +0800, Yi Liu wrote:
> > +#if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)
> > +MODULE_ALIAS_MISCDEV(VFIO_MINOR);
> > +MODULE_ALIAS("devname:vfio/vfio");
> 
> will this line also result in systemd to create this devnodes at boot
> based on the module info even if the IOMMUFD_VFIO_CONTAINER is not
> configured?

No, it is contained by an ifdef.

The MODULE_ALIAS mechanism works by inspecting the compiled object
files for special ELF sections, if the code is discarded by the
preprocessor then the sections will not be created by the compiler.

Jason
diff mbox series

Patch

diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig
index 399a2edeaef6de..f387f803dc6f7f 100644
--- a/drivers/iommu/iommufd/Kconfig
+++ b/drivers/iommu/iommufd/Kconfig
@@ -12,6 +12,18 @@  config IOMMUFD
 	  If you don't know what to do here, say N.
 
 if IOMMUFD
+config IOMMUFD_VFIO_CONTAINER
+	bool "IOMMUFD provides the VFIO container /dev/vfio/vfio"
+	depends on VFIO && !VFIO_CONTAINER
+	default VFIO && !VFIO_CONTAINER
+	help
+	  IOMMUFD will provide /dev/vfio/vfio instead of VFIO. This relies on
+	  IOMMUFD providing compatibility emulation to give the same ioctls.
+	  It provides an option to build a kernel with legacy VFIO components
+	  removed.
+
+	  Unless testing IOMMUFD say N here.
+
 config IOMMUFD_TEST
 	bool "IOMMU Userspace API Test support"
 	depends on RUNTIME_TESTING_MENU
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
index ab3fa05f38505d..1eeb326f74f005 100644
--- a/drivers/iommu/iommufd/main.c
+++ b/drivers/iommu/iommufd/main.c
@@ -18,6 +18,7 @@ 
 #include <uapi/linux/iommufd.h>
 #include <linux/iommufd.h>
 
+#include "io_pagetable.h"
 #include "iommufd_private.h"
 #include "iommufd_test.h"
 
@@ -25,6 +26,7 @@  struct iommufd_object_ops {
 	void (*destroy)(struct iommufd_object *obj);
 };
 static const struct iommufd_object_ops iommufd_object_ops[];
+static struct miscdevice vfio_misc_dev;
 
 struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,
 					     size_t size,
@@ -170,6 +172,16 @@  static int iommufd_fops_open(struct inode *inode, struct file *filp)
 	if (!ictx)
 		return -ENOMEM;
 
+	/*
+	 * For compatibility with VFIO when /dev/vfio/vfio is opened we default
+	 * to the same rlimit accounting as vfio uses.
+	 */
+	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER) &&
+	    filp->private_data == &vfio_misc_dev) {
+		ictx->account_mode = IOPT_PAGES_ACCOUNT_MM;
+		pr_info_once("IOMMUFD is providing /dev/vfio/vfio, not VFIO.\n");
+	}
+
 	xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT);
 	ictx->file = filp;
 	filp->private_data = ictx;
@@ -395,6 +407,15 @@  static struct miscdevice iommu_misc_dev = {
 	.mode = 0660,
 };
 
+
+static struct miscdevice vfio_misc_dev = {
+	.minor = VFIO_MINOR,
+	.name = "vfio",
+	.fops = &iommufd_fops,
+	.nodename = "vfio/vfio",
+	.mode = 0666,
+};
+
 static int __init iommufd_init(void)
 {
 	int ret;
@@ -402,18 +423,33 @@  static int __init iommufd_init(void)
 	ret = misc_register(&iommu_misc_dev);
 	if (ret)
 		return ret;
+
+	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)) {
+		ret = misc_register(&vfio_misc_dev);
+		if (ret)
+			goto err_misc;
+	}
 	iommufd_test_init();
 	return 0;
+err_misc:
+	misc_deregister(&iommu_misc_dev);
+	return ret;
 }
 
 static void __exit iommufd_exit(void)
 {
 	iommufd_test_exit();
+	if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER))
+		misc_deregister(&vfio_misc_dev);
 	misc_deregister(&iommu_misc_dev);
 }
 
 module_init(iommufd_init);
 module_exit(iommufd_exit);
 
+#if IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)
+MODULE_ALIAS_MISCDEV(VFIO_MINOR);
+MODULE_ALIAS("devname:vfio/vfio");
+#endif
 MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices");
 MODULE_LICENSE("GPL");