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

[Joao Martins]

On 18/07/2024 08:20, Duan, Zhenzhong wrote:
> 
> 
>> -----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.
> 

Heh, That's just because legacy is always marking true (and marking anything as
dirty) regardless of what the hardware does :)