On Fri, 13 Oct 2023 22:20:31 +0100 Joao Martins <joao.m.martins@xxxxxxxxxx> wrote: > On 13/10/2023 21:41, Alex Williamson wrote: > > On Fri, 13 Oct 2023 18:23:09 +0100 > > Joao Martins <joao.m.martins@xxxxxxxxxx> 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 >
