Re: [Intel-gfx] [PATCH 10/10] iommufd: Allow iommufd to supply /dev/vfio/vfio

Jason Gunthorpe
2022-11-07T13:57:50Z

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