Re: [PATCH RFC 07/19] iommufd/vfio-compat: Dirty tracking IOCTLs compatibility

Joao Martins
2022-04-29T14:27:44Z

On 4/29/22 13:19, Jason Gunthorpe wrote:
> On Thu, Apr 28, 2022 at 10:09:21PM +0100, Joao Martins wrote:
>> Add the correspondent APIs for performing VFIO dirty tracking,
>> particularly VFIO_IOMMU_DIRTY_PAGES ioctl subcmds:
>> * VFIO_IOMMU_DIRTY_PAGES_FLAG_START: Start dirty tracking and allocates
>> the area @dirty_bitmap
>> * VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP: Stop dirty tracking and frees
>> the area @dirty_bitmap
>> * VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP: Fetch dirty bitmap while dirty
>> tracking is active.
>>
>> Advertise the VFIO_IOMMU_TYPE1_INFO_CAP_MIGRATION
>> whereas it gets set the domain configured page size the same as
>> iopt::iova_alignment and maximum dirty bitmap size same
>> as VFIO. Compared to VFIO type1 iommu, the perpectual dirtying is
>> not implemented and userspace gets -EOPNOTSUPP which is handled by
>> today's userspace.
>>
>> Move iommufd_get_pagesizes() definition prior to unmap for
>> iommufd_vfio_unmap_dma() dirty support to validate the user bitmap page
>> size against IOPT pagesize.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> drivers/iommu/iommufd/vfio_compat.c | 221 ++++++++++++++++++++++++++--
>> 1 file changed, 209 insertions(+), 12 deletions(-)
>
> I think I would probably not do this patch, it has behavior that is
> quite different from the current vfio - ie the interaction with the
> mdevs, and I don't intend to fix that.

I'll drop this, until I hear otherwise.

I wasn't sure what people leaning towards to and keeping perpectual-dirty
stuff didn't feel right for a new UAPI either.

> So, with this patch and a mdev
> then vfio_compat will return all-not-dirty but current vfio will
> return all-dirty - and that is significant enough to break qemu.
>
Ack

> We've made a qemu patch to allow qemu to be happy if dirty tracking is
> not supported in the vfio container for migration, which is part of
> the v2 enablement series. That seems like the better direction.
>
So in my auditing/testing, the listener callbacks are called but the dirty ioctls
return an error at start, and bails out early on sync. I suppose migration
won't really work, as no pages aren't set and what not but it could
cope with no-dirty-tracking support. So by 'making qemu happy' is this mainly
cleaning out the constant error messages you get and not even attempt
migration by introducing a migration blocker early on ... should it fetch
no migration capability?

> I can see why this is useful to test with the current qemu however.

Yes, it is indeed useful for testing.

I am wondering if we can still emulate that in userspace, given that the expectation
from each GET_BITMAP call is to get all dirties, likewise for type1 unmap dirty. Unless
I am missed something obvious.