[PATCH 1/2] KVM MMU: cleanup/fix mmu audit code

[PATCH 1/2] KVM MMU: cleanup/fix mmu audit code

[Xiao Guangrong]

This patch does:
- 'sp' parameter in inspect_spte_fn() is not used, so remove it
- fix 'kvm' and 'slots' is not defined in count_rmaps()
- fix a bug in inspect_spte_has_rmap()

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/mmu.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d7700bb..5de92ae 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3178,8 +3178,7 @@ static gva_t canonicalize(gva_t gva)
 }
 
 
-typedef void (*inspect_spte_fn) (struct kvm *kvm, struct kvm_mmu_page *sp,
-				 u64 *sptep);
+typedef void (*inspect_spte_fn) (struct kvm *kvm, u64 *sptep);
 
 static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp,
 			    inspect_spte_fn fn)
@@ -3195,7 +3194,7 @@ static void __mmu_spte_walk(struct kvm *kvm, struct kvm_mmu_page *sp,
 				child = page_header(ent & PT64_BASE_ADDR_MASK);
 				__mmu_spte_walk(kvm, child, fn);
 			} else
-				fn(kvm, sp, &sp->spt[i]);
+				fn(kvm, &sp->spt[i]);
 		}
 	}
 }
@@ -3286,6 +3285,8 @@ static void audit_mappings(struct kvm_vcpu *vcpu)
 
 static int count_rmaps(struct kvm_vcpu *vcpu)
 {
+	struct kvm *kvm = vcpu->kvm;
+	struct kvm_memslots *slots;
 	int nmaps = 0;
 	int i, j, k, idx;
 
@@ -3319,7 +3320,7 @@ static int count_rmaps(struct kvm_vcpu *vcpu)
 	return nmaps;
 }
 
-void inspect_spte_has_rmap(struct kvm *kvm, struct kvm_mmu_page *sp, u64 *sptep)
+void inspect_spte_has_rmap(struct kvm *kvm, u64 *sptep)
 {
 	unsigned long *rmapp;
 	struct kvm_mmu_page *rev_sp;
@@ -3335,14 +3336,14 @@ void inspect_spte_has_rmap(struct kvm *kvm, struct kvm_mmu_page *sp, u64 *sptep)
 			printk(KERN_ERR "%s: no memslot for gfn %ld\n",
 					 audit_msg, gfn);
 			printk(KERN_ERR "%s: index %ld of sp (gfn=%lx)\n",
-					audit_msg, sptep - rev_sp->spt,
+			       audit_msg, (long int)(sptep - rev_sp->spt),
 					rev_sp->gfn);
 			dump_stack();
 			return;
 		}
 
 		rmapp = gfn_to_rmap(kvm, rev_sp->gfns[sptep - rev_sp->spt],
-				    is_large_pte(*sptep));
+				    rev_sp->role.level);
 		if (!*rmapp) {
 			if (!printk_ratelimit())
 				return;
@@ -3377,7 +3378,7 @@ static void check_writable_mappings_rmap(struct kvm_vcpu *vcpu)
 				continue;
 			if (!(ent & PT_WRITABLE_MASK))
 				continue;
-			inspect_spte_has_rmap(vcpu->kvm, sp, &pt[i]);
+			inspect_spte_has_rmap(vcpu->kvm, &pt[i]);
 		}
 	}
 	return;
-- 
1.6.1.2

[PATCH 2/2] KVM MMU: record reverse mapping for spte only if it's writable

[Xiao Guangrong]

The read only spte mapping can't hurt shadow page cache,
so, no need to record it.

Using bit9 to record whether the spte is re-mapped

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 arch/x86/kvm/mmu.c |   17 +++++++++++++++--
 arch/x86/kvm/mmu.h |    1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5de92ae..999f572 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -259,7 +259,17 @@ static int is_dirty_gpte(unsigned long pte)
 
 static int is_rmap_spte(u64 pte)
 {
-	return is_shadow_present_pte(pte);
+	return pte & PT_RMAP_MASK;
+}
+
+static void spte_set_rmap(u64 *spte)
+{
+	*spte |= PT_RMAP_MASK;
+}
+
+static void spte_clear_rmap(u64 *spte)
+{
+	*spte &= ~PT_RMAP_MASK;
 }
 
 static int is_last_spte(u64 pte, int level)
@@ -543,7 +553,7 @@ static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 	unsigned long *rmapp;
 	int i, count = 0;
 
-	if (!is_rmap_spte(*spte))
+	if (!is_shadow_present_pte(*spte) || !is_writable_pte(*spte))
 		return count;
 	gfn = unalias_gfn(vcpu->kvm, gfn);
 	sp = page_header(__pa(spte));
@@ -573,6 +583,7 @@ static int rmap_add(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 			;
 		desc->sptes[i] = spte;
 	}
+	spte_set_rmap(spte);
 	return count;
 }
 
@@ -610,6 +621,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 
 	if (!is_rmap_spte(*spte))
 		return;
+	spte_clear_rmap(spte);
 	sp = page_header(__pa(spte));
 	pfn = spte_to_pfn(*spte);
 	if (*spte & shadow_accessed_mask)
@@ -646,6 +658,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 		pr_err("rmap_remove: %p %llx many->many\n", spte, *spte);
 		BUG();
 	}
+
 }
 
 static u64 *rmap_next(struct kvm *kvm, unsigned long *rmapp, u64 *spte)
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index be66759..166b9b5 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -22,6 +22,7 @@
 #define PT_PAGE_SIZE_MASK (1ULL << 7)
 #define PT_PAT_MASK (1ULL << 7)
 #define PT_GLOBAL_MASK (1ULL << 8)
+#define PT_RMAP_MASK (1ULL << 9)
 #define PT64_NX_SHIFT 63
 #define PT64_NX_MASK (1ULL << PT64_NX_SHIFT)
 
-- 
1.6.1.2

Re: [PATCH 2/2] KVM MMU: record reverse mapping for spte only if it's writable

[Avi Kivity]

On 04/01/2010 11:52 AM, Xiao Guangrong wrote:
> The read only spte mapping can't hurt shadow page cache,
> so, no need to record it.
>
>    

We do need to keep track of read-only mappings, that's how swapping 
works.  See commit ca335c8f08d.

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

Re: [PATCH 1/2] KVM MMU: cleanup/fix mmu audit code

[Marcelo Tosatti]

On Thu, Apr 01, 2010 at 04:50:45PM +0800, Xiao Guangrong wrote:
> This patch does:
> - 'sp' parameter in inspect_spte_fn() is not used, so remove it
> - fix 'kvm' and 'slots' is not defined in count_rmaps()
> - fix a bug in inspect_spte_has_rmap()
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>

Applied, thanks.