Skip to contentSkip to conversation

KVM: Flush out my patch queue

PATCH New Avi Kivity 1 participant 10 messages Jan 5, 2007 KVM

lore
Share & tools
lore

Avi Kivity

author
#0 lore raw

cover KVM: Flush out my patch queue

This patchset is mostly fallout from the mmu stuff that I've neglected to integrate with the main patchset sent yesterday. It includes a fashionable missing dirty bit fix, and other fixes and cleanups.

Signature · 8 lines
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
author
#1 lore raw

PATCH 1/9 KVM: Improve reporting of vmwrite errors

This will allow us to see the root cause when a vmwrite error happens.

drivers/kvm/vmx.c +9 -3
Index: linux-2.6/drivers/kvm/vmx.c
===================================================================
@@ -152,15 +152,21 @@ static u64 vmcs_read64(unsigned long fie
 #endif
 }
 
+static noinline void vmwrite_error(unsigned long field, unsigned long value)
+{
+	printk(KERN_ERR "vmwrite error: reg %lx value %lx (err %d)\n",
+	       field, value, vmcs_read32(VM_INSTRUCTION_ERROR));
+	dump_stack();
+}
+
 static void vmcs_writel(unsigned long field, unsigned long value)
 {
 	u8 error;
 
 	asm volatile (ASM_VMX_VMWRITE_RAX_RDX "; setna %0"
 		       : "=q"(error) : "a"(value), "d"(field) : "cc" );
-	if (error)
-		printk(KERN_ERR "vmwrite error: reg %lx value %lx (err %d)\n",
-		       field, value, vmcs_read32(VM_INSTRUCTION_ERROR));
+	if (unlikely(error))
+		vmwrite_error(field, value);
 }
 
 static void vmcs_write16(unsigned long field, u16 value)

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#2 lore raw

PATCH 2/9 KVM: Initialize vcpu->kvm a little earlier

In reply to Avi Kivity

Fixes oops on early close of /dev/kvm.

drivers/kvm/kvm_main.c +1 -1
Index: linux-2.6/drivers/kvm/kvm_main.c
===================================================================
@@ -230,6 +230,7 @@ static int kvm_dev_open(struct inode *in
 		struct kvm_vcpu *vcpu = &kvm->vcpus[i];
 
 		mutex_init(&vcpu->mutex);
+		vcpu->kvm = kvm;
 		vcpu->mmu.root_hpa = INVALID_PAGE;
 		INIT_LIST_HEAD(&vcpu->free_pages);
 	}
@@ -530,7 +531,6 @@ static int kvm_dev_ioctl_create_vcpu(str
 	vcpu->guest_fx_image = vcpu->host_fx_image + FX_IMAGE_SIZE;
 
 	vcpu->cpu = -1;  /* First load will set up TR */
-	vcpu->kvm = kvm;
 	r = kvm_arch_ops->vcpu_create(vcpu);
 	if (r < 0)
 		goto out_free_vcpus;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#3 lore raw

PATCH 3/9 KVM: Avoid oom on cr3 switch

In reply to Avi Kivity

From: Ingo Molnar <[email protected]>

drivers/kvm/mmu.c +2 -0
Index: linux-2.6/drivers/kvm/mmu.c
===================================================================
@@ -905,6 +905,8 @@ static void paging_new_cr3(struct kvm_vc
 {
 	pgprintk("%s: cr3 %lx\n", __FUNCTION__, vcpu->cr3);
 	mmu_free_roots(vcpu);
+	if (unlikely(vcpu->kvm->n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES))
+		kvm_mmu_free_some_pages(vcpu);
 	mmu_alloc_roots(vcpu);
 	kvm_mmu_flush_tlb(vcpu);
 	kvm_arch_ops->set_cr3(vcpu, vcpu->mmu.root_hpa);

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#4 lore raw

PATCH 4/9 KVM: Add missing 'break'

In reply to Avi Kivity
drivers/kvm/kvm_main.c +1 -0
Index: linux-2.6/drivers/kvm/kvm_main.c
===================================================================
@@ -1922,6 +1922,7 @@ static long kvm_dev_ioctl(struct file *f
 				 num_msrs_to_save * sizeof(u32)))
 			goto out;
 		r = 0;
+		break;
 	}
 	default:
 		;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#5 lore raw

PATCH 5/9 KVM: Don't set guest cr3 from vmx_vcpu_setup()

In reply to Avi Kivity

It overwrites the right cr3 set from mmu setup. Happens only with the test harness.

drivers/kvm/vmx.c +0 -2
Index: linux-2.6/drivers/kvm/vmx.c
===================================================================
@@ -1027,8 +1027,6 @@ static int vmx_vcpu_setup(struct kvm_vcp
 	vmcs_writel(GUEST_RIP, 0xfff0);
 	vmcs_writel(GUEST_RSP, 0);
 
-	vmcs_writel(GUEST_CR3, 0);
-
 	//todo: dr0 = dr1 = dr2 = dr3 = 0; dr6 = 0xffff0ff0
 	vmcs_writel(GUEST_DR7, 0x400);
 

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#6 lore raw

PATCH 6/9 KVM: MMU: Add missing dirty bit

In reply to Avi Kivity

If we emulate a write, we fail to set the dirty bit on the guest pte, leading the guest to believe the page is clean, and thus lose data. Bad.

Fix by setting the guest pte dirty bit under such conditions.

drivers/kvm/paging_tmpl.h +1 -0
Index: linux-2.6/drivers/kvm/paging_tmpl.h
===================================================================
@@ -317,6 +317,7 @@ static int FNAME(fix_write_pf)(struct kv
 	} else if (kvm_mmu_lookup_page(vcpu, gfn)) {
 		pgprintk("%s: found shadow page for %lx, marking ro\n",
 			 __FUNCTION__, gfn);
+		*guest_ent |= PT_DIRTY_MASK;
 		*write_pt = 1;
 		return 0;
 	}

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#7 lore raw

PATCH 7/9 KVM: Make loading cr3 more robust

In reply to Avi Kivity

From: Ingo Molnar <[email protected]>

Prevent the guest's loading of a corrupt cr3 (pointing at no guest phsyical page) from crashing the host.

drivers/kvm/kvm_main.c +13 -1
Index: linux-2.6/drivers/kvm/kvm_main.c
===================================================================
@@ -463,7 +463,19 @@ void set_cr3(struct kvm_vcpu *vcpu, unsi
 
 	vcpu->cr3 = cr3;
 	spin_lock(&vcpu->kvm->lock);
-	vcpu->mmu.new_cr3(vcpu);
+	/*
+	 * Does the new cr3 value map to physical memory? (Note, we
+	 * catch an invalid cr3 even in real-mode, because it would
+	 * cause trouble later on when we turn on paging anyway.)
+	 *
+	 * A real CPU would silently accept an invalid cr3 and would
+	 * attempt to use it - with largely undefined (and often hard
+	 * to debug) behavior on the guest side.
+	 */
+	if (unlikely(!gfn_to_memslot(vcpu->kvm, cr3 >> PAGE_SHIFT)))
+		inject_gp(vcpu);
+	else
+		vcpu->mmu.new_cr3(vcpu);
 	spin_unlock(&vcpu->kvm->lock);
 }
 EXPORT_SYMBOL_GPL(set_cr3);

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#8 lore raw

PATCH 8/9 KVM: Simplify mmu_alloc_roots()

In reply to Avi Kivity

From: Ingo Molnar <[email protected]>

Small optimization/cleanup:

    page == page_header(page->page_hpa)
drivers/kvm/mmu.c +6 -6
Index: linux-2.6/drivers/kvm/mmu.c
===================================================================
@@ -820,9 +820,9 @@ static void mmu_alloc_roots(struct kvm_v
 		hpa_t root = vcpu->mmu.root_hpa;
 
 		ASSERT(!VALID_PAGE(root));
-		root = kvm_mmu_get_page(vcpu, root_gfn, 0,
-					PT64_ROOT_LEVEL, 0, NULL)->page_hpa;
-		page = page_header(root);
+		page = kvm_mmu_get_page(vcpu, root_gfn, 0,
+					PT64_ROOT_LEVEL, 0, NULL);
+		root = page->page_hpa;
 		++page->root_count;
 		vcpu->mmu.root_hpa = root;
 		return;
@@ -836,10 +836,10 @@ static void mmu_alloc_roots(struct kvm_v
 			root_gfn = vcpu->pdptrs[i] >> PAGE_SHIFT;
 		else if (vcpu->mmu.root_level == 0)
 			root_gfn = 0;
-		root = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
+		page = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
 					PT32_ROOT_LEVEL, !is_paging(vcpu),
-					NULL)->page_hpa;
-		page = page_header(root);
+					NULL);
+		root = page->page_hpa;
 		++page->root_count;
 		vcpu->mmu.pae_root[i] = root | PT_PRESENT_MASK;
 	}

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

author
#9 lore raw

PATCH 9/9 KVM: Simplify test for interrupt window

In reply to Avi Kivity

No need to test for rflags.if as both VT and SVM specs assure us that on exit caused from interrupt window opening, 'if' is set.

drivers/kvm/svm.c +1 -2
Index: linux-2.6/drivers/kvm/svm.c
===================================================================
@@ -1206,8 +1206,7 @@ static int interrupt_window_interception
 	 * possible
 	 */
 	if (kvm_run->request_interrupt_window &&
-	    !vcpu->irq_summary &&
-	    (vcpu->svm->vmcb->save.rflags & X86_EFLAGS_IF)) {
+	    !vcpu->irq_summary) {
 		++kvm_stat.irq_window_exits;
 		kvm_run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
 		return 0;
drivers/kvm/vmx.c +1 -2
Index: linux-2.6/drivers/kvm/vmx.c
===================================================================
@@ -1604,8 +1604,7 @@ static int handle_interrupt_window(struc
 	 * possible
 	 */
 	if (kvm_run->request_interrupt_window &&
-	    !vcpu->irq_summary &&
-	    (vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF)) {
+	    !vcpu->irq_summary) {
 		kvm_run->exit_reason = KVM_EXIT_IRQ_WINDOW_OPEN;
 		++kvm_stat.irq_window_exits;
 		return 0;

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV