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

Joao Martins
2023-10-13T17:23:53Z

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

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

> And the core code should just gain a
>
> if (!IS_SUPPORTED(CONFIG_IOMMUFD_DRIVER))
> return -EOPNOTSUPP
>
> On the two functions grep found above so the compiler eliminates all
> the symbols. No kconfig needed.
>
> Jason