2021-05-03 18:06:32

by Maxim Levitsky

[permalink] [raw]
Subject: [PATCH 0/5] KVM: nSVM: few fixes for the nested migration

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



--

2.26.2





2021-05-03 18:06:43

by Maxim Levitsky

[permalink] [raw]
Subject: [PATCH 2/5] KVM: nSVM: fix few bugs in the vmcb02 caching logic

* 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 <[email protected]>
---
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

2021-05-03 18:09:49

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH 0/5] KVM: nSVM: few fixes for the nested migration

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