[PATCH 0/3] kvm ksm support v3

[PATCH 0/3] kvm ksm support v3

[Izik Eidus]

Change from v2 : remove unused if.

Thanks.

Izik Eidus (3):
  kvm: dont hold pagecount reference for mapped sptes pages
  add SPTE_HOST_WRITEABLE flag to the shadow ptes
  add support for change_pte mmu notifiers

 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   84 ++++++++++++++++++++++++++++++--------
 arch/x86/kvm/paging_tmpl.h      |   18 +++++++-
 virt/kvm/kvm_main.c             |   14 ++++++
 4 files changed, 96 insertions(+), 21 deletions(-)

[PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages

[Izik Eidus]

When using mmu notifiers, we are allowed to remove the page count
reference tooken by get_user_pages to a specific page that is mapped
inside the shadow page tables.

This is needed so we can balance the pagecount against mapcount
checking.

(Right now kvm increase the pagecount and does not increase the
mapcount when mapping page into shadow page table entry,
so when comparing pagecount against mapcount, you have no
reliable result.)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index eca41ae..6c67b23 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -634,9 +634,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	if (*spte & shadow_accessed_mask)
 		kvm_set_pfn_accessed(pfn);
 	if (is_writeble_pte(*spte))
-		kvm_release_pfn_dirty(pfn);
-	else
-		kvm_release_pfn_clean(pfn);
+		kvm_set_pfn_dirty(pfn);
 	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], sp->role.level);
 	if (!*rmapp) {
 		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
@@ -1877,8 +1875,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	page_header_update_slot(vcpu->kvm, sptep, gfn);
 	if (!was_rmapped) {
 		rmap_count = rmap_add(vcpu, sptep, gfn);
-		if (!is_rmap_spte(*sptep))
-			kvm_release_pfn_clean(pfn);
+		kvm_release_pfn_clean(pfn);
 		if (rmap_count > RMAP_RECYCLE_THRESHOLD)
 			rmap_recycle(vcpu, sptep, gfn);
 	} else {
-- 
1.5.6.5

[PATCH 2/3] add SPTE_HOST_WRITEABLE flag to the shadow ptes

[Izik Eidus]

this flag notify that the host physical page we are pointing to from
the spte is write protected, and therefore we cant change its access
to be write unless we run get_user_pages(write = 1).

(this is needed for change_pte support in kvm)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c         |   15 +++++++++++----
 arch/x86/kvm/paging_tmpl.h |   18 +++++++++++++++---
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 6c67b23..5cd8b4e 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -156,6 +156,8 @@ module_param(oos_shadow, bool, 0644);
 #define CREATE_TRACE_POINTS
 #include "mmutrace.h"
 
+#define SPTE_HOST_WRITEABLE (1ULL << PT_FIRST_AVAIL_BITS_SHIFT)
+
 #define SHADOW_PT_INDEX(addr, level) PT64_INDEX(addr, level)
 
 struct kvm_rmap_desc {
@@ -1754,7 +1756,7 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 		    unsigned pte_access, int user_fault,
 		    int write_fault, int dirty, int level,
 		    gfn_t gfn, pfn_t pfn, bool speculative,
-		    bool can_unsync)
+		    bool can_unsync, bool reset_host_protection)
 {
 	u64 spte;
 	int ret = 0;
@@ -1781,6 +1783,9 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 		spte |= kvm_x86_ops->get_mt_mask(vcpu, gfn,
 			kvm_is_mmio_pfn(pfn));
 
+	if (reset_host_protection)
+		spte |= SPTE_HOST_WRITEABLE;
+
 	spte |= (u64)pfn << PAGE_SHIFT;
 
 	if ((pte_access & ACC_WRITE_MASK)
@@ -1826,7 +1831,8 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 			 unsigned pt_access, unsigned pte_access,
 			 int user_fault, int write_fault, int dirty,
 			 int *ptwrite, int level, gfn_t gfn,
-			 pfn_t pfn, bool speculative)
+			 pfn_t pfn, bool speculative,
+			 bool reset_host_protection)
 {
 	int was_rmapped = 0;
 	int was_writeble = is_writeble_pte(*sptep);
@@ -1858,7 +1864,8 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	}
 
 	if (set_spte(vcpu, sptep, pte_access, user_fault, write_fault,
-		      dirty, level, gfn, pfn, speculative, true)) {
+		      dirty, level, gfn, pfn, speculative, true,
+		      reset_host_protection)) {
 		if (write_fault)
 			*ptwrite = 1;
 		kvm_x86_ops->tlb_flush(vcpu);
@@ -1906,7 +1913,7 @@ static int __direct_map(struct kvm_vcpu *vcpu, gpa_t v, int write,
 		if (iterator.level == level) {
 			mmu_set_spte(vcpu, iterator.sptep, ACC_ALL, ACC_ALL,
 				     0, write, 1, &pt_write,
-				     level, gfn, pfn, false);
+				     level, gfn, pfn, false, true);
 			++vcpu->stat.pf_fixed;
 			break;
 		}
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index d2fec9c..cfd2424 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -273,9 +273,13 @@ static void FNAME(update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *page,
 	if (mmu_notifier_retry(vcpu, vcpu->arch.update_pte.mmu_seq))
 		return;
 	kvm_get_pfn(pfn);
+ 	/*
+ 	 * we call mmu_set_spte() with reset_host_protection = true beacuse that
+ 	 * vcpu->arch.update_pte.pfn was fetched from get_user_pages(write = 1).
+ 	 */
 	mmu_set_spte(vcpu, spte, page->role.access, pte_access, 0, 0,
 		     gpte & PT_DIRTY_MASK, NULL, PT_PAGE_TABLE_LEVEL,
-		     gpte_to_gfn(gpte), pfn, true);
+		     gpte_to_gfn(gpte), pfn, true, true);
 }
 
 /*
@@ -308,7 +312,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
 				     user_fault, write_fault,
 				     gw->ptes[gw->level-1] & PT_DIRTY_MASK,
 				     ptwrite, level,
-				     gw->gfn, pfn, false);
+				     gw->gfn, pfn, false, true);
 			break;
 		}
 
@@ -558,6 +562,7 @@ static void FNAME(prefetch_page)(struct kvm_vcpu *vcpu,
 static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 {
 	int i, offset, nr_present;
+	bool reset_host_protection;
 
 	offset = nr_present = 0;
 
@@ -595,9 +600,16 @@ static int FNAME(sync_page)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp)
 
 		nr_present++;
 		pte_access = sp->role.access & FNAME(gpte_access)(vcpu, gpte);
+		if (!(sp->spt[i] & SPTE_HOST_WRITEABLE)) {
+ 			pte_access &= ~ACC_WRITE_MASK;
+			reset_host_protection = 0;
+		} else {
+			reset_host_protection = 1;
+		}
 		set_spte(vcpu, &sp->spt[i], pte_access, 0, 0,
 			 is_dirty_gpte(gpte), PT_PAGE_TABLE_LEVEL, gfn,
-			 spte_to_pfn(sp->spt[i]), true, false);
+			 spte_to_pfn(sp->spt[i]), true, false,
+			 reset_host_protection);
 	}
 
 	return !nr_present;
-- 
1.5.6.5

[PATCH 3/3] add support for change_pte mmu notifiers

[Izik Eidus]

this is needed for kvm if it want ksm to directly map pages into its
shadow page tables.

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   62 +++++++++++++++++++++++++++++++++-----
 virt/kvm/kvm_main.c             |   14 +++++++++
 3 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3be0004..d838922 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -796,6 +796,7 @@ asmlinkage void kvm_handle_fault_on_reboot(void);
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
 int kvm_age_hva(struct kvm *kvm, unsigned long hva);
+void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 int cpuid_maxphyaddr(struct kvm_vcpu *vcpu);
 int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu);
 int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 5cd8b4e..ceec065 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -748,7 +748,7 @@ static int rmap_write_protect(struct kvm *kvm, u64 gfn)
 	return write_protected;
 }
 
-static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
+static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp, u64 data)
 {
 	u64 *spte;
 	int need_tlb_flush = 0;
@@ -763,8 +763,45 @@ static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
 	return need_tlb_flush;
 }
 
-static int kvm_handle_hva(struct kvm *kvm, unsigned long hva,
-			  int (*handler)(struct kvm *kvm, unsigned long *rmapp))
+static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp, u64 data)
+{
+	int need_flush = 0;
+	u64 *spte, new_spte;
+	pte_t *ptep = (pte_t *)data;
+	pfn_t new_pfn;
+
+	WARN_ON(pte_huge(*ptep));
+	new_pfn = pte_pfn(*ptep);
+	spte = rmap_next(kvm, rmapp, NULL);
+	while (spte) {
+		BUG_ON(!is_shadow_present_pte(*spte));
+		rmap_printk("kvm_set_pte_rmapp: spte %p %llx\n", spte, *spte);
+		need_flush = 1;
+		if (pte_write(*ptep)) {
+			rmap_remove(kvm, spte);
+			__set_spte(spte, shadow_trap_nonpresent_pte);
+			spte = rmap_next(kvm, rmapp, NULL);
+		} else {
+			new_spte = *spte &~ (PT64_BASE_ADDR_MASK);
+			new_spte |= new_pfn << PAGE_SHIFT;
+
+			new_spte &= ~PT_WRITABLE_MASK;
+			new_spte &= ~SPTE_HOST_WRITEABLE;
+			if (is_writeble_pte(*spte))
+				kvm_set_pfn_dirty(spte_to_pfn(*spte));
+			__set_spte(spte, new_spte);
+			spte = rmap_next(kvm, rmapp, spte);
+		}
+	}
+	if (need_flush)
+		kvm_flush_remote_tlbs(kvm);
+
+	return 0;
+}
+
+static int kvm_handle_hva(struct kvm *kvm, unsigned long hva, u64 data,
+			  int (*handler)(struct kvm *kvm, unsigned long *rmapp,
+					 u64 data))
 {
 	int i, j;
 	int retval = 0;
@@ -786,13 +823,15 @@ static int kvm_handle_hva(struct kvm *kvm, unsigned long hva,
 		if (hva >= start && hva < end) {
 			gfn_t gfn_offset = (hva - start) >> PAGE_SHIFT;
 
-			retval |= handler(kvm, &memslot->rmap[gfn_offset]);
+			retval |= handler(kvm, &memslot->rmap[gfn_offset],
+					  data);
 
 			for (j = 0; j < KVM_NR_PAGE_SIZES - 1; ++j) {
 				int idx = gfn_offset;
 				idx /= KVM_PAGES_PER_HPAGE(PT_DIRECTORY_LEVEL + j);
 				retval |= handler(kvm,
-					&memslot->lpage_info[j][idx].rmap_pde);
+					&memslot->lpage_info[j][idx].rmap_pde,
+					data);
 			}
 		}
 	}
@@ -802,10 +841,15 @@ static int kvm_handle_hva(struct kvm *kvm, unsigned long hva,
 
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
 {
-	return kvm_handle_hva(kvm, hva, kvm_unmap_rmapp);
+	return kvm_handle_hva(kvm, hva, 0, kvm_unmap_rmapp);
+}
+
+void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte)
+{
+	kvm_handle_hva(kvm, hva, (u64)&pte, kvm_set_pte_rmapp);
 }
 
-static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp)
+static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp, u64 data)
 {
 	u64 *spte;
 	int young = 0;
@@ -841,13 +885,13 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
 	gfn = unalias_gfn(vcpu->kvm, gfn);
 	rmapp = gfn_to_rmap(vcpu->kvm, gfn, sp->role.level);
 
-	kvm_unmap_rmapp(vcpu->kvm, rmapp);
+	kvm_unmap_rmapp(vcpu->kvm, rmapp, 0);
 	kvm_flush_remote_tlbs(vcpu->kvm);
 }
 
 int kvm_age_hva(struct kvm *kvm, unsigned long hva)
 {
-	return kvm_handle_hva(kvm, hva, kvm_age_rmapp);
+	return kvm_handle_hva(kvm, hva, 0, kvm_age_rmapp);
 }
 
 #ifdef MMU_DEBUG
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 897bff3..4cbd396 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -851,6 +851,19 @@ static void kvm_mmu_notifier_invalidate_page(struct mmu_notifier *mn,
 
 }
 
+static void kvm_mmu_notifier_change_pte(struct mmu_notifier *mn,
+					struct mm_struct *mm,
+					unsigned long address,
+					pte_t pte)
+{
+	struct kvm *kvm = mmu_notifier_to_kvm(mn);
+
+	spin_lock(&kvm->mmu_lock);
+	kvm->mmu_notifier_seq++;
+	kvm_set_spte_hva(kvm, address, pte);
+	spin_unlock(&kvm->mmu_lock);
+}
+
 static void kvm_mmu_notifier_invalidate_range_start(struct mmu_notifier *mn,
 						    struct mm_struct *mm,
 						    unsigned long start,
@@ -930,6 +943,7 @@ static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
 	.invalidate_range_start	= kvm_mmu_notifier_invalidate_range_start,
 	.invalidate_range_end	= kvm_mmu_notifier_invalidate_range_end,
 	.clear_flush_young	= kvm_mmu_notifier_clear_flush_young,
+	.change_pte		= kvm_mmu_notifier_change_pte,
 	.release		= kvm_mmu_notifier_release,
 };
 #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
-- 
1.5.6.5

Re: [PATCH 3/3] add support for change_pte mmu notifiers

[Marcelo Tosatti]

On Wed, Sep 23, 2009 at 09:47:18PM +0300, Izik Eidus wrote:
> this is needed for kvm if it want ksm to directly map pages into its
> shadow page tables.
> 
> Signed-off-by: Izik Eidus <ieidus@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |    1 +
>  arch/x86/kvm/mmu.c              |   62 +++++++++++++++++++++++++++++++++-----
>  virt/kvm/kvm_main.c             |   14 +++++++++
>  3 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 3be0004..d838922 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -796,6 +796,7 @@ asmlinkage void kvm_handle_fault_on_reboot(void);
>  #define KVM_ARCH_WANT_MMU_NOTIFIER
>  int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
>  int kvm_age_hva(struct kvm *kvm, unsigned long hva);
> +void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
>  int cpuid_maxphyaddr(struct kvm_vcpu *vcpu);
>  int kvm_cpu_has_interrupt(struct kvm_vcpu *vcpu);
>  int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 5cd8b4e..ceec065 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -748,7 +748,7 @@ static int rmap_write_protect(struct kvm *kvm, u64 gfn)
>  	return write_protected;
>  }
>  
> -static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
> +static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp, u64 data)
>  {
>  	u64 *spte;
>  	int need_tlb_flush = 0;
> @@ -763,8 +763,45 @@ static int kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
>  	return need_tlb_flush;
>  }
>  
> -static int kvm_handle_hva(struct kvm *kvm, unsigned long hva,
> -			  int (*handler)(struct kvm *kvm, unsigned long *rmapp))
> +static int kvm_set_pte_rmapp(struct kvm *kvm, unsigned long *rmapp, u64 data)
> +{
> +	int need_flush = 0;
> +	u64 *spte, new_spte;
> +	pte_t *ptep = (pte_t *)data;
> +	pfn_t new_pfn;
> +
> +	WARN_ON(pte_huge(*ptep));
> +	new_pfn = pte_pfn(*ptep);
> +	spte = rmap_next(kvm, rmapp, NULL);
> +	while (spte) {
> +		BUG_ON(!is_shadow_present_pte(*spte));
> +		rmap_printk("kvm_set_pte_rmapp: spte %p %llx\n", spte, *spte);
> +		need_flush = 1;
> +		if (pte_write(*ptep)) {
> +			rmap_remove(kvm, spte);
> +			__set_spte(spte, shadow_trap_nonpresent_pte);
> +			spte = rmap_next(kvm, rmapp, NULL);
> +		} else {
> +			new_spte = *spte &~ (PT64_BASE_ADDR_MASK);
> +			new_spte |= new_pfn << PAGE_SHIFT;

		        new_spte |= (u64)new_pfn << PAGE_SHIFT;

Otherwise looks good to me.

> +			new_spte &= ~PT_WRITABLE_MASK;
> +			new_spte &= ~SPTE_HOST_WRITEABLE;
> +			if (is_writeble_pte(*spte))
> +				kvm_set_pfn_dirty(spte_to_pfn(*spte));
> +			__set_spte(spte, new_spte);
> +			spte = rmap_next(kvm, rmapp, spte);
> +		}
> +	}
> +	if (need_flush)
> +		kvm_flush_remote_tlbs(kvm);
> +
> +	return 0;
> +}

Re: [PATCH 3/3] add support for change_pte mmu notifiers

[Andrea Arcangeli]

On Wed, Sep 23, 2009 at 09:47:18PM +0300, Izik Eidus wrote:
> +	if (need_flush)
> +		kvm_flush_remote_tlbs(kvm);

need_flush can be return to kvm_mmu_notifier_change_pte to defer the
tlb flush after dropping the spin lock I think. We are forced to flush
the tlb inside spin_lock in kvm normal context because that stops the
VM from freeing the page (it hangs on the mmu_lock taken by kvm
invalidate_page/change_pte) so we can unmap tons of sptes and do a
single kvm tlb flush that covers them all (by keeping both actions
under the mmu_lock), but in mmu notifier context the pages can't be
freed from under the guest, so we can flush the tlb flushing the tlb
before making the page freeable, because both old and new page in
do_wp_page are still pinned and can't be freed and reused from under
us even if we release mmu_lock before tlb flush.

Re: [PATCH 2/3] add SPTE_HOST_WRITEABLE flag to the shadow ptes

[Andrea Arcangeli]

On Wed, Sep 23, 2009 at 09:47:17PM +0300, Izik Eidus wrote:
> this flag notify that the host physical page we are pointing to from
> the spte is write protected, and therefore we cant change its access
> to be write unless we run get_user_pages(write = 1).
> 
> (this is needed for change_pte support in kvm)
> 
> Signed-off-by: Izik Eidus <ieidus@redhat.com>

Acked-by: Andrea Arcangeli <aarcange@redhat.com>

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages

[Marcelo Tosatti]

This needs compat code for !MMU_NOTIFIERS case in kvm-kmod (Jan CC'ed).

Otherwise looks good.

On Wed, Sep 23, 2009 at 09:47:16PM +0300, Izik Eidus wrote:
> When using mmu notifiers, we are allowed to remove the page count
> reference tooken by get_user_pages to a specific page that is mapped
> inside the shadow page tables.
> 
> This is needed so we can balance the pagecount against mapcount
> checking.
> 
> (Right now kvm increase the pagecount and does not increase the
> mapcount when mapping page into shadow page table entry,
> so when comparing pagecount against mapcount, you have no
> reliable result.)
> 
> Signed-off-by: Izik Eidus <ieidus@redhat.com>
> ---
>  arch/x86/kvm/mmu.c |    7 ++-----
>  1 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index eca41ae..6c67b23 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -634,9 +634,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
>  	if (*spte & shadow_accessed_mask)
>  		kvm_set_pfn_accessed(pfn);
>  	if (is_writeble_pte(*spte))
> -		kvm_release_pfn_dirty(pfn);
> -	else
> -		kvm_release_pfn_clean(pfn);
> +		kvm_set_pfn_dirty(pfn);
>  	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], sp->role.level);
>  	if (!*rmapp) {
>  		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
> @@ -1877,8 +1875,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
>  	page_header_update_slot(vcpu->kvm, sptep, gfn);
>  	if (!was_rmapped) {
>  		rmap_count = rmap_add(vcpu, sptep, gfn);
> -		if (!is_rmap_spte(*sptep))
> -			kvm_release_pfn_clean(pfn);
> +		kvm_release_pfn_clean(pfn);
>  		if (rmap_count > RMAP_RECYCLE_THRESHOLD)
>  			rmap_recycle(vcpu, sptep, gfn);
>  	} else {
> -- 
> 1.5.6.5

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages

[Andrea Arcangeli]

On Wed, Sep 23, 2009 at 09:47:16PM +0300, Izik Eidus wrote:
> When using mmu notifiers, we are allowed to remove the page count
> reference tooken by get_user_pages to a specific page that is mapped
> inside the shadow page tables.
> 
> This is needed so we can balance the pagecount against mapcount
> checking.
> 
> (Right now kvm increase the pagecount and does not increase the
> mapcount when mapping page into shadow page table entry,
> so when comparing pagecount against mapcount, you have no
> reliable result.)
> 
> Signed-off-by: Izik Eidus <ieidus@redhat.com>

Acked-by: Andrea Arcangeli <aarcange@redhat.com>

Re: [PATCH 0/3] kvm ksm support v3

[Marcelo Tosatti]

On Wed, Sep 23, 2009 at 09:47:15PM +0300, Izik Eidus wrote:
> Change from v2 : remove unused if.
> 
> Thanks.
> 
> Izik Eidus (3):
>   kvm: dont hold pagecount reference for mapped sptes pages
>   add SPTE_HOST_WRITEABLE flag to the shadow ptes
>   add support for change_pte mmu notifiers
> 
>  arch/x86/include/asm/kvm_host.h |    1 +
>  arch/x86/kvm/mmu.c              |   84 ++++++++++++++++++++++++++++++--------
>  arch/x86/kvm/paging_tmpl.h      |   18 +++++++-
>  virt/kvm/kvm_main.c             |   14 ++++++
>  4 files changed, 96 insertions(+), 21 deletions(-)

Applied, thanks.

[PATCH 0/3] ksm support for kvm v2

[Izik Eidus]

Hope i fixed everything i was asked...
please tell me if I forgot anything.

Izik Eidus (3):
  kvm: dont hold pagecount reference for mapped sptes pages
  add SPTE_HOST_WRITEABLE flag to the shadow ptes
  add support for change_pte mmu notifiers

 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   86 ++++++++++++++++++++++++++++++--------
 arch/x86/kvm/paging_tmpl.h      |   18 +++++++-
 virt/kvm/kvm_main.c             |   14 ++++++
 4 files changed, 98 insertions(+), 21 deletions(-)

[PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages

[Izik Eidus]

When using mmu notifiers, we are allowed to remove the page count
reference tooken by get_user_pages to a specific page that is mapped
inside the shadow page tables.

This is needed so we can balance the pagecount against mapcount
checking.

(Right now kvm increase the pagecount and does not increase the
mapcount when mapping page into shadow page table entry,
so when comparing pagecount against mapcount, you have no
reliable result.)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index eca41ae..6c67b23 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -634,9 +634,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	if (*spte & shadow_accessed_mask)
 		kvm_set_pfn_accessed(pfn);
 	if (is_writeble_pte(*spte))
-		kvm_release_pfn_dirty(pfn);
-	else
-		kvm_release_pfn_clean(pfn);
+		kvm_set_pfn_dirty(pfn);
 	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], sp->role.level);
 	if (!*rmapp) {
 		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
@@ -1877,8 +1875,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	page_header_update_slot(vcpu->kvm, sptep, gfn);
 	if (!was_rmapped) {
 		rmap_count = rmap_add(vcpu, sptep, gfn);
-		if (!is_rmap_spte(*sptep))
-			kvm_release_pfn_clean(pfn);
+		kvm_release_pfn_clean(pfn);
 		if (rmap_count > RMAP_RECYCLE_THRESHOLD)
 			rmap_recycle(vcpu, sptep, gfn);
 	} else {
-- 
1.5.6.5

[PATCH 0/3] ksm support for kvm

[Izik Eidus]

Hi,

The following seires add ksm support to the kvm mmu.

Thanks.

[PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages

[Izik Eidus]

When using mmu notifiers, we are allowed to remove the page count
reference tooken by get_user_pages to a specific page that is mapped
inside the shadow page tables.

This is needed so we can balance the pagecount against mapcount
checking.

(Right now kvm increase the pagecount and does not increase the
mapcount when mapping page into shadow page table entry,
so when comparing pagecount against mapcount, you have no
reliable result.)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index f76d086..62d2f86 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -634,9 +634,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	if (*spte & shadow_accessed_mask)
 		kvm_set_pfn_accessed(pfn);
 	if (is_writeble_pte(*spte))
-		kvm_release_pfn_dirty(pfn);
-	else
-		kvm_release_pfn_clean(pfn);
+		kvm_set_pfn_dirty(pfn);
 	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], sp->role.level);
 	if (!*rmapp) {
 		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
@@ -1877,8 +1875,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
 	page_header_update_slot(vcpu->kvm, sptep, gfn);
 	if (!was_rmapped) {
 		rmap_count = rmap_add(vcpu, sptep, gfn);
-		if (!is_rmap_spte(*sptep))
-			kvm_release_pfn_clean(pfn);
+		kvm_release_pfn_clean(pfn);
 		if (rmap_count > RMAP_RECYCLE_THRESHOLD)
 			rmap_recycle(vcpu, sptep, gfn);
 	} else {
-- 
1.5.6.5

[PATCH 0/3] kvm support for ksm

[Izik Eidus]

apply it against Avi git tree.

Izik Eidus (3):
  kvm: dont hold pagecount reference for mapped sptes pages.
  kvm: add SPTE_HOST_WRITEABLE flag to the shadow ptes.
  kvm: add support for change_pte mmu notifiers

 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/mmu.c              |   89 ++++++++++++++++++++++++++++++++-------
 arch/x86/kvm/paging_tmpl.h      |   16 ++++++-
 virt/kvm/kvm_main.c             |   14 ++++++
 4 files changed, 101 insertions(+), 19 deletions(-)

[PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Izik Eidus]

When using mmu notifiers, we are allowed to remove the page count
reference tooken by get_user_pages to a specific page that is mapped
inside the shadow page tables.

This is needed so we can balance the pagecount against mapcount
checking.

(Right now kvm increase the pagecount and does not increase the
mapcount when mapping page into shadow page table entry,
so when comparing pagecount against mapcount, you have no
reliable result.)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index b625ed4..df8fbaf 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -567,9 +567,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	if (*spte & shadow_accessed_mask)
 		kvm_set_pfn_accessed(pfn);
 	if (is_writeble_pte(*spte))
-		kvm_release_pfn_dirty(pfn);
-	else
-		kvm_release_pfn_clean(pfn);
+		kvm_set_pfn_dirty(pfn);
 	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], is_large_pte(*spte));
 	if (!*rmapp) {
 		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
@@ -1812,8 +1810,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *shadow_pte,
 	page_header_update_slot(vcpu->kvm, shadow_pte, gfn);
 	if (!was_rmapped) {
 		rmap_add(vcpu, shadow_pte, gfn, largepage);
-		if (!is_rmap_pte(*shadow_pte))
-			kvm_release_pfn_clean(pfn);
+		kvm_release_pfn_clean(pfn);
 	} else {
 		if (was_writeble)
 			kvm_release_pfn_dirty(pfn);
-- 
1.5.6.5

[PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Izik Eidus]

When using mmu notifiers, we are allowed to remove the page count
reference tooken by get_user_pages to a specific page that is mapped
inside the shadow page tables.

This is needed so we can balance the pagecount against mapcount
checking.

(Right now kvm increase the pagecount and does not increase the
mapcount when mapping page into shadow page table entry,
so when comparing pagecount against mapcount, you have no
reliable result.)

Signed-off-by: Izik Eidus <ieidus@redhat.com>
---
 arch/x86/kvm/mmu.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index b625ed4..df8fbaf 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -567,9 +567,7 @@ static void rmap_remove(struct kvm *kvm, u64 *spte)
 	if (*spte & shadow_accessed_mask)
 		kvm_set_pfn_accessed(pfn);
 	if (is_writeble_pte(*spte))
-		kvm_release_pfn_dirty(pfn);
-	else
-		kvm_release_pfn_clean(pfn);
+		kvm_set_pfn_dirty(pfn);
 	rmapp = gfn_to_rmap(kvm, sp->gfns[spte - sp->spt], is_large_pte(*spte));
 	if (!*rmapp) {
 		printk(KERN_ERR "rmap_remove: %p %llx 0->BUG\n", spte, *spte);
@@ -1812,8 +1810,7 @@ static void mmu_set_spte(struct kvm_vcpu *vcpu, u64 *shadow_pte,
 	page_header_update_slot(vcpu->kvm, shadow_pte, gfn);
 	if (!was_rmapped) {
 		rmap_add(vcpu, shadow_pte, gfn, largepage);
-		if (!is_rmap_pte(*shadow_pte))
-			kvm_release_pfn_clean(pfn);
+		kvm_release_pfn_clean(pfn);
 	} else {
 		if (was_writeble)
 			kvm_release_pfn_dirty(pfn);
-- 
1.5.6.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Marcelo Tosatti]

On Tue, Mar 31, 2009 at 03:00:02AM +0300, Izik Eidus wrote:
> When using mmu notifiers, we are allowed to remove the page count
> reference tooken by get_user_pages to a specific page that is mapped
> inside the shadow page tables.
> 
> This is needed so we can balance the pagecount against mapcount
> checking.
> 
> (Right now kvm increase the pagecount and does not increase the
> mapcount when mapping page into shadow page table entry,
> so when comparing pagecount against mapcount, you have no
> reliable result.)

IMO ifdef'ing CONFIG_MMU_NOTIFIERS here (and keeping the ref if unset)
instead of in the backward compat code gives less room for headaches.

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Izik Eidus]

Marcelo Tosatti wrote:
> On Tue, Mar 31, 2009 at 03:00:02AM +0300, Izik Eidus wrote:
>   
>> When using mmu notifiers, we are allowed to remove the page count
>> reference tooken by get_user_pages to a specific page that is mapped
>> inside the shadow page tables.
>>
>> This is needed so we can balance the pagecount against mapcount
>> checking.
>>
>> (Right now kvm increase the pagecount and does not increase the
>> mapcount when mapping page into shadow page table entry,
>> so when comparing pagecount against mapcount, you have no
>> reliable result.)
>>     
>
> IMO ifdef'ing CONFIG_MMU_NOTIFIERS here (and keeping the ref if unset)
> instead of in the backward compat code gives less room for headaches.
>
>   
That was the first version of this patch, Avi preferred not to do it...

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Izik Eidus]

Izik Eidus wrote:
> Marcelo Tosatti wrote:
>> On Tue, Mar 31, 2009 at 03:00:02AM +0300, Izik Eidus wrote:
>>  
>>> When using mmu notifiers, we are allowed to remove the page count
>>> reference tooken by get_user_pages to a specific page that is mapped
>>> inside the shadow page tables.
>>>
>>> This is needed so we can balance the pagecount against mapcount
>>> checking.
>>>
>>> (Right now kvm increase the pagecount and does not increase the
>>> mapcount when mapping page into shadow page table entry,
>>> so when comparing pagecount against mapcount, you have no
>>> reliable result.)
>>>     
>>
>> IMO ifdef'ing CONFIG_MMU_NOTIFIERS here (and keeping the ref if unset)
>> instead of in the backward compat code gives less room for headaches.
>>
>>   
> That was the first version of this patch, Avi preferred not to do it...
>
Avi, You mind if i changed it to use the IFDEF ?

Re: [PATCH 1/3] kvm: dont hold pagecount reference for mapped sptes pages.

[Avi Kivity]

Izik Eidus wrote:
> Izik Eidus wrote:
>> Marcelo Tosatti wrote:
>>> On Tue, Mar 31, 2009 at 03:00:02AM +0300, Izik Eidus wrote:
>>>  
>>>> When using mmu notifiers, we are allowed to remove the page count
>>>> reference tooken by get_user_pages to a specific page that is mapped
>>>> inside the shadow page tables.
>>>>
>>>> This is needed so we can balance the pagecount against mapcount
>>>> checking.
>>>>
>>>> (Right now kvm increase the pagecount and does not increase the
>>>> mapcount when mapping page into shadow page table entry,
>>>> so when comparing pagecount against mapcount, you have no
>>>> reliable result.)
>>>>     
>>>
>>> IMO ifdef'ing CONFIG_MMU_NOTIFIERS here (and keeping the ref if unset)
>>> instead of in the backward compat code gives less room for headaches.
>>>
>>>   
>> That was the first version of this patch, Avi preferred not to do it...
>>
> Avi, You mind if i changed it to use the IFDEF ?

No, let's not have too many ifdefs in the code.  I'll hack it in 
hack-module.awk.

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