KVM: VMX: Fix repeatly allocate of apic access page

PATCH In review Sheng Yang 2 participants 2 messages Nov 6, 2007 1 message not in the archive KVM

Share & tools

Sheng Yang

author

2007-11-06 09:55 UTC #0 lore raw

PATCH 1/1 KVM: VMX: Fix repeatly allocate of apic access page

From 617412380eec67503d45cd91860b756f15293e73 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Tue, 6 Nov 2007 11:37:44 +0800
Subject: [PATCH] KVM: VMX: Fix repeatly allocate of apic access page

For SMP guest, alloc_apic_access_page() would be called more than once. So only the last one works, causing SMP guest can't benifit from FlexPriority.

This patch fixed this issue.

---
 drivers/kvm/kvm.h      |    3 +++
 drivers/kvm/kvm_main.c |   38 ++++++++++++++++++++++++--------------
 drivers/kvm/vmx.c      |   14 +++++++++-----
 3 files changed, 36 insertions(+), 19 deletions(-)

drivers/kvm/kvm.h +3 -0

index 6b3b5e9..3ebfb19 100644
@@ -524,6 +524,9 @@ int is_error_page(struct page *page);
 int kvm_set_memory_region(struct kvm *kvm,
 			  struct kvm_userspace_memory_region *mem,
 			  int user_alloc);
+int __kvm_set_memory_region(struct kvm *kvm,
+			    struct kvm_userspace_memory_region *mem,
+			    int user_alloc);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
 void kvm_release_page(struct page *page);

drivers/kvm/kvm_main.c +0 -0

index af7090f..d332a77 100644
@@ -277,10 +277,12 @@ static int kvm_vm_release(struct inode *inode, struct

file *filp)
* space.
*
* Discontiguous memory is allowed, mostly for framebuffers.
+ *
+ * Must be called holding kvm->lock.
*/
-int kvm_set_memory_region(struct kvm *kvm,
- struct kvm_userspace_memory_region *mem,
- int user_alloc)
+int __kvm_set_memory_region(struct kvm *kvm,
+ struct kvm_userspace_memory_region *mem,
+ int user_alloc)
{
int r;
gfn_t base_gfn;
@@ -307,8 +309,6 @@ int kvm_set_memory_region(struct kvm *kvm,
if (!npages)
mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;

- mutex_lock(&kvm->lock);
-
new = old = *memslot;

new.base_gfn = base_gfn;
@@ -318,7 +318,7 @@ int kvm_set_memory_region(struct kvm *kvm,
/* Disallow changing a memory slot's size. */ r = -EINVAL;
if (npages && old.npages && npages != old.npages)
- goto out_unlock;
+ goto out_free;

/* Check for overlaps */
r = -EEXIST;
@@ -329,7 +329,7 @@ int kvm_set_memory_region(struct kvm *kvm,
continue;
if (!((base_gfn + npages <= s->base_gfn) ||
(base_gfn >= s->base_gfn + s->npages)))
- goto out_unlock;
+ goto out_free;
}

/* Free page dirty bitmap if unneeded */
@@ -343,7 +343,7 @@ int kvm_set_memory_region(struct kvm *kvm,
new.rmap = vmalloc(npages * sizeof(struct page *));

if (!new.rmap)
- goto out_unlock;
+ goto out_free;

memset(new.rmap, 0, npages * sizeof(*new.rmap));

@@ -360,7 +360,7 @@ int kvm_set_memory_region(struct kvm *kvm,
up_write(&current->mm->mmap_sem);

if (IS_ERR((void *)new.userspace_addr))
- goto out_unlock;
+ goto out_free;
}
} else {
if (!old.user_alloc && old.rmap) {
@@ -383,7 +383,7 @@ int kvm_set_memory_region(struct kvm *kvm,

new.dirty_bitmap = vmalloc(dirty_bytes); if (!new.dirty_bitmap)
- goto out_unlock;
+ goto out_free;
memset(new.dirty_bitmap, 0, dirty_bytes);
}

@@ -413,18 +413,28 @@ int kvm_set_memory_region(struct kvm *kvm,
kvm_mmu_slot_remove_write_access(kvm, mem->slot); kvm_flush_remote_tlbs(kvm);

- mutex_unlock(&kvm->lock);
-
kvm_free_physmem_slot(&old, &new);
return 0;

-out_unlock:
- mutex_unlock(&kvm->lock);
+out_free:
kvm_free_physmem_slot(&new, &old);
out:
return r;

}
+EXPORT_SYMBOL_GPL(__kvm_set_memory_region);
+
+int kvm_set_memory_region(struct kvm *kvm,
+ struct kvm_userspace_memory_region *mem,
+ int user_alloc)
+{
+ int r;
+
+ mutex_lock(&kvm->lock);
+ r = __kvm_set_memory_region(kvm, mem, user_alloc);
+ mutex_unlock(&kvm->lock);
+ return r;
+}
EXPORT_SYMBOL_GPL(kvm_set_memory_region);

int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,

drivers/kvm/vmx.c +9 -5

index 42e7fad..da3a339 100644
@@ -1463,18 +1463,22 @@ static void seg_setup(int seg)
 static int alloc_apic_access_page(struct kvm *kvm)
 {
 	struct kvm_userspace_memory_region kvm_userspace_mem;
-	int r;
+	int r = 0;
 
-	r = -EFAULT;
+	mutex_lock(&kvm->lock);
+	if (kvm->apic_access_page)
+		goto out;
 	kvm_userspace_mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT;
 	kvm_userspace_mem.flags = 0;
 	kvm_userspace_mem.guest_phys_addr = 0xfee00000ULL;
 	kvm_userspace_mem.memory_size = PAGE_SIZE;
-	r = kvm_set_memory_region(kvm, &kvm_userspace_mem, 0);
+	r = __kvm_set_memory_region(kvm, &kvm_userspace_mem, 0);
 	if (r)
-		return r;
+		goto out;
 	kvm->apic_access_page = gfn_to_page(kvm, 0xfee00);
-	return 0;
+out:
+	mutex_unlock(&kvm->lock);
+	return r;
 }
 
 /*

Signature · 1 line

1.5.2

For SMP guest, alloc_apic_access_page() would be called more than once. So only the last one works, causing SMP guest can't benifit from FlexPriority.

This patch fixed this issue.

---
 drivers/kvm/kvm.h      |    3 +++
 drivers/kvm/kvm_main.c |   38 ++++++++++++++++++++++++--------------
 drivers/kvm/vmx.c      |   14 +++++++++-----
 3 files changed, 36 insertions(+), 19 deletions(-)

drivers/kvm/kvm.h +3 -0

index 6b3b5e9..3ebfb19 100644
@@ -524,6 +524,9 @@ int is_error_page(struct page *page);
 int kvm_set_memory_region(struct kvm *kvm,
 			  struct kvm_userspace_memory_region *mem,
 			  int user_alloc);
+int __kvm_set_memory_region(struct kvm *kvm,
+			    struct kvm_userspace_memory_region *mem,
+			    int user_alloc);
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn);
 struct page *gfn_to_page(struct kvm *kvm, gfn_t gfn);
 void kvm_release_page(struct page *page);

drivers/kvm/kvm_main.c +24 ~~-14~~

index af7090f..d332a77 100644
@@ -277,10 +277,12 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
  * space.
  *
  * Discontiguous memory is allowed, mostly for framebuffers.
+ *
+ * Must be called holding kvm->lock.
  */
-int kvm_set_memory_region(struct kvm *kvm,
-			  struct kvm_userspace_memory_region *mem,
-			  int user_alloc)
+int __kvm_set_memory_region(struct kvm *kvm,
+			    struct kvm_userspace_memory_region *mem,
+			    int user_alloc)
 {
 	int r;
 	gfn_t base_gfn;

@@ -307,8 +309,6 @@ int kvm_set_memory_region(struct kvm *kvm,
 	if (!npages)
 		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
 
-	mutex_lock(&kvm->lock);
-
 	new = old = *memslot;
 
 	new.base_gfn = base_gfn;

@@ -318,7 +318,7 @@ int kvm_set_memory_region(struct kvm *kvm,
 	/* Disallow changing a memory slot's size. */
 	r = -EINVAL;
 	if (npages && old.npages && npages != old.npages)
-		goto out_unlock;
+		goto out_free;
 
 	/* Check for overlaps */
 	r = -EEXIST;

@@ -329,7 +329,7 @@ int kvm_set_memory_region(struct kvm *kvm,
 			continue;
 		if (!((base_gfn + npages <= s->base_gfn) ||
 		      (base_gfn >= s->base_gfn + s->npages)))
-			goto out_unlock;
+			goto out_free;
 	}
 
 	/* Free page dirty bitmap if unneeded */

@@ -343,7 +343,7 @@ int kvm_set_memory_region(struct kvm *kvm,
 		new.rmap = vmalloc(npages * sizeof(struct page *));
 
 		if (!new.rmap)
-			goto out_unlock;
+			goto out_free;
 
 		memset(new.rmap, 0, npages * sizeof(*new.rmap));

@@ -360,7 +360,7 @@ int kvm_set_memory_region(struct kvm *kvm,
 			up_write(&current->mm->mmap_sem);
 
 			if (IS_ERR((void *)new.userspace_addr))
-				goto out_unlock;
+				goto out_free;
 		}
 	} else {
 		if (!old.user_alloc && old.rmap) {

@@ -383,7 +383,7 @@ int kvm_set_memory_region(struct kvm *kvm,
 
 		new.dirty_bitmap = vmalloc(dirty_bytes);
 		if (!new.dirty_bitmap)
-			goto out_unlock;
+			goto out_free;
 		memset(new.dirty_bitmap, 0, dirty_bytes);
 	}

@@ -413,18 +413,28 @@ int kvm_set_memory_region(struct kvm *kvm,
 	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
 	kvm_flush_remote_tlbs(kvm);
 
-	mutex_unlock(&kvm->lock);
-
 	kvm_free_physmem_slot(&old, &new);
 	return 0;
 
-out_unlock:
-	mutex_unlock(&kvm->lock);
+out_free:
 	kvm_free_physmem_slot(&new, &old);
 out:
 	return r;
 
 }
+EXPORT_SYMBOL_GPL(__kvm_set_memory_region);
+
+int kvm_set_memory_region(struct kvm *kvm,
+			  struct kvm_userspace_memory_region *mem,
+			  int user_alloc)
+{
+	int r;
+
+	mutex_lock(&kvm->lock);
+	r = __kvm_set_memory_region(kvm, mem, user_alloc);
+	mutex_unlock(&kvm->lock);
+	return r;
+}
 EXPORT_SYMBOL_GPL(kvm_set_memory_region);
 
 int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,

drivers/kvm/vmx.c +9 -5

index 42e7fad..da3a339 100644
@@ -1463,18 +1463,22 @@ static void seg_setup(int seg)
 static int alloc_apic_access_page(struct kvm *kvm)
 {
 	struct kvm_userspace_memory_region kvm_userspace_mem;
-	int r;
+	int r = 0;
 
-	r = -EFAULT;
+	mutex_lock(&kvm->lock);
+	if (kvm->apic_access_page)
+		goto out;
 	kvm_userspace_mem.slot = APIC_ACCESS_PAGE_PRIVATE_MEMSLOT;
 	kvm_userspace_mem.flags = 0;
 	kvm_userspace_mem.guest_phys_addr = 0xfee00000ULL;
 	kvm_userspace_mem.memory_size = PAGE_SIZE;
-	r = kvm_set_memory_region(kvm, &kvm_userspace_mem, 0);
+	r = __kvm_set_memory_region(kvm, &kvm_userspace_mem, 0);
 	if (r)
-		return r;
+		goto out;
 	kvm->apic_access_page = gfn_to_page(kvm, 0xfee00);
-	return 0;
+out:
+	mutex_unlock(&kvm->lock);
+	return r;
 }
 
 /*

Signature · 1 line

1.5.2

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/

_______________________________________________
kvm-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel

A message that is not in the archive (sent off-list or not yet received): 200711061755.14444.sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

Avi Kivity

2007-11-06 10:04 UTC #2 lore raw

Quoted 9 lines · Sheng Yang

From 617412380eec67503d45cd91860b756f15293e73 Mon Sep 17 00:00:00 2001
From: Sheng Yang <[email protected]>
Date: Tue, 6 Nov 2007 11:37:44 +0800
Subject: [PATCH] KVM: VMX: Fix repeatly allocate of apic access page
For SMP guest, alloc_apic_access_page() would be called more than once. So only the last one works, causing SMP guest can't benifit from FlexPriority.

Patch is good, but no signoff...

(no need to resend, just provide one in the reply, sorry about the red tape)

Signature · 8 lines

error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/