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, 3 Mar 2022 23:06:57 +0000
From:   Sean Christopherson <seanjc@google.com>
To:     Mingwei Zhang <mizhang@google.com>
Cc:     Paolo Bonzini <pbonzini@redhat.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Janosch Frank <frankja@linux.ibm.com>,
        Claudio Imbrenda <imbrenda@linux.ibm.com>,
        Vitaly Kuznetsov <vkuznets@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>,
        Joerg Roedel <joro@8bytes.org>,
        David Hildenbrand <david@redhat.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, David Matlack <dmatlack@google.com>,
        Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v3 15/28] KVM: x86/mmu: Add dedicated helper to zap TDP
 MMU root shadow page
Message-ID: <YiFKEQPLzlcA403J@google.com>
References: <20220226001546.360188-1-seanjc@google.com>
 <20220226001546.360188-16-seanjc@google.com>
 <YiEw7z9TCQJl+udS@google.com>
 <YiEyEWDkNxNIAn/z@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YiEyEWDkNxNIAn/z@google.com>
Precedence: bulk

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.