Re: [PATCH v5 8/9] vfio/migration: Add x-allow-pre-copy VFIO device property

[Avihai Horon <avihaih@nvidia.com>]

On 01/06/2023 23:22, Alex Williamson wrote:
> External email: Use caution opening links or attachments
>
>
> On Tue, 30 May 2023 17:48:20 +0300
> Avihai Horon <avihaih@nvidia.com> wrote:
>
>> Add a new VFIO device property x-allow-pre-copy to keep migration
>> compatibility to/from older QEMU versions that don't have VFIO pre-copy
>> support.
> This doesn't make sense to me, vfio migration is not currently
> supported, it can only be enabled via an experimental flag.  AFAIK we
> have no obligation to maintain migration compatibility against
> experimental features.  Is there any other reason we need a flag to
> disable pre-copy?

This could give flexibility to do migration between hosts without 
matching VFIO device kernel drivers. E.g., source driver doesn't have 
precopy support and dest driver has or vice versa.

>
> OTOH, should this series finally remove the experimental migration
> flag?  Do we require Joao's vIOMMU support to finally make it
> supportable?  Is there something else?

I think that after precopy is accepted we can remove the experimental 
flag, as we'll have the major parts of VFIO migration upstream.
After that we will still need to add Joao's vIOMMU support and P2P support.
Do you want me to add a patch to this series that makes VFIO migration 
non-experimental?

Thanks.

>> Signed-off-by: Avihai Horon <avihaih@nvidia.com>
>> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>> ---
>>   include/hw/vfio/vfio-common.h | 1 +
>>   hw/core/machine.c             | 1 +
>>   hw/vfio/migration.c           | 3 ++-
>>   hw/vfio/pci.c                 | 2 ++
>>   4 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 1db901c194..a53ecbe2e0 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -146,6 +146,7 @@ typedef struct VFIODevice {
>>       VFIOMigration *migration;
>>       Error *migration_blocker;
>>       OnOffAuto pre_copy_dirty_page_tracking;
>> +    bool allow_pre_copy;
>>       bool dirty_pages_supported;
>>       bool dirty_tracking;
>>   } VFIODevice;
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index 1000406211..64ac3fe38e 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -41,6 +41,7 @@
>>
>>   GlobalProperty hw_compat_8_0[] = {
>>       { "migration", "multifd-flush-after-each-section", "on"},
>> +    { "vfio-pci", "x-allow-pre-copy", "false" },
>>   };
>>   const size_t hw_compat_8_0_len = G_N_ELEMENTS(hw_compat_8_0);
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index d8f6a22ae1..cb6923ed3f 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -323,7 +323,8 @@ static bool vfio_precopy_supported(VFIODevice *vbasedev)
>>   {
>>       VFIOMigration *migration = vbasedev->migration;
>>
>> -    return migration->mig_flags & VFIO_MIGRATION_PRE_COPY;
>> +    return vbasedev->allow_pre_copy &&
>> +           migration->mig_flags & VFIO_MIGRATION_PRE_COPY;
>>   }
>>
>>   /* ---------------------------------------------------------------------- */
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 73874a94de..c69813af7f 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -3335,6 +3335,8 @@ static Property vfio_pci_dev_properties[] = {
>>       DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", VFIOPCIDevice,
>>                               vbasedev.pre_copy_dirty_page_tracking,
>>                               ON_OFF_AUTO_ON),
>> +    DEFINE_PROP_BOOL("x-allow-pre-copy", VFIOPCIDevice,
>> +                     vbasedev.allow_pre_copy, true),
>>       DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
>>                               display, ON_OFF_AUTO_OFF),
>>       DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),