On 18/07/2024 08:20, Duan, Zhenzhong wrote: > > >> -----Original Message----- >> From: Joao Martins <joao.m.mart...@oracle.com> >> Subject: Re: [PATCH v4 11/12] vfio/migration: Don't block migration device >> dirty tracking is unsupported >> >> On 17/07/2024 03:38, Duan, Zhenzhong wrote: >>> >>> >>>> -----Original Message----- >>>> From: Joao Martins <joao.m.mart...@oracle.com> >>>> Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device >> dirty >>>> tracking is unsupported >>>> >>>> By default VFIO migration is set to auto, which will support live >>>> migration if the migration capability is set *and* also dirty page >>>> tracking is supported. >>>> >>>> For testing purposes one can force enable without dirty page tracking >>>> via enable-migration=on, but that option is generally left for testing >>>> purposes. >>>> >>>> So starting with IOMMU dirty tracking it can use to acomodate the lack of >>>> VF dirty page tracking allowing us to minimize the VF requirements for >>>> migration and thus enabling migration by default for those too. >>>> >>>> Signed-off-by: Joao Martins <joao.m.mart...@oracle.com> >>>> --- >>>> hw/vfio/migration.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c >>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644 >>>> --- a/hw/vfio/migration.c >>>> +++ b/hw/vfio/migration.c >>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice >>>> *vbasedev, Error **errp) >>>> return !vfio_block_migration(vbasedev, err, errp); >>>> } >>>> >>>> - if (!vbasedev->dirty_pages_supported) { >>>> + if (!vbasedev->dirty_pages_supported && >>>> + !vbasedev->bcontainer->dirty_pages_supported) { >>>> if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) { >>>> error_setg(&err, >>>> "%s: VFIO device doesn't support device dirty >>>> tracking", >>> >>> I'm not sure if this message needs to be updated, " VFIO device doesn't >> support device and IOMMU dirty tracking" >>> >>> Same for the below: >>> >>> warn_report("%s: VFIO device doesn't support device dirty tracking" >> >> >> Ah yes, good catch. Additionally I think I should check device hwpt rather >> than >> container::dirty_pages_supported i.e. >> >> if (!vbasedev->dirty_pages_supported && >> (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt))) >> >> This makes sure that migration is blocked with more accuracy > > Yes, this is better. Looks bcontainer->dirty_pages_supported is not as > accurate as in legacy VFIO days. >
Heh, That's just because legacy is always marking true (and marking anything as dirty) regardless of what the hardware does :)