| Message ID | 10-v1-4991695894d8+211-vfio_iommufd_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Connect VFIO to IOMMUFD | expand |
On Tue, 25 Oct 2022 15:50:45 -0300 Jason Gunthorpe <jgg@nvidia.com> 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. I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is provided by something other than the vfio container code. If we intend to include this before P2P is resolved, that breadcrumb (dmesg I'm guessing) might also list any known limitations of the compatibility to save time with debugging. Thanks, Alex > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> > --- > drivers/iommu/iommufd/Kconfig | 12 ++++++++++++ > drivers/iommu/iommufd/main.c | 35 ++++++++++++++++++++++++++++++++--- > 2 files changed, 44 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig > index f0a2012234fa09..afc83b7575cce6 100644 > --- a/drivers/iommu/iommufd/Kconfig > +++ b/drivers/iommu/iommufd/Kconfig > @@ -14,6 +14,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 8a31c1a14cdd53..19db81fbf7f08f 100644 > --- a/drivers/iommu/iommufd/main.c > +++ b/drivers/iommu/iommufd/main.c > @@ -24,6 +24,7 @@ > #include <uapi/linux/iommufd.h> > #include <linux/iommufd.h> > > +#include "io_pagetable.h" > #include "iommufd_private.h" > #include "iommufd_test.h" > > @@ -31,6 +32,7 @@ struct iommufd_object_ops { > void (*destroy)(struct iommufd_object *obj); > }; > static 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, > @@ -167,6 +169,13 @@ 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 (filp->private_data == &vfio_misc_dev) > + ictx->account_mode = IOPT_PAGES_ACCOUNT_MM; > + > xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT); > ictx->file = filp; > filp->private_data = ictx; > @@ -392,26 +401,46 @@ 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; > > ret = misc_register(&iommu_misc_dev); > - if (ret) { > - pr_err("Failed to register misc device\n"); > + if (ret) > return ret; > - } > > + if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)) { > + ret = misc_register(&vfio_misc_dev); > + if (ret) > + goto err_misc; > + } > return 0; > +err_misc: > + misc_deregister(&iommu_misc_dev); > + return ret; > } > > static void __exit iommufd_exit(void) > { > + 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); > +#endif > MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices"); > MODULE_LICENSE("GPL");
On Wed, Oct 26, 2022 at 03:31:33PM -0600, Alex Williamson wrote: > On Tue, 25 Oct 2022 15:50:45 -0300 > Jason Gunthorpe <jgg@nvidia.com> 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. > > I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is > provided by something other than the vfio container code. If we intend > to include this before P2P is resolved, that breadcrumb I don't belive I can get P2P done soon enough. I plan to do it after this is merged. Right now these two series are taking all my time. > (dmesg I'm guessing) might also list any known limitations of the > compatibility to save time with debugging. Thanks, Yes, that makes sense. Do you want a dmesg at module load time, on every open, or a sysfs something? What seems like it would make it into a bug report? Thanks, Jason
On Fri, 28 Oct 2022 15:44:36 -0300 Jason Gunthorpe <jgg@nvidia.com> wrote: > On Wed, Oct 26, 2022 at 03:31:33PM -0600, Alex Williamson wrote: > > On Tue, 25 Oct 2022 15:50:45 -0300 > > Jason Gunthorpe <jgg@nvidia.com> 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. > > > > I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is > > provided by something other than the vfio container code. If we intend > > to include this before P2P is resolved, that breadcrumb > > I don't belive I can get P2P done soon enough. I plan to do it after > this is merged. Right now these two series are taking all my time. > > > (dmesg I'm guessing) might also list any known limitations of the > > compatibility to save time with debugging. Thanks, > > Yes, that makes sense. > > Do you want a dmesg at module load time, on every open, or a sysfs > something? What seems like it would make it into a bug report? I think dmesg at module load time should probably be ok, every open seems like harassment and sysfs would require updated support in various bug reporting tools. Users are often terrible about reporting full dmesg in bugs, but they do often filter it for "IOMMU" or "VFIO", so keep that in mind when crafting the log message. Thanks, Alex
On Mon, Oct 31, 2022 at 04:53:11PM -0600, Alex Williamson wrote: > On Fri, 28 Oct 2022 15:44:36 -0300 > Jason Gunthorpe <jgg@nvidia.com> wrote: > > > On Wed, Oct 26, 2022 at 03:31:33PM -0600, Alex Williamson wrote: > > > On Tue, 25 Oct 2022 15:50:45 -0300 > > > Jason Gunthorpe <jgg@nvidia.com> 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. > > > > > > I think I'd like to see some sort of breadcrumb when /dev/vfio/vfio is > > > provided by something other than the vfio container code. If we intend > > > to include this before P2P is resolved, that breadcrumb > > > > I don't belive I can get P2P done soon enough. I plan to do it after > > this is merged. Right now these two series are taking all my time. > > > > > (dmesg I'm guessing) might also list any known limitations of the > > > compatibility to save time with debugging. Thanks, > > > > Yes, that makes sense. > > > > Do you want a dmesg at module load time, on every open, or a sysfs > > something? What seems like it would make it into a bug report? > > I think dmesg at module load time should probably be ok, every open > seems like harassment and sysfs would require updated support in > various bug reporting tools. Users are often terrible about reporting > full dmesg in bugs, but they do often filter it for "IOMMU" or "VFIO", > so keep that in mind when crafting the log message. Thanks, This seems like the right approach, the message comes out once when it might be most useful: @@ -176,8 +176,11 @@ static int iommufd_fops_open(struct inode *inode, struct file *filp) * For compatibility with VFIO when /dev/vfio/vfio is opened we default * to the same rlimit accounting as vfio uses. */ - if (filp->private_data == &vfio_misc_dev) + 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; Also this is needed: @@ -446,6 +449,7 @@ 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"); Thanks, Jason
diff --git a/drivers/iommu/iommufd/Kconfig b/drivers/iommu/iommufd/Kconfig index f0a2012234fa09..afc83b7575cce6 100644 --- a/drivers/iommu/iommufd/Kconfig +++ b/drivers/iommu/iommufd/Kconfig @@ -14,6 +14,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 8a31c1a14cdd53..19db81fbf7f08f 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -24,6 +24,7 @@ #include <uapi/linux/iommufd.h> #include <linux/iommufd.h> +#include "io_pagetable.h" #include "iommufd_private.h" #include "iommufd_test.h" @@ -31,6 +32,7 @@ struct iommufd_object_ops { void (*destroy)(struct iommufd_object *obj); }; static 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, @@ -167,6 +169,13 @@ 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 (filp->private_data == &vfio_misc_dev) + ictx->account_mode = IOPT_PAGES_ACCOUNT_MM; + xa_init_flags(&ictx->objects, XA_FLAGS_ALLOC1 | XA_FLAGS_ACCOUNT); ictx->file = filp; filp->private_data = ictx; @@ -392,26 +401,46 @@ 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; ret = misc_register(&iommu_misc_dev); - if (ret) { - pr_err("Failed to register misc device\n"); + if (ret) return ret; - } + if (IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)) { + ret = misc_register(&vfio_misc_dev); + if (ret) + goto err_misc; + } return 0; +err_misc: + misc_deregister(&iommu_misc_dev); + return ret; } static void __exit iommufd_exit(void) { + 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); +#endif MODULE_DESCRIPTION("I/O Address Space Management for passthrough devices"); MODULE_LICENSE("GPL");
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. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommufd/Kconfig | 12 ++++++++++++ drivers/iommu/iommufd/main.c | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 3 deletions(-)