| Message ID | 20230530144821.1557-9-avihaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Add switchover ack capability and VFIO precopy support | expand |
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? 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? Thanks, Alex > 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),
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),
On Sun, 4 Jun 2023 12:33:43 +0300 Avihai Horon <avihaih@nvidia.com> wrote: > 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. If these are valid scenarios, the protocol should support negotiation without requiring an experimental flag to do so. > > 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? I'd keep it as a separate patch with a clearly described dependency on this series so that we can discuss it separately. Thanks, Alex > >> 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), >
On 05/06/2023 17:56, Alex Williamson wrote: > External email: Use caution opening links or attachments > > > On Sun, 4 Jun 2023 12:33:43 +0300 > Avihai Horon <avihaih@nvidia.com> wrote: > >> 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. > If these are valid scenarios, the protocol should support negotiation > without requiring an experimental flag to do so. Thinking again, the two intree drivers we have support precopy and as you said, this is still experimental, so I assume that we can drop this patch in v6. > >>> 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? > I'd keep it as a separate patch with a clearly described dependency on > this series so that we can discuss it separately. Sure, so I will post it separately. 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),
Hello Avihai On 6/6/23 13:59, Avihai Horon wrote: > > On 05/06/2023 17:56, Alex Williamson wrote: >> External email: Use caution opening links or attachments >> >> >> On Sun, 4 Jun 2023 12:33:43 +0300 >> Avihai Horon <avihaih@nvidia.com> wrote: >> >>> 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. >> If these are valid scenarios, the protocol should support negotiation >> without requiring an experimental flag to do so. > > Thinking again, the two intree drivers we have support precopy and as you said, this is still experimental, so I assume that we can drop this patch in v6. No need to resend if there are no other changes. Thanks, C. > >> >>>> 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? >> I'd keep it as a separate patch with a clearly described dependency on >> this series so that we can discuss it separately. > > Sure, so I will post it separately. > > 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), >
On 06/06/2023 16:40, Cédric Le Goater wrote: > External email: Use caution opening links or attachments > > > Hello Avihai > > On 6/6/23 13:59, Avihai Horon wrote: >> >> On 05/06/2023 17:56, Alex Williamson wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> On Sun, 4 Jun 2023 12:33:43 +0300 >>> Avihai Horon <avihaih@nvidia.com> wrote: >>> >>>> 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. >>> If these are valid scenarios, the protocol should support negotiation >>> without requiring an experimental flag to do so. >> >> Thinking again, the two intree drivers we have support precopy and as >> you said, this is still experimental, so I assume that we can drop >> this patch in v6. > > No need to resend if there are no other changes. I think that just the last paragraph of the commit message in patch #9 should be removed. Thanks. > >> >>> >>>>> 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? >>> I'd keep it as a separate patch with a clearly described dependency on >>> this series so that we can discuss it separately. >> >> Sure, so I will post it separately. >> >> 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), >> >
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),