Re: [PATCH V1 vfio 7/7] vfio/virtio: Enable live migration once VIRTIO_PCI was configured

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

On Mon, 11 Nov 2024 15:30:47 +0000
Joao Martins <joao.m.martins@oracle.com> wrote:

> On 11/11/2024 14:17, Yishai Hadas wrote:
> > On 11/11/2024 12:32, Joao Martins wrote:  
> >>>>>            depends on VIRTIO_PCI
> >>>>>            select VFIO_PCI_CORE
> >>>>>            select IOMMUFD_DRIVER  
> >>>>
> >>>> IIUC, this is not a dependency, the device will just lack dirty page
> >>>> tracking with either the type1 backend or when using iommufd when the
> >>>> IOMMU hardware doesn't have dirty page tracking, therefore all VM
> >>>> memory is perpetually dirty.  Do I have that right?  
> >>>
> >>> IOMMUFD_DRIVER is selected to utilize the dirty tracking functionality of IOMMU.
> >>> Therefore, this is a select option rather than a dependency, similar to how the
> >>> pds and mlx5 VFIO drivers handle it in their Kconfig files.
> >>>  
> >>
> >> Yishai, I think Alex is right here.
> >>
> >> 'select IOMMUFD_DRIVER' is more for VF dirty trackers where it uses the same
> >> helpers as IOMMUFD does for dirty tracking. But it's definitely not signaling
> >> intent for 'IOMMU dirty tracking' but rather 'VF dirty tracking'  
> > 
> > I see, please see below.
> >   
> I should have said that in the context of VFIO drivers selecting it, not broadly
> across all drivers that select it.
> 
> >>
> >> If you want to tie in to IOMMU dirty tracking you probably want to do:
> >>
> >>     'select IOMMUFD'
> >>  
> > 
> > Looking at the below Kconfig(s) for AMD/INTEL_IOMMU [1], we can see that if
> > IOMMUFD is set IOMMFD_DRIVER is selected.
> >   
> Correct.
> 
> > From that we can assume that to have 'IOMMU dirty tracking' the IOMMFD_DRIVER is
> > finally needed/selected, right ?
> >   
> 
> Right, if you have CONFIG_IOMMUFD then the IOMMU will in the end auto-select
> IOMMU_DRIVER. But standalone at best you can assume that 'something does dirty
> tracking'. The context (i.e. who selects it) is what tells you if it's VF or IOMMU.
> 
> In your example above, that option is meant to be selected by *a* dirty tracker,
> and it's because AMD/Intel was selecting that you would have IOMMU dirty
> tracking. The option essentially selects IOVA bitmaps helpers (zerocopy scheme
> to set bits in a bitmap) which is both used by VF dirty trackers and IOMMU dirty
> trackers. Originally this started in VFIO and later got moved into IOMMUFD which
> is why we have this kconfig to allow independent use.

Yeah, I agree.  IOMMUFD_DRIVER is only configuring in iova_bitmap
support independent of IOMMUFD, which mlx5 requires, but this does not.

> > So you are saying that it's redundant in the vfio/virtio driver as it will be
> > selected down the road once needed ?
> >   
> Right.
> 
> Of course, it will always depend on user enabling the right kconfigs and such.
> But that would be no different than the other drivers than don't support VF
> dirty tracking.
> 
> > [1]
> > https://elixir.bootlin.com/linux/v6.12-rc6/source/drivers/iommu/intel/Kconfig#L17
> > https://elixir.bootlin.com/linux/v6.12-rc6/source/drivers/iommu/amd/Kconfig#L16
> >   
> >> But that is a big hammer, as you also need the VFIO_DEVICE_CDEV kconfig selected
> >> as well and probably more.  
> > 
> > I agree, we didn't plan to add those dependencies.
> >   
> >>
> >> Perhaps best to do like qat/hisilicon drivers and letting the user optionally
> >> pick it. Migration is anyways disabled when using type1 (unless you force it,
> >> and it then it does the perpectual dirty trick).  

Yes, at least for QEMU, unless the user forces the device to support
migration we'll add a migration blocker rather than use the perpetually
dirty trick.  Ultimately the support depends on the underlying IOMMU
capabilities where we're running anyway, so it makes sense to me to
leave this to the user rather than trying to force a kernel config that
can support IOMMU dirty page tracking.  Thanks,

Alex