Re: [PATCH RFCv2 7/8] vfio/migration: Don't block migration device dirty tracking is unsupported

[Joao Martins <joao.m.martins@oracle.com>]

On 19/02/2024 10:12, Avihai Horon wrote:
> Hi Joao,
> 
> On 12/02/2024 15:56, Joao Martins wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> 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.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>   hw/vfio/iommufd.c        | 3 +--
>>   hw/vfio/migration.c      | 4 +++-
>>   include/sysemu/iommufd.h | 1 +
>>   3 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 697d40841d7f..78d8f4391b68 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -275,8 +275,7 @@ static int iommufd_cdev_detach_ioas_hwpt(VFIODevice
>> *vbasedev, Error **errp)
>>       return ret;
>>   }
>>
>> -static bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev,
>> -                                          Error **errp)
>> +bool iommufd_dirty_pages_supported(IOMMUFDDevice *iommufd_dev, Error **errp)
>>   {
>>       uint64_t caps;
>>       int r;
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 70e6b1a709f9..674e76b3f3df 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -938,7 +938,9 @@ 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->iommufd_dev.iommufd &&
> 
> Shouldn't we check the type of base_hdev instead?
> 

This was just me trying to be less tied to Zhenzhong series, but yes

>> +         !iommufd_dirty_pages_supported(&vbasedev->iommufd_dev, &err))) {
> 
> Maybe we can store IOMMUFD DPT support in iommufd_dev and use it instead of
> querying it here?

It's a good idea, and originally I had the ::capabilities stored in the
iommufd_dev and I was mainly checking this there.

I could fetch the capabilities after we get an idev and then this just tests
against HWCAP and this avoids having to call GET_HWINFO all the time