On Mon, Jul 20, 2015 at 11:34:21AM -0700, Andres Lagar-Cavilla wrote:

quoted

On Sun, Jul 19, 2015 at 5:31 AM, Vladimir Davydov [off-list ref]

[...]

quoted

quoted

+static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn,
+                                       struct mm_struct *mm,
+                                       unsigned long start,
+                                       unsigned long end)
+{
+       struct kvm *kvm = mmu_notifier_to_kvm(mn);
+       int young, idx;
+

If you need to cut out another version please add comments as to the two
issues raised:
- This doesn't proactively flush TLBs -- not obvious if it should.
- This adversely affects performance in Pre_haswell Intel EPT.

Oops, I stopped reading your e-mail in reply to the previous version of
this patch as soon as I saw the Reviewed-by tag, so I missed your
request for the comment, sorry about that.

Here it goes (incremental):
---

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ff4173ce6924..e69a5cb99571 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c

@@ -397,6 +397,19 @@ static int kvm_mmu_notifier_clear_young(struct mmu_notifier *mn,
 
 	idx = srcu_read_lock(&kvm->srcu);
 	spin_lock(&kvm->mmu_lock);
+	/*
+	 * Even though we do not flush TLB, this will still adversely
+	 * affect performance on pre-Haswell Intel EPT, where there is
+	 * no EPT Access Bit to clear so that we have to tear down EPT
+	 * tables instead. If we find this unacceptable, we can always
+	 * add a parameter to kvm_age_hva so that it effectively doesn't
+	 * do anything on clear_young.
+	 *
+	 * Also note that currently we never issue secondary TLB flushes
+	 * from clear_young, leaving this job up to the regular system
+	 * cadence. If we find this inaccurate, we might come up with a
+	 * more sophisticated heuristic later.
+	 */
 	young = kvm_age_hva(kvm, start, end);
 	spin_unlock(&kvm->mmu_lock);
 	srcu_read_unlock(&kvm->srcu, idx);