Re: [PATCH v3 02/19] vfio: Move iova_bitmap into iommu core

Alex Williamson
2023-10-13T21:52:29Z

On Fri, 13 Oct 2023 22:20:31 +0100
Joao Martins <joao.m.martins@oracle.com> wrote:

> On 13/10/2023 21:41, Alex Williamson wrote:
> > On Fri, 13 Oct 2023 18:23:09 +0100
> > Joao Martins <joao.m.martins@oracle.com> wrote:
> >
> >> On 13/10/2023 18:16, Jason Gunthorpe wrote:
> >>> On Fri, Oct 13, 2023 at 06:10:04PM +0100, Joao Martins wrote:
> >>>> On 13/10/2023 17:00, Joao Martins wrote:
> >>>>> On 13/10/2023 16:48, Jason Gunthorpe wrote:
> >>>> But if it's exists an IOMMUFD_DRIVER kconfig, then VFIO_CONTAINER can instead
> >>>> select the IOMMUFD_DRIVER alone so long as CONFIG_IOMMUFD isn't required? I am
> >>>> essentially talking about:
> >>>
> >>> Not VFIO_CONTAINER, the dirty tracking code is in vfio_main:
> >>>
> >>> vfio_main.c:#include <linux/iova_bitmap.h>
> >>> vfio_main.c:static int vfio_device_log_read_and_clear(struct iova_bitmap *iter,
> >>> vfio_main.c: struct iova_bitmap *iter;
> >>> vfio_main.c: iter = iova_bitmap_alloc(report.iova, report.length,
> >>> vfio_main.c: ret = iova_bitmap_for_each(iter, device,
> >>> vfio_main.c: iova_bitmap_free(iter);
> >>>
> >>> And in various vfio device drivers.
> >>>
> >>> So the various drivers can select IOMMUFD_DRIVER
> >>>
> >>
> >> It isn't so much that type1 requires IOMMUFD, but more that it is used together
> >> with the core code that allows the vfio drivers to do migration. So the concern
> >> is if we make VFIO core depend on IOMMU that we prevent
> >> VFIO_CONTAINER/VFIO_GROUP to not be selected. My kconfig read was that we either
> >> select VFIO_GROUP or VFIO_DEVICE_CDEV but not both
> >
> > That's not true. We can have both. In fact we rely on having both to
> > support a smooth transition to the cdev interface. Thanks,
>
> On a triple look, mixed defaults[0] vs manual config: having IOMMUFD=y|m today
> it won't select VFIO_CONTAINER, nobody stops one from actually selecting it
> both. Unless I missed something

Oh! I misunderstood your comment, you're referring to default
selections rather than possible selections. So yes, if VFIO depends on
IOMMUFD then suddenly our default configs shift to IOMMUFD/CDEV rather
than legacy CONTAINER/GROUP. So perhaps if VFIO selects IOMMUFD, that's
not exactly harmless currently.

I think Jason is describing this would eventually be in a built-in
portion of IOMMUFD, but I think currently that built-in portion is
IOMMU. So until we have this IOMMUFD_DRIVER that enables that built-in
portion, it seems unnecessarily disruptive to make VFIO select IOMMUFD
to get this iova bitmap support. Thanks,

Alex

> [0] Ref:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/vfio/Kconfig
>
> menuconfig VFIO
> [...]
> select VFIO_GROUP if SPAPR_TCE_IOMMU || IOMMUFD=n
> select VFIO_DEVICE_CDEV if !VFIO_GROUP
> select VFIO_CONTAINER if IOMMUFD=n
> [...]
>
> if VFIO
> config VFIO_DEVICE_CDEV
> [...]
> depends on IOMMUFD && !SPAPR_TCE_IOMMU
> default !VFIO_GROUP
> [...]
> config VFIO_GROUP
> default y
> [...]
> config VFIO_CONTAINER
> [...]
> select VFIO_IOMMU_TYPE1 if MMU && (X86 || S390 || ARM || ARM64)
> depends on VFIO_GROUP
> default y
>