[PATCH 0/5] mmu root page cleanups

[PATCH 0/5] mmu root page cleanups

[Avi Kivity]

I tried to separate direct and indirect page allocation to reduce memory
usage with tdp, but that turned out too messy (and not much of a win - we
allocate 8K per page outside the struct kvm_mmu_page).  However the cleanups
on the way are worthwhile.

Avi Kivity (5):
  KVM: MMU: Rearrange struct kvm_mmu_page
  KVM: MMU: use 16 bits for root_count
  KVM: MMU: Unify 32-pae and single-root mmu setup
  KVM: MMU: Use correct root gfn for direct maps
  KVM: MMU: Fix check for cr3 outside guest memory

 arch/x86/include/asm/kvm_host.h |   15 ++++++-----
 arch/x86/kvm/mmu.c              |   53 +++++++++++++++++++--------------------
 2 files changed, 34 insertions(+), 34 deletions(-)

[PATCH 1/5] KVM: MMU: Rearrange struct kvm_mmu_page

[Avi Kivity]

Put all members required for direct pages together, to reduce cache footprint
for those pages.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3f0007b..cdaaedc 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -192,6 +192,13 @@ struct kvm_mmu_page {
 	 */
 	gfn_t gfn;
 	union kvm_mmu_page_role role;
+	int root_count;           /* Currently serving as active root */
+	bool multimapped;         /* More than one parent_pte? */
+	bool unsync;
+	union {
+		u64 *parent_pte;               /* !multimapped */
+		struct hlist_head parent_ptes; /* multimapped, kvm_pte_chain */
+	};
 
 	u64 *spt;
 	/* hold the gfn of each spte inside spt */
@@ -201,14 +208,8 @@ struct kvm_mmu_page {
 	 * in this shadow page.
 	 */
 	DECLARE_BITMAP(slot_bitmap, KVM_MEMORY_SLOTS + KVM_PRIVATE_MEM_SLOTS);
-	bool multimapped;         /* More than one parent_pte? */
-	bool unsync;
-	int root_count;          /* Currently serving as active root */
+
 	unsigned int unsync_children;
-	union {
-		u64 *parent_pte;               /* !multimapped */
-		struct hlist_head parent_ptes; /* multimapped, kvm_pte_chain */
-	};
 	DECLARE_BITMAP(unsync_child_bitmap, 512);
 };
 
-- 
1.7.0.4

[PATCH 2/5] KVM: MMU: use 16 bits for root_count

[Avi Kivity]

This is incremented by a maximum of 4 for every vcpu, so we are far from
overflow.  16 bits improve struct packing.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index cdaaedc..f38007d 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -192,7 +192,7 @@ struct kvm_mmu_page {
 	 */
 	gfn_t gfn;
 	union kvm_mmu_page_role role;
-	int root_count;           /* Currently serving as active root */
+	short root_count;         /* Currently serving as active root */
 	bool multimapped;         /* More than one parent_pte? */
 	bool unsync;
 	union {
-- 
1.7.0.4

[PATCH 3/5] KVM: MMU: Unify 32-pae and single-root mmu setup

[Avi Kivity]

Reduce code duplication.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |   44 +++++++++++++++++++++-----------------------
 1 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index ddfa865..4da4ff1 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2052,41 +2052,36 @@ static int mmu_alloc_roots(struct kvm_vcpu *vcpu)
 	struct kvm_mmu_page *sp;
 	int direct = 0;
 	u64 pdptr;
+	hpa_t *rootp, root, root_flags;
+	int nr_roots;
 
-	root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
+	direct = tdp_enabled || !is_paging(vcpu);
 
 	if (vcpu->arch.mmu.shadow_root_level == PT64_ROOT_LEVEL) {
-		hpa_t root = vcpu->arch.mmu.root_hpa;
-
-		ASSERT(!VALID_PAGE(root));
-		if (tdp_enabled)
-			direct = 1;
-		if (mmu_check_root(vcpu, root_gfn))
-			return 1;
-		sp = kvm_mmu_get_page(vcpu, root_gfn, 0,
-				      PT64_ROOT_LEVEL, direct,
-				      ACC_ALL, NULL);
-		root = __pa(sp->spt);
-		++sp->root_count;
-		vcpu->arch.mmu.root_hpa = root;
-		return 0;
+		rootp = &vcpu->arch.mmu.root_hpa;
+		nr_roots = 1;
+		root_flags = 0;
+	} else {
+		rootp = vcpu->arch.mmu.pae_root;
+		nr_roots = 4;
+		root_flags = PT_PRESENT_MASK;
 	}
-	direct = !is_paging(vcpu);
-	if (tdp_enabled)
-		direct = 1;
-	for (i = 0; i < 4; ++i) {
-		hpa_t root = vcpu->arch.mmu.pae_root[i];
 
+	for (i = 0; i < nr_roots; ++i) {
+		root = rootp[i];
 		ASSERT(!VALID_PAGE(root));
 		if (vcpu->arch.mmu.root_level == PT32E_ROOT_LEVEL) {
 			pdptr = kvm_pdptr_read(vcpu, i);
 			if (!is_present_gpte(pdptr)) {
-				vcpu->arch.mmu.pae_root[i] = 0;
+				rootp[i] = 0;
 				continue;
 			}
 			root_gfn = pdptr >> PAGE_SHIFT;
 		} else if (vcpu->arch.mmu.root_level == 0)
 			root_gfn = 0;
+		else
+			root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
+
 		if (mmu_check_root(vcpu, root_gfn))
 			return 1;
 		sp = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
@@ -2094,9 +2089,12 @@ static int mmu_alloc_roots(struct kvm_vcpu *vcpu)
 				      ACC_ALL, NULL);
 		root = __pa(sp->spt);
 		++sp->root_count;
-		vcpu->arch.mmu.pae_root[i] = root | PT_PRESENT_MASK;
+		rootp[i] = root | root_flags;
 	}
-	vcpu->arch.mmu.root_hpa = __pa(vcpu->arch.mmu.pae_root);
+
+	if (nr_roots == 4)
+		vcpu->arch.mmu.root_hpa = __pa(rootp);
+
 	return 0;
 }
 
-- 
1.7.0.4

[PATCH 4/5] KVM: MMU: Use correct root gfn for direct maps

[Avi Kivity]

We currently use cr3, which is a random number for direct maps.  Use zero
instead (the start of the linear range mapped by the root).

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 4da4ff1..6e925b3 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2070,16 +2070,17 @@ static int mmu_alloc_roots(struct kvm_vcpu *vcpu)
 	for (i = 0; i < nr_roots; ++i) {
 		root = rootp[i];
 		ASSERT(!VALID_PAGE(root));
-		if (vcpu->arch.mmu.root_level == PT32E_ROOT_LEVEL) {
+
+		if (direct)
+			root_gfn = 0;
+		else if (vcpu->arch.mmu.root_level == PT32E_ROOT_LEVEL) {
 			pdptr = kvm_pdptr_read(vcpu, i);
 			if (!is_present_gpte(pdptr)) {
 				rootp[i] = 0;
 				continue;
 			}
 			root_gfn = pdptr >> PAGE_SHIFT;
-		} else if (vcpu->arch.mmu.root_level == 0)
-			root_gfn = 0;
-		else
+		} else
 			root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
 
 		if (mmu_check_root(vcpu, root_gfn))
-- 
1.7.0.4

[PATCH 5/5] KVM: MMU: Fix check for cr3 outside guest memory

[Avi Kivity]

Fir direct maps, root_gfn or cr3 are meaningless.  This hasn't bitten us
because no sane guest sets cr3 outside its own memory even in real mode.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 6e925b3..e3acc9e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2083,7 +2083,7 @@ static int mmu_alloc_roots(struct kvm_vcpu *vcpu)
 		} else
 			root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT;
 
-		if (mmu_check_root(vcpu, root_gfn))
+		if (!direct && mmu_check_root(vcpu, root_gfn))
 			return 1;
 		sp = kvm_mmu_get_page(vcpu, root_gfn, i << 30,
 				      PT32_ROOT_LEVEL, direct,
-- 
1.7.0.4

Re: [PATCH 0/5] mmu root page cleanups

[Avi Kivity]

On 04/26/2010 11:48 AM, Avi Kivity wrote:
> I tried to separate direct and indirect page allocation to reduce memory
> usage with tdp, but that turned out too messy (and not much of a win - we
> allocate 8K per page outside the struct kvm_mmu_page).  However the cleanups
> on the way are worthwhile.
>    

Breaks during testing, please don't apply.

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