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
