Skip to contentSkip to conversation

KVM: MMU: unify slots_lock usage

PATCH In review Marcelo Tosatti 2 participants 2 messages Mar 29, 2008 – Mar 30, 2008

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: MMU: unify slots_lock usage

Unify slots_lock acquision around vcpu_run(). This is simpler and less error-prone.

Also fix some callsites that were not grabbing the lock properly.

Please review.

arch/x86/kvm/mmu.c +2 -11
Index: kvm/arch/x86/kvm/mmu.c
===================================================================
@@ -1202,8 +1202,6 @@ static int nonpaging_map(struct kvm_vcpu
 
 	struct page *page;
 
-	down_read(&vcpu->kvm->slots_lock);
-
 	down_read(&current->mm->mmap_sem);
 	if (is_largepage_backed(vcpu, gfn & ~(KVM_PAGES_PER_HPAGE-1))) {
 		gfn &= ~(KVM_PAGES_PER_HPAGE-1);
@@ -1216,7 +1214,6 @@ static int nonpaging_map(struct kvm_vcpu
 	/* mmio */
 	if (is_error_page(page)) {
 		kvm_release_page_clean(page);
-		up_read(&vcpu->kvm->slots_lock);
 		return 1;
 	}
 
@@ -1226,7 +1223,6 @@ static int nonpaging_map(struct kvm_vcpu
 			 PT32E_ROOT_LEVEL);
 	spin_unlock(&vcpu->kvm->mmu_lock);
 
-	up_read(&vcpu->kvm->slots_lock);
 
 	return r;
 }
@@ -1374,9 +1370,9 @@ static int tdp_page_fault(struct kvm_vcp
 		largepage = 1;
 	}
 	page = gfn_to_page(vcpu->kvm, gfn);
+	up_read(&current->mm->mmap_sem);
 	if (is_error_page(page)) {
 		kvm_release_page_clean(page);
-		up_read(&current->mm->mmap_sem);
 		return 1;
 	}
 	spin_lock(&vcpu->kvm->mmu_lock);
@@ -1384,7 +1380,6 @@ static int tdp_page_fault(struct kvm_vcp
 	r = __direct_map(vcpu, gpa, error_code & PFERR_WRITE_MASK,
 			 largepage, gfn, page, TDP_ROOT_LEVEL);
 	spin_unlock(&vcpu->kvm->mmu_lock);
-	up_read(&current->mm->mmap_sem);
 
 	return r;
 }
@@ -1806,9 +1801,7 @@ int kvm_mmu_unprotect_page_virt(struct k
 	gpa_t gpa;
 	int r;
 
-	down_read(&vcpu->kvm->slots_lock);
 	gpa = vcpu->arch.mmu.gva_to_gpa(vcpu, gva);
-	up_read(&vcpu->kvm->slots_lock);
 
 	spin_lock(&vcpu->kvm->mmu_lock);
 	r = kvm_mmu_unprotect_page(vcpu->kvm, gpa >> PAGE_SHIFT);
@@ -2061,7 +2054,7 @@ static int kvm_pv_mmu_write(struct kvm_v
 	if (r)
 		return r;
 
-	if (!__emulator_write_phys(vcpu, addr, &value, bytes))
+	if (!emulator_write_phys(vcpu, addr, &value, bytes))
 		return -EFAULT;
 
 	return 1;
@@ -2125,7 +2118,6 @@ int kvm_pv_mmu_op(struct kvm_vcpu *vcpu,
 	int r;
 	struct kvm_pv_mmu_op_buffer buffer;
 
-	down_read(&vcpu->kvm->slots_lock);
 	down_read(&current->mm->mmap_sem);
 
 	buffer.ptr = buffer.buf;
@@ -2148,7 +2140,6 @@ int kvm_pv_mmu_op(struct kvm_vcpu *vcpu,
 out:
 	*ret = buffer.processed;
 	up_read(&current->mm->mmap_sem);
-	up_read(&vcpu->kvm->slots_lock);
 	return r;
 }
 
arch/x86/kvm/x86.c +16 -33

215 lines not on this page · raw message

include/asm-x86/kvm_host.h +1 -1
Index: kvm/include/asm-x86/kvm_host.h
===================================================================
@@ -445,7 +445,7 @@ void kvm_mmu_change_mmu_pages(struct kvm
 
 int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3);
 
-int __emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
+int emulator_write_phys(struct kvm_vcpu *vcpu, gpa_t gpa,
 			  const void *val, int bytes);
 int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long bytes,
 		  gpa_t addr, unsigned long *ret);
arch/x86/kvm/paging_tmpl.h +0 -4
Index: kvm/arch/x86/kvm/paging_tmpl.h
===================================================================
@@ -388,7 +388,6 @@ static int FNAME(page_fault)(struct kvm_
 	if (r)
 		return r;
 
-	down_read(&vcpu->kvm->slots_lock);
 	/*
 	 * Look up the shadow pte for the faulting address.
 	 */
@@ -402,7 +401,6 @@ static int FNAME(page_fault)(struct kvm_
 		pgprintk("%s: guest page fault\n", __func__);
 		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;
 	}
 
@@ -422,7 +420,6 @@ static int FNAME(page_fault)(struct kvm_
 	if (is_error_page(page)) {
 		pgprintk("gfn %x is mmio\n", walker.gfn);
 		kvm_release_page_clean(page);
-		up_read(&vcpu->kvm->slots_lock);
 		return 1;
 	}
 
@@ -440,7 +437,6 @@ static int FNAME(page_fault)(struct kvm_
 	++vcpu->stat.pf_fixed;
 	kvm_mmu_audit(vcpu, "post page fault (fixed)");
 	spin_unlock(&vcpu->kvm->mmu_lock);
-	up_read(&vcpu->kvm->slots_lock);
 
 	return write_pt;
 }
arch/x86/kvm/vmx.c +3 -3
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
@@ -1498,7 +1498,6 @@ static int init_rmode_tss(struct kvm *kv
 	int ret = 0;
 	int r;
 
-	down_read(&kvm->slots_lock);
 	r = kvm_clear_guest_page(kvm, fn, 0, PAGE_SIZE);
 	if (r < 0)
 		goto out;
@@ -1521,7 +1520,6 @@ static int init_rmode_tss(struct kvm *kv
 
 	ret = 1;
 out:
-	up_read(&kvm->slots_lock);
 	return ret;
 }
 
@@ -1696,6 +1694,7 @@ static int vmx_vcpu_reset(struct kvm_vcp
 	u64 msr;
 	int ret;
 
+	down_read(&vcpu->kvm->slots_lock);
 	if (!init_rmode_tss(vmx->vcpu.kvm)) {
 		ret = -ENOMEM;
 		goto out;
@@ -1799,9 +1798,10 @@ static int vmx_vcpu_reset(struct kvm_vcp
 
 	vpid_sync_vcpu_all(vmx);
 
-	return 0;
+	ret = 0;
 
 out:
+	up_read(&vcpu->kvm->slots_lock);
 	return ret;
 }
 

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace

#1 lore raw
Quoted 8 lines · Marcelo Tosatti

Unify slots_lock acquision around vcpu_run(). This is simpler and less error-prone.

Also fix some callsites that were not grabbing the lock properly.

Please review.

While I think we could simplify the locking even further (by locking at the caller of __vcpu_run), this is already a huge improvement, so I applied it. Thanks.

Signature · 8 lines
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace