kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch
@ 2012-01-30  3:48 Takuya Yoshikawa
  2012-01-30  3:50 ` [PATCH 1/4] KVM: Introduce gfn_to_index() which returns the index for a given level Takuya Yoshikawa
                   ` (5 more replies)
  0 siblings, 6 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  3:48 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

This is the first step to separate the architecture specific members.
The rmap and dirty_bitmap can be treated later based on this.

v1->v2:

  Patch 3:
  - Removed extra checks for NULL when we create a new slot.
  - Removed "if (user_alloc)" check taken from the s390 code.

  Patch 4:
  - Just rebased.

        Takuya

 arch/ia64/include/asm/kvm_host.h    |    3 +
 arch/ia64/kvm/kvm-ia64.c            |   10 +++++
 arch/powerpc/include/asm/kvm_host.h |    3 +
 arch/powerpc/kvm/powerpc.c          |   10 +++++
 arch/s390/include/asm/kvm_host.h    |    3 +
 arch/s390/kvm/kvm-s390.c            |   10 +++++
 arch/x86/include/asm/kvm_host.h     |    9 ++++
 arch/x86/kvm/mmu.c                  |    5 +-
 arch/x86/kvm/x86.c                  |   59 +++++++++++++++++++++++++++
 include/linux/kvm_host.h            |   18 +++++---
 virt/kvm/kvm_main.c                 |   76 ++++++----------------------------
 11 files changed, 135 insertions(+), 71 deletions(-)

-- 
1.7.5.4


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/4] KVM: Introduce gfn_to_index() which returns the index for a given level
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
@ 2012-01-30  3:50 ` Takuya Yoshikawa
  2012-01-30  3:50 ` [PATCH 2/4] KVM: Split lpage_info creation out from __kvm_set_memory_region() Takuya Yoshikawa
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  3:50 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

This patch cleans up the code and removes the "(void)level;" warning
suppressor.

Note that we can also use this for PT_PAGE_TABLE_LEVEL to treat every
level uniformly later.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 arch/x86/kvm/mmu.c       |    3 +--
 include/linux/kvm_host.h |    7 +++++++
 virt/kvm/kvm_main.c      |    7 +------
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ae76cc3..37e7f10 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -688,8 +688,7 @@ static struct kvm_lpage_info *lpage_info_slot(gfn_t gfn,
 {
 	unsigned long idx;
 
-	idx = (gfn >> KVM_HPAGE_GFN_SHIFT(level)) -
-	      (slot->base_gfn >> KVM_HPAGE_GFN_SHIFT(level));
+	idx = gfn_to_index(gfn, slot->base_gfn, level);
 	return &slot->lpage_info[level - 2][idx];
 }
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index eada8e6..06d4e41 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -656,6 +656,13 @@ static inline int memslot_id(struct kvm *kvm, gfn_t gfn)
 	return gfn_to_memslot(kvm, gfn)->id;
 }
 
+static inline gfn_t gfn_to_index(gfn_t gfn, gfn_t base_gfn, int level)
+{
+	/* KVM_HPAGE_GFN_SHIFT(PT_PAGE_TABLE_LEVEL) must be 0. */
+	return (gfn >> KVM_HPAGE_GFN_SHIFT(level)) -
+		(base_gfn >> KVM_HPAGE_GFN_SHIFT(level));
+}
+
 static inline unsigned long gfn_to_hva_memslot(struct kvm_memory_slot *slot,
 					       gfn_t gfn)
 {
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9f32bff..e483ae4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -797,15 +797,10 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		int lpages;
 		int level = i + 2;
 
-		/* Avoid unused variable warning if no large pages */
-		(void)level;
-
 		if (new.lpage_info[i])
 			continue;
 
-		lpages = 1 + ((base_gfn + npages - 1)
-			     >> KVM_HPAGE_GFN_SHIFT(level));
-		lpages -= base_gfn >> KVM_HPAGE_GFN_SHIFT(level);
+		lpages = gfn_to_index(base_gfn + npages - 1, base_gfn, level) + 1;
 
 		new.lpage_info[i] = vzalloc(lpages * sizeof(*new.lpage_info[i]));
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/4] KVM: Split lpage_info creation out from __kvm_set_memory_region()
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
  2012-01-30  3:50 ` [PATCH 1/4] KVM: Introduce gfn_to_index() which returns the index for a given level Takuya Yoshikawa
@ 2012-01-30  3:50 ` Takuya Yoshikawa
  2012-01-30  3:51 ` [PATCH 3/4 v2] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region() Takuya Yoshikawa
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  3:50 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

This makes it easy to make lpage_info architecture specific.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 virt/kvm/kvm_main.c |   83 ++++++++++++++++++++++++++++++++-------------------
 1 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e483ae4..2ef9f2d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -698,6 +698,56 @@ void update_memslots(struct kvm_memslots *slots, struct kvm_memory_slot *new)
 	slots->generation++;
 }
 
+#ifndef CONFIG_S390
+static int create_lpage_info(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	int i;
+
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		unsigned long ugfn;
+		int lpages;
+		int level = i + 2;
+
+		if (slot->lpage_info[i])
+			continue;
+
+		lpages = gfn_to_index(slot->base_gfn + npages - 1,
+				      slot->base_gfn, level) + 1;
+
+		slot->lpage_info[i] = vzalloc(lpages * sizeof(*slot->lpage_info[i]));
+		if (!slot->lpage_info[i])
+			goto out_free;
+
+		if (slot->base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->lpage_info[i][0].write_count = 1;
+		if ((slot->base_gfn + npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->lpage_info[i][lpages - 1].write_count = 1;
+		ugfn = slot->userspace_addr >> PAGE_SHIFT;
+		/*
+		 * If the gfn and userspace address are not aligned wrt each
+		 * other, or if explicitly asked to, disable large page
+		 * support for this slot
+		 */
+		if ((slot->base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
+		    !largepages_enabled) {
+			unsigned long j;
+
+			for (j = 0; j < lpages; ++j)
+				slot->lpage_info[i][j].write_count = 1;
+		}
+	}
+
+	return 0;
+
+out_free:
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		vfree(slot->lpage_info[i]);
+		slot->lpage_info[i] = NULL;
+	}
+	return -ENOMEM;
+}
+#endif /* not defined CONFIG_S390 */
+
 /*
  * Allocate some memory and give it an address in the guest physical address
  * space.
@@ -791,37 +841,8 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	if (!npages)
 		goto skip_lpage;
 
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		unsigned long ugfn;
-		unsigned long j;
-		int lpages;
-		int level = i + 2;
-
-		if (new.lpage_info[i])
-			continue;
-
-		lpages = gfn_to_index(base_gfn + npages - 1, base_gfn, level) + 1;
-
-		new.lpage_info[i] = vzalloc(lpages * sizeof(*new.lpage_info[i]));
-
-		if (!new.lpage_info[i])
-			goto out_free;
-
-		if (base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
-			new.lpage_info[i][0].write_count = 1;
-		if ((base_gfn+npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
-			new.lpage_info[i][lpages - 1].write_count = 1;
-		ugfn = new.userspace_addr >> PAGE_SHIFT;
-		/*
-		 * If the gfn and userspace address are not aligned wrt each
-		 * other, or if explicitly asked to, disable large page
-		 * support for this slot
-		 */
-		if ((base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
-		    !largepages_enabled)
-			for (j = 0; j < lpages; ++j)
-				new.lpage_info[i][j].write_count = 1;
-	}
+	if (create_lpage_info(&new, npages))
+		goto out_free;
 
 skip_lpage:
 
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/4 v2] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region()
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
  2012-01-30  3:50 ` [PATCH 1/4] KVM: Introduce gfn_to_index() which returns the index for a given level Takuya Yoshikawa
  2012-01-30  3:50 ` [PATCH 2/4] KVM: Split lpage_info creation out from __kvm_set_memory_region() Takuya Yoshikawa
@ 2012-01-30  3:51 ` Takuya Yoshikawa
  2012-01-30  3:53 ` [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it Takuya Yoshikawa
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  3:51 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

Narrow down the controlled text inside the conditional so that it will
include lpage_info and rmap stuff only.

For this we change the way we check whether the slot is being created
from "if (npages && !new.rmap)" to "if (npages && !old.npages)".

We also stop checking if lpage_info is NULL when we create lpage_info
because we do it from inside the slot creation code block.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 virt/kvm/kvm_main.c |   29 ++++++++---------------------
 1 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2ef9f2d..b6bc866 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -616,7 +616,6 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-#ifndef CONFIG_S390
 /*
  * Allocation size is twice as large as the actual dirty bitmap size.
  * This makes it possible to do double buffering: see x86's
@@ -624,6 +623,7 @@ static int kvm_vm_release(struct inode *inode, struct file *filp)
  */
 static int kvm_create_dirty_bitmap(struct kvm_memory_slot *memslot)
 {
+#ifndef CONFIG_S390
 	unsigned long dirty_bytes = 2 * kvm_dirty_bitmap_bytes(memslot);
 
 	if (dirty_bytes > PAGE_SIZE)
@@ -636,9 +636,9 @@ static int kvm_create_dirty_bitmap(struct kvm_memory_slot *memslot)
 
 	memslot->dirty_bitmap_head = memslot->dirty_bitmap;
 	memslot->nr_dirty_pages = 0;
+#endif /* !CONFIG_S390 */
 	return 0;
 }
-#endif /* !CONFIG_S390 */
 
 static struct kvm_memory_slot *
 search_memslots(struct kvm_memslots *slots, gfn_t gfn)
@@ -708,9 +708,6 @@ static int create_lpage_info(struct kvm_memory_slot *slot, unsigned long npages)
 		int lpages;
 		int level = i + 2;
 
-		if (slot->lpage_info[i])
-			continue;
-
 		lpages = gfn_to_index(slot->base_gfn + npages - 1,
 				      slot->base_gfn, level) + 1;
 
@@ -828,23 +825,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	r = -ENOMEM;
 
 	/* Allocate if a slot is being created */
+	if (npages && !old.npages) {
+		new.user_alloc = user_alloc;
+		new.userspace_addr = mem->userspace_addr;
 #ifndef CONFIG_S390
-	if (npages && !new.rmap) {
 		new.rmap = vzalloc(npages * sizeof(*new.rmap));
-
 		if (!new.rmap)
 			goto out_free;
 
-		new.user_alloc = user_alloc;
-		new.userspace_addr = mem->userspace_addr;
+		if (create_lpage_info(&new, npages))
+			goto out_free;
+#endif /* not defined CONFIG_S390 */
 	}
-	if (!npages)
-		goto skip_lpage;
-
-	if (create_lpage_info(&new, npages))
-		goto out_free;
-
-skip_lpage:
 
 	/* Allocate page dirty bitmap if needed */
 	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
@@ -852,11 +844,6 @@ skip_lpage:
 			goto out_free;
 		/* destroy any largepage mappings for dirty tracking */
 	}
-#else  /* not defined CONFIG_S390 */
-	new.user_alloc = user_alloc;
-	if (user_alloc)
-		new.userspace_addr = mem->userspace_addr;
-#endif /* not defined CONFIG_S390 */
 
 	if (!npages) {
 		struct kvm_memory_slot *slot;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
                   ` (2 preceding siblings ...)
  2012-01-30  3:51 ` [PATCH 3/4 v2] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region() Takuya Yoshikawa
@ 2012-01-30  3:53 ` Takuya Yoshikawa
  2012-01-30  4:58   ` Takuya Yoshikawa
  2012-01-30  5:35   ` [PATCH 4/4 v3] " Takuya Yoshikawa
  2012-02-06 10:10 ` [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
  2012-02-07 17:42 ` Marcelo Tosatti
  5 siblings, 2 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  3:53 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

Some members of kvm_memory_slot are not used by every architecture.

This patch is the first step to make this difference clear by
introducing kvm_memory_slot::arch;  lpage_info is moved into it.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 arch/ia64/include/asm/kvm_host.h    |    3 ++
 arch/ia64/kvm/kvm-ia64.c            |   10 +++++
 arch/powerpc/include/asm/kvm_host.h |    3 ++
 arch/powerpc/kvm/powerpc.c          |   10 +++++
 arch/s390/include/asm/kvm_host.h    |    3 ++
 arch/s390/kvm/kvm-s390.c            |   10 +++++
 arch/x86/include/asm/kvm_host.h     |    9 +++++
 arch/x86/kvm/mmu.c                  |    2 +-
 arch/x86/kvm/x86.c                  |   59 ++++++++++++++++++++++++++++++
 include/linux/kvm_host.h            |   11 +++---
 virt/kvm/kvm_main.c                 |   67 ++++------------------------------
 11 files changed, 121 insertions(+), 66 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
index 2689ee5..e35b3a8 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -459,6 +459,9 @@ struct kvm_sal_data {
 	unsigned long boot_gp;
 };
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch {
 	spinlock_t dirty_log_lock;
 
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 8ca7261..d8ddbba 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1571,6 +1571,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 		struct kvm_memory_slot *memslot,
 		struct kvm_memory_slot old,
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af438b1..b9188aa 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -212,6 +212,9 @@ struct revmap_entry {
 #define KVMPPC_PAGE_WRITETHRU	HPTE_R_W	/* 0x40 */
 #define KVMPPC_GOT_PAGE		0x80
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch {
 #ifdef CONFIG_KVM_BOOK3S_64_HV
 	unsigned long hpt_virt;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 0e21d15..00d7e34 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -281,6 +281,16 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return -EINVAL;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
                                    struct kvm_memory_slot *memslot,
                                    struct kvm_memory_slot old,
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index e630426..7343872 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -245,6 +245,9 @@ struct kvm_vm_stat {
 	u32 remote_tlb_flush;
 };
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch{
 	struct sca_block *sca;
 	debug_info_t *dbf;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0b91679..418a69c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -807,6 +807,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 /* Section: memory related */
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				   struct kvm_memory_slot *memslot,
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4610166..de3aa43 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -479,6 +479,15 @@ struct kvm_vcpu_arch {
 	} osvw;
 };
 
+struct kvm_lpage_info {
+	unsigned long rmap_pde;
+	int write_count;
+};
+
+struct kvm_arch_memory_slot {
+	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
+};
+
 struct kvm_arch {
 	unsigned int n_used_mmu_pages;
 	unsigned int n_requested_mmu_pages;
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 37e7f10..ff053ca 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -689,7 +689,7 @@ static struct kvm_lpage_info *lpage_info_slot(gfn_t gfn,
 	unsigned long idx;
 
 	idx = gfn_to_index(gfn, slot->base_gfn, level);
-	return &slot->lpage_info[level - 2][idx];
+	return &slot->arch.lpage_info[level - 2][idx];
 }
 
 static void account_shadowed(struct kvm *kvm, gfn_t gfn)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2bd77a3..45e4197 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6120,6 +6120,65 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 		put_page(kvm->arch.ept_identity_pagetable);
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+	int i;
+
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		if (!dont || free->arch.lpage_info[i] != dont->arch.lpage_info[i]) {
+			vfree(free->arch.lpage_info[i]);
+			free->arch.lpage_info[i] = NULL;
+		}
+	}
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	int i;
+
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		unsigned long ugfn;
+		int lpages;
+		int level = i + 2;
+
+		lpages = gfn_to_index(slot->base_gfn + npages - 1,
+				      slot->base_gfn, level) + 1;
+
+		slot->arch.lpage_info[i] =
+			vzalloc(lpages * sizeof(*slot->arch.lpage_info[i]));
+		if (!slot->arch.lpage_info[i])
+			goto out_free;
+
+		if (slot->base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->arch.lpage_info[i][0].write_count = 1;
+		if ((slot->base_gfn + npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->arch.lpage_info[i][lpages - 1].write_count = 1;
+		ugfn = slot->userspace_addr >> PAGE_SHIFT;
+		/*
+		 * If the gfn and userspace address are not aligned wrt each
+		 * other, or if explicitly asked to, disable large page
+		 * support for this slot
+		 */
+		if ((slot->base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
+		    !kvm_largepages_enabled()) {
+			unsigned long j;
+
+			for (j = 0; j < lpages; ++j)
+				slot->arch.lpage_info[i][j].write_count = 1;
+		}
+	}
+
+	return 0;
+
+out_free:
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		vfree(slot->arch.lpage_info[i]);
+		slot->arch.lpage_info[i] = NULL;
+	}
+	return -ENOMEM;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				struct kvm_memory_slot *memslot,
 				struct kvm_memory_slot old,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 06d4e41..1d833d7 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -171,11 +171,6 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
  */
 #define KVM_MEM_MAX_NR_PAGES ((1UL << 31) - 1)
 
-struct kvm_lpage_info {
-	unsigned long rmap_pde;
-	int write_count;
-};
-
 struct kvm_memory_slot {
 	gfn_t base_gfn;
 	unsigned long npages;
@@ -184,7 +179,7 @@ struct kvm_memory_slot {
 	unsigned long *dirty_bitmap;
 	unsigned long *dirty_bitmap_head;
 	unsigned long nr_dirty_pages;
-	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
+	struct kvm_arch_memory_slot arch;
 	unsigned long userspace_addr;
 	int user_alloc;
 	int id;
@@ -376,6 +371,9 @@ int kvm_set_memory_region(struct kvm *kvm,
 int __kvm_set_memory_region(struct kvm *kvm,
 			    struct kvm_userspace_memory_region *mem,
 			    int user_alloc);
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont);
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages);
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				struct kvm_memory_slot *memslot,
 				struct kvm_memory_slot old,
@@ -385,6 +383,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+bool kvm_largepages_enabled(void);
 void kvm_disable_largepages(void);
 void kvm_arch_flush_shadow(struct kvm *kvm);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b6bc866..cef760e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -535,21 +535,13 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot)
 static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
 				  struct kvm_memory_slot *dont)
 {
-	int i;
-
 	if (!dont || free->rmap != dont->rmap)
 		vfree(free->rmap);
 
 	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
 		kvm_destroy_dirty_bitmap(free);
 
-
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		if (!dont || free->lpage_info[i] != dont->lpage_info[i]) {
-			vfree(free->lpage_info[i]);
-			free->lpage_info[i] = NULL;
-		}
-	}
+	kvm_arch_free_memslot(free, dont);
 
 	free->npages = 0;
 	free->rmap = NULL;
@@ -698,53 +690,6 @@ void update_memslots(struct kvm_memslots *slots, struct kvm_memory_slot *new)
 	slots->generation++;
 }
 
-#ifndef CONFIG_S390
-static int create_lpage_info(struct kvm_memory_slot *slot, unsigned long npages)
-{
-	int i;
-
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		unsigned long ugfn;
-		int lpages;
-		int level = i + 2;
-
-		lpages = gfn_to_index(slot->base_gfn + npages - 1,
-				      slot->base_gfn, level) + 1;
-
-		slot->lpage_info[i] = vzalloc(lpages * sizeof(*slot->lpage_info[i]));
-		if (!slot->lpage_info[i])
-			goto out_free;
-
-		if (slot->base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
-			slot->lpage_info[i][0].write_count = 1;
-		if ((slot->base_gfn + npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
-			slot->lpage_info[i][lpages - 1].write_count = 1;
-		ugfn = slot->userspace_addr >> PAGE_SHIFT;
-		/*
-		 * If the gfn and userspace address are not aligned wrt each
-		 * other, or if explicitly asked to, disable large page
-		 * support for this slot
-		 */
-		if ((slot->base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
-		    !largepages_enabled) {
-			unsigned long j;
-
-			for (j = 0; j < lpages; ++j)
-				slot->lpage_info[i][j].write_count = 1;
-		}
-	}
-
-	return 0;
-
-out_free:
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		vfree(slot->lpage_info[i]);
-		slot->lpage_info[i] = NULL;
-	}
-	return -ENOMEM;
-}
-#endif /* not defined CONFIG_S390 */
-
 /*
  * Allocate some memory and give it an address in the guest physical address
  * space.
@@ -833,7 +778,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		if (!new.rmap)
 			goto out_free;
 
-		if (create_lpage_info(&new, npages))
+		if (kvm_arch_create_memslot(&new, npages))
 			goto out_free;
 #endif /* not defined CONFIG_S390 */
 	}
@@ -893,8 +838,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	if (!npages) {
 		new.rmap = NULL;
 		new.dirty_bitmap = NULL;
-		for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i)
-			new.lpage_info[i] = NULL;
+		memset(&new.arch, 0, sizeof(new.arch));
 	}
 
 	update_memslots(slots, &new);
@@ -981,6 +925,11 @@ out:
 	return r;
 }
 
+bool kvm_largepages_enabled(void)
+{
+	return largepages_enabled;
+}
+
 void kvm_disable_largepages(void)
 {
 	largepages_enabled = false;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-30  3:53 ` [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it Takuya Yoshikawa
@ 2012-01-30  4:58   ` Takuya Yoshikawa
  2012-01-30  5:35   ` [PATCH 4/4 v3] " Takuya Yoshikawa
  1 sibling, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  4:58 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

(2012/01/30 12:53), Takuya Yoshikawa wrote:

> @@ -833,7 +778,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   		if (!new.rmap)
>   			goto out_free;
>
> -		if (create_lpage_info(&new, npages))
> +		if (kvm_arch_create_memslot(&new, npages))
>   			goto out_free;
>   #endif /* not defined CONFIG_S390 */

I have to move this #endif upwards to not include kvm_arch_create_memslot().

Will update patch 4 and resend from now.

	Takuya

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-30  3:53 ` [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it Takuya Yoshikawa
  2012-01-30  4:58   ` Takuya Yoshikawa
@ 2012-01-30  5:35   ` Takuya Yoshikawa
  2012-01-31  1:17     ` Takuya Yoshikawa
  1 sibling, 1 reply; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-30  5:35 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

Some members of kvm_memory_slot are not used by every architecture.

This patch is the first step to make this difference clear by
introducing kvm_memory_slot::arch;  lpage_info is moved into it.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 arch/ia64/include/asm/kvm_host.h    |    3 +
 arch/ia64/kvm/kvm-ia64.c            |   10 +++++
 arch/powerpc/include/asm/kvm_host.h |    3 +
 arch/powerpc/kvm/powerpc.c          |   10 +++++
 arch/s390/include/asm/kvm_host.h    |    3 +
 arch/s390/kvm/kvm-s390.c            |   10 +++++
 arch/x86/include/asm/kvm_host.h     |    9 ++++
 arch/x86/kvm/mmu.c                  |    2 +-
 arch/x86/kvm/x86.c                  |   59 +++++++++++++++++++++++++++++
 include/linux/kvm_host.h            |   11 ++---
 virt/kvm/kvm_main.c                 |   70 ++++------------------------------
 11 files changed, 122 insertions(+), 68 deletions(-)

diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
index 2689ee5..e35b3a8 100644
--- a/arch/ia64/include/asm/kvm_host.h
+++ b/arch/ia64/include/asm/kvm_host.h
@@ -459,6 +459,9 @@ struct kvm_sal_data {
 	unsigned long boot_gp;
 };
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch {
 	spinlock_t dirty_log_lock;
 
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index 8ca7261..d8ddbba 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1571,6 +1571,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 		struct kvm_memory_slot *memslot,
 		struct kvm_memory_slot old,
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index af438b1..b9188aa 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -212,6 +212,9 @@ struct revmap_entry {
 #define KVMPPC_PAGE_WRITETHRU	HPTE_R_W	/* 0x40 */
 #define KVMPPC_GOT_PAGE		0x80
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch {
 #ifdef CONFIG_KVM_BOOK3S_64_HV
 	unsigned long hpt_virt;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 0e21d15..00d7e34 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -281,6 +281,16 @@ long kvm_arch_dev_ioctl(struct file *filp,
 	return -EINVAL;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
                                    struct kvm_memory_slot *memslot,
                                    struct kvm_memory_slot old,
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index e630426..7343872 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -245,6 +245,9 @@ struct kvm_vm_stat {
 	u32 remote_tlb_flush;
 };
 
+struct kvm_arch_memory_slot {
+};
+
 struct kvm_arch{
 	struct sca_block *sca;
 	debug_info_t *dbf;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 0b91679..418a69c 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -807,6 +807,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
 	return VM_FAULT_SIGBUS;
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	return 0;
+}
+
 /* Section: memory related */
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				   struct kvm_memory_slot *memslot,
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 4610166..de3aa43 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -479,6 +479,15 @@ struct kvm_vcpu_arch {
 	} osvw;
 };
 
+struct kvm_lpage_info {
+	unsigned long rmap_pde;
+	int write_count;
+};
+
+struct kvm_arch_memory_slot {
+	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
+};
+
 struct kvm_arch {
 	unsigned int n_used_mmu_pages;
 	unsigned int n_requested_mmu_pages;
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 37e7f10..ff053ca 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -689,7 +689,7 @@ static struct kvm_lpage_info *lpage_info_slot(gfn_t gfn,
 	unsigned long idx;
 
 	idx = gfn_to_index(gfn, slot->base_gfn, level);
-	return &slot->lpage_info[level - 2][idx];
+	return &slot->arch.lpage_info[level - 2][idx];
 }
 
 static void account_shadowed(struct kvm *kvm, gfn_t gfn)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2bd77a3..45e4197 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6120,6 +6120,65 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 		put_page(kvm->arch.ept_identity_pagetable);
 }
 
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont)
+{
+	int i;
+
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		if (!dont || free->arch.lpage_info[i] != dont->arch.lpage_info[i]) {
+			vfree(free->arch.lpage_info[i]);
+			free->arch.lpage_info[i] = NULL;
+		}
+	}
+}
+
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
+{
+	int i;
+
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		unsigned long ugfn;
+		int lpages;
+		int level = i + 2;
+
+		lpages = gfn_to_index(slot->base_gfn + npages - 1,
+				      slot->base_gfn, level) + 1;
+
+		slot->arch.lpage_info[i] =
+			vzalloc(lpages * sizeof(*slot->arch.lpage_info[i]));
+		if (!slot->arch.lpage_info[i])
+			goto out_free;
+
+		if (slot->base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->arch.lpage_info[i][0].write_count = 1;
+		if ((slot->base_gfn + npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
+			slot->arch.lpage_info[i][lpages - 1].write_count = 1;
+		ugfn = slot->userspace_addr >> PAGE_SHIFT;
+		/*
+		 * If the gfn and userspace address are not aligned wrt each
+		 * other, or if explicitly asked to, disable large page
+		 * support for this slot
+		 */
+		if ((slot->base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
+		    !kvm_largepages_enabled()) {
+			unsigned long j;
+
+			for (j = 0; j < lpages; ++j)
+				slot->arch.lpage_info[i][j].write_count = 1;
+		}
+	}
+
+	return 0;
+
+out_free:
+	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
+		vfree(slot->arch.lpage_info[i]);
+		slot->arch.lpage_info[i] = NULL;
+	}
+	return -ENOMEM;
+}
+
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				struct kvm_memory_slot *memslot,
 				struct kvm_memory_slot old,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 06d4e41..1d833d7 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -171,11 +171,6 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
  */
 #define KVM_MEM_MAX_NR_PAGES ((1UL << 31) - 1)
 
-struct kvm_lpage_info {
-	unsigned long rmap_pde;
-	int write_count;
-};
-
 struct kvm_memory_slot {
 	gfn_t base_gfn;
 	unsigned long npages;
@@ -184,7 +179,7 @@ struct kvm_memory_slot {
 	unsigned long *dirty_bitmap;
 	unsigned long *dirty_bitmap_head;
 	unsigned long nr_dirty_pages;
-	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
+	struct kvm_arch_memory_slot arch;
 	unsigned long userspace_addr;
 	int user_alloc;
 	int id;
@@ -376,6 +371,9 @@ int kvm_set_memory_region(struct kvm *kvm,
 int __kvm_set_memory_region(struct kvm *kvm,
 			    struct kvm_userspace_memory_region *mem,
 			    int user_alloc);
+void kvm_arch_free_memslot(struct kvm_memory_slot *free,
+			   struct kvm_memory_slot *dont);
+int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages);
 int kvm_arch_prepare_memory_region(struct kvm *kvm,
 				struct kvm_memory_slot *memslot,
 				struct kvm_memory_slot old,
@@ -385,6 +383,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
 				struct kvm_userspace_memory_region *mem,
 				struct kvm_memory_slot old,
 				int user_alloc);
+bool kvm_largepages_enabled(void);
 void kvm_disable_largepages(void);
 void kvm_arch_flush_shadow(struct kvm *kvm);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index b6bc866..86cf7fb 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -535,21 +535,13 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot)
 static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
 				  struct kvm_memory_slot *dont)
 {
-	int i;
-
 	if (!dont || free->rmap != dont->rmap)
 		vfree(free->rmap);
 
 	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
 		kvm_destroy_dirty_bitmap(free);
 
-
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		if (!dont || free->lpage_info[i] != dont->lpage_info[i]) {
-			vfree(free->lpage_info[i]);
-			free->lpage_info[i] = NULL;
-		}
-	}
+	kvm_arch_free_memslot(free, dont);
 
 	free->npages = 0;
 	free->rmap = NULL;
@@ -698,53 +690,6 @@ void update_memslots(struct kvm_memslots *slots, struct kvm_memory_slot *new)
 	slots->generation++;
 }
 
-#ifndef CONFIG_S390
-static int create_lpage_info(struct kvm_memory_slot *slot, unsigned long npages)
-{
-	int i;
-
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		unsigned long ugfn;
-		int lpages;
-		int level = i + 2;
-
-		lpages = gfn_to_index(slot->base_gfn + npages - 1,
-				      slot->base_gfn, level) + 1;
-
-		slot->lpage_info[i] = vzalloc(lpages * sizeof(*slot->lpage_info[i]));
-		if (!slot->lpage_info[i])
-			goto out_free;
-
-		if (slot->base_gfn & (KVM_PAGES_PER_HPAGE(level) - 1))
-			slot->lpage_info[i][0].write_count = 1;
-		if ((slot->base_gfn + npages) & (KVM_PAGES_PER_HPAGE(level) - 1))
-			slot->lpage_info[i][lpages - 1].write_count = 1;
-		ugfn = slot->userspace_addr >> PAGE_SHIFT;
-		/*
-		 * If the gfn and userspace address are not aligned wrt each
-		 * other, or if explicitly asked to, disable large page
-		 * support for this slot
-		 */
-		if ((slot->base_gfn ^ ugfn) & (KVM_PAGES_PER_HPAGE(level) - 1) ||
-		    !largepages_enabled) {
-			unsigned long j;
-
-			for (j = 0; j < lpages; ++j)
-				slot->lpage_info[i][j].write_count = 1;
-		}
-	}
-
-	return 0;
-
-out_free:
-	for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i) {
-		vfree(slot->lpage_info[i]);
-		slot->lpage_info[i] = NULL;
-	}
-	return -ENOMEM;
-}
-#endif /* not defined CONFIG_S390 */
-
 /*
  * Allocate some memory and give it an address in the guest physical address
  * space.
@@ -832,10 +777,9 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		new.rmap = vzalloc(npages * sizeof(*new.rmap));
 		if (!new.rmap)
 			goto out_free;
-
-		if (create_lpage_info(&new, npages))
-			goto out_free;
 #endif /* not defined CONFIG_S390 */
+		if (kvm_arch_create_memslot(&new, npages))
+			goto out_free;
 	}
 
 	/* Allocate page dirty bitmap if needed */
@@ -893,8 +837,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	if (!npages) {
 		new.rmap = NULL;
 		new.dirty_bitmap = NULL;
-		for (i = 0; i < KVM_NR_PAGE_SIZES - 1; ++i)
-			new.lpage_info[i] = NULL;
+		memset(&new.arch, 0, sizeof(new.arch));
 	}
 
 	update_memslots(slots, &new);
@@ -981,6 +924,11 @@ out:
 	return r;
 }
 
+bool kvm_largepages_enabled(void)
+{
+	return largepages_enabled;
+}
+
 void kvm_disable_largepages(void)
 {
 	largepages_enabled = false;
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-30  5:35   ` [PATCH 4/4 v3] " Takuya Yoshikawa
@ 2012-01-31  1:17     ` Takuya Yoshikawa
  2012-01-31  7:51       ` Christian Borntraeger
                         ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-31  1:17 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm, Alexander Graf, cotte, Paul Mackerras, borntraeger

Added s390 and ppc developers to Cc,

(2012/01/30 14:35), Takuya Yoshikawa wrote:
> Some members of kvm_memory_slot are not used by every architecture.
>
> This patch is the first step to make this difference clear by
> introducing kvm_memory_slot::arch;  lpage_info is moved into it.

I am planning to move rmap stuff into arch next if this patch is accepted.

Please let me know if you have some opinion about which members should be
moved into this.


Thanks,
	Takuya


>
> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
> ---
>   arch/ia64/include/asm/kvm_host.h    |    3 +
>   arch/ia64/kvm/kvm-ia64.c            |   10 +++++
>   arch/powerpc/include/asm/kvm_host.h |    3 +
>   arch/powerpc/kvm/powerpc.c          |   10 +++++
>   arch/s390/include/asm/kvm_host.h    |    3 +
>   arch/s390/kvm/kvm-s390.c            |   10 +++++
>   arch/x86/include/asm/kvm_host.h     |    9 ++++
>   arch/x86/kvm/mmu.c                  |    2 +-
>   arch/x86/kvm/x86.c                  |   59 +++++++++++++++++++++++++++++
>   include/linux/kvm_host.h            |   11 ++---
>   virt/kvm/kvm_main.c                 |   70 ++++------------------------------
>   11 files changed, 122 insertions(+), 68 deletions(-)
>
> diff --git a/arch/ia64/include/asm/kvm_host.h b/arch/ia64/include/asm/kvm_host.h
> index 2689ee5..e35b3a8 100644
> --- a/arch/ia64/include/asm/kvm_host.h
> +++ b/arch/ia64/include/asm/kvm_host.h
> @@ -459,6 +459,9 @@ struct kvm_sal_data {
>   	unsigned long boot_gp;
>   };
>
> +struct kvm_arch_memory_slot {
> +};
> +
>   struct kvm_arch {
>   	spinlock_t dirty_log_lock;
>
> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
> index 8ca7261..d8ddbba 100644
> --- a/arch/ia64/kvm/kvm-ia64.c
> +++ b/arch/ia64/kvm/kvm-ia64.c
> @@ -1571,6 +1571,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
>   	return VM_FAULT_SIGBUS;
>   }
>
> +void kvm_arch_free_memslot(struct kvm_memory_slot *free,
> +			   struct kvm_memory_slot *dont)
> +{
> +}
> +
> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
> +{
> +	return 0;
> +}
> +
>   int kvm_arch_prepare_memory_region(struct kvm *kvm,
>   		struct kvm_memory_slot *memslot,
>   		struct kvm_memory_slot old,
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index af438b1..b9188aa 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -212,6 +212,9 @@ struct revmap_entry {
>   #define KVMPPC_PAGE_WRITETHRU	HPTE_R_W	/* 0x40 */
>   #define KVMPPC_GOT_PAGE		0x80
>
> +struct kvm_arch_memory_slot {
> +};
> +
>   struct kvm_arch {
>   #ifdef CONFIG_KVM_BOOK3S_64_HV
>   	unsigned long hpt_virt;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 0e21d15..00d7e34 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -281,6 +281,16 @@ long kvm_arch_dev_ioctl(struct file *filp,
>   	return -EINVAL;
>   }
>
> +void kvm_arch_free_memslot(struct kvm_memory_slot *free,
> +			   struct kvm_memory_slot *dont)
> +{
> +}
> +
> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
> +{
> +	return 0;
> +}
> +
>   int kvm_arch_prepare_memory_region(struct kvm *kvm,
>                                      struct kvm_memory_slot *memslot,
>                                      struct kvm_memory_slot old,
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index e630426..7343872 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -245,6 +245,9 @@ struct kvm_vm_stat {
>   	u32 remote_tlb_flush;
>   };
>
> +struct kvm_arch_memory_slot {
> +};
> +
>   struct kvm_arch{
>   	struct sca_block *sca;
>   	debug_info_t *dbf;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 0b91679..418a69c 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -807,6 +807,16 @@ int kvm_arch_vcpu_fault(struct kvm_vcpu *vcpu, struct vm_fault *vmf)
>   	return VM_FAULT_SIGBUS;
>   }
>
> +void kvm_arch_free_memslot(struct kvm_memory_slot *free,
> +			   struct kvm_memory_slot *dont)
> +{
> +}
> +
> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
> +{
> +	return 0;
> +}
> +
>   /* Section: memory related */
>   int kvm_arch_prepare_memory_region(struct kvm *kvm,
>   				   struct kvm_memory_slot *memslot,
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 4610166..de3aa43 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -479,6 +479,15 @@ struct kvm_vcpu_arch {
>   	} osvw;
>   };
>
> +struct kvm_lpage_info {
> +	unsigned long rmap_pde;
> +	int write_count;
> +};
> +
> +struct kvm_arch_memory_slot {
> +	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
> +};
> +
>   struct kvm_arch {
>   	unsigned int n_used_mmu_pages;
>   	unsigned int n_requested_mmu_pages;
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 37e7f10..ff053ca 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -689,7 +689,7 @@ static struct kvm_lpage_info *lpage_info_slot(gfn_t gfn,
>   	unsigned long idx;
>
>   	idx = gfn_to_index(gfn, slot->base_gfn, level);
> -	return&slot->lpage_info[level - 2][idx];
> +	return&slot->arch.lpage_info[level - 2][idx];
>   }
>
>   static void account_shadowed(struct kvm *kvm, gfn_t gfn)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2bd77a3..45e4197 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6120,6 +6120,65 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
>   		put_page(kvm->arch.ept_identity_pagetable);
>   }
>
> +void kvm_arch_free_memslot(struct kvm_memory_slot *free,
> +			   struct kvm_memory_slot *dont)
> +{
> +	int i;
> +
> +	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> +		if (!dont || free->arch.lpage_info[i] != dont->arch.lpage_info[i]) {
> +			vfree(free->arch.lpage_info[i]);
> +			free->arch.lpage_info[i] = NULL;
> +		}
> +	}
> +}
> +
> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages)
> +{
> +	int i;
> +
> +	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> +		unsigned long ugfn;
> +		int lpages;
> +		int level = i + 2;
> +
> +		lpages = gfn_to_index(slot->base_gfn + npages - 1,
> +				      slot->base_gfn, level) + 1;
> +
> +		slot->arch.lpage_info[i] =
> +			vzalloc(lpages * sizeof(*slot->arch.lpage_info[i]));
> +		if (!slot->arch.lpage_info[i])
> +			goto out_free;
> +
> +		if (slot->base_gfn&  (KVM_PAGES_PER_HPAGE(level) - 1))
> +			slot->arch.lpage_info[i][0].write_count = 1;
> +		if ((slot->base_gfn + npages)&  (KVM_PAGES_PER_HPAGE(level) - 1))
> +			slot->arch.lpage_info[i][lpages - 1].write_count = 1;
> +		ugfn = slot->userspace_addr>>  PAGE_SHIFT;
> +		/*
> +		 * If the gfn and userspace address are not aligned wrt each
> +		 * other, or if explicitly asked to, disable large page
> +		 * support for this slot
> +		 */
> +		if ((slot->base_gfn ^ ugfn)&  (KVM_PAGES_PER_HPAGE(level) - 1) ||
> +		    !kvm_largepages_enabled()) {
> +			unsigned long j;
> +
> +			for (j = 0; j<  lpages; ++j)
> +				slot->arch.lpage_info[i][j].write_count = 1;
> +		}
> +	}
> +
> +	return 0;
> +
> +out_free:
> +	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> +		vfree(slot->arch.lpage_info[i]);
> +		slot->arch.lpage_info[i] = NULL;
> +	}
> +	return -ENOMEM;
> +}
> +
>   int kvm_arch_prepare_memory_region(struct kvm *kvm,
>   				struct kvm_memory_slot *memslot,
>   				struct kvm_memory_slot old,
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 06d4e41..1d833d7 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -171,11 +171,6 @@ static inline int kvm_vcpu_exiting_guest_mode(struct kvm_vcpu *vcpu)
>    */
>   #define KVM_MEM_MAX_NR_PAGES ((1UL<<  31) - 1)
>
> -struct kvm_lpage_info {
> -	unsigned long rmap_pde;
> -	int write_count;
> -};
> -
>   struct kvm_memory_slot {
>   	gfn_t base_gfn;
>   	unsigned long npages;
> @@ -184,7 +179,7 @@ struct kvm_memory_slot {
>   	unsigned long *dirty_bitmap;
>   	unsigned long *dirty_bitmap_head;
>   	unsigned long nr_dirty_pages;
> -	struct kvm_lpage_info *lpage_info[KVM_NR_PAGE_SIZES - 1];
> +	struct kvm_arch_memory_slot arch;
>   	unsigned long userspace_addr;
>   	int user_alloc;
>   	int id;
> @@ -376,6 +371,9 @@ int kvm_set_memory_region(struct kvm *kvm,
>   int __kvm_set_memory_region(struct kvm *kvm,
>   			    struct kvm_userspace_memory_region *mem,
>   			    int user_alloc);
> +void kvm_arch_free_memslot(struct kvm_memory_slot *free,
> +			   struct kvm_memory_slot *dont);
> +int kvm_arch_create_memslot(struct kvm_memory_slot *slot, unsigned long npages);
>   int kvm_arch_prepare_memory_region(struct kvm *kvm,
>   				struct kvm_memory_slot *memslot,
>   				struct kvm_memory_slot old,
> @@ -385,6 +383,7 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
>   				struct kvm_userspace_memory_region *mem,
>   				struct kvm_memory_slot old,
>   				int user_alloc);
> +bool kvm_largepages_enabled(void);
>   void kvm_disable_largepages(void);
>   void kvm_arch_flush_shadow(struct kvm *kvm);
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index b6bc866..86cf7fb 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -535,21 +535,13 @@ static void kvm_destroy_dirty_bitmap(struct kvm_memory_slot *memslot)
>   static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
>   				  struct kvm_memory_slot *dont)
>   {
> -	int i;
> -
>   	if (!dont || free->rmap != dont->rmap)
>   		vfree(free->rmap);
>
>   	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
>   		kvm_destroy_dirty_bitmap(free);
>
> -
> -	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> -		if (!dont || free->lpage_info[i] != dont->lpage_info[i]) {
> -			vfree(free->lpage_info[i]);
> -			free->lpage_info[i] = NULL;
> -		}
> -	}
> +	kvm_arch_free_memslot(free, dont);
>
>   	free->npages = 0;
>   	free->rmap = NULL;
> @@ -698,53 +690,6 @@ void update_memslots(struct kvm_memslots *slots, struct kvm_memory_slot *new)
>   	slots->generation++;
>   }
>
> -#ifndef CONFIG_S390
> -static int create_lpage_info(struct kvm_memory_slot *slot, unsigned long npages)
> -{
> -	int i;
> -
> -	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> -		unsigned long ugfn;
> -		int lpages;
> -		int level = i + 2;
> -
> -		lpages = gfn_to_index(slot->base_gfn + npages - 1,
> -				      slot->base_gfn, level) + 1;
> -
> -		slot->lpage_info[i] = vzalloc(lpages * sizeof(*slot->lpage_info[i]));
> -		if (!slot->lpage_info[i])
> -			goto out_free;
> -
> -		if (slot->base_gfn&  (KVM_PAGES_PER_HPAGE(level) - 1))
> -			slot->lpage_info[i][0].write_count = 1;
> -		if ((slot->base_gfn + npages)&  (KVM_PAGES_PER_HPAGE(level) - 1))
> -			slot->lpage_info[i][lpages - 1].write_count = 1;
> -		ugfn = slot->userspace_addr>>  PAGE_SHIFT;
> -		/*
> -		 * If the gfn and userspace address are not aligned wrt each
> -		 * other, or if explicitly asked to, disable large page
> -		 * support for this slot
> -		 */
> -		if ((slot->base_gfn ^ ugfn)&  (KVM_PAGES_PER_HPAGE(level) - 1) ||
> -		    !largepages_enabled) {
> -			unsigned long j;
> -
> -			for (j = 0; j<  lpages; ++j)
> -				slot->lpage_info[i][j].write_count = 1;
> -		}
> -	}
> -
> -	return 0;
> -
> -out_free:
> -	for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i) {
> -		vfree(slot->lpage_info[i]);
> -		slot->lpage_info[i] = NULL;
> -	}
> -	return -ENOMEM;
> -}
> -#endif /* not defined CONFIG_S390 */
> -
>   /*
>    * Allocate some memory and give it an address in the guest physical address
>    * space.
> @@ -832,10 +777,9 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   		new.rmap = vzalloc(npages * sizeof(*new.rmap));
>   		if (!new.rmap)
>   			goto out_free;
> -
> -		if (create_lpage_info(&new, npages))
> -			goto out_free;
>   #endif /* not defined CONFIG_S390 */
> +		if (kvm_arch_create_memslot(&new, npages))
> +			goto out_free;
>   	}
>
>   	/* Allocate page dirty bitmap if needed */
> @@ -893,8 +837,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
>   	if (!npages) {
>   		new.rmap = NULL;
>   		new.dirty_bitmap = NULL;
> -		for (i = 0; i<  KVM_NR_PAGE_SIZES - 1; ++i)
> -			new.lpage_info[i] = NULL;
> +		memset(&new.arch, 0, sizeof(new.arch));
>   	}
>
>   	update_memslots(slots,&new);
> @@ -981,6 +924,11 @@ out:
>   	return r;
>   }
>
> +bool kvm_largepages_enabled(void)
> +{
> +	return largepages_enabled;
> +}
> +
>   void kvm_disable_largepages(void)
>   {
>   	largepages_enabled = false;


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-31  1:17     ` Takuya Yoshikawa
@ 2012-01-31  7:51       ` Christian Borntraeger
  2012-01-31 12:42         ` Takuya Yoshikawa
  2012-01-31  9:18       ` Avi Kivity
  2012-03-06 23:01       ` Alexander Graf
  2 siblings, 1 reply; 24+ messages in thread
From: Christian Borntraeger @ 2012-01-31  7:51 UTC (permalink / raw)
  To: Takuya Yoshikawa
  Cc: avi, mtosatti, kvm, Alexander Graf, cotte, Paul Mackerras

On 31/01/12 02:17, Takuya Yoshikawa wrote:
> Added s390 and ppc developers to Cc,
> 
> (2012/01/30 14:35), Takuya Yoshikawa wrote:
>> Some members of kvm_memory_slot are not used by every architecture.
>>
>> This patch is the first step to make this difference clear by
>> introducing kvm_memory_slot::arch;  lpage_info is moved into it.

Patch series seems to work on s390.

Christian


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-31  1:17     ` Takuya Yoshikawa
  2012-01-31  7:51       ` Christian Borntraeger
@ 2012-01-31  9:18       ` Avi Kivity
  2012-01-31  9:32         ` Takuya Yoshikawa
  2012-03-06 23:01       ` Alexander Graf
  2 siblings, 1 reply; 24+ messages in thread
From: Avi Kivity @ 2012-01-31  9:18 UTC (permalink / raw)
  To: Takuya Yoshikawa
  Cc: mtosatti, kvm, Alexander Graf, cotte, Paul Mackerras, borntraeger

On 01/31/2012 03:17 AM, Takuya Yoshikawa wrote:
> Added s390 and ppc developers to Cc,
>
> (2012/01/30 14:35), Takuya Yoshikawa wrote:
>> Some members of kvm_memory_slot are not used by every architecture.
>>
>> This patch is the first step to make this difference clear by
>> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
>
> I am planning to move rmap stuff into arch next if this patch is
> accepted.
>
> Please let me know if you have some opinion about which members should be
> moved into this.
>

Is there anything else?  Everything else seems to be generic.

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-31  9:18       ` Avi Kivity
@ 2012-01-31  9:32         ` Takuya Yoshikawa
  0 siblings, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-31  9:32 UTC (permalink / raw)
  To: Avi Kivity
  Cc: mtosatti, kvm, Alexander Graf, cotte, Paul Mackerras, borntraeger

(2012/01/31 18:18), Avi Kivity wrote:
> On 01/31/2012 03:17 AM, Takuya Yoshikawa wrote:
>> Added s390 and ppc developers to Cc,
>>
>> (2012/01/30 14:35), Takuya Yoshikawa wrote:
>>> Some members of kvm_memory_slot are not used by every architecture.
>>>
>>> This patch is the first step to make this difference clear by
>>> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
>>
>> I am planning to move rmap stuff into arch next if this patch is
>> accepted.
>>
>> Please let me know if you have some opinion about which members should be
>> moved into this.
>>
>
> Is there anything else?  Everything else seems to be generic.
>

About members, I agree.

But dirty_bitmap allocation/destruction should be implemented by
kvm_arch_create_*_dirty_bitmap().


Of course I may want to add another x86 specific member in the future.

	Takuya

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-31  7:51       ` Christian Borntraeger
@ 2012-01-31 12:42         ` Takuya Yoshikawa
  0 siblings, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-01-31 12:42 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Takuya Yoshikawa, avi, mtosatti, kvm, Alexander Graf, cotte,
	Paul Mackerras

Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> >> Some members of kvm_memory_slot are not used by every architecture.
> >>
> >> This patch is the first step to make this difference clear by
> >> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
> 
> Patch series seems to work on s390.
> 
> Christian
> 

Thanks!

	Takuya

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
                   ` (3 preceding siblings ...)
  2012-01-30  3:53 ` [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it Takuya Yoshikawa
@ 2012-02-06 10:10 ` Takuya Yoshikawa
  2012-02-06 10:10   ` Avi Kivity
  2012-02-07 17:42 ` Marcelo Tosatti
  5 siblings, 1 reply; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-02-06 10:10 UTC (permalink / raw)
  To: avi, mtosatti; +Cc: kvm

(2012/01/30 12:48), Takuya Yoshikawa wrote:
> This is the first step to separate the architecture specific members.
> The rmap and dirty_bitmap can be treated later based on this.


Any further comments?


If patch 4 (v3) looks controversial, please consider taking
patch 1 to 3.

I may use gfn_to_index() for dirty logging.


Thanks,
	Takuya

>
> v1->v2:
>
>    Patch 3:
>    - Removed extra checks for NULL when we create a new slot.
>    - Removed "if (user_alloc)" check taken from the s390 code.
>
>    Patch 4:
>    - Just rebased.
>
>          Takuya
>
>   arch/ia64/include/asm/kvm_host.h    |    3 +
>   arch/ia64/kvm/kvm-ia64.c            |   10 +++++
>   arch/powerpc/include/asm/kvm_host.h |    3 +
>   arch/powerpc/kvm/powerpc.c          |   10 +++++
>   arch/s390/include/asm/kvm_host.h    |    3 +
>   arch/s390/kvm/kvm-s390.c            |   10 +++++
>   arch/x86/include/asm/kvm_host.h     |    9 ++++
>   arch/x86/kvm/mmu.c                  |    5 +-
>   arch/x86/kvm/x86.c                  |   59 +++++++++++++++++++++++++++
>   include/linux/kvm_host.h            |   18 +++++---
>   virt/kvm/kvm_main.c                 |   76 ++++++----------------------------
>   11 files changed, 135 insertions(+), 71 deletions(-)
>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch
  2012-02-06 10:10 ` [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
@ 2012-02-06 10:10   ` Avi Kivity
  0 siblings, 0 replies; 24+ messages in thread
From: Avi Kivity @ 2012-02-06 10:10 UTC (permalink / raw)
  To: Takuya Yoshikawa; +Cc: mtosatti, kvm

On 02/06/2012 12:10 PM, Takuya Yoshikawa wrote:
> (2012/01/30 12:48), Takuya Yoshikawa wrote:
>> This is the first step to separate the architecture specific members.
>> The rmap and dirty_bitmap can be treated later based on this.
>
>
> Any further comments?
>
>
> If patch 4 (v3) looks controversial, please consider taking
> patch 1 to 3.
>

No, patch 4 seems fine.  I was just waiting for more comments, and now
it's Marcelo's turn on kvm.git.

> I may use gfn_to_index() for dirty logging.
>
>

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch
  2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
                   ` (4 preceding siblings ...)
  2012-02-06 10:10 ` [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
@ 2012-02-07 17:42 ` Marcelo Tosatti
  5 siblings, 0 replies; 24+ messages in thread
From: Marcelo Tosatti @ 2012-02-07 17:42 UTC (permalink / raw)
  To: Takuya Yoshikawa; +Cc: avi, kvm

On Mon, Jan 30, 2012 at 12:48:59PM +0900, Takuya Yoshikawa wrote:
> This is the first step to separate the architecture specific members.
> The rmap and dirty_bitmap can be treated later based on this.
> 
> v1->v2:
> 
>   Patch 3:
>   - Removed extra checks for NULL when we create a new slot.
>   - Removed "if (user_alloc)" check taken from the s390 code.
> 
>   Patch 4:
>   - Just rebased.

Patch 3 does not apply, please rebase and resend whole series.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-01-31  1:17     ` Takuya Yoshikawa
  2012-01-31  7:51       ` Christian Borntraeger
  2012-01-31  9:18       ` Avi Kivity
@ 2012-03-06 23:01       ` Alexander Graf
  2012-03-07  4:46         ` Takuya Yoshikawa
  2012-03-07 21:58         ` Paul Mackerras
  2 siblings, 2 replies; 24+ messages in thread
From: Alexander Graf @ 2012-03-06 23:01 UTC (permalink / raw)
  To: Takuya Yoshikawa; +Cc: avi, mtosatti, kvm, cotte, Paul Mackerras, borntraeger


On 31.01.2012, at 02:17, Takuya Yoshikawa wrote:

> Added s390 and ppc developers to Cc,
> 
> (2012/01/30 14:35), Takuya Yoshikawa wrote:
>> Some members of kvm_memory_slot are not used by every architecture.
>> 
>> This patch is the first step to make this difference clear by
>> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
> 
> I am planning to move rmap stuff into arch next if this patch is accepted.
> 
> Please let me know if you have some opinion about which members should be
> moved into this.

What is this lpage stuff? When do we need it? Right now the code gets executed on ppc, right? And with the patch it doesn't, no?


Alex


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-06 23:01       ` Alexander Graf
@ 2012-03-07  4:46         ` Takuya Yoshikawa
  2012-03-07 13:27           ` Alexander Graf
  2012-03-07 21:58         ` Paul Mackerras
  1 sibling, 1 reply; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-03-07  4:46 UTC (permalink / raw)
  To: Alexander Graf; +Cc: avi, mtosatti, kvm, cotte, Paul Mackerras, borntraeger

Alexander Graf <agraf@suse.de> wrote:

> >> This patch is the first step to make this difference clear by
> >> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
> > 
> > I am planning to move rmap stuff into arch next if this patch is accepted.
> > 
> > Please let me know if you have some opinion about which members should be
> > moved into this.
> 
> What is this lpage stuff? When do we need it? Right now the code gets executed on ppc, right? And with the patch it doesn't, no?

lpage_info is used for supporting huge pages on kvm-x86: we have
write_count and rmap_pde for each largepage and these are in lpage_info.

At the time I made this patch, it seemed that only kvm-x86 supported
huge pages, on ppc the array should be empty:

/* We don't currently support large pages. */
#define KVM_HPAGE_GFN_SHIFT(x)  0
#define KVM_NR_PAGE_SIZES       1

How each architecture supports huge pages will differ a lot.
So this kind of memory consuming stuff should be arch specific.

IMO rmap also should to be moved into the arch.
s390 does not need it and other architectures than x86 will be happy if
the type of it can be changed from unsigned long to a pointer, no?

	Takuya

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07  4:46         ` Takuya Yoshikawa
@ 2012-03-07 13:27           ` Alexander Graf
  2012-03-07 14:04             ` Avi Kivity
  2012-03-07 14:15             ` Takuya Yoshikawa
  0 siblings, 2 replies; 24+ messages in thread
From: Alexander Graf @ 2012-03-07 13:27 UTC (permalink / raw)
  To: Takuya Yoshikawa
  Cc: avi, mtosatti, kvm, cotte, Paul Mackerras, borntraeger,
	Andrea Arcangeli

On 03/07/2012 05:46 AM, Takuya Yoshikawa wrote:
> Alexander Graf<agraf@suse.de>  wrote:
>
>>>> This patch is the first step to make this difference clear by
>>>> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
>>> I am planning to move rmap stuff into arch next if this patch is accepted.
>>>
>>> Please let me know if you have some opinion about which members should be
>>> moved into this.
>> What is this lpage stuff? When do we need it? Right now the code gets executed on ppc, right? And with the patch it doesn't, no?
> lpage_info is used for supporting huge pages on kvm-x86: we have
> write_count and rmap_pde for each largepage and these are in lpage_info.
>
> At the time I made this patch, it seemed that only kvm-x86 supported
> huge pages, on ppc the array should be empty:

Hrm. I suppose this refers to transparent huge pages? Andrea, Paul, is 
there anything keeping is from also needing/using that logic?

> /* We don't currently support large pages. */
> #define KVM_HPAGE_GFN_SHIFT(x)  0
> #define KVM_NR_PAGE_SIZES       1
>
> How each architecture supports huge pages will differ a lot.
> So this kind of memory consuming stuff should be arch specific.

Yeah, probably.

> IMO rmap also should to be moved into the arch.
> s390 does not need it and other architectures than x86 will be happy if
> the type of it can be changed from unsigned long to a pointer, no?

How would an unsigned long make a difference over a pointer?


Alex


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07 13:27           ` Alexander Graf
@ 2012-03-07 14:04             ` Avi Kivity
  2012-03-07 14:57               ` Alexander Graf
  2012-03-07 14:15             ` Takuya Yoshikawa
  1 sibling, 1 reply; 24+ messages in thread
From: Avi Kivity @ 2012-03-07 14:04 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Takuya Yoshikawa, mtosatti, kvm, cotte, Paul Mackerras,
	borntraeger, Andrea Arcangeli

On 03/07/2012 03:27 PM, Alexander Graf wrote:
>> At the time I made this patch, it seemed that only kvm-x86 supported
>> huge pages, on ppc the array should be empty:
>
>
> Hrm. I suppose this refers to transparent huge pages? 

Just huge pages.  Whether they are static or dynamic is immaterial in
this context.

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07 13:27           ` Alexander Graf
  2012-03-07 14:04             ` Avi Kivity
@ 2012-03-07 14:15             ` Takuya Yoshikawa
  1 sibling, 0 replies; 24+ messages in thread
From: Takuya Yoshikawa @ 2012-03-07 14:15 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Takuya Yoshikawa, avi, mtosatti, kvm, cotte, Paul Mackerras,
	borntraeger, Andrea Arcangeli

Alexander Graf <agraf@suse.de> wrote:

> > IMO rmap also should to be moved into the arch.
> > s390 does not need it and other architectures than x86 will be happy if
> > the type of it can be changed from unsigned long to a pointer, no?
> 
> How would an unsigned long make a difference over a pointer?

Not so much.  Just a matter of casting.

x86 is using the least significant bit for special encoding, so unsigned long
is natural.

My initial motivation was to make rmap multi-dimensional so that it can also
hold rmap_pde.

	Takuya

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07 14:04             ` Avi Kivity
@ 2012-03-07 14:57               ` Alexander Graf
  2012-03-07 15:03                 ` Avi Kivity
  0 siblings, 1 reply; 24+ messages in thread
From: Alexander Graf @ 2012-03-07 14:57 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Takuya Yoshikawa, mtosatti, kvm, cotte, Paul Mackerras,
	borntraeger, Andrea Arcangeli

On 03/07/2012 03:04 PM, Avi Kivity wrote:
> On 03/07/2012 03:27 PM, Alexander Graf wrote:
>>> At the time I made this patch, it seemed that only kvm-x86 supported
>>> huge pages, on ppc the array should be empty:
>>
>> Hrm. I suppose this refers to transparent huge pages?
> Just huge pages.  Whether they are static or dynamic is immaterial in
> this context.

Well, book3s_hv and e500 support hugetlbfs. I've never had to touch that 
patches code though - so I guess I'm still not really understanding what 
it's there for O_o.


Alex


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07 14:57               ` Alexander Graf
@ 2012-03-07 15:03                 ` Avi Kivity
  2012-03-07 15:28                   ` Alexander Graf
  0 siblings, 1 reply; 24+ messages in thread
From: Avi Kivity @ 2012-03-07 15:03 UTC (permalink / raw)
  To: Alexander Graf
  Cc: Takuya Yoshikawa, mtosatti, kvm, cotte, Paul Mackerras,
	borntraeger, Andrea Arcangeli

On 03/07/2012 04:57 PM, Alexander Graf wrote:
> On 03/07/2012 03:04 PM, Avi Kivity wrote:
>> On 03/07/2012 03:27 PM, Alexander Graf wrote:
>>>> At the time I made this patch, it seemed that only kvm-x86 supported
>>>> huge pages, on ppc the array should be empty:
>>>
>>> Hrm. I suppose this refers to transparent huge pages?
>> Just huge pages.  Whether they are static or dynamic is immaterial in
>> this context.
>
> Well, book3s_hv and e500 support hugetlbfs. I've never had to touch
> that patches code though - so I guess I'm still not really
> understanding what it's there for O_o.
>

The kvm hugepage code uses large sptes to map large pages, when
available (either via hugetlbfs or transparent hugepages).  Since x86
supports swapping, and needs to write-protect pages for dirty logging
and for shadowing guest pagetables, it needs a reverse map from pages to
sptes.  The data structure we're discussing is part of the reverse map
for large pages.

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-07 15:03                 ` Avi Kivity
@ 2012-03-07 15:28                   ` Alexander Graf
  0 siblings, 0 replies; 24+ messages in thread
From: Alexander Graf @ 2012-03-07 15:28 UTC (permalink / raw)
  To: Avi Kivity
  Cc: Takuya Yoshikawa, mtosatti, kvm, cotte, Paul Mackerras,
	borntraeger, Andrea Arcangeli

On 03/07/2012 04:03 PM, Avi Kivity wrote:
> On 03/07/2012 04:57 PM, Alexander Graf wrote:
>> On 03/07/2012 03:04 PM, Avi Kivity wrote:
>>> On 03/07/2012 03:27 PM, Alexander Graf wrote:
>>>>> At the time I made this patch, it seemed that only kvm-x86 supported
>>>>> huge pages, on ppc the array should be empty:
>>>> Hrm. I suppose this refers to transparent huge pages?
>>> Just huge pages.  Whether they are static or dynamic is immaterial in
>>> this context.
>> Well, book3s_hv and e500 support hugetlbfs. I've never had to touch
>> that patches code though - so I guess I'm still not really
>> understanding what it's there for O_o.
>>
> The kvm hugepage code uses large sptes to map large pages, when
> available (either via hugetlbfs or transparent hugepages).  Since x86
> supports swapping, and needs to write-protect pages for dirty logging
> and for shadowing guest pagetables, it needs a reverse map from pages to
> sptes.  The data structure we're discussing is part of the reverse map
> for large pages.

Ah, now that makes more sense. On booke, we don't do rmap yet. On 
book3s_hv, IIRC Paul did implement something, so I'd like to hear his 
opinion on it really.

Alex


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 4/4 v3] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it
  2012-03-06 23:01       ` Alexander Graf
  2012-03-07  4:46         ` Takuya Yoshikawa
@ 2012-03-07 21:58         ` Paul Mackerras
  1 sibling, 0 replies; 24+ messages in thread
From: Paul Mackerras @ 2012-03-07 21:58 UTC (permalink / raw)
  To: Alexander Graf; +Cc: Takuya Yoshikawa, avi, mtosatti, kvm, cotte, borntraeger

On Wed, Mar 07, 2012 at 12:01:38AM +0100, Alexander Graf wrote:
> 
> On 31.01.2012, at 02:17, Takuya Yoshikawa wrote:
> 
> > Added s390 and ppc developers to Cc,
> > 
> > (2012/01/30 14:35), Takuya Yoshikawa wrote:
> >> Some members of kvm_memory_slot are not used by every architecture.
> >> 
> >> This patch is the first step to make this difference clear by
> >> introducing kvm_memory_slot::arch;  lpage_info is moved into it.
> > 
> > I am planning to move rmap stuff into arch next if this patch is accepted.
> > 
> > Please let me know if you have some opinion about which members should be
> > moved into this.
> 
> What is this lpage stuff? When do we need it? Right now the code
> gets executed on ppc, right? And with the patch it doesn't, no?

We do support large pages backing the guest on powerpc, at least for
the Book3S_HV style of KVM, but we don't use the lpage_info array.
The reason is that we only allow the guest to create large-page PTEs
in regions which are backed by large pages on the host side (and which
are therefore large-page aligned on both the host and guest side).  We
can enforce that because guests use a hypercall to create PTEs in the
hashed page table, and we have a way (via the device tree) to tell the
guest what page sizes it can use.

In contrast, on x86 we have no control over what PTEs the guest
creates in its page tables, so it can create large-page PTEs inside a
region which is backed by small pages, and which might not be
large-page aligned.  This is why we have the separate arrays pointed
to by lpage_info and why there is the logic in kvm_main.c for handling
misalignment at the ends.

So, at the moment on Book3S_HV, I have one entry in the rmap array for
each small page in a memslot.  Each entry is an unsigned long and
contains some control bits (dirty and referenced bits, among others)
and the index in the hashed page table (HPT) of one guest PTE that
references that page.  There is another array that then forms a
doubly-linked circular list of all the guest PTEs that reference the
page.  At present, guest PTEs are linked into the rmap lists based on
the starting address of the page irrespective of the page size, so a
large-page guest PTE gets linked into the same list as a small-page
guest PTE mapping the first small page of the large page.  That isn't
ideal from the point of view of dirty and reference tracking, so I
will probably move to having separate lists for the different page
sizes, meaning I will need something like the lpage_info array, but
I won't need the logic that is currently in kvm_main.c for handling
it.

Paul.


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2012-03-07 21:59 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-30  3:48 [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
2012-01-30  3:50 ` [PATCH 1/4] KVM: Introduce gfn_to_index() which returns the index for a given level Takuya Yoshikawa
2012-01-30  3:50 ` [PATCH 2/4] KVM: Split lpage_info creation out from __kvm_set_memory_region() Takuya Yoshikawa
2012-01-30  3:51 ` [PATCH 3/4 v2] KVM: Simplify ifndef conditional usage in __kvm_set_memory_region() Takuya Yoshikawa
2012-01-30  3:53 ` [PATCH 4/4 v2] KVM: Introduce kvm_memory_slot::arch and move lpage_info into it Takuya Yoshikawa
2012-01-30  4:58   ` Takuya Yoshikawa
2012-01-30  5:35   ` [PATCH 4/4 v3] " Takuya Yoshikawa
2012-01-31  1:17     ` Takuya Yoshikawa
2012-01-31  7:51       ` Christian Borntraeger
2012-01-31 12:42         ` Takuya Yoshikawa
2012-01-31  9:18       ` Avi Kivity
2012-01-31  9:32         ` Takuya Yoshikawa
2012-03-06 23:01       ` Alexander Graf
2012-03-07  4:46         ` Takuya Yoshikawa
2012-03-07 13:27           ` Alexander Graf
2012-03-07 14:04             ` Avi Kivity
2012-03-07 14:57               ` Alexander Graf
2012-03-07 15:03                 ` Avi Kivity
2012-03-07 15:28                   ` Alexander Graf
2012-03-07 14:15             ` Takuya Yoshikawa
2012-03-07 21:58         ` Paul Mackerras
2012-02-06 10:10 ` [PATCH 0/4] KVM: Introduce kvm_memory_slot::arch Takuya Yoshikawa
2012-02-06 10:10   ` Avi Kivity
2012-02-07 17:42 ` Marcelo Tosatti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).