| Message ID | 20200723142701.2521161-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: fix CPU reset wrt HF2_GIF_MASK | expand |
On 23.07.20 16:27, Vitaly Kuznetsov wrote: > HF2_GIF_MASK is set in env->hflags2 unconditionally on CPU reset > (see x86_cpu_reset()) but when calling KVM_SET_NESTED_STATE, > KVM_STATE_NESTED_GIF_SET is only valid for nSVM as e.g. nVMX code > looks like > > if (kvm_state->hdr.vmx.vmxon_pa == -1ull) { > if (kvm_state->flags & ~KVM_STATE_NESTED_EVMCS) > return -EINVAL; > } > > Also, when adjusting the environment after KVM_GET_NESTED_STATE we > need not reset HF2_GIF_MASK on VMX as e.g. x86_cpu_pending_interrupt() > expects it to be set. > > Alternatively, we could've made env->hflags2 SVM-only. > > Reported-by: Jan Kiszka <jan.kiszka@siemens.com> > Fixes: b16c0e20c742 ("KVM: add support for AMD nested live migration") > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > target/i386/kvm.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/target/i386/kvm.c b/target/i386/kvm.c > index 2b6b7443d273..3c4647fbfbf6 100644 > --- a/target/i386/kvm.c > +++ b/target/i386/kvm.c > @@ -3883,7 +3883,9 @@ static int kvm_put_nested_state(X86CPU *cpu) > } else { > env->nested_state->flags &= ~KVM_STATE_NESTED_GUEST_MODE; > } > - if (env->hflags2 & HF2_GIF_MASK) { > + > + /* Don't set KVM_STATE_NESTED_GIF_SET on VMX as it is illegal */ > + if (cpu_has_svm(env) && (env->hflags2 & HF2_GIF_MASK)) { > env->nested_state->flags |= KVM_STATE_NESTED_GIF_SET; > } else { > env->nested_state->flags &= ~KVM_STATE_NESTED_GIF_SET; > @@ -3925,10 +3927,14 @@ static int kvm_get_nested_state(X86CPU *cpu) > } else { > env->hflags &= ~HF_GUEST_MASK; > } > - if (env->nested_state->flags & KVM_STATE_NESTED_GIF_SET) { > - env->hflags2 |= HF2_GIF_MASK; > - } else { > - env->hflags2 &= ~HF2_GIF_MASK; > + > + /* Keep HF2_GIF_MASK set on !SVM as x86_cpu_pending_interrupt() needs it */ > + if (cpu_has_svm(env)) { > + if (env->nested_state->flags & KVM_STATE_NESTED_GIF_SET) { > + env->hflags2 |= HF2_GIF_MASK; > + } else { > + env->hflags2 &= ~HF2_GIF_MASK; > + } > } > > return ret; > Tested-by: Jan Kiszka <jan.kiszka@siemens.com> Thanks! Jan
diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 2b6b7443d273..3c4647fbfbf6 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -3883,7 +3883,9 @@ static int kvm_put_nested_state(X86CPU *cpu) } else { env->nested_state->flags &= ~KVM_STATE_NESTED_GUEST_MODE; } - if (env->hflags2 & HF2_GIF_MASK) { + + /* Don't set KVM_STATE_NESTED_GIF_SET on VMX as it is illegal */ + if (cpu_has_svm(env) && (env->hflags2 & HF2_GIF_MASK)) { env->nested_state->flags |= KVM_STATE_NESTED_GIF_SET; } else { env->nested_state->flags &= ~KVM_STATE_NESTED_GIF_SET; @@ -3925,10 +3927,14 @@ static int kvm_get_nested_state(X86CPU *cpu) } else { env->hflags &= ~HF_GUEST_MASK; } - if (env->nested_state->flags & KVM_STATE_NESTED_GIF_SET) { - env->hflags2 |= HF2_GIF_MASK; - } else { - env->hflags2 &= ~HF2_GIF_MASK; + + /* Keep HF2_GIF_MASK set on !SVM as x86_cpu_pending_interrupt() needs it */ + if (cpu_has_svm(env)) { + if (env->nested_state->flags & KVM_STATE_NESTED_GIF_SET) { + env->hflags2 |= HF2_GIF_MASK; + } else { + env->hflags2 &= ~HF2_GIF_MASK; + } } return ret;
HF2_GIF_MASK is set in env->hflags2 unconditionally on CPU reset (see x86_cpu_reset()) but when calling KVM_SET_NESTED_STATE, KVM_STATE_NESTED_GIF_SET is only valid for nSVM as e.g. nVMX code looks like if (kvm_state->hdr.vmx.vmxon_pa == -1ull) { if (kvm_state->flags & ~KVM_STATE_NESTED_EVMCS) return -EINVAL; } Also, when adjusting the environment after KVM_GET_NESTED_STATE we need not reset HF2_GIF_MASK on VMX as e.g. x86_cpu_pending_interrupt() expects it to be set. Alternatively, we could've made env->hflags2 SVM-only. Reported-by: Jan Kiszka <jan.kiszka@siemens.com> Fixes: b16c0e20c742 ("KVM: add support for AMD nested live migration") Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- target/i386/kvm.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)