mmu root page cleanups

PATCH In review Avi Kivity 1 participant 7 messages Apr 26, 2010

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Avi Kivity

author

2010-04-26 08:48 UTC #0 lore raw

cover mmu root page cleanups

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(-)

Avi Kivity

author

2010-04-26 08:48 UTC #1 lore raw

PATCH 1/5 KVM: MMU: Rearrange struct kvm_mmu_page

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

Signed-off-by Avi Kivity <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

arch/x86/include/asm/kvm_host.h +8 -7

index 3f0007b..cdaaedc 100644
@@ -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);
 };

Signature · 1 line

1.7.0.4

Avi Kivity

author

2010-04-26 08:48 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/include/asm/kvm_host.h +1 -1

index cdaaedc..f38007d 100644
@@ -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 {

Signature · 1 line

1.7.0.4

Avi Kivity

author

2010-04-26 08:48 UTC #3 lore raw

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

In reply to Avi Kivity

Reduce code duplication.

Signed-off-by Avi Kivity <[email protected]>

---
 arch/x86/kvm/mmu.c |   44 +++++++++++++++++++++-----------------------
 1 files changed, 21 insertions(+), 23 deletions(-)

arch/x86/kvm/mmu.c +21 ~~-23~~

index ddfa865..4da4ff1 100644
@@ -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;
 }

Signature · 1 line

1.7.0.4

Avi Kivity

author

2010-04-26 08:48 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 arch/x86/kvm/mmu.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

arch/x86/kvm/mmu.c +5 -4

index 4da4ff1..6e925b3 100644
@@ -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))

Signature · 1 line

1.7.0.4

Avi Kivity

author

2010-04-26 08:48 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 arch/x86/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/x86/kvm/mmu.c +1 -1

index 6e925b3..e3acc9e 100644
@@ -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,

Signature · 1 line

1.7.0.4

Avi Kivity

author

2010-04-26 09:23 UTC #6 lore raw

In reply to Avi Kivity

Quoted 5 lines · 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.

Breaks during testing, please don't apply.

Signature · 1 line

error compiling committee.c: too many arguments to function