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

[Alex Williamson <alex.williamson@redhat.com>]

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
>