Re: [PATCH v3 15/28] KVM: x86/mmu: Add dedicated helper to zap TDP MMU root shadow page

[Sean Christopherson <seanjc@google.com>]

On Thu, Mar 03, 2022, Mingwei Zhang wrote:
> On Thu, Mar 03, 2022, Mingwei Zhang wrote:
> > > +	/*
> > > +	 * No need to try to step down in the iterator when zapping an entire
> > > +	 * root, zapping an upper-level SPTE will recurse on its children.
> > > +	 */
> > > +	for_each_tdp_pte_min_level(iter, root, root->role.level, start, end) {
> > > +retry:
> > > +		/*
> > > +		 * Yielding isn't allowed when zapping an unreachable root as
> > > +		 * the root won't be processed by mmu_notifier callbacks.  When
> > > +		 * handling an unmap/release mmu_notifier command, KVM must
> > > +		 * drop all references to relevant pages prior to completing
> > > +		 * the callback.  Dropping mmu_lock can result in zapping SPTEs
> > > +		 * for an unreachable root after a relevant callback completes,
> > > +		 * which leads to use-after-free as zapping a SPTE triggers
> > > +		 * "writeback" of dirty/accessed bits to the SPTE's associated
> > > +		 * struct page.
> > > +		 */
> > 
> > I have a quick question here: when the roots are unreachable, we can't
> > yield, understand that after reading the comments. However, what if
> > there are too many SPTEs that need to be zapped that requires yielding.
> > In this case, I guess we will have a RCU warning, which is unavoidable,
> > right?
> 
> I will take that back. I think the subsequent patches solve the problem
> using two passes.

Yes, but it's worth noting that the yielding problem is also solved by keeping
roots reachable while they're being zapped (also done in later patches).  That
way if a mmu_notifier event comes along, it can guarantee the SPTEs it cares about
are zapped (and their metadata flushed) even if the MMU root is no longer usable
by a vCPU.