qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KVM: fix CPU reset wrt HF2_GIF_MASK
@ 2020-07-23 14:27 Vitaly Kuznetsov
  2020-07-23 14:39 ` Jan Kiszka
  0 siblings, 1 reply; 2+ messages in thread
From: Vitaly Kuznetsov @ 2020-07-23 14:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Eduardo Habkost, Jan Kiszka, Marcelo Tosatti,
	Dr. David Alan Gilbert, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	Richard Henderson

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;
-- 
2.25.4



^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] KVM: fix CPU reset wrt HF2_GIF_MASK
  2020-07-23 14:27 [PATCH] KVM: fix CPU reset wrt HF2_GIF_MASK Vitaly Kuznetsov
@ 2020-07-23 14:39 ` Jan Kiszka
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Kiszka @ 2020-07-23 14:39 UTC (permalink / raw)
  To: Vitaly Kuznetsov, qemu-devel
  Cc: Eduardo Habkost, Marcelo Tosatti, Dr. David Alan Gilbert,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	Richard Henderson

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

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-07-23 14:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 14:27 [PATCH] KVM: fix CPU reset wrt HF2_GIF_MASK Vitaly Kuznetsov
2020-07-23 14:39 ` Jan Kiszka

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).