All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Matlack <dmatlack@google.com>
To: Hou Wenlong <houwenlong.hwl@antgroup.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH 0/5] Fix wrong gfn range of tlb flushing with range
Date: Tue, 26 Jul 2022 15:59:20 -0700	[thread overview]
Message-ID: <YuBxyPl9W9mWaBRS@google.com> (raw)
In-Reply-To: <cover.1656039275.git.houwenlong.hwl@antgroup.com>

On Fri, Jun 24, 2022 at 11:36:56AM +0800, Hou Wenlong wrote:
> Commit c3134ce240eed
> ("KVM: Replace old tlb flush function with new one to flush a specified range.")
> replaces old tlb flush function with kvm_flush_remote_tlbs_with_address()
> to do tlb flushing. However, the gfn range of tlb flushing is wrong in
> some cases. E.g., when a spte is dropped, the start gfn of tlb flushing
> should be the gfn of spte not the base gfn of SP which contains the spte.
> So this patchset would fix them and do some cleanups.

One thing that would help prevent future buggy use of
kvm_flush_remote_tlbs_with_address(), and clean up this series, would be to
introduce some helper functions for common operations. In fact, even if
there is only one caller, I still think it would be useful to have helper
functions because it makes it clear the author's intent.

For example, I think the following helpers would be useful in this series:

/* Flush the given page (huge or not) of guest memory. */
static void kvm_flush_remote_tlbs_gfn(struct kvm *kvm, gfn_t gfn, int level)
{
        u64 pages = KVM_PAGES_PER_HPAGE(level);

        kvm_flush_remote_tlbs_with_address(kvm, gfn, pages);
}

/* Flush the range of guest memory mapped by the given SPTE. */
static void kvm_flush_remote_tlbs_sptep(struct kvm *kvm, u64 *sptep)
{
        struct kvm_mmu_page *sp = sptep_to_sp(sptep);
        gfn_t gfn = kvm_mmu_page_get_gfn(sp, spte_index(sptep));

        kvm_flush_remote_tlbs_gfn(kvm, gfn, sp->role.level);
}

/* Flush all memory mapped by the given direct SP. */
static void kvm_flush_remote_tlbs_direct_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
{
        WARN_ON_ONCE(!sp->role.direct);
        kvm_flush_remote_tlbs_gfn(kvm, sp->gfn, sp->role.level + 1);
}

> 
> Hou Wenlong (5):
>   KVM: x86/mmu: Fix wrong gfn range of tlb flushing in
>     validate_direct_spte()
>   KVM: x86/mmu: Fix wrong gfn range of tlb flushing in
>     kvm_set_pte_rmapp()
>   KVM: x86/mmu: Reduce gfn range of tlb flushing in
>     tdp_mmu_map_handle_target_level()
>   KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range
>   KVM: x86/mmu: Use 1 as the size of gfn range for tlb flushing in
>     FNAME(invlpg)()
> 
>  arch/x86/kvm/mmu/mmu.c         | 15 +++++++++------
>  arch/x86/kvm/mmu/paging_tmpl.h |  2 +-
>  arch/x86/kvm/mmu/tdp_mmu.c     |  4 ++--
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> --
> 2.31.1
> 

  parent reply	other threads:[~2022-07-26 22:59 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24  3:36 [PATCH 0/5] Fix wrong gfn range of tlb flushing with range Hou Wenlong
2022-06-24  3:36 ` [PATCH 1/5] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in validate_direct_spte() Hou Wenlong
2022-06-24  3:36 ` [PATCH 2/5] KVM: x86/mmu: Fix wrong gfn range of tlb flushing in kvm_set_pte_rmapp() Hou Wenlong
2022-06-24 22:53   ` Sean Christopherson
2022-06-27  4:00     ` Hou Wenlong
2022-06-24  3:36 ` [PATCH 3/5] KVM: x86/mmu: Reduce gfn range of tlb flushing in tdp_mmu_map_handle_target_level() Hou Wenlong
2022-06-24  3:37 ` [PATCH 4/5] KVM: x86/mmu: Fix wrong start gfn of tlb flushing with range Hou Wenlong
2022-06-24  3:37 ` [PATCH 5/5] KVM: x86/mmu: Use 1 as the size of gfn range for tlb flushing in FNAME(invlpg)() Hou Wenlong
2022-06-24 23:06 ` [PATCH 0/5] Fix wrong gfn range of tlb flushing with range Sean Christopherson
2022-06-25  9:13   ` Paolo Bonzini
2022-06-27  4:15     ` Hou Wenlong
2022-06-28 12:59       ` Paolo Bonzini
2022-07-26 22:59 ` David Matlack [this message]
2022-07-27 12:14   ` Hou Wenlong

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=YuBxyPl9W9mWaBRS@google.com \
    --to=dmatlack@google.com \
    --cc=houwenlong.hwl@antgroup.com \
    --cc=kvm@vger.kernel.org \
    /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: link
Be 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.