Re: [PATCH v13 22/22] docs: vfio: Add vfio device cdev description

Jason Gunthorpe
2023-06-27T16:12:22Z

On Tue, Jun 27, 2023 at 08:54:33AM +0000, Liu, Yi L wrote:
> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Thursday, June 22, 2023 5:54 AM
> >
> > On Fri, 16 Jun 2023 02:39:46 -0700
> > Yi Liu <yi.l.liu@intel.com> wrote:
>
> > > +VFIO device cdev doesn't rely on VFIO group/container/iommu drivers.
> > > +Hence those modules can be fully compiled out in an environment
> > > +where no legacy VFIO application exists.
> > > +
> > > +So far SPAPR does not support IOMMUFD yet. So it cannot support device
> > > +cdev either.
> >
> > Why isn´t this enforced via Kconfig? At the vfio level we could simply
> > add the following in patch 17/:
> >
> > config VFIO_DEVICE_CDEV
> > bool "Support for the VFIO cdev /dev/vfio/devices/vfioX"
> > depends on IOMMUFD && !SPAPR_TCE_IOMMU
> > ^^^^^^^^^^^^^^^^^^^
> >
> > Or if Jason wants, IOMMUFD could depend on !SPAPR_TCE_IOMMU for now and
> > the existing Kconfig options would exclude it. If we know it doesn't
> > work, let's not put the burden on the user to figure that out. A
> > follow-up patch for this would be fine if there's no other reason to
> > respin the series.
>
> @Jason,
> How about your opinion? Seems reasonable to make IOMMUFD
> depend on !SPAPR_TCE_IOMMU. Is it?

The right kconfig would be to list all the iommu drivers that can
support iommufd and allow it to be selected if any of them are
enabled.

This seems too complex to bother with, so I like Alex's version above..

> > Otherwise the series is looking pretty good to me. It still requires
> > some reviews/acks in the iommufd space and it would be good to see more
> > reviews for the remainder given the amount of collaboration here.
> >
> > I'm out for the rest of the week, but I'll leave open accepting this
> > and the hot-reset series next week for the merge window. Thanks,
>
> @Alex,
> Given Jason's remarks on cdev v12, I've already got a new version as below.
> I can post it once the above kconfig open is closed.

I think we don't need to bend the rules, Linus would not be happy to
see 30 major patches that never hit linux-next at all.

I'm happy if we put it on a branch at RC1 and merge it to the vfio &
iommufd trees, it is functionally the same outcome in the same time
frame.

Jason