From: Paul Mackerras <paulus@ozlabs.org> To: kvm@vger.kernel.org Cc: kvm-ppc@vger.kernel.org, Laurent Vivier <lvivier@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Nick Piggin <npiggin@au1.ibm.com> Subject: [PATCH 1/2] KVM: PPC: Book3S HV: Remove user-triggerable WARN_ON Date: Thu, 28 May 2020 11:53:31 +1000 [thread overview] Message-ID: <20200528015331.GD307798@thinks.paulus.ozlabs.org> (raw) Although in general we do not expect valid PTEs to be found in kvmppc_create_pte when we are inserting a large page mapping, there is one situation where this can occur. That is when dirty page logging is turned off for a memslot while the VM is running. Because the new memslots are installed before the old memslot is flushed in kvmppc_core_commit_memory_region_hv(), there is a window where a hypervisor page fault can try to install a 2MB (or 1GB) page where there are already small page mappings which were installed while dirty page logging was enabled and which have not yet been flushed. Since we have a situation where valid PTEs can legitimately be found by kvmppc_unmap_free_pte, and which can be triggered by userspace, just remove the WARN_ON_ONCE, since it is undesirable to have userspace able to trigger a kernel warning. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_64_mmu_radix.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c index 97b45ea..bc3f795 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c @@ -429,9 +429,13 @@ void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte, unsigned long gpa, * Callers are responsible for flushing the PWC. * * When page tables are being unmapped/freed as part of page fault path - * (full == false), ptes are not expected. There is code to unmap them - * and emit a warning if encountered, but there may already be data - * corruption due to the unexpected mappings. + * (full == false), valid ptes are generally not expected; however, there + * is one situation where they arise, which is when dirty page logging is + * turned off for a memslot while the VM is running. The new memslot + * becomes visible to page faults before the memslot commit function + * gets to flush the memslot, which can lead to a 2MB page mapping being + * installed for a guest physical address where there are already 64kB + * (or 4kB) mappings (of sub-pages of the same 2MB page). */ static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full, unsigned int lpid) @@ -445,7 +449,6 @@ static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full, for (it = 0; it < PTRS_PER_PTE; ++it, ++p) { if (pte_val(*p) == 0) continue; - WARN_ON_ONCE(1); kvmppc_unmap_pte(kvm, p, pte_pfn(*p) << PAGE_SHIFT, PAGE_SHIFT, NULL, lpid); -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: Paul Mackerras <paulus@ozlabs.org> To: kvm@vger.kernel.org Cc: kvm-ppc@vger.kernel.org, Laurent Vivier <lvivier@redhat.com>, David Gibson <david@gibson.dropbear.id.au>, Nick Piggin <npiggin@au1.ibm.com> Subject: [PATCH 1/2] KVM: PPC: Book3S HV: Remove user-triggerable WARN_ON Date: Thu, 28 May 2020 01:53:31 +0000 [thread overview] Message-ID: <20200528015331.GD307798@thinks.paulus.ozlabs.org> (raw) Although in general we do not expect valid PTEs to be found in kvmppc_create_pte when we are inserting a large page mapping, there is one situation where this can occur. That is when dirty page logging is turned off for a memslot while the VM is running. Because the new memslots are installed before the old memslot is flushed in kvmppc_core_commit_memory_region_hv(), there is a window where a hypervisor page fault can try to install a 2MB (or 1GB) page where there are already small page mappings which were installed while dirty page logging was enabled and which have not yet been flushed. Since we have a situation where valid PTEs can legitimately be found by kvmppc_unmap_free_pte, and which can be triggered by userspace, just remove the WARN_ON_ONCE, since it is undesirable to have userspace able to trigger a kernel warning. Signed-off-by: Paul Mackerras <paulus@ozlabs.org> --- arch/powerpc/kvm/book3s_64_mmu_radix.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/kvm/book3s_64_mmu_radix.c b/arch/powerpc/kvm/book3s_64_mmu_radix.c index 97b45ea..bc3f795 100644 --- a/arch/powerpc/kvm/book3s_64_mmu_radix.c +++ b/arch/powerpc/kvm/book3s_64_mmu_radix.c @@ -429,9 +429,13 @@ void kvmppc_unmap_pte(struct kvm *kvm, pte_t *pte, unsigned long gpa, * Callers are responsible for flushing the PWC. * * When page tables are being unmapped/freed as part of page fault path - * (full = false), ptes are not expected. There is code to unmap them - * and emit a warning if encountered, but there may already be data - * corruption due to the unexpected mappings. + * (full = false), valid ptes are generally not expected; however, there + * is one situation where they arise, which is when dirty page logging is + * turned off for a memslot while the VM is running. The new memslot + * becomes visible to page faults before the memslot commit function + * gets to flush the memslot, which can lead to a 2MB page mapping being + * installed for a guest physical address where there are already 64kB + * (or 4kB) mappings (of sub-pages of the same 2MB page). */ static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full, unsigned int lpid) @@ -445,7 +449,6 @@ static void kvmppc_unmap_free_pte(struct kvm *kvm, pte_t *pte, bool full, for (it = 0; it < PTRS_PER_PTE; ++it, ++p) { if (pte_val(*p) = 0) continue; - WARN_ON_ONCE(1); kvmppc_unmap_pte(kvm, p, pte_pfn(*p) << PAGE_SHIFT, PAGE_SHIFT, NULL, lpid); -- 2.7.4
next reply other threads:[~2020-05-28 1:54 UTC|newest] Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-05-28 1:53 Paul Mackerras [this message] 2020-05-28 1:53 ` [PATCH 1/2] KVM: PPC: Book3S HV: Remove user-triggerable WARN_ON Paul Mackerras 2020-05-28 1:54 ` [PATCH 2/2] KVM: PPC: Book3S HV: Close race with page faults around memslot flushes Paul Mackerras 2020-05-28 1:54 ` Paul Mackerras 2021-01-18 12:26 ` [PATCH 1/2] KVM: PPC: Book3S HV: Remove shared-TLB optimisation from vCPU TLB coherency logic Nicholas Piggin 2021-02-09 7:19 ` Paul Mackerras 2021-02-09 8:44 ` Nicholas Piggin 2021-02-10 0:39 ` Paul Mackerras 2021-02-10 1:44 ` Nicholas Piggin 2021-02-10 2:46 ` Paul Mackerras 2021-02-10 5:33 ` Nicholas Piggin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200528015331.GD307798@thinks.paulus.ozlabs.org \ --to=paulus@ozlabs.org \ --cc=david@gibson.dropbear.id.au \ --cc=kvm-ppc@vger.kernel.org \ --cc=kvm@vger.kernel.org \ --cc=lvivier@redhat.com \ --cc=npiggin@au1.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.