RE: [PATCH v4 11/12] vfio/migration: Don't block migration device dirty tracking is unsupported

[Duan, Zhenzhong]

>-----Original Message-----
>From: Joao Martins <joao.m.martins@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.martins@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.martins@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.

Thanks
Zhenzhong