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

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Oct 13, 2023 at 06:23:09PM +0100, Joao Martins 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:
> >>>> On Sat, Sep 23, 2023 at 02:24:54AM +0100, Joao Martins wrote:
> >>>>> Both VFIO and IOMMUFD will need iova bitmap for storing dirties and walking
> >>>>> the user bitmaps, so move to the common dependency into IOMMU core. IOMMUFD
> >>>>> can't exactly host it given that VFIO dirty tracking can be used without
> >>>>> IOMMUFD.
> >>>>
> >>>> Hum, this seems strange. Why not just make those VFIO drivers depends
> >>>> on iommufd? That seems harmless to me.
> >>>>
> >>>
> >>> IF you and Alex are OK with it then I can move to IOMMUFD.
> >>>
> >>>> However, I think the real issue is that iommu drivers need to use this
> >>>> API too for their part?
> >>>>
> >>>
> >>> Exactly.
> >>>
> >>
> >> My other concern into moving to IOMMUFD instead of core was VFIO_IOMMU_TYPE1,
> >> and if we always make it depend on IOMMUFD then we can't have what is today
> >> something supported because of VFIO_IOMMU_TYPE1 stuff with migration drivers
> >> (i.e. vfio-iommu-type1 with the live migration stuff).
> > 
> > I plan to remove the live migration stuff from vfio-iommu-type1, it is
> > all dead code now.
> > 
> 
> I wasn't referring to the type1 dirty tracking stuff -- I was referring the
> stuff related to vfio devices, used *together* with type1 (for DMA
> map/unmap).

Ah, well, I guess that is true

> >> 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

Doing it as I said is still the right thing.

If someone has turned on one of the drivers that actually implements
dirty tracking it will turn on IOMMUFD_DRIVER and that will cause the
supporting core code to compile in the support functions.

Jason