Re: [PATCH v10 03/12] vfio/migration: Allow migration without VFIO IOMMU dirty tracking support

[Alex Williamson]

On Wed, 15 Feb 2023 19:04:33 +0100
Juan Quintela <quintela@redhat.com> wrote:

> Avihai Horon <avihaih@nvidia.com> wrote:
> > On 15/02/2023 14:43, Juan Quintela wrote:  
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> Avihai Horon <avihaih@nvidia.com> wrote:  
> >>> Currently, if IOMMU of a VFIO container doesn't support dirty page
> >>> tracking, migration is blocked. This is because a DMA-able VFIO device
> >>> can dirty RAM pages without updating QEMU about it, thus breaking the
> >>> migration.
> >>>
> >>> However, this doesn't mean that migration can't be done at all.
> >>> In such case, allow migration and let QEMU VFIO code mark all pages
> >>> dirty.
> >>>
> >>> This guarantees that all pages that might have gotten dirty are reported
> >>> back, and thus guarantees a valid migration even without VFIO IOMMU
> >>> dirty tracking support.
> >>>
> >>> The motivation for this patch is the introduction of iommufd [1].
> >>> iommufd can directly implement the /dev/vfio/vfio container IOCTLs by
> >>> mapping them into its internal ops, allowing the usage of these IOCTLs
> >>> over iommufd. However, VFIO IOMMU dirty tracking is not supported by
> >>> this VFIO compatibility API.
> >>>
> >>> This patch will allow migration by hosts that use the VFIO compatibility
> >>> API and prevent migration regressions caused by the lack of VFIO IOMMU
> >>> dirty tracking support.
> >>>
> >>> [1]
> >>> https://lore.kernel.org/kvm/0-v6-a196d26f289e+11787-iommufd_jgg@nvidia.com/
> >>>
> >>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
> >>> Reviewed-by: Cédric Le Goater <clg@redhat.com>  
> >> Reviewed-by: Juan Quintela <quintela@redhat.com>
> >>
> >> I know why you are doing this.
> >>
> >> But I think this should print a warning, error, somewhere.  
> >
> > IMHO, I'm not sure it's really necessary.
> >
> > To enable VFIO migration the user must explicitly set x-enable-migration=on.
> > I guess in this case the user is well aware of the dirty tracking
> > capabilities the VFIO device has or doesn't have.
> > So I don't think adding this error/warning will help much.  
> 
> Oops.  I missed that bit.
> I retire my objection.

Keep it in mind though as hopefully we'll be making vfio migration
non-experimental soon and enabled by default where devices support it.
We'll need to consider whether we want to keep "dumb" dirty tracking,
or even any form of dirty tracking in the type1 uAPI, under an
experimental opt-in.  Thanks,

Alex