[PATCH] KVM: detect if VCPU triple faults

[PATCH] KVM: detect if VCPU triple faults

[Joerg Roedel]

In the current inject_page_fault path KVM only checks if there is another PF
pending and injects a DF then. But it has to check for a pendig DF too to
detect a shutdown condition in the VCPU. If this is not detected the VCPU goes
to a PF -> DF -> PF loop when it should triple fault. This patch detects this
condition and handles it with an KVM_SHUTDOWN exit to userspace. As a side
effect it fixes the following warning when trying to reboot a SMP guest on SVM:

--------------------------<snip>---------------------------------------
kvm: inject_page_fault: double fault 0x3f72
WARNING: at /home/jroedel/src/kvm/kvm-userspace/kernel/x86.c:171 kvm_queue_exception_e()
Pid: 12088, comm: qemu-system-x86 Not tainted 2.6.24-rc8 #1

Call Trace:
 [<ffffffff88033328>] :kvm:kvm_queue_exception_e+0x78/0x80
 [<ffffffff8803995c>] :kvm:paging64_page_fault+0xec/0x510
 [<ffffffff8020a0d2>] __switch_to+0x42/0x310
 [<ffffffff805f8785>] _spin_unlock_irq+0x15/0x40
 [<ffffffff80233f1d>] __dequeue_entity+0x3d/0x50
 [<ffffffff8020a0d2>] __switch_to+0x42/0x310
 [<ffffffff80238dc7>] finish_task_switch+0x57/0xb0
 [<ffffffff805f64f0>] thread_return+0x5b/0x53b
 [<ffffffff88038679>] :kvm:kvm_mmu_page_fault+0x19/0x90
 [<ffffffff880351f7>] :kvm:kvm_arch_vcpu_ioctl_run+0x167/0x6e0
 [<ffffffff88030e81>] :kvm:kvm_vcpu_ioctl+0x351/0x360
 [<ffffffff80281d54>] find_extend_vma+0x24/0x80
 [<ffffffff8025f153>] get_futex_key+0x53/0x160
 [<ffffffff80399771>] __up_read+0x21/0xb0
 [<ffffffff805f8746>] _spin_unlock_irqrestore+0x16/0x40
 [<ffffffff8025fb6f>] futex_wake+0xcf/0xf0
 [<ffffffff80260f9b>] do_futex+0x66b/0xbf0
 [<ffffffff8026e1cb>] find_get_pages_tag+0x8b/0xb0
 [<ffffffff8024abd9>] __dequeue_signal+0x19/0x1c0
 [<ffffffff8024a67e>] recalc_sigpending+0xe/0x40
 [<ffffffff8024c10d>] dequeue_signal+0x5d/0x160
 [<ffffffff805f8785>] _spin_unlock_irq+0x15/0x40
 [<ffffffff8024d502>] sys_rt_sigtimedwait+0x2e2/0x2f0
 [<ffffffff802a58df>] do_ioctl+0x2f/0xa0
 [<ffffffff802a59c4>] vfs_ioctl+0x74/0x2d0
 [<ffffffff802a5c69>] sys_ioctl+0x49/0x80
 [<ffffffff8020bcfe>] system_call+0x7e/0x83
--------------------------<snip>---------------------------------------

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/mmu.c         |    9 +++++++--
 arch/x86/kvm/paging_tmpl.h |   10 ++++++----
 arch/x86/kvm/x86.c         |   21 ++++++++++++++-------
 include/asm-x86/kvm_host.h |    2 +-
 4 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5c0f0af..2906ee5 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1394,11 +1394,11 @@ static void paging_new_cr3(struct kvm_vcpu *vcpu)
 	mmu_free_roots(vcpu);
 }
 
-static void inject_page_fault(struct kvm_vcpu *vcpu,
+static bool inject_page_fault(struct kvm_vcpu *vcpu,
 			      u64 addr,
 			      u32 err_code)
 {
-	kvm_inject_page_fault(vcpu, addr, err_code);
+	return kvm_inject_page_fault(vcpu, addr, err_code);
 }
 
 static void paging_free(struct kvm_vcpu *vcpu)
@@ -1815,6 +1815,11 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u32 error_code)
 		goto out;
 	}
 
+	if (r == 2) {
+		vcpu->run->exit_reason = KVM_EXIT_SHUTDOWN;
+		return 0;
+	}
+
 	r = mmu_topup_memory_caches(vcpu);
 	if (r)
 		goto out;
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 1af0ceb..01708b7 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -366,8 +366,9 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
  *   - normal guest page fault due to the guest pte marked not present, not
  *     writable, or not executable
  *
- *  Returns: 1 if we need to emulate the instruction, 0 otherwise, or
- *           a negative value on error.
+ *  Returns: 2 if the vcpu triple faulted
+ *	     1 if we need to emulate the instruction
+ *	     0 otherwise, or a negative value on error.
  */
 static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr,
 			       u32 error_code)
@@ -381,6 +382,7 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr,
 	int r;
 	struct page *page;
 	int largepage = 0;
+	bool injected;
 
 	pgprintk("%s: addr %lx err %x\n", __FUNCTION__, addr, error_code);
 	kvm_mmu_audit(vcpu, "pre page fault");
@@ -401,10 +403,10 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, gva_t addr,
 	 */
 	if (!r) {
 		pgprintk("%s: guest page fault\n", __FUNCTION__);
-		inject_page_fault(vcpu, addr, walker.error_code);
+		injected = inject_page_fault(vcpu, addr, walker.error_code);
 		vcpu->arch.last_pt_write_count = 0; /* reset fork detector */
 		up_read(&vcpu->kvm->slots_lock);
-		return 0;
+		return injected ? 0 : 2;
 	}
 
 	down_read(&current->mm->mmap_sem);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e1aa6c9..67b9f10 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -151,19 +151,26 @@ void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr)
 }
 EXPORT_SYMBOL_GPL(kvm_queue_exception);
 
-void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr,
+bool kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr,
 			   u32 error_code)
 {
 	++vcpu->stat.pf_guest;
-	if (vcpu->arch.exception.pending && vcpu->arch.exception.nr == PF_VECTOR) {
-		printk(KERN_DEBUG "kvm: inject_page_fault:"
-		       " double fault 0x%lx\n", addr);
-		vcpu->arch.exception.nr = DF_VECTOR;
-		vcpu->arch.exception.error_code = 0;
-		return;
+	if (vcpu->arch.exception.pending) {
+		if (vcpu->arch.exception.nr == PF_VECTOR) {
+			printk(KERN_DEBUG "kvm: inject_page_fault:"
+					" double fault 0x%lx\n", addr);
+			vcpu->arch.exception.nr = DF_VECTOR;
+			vcpu->arch.exception.error_code = 0;
+			return true;
+		} else if (vcpu->arch.exception.nr == DF_VECTOR) {
+			/* triple fault,-> exit to userspace */
+			return false;
+		}
 	}
+
 	vcpu->arch.cr2 = addr;
 	kvm_queue_exception_e(vcpu, PF_VECTOR, error_code);
+	return true;
 }
 
 void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code)
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 024b57c..3d73df0 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -482,7 +482,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data);
 
 void kvm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr);
 void kvm_queue_exception_e(struct kvm_vcpu *vcpu, unsigned nr, u32 error_code);
-void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long cr2,
+bool kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long cr2,
 			   u32 error_code);
 
 void fx_init(struct kvm_vcpu *vcpu);
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM: detect if VCPU triple faults

[Avi Kivity]

Joerg Roedel wrote:
> In the current inject_page_fault path KVM only checks if there is another PF
> pending and injects a DF then. But it has to check for a pendig DF too to
> detect a shutdown condition in the VCPU. If this is not detected the VCPU goes
> to a PF -> DF -> PF loop when it should triple fault. This patch detects this
> condition and handles it with an KVM_SHUTDOWN exit to userspace. As a side
> effect it fixes the following warning when trying to reboot a SMP guest on SVM:
> diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> index 1af0ceb..01708b7 100644
> --- a/arch/x86/kvm/paging_tmpl.h
> +++ b/arch/x86/kvm/paging_tmpl.h
> @@ -366,8 +366,9 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
>   *   - normal guest page fault due to the guest pte marked not present, not
>   *     writable, or not executable
>   *
> - *  Returns: 1 if we need to emulate the instruction, 0 otherwise, or
> - *           a negative value on error.
> + *  Returns: 2 if the vcpu triple faulted
> + *	     1 if we need to emulate the instruction
> + *	     0 otherwise, or a negative value on error.
>   */
>   

This is a little icky.  What about setting vcpu->requests bit 
KVM_REQ_TRIPLE_FAULT and checking it on the next entry?

The check is zero cost since we check vcpu->requests anyway.  We can use 
this for other situations as well (like setting cr3 to mmio space, bad 
paravirt mmio operation, etc.)

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM: detect if VCPU triple faults

[Joerg Roedel]

On Tue, Feb 26, 2008 at 03:02:31PM +0200, Avi Kivity wrote:
> Joerg Roedel wrote:
> >In the current inject_page_fault path KVM only checks if there is another PF
> >pending and injects a DF then. But it has to check for a pendig DF too to
> >detect a shutdown condition in the VCPU. If this is not detected the VCPU goes
> >to a PF -> DF -> PF loop when it should triple fault. This patch detects this
> >condition and handles it with an KVM_SHUTDOWN exit to userspace. As a side
> >effect it fixes the following warning when trying to reboot a SMP guest on SVM:
> >diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
> >index 1af0ceb..01708b7 100644
> >--- a/arch/x86/kvm/paging_tmpl.h
> >+++ b/arch/x86/kvm/paging_tmpl.h
> >@@ -366,8 +366,9 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
> >  *   - normal guest page fault due to the guest pte marked not present, not
> >  *     writable, or not executable
> >  *
> >- *  Returns: 1 if we need to emulate the instruction, 0 otherwise, or
> >- *           a negative value on error.
> >+ *  Returns: 2 if the vcpu triple faulted
> >+ *	     1 if we need to emulate the instruction
> >+ *	     0 otherwise, or a negative value on error.
> >  */
> >  
> 
> This is a little icky.  What about setting vcpu->requests bit KVM_REQ_TRIPLE_FAULT and checking it on the next entry?
> 
> The check is zero cost since we check vcpu->requests anyway.  We can use this for other situations as well (like setting cr3 to mmio space, bad paravirt mmio operation, etc.)

Ah, right. This will be much simpler and cleaner. I will send an updated
patch.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[PATCH] KVM: detect if VCPU triple faults

[Joerg Roedel]

In the current inject_page_fault path KVM only checks if there is another PF
pending and injects a DF then. But it has to check for a pendig DF too to
detect a shutdown condition in the VCPU. If this is not detected the VCPU goes
to a PF -> DF -> PF loop when it should triple fault. This patch detects this
condition and handles it with an KVM_SHUTDOWN exit to userspace.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/x86.c       |   20 +++++++++++++++-----
 include/linux/kvm_host.h |    1 +
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff7ef12..e31c6ff 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -155,11 +155,16 @@ void kvm_inject_page_fault(struct kvm_vcpu *vcpu, unsigned long addr,
 			   u32 error_code)
 {
 	++vcpu->stat.pf_guest;
-	if (vcpu->arch.exception.pending && vcpu->arch.exception.nr == PF_VECTOR) {
-		printk(KERN_DEBUG "kvm: inject_page_fault:"
-		       " double fault 0x%lx\n", addr);
-		vcpu->arch.exception.nr = DF_VECTOR;
-		vcpu->arch.exception.error_code = 0;
+	if (vcpu->arch.exception.pending) {
+		if (vcpu->arch.exception.nr == PF_VECTOR) {
+			printk(KERN_DEBUG "kvm: inject_page_fault:"
+					" double fault 0x%lx\n", addr);
+			vcpu->arch.exception.nr = DF_VECTOR;
+			vcpu->arch.exception.error_code = 0;
+		} else if (vcpu->arch.exception.nr == DF_VECTOR) {
+			/* triple fault -> shutdown */
+			set_bit(&vcpu->requests, KVM_REQ_TRIPLE_FAULT);
+		}
 		return;
 	}
 	vcpu->arch.cr2 = addr;
@@ -2663,6 +2668,11 @@ again:
 			r = 0;
 			goto out;
 		}
+		if (test_and_clear_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests)) {
+			kvm_run->exit_reason = KVM_EXIT_SHUTDOWN;
+			r = 0;
+			goto out;
+		}
 	}
 
 	kvm_inject_pending_timer_irqs(vcpu);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9750bb3..958e003 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -38,6 +38,7 @@
 #define KVM_REQ_MIGRATE_TIMER      1
 #define KVM_REQ_REPORT_TPR_ACCESS  2
 #define KVM_REQ_MMU_RELOAD         3
+#define KVM_REQ_TRIPLE_FAULT       4
 
 struct kvm_vcpu;
 extern struct kmem_cache *kvm_vcpu_cache;
-- 
1.5.3.7




-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: [PATCH] KVM: detect if VCPU triple faults

[Avi Kivity]

Joerg Roedel wrote:
> In the current inject_page_fault path KVM only checks if there is another PF
> pending and injects a DF then. But it has to check for a pendig DF too to
> detect a shutdown condition in the VCPU. If this is not detected the VCPU goes
> to a PF -> DF -> PF loop when it should triple fault. This patch detects this
> condition and handles it with an KVM_SHUTDOWN exit to userspace.
>
>   

Applied, thanks.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/