[Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker

Paolo Bonzini posted 1 patch 5 years, 5 months ago
Test asan passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181119182330.17147-1-pbonzini@redhat.com
target/i386/kvm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
[Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Paolo Bonzini 5 years, 5 months ago
Nested VMX does not support live migration yet.  Add a blocker
until that is worked out.

Nested SVM only does not support it, but unfortunately it is
enabled by default for -cpu host so we cannot really disable it.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/kvm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index f524e7d929..27dcca5365 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -854,6 +854,7 @@ static int hyperv_init_vcpu(X86CPU *cpu)
 }
 
 static Error *invtsc_mig_blocker;
+static Error *vmx_mig_blocker;
 
 #define KVM_MAX_CPUID_ENTRIES  100
 
@@ -1246,6 +1247,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
                                   !!(c->ecx & CPUID_EXT_SMX);
     }
 
+    if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) {
+        error_setg(&vmx_mig_blocker,
+                   "Nested VMX virtualization does not support live migration yet");
+        r = migrate_add_blocker(vmx_mig_blocker, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_free(vmx_mig_blocker);
+            return r;
+        }
+    }
+
     if (env->mcg_cap & MCG_LMCE_P) {
         has_msr_mcg_ext_ctl = has_msr_feature_control = true;
     }
-- 
2.19.1


Re: [Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Dr. David Alan Gilbert 5 years, 5 months ago
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Nested VMX does not support live migration yet.  Add a blocker
> until that is worked out.
> 
> Nested SVM only does not support it, but unfortunately it is
> enabled by default for -cpu host so we cannot really disable it.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

So I'm OK with this, but it does need a release note warning whenever it
goes in, because it'll surprise those who've already enabled nesting
but don't use it on all their VMs.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> ---
>  target/i386/kvm.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index f524e7d929..27dcca5365 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -854,6 +854,7 @@ static int hyperv_init_vcpu(X86CPU *cpu)
>  }
>  
>  static Error *invtsc_mig_blocker;
> +static Error *vmx_mig_blocker;
>  
>  #define KVM_MAX_CPUID_ENTRIES  100
>  
> @@ -1246,6 +1247,17 @@ int kvm_arch_init_vcpu(CPUState *cs)
>                                    !!(c->ecx & CPUID_EXT_SMX);
>      }
>  
> +    if ((env->features[FEAT_1_ECX] & CPUID_EXT_VMX) && !vmx_mig_blocker) {
> +        error_setg(&vmx_mig_blocker,
> +                   "Nested VMX virtualization does not support live migration yet");
> +        r = migrate_add_blocker(vmx_mig_blocker, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +            error_free(vmx_mig_blocker);
> +            return r;
> +        }
> +    }
> +
>      if (env->mcg_cap & MCG_LMCE_P) {
>          has_msr_mcg_ext_ctl = has_msr_feature_control = true;
>      }
> -- 
> 2.19.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Paolo Bonzini 5 years, 5 months ago
On 20/11/18 12:44, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> Nested VMX does not support live migration yet.  Add a blocker
>> until that is worked out.
>>
>> Nested SVM only does not support it, but unfortunately it is
>> enabled by default for -cpu host so we cannot really disable it.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> So I'm OK with this, but it does need a release note warning whenever it
> goes in, because it'll surprise those who've already enabled nesting
> but don't use it on all their VMs.

Yes, of course.

Paolo


Re: [Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Cole Robinson 5 years ago
On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>> Nested VMX does not support live migration yet.  Add a blocker
>> until that is worked out.
>>
>> Nested SVM only does not support it, but unfortunately it is
>> enabled by default for -cpu host so we cannot really disable it.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> So I'm OK with this, but it does need a release note warning whenever it
> goes in, because it'll surprise those who've already enabled nesting
> but don't use it on all their VMs.
> 

We are hitting this in Fedora 30. Now that nested VMX is enabled by
default at the kernel level, and virt-manager/boxes will use the
equivalent of -cpu host by default, libvirt managedsave (migrate to
file) and virt-manager snapshots (savevm) are rejected for default
created VMs on intel. That's quite unfortunate.

Any ideas on how to resolve this?

FYI it's the root of this bug though there's libvirt issues mixed in:
https://bugzilla.redhat.com/show_bug.cgi?id=1697997

Thanks,
Cole

Re: [Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Paolo Bonzini 5 years ago
On 10/04/19 20:26, Cole Robinson wrote:
> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>> Nested VMX does not support live migration yet.  Add a blocker
>>> until that is worked out.
>>>
>>> Nested SVM only does not support it, but unfortunately it is
>>> enabled by default for -cpu host so we cannot really disable it.
>>>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>
>> So I'm OK with this, but it does need a release note warning whenever it
>> goes in, because it'll surprise those who've already enabled nesting
>> but don't use it on all their VMs.
>>
> 
> We are hitting this in Fedora 30. Now that nested VMX is enabled by
> default at the kernel level, and virt-manager/boxes will use the
> equivalent of -cpu host by default, libvirt managedsave (migrate to
> file) and virt-manager snapshots (savevm) are rejected for default
> created VMs on intel. That's quite unfortunate.
> 
> Any ideas on how to resolve this?

I think the simplest solution is just to finish implementation of nested
VMX live migration and backport it to Fedora 30.

Paolo


Re: [Qemu-devel] [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Cole Robinson 5 years ago
On 4/12/19 3:47 AM, Paolo Bonzini wrote:
> On 10/04/19 20:26, Cole Robinson wrote:
>> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
>>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>>> Nested VMX does not support live migration yet.  Add a blocker
>>>> until that is worked out.
>>>>
>>>> Nested SVM only does not support it, but unfortunately it is
>>>> enabled by default for -cpu host so we cannot really disable it.
>>>>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> So I'm OK with this, but it does need a release note warning whenever it
>>> goes in, because it'll surprise those who've already enabled nesting
>>> but don't use it on all their VMs.
>>>
>>
>> We are hitting this in Fedora 30. Now that nested VMX is enabled by
>> default at the kernel level, and virt-manager/boxes will use the
>> equivalent of -cpu host by default, libvirt managedsave (migrate to
>> file) and virt-manager snapshots (savevm) are rejected for default
>> created VMs on intel. That's quite unfortunate.
>>
>> Any ideas on how to resolve this?
> 
> I think the simplest solution is just to finish implementation of nested
> VMX live migration and backport it to Fedora 30.
> 

That would simplify things :) Any guess on the timeframe? This is kernel
work I presume?

If changes aren't landing in the near term I think we should disable
nested VMX by default in Fedora, maybe just with modules.d/kvm.conf
override. (Or revert this patch downstream, but I presume that's not a
good idea).

The alternative of just letting it sit is going to generate a lot of
complaints I suspect. And the only solutions will be 1) disable nested
VMx for your whole machine and reboot, or 2) run this virt-xml command
to disable VMX in your domain config... and probably forget that it's
there and then a year later when this is all sorted out file a bug
asking why nested virt isn't working for this one VM ;)

I guess #2 might not be avoidable anyways for the amount of people that
have already opted into nested VMX

Thanks,
Cole

[Qemu-devel] R: Re: [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Paolo Bonzini 5 years ago
----- Cole Robinson <crobinso@redhat.com> ha scritto:
> On 4/12/19 3:47 AM, Paolo Bonzini wrote:
> > On 10/04/19 20:26, Cole Robinson wrote:
> >> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
> >>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
> >>>> Nested VMX does not support live migration yet.  Add a blocker
> >>>> until that is worked out.
> >>>>
> >>>> Nested SVM only does not support it, but unfortunately it is
> >>>> enabled by default for -cpu host so we cannot really disable it.
> >>>>
> >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >>>
> >>> So I'm OK with this, but it does need a release note warning whenever it
> >>> goes in, because it'll surprise those who've already enabled nesting
> >>> but don't use it on all their VMs.
> >>>
> >>
> >> We are hitting this in Fedora 30. Now that nested VMX is enabled by
> >> default at the kernel level, and virt-manager/boxes will use the
> >> equivalent of -cpu host by default, libvirt managedsave (migrate to
> >> file) and virt-manager snapshots (savevm) are rejected for default
> >> created VMs on intel. That's quite unfortunate.
> >>
> >> Any ideas on how to resolve this?
> > 
> > I think the simplest solution is just to finish implementation of nested
> > VMX live migration and backport it to Fedora 30.
> > 
> 
> That would simplify things :) Any guess on the timeframe? This is kernel
> work I presume?

No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch.

Paolo

> 
> If changes aren't landing in the near term I think we should disable
> nested VMX by default in Fedora, maybe just with modules.d/kvm.conf
> override. (Or revert this patch downstream, but I presume that's not a
> good idea).
> 
> The alternative of just letting it sit is going to generate a lot of
> complaints I suspect. And the only solutions will be 1) disable nested
> VMx for your whole machine and reboot, or 2) run this virt-xml command
> to disable VMX in your domain config... and probably forget that it's
> there and then a year later when this is all sorted out file a bug
> asking why nested virt isn't working for this one VM ;)
> 
> I guess #2 might not be avoidable anyways for the amount of people that
> have already opted into nested VMX
> 
> Thanks,
> Cole


Re: [Qemu-devel] R: Re: [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Dr. David Alan Gilbert 5 years ago
* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> ----- Cole Robinson <crobinso@redhat.com> ha scritto:
> > On 4/12/19 3:47 AM, Paolo Bonzini wrote:
> > > On 10/04/19 20:26, Cole Robinson wrote:
> > >> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
> > >>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
> > >>>> Nested VMX does not support live migration yet.  Add a blocker
> > >>>> until that is worked out.
> > >>>>
> > >>>> Nested SVM only does not support it, but unfortunately it is
> > >>>> enabled by default for -cpu host so we cannot really disable it.
> > >>>>
> > >>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > >>>
> > >>> So I'm OK with this, but it does need a release note warning whenever it
> > >>> goes in, because it'll surprise those who've already enabled nesting
> > >>> but don't use it on all their VMs.
> > >>>
> > >>
> > >> We are hitting this in Fedora 30. Now that nested VMX is enabled by
> > >> default at the kernel level, and virt-manager/boxes will use the
> > >> equivalent of -cpu host by default, libvirt managedsave (migrate to
> > >> file) and virt-manager snapshots (savevm) are rejected for default
> > >> created VMs on intel. That's quite unfortunate.
> > >>
> > >> Any ideas on how to resolve this?
> > > 
> > > I think the simplest solution is just to finish implementation of nested
> > > VMX live migration and backport it to Fedora 30.
> > > 
> > 
> > That would simplify things :) Any guess on the timeframe? This is kernel
> > work I presume?
> 
> No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch.

With a new-enough kernel, how hard is it to detect that this actual
migration is safe?

Dave

> Paolo
> 
> > 
> > If changes aren't landing in the near term I think we should disable
> > nested VMX by default in Fedora, maybe just with modules.d/kvm.conf
> > override. (Or revert this patch downstream, but I presume that's not a
> > good idea).
> > 
> > The alternative of just letting it sit is going to generate a lot of
> > complaints I suspect. And the only solutions will be 1) disable nested
> > VMx for your whole machine and reboot, or 2) run this virt-xml command
> > to disable VMX in your domain config... and probably forget that it's
> > there and then a year later when this is all sorted out file a bug
> > asking why nested virt isn't working for this one VM ;)
> > 
> > I guess #2 might not be avoidable anyways for the amount of people that
> > have already opted into nested VMX
> > 
> > Thanks,
> > Cole
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] R: Re: [PATCH v2] target/i386: kvm: add VMX migration blocker
Posted by Paolo Bonzini 5 years ago
On 15/04/19 13:22, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>
>> ----- Cole Robinson <crobinso@redhat.com> ha scritto:
>>> On 4/12/19 3:47 AM, Paolo Bonzini wrote:
>>>> On 10/04/19 20:26, Cole Robinson wrote:
>>>>> On 11/20/18 6:44 AM, Dr. David Alan Gilbert wrote:
>>>>>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>>>>>> Nested VMX does not support live migration yet.  Add a blocker
>>>>>>> until that is worked out.
>>>>>>>
>>>>>>> Nested SVM only does not support it, but unfortunately it is
>>>>>>> enabled by default for -cpu host so we cannot really disable it.
>>>>>>>
>>>>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>>>>>
>>>>>> So I'm OK with this, but it does need a release note warning whenever it
>>>>>> goes in, because it'll surprise those who've already enabled nesting
>>>>>> but don't use it on all their VMs.
>>>>>>
>>>>>
>>>>> We are hitting this in Fedora 30. Now that nested VMX is enabled by
>>>>> default at the kernel level, and virt-manager/boxes will use the
>>>>> equivalent of -cpu host by default, libvirt managedsave (migrate to
>>>>> file) and virt-manager snapshots (savevm) are rejected for default
>>>>> created VMs on intel. That's quite unfortunate.
>>>>>
>>>>> Any ideas on how to resolve this?
>>>>
>>>> I think the simplest solution is just to finish implementation of nested
>>>> VMX live migration and backport it to Fedora 30.
>>>>
>>>
>>> That would simplify things :) Any guess on the timeframe? This is kernel
>>> work I presume?
>>
>> No, the kernel part is already in. As a contingency plan, you could just revert this QEMU patch.
> 
> With a new-enough kernel, how hard is it to detect that this actual
> migration is safe?

Not hard, but it would fail whenever the KVM kernel module is loaded, so
pretty much for all Linux guests.

Paolo