Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 14 Jul 2022 15:27:17 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Paolo Bonzini <pbonzini@redhat.com>
Cc:     Ben Gardon <bgardon@google.com>, linux-kernel@vger.kernel.org,
        kvm@vger.kernel.org, Peter Xu <peterx@redhat.com>,
        David Matlack <dmatlack@google.com>,
        Jim Mattson <jmattson@google.com>,
        David Dunn <daviddunn@google.com>,
        Jing Zhang <jingzhangos@google.com>,
        Junaid Shahid <junaids@google.com>
Subject: Re: [PATCH v2 0/9] KVM: x86/MMU: Optimize disabling dirty logging
Message-ID: <YtA11dFqXG6Ou5WE@google.com>
References: <20220321224358.1305530-1-bgardon@google.com>
 <dba0ecc8-90ae-975f-7a27-3049d6951ba0@redhat.com>
 <YszQcBy1RwGmkkht@google.com>
 <52ef13d4-068d-bd2c-11aa-c7053798aee9@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <52ef13d4-068d-bd2c-11aa-c7053798aee9@redhat.com>
Precedence: bulk

On Thu, Jul 14, 2022, Paolo Bonzini wrote:
> On 7/12/22 03:37, Sean Christopherson wrote:
> > This fell through the cracks.  Ben is on a long vacation, I'll find my copy of
> > the Necronomicon and do a bit of resurrection, and address the feedback from v2
> > along the way.
> 
> This was superseded by the simple patch to zap only the leaves I think?

Ah, right you are, commit 5ba7c4c6d1c7 ("KVM: x86/MMU: Zap non-leaf SPTEs when
disabling dirty logging").  I got somewhat confused because there's a stale comment
above the inner helper:

	/*
	 * Clear leaf entries which could be replaced by large mappings, for
	 * GFNs within the slot.
	 */

If we drop the "only refcounted struct pages can be huge" requirement, then the
flow becomes much simpler as there's no need to recurse down to the leafs only to
step back up:

	for_each_tdp_pte_min_level(iter, root, PG_LEVEL_2M, start, end) {
retry:
		if (tdp_mmu_iter_cond_resched(kvm, &iter, false, true))
			continue;

		if (!is_shadow_present_pte(iter.old_spte))
			continue;

		/*
		 * Don't zap leaf SPTEs, if a leaf SPTE could be replaced with
		 * a large page size, then its parent would have been zapped
		 * instead of stepping down.
		 */
		if (is_last_spte(iter.old_spte, iter.level))
			continue;

		max_mapping_level = kvm_mmu_max_mapping_level(kvm, slot,
							      iter.gfn, PG_LEVEL_NUM);
		if (max_mapping_level <= iter.level)
			continue;

		/* Note, a successful atomic zap also does a remote TLB flush. */
		if (tdp_mmu_zap_spte_atomic(kvm, &iter))
			goto retry;
	}