On Sun, Mar 13, 2022, Mingwei Zhang wrote:
> On Thu, Mar 3, 2022 at 11:39 AM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
> > @@ -898,13 +879,13 @@ static bool zap_gfn_range(struct kvm *kvm, struct kvm_mmu_page *root,
> > * SPTEs have been cleared and a TLB flush is needed before releasing the
> > * MMU lock.
> > */
> > -bool __kvm_tdp_mmu_zap_gfn_range(struct kvm *kvm, int as_id, gfn_t start,
> > - gfn_t end, bool can_yield, bool flush)
> > +bool kvm_tdp_mmu_zap_leafs(struct kvm *kvm, int as_id, gfn_t start, gfn_t end,
> > + bool can_yield, bool flush)
> > {
> > struct kvm_mmu_page *root;
> >
> > for_each_tdp_mmu_root_yield_safe(kvm, root, as_id)
> > - flush = zap_gfn_range(kvm, root, start, end, can_yield, flush);
> > + flush = tdp_mmu_zap_leafs(kvm, root, start, end, can_yield, false);
>
> hmm, I think we might have to be very careful here. If we only zap
> leafs, then there could be side effects. For instance, the code in
> disallowed_hugepage_adjust() may not work as intended. If you check
> the following condition in arch/x86/kvm/mmu/mmu.c:2918
>
> if (cur_level > PG_LEVEL_4K &&
> cur_level == fault->goal_level &&
> is_shadow_present_pte(spte) &&
> !is_large_pte(spte)) {
>
> If we previously use 4K mappings in this range due to various reasons
> (dirty logging etc), then afterwards, we zap the range. Then the guest
> touches a 4K and now we should map the range with whatever the maximum
> level we can for the guest.
>
> However, if we just zap only the leafs, then when the code comes to
> the above location, is_shadow_present_pte(spte) will return true,
> since the spte is a non-leaf (say a regular PMD entry). The whole if
> statement will be true, then we never allow remapping guest memory
> with huge pages.
But that's at worst a performance issue, and arguably working as intended. The
zap in this case is never due to the _guest_ unmapping the pfn, so odds are good
the guest will want to map back in the same pfns with the same permissions.
Zapping shadow pages so that the guest can maybe create a hugepage may end up
being a lot of extra work for no benefit. Or it may be a net positive. Either
way, it's not a functional issue.
