Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1341959 > unrolled thread

[PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few micro-optimizations

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few micro-optimizations Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:20 +0100
    [PATCH 10/12] KVM: mark memory barrier with smp_mb__after_atomic Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:20 +0100
    [PATCH 05/12] KVM: MMU: use kvm_sync_page in kvm_sync_pages Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:30 +0100
    [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:30 +0100
      Re: [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-25 08:40 +0100
        Re: [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page Paolo Bonzini <pbonzini@redhat.com> - 2016-02-25 09:50 +0100
    [PATCH 02/12] KVM: MMU: check kvm_mmu_pages and mmu_page_path indices Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:30 +0100
    [PATCH 03/12] KVM: MMU: introduce kvm_mmu_flush_or_zap Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:30 +0100
    [PATCH 04/12] KVM: MMU: move TLB flush out of __kvm_sync_page Paolo Bonzini <pbonzini@redhat.com> - 2016-02-24 14:30 +0100
    Re: [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few  micro-optimizations Xiao Guangrong <guangrong.xiao@linux.intel.com> - 2016-02-25 09:30 +0100
      Re: [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few  micro-optimizations Paolo Bonzini <pbonzini@redhat.com> - 2016-02-25 09:50 +0100

#1341959 — [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few micro-optimizations

This series started from looking at mmu_unsync_walk for the ubsan thread.
Patches 1 and 2 are the result of the discussions in that thread.

Patches 3 to 9 do more cleanups in __kvm_sync_page and its callers.
Among other changes, it removes kvm_sync_page_transient and avoids
duplicate code between __kvm_sync_page and kvm_sync_pages.

I stopped where I had questions about the existing kvm_mmu_get_page
code (see patch 8 for the question).  However perhaps more cleanups
are possible, also thanks to Takuya's work on that function and
link_shadow_page.

Patches 10 to 12 are just micro-optimizations.

Guangrong, it would be great if you took a look since you know this part
of KVM very well.

I have tested this series minus patch 9, and it survived installation
of various Linux and Windows guests with EPT disabled.  Of course before
committing the patches I will retest with patch 9 included.

Paolo

Paolo Bonzini (11):
  KVM: MMU: Fix ubsan warnings
  KVM: MMU: introduce kvm_mmu_flush_or_zap
  KVM: MMU: move TLB flush out of __kvm_sync_page
  KVM: MMU: use kvm_sync_page in kvm_sync_pages
  KVM: MMU: cleanup __kvm_sync_page and its callers
  KVM: MMU: invert return value of FNAME(sync_page) and *kvm_sync_page*
  KVM: MMU: move zap/flush to kvm_mmu_get_page
  KVM: MMU: coalesce zapping page after mmu_sync_children
  KVM: mark memory barrier with smp_mb__after_atomic
  KVM: MMU: simplify last_pte_bitmap
  KVM: MMU: micro-optimize gpte_access

Xiao Guangrong (1):
  KVM: MMU: check kvm_mmu_pages and mmu_page_path indices

 arch/x86/include/asm/kvm_host.h |   6 +-
 arch/x86/kvm/mmu.c              | 216 ++++++++++++++++++++++------------------
 arch/x86/kvm/paging_tmpl.h      |  11 +-
 virt/kvm/kvm_main.c             |   2 +-
 4 files changed, 126 insertions(+), 109 deletions(-)

-- 
1.8.3.1

[toc] | [next] | [standalone]

#1341961 — [PATCH 10/12] KVM: mark memory barrier with smp_mb__after_atomic

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/kvm_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 39c36d4f4f5c..77a73739b08e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -171,7 +171,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 		cpu = vcpu->cpu;
 
 		/* Set ->requests bit before we read ->mode */
-		smp_mb();
+		smp_mb__after_atomic();
 
 		if (cpus != NULL && cpu != -1 && cpu != me &&
 		      kvm_vcpu_exiting_guest_mode(vcpu) != OUTSIDE_GUEST_MODE)
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1341963 — [PATCH 05/12] KVM: MMU: use kvm_sync_page in kvm_sync_pages

If the last argument is true, kvm_unlink_unsync_page is called anyway in
__kvm_sync_page (either by kvm_mmu_prepare_zap_page or by __kvm_sync_page
itself).  Therefore, kvm_sync_pages can just call kvm_sync_page, instead
of going through kvm_unlink_unsync_page+__kvm_sync_page.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index c440a28822b7..240c0dbb140f 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1957,8 +1957,7 @@ static void kvm_sync_pages(struct kvm_vcpu *vcpu,  gfn_t gfn)
 			continue;
 
 		WARN_ON(s->role.level != PT_PAGE_TABLE_LEVEL);
-		kvm_unlink_unsync_page(vcpu->kvm, s);
-		if (!__kvm_sync_page(vcpu, s, &invalid_list, false))
+		if (!kvm_sync_page(vcpu, s, &invalid_list))
 			flush = true;
 	}
 
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1341967 — [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page

kvm_mmu_get_page is the only caller of kvm_sync_page_transient
and kvm_sync_pages.  Moving the handling of the invalid_list there
removes the need for the underdocumented kvm_sync_page_transient
function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
	Guangrong, at this point I am confused about why
	kvm_sync_page_transient didn't clear sp->unsync.  Do
	you remember?  Or perhaps kvm_mmu_get_page could just
	call kvm_sync_page now?

	Also, can you explain the need_sync variable in
	kvm_mmu_get_page?

 arch/x86/kvm/mmu.c | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index e3215cc89d97..725316df32ec 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1917,18 +1917,6 @@ static void kvm_mmu_flush_or_zap(struct kvm_vcpu *vcpu,
 		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 }
 
-static bool kvm_sync_page_transient(struct kvm_vcpu *vcpu,
-				    struct kvm_mmu_page *sp)
-{
-	LIST_HEAD(invalid_list);
-	int ret;
-
-	ret = __kvm_sync_page(vcpu, sp, &invalid_list);
-	kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, ret);
-
-	return ret;
-}
-
 #ifdef CONFIG_KVM_MMU_AUDIT
 #include "mmu_audit.c"
 #else
@@ -1944,21 +1932,21 @@ static bool kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 }
 
 /* @gfn should be write-protected at the call site */
-static void kvm_sync_pages(struct kvm_vcpu *vcpu,  gfn_t gfn)
+static bool kvm_sync_pages(struct kvm_vcpu *vcpu, gfn_t gfn,
+			   struct list_head *invalid_list)
 {
 	struct kvm_mmu_page *s;
-	LIST_HEAD(invalid_list);
-	bool flush = false;
+	bool ret = false;
 
 	for_each_gfn_indirect_valid_sp(vcpu->kvm, s, gfn) {
 		if (!s->unsync)
 			continue;
 
 		WARN_ON(s->role.level != PT_PAGE_TABLE_LEVEL);
-		flush |= kvm_sync_page(vcpu, s, &invalid_list);
+		ret |= kvm_sync_page(vcpu, s, invalid_list);
 	}
 
-	kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, flush);
+	return ret;
 }
 
 struct mmu_page_path {
@@ -2089,6 +2077,8 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 	unsigned quadrant;
 	struct kvm_mmu_page *sp;
 	bool need_sync = false;
+	bool flush = false;
+	LIST_HEAD(invalid_list);
 
 	role = vcpu->arch.mmu.base_role;
 	role.level = level;
@@ -2112,8 +2103,16 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		if (sp->role.word != role.word)
 			continue;
 
-		if (sp->unsync && !kvm_sync_page_transient(vcpu, sp))
-			break;
+		if (sp->unsync) {
+			/* The page is good, but __kvm_sync_page might still end
+			 * up zapping it.  If so, break in order to rebuild it.
+			 */
+			if (!__kvm_sync_page(vcpu, sp, &invalid_list))
+				break;
+
+			WARN_ON(!list_empty(&invalid_list));
+			kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+		}
 
 		if (sp->unsync_children)
 			kvm_make_request(KVM_REQ_MMU_SYNC, vcpu);
@@ -2133,13 +2132,15 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		if (rmap_write_protect(vcpu, gfn))
 			kvm_flush_remote_tlbs(vcpu->kvm);
 		if (level > PT_PAGE_TABLE_LEVEL && need_sync)
-			kvm_sync_pages(vcpu, gfn);
+			flush |= kvm_sync_pages(vcpu, gfn, &invalid_list);
 
 		account_shadowed(vcpu->kvm, sp);
 	}
 	sp->mmu_valid_gen = vcpu->kvm->arch.mmu_valid_gen;
 	clear_page(sp->spt);
 	trace_kvm_mmu_get_page(sp, true);
+
+	kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, flush);
 	return sp;
 }
 
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1342902 — Re: [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page

On 02/24/2016 09:17 PM, Paolo Bonzini wrote:
> kvm_mmu_get_page is the only caller of kvm_sync_page_transient
> and kvm_sync_pages.  Moving the handling of the invalid_list there
> removes the need for the underdocumented kvm_sync_page_transient
> function.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 	Guangrong, at this point I am confused about why
> 	kvm_sync_page_transient didn't clear sp->unsync.  Do
> 	you remember?  Or perhaps kvm_mmu_get_page could just
> 	call kvm_sync_page now?
>

It is the optimization to reduce write-protect as changing unsync to
sync need to write-protect the page and sync all sptes pointing to the
same gfn.

However, after syncing the content between unsync-ed spte and guest pte,
we can reuse this spte perfectly.

> 	Also, can you explain the need_sync variable in
> 	kvm_mmu_get_page?

This is because we need to to protect the semanteme of 'unsync spte' as
only the spte on last level (level = 1) can be unsync so that if a spte
on the upper level is created we should eliminate all the unsync sptes
pointing to the same gfn.

As you have already merged this patchset to the kvm tree, i will post
a patch to comment these cases to make the code be more understandable.

[toc] | [prev] | [next] | [standalone]

#1342983 — Re: [PATCH 08/12] KVM: MMU: move zap/flush to kvm_mmu_get_page

On 25/02/2016 08:32, Xiao Guangrong wrote:
> 
> As you have already merged this patchset to the kvm tree, i will post
> a patch to comment these cases to make the code be more understandable.

I've only merged it to kvm/queue so that it gets into all my testing
(and the buildbot's).  I won't move it to kvm/next until I get reviews.

Paolo

[toc] | [prev] | [next] | [standalone]

#1341968 — [PATCH 02/12] KVM: MMU: check kvm_mmu_pages and mmu_page_path indices

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Give a special invalid index to the root of the walk, so that we
can check the consistency of kvm_mmu_pages and mmu_page_path.

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
[Extracted from a bigger patch proposed by Guangrong. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 4dee855897cf..3060873d8cab 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1840,6 +1840,8 @@ static int __mmu_unsync_walk(struct kvm_mmu_page *sp,
 	return nr_unsync_leaf;
 }
 
+#define INVALID_INDEX (-1)
+
 static int mmu_unsync_walk(struct kvm_mmu_page *sp,
 			   struct kvm_mmu_pages *pvec)
 {
@@ -1847,7 +1849,7 @@ static int mmu_unsync_walk(struct kvm_mmu_page *sp,
 	if (!sp->unsync_children)
 		return 0;
 
-	mmu_pages_add(pvec, sp, 0);
+	mmu_pages_add(pvec, sp, INVALID_INDEX);
 	return __mmu_unsync_walk(sp, pvec);
 }
 
@@ -1996,6 +1998,8 @@ static int mmu_pages_first(struct kvm_mmu_pages *pvec,
 	if (pvec->nr == 0)
 		return 0;
 
+	WARN_ON(pvec->page[0].idx != INVALID_INDEX);
+
 	sp = pvec->page[0].sp;
 	level = sp->role.level;
 	WARN_ON(level == PT_PAGE_TABLE_LEVEL);
@@ -2020,6 +2024,7 @@ static void mmu_pages_clear_parents(struct mmu_page_path *parents)
 		if (!sp)
 			return;
 
+		WARN_ON(idx == INVALID_INDEX);
 		clear_unsync_child_bit(sp, idx);
 		level++;
 	} while (!sp->unsync_children);
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1341969 — [PATCH 03/12] KVM: MMU: introduce kvm_mmu_flush_or_zap

This is a generalization of mmu_pte_write_flush_tlb, that also
takes care of calling kvm_mmu_commit_zap_page.  The next
patches will introduce more uses.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 3060873d8cab..6bf74d8d4989 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4125,11 +4125,14 @@ static bool need_remote_flush(u64 old, u64 new)
 	return (old & ~new & PT64_PERM_MASK) != 0;
 }
 
-static void mmu_pte_write_flush_tlb(struct kvm_vcpu *vcpu, bool zap_page,
-				    bool remote_flush, bool local_flush)
+static void kvm_mmu_flush_or_zap(struct kvm_vcpu *vcpu,
+				 struct list_head *invalid_list,
+				 bool remote_flush, bool local_flush)
 {
-	if (zap_page)
+	if (!list_empty(invalid_list)) {
+		kvm_mmu_commit_zap_page(vcpu->kvm, invalid_list);
 		return;
+	}
 
 	if (remote_flush)
 		kvm_flush_remote_tlbs(vcpu->kvm);
@@ -4256,7 +4259,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 	LIST_HEAD(invalid_list);
 	u64 entry, gentry, *spte;
 	int npte;
-	bool remote_flush, local_flush, zap_page;
+	bool remote_flush, local_flush;
 	union kvm_mmu_page_role mask = { };
 
 	mask.cr0_wp = 1;
@@ -4273,7 +4276,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 	if (!ACCESS_ONCE(vcpu->kvm->arch.indirect_shadow_pages))
 		return;
 
-	zap_page = remote_flush = local_flush = false;
+	remote_flush = local_flush = false;
 
 	pgprintk("%s: gpa %llx bytes %d\n", __func__, gpa, bytes);
 
@@ -4293,8 +4296,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 	for_each_gfn_indirect_valid_sp(vcpu->kvm, sp, gfn) {
 		if (detect_write_misaligned(sp, gpa, bytes) ||
 		      detect_write_flooding(sp)) {
-			zap_page |= !!kvm_mmu_prepare_zap_page(vcpu->kvm, sp,
-						     &invalid_list);
+			kvm_mmu_prepare_zap_page(vcpu->kvm, sp, &invalid_list);
 			++vcpu->kvm->stat.mmu_flooded;
 			continue;
 		}
@@ -4316,8 +4318,7 @@ void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
 			++spte;
 		}
 	}
-	mmu_pte_write_flush_tlb(vcpu, zap_page, remote_flush, local_flush);
-	kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
+	kvm_mmu_flush_or_zap(vcpu, &invalid_list, remote_flush, local_flush);
 	kvm_mmu_audit(vcpu, AUDIT_POST_PTE_WRITE);
 	spin_unlock(&vcpu->kvm->mmu_lock);
 }
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1341970 — [PATCH 04/12] KVM: MMU: move TLB flush out of __kvm_sync_page

By doing this, kvm_sync_pages can use __kvm_sync_page instead of
reinventing it.  Because of kvm_mmu_flush_or_zap, the code does not
end up being more complex than before, and more cleanups to kvm_sync_pages
will come in the next patches.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu.c | 53 ++++++++++++++++++++++++-----------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 6bf74d8d4989..c440a28822b7 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1902,10 +1902,24 @@ static int __kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 		return 1;
 	}
 
-	kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
 	return 0;
 }
 
+static void kvm_mmu_flush_or_zap(struct kvm_vcpu *vcpu,
+				 struct list_head *invalid_list,
+				 bool remote_flush, bool local_flush)
+{
+	if (!list_empty(invalid_list)) {
+		kvm_mmu_commit_zap_page(vcpu->kvm, invalid_list);
+		return;
+	}
+
+	if (remote_flush)
+		kvm_flush_remote_tlbs(vcpu->kvm);
+	else if (local_flush)
+		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+}
+
 static int kvm_sync_page_transient(struct kvm_vcpu *vcpu,
 				   struct kvm_mmu_page *sp)
 {
@@ -1913,8 +1927,7 @@ static int kvm_sync_page_transient(struct kvm_vcpu *vcpu,
 	int ret;
 
 	ret = __kvm_sync_page(vcpu, sp, &invalid_list, false);
-	if (ret)
-		kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
+	kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, !ret);
 
 	return ret;
 }
@@ -1945,17 +1958,11 @@ static void kvm_sync_pages(struct kvm_vcpu *vcpu,  gfn_t gfn)
 
 		WARN_ON(s->role.level != PT_PAGE_TABLE_LEVEL);
 		kvm_unlink_unsync_page(vcpu->kvm, s);
-		if ((s->role.cr4_pae != !!is_pae(vcpu)) ||
-			(vcpu->arch.mmu.sync_page(vcpu, s))) {
-			kvm_mmu_prepare_zap_page(vcpu->kvm, s, &invalid_list);
-			continue;
-		}
-		flush = true;
+		if (!__kvm_sync_page(vcpu, s, &invalid_list, false))
+			flush = true;
 	}
 
-	kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
-	if (flush)
-		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+	kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, flush);
 }
 
 struct mmu_page_path {
@@ -2041,6 +2048,7 @@ static void mmu_sync_children(struct kvm_vcpu *vcpu,
 
 	while (mmu_unsync_walk(parent, &pages)) {
 		bool protected = false;
+		bool flush = false;
 
 		for_each_sp(pages, sp, parents, i)
 			protected |= rmap_write_protect(vcpu, sp->gfn);
@@ -2049,10 +2057,12 @@ static void mmu_sync_children(struct kvm_vcpu *vcpu,
 			kvm_flush_remote_tlbs(vcpu->kvm);
 
 		for_each_sp(pages, sp, parents, i) {
-			kvm_sync_page(vcpu, sp, &invalid_list);
+			if (!kvm_sync_page(vcpu, sp, &invalid_list))
+				flush = true;
+
 			mmu_pages_clear_parents(&parents);
 		}
-		kvm_mmu_commit_zap_page(vcpu->kvm, &invalid_list);
+		kvm_mmu_flush_or_zap(vcpu, &invalid_list, false, flush);
 		cond_resched_lock(&vcpu->kvm->mmu_lock);
 	}
 }
@@ -4125,21 +4135,6 @@ static bool need_remote_flush(u64 old, u64 new)
 	return (old & ~new & PT64_PERM_MASK) != 0;
 }
 
-static void kvm_mmu_flush_or_zap(struct kvm_vcpu *vcpu,
-				 struct list_head *invalid_list,
-				 bool remote_flush, bool local_flush)
-{
-	if (!list_empty(invalid_list)) {
-		kvm_mmu_commit_zap_page(vcpu->kvm, invalid_list);
-		return;
-	}
-
-	if (remote_flush)
-		kvm_flush_remote_tlbs(vcpu->kvm);
-	else if (local_flush)
-		kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
-}
-
 static u64 mmu_pte_write_fetch_gpte(struct kvm_vcpu *vcpu, gpa_t *gpa,
 				    const u8 *new, int *bytes)
 {
-- 
1.8.3.1

[toc] | [prev] | [next] | [standalone]

#1342966 — Re: [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few micro-optimizations

On 02/24/2016 09:17 PM, Paolo Bonzini wrote:
> This series started from looking at mmu_unsync_walk for the ubsan thread.
> Patches 1 and 2 are the result of the discussions in that thread.
>
> Patches 3 to 9 do more cleanups in __kvm_sync_page and its callers.
> Among other changes, it removes kvm_sync_page_transient and avoids
> duplicate code between __kvm_sync_page and kvm_sync_pages.
>
> I stopped where I had questions about the existing kvm_mmu_get_page
> code (see patch 8 for the question).  However perhaps more cleanups
> are possible, also thanks to Takuya's work on that function and
> link_shadow_page.
>
> Patches 10 to 12 are just micro-optimizations.
>
> Guangrong, it would be great if you took a look since you know this part
> of KVM very well.

I have reviewed it and it works fine except the one leaking tlb flush out
of mmu-lock.

I will continue to simplify the path of walking unsync sp to keep
mmu_page_path smaller and make comments for kvm_mmu_get_page on top of
this patchset.

BTW, is any conflict to apply my page-tracking patchset on top of this
patchset (i noticed you've merged this patchset on kvm/queue)? Please
tell me to rebase it if it is needed.

[toc] | [prev] | [next] | [standalone]

#1342982 — Re: [PATCH 00/12] KVM: MMU: cleanup around kvm_sync_page, and a few micro-optimizations

On 25/02/2016 09:28, Xiao Guangrong wrote:
> 
> BTW, is any conflict to apply my page-tracking patchset on top of this
> patchset (i noticed you've merged this patchset on kvm/queue)? Please
> tell me to rebase it if it is needed.

No, there shouldn't be any conflict.

Paolo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web