kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] KVM: nSVM: few fixes for the nested migration
@ 2021-05-03 12:54 Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 1/5] KVM: nSVM: fix a typo in svm_leave_nested Maxim Levitsky
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

Those are few fixes for issues I uncovered by doing variants of a\r
synthetic migration test I just created:\r
\r
I modified the qemu, such that on each vm pause/resume cycle,\r
just prior to resuming a vCPU, qemu reads its KVM state,\r
then (optionaly) resets this state by uploading a\r
dummy reset state to KVM, and then it uploads back to KVM,\r
the state that this vCPU had before.\r
\r
I'll try to make this test upstreamable soon, pending few details\r
I need to figure out.\r
\r
Last patch in this series is for false positive warning\r
that I have seen lately when setting the nested state,\r
in nested_svm_vmexit, where it expects the vmcb01 to have\r
VMRUN vmexit, which is not true after nested migration,\r
as it is not fully initialized.\r
If you prefer the warning can be removed instead.\r
\r
Best regards,\r
	Maxim Levitsky\r
\r
Maxim Levitsky (5):\r
  KVM: nSVM: fix a typo in svm_leave_nested\r
  KVM: nSVM: fix few bugs in the vmcb02 caching logic\r
  KVM: nSVM: leave the guest mode prior to loading a nested state\r
  KVM: nSVM: force L1's GIF to 1 when setting the nested state\r
  KVM: nSVM: set a dummy exit reason in L1 vmcb when loading the nested\r
    state\r
\r
 arch/x86/include/asm/kvm_host.h |  1 +\r
 arch/x86/kvm/svm/nested.c       | 29 ++++++++++++++++++++++++++---\r
 arch/x86/kvm/svm/svm.c          |  4 ++--\r
 3 files changed, 29 insertions(+), 5 deletions(-)\r
\r
-- \r
2.26.2\r
\r


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

* [PATCH 1/5] KVM: nSVM: fix a typo in svm_leave_nested
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
@ 2021-05-03 12:54 ` Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 2/5] KVM: nSVM: fix few bugs in the vmcb02 caching logic Maxim Levitsky
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

When forcibly leaving the nested mode, we should switch to vmcb01

Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 540d43ba2cf4..3321220f3deb 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -886,7 +886,7 @@ void svm_leave_nested(struct vcpu_svm *svm)
 		svm->nested.nested_run_pending = 0;
 		leave_guest_mode(vcpu);
 
-		svm_switch_vmcb(svm, &svm->nested.vmcb02);
+		svm_switch_vmcb(svm, &svm->vmcb01);
 
 		nested_svm_uninit_mmu_context(vcpu);
 		vmcb_mark_all_dirty(svm->vmcb);
-- 
2.26.2


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

* [PATCH 2/5] KVM: nSVM: fix few bugs in the vmcb02 caching logic
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 1/5] KVM: nSVM: fix a typo in svm_leave_nested Maxim Levitsky
@ 2021-05-03 12:54 ` Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 3/5] KVM: nSVM: leave the guest mode prior to loading a nested state Maxim Levitsky
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

* Define and use an invalid GPA (all ones) for init value of last
  and current nested vmcb physical addresses.

* Reset the current vmcb12 gpa to the invalid value when leaving
  the nested mode, similar to what is done on nested vmexit.

* Reset	the last seen vmcb12 address when disabling the nested SVM,
  as it relies on vmcb02 fields which are freed at that point.

Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/svm/nested.c       | 11 +++++++++++
 arch/x86/kvm/svm/svm.c          |  4 ++--
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3e5fc80a35c8..4b95e7c4a4e3 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -113,6 +113,7 @@
 #define VALID_PAGE(x) ((x) != INVALID_PAGE)
 
 #define UNMAPPED_GVA (~(gpa_t)0)
+#define INVALID_GPA (~(gpa_t)0)
 
 /* KVM Hugepage definitions for x86 */
 #define KVM_MAX_HUGEPAGE_LEVEL	PG_LEVEL_1G
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 3321220f3deb..a88c64e004c3 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -872,6 +872,15 @@ void svm_free_nested(struct vcpu_svm *svm)
 	__free_page(virt_to_page(svm->nested.vmcb02.ptr));
 	svm->nested.vmcb02.ptr = NULL;
 
+	/*
+	 * When last_vmcb12_gpa matches the current vmcb12 gpa,
+	 * some vmcb12 fields are not loaded if they are marked clean
+	 * in the vmcb12, since in this case they are up to date already.
+	 *
+	 * When the vmcb02 is freed, this optimization becomes invalid.
+	 */
+	svm->nested.last_vmcb12_gpa = INVALID_GPA;
+
 	svm->nested.initialized = false;
 }
 
@@ -884,6 +893,8 @@ void svm_leave_nested(struct vcpu_svm *svm)
 
 	if (is_guest_mode(vcpu)) {
 		svm->nested.nested_run_pending = 0;
+		svm->nested.vmcb12_gpa = INVALID_GPA;
+
 		leave_guest_mode(vcpu);
 
 		svm_switch_vmcb(svm, &svm->vmcb01);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index a7271f31df47..987173e1f954 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1235,8 +1235,8 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
 	svm->current_vmcb->asid_generation = 0;
 	svm->asid = 0;
 
-	svm->nested.vmcb12_gpa = 0;
-	svm->nested.last_vmcb12_gpa = 0;
+	svm->nested.vmcb12_gpa = INVALID_GPA;
+	svm->nested.last_vmcb12_gpa = INVALID_GPA;
 	vcpu->arch.hflags = 0;
 
 	if (!kvm_pause_in_guest(vcpu->kvm)) {
-- 
2.26.2


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

* [PATCH 3/5] KVM: nSVM: leave the guest mode prior to loading a nested state
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 1/5] KVM: nSVM: fix a typo in svm_leave_nested Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 2/5] KVM: nSVM: fix few bugs in the vmcb02 caching logic Maxim Levitsky
@ 2021-05-03 12:54 ` Maxim Levitsky
  2021-05-03 12:54 ` [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the " Maxim Levitsky
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

This allows the KVM to load the nested state more than
once without warnings.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index a88c64e004c3..32400cba608d 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1309,12 +1309,15 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	 * L2 registers if needed are moved from the current VMCB to VMCB02.
 	 */
 
+	if (is_guest_mode(vcpu))
+		svm_leave_nested(svm);
+	else
+		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
+
 	svm->nested.nested_run_pending =
 		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
 
 	svm->nested.vmcb12_gpa = kvm_state->hdr.svm.vmcb_pa;
-	if (svm->current_vmcb == &svm->vmcb01)
-		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
 
 	svm->vmcb01.ptr->save.es = save->es;
 	svm->vmcb01.ptr->save.cs = save->cs;
-- 
2.26.2


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

* [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the nested state
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
                   ` (2 preceding siblings ...)
  2021-05-03 12:54 ` [PATCH 3/5] KVM: nSVM: leave the guest mode prior to loading a nested state Maxim Levitsky
@ 2021-05-03 12:54 ` Maxim Levitsky
  2021-05-03 14:00   ` Paolo Bonzini
  2021-05-03 12:54 ` [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading " Maxim Levitsky
  2021-05-03 14:03 ` [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Paolo Bonzini
  5 siblings, 1 reply; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

While after a reset the GIF value is already 1,
it doesn't have to have this value if the nested state
is loaded later.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 32400cba608d..12a12ae940fa 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1314,6 +1314,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	else
 		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
 
+	/* Force L1's GIF to true */
+	svm_set_gif(svm, true);
+
 	svm->nested.nested_run_pending =
 		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
 
-- 
2.26.2


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

* [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading the nested state
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
                   ` (3 preceding siblings ...)
  2021-05-03 12:54 ` [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the " Maxim Levitsky
@ 2021-05-03 12:54 ` Maxim Levitsky
  2021-05-03 14:02   ` Paolo Bonzini
  2021-05-03 14:03 ` [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Paolo Bonzini
  5 siblings, 1 reply; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 12:54 UTC (permalink / raw)
  To: kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, Paolo Bonzini, H. Peter Anvin,
	Borislav Petkov, Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery, Maxim Levitsky

Since the nested migration is a result of a VMRUN, this makes it
possible to keep a warning that L1 vmcb should always have
VMRUN exit reason when switching back to it, which
otherwise triggers incorrectly.

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/svm/nested.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 12a12ae940fa..146be4b5084b 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1338,6 +1338,12 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	svm->vmcb01.ptr->save.rip = save->rip;
 	svm->vmcb01.ptr->save.cpl = 0;
 
+	/*
+	 * For consistency sake, restore the L1 exit reason
+	 * (that happened prior to the migration) to SVM_EXIT_VMRUN.
+	 */
+	svm->vmcb->control.exit_code = SVM_EXIT_VMRUN;
+
 	nested_load_control_from_vmcb12(svm, ctl);
 
 	svm_switch_vmcb(svm, &svm->nested.vmcb02);
-- 
2.26.2


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

* Re: [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the nested state
  2021-05-03 12:54 ` [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the " Maxim Levitsky
@ 2021-05-03 14:00   ` Paolo Bonzini
  2021-05-03 14:24     ` Maxim Levitsky
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2021-05-03 14:00 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, H. Peter Anvin, Borislav Petkov,
	Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery

On 03/05/21 14:54, Maxim Levitsky wrote:
> While after a reset the GIF value is already 1,
> it doesn't have to have this value if the nested state
> is loaded later.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>   arch/x86/kvm/svm/nested.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 32400cba608d..12a12ae940fa 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1314,6 +1314,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>   	else
>   		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
>   
> +	/* Force L1's GIF to true */
> +	svm_set_gif(svm, true);
> +
>   	svm->nested.nested_run_pending =
>   		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
>   
> 

Hmm, not sure about this one.  It is possible in principle to do CLGI in 
L2 with the intercept disabled.

You need to use

svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));

instead.

Paolo


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

* Re: [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading the nested state
  2021-05-03 12:54 ` [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading " Maxim Levitsky
@ 2021-05-03 14:02   ` Paolo Bonzini
  2021-05-03 14:26     ` Maxim Levitsky
  0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2021-05-03 14:02 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, H. Peter Anvin, Borislav Petkov,
	Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery

On 03/05/21 14:54, Maxim Levitsky wrote:
> Since the nested migration is a result of a VMRUN, this makes it
> possible to keep a warning that L1 vmcb should always have
> VMRUN exit reason when switching back to it, which
> otherwise triggers incorrectly.
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>

Does this fix "KVM: nSVM: If VMRUN is single-stepped, queue the #DB 
intercept in nested_svm_vmexit()"?  I don't like this, and also vmcb12 
is not initialized here (nested_load_control_from_vmcb12 is using the 
state passed in from userspace instead).

I think you should just remove the WARN instead.

Paolo

> ---
>   arch/x86/kvm/svm/nested.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 12a12ae940fa..146be4b5084b 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1338,6 +1338,12 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
>   	svm->vmcb01.ptr->save.rip = save->rip;
>   	svm->vmcb01.ptr->save.cpl = 0;
>   
> +	/*
> +	 * For consistency sake, restore the L1 exit reason
> +	 * (that happened prior to the migration) to SVM_EXIT_VMRUN.
> +	 */
> +	svm->vmcb->control.exit_code = SVM_EXIT_VMRUN;
> +
>   	nested_load_control_from_vmcb12(svm, ctl);
>   
>   	svm_switch_vmcb(svm, &svm->nested.vmcb02);
> 


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

* Re: [PATCH 0/5] KVM: nSVM: few fixes for the nested migration
  2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
                   ` (4 preceding siblings ...)
  2021-05-03 12:54 ` [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading " Maxim Levitsky
@ 2021-05-03 14:03 ` Paolo Bonzini
  5 siblings, 0 replies; 11+ messages in thread
From: Paolo Bonzini @ 2021-05-03 14:03 UTC (permalink / raw)
  To: Maxim Levitsky, kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, H. Peter Anvin, Borislav Petkov,
	Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery

On 03/05/21 14:54, Maxim Levitsky wrote:
> Those are few fixes for issues I uncovered by doing variants of a
> synthetic migration test I just created:
> 
> I modified the qemu, such that on each vm pause/resume cycle,
> just prior to resuming a vCPU, qemu reads its KVM state,
> then (optionaly) resets this state by uploading a
> dummy reset state to KVM, and then it uploads back to KVM,
> the state that this vCPU had before.
> 
> I'll try to make this test upstreamable soon, pending few details
> I need to figure out.
> 
> Last patch in this series is for false positive warning
> that I have seen lately when setting the nested state,
> in nested_svm_vmexit, where it expects the vmcb01 to have
> VMRUN vmexit, which is not true after nested migration,
> as it is not fully initialized.
> If you prefer the warning can be removed instead.
> 
> Best regards,
> 	Maxim Levitsky
> 
> Maxim Levitsky (5):
>    KVM: nSVM: fix a typo in svm_leave_nested
>    KVM: nSVM: fix few bugs in the vmcb02 caching logic
>    KVM: nSVM: leave the guest mode prior to loading a nested state
>    KVM: nSVM: force L1's GIF to 1 when setting the nested state
>    KVM: nSVM: set a dummy exit reason in L1 vmcb when loading the nested
>      state
> 
>   arch/x86/include/asm/kvm_host.h |  1 +
>   arch/x86/kvm/svm/nested.c       | 29 ++++++++++++++++++++++++++---
>   arch/x86/kvm/svm/svm.c          |  4 ++--
>   3 files changed, 29 insertions(+), 5 deletions(-)
> 

Queued patches 1-3.

Paolo


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

* Re: [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the nested state
  2021-05-03 14:00   ` Paolo Bonzini
@ 2021-05-03 14:24     ` Maxim Levitsky
  0 siblings, 0 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 14:24 UTC (permalink / raw)
  To: Paolo Bonzini, kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, H. Peter Anvin, Borislav Petkov,
	Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery

On Mon, 2021-05-03 at 16:00 +0200, Paolo Bonzini wrote:
> On 03/05/21 14:54, Maxim Levitsky wrote:
> > While after a reset the GIF value is already 1,
> > it doesn't have to have this value if the nested state
> > is loaded later.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> >   arch/x86/kvm/svm/nested.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 32400cba608d..12a12ae940fa 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -1314,6 +1314,9 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >   	else
> >   		svm->nested.vmcb02.ptr->save = svm->vmcb01.ptr->save;
> >   
> > +	/* Force L1's GIF to true */
> > +	svm_set_gif(svm, true);
> > +
> >   	svm->nested.nested_run_pending =
> >   		!!(kvm_state->flags & KVM_STATE_NESTED_RUN_PENDING);
> >   
> > 
> 
> Hmm, not sure about this one.  It is possible in principle to do CLGI in 
> L2 with the intercept disabled.

I need to think about this a bit more. 
In theory we have L0 GIF, the L1 GIF and the L2 GIF.
L0 GIF is always KVM's, so no problem.
L1 GIF can be toggled with L1 executing clgi/stgi, and it will be either stored in 
vmcb.int_ctl (vmcb01 or vmcb02) or in hflags depending if vGIF is enabled.
(the L1 owned bits are copied in nested_vmcb02_prepare_control)

For L2 we never advertise virtual gif and we don't let it set V_GIF_ENABLE_MASK
in int_ctl, so it either intercepts clgi/stgi and does its own businesses with it
or it doesn't intercept it in which case L2 indeed just modifies L1 GIF.


> 
> You need to use
> 
> svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));


Assuming that the above is correct, then indeed, this should be done,
so I'll send a patch for this.
Thanks a lot!!

Best regards,
	Maxim Levitsky



> 
> instead.
> 
> Paolo
> 



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

* Re: [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading the nested state
  2021-05-03 14:02   ` Paolo Bonzini
@ 2021-05-03 14:26     ` Maxim Levitsky
  0 siblings, 0 replies; 11+ messages in thread
From: Maxim Levitsky @ 2021-05-03 14:26 UTC (permalink / raw)
  To: Paolo Bonzini, kvm
  Cc: Wanpeng Li, Thomas Gleixner,
	open list:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Jim Mattson, Vitaly Kuznetsov, H. Peter Anvin, Borislav Petkov,
	Joerg Roedel, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	Sean Christopherson, Cathy Avery

On Mon, 2021-05-03 at 16:02 +0200, Paolo Bonzini wrote:
> On 03/05/21 14:54, Maxim Levitsky wrote:
> > Since the nested migration is a result of a VMRUN, this makes it
> > possible to keep a warning that L1 vmcb should always have
> > VMRUN exit reason when switching back to it, which
> > otherwise triggers incorrectly.
> > 
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> 
> Does this fix "KVM: nSVM: If VMRUN is single-stepped, queue the #DB 
> intercept in nested_svm_vmexit()"?  I don't like this, and also vmcb12 
> is not initialized here (nested_load_control_from_vmcb12 is using the 
> state passed in from userspace instead).

> 
> I think you should just remove the WARN instead.
All right, I'll do this!

Best regards,
	Maxim Levitsky


> 
> Paolo
> 
> > ---
> >   arch/x86/kvm/svm/nested.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > index 12a12ae940fa..146be4b5084b 100644
> > --- a/arch/x86/kvm/svm/nested.c
> > +++ b/arch/x86/kvm/svm/nested.c
> > @@ -1338,6 +1338,12 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> >   	svm->vmcb01.ptr->save.rip = save->rip;
> >   	svm->vmcb01.ptr->save.cpl = 0;
> >   
> > +	/*
> > +	 * For consistency sake, restore the L1 exit reason
> > +	 * (that happened prior to the migration) to SVM_EXIT_VMRUN.
> > +	 */
> > +	svm->vmcb->control.exit_code = SVM_EXIT_VMRUN;
> > +
> >   	nested_load_control_from_vmcb12(svm, ctl);
> >   
> >   	svm_switch_vmcb(svm, &svm->nested.vmcb02);
> > 



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

end of thread, other threads:[~2021-05-03 14:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-03 12:54 [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Maxim Levitsky
2021-05-03 12:54 ` [PATCH 1/5] KVM: nSVM: fix a typo in svm_leave_nested Maxim Levitsky
2021-05-03 12:54 ` [PATCH 2/5] KVM: nSVM: fix few bugs in the vmcb02 caching logic Maxim Levitsky
2021-05-03 12:54 ` [PATCH 3/5] KVM: nSVM: leave the guest mode prior to loading a nested state Maxim Levitsky
2021-05-03 12:54 ` [PATCH 4/5] KVM: nSVM: force L1's GIF to 1 when setting the " Maxim Levitsky
2021-05-03 14:00   ` Paolo Bonzini
2021-05-03 14:24     ` Maxim Levitsky
2021-05-03 12:54 ` [PATCH 5/5] KVM: nSVM: set a dummy exit reason in L1 vmcb when loading " Maxim Levitsky
2021-05-03 14:02   ` Paolo Bonzini
2021-05-03 14:26     ` Maxim Levitsky
2021-05-03 14:03 ` [PATCH 0/5] KVM: nSVM: few fixes for the nested migration Paolo Bonzini

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).