[PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[Joerg Roedel]

Hi,

here is a couple of fixes for the nested SVM implementation. I collected these
fixes mostly when trying to get Windows 7 64bit running as an L2 guest. Most
important fixes in this set make lazy fpu switching working with nested SVM and
the nested tpr handling fixes. Without the later fix the l1 guest freezes when
trying to run win7 as l2 guest. Please review and comment on these patches :-)

	Joerg

Diffstat:

 arch/x86/kvm/svm.c   |  187 ++++++++++++++++++++++++++++++++++----------------
 arch/x86/kvm/trace.h |   12 ++--
 2 files changed, 133 insertions(+), 66 deletions(-)

Shortlog:

Joerg Roedel (10):
      KVM: SVM: Don't use kmap_atomic in nested_svm_map
      KVM: SVM: Fix wrong interrupt injection in enable_irq_windows
      KVM: SVM: Fix schedule-while-atomic on nested exception handling
      KVM: SVM: Sync all control registers on nested vmexit
      KVM: SVM: Annotate nested_svm_map with might_sleep()
      KVM: SVM: Fix nested msr intercept handling
      KVM: SVM: Don't sync nested cr8 to lapic and back
      KVM: SVM: Activate nested state only when guest state is complete
      KVM: SVM: Make lazy FPU switching work with nested svm
      KVM: SVM: Remove newlines from nested trace points

[PATCH 01/10] KVM: SVM: Don't use kmap_atomic in nested_svm_map

[Joerg Roedel]

Use of kmap_atomic disables preemption but if we run in
shadow-shadow mode the vmrun emulation executes kvm_set_cr3
which might sleep or fault. So use kmap instead for
nested_svm_map.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   32 ++++++++++++++++----------------
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 52f78dd..041ef6f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1417,7 +1417,7 @@ static inline int nested_svm_intr(struct vcpu_svm *svm)
 	return 0;
 }
 
-static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa, enum km_type idx)
+static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa)
 {
 	struct page *page;
 
@@ -1425,7 +1425,7 @@ static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa, enum km_type idx)
 	if (is_error_page(page))
 		goto error;
 
-	return kmap_atomic(page, idx);
+	return kmap(page);
 
 error:
 	kvm_release_page_clean(page);
@@ -1434,7 +1434,7 @@ error:
 	return NULL;
 }
 
-static void nested_svm_unmap(void *addr, enum km_type idx)
+static void nested_svm_unmap(void *addr)
 {
 	struct page *page;
 
@@ -1443,7 +1443,7 @@ static void nested_svm_unmap(void *addr, enum km_type idx)
 
 	page = kmap_atomic_to_page(addr);
 
-	kunmap_atomic(addr, idx);
+	kunmap(addr);
 	kvm_release_page_dirty(page);
 }
 
@@ -1458,7 +1458,7 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
 	if (!(svm->nested.intercept & (1ULL << INTERCEPT_MSR_PROT)))
 		return false;
 
-	msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm, KM_USER0);
+	msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm);
 
 	if (!msrpm)
 		goto out;
@@ -1486,7 +1486,7 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
 	ret = msrpm[t1] & ((1 << param) << t0);
 
 out:
-	nested_svm_unmap(msrpm, KM_USER0);
+	nested_svm_unmap(msrpm);
 
 	return ret;
 }
@@ -1616,7 +1616,7 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
 				       vmcb->control.exit_int_info,
 				       vmcb->control.exit_int_info_err);
 
-	nested_vmcb = nested_svm_map(svm, svm->nested.vmcb, KM_USER0);
+	nested_vmcb = nested_svm_map(svm, svm->nested.vmcb);
 	if (!nested_vmcb)
 		return 1;
 
@@ -1706,7 +1706,7 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
 	/* Exit nested SVM mode */
 	svm->nested.vmcb = 0;
 
-	nested_svm_unmap(nested_vmcb, KM_USER0);
+	nested_svm_unmap(nested_vmcb);
 
 	kvm_mmu_reset_context(&svm->vcpu);
 	kvm_mmu_load(&svm->vcpu);
@@ -1719,7 +1719,7 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 	u32 *nested_msrpm;
 	int i;
 
-	nested_msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm, KM_USER0);
+	nested_msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm);
 	if (!nested_msrpm)
 		return false;
 
@@ -1728,7 +1728,7 @@ static bool nested_svm_vmrun_msrpm(struct vcpu_svm *svm)
 
 	svm->vmcb->control.msrpm_base_pa = __pa(svm->nested.msrpm);
 
-	nested_svm_unmap(nested_msrpm, KM_USER0);
+	nested_svm_unmap(nested_msrpm);
 
 	return true;
 }
@@ -1739,7 +1739,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	struct vmcb *hsave = svm->nested.hsave;
 	struct vmcb *vmcb = svm->vmcb;
 
-	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax, KM_USER0);
+	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax);
 	if (!nested_vmcb)
 		return false;
 
@@ -1851,7 +1851,7 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	svm->vmcb->control.event_inj = nested_vmcb->control.event_inj;
 	svm->vmcb->control.event_inj_err = nested_vmcb->control.event_inj_err;
 
-	nested_svm_unmap(nested_vmcb, KM_USER0);
+	nested_svm_unmap(nested_vmcb);
 
 	enable_gif(svm);
 
@@ -1884,12 +1884,12 @@ static int vmload_interception(struct vcpu_svm *svm)
 	svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
 	skip_emulated_instruction(&svm->vcpu);
 
-	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax, KM_USER0);
+	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax);
 	if (!nested_vmcb)
 		return 1;
 
 	nested_svm_vmloadsave(nested_vmcb, svm->vmcb);
-	nested_svm_unmap(nested_vmcb, KM_USER0);
+	nested_svm_unmap(nested_vmcb);
 
 	return 1;
 }
@@ -1904,12 +1904,12 @@ static int vmsave_interception(struct vcpu_svm *svm)
 	svm->next_rip = kvm_rip_read(&svm->vcpu) + 3;
 	skip_emulated_instruction(&svm->vcpu);
 
-	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax, KM_USER0);
+	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax);
 	if (!nested_vmcb)
 		return 1;
 
 	nested_svm_vmloadsave(svm->vmcb, nested_vmcb);
-	nested_svm_unmap(nested_vmcb, KM_USER0);
+	nested_svm_unmap(nested_vmcb);
 
 	return 1;
 }
-- 
1.6.6

[PATCH 02/10] KVM: SVM: Fix wrong interrupt injection in enable_irq_windows

[Joerg Roedel]

The nested_svm_intr() function does not execute the vmexit
anymore. Therefore we may still be in the nested state after
that function ran. This patch changes the nested_svm_intr()
function to return wether the irq window could be enabled.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 041ef6f..7c96b8b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1389,16 +1389,17 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 	return nested_svm_exit_handled(svm);
 }
 
-static inline int nested_svm_intr(struct vcpu_svm *svm)
+/* This function returns true if it is save to enable the irq window */
+static inline bool nested_svm_intr(struct vcpu_svm *svm)
 {
 	if (!is_nested(svm))
-		return 0;
+		return true;
 
 	if (!(svm->vcpu.arch.hflags & HF_VINTR_MASK))
-		return 0;
+		return true;
 
 	if (!(svm->vcpu.arch.hflags & HF_HIF_MASK))
-		return 0;
+		return false;
 
 	svm->vmcb->control.exit_code = SVM_EXIT_INTR;
 
@@ -1411,10 +1412,10 @@ static inline int nested_svm_intr(struct vcpu_svm *svm)
 		 */
 		svm->nested.exit_required = true;
 		trace_kvm_nested_intr_vmexit(svm->vmcb->save.rip);
-		return 1;
+		return false;
 	}
 
-	return 0;
+	return true;
 }
 
 static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa)
@@ -2562,13 +2563,11 @@ static void enable_irq_window(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	nested_svm_intr(svm);
-
 	/* In case GIF=0 we can't rely on the CPU to tell us when
 	 * GIF becomes 1, because that's a separate STGI/VMRUN intercept.
 	 * The next time we get that intercept, this function will be
 	 * called again though and we'll get the vintr intercept. */
-	if (gif_set(svm)) {
+	if (gif_set(svm) && nested_svm_intr(svm)) {
 		svm_set_vintr(svm);
 		svm_inject_irq(svm, 0x0);
 	}
-- 
1.6.6

[PATCH 03/10] KVM: SVM: Fix schedule-while-atomic on nested exception handling

[Joerg Roedel]

Move the actual vmexit routine out of code that runs with
irqs and preemption disabled.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 7c96b8b..25d26ec 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -128,6 +128,7 @@ static void svm_flush_tlb(struct kvm_vcpu *vcpu);
 static void svm_complete_interrupts(struct vcpu_svm *svm);
 
 static int nested_svm_exit_handled(struct vcpu_svm *svm);
+static int nested_svm_exit_handled_atomic(struct vcpu_svm *svm);
 static int nested_svm_vmexit(struct vcpu_svm *svm);
 static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 				      bool has_error_code, u32 error_code);
@@ -1386,7 +1387,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 	svm->vmcb->control.exit_info_1 = error_code;
 	svm->vmcb->control.exit_info_2 = svm->vcpu.arch.cr2;
 
-	return nested_svm_exit_handled(svm);
+	return nested_svm_exit_handled_atomic(svm);
 }
 
 /* This function returns true if it is save to enable the irq window */
@@ -1520,7 +1521,7 @@ static int nested_svm_exit_special(struct vcpu_svm *svm)
 /*
  * If this function returns true, this #vmexit was already handled
  */
-static int nested_svm_exit_handled(struct vcpu_svm *svm)
+static int __nested_svm_exit_handled(struct vcpu_svm *svm, bool atomic)
 {
 	u32 exit_code = svm->vmcb->control.exit_code;
 	int vmexit = NESTED_EXIT_HOST;
@@ -1567,12 +1568,25 @@ static int nested_svm_exit_handled(struct vcpu_svm *svm)
 	}
 
 	if (vmexit == NESTED_EXIT_DONE) {
-		nested_svm_vmexit(svm);
+		if (!atomic)
+			nested_svm_vmexit(svm);
+		else
+			svm->nested.exit_required = true;
 	}
 
 	return vmexit;
 }
 
+static int nested_svm_exit_handled(struct vcpu_svm *svm)
+{
+	return __nested_svm_exit_handled(svm, false);
+}
+
+static int nested_svm_exit_handled_atomic(struct vcpu_svm *svm)
+{
+	return __nested_svm_exit_handled(svm, true);
+}
+
 static inline void copy_vmcb_control_area(struct vmcb *dst_vmcb, struct vmcb *from_vmcb)
 {
 	struct vmcb_control_area *dst  = &dst_vmcb->control;
-- 
1.6.6

[PATCH 04/10] KVM: SVM: Sync all control registers on nested vmexit

[Joerg Roedel]

Currently the vmexit emulation does not sync control
registers were the access is typically intercepted by the
nested hypervisor. But we can not count on that intercepts
to sync these registers too and make the code
architecturally more correct.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 25d26ec..9a4f9ee 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1644,9 +1644,13 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
 	nested_vmcb->save.ds     = vmcb->save.ds;
 	nested_vmcb->save.gdtr   = vmcb->save.gdtr;
 	nested_vmcb->save.idtr   = vmcb->save.idtr;
+	nested_vmcb->save.cr0    = kvm_read_cr0(&svm->vcpu);
 	if (npt_enabled)
 		nested_vmcb->save.cr3    = vmcb->save.cr3;
+	else
+		nested_vmcb->save.cr3    = svm->vcpu.arch.cr3;
 	nested_vmcb->save.cr2    = vmcb->save.cr2;
+	nested_vmcb->save.cr4    = svm->vcpu.arch.cr4;
 	nested_vmcb->save.rflags = vmcb->save.rflags;
 	nested_vmcb->save.rip    = vmcb->save.rip;
 	nested_vmcb->save.rsp    = vmcb->save.rsp;
-- 
1.6.6

[PATCH 05/10] KVM: SVM: Annotate nested_svm_map with might_sleep()

[Joerg Roedel]

The nested_svm_map() function can sleep and must not be
called from atomic context. So annotate that function.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 9a4f9ee..3f59cbd 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1423,6 +1423,8 @@ static void *nested_svm_map(struct vcpu_svm *svm, u64 gpa)
 {
 	struct page *page;
 
+	might_sleep();
+
 	page = gfn_to_page(svm->vcpu.kvm, gpa >> PAGE_SHIFT);
 	if (is_error_page(page))
 		goto error;
-- 
1.6.6

[PATCH 06/10] KVM: SVM: Fix nested msr intercept handling

[Joerg Roedel]

The nested_svm_exit_handled_msr() function maps only one
page of the guests msr permission bitmap. This patch changes
the code to use kvm_read_guest to fix the bug.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   12 +++---------
 1 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 3f59cbd..cbf798f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1457,16 +1457,11 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
 	u32 msr = svm->vcpu.arch.regs[VCPU_REGS_RCX];
 	bool ret = false;
 	u32 t0, t1;
-	u8 *msrpm;
+	u8 val;
 
 	if (!(svm->nested.intercept & (1ULL << INTERCEPT_MSR_PROT)))
 		return false;
 
-	msrpm = nested_svm_map(svm, svm->nested.vmcb_msrpm);
-
-	if (!msrpm)
-		goto out;
-
 	switch (msr) {
 	case 0 ... 0x1fff:
 		t0 = (msr * 2) % 8;
@@ -1487,11 +1482,10 @@ static bool nested_svm_exit_handled_msr(struct vcpu_svm *svm)
 		goto out;
 	}
 
-	ret = msrpm[t1] & ((1 << param) << t0);
+	if (!kvm_read_guest(svm->vcpu.kvm, svm->nested.vmcb_msrpm + t1, &val, 1))
+		ret = val & ((1 << param) << t0);
 
 out:
-	nested_svm_unmap(msrpm);
-
 	return ret;
 }
 
-- 
1.6.6

[PATCH 07/10] KVM: SVM: Don't sync nested cr8 to lapic and back

[Joerg Roedel]

This patch makes syncing of the guest tpr to the lapic
conditional on !nested. Otherwise a nested guest using the
TPR could freeze the guest.
Another important change this patch introduces is that the
cr8 intercept bits are no longer ORed at vmrun emulation if
the guest sets VINTR_MASKING in its VMCB. The reason is that
nested cr8 accesses need alway be handled by the nested
hypervisor because they change the shadow version of the
tpr.

Cc: stable@kernel.org
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   46 +++++++++++++++++++++++++++++++---------------
 1 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index cbf798f..2a3d525 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1828,21 +1828,6 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	svm->vmcb->save.dr6 = nested_vmcb->save.dr6;
 	svm->vmcb->save.cpl = nested_vmcb->save.cpl;
 
-	/* We don't want a nested guest to be more powerful than the guest,
-	   so all intercepts are ORed */
-	svm->vmcb->control.intercept_cr_read |=
-		nested_vmcb->control.intercept_cr_read;
-	svm->vmcb->control.intercept_cr_write |=
-		nested_vmcb->control.intercept_cr_write;
-	svm->vmcb->control.intercept_dr_read |=
-		nested_vmcb->control.intercept_dr_read;
-	svm->vmcb->control.intercept_dr_write |=
-		nested_vmcb->control.intercept_dr_write;
-	svm->vmcb->control.intercept_exceptions |=
-		nested_vmcb->control.intercept_exceptions;
-
-	svm->vmcb->control.intercept |= nested_vmcb->control.intercept;
-
 	svm->nested.vmcb_msrpm = nested_vmcb->control.msrpm_base_pa;
 
 	/* cache intercepts */
@@ -1860,6 +1845,28 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	else
 		svm->vcpu.arch.hflags &= ~HF_VINTR_MASK;
 
+	if (svm->vcpu.arch.hflags & HF_VINTR_MASK) {
+		/* We only want the cr8 intercept bits of the guest */
+		svm->vmcb->control.intercept_cr_read &= ~INTERCEPT_CR8_MASK;
+		svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR8_MASK;
+	}
+
+	/* We don't want a nested guest to be more powerful than the guest,
+	   so all intercepts are ORed */
+	svm->vmcb->control.intercept_cr_read |=
+		nested_vmcb->control.intercept_cr_read;
+	svm->vmcb->control.intercept_cr_write |=
+		nested_vmcb->control.intercept_cr_write;
+	svm->vmcb->control.intercept_dr_read |=
+		nested_vmcb->control.intercept_dr_read;
+	svm->vmcb->control.intercept_dr_write |=
+		nested_vmcb->control.intercept_dr_write;
+	svm->vmcb->control.intercept_exceptions |=
+		nested_vmcb->control.intercept_exceptions;
+
+	svm->vmcb->control.intercept |= nested_vmcb->control.intercept;
+
+	svm->vmcb->control.lbr_ctl = nested_vmcb->control.lbr_ctl;
 	svm->vmcb->control.int_vector = nested_vmcb->control.int_vector;
 	svm->vmcb->control.int_state = nested_vmcb->control.int_state;
 	svm->vmcb->control.tsc_offset += nested_vmcb->control.tsc_offset;
@@ -2520,6 +2527,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
+	if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK))
+		return;
+
 	if (irr == -1)
 		return;
 
@@ -2621,6 +2631,9 @@ static inline void sync_cr8_to_lapic(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
+	if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK))
+		return;
+
 	if (!(svm->vmcb->control.intercept_cr_write & INTERCEPT_CR8_MASK)) {
 		int cr8 = svm->vmcb->control.int_ctl & V_TPR_MASK;
 		kvm_set_cr8(vcpu, cr8);
@@ -2632,6 +2645,9 @@ static inline void sync_lapic_to_cr8(struct kvm_vcpu *vcpu)
 	struct vcpu_svm *svm = to_svm(vcpu);
 	u64 cr8;
 
+	if (is_nested(svm) && (vcpu->arch.hflags & HF_VINTR_MASK))
+		return;
+
 	cr8 = kvm_get_cr8(vcpu);
 	svm->vmcb->control.int_ctl &= ~V_TPR_MASK;
 	svm->vmcb->control.int_ctl |= cr8 & V_TPR_MASK;
-- 
1.6.6

[PATCH 08/10] KVM: SVM: Activate nested state only when guest state is complete

[Joerg Roedel]

Certain functions called during the emulated world switch
behave differently when the vcpu is running nested. This is
not the expected behavior during a world switch emulation.
This patch ensures that the nested state is activated only
if the vcpu is completly in nested state.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2a3d525..a64b871 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1631,6 +1631,9 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
 	if (!nested_vmcb)
 		return 1;
 
+	/* Exit nested SVM mode */
+	svm->nested.vmcb = 0;
+
 	/* Give the current vmcb to the guest */
 	disable_gif(svm);
 
@@ -1718,9 +1721,6 @@ static int nested_svm_vmexit(struct vcpu_svm *svm)
 	svm->vmcb->save.cpl = 0;
 	svm->vmcb->control.exit_int_info = 0;
 
-	/* Exit nested SVM mode */
-	svm->nested.vmcb = 0;
-
 	nested_svm_unmap(nested_vmcb);
 
 	kvm_mmu_reset_context(&svm->vcpu);
@@ -1753,14 +1753,14 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 	struct vmcb *nested_vmcb;
 	struct vmcb *hsave = svm->nested.hsave;
 	struct vmcb *vmcb = svm->vmcb;
+	u64 vmcb_gpa;
+
+	vmcb_gpa = svm->vmcb->save.rax;
 
 	nested_vmcb = nested_svm_map(svm, svm->vmcb->save.rax);
 	if (!nested_vmcb)
 		return false;
 
-	/* nested_vmcb is our indicator if nested SVM is activated */
-	svm->nested.vmcb = svm->vmcb->save.rax;
-
 	trace_kvm_nested_vmrun(svm->vmcb->save.rip - 3, svm->nested.vmcb,
 			       nested_vmcb->save.rip,
 			       nested_vmcb->control.int_ctl,
@@ -1875,6 +1875,9 @@ static bool nested_svm_vmrun(struct vcpu_svm *svm)
 
 	nested_svm_unmap(nested_vmcb);
 
+	/* nested_vmcb is our indicator if nested SVM is activated */
+	svm->nested.vmcb = vmcb_gpa;
+
 	enable_gif(svm);
 
 	return true;
-- 
1.6.6

[PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Joerg Roedel]

TDB.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/svm.c |   43 +++++++++++++++++++++++++++++++++++++++----
 1 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index a64b871..ad419aa 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -973,6 +973,7 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)
 
 static void update_cr0_intercept(struct vcpu_svm *svm)
 {
+	struct vmcb *vmcb = svm->vmcb;
 	ulong gcr0 = svm->vcpu.arch.cr0;
 	u64 *hcr0 = &svm->vmcb->save.cr0;
 
@@ -984,11 +985,25 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
 
 
 	if (gcr0 == *hcr0 && svm->vcpu.fpu_active) {
-		svm->vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
-		svm->vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
+		vmcb->control.intercept_cr_read &= ~INTERCEPT_CR0_MASK;
+		vmcb->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
+		if (is_nested(svm)) {
+			struct vmcb *hsave = svm->nested.hsave;
+
+			hsave->control.intercept_cr_read  &= ~INTERCEPT_CR0_MASK;
+			hsave->control.intercept_cr_write &= ~INTERCEPT_CR0_MASK;
+			vmcb->control.intercept_cr_read  |= svm->nested.intercept_cr_read;
+			vmcb->control.intercept_cr_write |= svm->nested.intercept_cr_write;
+		}
 	} else {
 		svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
 		svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
+		if (is_nested(svm)) {
+			struct vmcb *hsave = svm->nested.hsave;
+
+			hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
+			hsave->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
+		}
 	}
 }
 
@@ -1263,7 +1278,22 @@ static int ud_interception(struct vcpu_svm *svm)
 static void svm_fpu_activate(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
-	svm->vmcb->control.intercept_exceptions &= ~(1 << NM_VECTOR);
+	u32 excp;
+
+	if (is_nested(svm)) {
+		u32 h_excp, n_excp;
+
+		h_excp  = svm->nested.hsave->control.intercept_exceptions;
+		n_excp  = svm->nested.intercept_exceptions;
+		h_excp &= ~(1 << NM_VECTOR);
+		excp    = h_excp | n_excp;
+	} else {
+		excp  = svm->vmcb->control.intercept_exceptions;
+	        excp &= ~(1 << NM_VECTOR);
+	}
+
+	svm->vmcb->control.intercept_exceptions = excp;
+
 	svm->vcpu.fpu_active = 1;
 	update_cr0_intercept(svm);
 }
@@ -1507,6 +1537,9 @@ static int nested_svm_exit_special(struct vcpu_svm *svm)
 		if (!npt_enabled)
 			return NESTED_EXIT_HOST;
 		break;
+	case SVM_EXIT_EXCP_BASE + NM_VECTOR:
+		nm_interception(svm);
+		break;
 	default:
 		break;
 	}
@@ -2972,8 +3005,10 @@ static void svm_fpu_deactivate(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
 
-	update_cr0_intercept(svm);
 	svm->vmcb->control.intercept_exceptions |= 1 << NM_VECTOR;
+	if (is_nested(svm))
+		svm->nested.hsave->control.intercept_exceptions |= 1 << NM_VECTOR;
+	update_cr0_intercept(svm);
 }
 
 static struct kvm_x86_ops svm_x86_ops = {
-- 
1.6.6

[PATCH 10/10] KVM: SVM: Remove newlines from nested trace points

[Joerg Roedel]

The tracing infrastructure adds its own newlines. Remove
them from the trace point printk format strings.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/trace.h |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 6ad30a2..12f8d2d 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -413,7 +413,7 @@ TRACE_EVENT(kvm_nested_vmrun,
 	),
 
 	TP_printk("rip: 0x%016llx vmcb: 0x%016llx nrip: 0x%016llx int_ctl: 0x%08x "
-		  "event_inj: 0x%08x npt: %s\n",
+		  "event_inj: 0x%08x npt: %s",
 		__entry->rip, __entry->vmcb, __entry->nested_rip,
 		__entry->int_ctl, __entry->event_inj,
 		__entry->npt ? "on" : "off")
@@ -447,7 +447,7 @@ TRACE_EVENT(kvm_nested_vmexit,
 		__entry->exit_int_info_err	= exit_int_info_err;
 	),
 	TP_printk("rip: 0x%016llx reason: %s ext_inf1: 0x%016llx "
-		  "ext_inf2: 0x%016llx ext_int: 0x%08x ext_int_err: 0x%08x\n",
+		  "ext_inf2: 0x%016llx ext_int: 0x%08x ext_int_err: 0x%08x",
 		  __entry->rip,
 		  ftrace_print_symbols_seq(p, __entry->exit_code,
 					   kvm_x86_ops->exit_reasons_str),
@@ -482,7 +482,7 @@ TRACE_EVENT(kvm_nested_vmexit_inject,
 	),
 
 	TP_printk("reason: %s ext_inf1: 0x%016llx "
-		  "ext_inf2: 0x%016llx ext_int: 0x%08x ext_int_err: 0x%08x\n",
+		  "ext_inf2: 0x%016llx ext_int: 0x%08x ext_int_err: 0x%08x",
 		  ftrace_print_symbols_seq(p, __entry->exit_code,
 					   kvm_x86_ops->exit_reasons_str),
 		__entry->exit_info1, __entry->exit_info2,
@@ -504,7 +504,7 @@ TRACE_EVENT(kvm_nested_intr_vmexit,
 		__entry->rip	=	rip
 	),
 
-	TP_printk("rip: 0x%016llx\n", __entry->rip)
+	TP_printk("rip: 0x%016llx", __entry->rip)
 );
 
 /*
@@ -526,7 +526,7 @@ TRACE_EVENT(kvm_invlpga,
 		__entry->address	=	address;
 	),
 
-	TP_printk("rip: 0x%016llx asid: %d address: 0x%016llx\n",
+	TP_printk("rip: 0x%016llx asid: %d address: 0x%016llx",
 		  __entry->rip, __entry->asid, __entry->address)
 );
 
@@ -547,7 +547,7 @@ TRACE_EVENT(kvm_skinit,
 		__entry->slb		=	slb;
 	),
 
-	TP_printk("rip: 0x%016llx slb: 0x%08x\n",
+	TP_printk("rip: 0x%016llx slb: 0x%08x",
 		  __entry->rip, __entry->slb)
 );
 
-- 
1.6.6

Re: [PATCH 01/10] KVM: SVM: Don't use kmap_atomic in nested_svm_map

[Avi Kivity]

On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> Use of kmap_atomic disables preemption but if we run in
> shadow-shadow mode the vmrun emulation executes kvm_set_cr3
> which might sleep or fault. So use kmap instead for
> nested_svm_map.
>
>
>
> -static void nested_svm_unmap(void *addr, enum km_type idx)
> +static void nested_svm_unmap(void *addr)
>   {
>   	struct page *page;
>
> @@ -1443,7 +1443,7 @@ static void nested_svm_unmap(void *addr, enum km_type idx)
>
>   	page = kmap_atomic_to_page(addr);
>
> -	kunmap_atomic(addr, idx);
> +	kunmap(addr);
>   	kvm_release_page_dirty(page);
>   }
>    

kunmap() takes a struct page *, not the virtual address (a consistent 
source of bugs).

kmap() is generally an unloved interface, it is slow and possibly 
deadlock prone, but it's better than sleeping in atomic context.  If you 
can hack your way around it, that is preferred.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 03/10] KVM: SVM: Fix schedule-while-atomic on nested exception handling

[Avi Kivity]

On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> Move the actual vmexit routine out of code that runs with
> irqs and preemption disabled.
>
> Cc: stable@kernel.org
> Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> ---
>   arch/x86/kvm/svm.c |   20 +++++++++++++++++---
>   1 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 7c96b8b..25d26ec 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -128,6 +128,7 @@ static void svm_flush_tlb(struct kvm_vcpu *vcpu);
>   static void svm_complete_interrupts(struct vcpu_svm *svm);
>
>   static int nested_svm_exit_handled(struct vcpu_svm *svm);
> +static int nested_svm_exit_handled_atomic(struct vcpu_svm *svm);
>   static int nested_svm_vmexit(struct vcpu_svm *svm);
>   static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
>   				      bool has_error_code, u32 error_code);
> @@ -1386,7 +1387,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
>   	svm->vmcb->control.exit_info_1 = error_code;
>   	svm->vmcb->control.exit_info_2 = svm->vcpu.arch.cr2;
>
> -	return nested_svm_exit_handled(svm);
> +	return nested_svm_exit_handled_atomic(svm);
>   }
>    

What do you say to


    if (nested_svm_intercepts(svm))
         svm->nested.exit_required = true;

here, and recoding nested_svm_exit_handled() to call 
nested_svm_intercepts()?  I think it improves readability a little by 
avoiding a function that changes behaviour according to how it is called.

Long term, we may want to split out the big switch into the individual 
handlers, to avoid decoding the exit reason twice.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Avi Kivity]

On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> TDB.
>
>    

...

> @@ -973,6 +973,7 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)
>
>   static void update_cr0_intercept(struct vcpu_svm *svm)
>   {
> +	struct vmcb *vmcb = svm->vmcb;
>   	ulong gcr0 = svm->vcpu.arch.cr0;
>   	u64 *hcr0 =&svm->vmcb->save.cr0;
>
> @@ -984,11 +985,25 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
>
>
>   	if (gcr0 == *hcr0&&  svm->vcpu.fpu_active) {
> -		svm->vmcb->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> -		svm->vmcb->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> +		vmcb->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> +		vmcb->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> +		if (is_nested(svm)) {
> +			struct vmcb *hsave = svm->nested.hsave;
> +
> +			hsave->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> +			hsave->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> +			vmcb->control.intercept_cr_read  |= svm->nested.intercept_cr_read;
> +			vmcb->control.intercept_cr_write |= svm->nested.intercept_cr_write;
>    

Why are the last two lines needed?

> +		}
>   	} else {
>   		svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
>   		svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
> +		if (is_nested(svm)) {
> +			struct vmcb *hsave = svm->nested.hsave;
> +
> +			hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
> +			hsave->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
> +		}
>   	}
>   }
>    

Maybe it's better to call update_cr0_intercept() after a vmexit instead, 
to avoid this repetition, and since the if () may take a different 
branch for the nested guest and guest cr0.


-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[Avi Kivity]

On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> Hi,
>
> here is a couple of fixes for the nested SVM implementation. I collected these
> fixes mostly when trying to get Windows 7 64bit running as an L2 guest. Most
> important fixes in this set make lazy fpu switching working with nested SVM and
> the nested tpr handling fixes. Without the later fix the l1 guest freezes when
> trying to run win7 as l2 guest. Please review and comment on these patches :-)
>    

Overall looks good.  Would appreciate Alex looking over these as well.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[Alexander Graf]

On 18.02.2010, at 15:33, Avi Kivity wrote:

> On 02/18/2010 01:38 PM, Joerg Roedel wrote:
>> Hi,
>> 
>> here is a couple of fixes for the nested SVM implementation. I collected these
>> fixes mostly when trying to get Windows 7 64bit running as an L2 guest. Most
>> important fixes in this set make lazy fpu switching working with nested SVM and
>> the nested tpr handling fixes. Without the later fix the l1 guest freezes when
>> trying to run win7 as l2 guest. Please review and comment on these patches :-)
>>   
> 
> Overall looks good.  Would appreciate Alex looking over these as well.

The kmap thing is broken though, right?

Alex

Re: [PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Alexander Graf]

On 18.02.2010, at 12:38, Joerg Roedel wrote:

> TDB.

TDB? That's not a patch description.


Alex

Re: [PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[Avi Kivity]

On 02/18/2010 04:48 PM, Alexander Graf wrote:
> On 18.02.2010, at 15:33, Avi Kivity wrote:
>
>    
>> On 02/18/2010 01:38 PM, Joerg Roedel wrote:
>>      
>>> Hi,
>>>
>>> here is a couple of fixes for the nested SVM implementation. I collected these
>>> fixes mostly when trying to get Windows 7 64bit running as an L2 guest. Most
>>> important fixes in this set make lazy fpu switching working with nested SVM and
>>> the nested tpr handling fixes. Without the later fix the l1 guest freezes when
>>> trying to run win7 as l2 guest. Please review and comment on these patches :-)
>>>
>>>        
>> Overall looks good.  Would appreciate Alex looking over these as well.
>>      
> The kmap thing is broken though, right?
>    

Oh yes, but that's a search and replace, not something needing deep rework.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Avi Kivity]

On 02/18/2010 04:51 PM, Alexander Graf wrote:
> On 18.02.2010, at 12:38, Joerg Roedel wrote:
>
>    
>> TDB.
>>      
> TDB? That's not a patch description.
>
>    

Short for "To De Befined", I presume.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 01/10] KVM: SVM: Don't use kmap_atomic in nested_svm_map

[Joerg Roedel]

On Thu, Feb 18, 2010 at 03:40:56PM +0200, Avi Kivity wrote:
> On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> >Use of kmap_atomic disables preemption but if we run in
> >shadow-shadow mode the vmrun emulation executes kvm_set_cr3
> >which might sleep or fault. So use kmap instead for
> >nested_svm_map.
> >
> >
> >
> >-static void nested_svm_unmap(void *addr, enum km_type idx)
> >+static void nested_svm_unmap(void *addr)
> >  {
> >  	struct page *page;
> >
> >@@ -1443,7 +1443,7 @@ static void nested_svm_unmap(void *addr, enum km_type idx)
> >
> >  	page = kmap_atomic_to_page(addr);
> >
> >-	kunmap_atomic(addr, idx);
> >+	kunmap(addr);
> >  	kvm_release_page_dirty(page);
> >  }
> 
> kunmap() takes a struct page *, not the virtual address (a
> consistent source of bugs).

Ah true, thanks. I'll fix that.

> kmap() is generally an unloved interface, it is slow and possibly
> deadlock prone, but it's better than sleeping in atomic context.  If
> you can hack your way around it, that is preferred.

Best would be to use kvm_read_guest, but I fear that this will have an
performance impact. Maybe I'll try this and measure if it really has a
significant performance impact.

	Joerg

Re: [PATCH 03/10] KVM: SVM: Fix schedule-while-atomic on nested exception handling

[Joerg Roedel]

On Thu, Feb 18, 2010 at 03:52:20PM +0200, Avi Kivity wrote:
> On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> >Move the actual vmexit routine out of code that runs with
> >irqs and preemption disabled.
> >
> >Cc: stable@kernel.org
> >Signed-off-by: Joerg Roedel<joerg.roedel@amd.com>
> >---
> >  arch/x86/kvm/svm.c |   20 +++++++++++++++++---
> >  1 files changed, 17 insertions(+), 3 deletions(-)
> >
> >diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> >index 7c96b8b..25d26ec 100644
> >--- a/arch/x86/kvm/svm.c
> >+++ b/arch/x86/kvm/svm.c
> >@@ -128,6 +128,7 @@ static void svm_flush_tlb(struct kvm_vcpu *vcpu);
> >  static void svm_complete_interrupts(struct vcpu_svm *svm);
> >
> >  static int nested_svm_exit_handled(struct vcpu_svm *svm);
> >+static int nested_svm_exit_handled_atomic(struct vcpu_svm *svm);
> >  static int nested_svm_vmexit(struct vcpu_svm *svm);
> >  static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
> >  				      bool has_error_code, u32 error_code);
> >@@ -1386,7 +1387,7 @@ static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
> >  	svm->vmcb->control.exit_info_1 = error_code;
> >  	svm->vmcb->control.exit_info_2 = svm->vcpu.arch.cr2;
> >
> >-	return nested_svm_exit_handled(svm);
> >+	return nested_svm_exit_handled_atomic(svm);
> >  }
> 
> What do you say to
> 
> 
>    if (nested_svm_intercepts(svm))
>         svm->nested.exit_required = true;
> 
> here, and recoding nested_svm_exit_handled() to call
> nested_svm_intercepts()?  I think it improves readability a little
> by avoiding a function that changes behaviour according to how it is
> called.

Thats a good idea, will change that. It improves readability.

> Long term, we may want to split out the big switch into the
> individual handlers, to avoid decoding the exit reason twice.

I don't think thats a good idea. The nested exit handling is at the
beginning of svm_handle_exit to hide the nested vcpu state from the kvm
logic which may not be aware of nesting at all. My rationale is that the
host hypervisor don't see any exits that needs to be reinjected to the
l1 hypervisor.

	Joerg

Re: [PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Joerg Roedel]

On Thu, Feb 18, 2010 at 04:32:02PM +0200, Avi Kivity wrote:
> On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> >TDB.
> >
> 
> ...
> 
> >@@ -973,6 +973,7 @@ static void svm_decache_cr4_guest_bits(struct kvm_vcpu *vcpu)
> >
> >  static void update_cr0_intercept(struct vcpu_svm *svm)
> >  {
> >+	struct vmcb *vmcb = svm->vmcb;
> >  	ulong gcr0 = svm->vcpu.arch.cr0;
> >  	u64 *hcr0 =&svm->vmcb->save.cr0;
> >
> >@@ -984,11 +985,25 @@ static void update_cr0_intercept(struct vcpu_svm *svm)
> >
> >
> >  	if (gcr0 == *hcr0&&  svm->vcpu.fpu_active) {
> >-		svm->vmcb->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> >-		svm->vmcb->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> >+		vmcb->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> >+		vmcb->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> >+		if (is_nested(svm)) {
> >+			struct vmcb *hsave = svm->nested.hsave;
> >+
> >+			hsave->control.intercept_cr_read&= ~INTERCEPT_CR0_MASK;
> >+			hsave->control.intercept_cr_write&= ~INTERCEPT_CR0_MASK;
> >+			vmcb->control.intercept_cr_read  |= svm->nested.intercept_cr_read;
> >+			vmcb->control.intercept_cr_write |= svm->nested.intercept_cr_write;
> 
> Why are the last two lines needed?

Because we don't know if the l1 hypervisor wants to intercept cr0. In
this case we need this intercept to stay enabled.

> >+		}
> >  	} else {
> >  		svm->vmcb->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
> >  		svm->vmcb->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
> >+		if (is_nested(svm)) {
> >+			struct vmcb *hsave = svm->nested.hsave;
> >+
> >+			hsave->control.intercept_cr_read |= INTERCEPT_CR0_MASK;
> >+			hsave->control.intercept_cr_write |= INTERCEPT_CR0_MASK;
> >+		}
> >  	}
> >  }
> 
> Maybe it's better to call update_cr0_intercept() after a vmexit
> instead, to avoid this repetition, and since the if () may take a
> different branch for the nested guest and guest cr0.

Thinking again about it I am not sure if this is needed at all. At
vmexit emulation we call svm_set_cr0 which itself calls
update_cr0_intercept. I'll try this.

	Joerg

Re: [PATCH 09/10] KVM: SVM: Make lazy FPU switching work with nested svm

[Joerg Roedel]

On Thu, Feb 18, 2010 at 04:55:06PM +0200, Avi Kivity wrote:
> On 02/18/2010 04:51 PM, Alexander Graf wrote:
> >On 18.02.2010, at 12:38, Joerg Roedel wrote:
> >
> >>TDB.
> >TDB? That's not a patch description.
> >
> 
> Short for "To De Befined", I presume.

Ups. I just forgot to give this patch a right commit message. I add one
for the next post.

	Joerg

Re: [PATCH 0/10] Nested SVM fixes (and Win7-64bit bringup)

[Joerg Roedel]

On Thu, Feb 18, 2010 at 04:54:37PM +0200, Avi Kivity wrote:
> On 02/18/2010 04:48 PM, Alexander Graf wrote:
> >On 18.02.2010, at 15:33, Avi Kivity wrote:
> >
> >>On 02/18/2010 01:38 PM, Joerg Roedel wrote:
> >>>Hi,
> >>>
> >>>here is a couple of fixes for the nested SVM implementation. I collected these
> >>>fixes mostly when trying to get Windows 7 64bit running as an L2 guest. Most
> >>>important fixes in this set make lazy fpu switching working with nested SVM and
> >>>the nested tpr handling fixes. Without the later fix the l1 guest freezes when
> >>>trying to run win7 as l2 guest. Please review and comment on these patches :-)
> >>>
> >>Overall looks good.  Would appreciate Alex looking over these as well.
> >The kmap thing is broken though, right?
> 
> Oh yes, but that's a search and replace, not something needing deep rework.

Great. I'll post again with fixes soon.

	Joerg