kvm: Fix page ageing bugs

1. We were calling clear_flush_young_notify in unmap_one, but we are
within an mmu notifier invalidate range scope. The spte exists no more
(due to range_start) and the accessed bit info has already been
propagated (due to kvm_pfn_set_accessed). Simply call
clear_flush_young.

2. We clear_flush_young on a primary MMU PMD, but this may be mapped
as a collection of PTEs by the secondary MMU (e.g. during log-dirty).
This required expanding the interface of the clear_flush_young mmu
notifier, so a lot of code has been trivially touched.

3. In the absence of shadow_accessed_mask (e.g. EPT A bit), we emulate
the access bit by blowing the spte. This requires proper synchronizing
with MMU notifier consumers, like every other removal of spte's does.

Signed-off-by: Andres Lagar-Cavilla <andreslc@google.com>
Acked-by: Rik van Riel <riel@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 032a8538318aadd1dee17b96466048cd4970b075..8c3f7eb62b54eaa54b39c4acc512f43d5bb2fd77 100644 (file)
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -170,7 +170,8 @@ unsigned long kvm_arm_num_regs(struct kvm_vcpu *vcpu);
 int kvm_arm_copy_reg_indices(struct kvm_vcpu *vcpu, u64 __user *indices);
 
 /* We do not have shadow page tables, hence the empty hooks */
-static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+static inline int kvm_age_hva(struct kvm *kvm, unsigned long start,
+                             unsigned long end)
 {
        return 0;
 }

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index be9970a59497f5b07870961e97673d0cc68a21f1..a3c671b3acc96773465d44bb0a0564106b58cfa2 100644 (file)
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -180,7 +180,8 @@ int kvm_unmap_hva_range(struct kvm *kvm,
 void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 
 /* We do not have shadow page tables, hence the empty hooks */
-static inline int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+static inline int kvm_age_hva(struct kvm *kvm, unsigned long start,
+                             unsigned long end)
 {
        return 0;
 }

diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 6040008823522f1bfe7d93b92dcea4626685322c..d329bc5543a2d2f36a370539e2ce56f6d38dcfc1 100644 (file)
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -56,7 +56,7 @@
 extern int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
 extern int kvm_unmap_hva_range(struct kvm *kvm,
                               unsigned long start, unsigned long end);
-extern int kvm_age_hva(struct kvm *kvm, unsigned long hva);
+extern int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 extern int kvm_test_age_hva(struct kvm *kvm, unsigned long hva);
 extern void kvm_set_spte_hva(struct kvm *kvm, unsigned long hva, pte_t pte);
 

diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index fb86a2299d8afb9f635b71a92ef2e7345abdedff..d4a92d7cea6a7acefa12e50f567117913ef77f80 100644 (file)
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -243,7 +243,7 @@ struct kvmppc_ops {
        int (*unmap_hva)(struct kvm *kvm, unsigned long hva);
        int (*unmap_hva_range)(struct kvm *kvm, unsigned long start,
                           unsigned long end);
-       int (*age_hva)(struct kvm *kvm, unsigned long hva);
+       int (*age_hva)(struct kvm *kvm, unsigned long start, unsigned long end);
        int (*test_age_hva)(struct kvm *kvm, unsigned long hva);
        void (*set_spte_hva)(struct kvm *kvm, unsigned long hva, pte_t pte);
        void (*mmu_destroy)(struct kvm_vcpu *vcpu);

diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index dd03f6b299ba13a38a28cd1c3b69e28041360ce1..c16cfbfeb7810b5cd1b71becf2239e249699cb27 100644 (file)
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -851,9 +851,9 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end)
        return kvm->arch.kvm_ops->unmap_hva_range(kvm, start, end);
 }
 
-int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end)
 {
-       return kvm->arch.kvm_ops->age_hva(kvm, hva);
+       return kvm->arch.kvm_ops->age_hva(kvm, start, end);
 }
 
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)

diff --git a/arch/powerpc/kvm/book3s.h b/arch/powerpc/kvm/book3s.h
index 4bf956cf94d639e8428b21ffc5015397bfdc46f1..d2b3ec088b8cd7f02a7c5852904335c64a256599 100644 (file)
--- a/arch/powerpc/kvm/book3s.h
+++ b/arch/powerpc/kvm/book3s.h
@@ -17,7 +17,8 @@ extern void kvmppc_core_flush_memslot_hv(struct kvm *kvm,
 extern int kvm_unmap_hva_hv(struct kvm *kvm, unsigned long hva);
 extern int kvm_unmap_hva_range_hv(struct kvm *kvm, unsigned long start,
                                  unsigned long end);
-extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long hva);
+extern int kvm_age_hva_hv(struct kvm *kvm, unsigned long start,
+                         unsigned long end);
 extern int kvm_test_age_hva_hv(struct kvm *kvm, unsigned long hva);
 extern void kvm_set_spte_hva_hv(struct kvm *kvm, unsigned long hva, pte_t pte);
 

diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c b/arch/powerpc/kvm/book3s_64_mmu_hv.c
index 72c20bb16d266b4ed0aa2160382414f161ad0663..81460c5359c0b9121a4c8dcbaf8ddeaa30bb97a6 100644 (file)
--- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
+++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
@@ -1002,11 +1002,11 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
        return ret;
 }
 
-int kvm_age_hva_hv(struct kvm *kvm, unsigned long hva)
+int kvm_age_hva_hv(struct kvm *kvm, unsigned long start, unsigned long end)
 {
        if (!kvm->arch.using_mmu_notifiers)
                return 0;
-       return kvm_handle_hva(kvm, hva, kvm_age_rmapp);
+       return kvm_handle_hva_range(kvm, start, end, kvm_age_rmapp);
 }
 
 static int kvm_test_age_rmapp(struct kvm *kvm, unsigned long *rmapp,

diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index faffb27badd9de362465c6a8ea12052cb7ab53ef..852fcd8951c4163adf6f79bad2ef28e0f6dc2909 100644 (file)
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -295,7 +295,8 @@ static int kvm_unmap_hva_range_pr(struct kvm *kvm, unsigned long start,
        return 0;
 }
 
-static int kvm_age_hva_pr(struct kvm *kvm, unsigned long hva)
+static int kvm_age_hva_pr(struct kvm *kvm, unsigned long start,
+                         unsigned long end)
 {
        /* XXX could be more clever ;) */
        return 0;

diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 08f14bb57897ea530f8ebc128c6e5cf204eadcd9..b1f3f630315ee67f9bb8a78a24c34572f1a789f1 100644 (file)
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -732,7 +732,7 @@ int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end)
        return 0;
 }
 
-int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end)
 {
        /* XXX could be more clever ;) */
        return 0;

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index eeeb573fcf6fb0f421e663171f0a2239c76af650..763d273cab1d862de9d6915c40ea77aaa66d3d93 100644 (file)
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1035,7 +1035,7 @@ asmlinkage void kvm_spurious_fault(void);
 #define KVM_ARCH_WANT_MMU_NOTIFIER
 int kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
 int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end);
-int kvm_age_hva(struct kvm *kvm, unsigned long hva);
+int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end);
 int kvm_test_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);

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 47d534066325a1cb1ec78a735fcf1a611f79c465..3201e93ebd07ca336e7e15c046c8596b73fdcbc7 100644 (file)
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1417,18 +1417,7 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
        struct rmap_iterator uninitialized_var(iter);
        int young = 0;
 
-       /*
-        * In case of absence of EPT Access and Dirty Bits supports,
-        * emulate the accessed bit for EPT, by checking if this page has
-        * an EPT mapping, and clearing it if it does. On the next access,
-        * a new EPT mapping will be established.
-        * This has some overhead, but not as much as the cost of swapping
-        * out actively used pages or breaking up actively used hugepages.
-        */
-       if (!shadow_accessed_mask) {
-               young = kvm_unmap_rmapp(kvm, rmapp, slot, gfn, level, data);
-               goto out;
-       }
+       BUG_ON(!shadow_accessed_mask);
 
        for (sptep = rmap_get_first(*rmapp, &iter); sptep;
             sptep = rmap_get_next(&iter)) {
@@ -1440,7 +1429,6 @@ static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp,
                                 (unsigned long *)sptep);
                }
        }
-out:
        trace_kvm_age_page(gfn, level, slot, young);
        return young;
 }
@@ -1489,9 +1477,29 @@ static void rmap_recycle(struct kvm_vcpu *vcpu, u64 *spte, gfn_t gfn)
        kvm_flush_remote_tlbs(vcpu->kvm);
 }
 
-int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+int kvm_age_hva(struct kvm *kvm, unsigned long start, unsigned long end)
 {
-       return kvm_handle_hva(kvm, hva, 0, kvm_age_rmapp);
+       /*
+        * In case of absence of EPT Access and Dirty Bits supports,
+        * emulate the accessed bit for EPT, by checking if this page has
+        * an EPT mapping, and clearing it if it does. On the next access,
+        * a new EPT mapping will be established.
+        * This has some overhead, but not as much as the cost of swapping
+        * out actively used pages or breaking up actively used hugepages.
+        */
+       if (!shadow_accessed_mask) {
+               /*
+                * We are holding the kvm->mmu_lock, and we are blowing up
+                * shadow PTEs. MMU notifier consumers need to be kept at bay.
+                * This is correct as long as we don't decouple the mmu_lock
+                * protected regions (like invalidate_range_start|end does).
+                */
+               kvm->mmu_notifier_seq++;
+               return kvm_handle_hva_range(kvm, start, end, 0,
+                                           kvm_unmap_rmapp);
+       }
+
+       return kvm_handle_hva_range(kvm, start, end, 0, kvm_age_rmapp);
 }
 
 int kvm_test_age_hva(struct kvm *kvm, unsigned long hva)

diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 5f578e850fc5fa29c44e02000d07622b5610aa45..90d734bbf467a2cec462282191f64c8f2131c1ae 100644 (file)
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -402,9 +402,11 @@ static void __mn_flush_page(struct mmu_notifier *mn,
 
 static int mn_clear_flush_young(struct mmu_notifier *mn,
                                struct mm_struct *mm,
-                               unsigned long address)
+                               unsigned long start,
+                               unsigned long end)
 {
-       __mn_flush_page(mn, address);
+       for (; start < end; start += PAGE_SIZE)
+               __mn_flush_page(mn, start);
 
        return 0;
 }

diff --git a/include/linux/mmu_notifier.h b/include/linux/mmu_notifier.h
index 27288692241eebe928d9ec6020dc3c42d13f3755..88787bb4b3b93fe08d322c2a54a183486427a2af 100644 (file)
--- a/include/linux/mmu_notifier.h
+++ b/include/linux/mmu_notifier.h
@@ -57,10 +57,13 @@ struct mmu_notifier_ops {
         * pte. This way the VM will provide proper aging to the
         * accesses to the page through the secondary MMUs and not
         * only to the ones through the Linux pte.
+        * Start-end is necessary in case the secondary MMU is mapping the page
+        * at a smaller granularity than the primary MMU.
         */
        int (*clear_flush_young)(struct mmu_notifier *mn,
                                 struct mm_struct *mm,
-                                unsigned long address);
+                                unsigned long start,
+                                unsigned long end);
 
        /*
         * test_young is called to check the young/accessed bitflag in
@@ -175,7 +178,8 @@ extern void mmu_notifier_unregister_no_release(struct mmu_notifier *mn,
 extern void __mmu_notifier_mm_destroy(struct mm_struct *mm);
 extern void __mmu_notifier_release(struct mm_struct *mm);
 extern int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
-                                         unsigned long address);
+                                         unsigned long start,
+                                         unsigned long end);
 extern int __mmu_notifier_test_young(struct mm_struct *mm,
                                     unsigned long address);
 extern void __mmu_notifier_change_pte(struct mm_struct *mm,
@@ -194,10 +198,11 @@ static inline void mmu_notifier_release(struct mm_struct *mm)
 }
 
 static inline int mmu_notifier_clear_flush_young(struct mm_struct *mm,
-                                         unsigned long address)
+                                         unsigned long start,
+                                         unsigned long end)
 {
        if (mm_has_notifiers(mm))
-               return __mmu_notifier_clear_flush_young(mm, address);
+               return __mmu_notifier_clear_flush_young(mm, start, end);
        return 0;
 }
 
@@ -255,7 +260,9 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct *mm)
        unsigned long ___address = __address;                           \
        __young = ptep_clear_flush_young(___vma, ___address, __ptep);   \
        __young |= mmu_notifier_clear_flush_young(___vma->vm_mm,        \
-                                                 ___address);          \
+                                                 ___address,           \
+                                                 ___address +          \
+                                                       PAGE_SIZE);     \
        __young;                                                        \
 })
 
@@ -266,7 +273,9 @@ static inline void mmu_notifier_mm_destroy(struct mm_struct *mm)
        unsigned long ___address = __address;                           \
        __young = pmdp_clear_flush_young(___vma, ___address, __pmdp);   \
        __young |= mmu_notifier_clear_flush_young(___vma->vm_mm,        \
-                                                 ___address);          \
+                                                 ___address,           \
+                                                 ___address +          \
+                                                       PMD_SIZE);      \
        __young;                                                        \
 })
 
@@ -301,7 +310,8 @@ static inline void mmu_notifier_release(struct mm_struct *mm)
 }
 
 static inline int mmu_notifier_clear_flush_young(struct mm_struct *mm,
-                                         unsigned long address)
+                                         unsigned long start,
+                                         unsigned long end)
 {
        return 0;
 }

diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
index 950813b1eb3656dc49e66eab4e912fff76dbfc2b..2c8da9825fe3482740ba4f4f2a0a003fabafd519 100644 (file)
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -107,7 +107,8 @@ void __mmu_notifier_release(struct mm_struct *mm)
  * existed or not.
  */
 int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
-                                       unsigned long address)
+                                       unsigned long start,
+                                       unsigned long end)
 {
        struct mmu_notifier *mn;
        int young = 0, id;
@@ -115,7 +116,7 @@ int __mmu_notifier_clear_flush_young(struct mm_struct *mm,
        id = srcu_read_lock(&srcu);
        hlist_for_each_entry_rcu(mn, &mm->mmu_notifier_mm->list, hlist) {
                if (mn->ops->clear_flush_young)
-                       young |= mn->ops->clear_flush_young(mn, mm, address);
+                       young |= mn->ops->clear_flush_young(mn, mm, start, end);
        }
        srcu_read_unlock(&srcu, id);
 

diff --git a/mm/rmap.c b/mm/rmap.c
index 3e8491c504f8bedc432293484bfe1f31fa1e6f46..bc74e0012809943930ab52fcc3857e080f152049 100644 (file)
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1355,7 +1355,11 @@ static int try_to_unmap_cluster(unsigned long cursor, unsigned int *mapcount,
                        continue;       /* don't unmap */
                }
 
-               if (ptep_clear_flush_young_notify(vma, address, pte))
+               /*
+                * No need for _notify because we're within an
+                * mmu_notifier_invalidate_range_ {start|end} scope.
+                */
+               if (ptep_clear_flush_young(vma, address, pte))
                        continue;
 
                /* Nuke the page table entry. */

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ff42b11d2b9cd36abb497cb9bbc6dbcb10553577..0316314d48f43b4a074732f321531487eb58a3d5 100644 (file)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -369,7 +369,8 @@ static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn,
 
 static int kvm_mmu_notifier_clear_flush_young(struct mmu_notifier *mn,
                                              struct mm_struct *mm,
-                                             unsigned long address)
+                                             unsigned long start,
+                                             unsigned long end)
 {
        struct kvm *kvm = mmu_notifier_to_kvm(mn);
        int young, idx;
@@ -377,7 +378,7 @@ static int kvm_mmu_notifier_clear_flush_young(struct mmu_notifier *mn,
        idx = srcu_read_lock(&kvm->srcu);
        spin_lock(&kvm->mmu_lock);
 
-       young = kvm_age_hva(kvm, address);
+       young = kvm_age_hva(kvm, start, end);
        if (young)
                kvm_flush_remote_tlbs(kvm);