Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 10.07.19 18:34, Paolo Bonzini wrote:
> On 10/07/19 18:08, Jan Kiszka wrote:
>> On 10.07.19 16:40, Paolo Bonzini wrote:
>>> On 08/07/19 20:31, Jan Kiszka wrote:
> -if (cpu_has_nested_virt(env) && !env->nested_state) {
> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
> error_report("Guest enabled nested virtualization but kernel "
> "does not support saving of nested state");
> return -EINVAL;
>
Starting with this commit latest (bisection issue...), running Jailhouse
in a
guest first stalls L1 (looks like we lose interrupts), and if I try to
reset
that VM, I lose my host as well:
>>>
>>> If 5.2 is still broken, can you gather a dump of KVM_SET_NESTED_STATE's
>>> payload?
>>
>> savevm or is there a dump function in QEMU or the kernel?
>
> There is qemu_hexdump.
>
Here are two states taken during the vmport loop:
2: nested_state: : 01 00 00 00 80 10 00 00 00 00 24 3a 00 00 00 00
..$:
2: nested_state: 0010: 00 10 24 3a 00 00 00 00 00 00 00 00 00 00 00 00
..$:
2: nested_state: 0020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0080: d0 7e e5 11 00 00 00 00 01 00 00 00 00 00 00 00
.~..
2: nested_state: 0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 00a0: 00 00 00 00 00 00 00 00 00 70 25 3a 00 00 00 00
.p%:
2: nested_state: 00b0: 00 80 25 3a 00 00 00 00 00 f0 00 3a 00 00 00 00
..%:...:
2: nested_state: 00c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 00d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 00e0: 00 00 00 00 00 00 00 00 00 50 01 3a 00 00 00 00
.P.:
2: nested_state: 00f0: 00 00 00 00 00 00 00 00 1e e0 22 3a 00 00 00 00
..":
2: nested_state: 0100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0110: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0120: 00 00 00 00 00 00 00 00 10 00 00 00 01 00 00 00
2: nested_state: 0130: ff ff ff ff ff ff ff ff 00 00 00 00 00 00 00 00
2: nested_state: 0140: 06 01 07 00 06 01 07 00 01 0d 00 00 00 00 00 00
2: nested_state: 0150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0170: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0180: 06 01 07 00 00 00 00 00 00 05 00 00 00 00 00 00
2: nested_state: 0190: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 01a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 01b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 01c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 01d0: 00 00 00 00 00 00 00 00 30 00 00 60 ff ff ff ff
0..`
2: nested_state: 01e0: 00 00 00 00 00 00 00 00 33 00 05 80 00 00 00 00
3...
2: nested_state: 01f0: a0 26 04 00 00 00 00 00 00 00 00 00 00 00 00 00
.&..
2: nested_state: 0200: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0210: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0220: 00 00 00 00 00 00 00 00 33 00 05 80 00 00 00 00
3...
2: nested_state: 0230: 00 20 6f 32 00 00 00 00 a0 26 04 00 00 00 00 00 .
o2.&..
2: nested_state: 0240: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0250: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0260: 00 97 c4 ec b4 7f 00 00 00 00 00 00 00 00 00 00
2: nested_state: 0270: 00 00 00 00 00 00 00 00 00 90 06 00 00 fe ff ff
2: nested_state: 0280: 00 70 06 00 00 fe ff ff 00 00 00 00 00 fe ff ff
.p..
2: nested_state: 0290: 00 04 00 00 00 00 00 00 88 33 16 8a fd 7f 00 00
.3..
2: nested_state: 02a0: e4 d4 70 e6 b4 7f 00 00 17 32 00 00 00 00 00 00
..p..2..
2: nested_state: 02b0: 00 00 00 00 00 00 00 00 00 82 06
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 10/07/19 18:08, Jan Kiszka wrote:
> On 10.07.19 16:40, Paolo Bonzini wrote:
>> On 08/07/19 20:31, Jan Kiszka wrote:
-if (cpu_has_nested_virt(env) && !env->nested_state) {
+if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
error_report("Guest enabled nested virtualization but kernel "
"does not support saving of nested state");
return -EINVAL;
>>> Starting with this commit latest (bisection issue...), running Jailhouse in
>>> a
>>> guest first stalls L1 (looks like we lose interrupts), and if I try to reset
>>> that VM, I lose my host as well:
>>
>> If 5.2 is still broken, can you gather a dump of KVM_SET_NESTED_STATE's
>> payload?
>
> savevm or is there a dump function in QEMU or the kernel?
There is qemu_hexdump.
Paolo
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 10.07.19 16:40, Paolo Bonzini wrote:
> On 08/07/19 20:31, Jan Kiszka wrote:
>>> -if (cpu_has_nested_virt(env) && !env->nested_state) {
>>> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
>>> error_report("Guest enabled nested virtualization but kernel "
>>> "does not support saving of nested state");
>>> return -EINVAL;
>>>
>> Starting with this commit latest (bisection issue...), running Jailhouse in a
>> guest first stalls L1 (looks like we lose interrupts), and if I try to reset
>> that VM, I lose my host as well:
>
> If 5.2 is still broken, can you gather a dump of KVM_SET_NESTED_STATE's
> payload?
savevm or is there a dump function in QEMU or the kernel?
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 08/07/19 20:31, Jan Kiszka wrote:
>> -if (cpu_has_nested_virt(env) && !env->nested_state) {
>> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
>> error_report("Guest enabled nested virtualization but kernel "
>> "does not support saving of nested state");
>> return -EINVAL;
>>
> Starting with this commit latest (bisection issue...), running Jailhouse in a
> guest first stalls L1 (looks like we lose interrupts), and if I try to reset
> that VM, I lose my host as well:
If 5.2 is still broken, can you gather a dump of KVM_SET_NESTED_STATE's
payload?
Paolo
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 08.07.19 20:31, Jan Kiszka wrote:
>
> On 21.06.19 13:30, Paolo Bonzini wrote:
>> From: Liran Alon
>>
>> Previous commits have added support for migration of nested virtualization
>> workloads. This was done by utilising two new KVM capabilities:
>> KVM_CAP_NESTED_STATE and KVM_CAP_EXCEPTION_PAYLOAD. Both which are
>> required in order to correctly migrate such workloads.
>>
>> Therefore, change code to add a migration blocker for vCPUs exposed with
>> Intel VMX or AMD SVM in case one of these kernel capabilities is
>> missing.
>>
>> Signed-off-by: Liran Alon
>> Reviewed-by: Maran Wilson
>> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
>> Signed-off-by: Paolo Bonzini
>> ---
>> target/i386/kvm.c | 9 +++--
>> target/i386/machine.c | 2 +-
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index c931e9d..e4b4f57 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1640,9 +1640,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>!!(c->ecx & CPUID_EXT_SMX);
>> }
>>
>> -if (cpu_has_nested_virt(env) && !nested_virt_mig_blocker) {
>> +if (cpu_has_vmx(env) && !nested_virt_mig_blocker &&
>> +((kvm_max_nested_state_length() <= 0) || !has_exception_payload)) {
>> error_setg(_virt_mig_blocker,
>> - "Nested virtualization does not support live migration
>> yet");
>> + "Kernel do not provide required capabilities for "
>> + "nested virtualization migration. "
>> + "(CAP_NESTED_STATE=%d, CAP_EXCEPTION_PAYLOAD=%d)",
>> + kvm_max_nested_state_length() > 0,
>> + has_exception_payload);
>> r = migrate_add_blocker(nested_virt_mig_blocker, _err);
>> if (local_err) {
>> error_report_err(local_err);
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index fc49e5a..851b249 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -233,7 +233,7 @@ static int cpu_pre_save(void *opaque)
>>
>> #ifdef CONFIG_KVM
>> /* Verify we have nested virtualization state from kernel if required */
>> -if (cpu_has_nested_virt(env) && !env->nested_state) {
>> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
>> error_report("Guest enabled nested virtualization but kernel "
>> "does not support saving of nested state");
>> return -EINVAL;
>>
>
> Starting with this commit latest (bisection issue...), running Jailhouse in a
> guest first stalls L1 (looks like we lose interrupts), and if I try to reset
> that VM, I lose my host as well:
>
> kvm: vmptrld (null)/6eb9 failed
> kvm: vmclear fail: (null)/6eb9
>
> and then things start to lock up because we seem to lose the CPUs the guest
> was
> running on. Once I had this in the logs:
>
> rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 7-... } 15040
> jiffies s: 4673 root: 0x80/.
> rcu: blocking rcu_node structures:
> Task dump for CPU 7:
> qemu-system-x86 R running task0 17413 17345 0x0008
> Call Trace:
> ? x86_virt_spec_ctrl+0x7/0xe0
> ? vmx_vcpu_run.part.0+0x2a4/0xfa0 [kvm_intel]
> ? vcpu_enter_guest+0x349/0xe80 [kvm]
> ? kvm_arch_vcpu_ioctl_run+0xff/0x550 [kvm]
> ? kvm_vcpu_ioctl+0x20d/0x590 [kvm]
> ? get_futex_key+0x35d/0x3b0
> ? do_vfs_ioctl+0x447/0x640
> ? do_futex+0x157/0x1d0
> ? ksys_ioctl+0x5e/0x90
> ? __x64_sys_ioctl+0x16/0x20
> ? do_syscall_64+0x60/0x120
> ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> This was on a 5.1.16 distro kernel. Currently rebuilding 5.2 vanilla.
>
> Looks like we have up to two critical bugs here...
>
> Jan
>
It turns out it's actually patch 20 that introduces the problem. Maybe it is
only the kernel, but I rather suspect a combination of userspace not providing
the right state (specifically on reset) and the kernel accepting that.
Continuing the search on 5.2 now.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 21.06.19 13:30, Paolo Bonzini wrote:
> From: Liran Alon
>
> Previous commits have added support for migration of nested virtualization
> workloads. This was done by utilising two new KVM capabilities:
> KVM_CAP_NESTED_STATE and KVM_CAP_EXCEPTION_PAYLOAD. Both which are
> required in order to correctly migrate such workloads.
>
> Therefore, change code to add a migration blocker for vCPUs exposed with
> Intel VMX or AMD SVM in case one of these kernel capabilities is
> missing.
>
> Signed-off-by: Liran Alon
> Reviewed-by: Maran Wilson
> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
> Signed-off-by: Paolo Bonzini
> ---
> target/i386/kvm.c | 9 +++--
> target/i386/machine.c | 2 +-
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index c931e9d..e4b4f57 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1640,9 +1640,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>!!(c->ecx & CPUID_EXT_SMX);
> }
>
> -if (cpu_has_nested_virt(env) && !nested_virt_mig_blocker) {
> +if (cpu_has_vmx(env) && !nested_virt_mig_blocker &&
> +((kvm_max_nested_state_length() <= 0) || !has_exception_payload)) {
> error_setg(_virt_mig_blocker,
> - "Nested virtualization does not support live migration
> yet");
> + "Kernel do not provide required capabilities for "
> + "nested virtualization migration. "
> + "(CAP_NESTED_STATE=%d, CAP_EXCEPTION_PAYLOAD=%d)",
> + kvm_max_nested_state_length() > 0,
> + has_exception_payload);
> r = migrate_add_blocker(nested_virt_mig_blocker, _err);
> if (local_err) {
> error_report_err(local_err);
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index fc49e5a..851b249 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -233,7 +233,7 @@ static int cpu_pre_save(void *opaque)
>
> #ifdef CONFIG_KVM
> /* Verify we have nested virtualization state from kernel if required */
> -if (cpu_has_nested_virt(env) && !env->nested_state) {
> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
> error_report("Guest enabled nested virtualization but kernel "
> "does not support saving of nested state");
> return -EINVAL;
>
Starting with this commit latest (bisection issue...), running Jailhouse in a
guest first stalls L1 (looks like we lose interrupts), and if I try to reset
that VM, I lose my host as well:
kvm: vmptrld (null)/6eb9 failed
kvm: vmclear fail: (null)/6eb9
and then things start to lock up because we seem to lose the CPUs the guest was
running on. Once I had this in the logs:
rcu: INFO: rcu_sched detected expedited stalls on CPUs/tasks: { 7-... } 15040
jiffies s: 4673 root: 0x80/.
rcu: blocking rcu_node structures:
Task dump for CPU 7:
qemu-system-x86 R running task0 17413 17345 0x0008
Call Trace:
? x86_virt_spec_ctrl+0x7/0xe0
? vmx_vcpu_run.part.0+0x2a4/0xfa0 [kvm_intel]
? vcpu_enter_guest+0x349/0xe80 [kvm]
? kvm_arch_vcpu_ioctl_run+0xff/0x550 [kvm]
? kvm_vcpu_ioctl+0x20d/0x590 [kvm]
? get_futex_key+0x35d/0x3b0
? do_vfs_ioctl+0x447/0x640
? do_futex+0x157/0x1d0
? ksys_ioctl+0x5e/0x90
? __x64_sys_ioctl+0x16/0x20
? do_syscall_64+0x60/0x120
? entry_SYSCALL_64_after_hwframe+0x49/0xbe
This was on a 5.1.16 distro kernel. Currently rebuilding 5.2 vanilla.
Looks like we have up to two critical bugs here...
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
> On 21 Jun 2019, at 20:27, Paolo Bonzini wrote:
>
> On 21/06/19 17:07, Liran Alon wrote:
>>> So, overall I prefer not to block migration.
>> I’m not sure I agree.
>> It is quite likely that vCPU is currently in guest-mode while you are
>> migrating…
>> A good hypervisor tries to maximise CPU time to be in guest-mode rather than
>> host-mode. :)
>
> True, but it is even more likely that they are not using KVM at all and
> just happen to have the CPUID flag set. :)
>
> Paolo
Since QEMU commit 75d373ef9729 ("target-i386: Disable SVM by default in KVM
mode"),
An AMD vCPU that is virtualized by KVM don’t expose SVM by default. Even if you
use "-cpu host".
Therefore, it is unlikely that vCPU expose SVM CPUID flag when user is not
running any nSVM
workload inside guest. Unless I’m missing something obvious.
Otherwise, I would have agreed with you.
-Liran
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 21/06/19 17:07, Liran Alon wrote: >> So, overall I prefer not to block migration. > I’m not sure I agree. > It is quite likely that vCPU is currently in guest-mode while you are > migrating… > A good hypervisor tries to maximise CPU time to be in guest-mode rather than > host-mode. :) True, but it is even more likely that they are not using KVM at all and just happen to have the CPUID flag set. :) Paolo
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
> On 21 Jun 2019, at 18:02, Paolo Bonzini wrote:
>
> On 21/06/19 14:39, Liran Alon wrote:
>>> On 21 Jun 2019, at 14:30, Paolo Bonzini wrote:
>>>
>>> From: Liran Alon
>>>
>>> Previous commits have added support for migration of nested virtualization
>>> workloads. This was done by utilising two new KVM capabilities:
>>> KVM_CAP_NESTED_STATE and KVM_CAP_EXCEPTION_PAYLOAD. Both which are
>>> required in order to correctly migrate such workloads.
>>>
>>> Therefore, change code to add a migration blocker for vCPUs exposed with
>>> Intel VMX or AMD SVM in case one of these kernel capabilities is
>>> missing.
>>>
>>> Signed-off-by: Liran Alon
>>> Reviewed-by: Maran Wilson
>>> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
>>> Signed-off-by: Paolo Bonzini
>>> ---
>>> target/i386/kvm.c | 9 +++--
>>> target/i386/machine.c | 2 +-
>>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>>> index c931e9d..e4b4f57 100644
>>> --- a/target/i386/kvm.c
>>> +++ b/target/i386/kvm.c
>>> @@ -1640,9 +1640,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>> !!(c->ecx & CPUID_EXT_SMX);
>>>}
>>>
>>> -if (cpu_has_nested_virt(env) && !nested_virt_mig_blocker) {
>>> +if (cpu_has_vmx(env) && !nested_virt_mig_blocker &&
>>
>> The change here from cpu_has_nested_virt(env) to cpu_has_vmx(env) is not
>> clear.
>> We should explicitly explain that it’s because we still wish to preserve
>> backwards-compatability
>> to migrating AMD vCPU exposed with SVM.
>>
>> In addition, commit ("target/i386: kvm: Block migration for vCPUs exposed
>> with nested virtualization")
>> doesn’t make sense in case we still want to allow migrating AMD vCPU exposed
>> with SVM.
>>
>> Since QEMU commit 75d373ef9729 ("target-i386: Disable SVM by default in KVM
>> mode"),
>> machine-types since v2.2 don’t expose AMD SVM by default.
>> Therefore, my personal opinion on this is that it’s fine to block migration
>> in this case.
>
> I totally missed that commit. My bad.
>
> Actually, now that I think about it SVM *will* have some state while
> running in guest mode, namely:
>
> - the NPT page table root
>
> - the L1 CR4.PAE, EFER.LMA and EFER.NXE bits, which determine the format
> of the NPT12 page tables
>
> These are covered by the existing vmstate_svm_npt subsection.
>
> On the other hand, the lack of something like VMXON/VMCS12 state means
> that AMD already sorta works unless you're migrating while in guest
> mode. For Intel, just execute VMXON before migration, and starting any
> VM after migration is doomed.
True.
>
> So, overall I prefer not to block migration.
I’m not sure I agree.
It is quite likely that vCPU is currently in guest-mode while you are migrating…
A good hypervisor tries to maximise CPU time to be in guest-mode rather than
host-mode. :)
I prefer to block migration and once we formally complete the implementation of
SVM nested state,
we can modify QEMU code such that migration of vCPU exposed with SVM will work
in case
nested-state states that guest is in host-mode.
>
>>> +((kvm_max_nested_state_length() <= 0) || !has_exception_payload)) {
>>>error_setg(_virt_mig_blocker,
>>> - "Nested virtualization does not support live migration
>>> yet");
>>> + "Kernel do not provide required capabilities for “
>>
>> As Maran have noted, we should change this “do not” to “does not”.
>> Sorry for my bad English grammer. :)
>>
>>> + "nested virtualization migration. "
>>> + "(CAP_NESTED_STATE=%d, CAP_EXCEPTION_PAYLOAD=%d)",
>>> + kvm_max_nested_state_length() > 0,
>>> + has_exception_payload);
>>>r = migrate_add_blocker(nested_virt_mig_blocker, _err);
>>>if (local_err) {
>>>error_report_err(local_err);
>>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>>> index fc49e5a..851b249 100644
>>> --- a/target/i386/machine.c
>>> +++ b/target/i386/machine.c
>>> @@ -233,7 +233,7 @@ static int cpu_pre_save(void *opaque)
>>>
>>> #ifdef CONFIG_KVM
>>>/* Verify we have nested virtualization state from kernel if required */
>>> -if (cpu_has_nested_virt(env) && !env->nested_state) {
>>> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
>>
>> Good catch regarding adding kvm_enabled() here.
>
> Caught by "make check", not by me!
Ah nice to know :) Thanks for the tip.
>
>> But I think this should have been added to commit ("target/i386: kvm: Add
>> support for save and restore nested state”).
>
> This commit is where bisection broke. :)
>
> Paolo
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
On 21/06/19 14:39, Liran Alon wrote:
>> On 21 Jun 2019, at 14:30, Paolo Bonzini wrote:
>>
>> From: Liran Alon
>>
>> Previous commits have added support for migration of nested virtualization
>> workloads. This was done by utilising two new KVM capabilities:
>> KVM_CAP_NESTED_STATE and KVM_CAP_EXCEPTION_PAYLOAD. Both which are
>> required in order to correctly migrate such workloads.
>>
>> Therefore, change code to add a migration blocker for vCPUs exposed with
>> Intel VMX or AMD SVM in case one of these kernel capabilities is
>> missing.
>>
>> Signed-off-by: Liran Alon
>> Reviewed-by: Maran Wilson
>> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
>> Signed-off-by: Paolo Bonzini
>> ---
>> target/i386/kvm.c | 9 +++--
>> target/i386/machine.c | 2 +-
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index c931e9d..e4b4f57 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1640,9 +1640,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> !!(c->ecx & CPUID_EXT_SMX);
>> }
>>
>> -if (cpu_has_nested_virt(env) && !nested_virt_mig_blocker) {
>> +if (cpu_has_vmx(env) && !nested_virt_mig_blocker &&
>
> The change here from cpu_has_nested_virt(env) to cpu_has_vmx(env) is not
> clear.
> We should explicitly explain that it’s because we still wish to preserve
> backwards-compatability
> to migrating AMD vCPU exposed with SVM.
>
> In addition, commit ("target/i386: kvm: Block migration for vCPUs exposed
> with nested virtualization")
> doesn’t make sense in case we still want to allow migrating AMD vCPU exposed
> with SVM.
>
> Since QEMU commit 75d373ef9729 ("target-i386: Disable SVM by default in KVM
> mode"),
> machine-types since v2.2 don’t expose AMD SVM by default.
> Therefore, my personal opinion on this is that it’s fine to block migration
> in this case.
I totally missed that commit. My bad.
Actually, now that I think about it SVM *will* have some state while
running in guest mode, namely:
- the NPT page table root
- the L1 CR4.PAE, EFER.LMA and EFER.NXE bits, which determine the format
of the NPT12 page tables
These are covered by the existing vmstate_svm_npt subsection.
On the other hand, the lack of something like VMXON/VMCS12 state means
that AMD already sorta works unless you're migrating while in guest
mode. For Intel, just execute VMXON before migration, and starting any
VM after migration is doomed.
So, overall I prefer not to block migration.
>> +((kvm_max_nested_state_length() <= 0) || !has_exception_payload)) {
>> error_setg(_virt_mig_blocker,
>> - "Nested virtualization does not support live migration
>> yet");
>> + "Kernel do not provide required capabilities for “
>
> As Maran have noted, we should change this “do not” to “does not”.
> Sorry for my bad English grammer. :)
>
>> + "nested virtualization migration. "
>> + "(CAP_NESTED_STATE=%d, CAP_EXCEPTION_PAYLOAD=%d)",
>> + kvm_max_nested_state_length() > 0,
>> + has_exception_payload);
>> r = migrate_add_blocker(nested_virt_mig_blocker, _err);
>> if (local_err) {
>> error_report_err(local_err);
>> diff --git a/target/i386/machine.c b/target/i386/machine.c
>> index fc49e5a..851b249 100644
>> --- a/target/i386/machine.c
>> +++ b/target/i386/machine.c
>> @@ -233,7 +233,7 @@ static int cpu_pre_save(void *opaque)
>>
>> #ifdef CONFIG_KVM
>> /* Verify we have nested virtualization state from kernel if required */
>> -if (cpu_has_nested_virt(env) && !env->nested_state) {
>> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
>
> Good catch regarding adding kvm_enabled() here.
Caught by "make check", not by me!
> But I think this should have been added to commit ("target/i386: kvm: Add
> support for save and restore nested state”).
This commit is where bisection broke. :)
Paolo
Re: [Qemu-devel] [PULL 22/25] target/i386: kvm: Add nested migration blocker only when kernel lacks required capabilities
> On 21 Jun 2019, at 14:30, Paolo Bonzini wrote:
>
> From: Liran Alon
>
> Previous commits have added support for migration of nested virtualization
> workloads. This was done by utilising two new KVM capabilities:
> KVM_CAP_NESTED_STATE and KVM_CAP_EXCEPTION_PAYLOAD. Both which are
> required in order to correctly migrate such workloads.
>
> Therefore, change code to add a migration blocker for vCPUs exposed with
> Intel VMX or AMD SVM in case one of these kernel capabilities is
> missing.
>
> Signed-off-by: Liran Alon
> Reviewed-by: Maran Wilson
> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
> Signed-off-by: Paolo Bonzini
> ---
> target/i386/kvm.c | 9 +++--
> target/i386/machine.c | 2 +-
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index c931e9d..e4b4f57 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1640,9 +1640,14 @@ int kvm_arch_init_vcpu(CPUState *cs)
> !!(c->ecx & CPUID_EXT_SMX);
> }
>
> -if (cpu_has_nested_virt(env) && !nested_virt_mig_blocker) {
> +if (cpu_has_vmx(env) && !nested_virt_mig_blocker &&
The change here from cpu_has_nested_virt(env) to cpu_has_vmx(env) is not clear.
We should explicitly explain that it’s because we still wish to preserve
backwards-compatability
to migrating AMD vCPU exposed with SVM.
In addition, commit ("target/i386: kvm: Block migration for vCPUs exposed with
nested virtualization")
doesn’t make sense in case we still want to allow migrating AMD vCPU exposed
with SVM.
Since QEMU commit 75d373ef9729 ("target-i386: Disable SVM by default in KVM
mode"),
machine-types since v2.2 don’t expose AMD SVM by default.
Therefore, my personal opinion on this is that it’s fine to block migration in
this case.
> +((kvm_max_nested_state_length() <= 0) || !has_exception_payload)) {
> error_setg(_virt_mig_blocker,
> - "Nested virtualization does not support live migration
> yet");
> + "Kernel do not provide required capabilities for “
As Maran have noted, we should change this “do not” to “does not”.
Sorry for my bad English grammer. :)
> + "nested virtualization migration. "
> + "(CAP_NESTED_STATE=%d, CAP_EXCEPTION_PAYLOAD=%d)",
> + kvm_max_nested_state_length() > 0,
> + has_exception_payload);
> r = migrate_add_blocker(nested_virt_mig_blocker, _err);
> if (local_err) {
> error_report_err(local_err);
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index fc49e5a..851b249 100644
> --- a/target/i386/machine.c
> +++ b/target/i386/machine.c
> @@ -233,7 +233,7 @@ static int cpu_pre_save(void *opaque)
>
> #ifdef CONFIG_KVM
> /* Verify we have nested virtualization state from kernel if required */
> -if (cpu_has_nested_virt(env) && !env->nested_state) {
> +if (kvm_enabled() && cpu_has_vmx(env) && !env->nested_state) {
Good catch regarding adding kvm_enabled() here.
But I think this should have been added to commit ("target/i386: kvm: Add
support for save and restore nested state”).
-Liran
> error_report("Guest enabled nested virtualization but kernel "
> "does not support saving of nested state");
> return -EINVAL;
> --
> 1.8.3.1
>
>
