> On 21 Jun 2019, at 14:30, Paolo Bonzini <pbonz...@redhat.com> wrote:
>
> From: Liran Alon <liran.a...@oracle.com>
>
> 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 <liran.a...@oracle.com>
> Reviewed-by: Maran Wilson <maran.wil...@oracle.com>
> Message-Id: <20190619162140.133674-11-liran.a...@oracle.com>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> 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(&nested_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, &local_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
>
>
