Re: [PATCH 2/4] KVM: x86/mmu: Harden "map private PFN" against unexpected root invalidation

On Mon, Aug 17, 2026, Yan Zhao wrote:
> On Tue, Aug 11, 2026 at 10:07:41AM -0700, Sean Christopherson wrote:
> > On Tue, Aug 11, 2026, Yan Zhao wrote:
> > > On Thu, Aug 06, 2026 at 02:40:48PM -0700, Sean Christopherson wrote:
> > > > Move kvm_tdp_mmu_map_private_pfn()'s reload of the MMU into its tight loop
> > > > so that an unexpected root invalidation has a better chance of being
> > > > handled gracefully, even though it should be impossible for the vCPU's root
> > > > to be invalidated after the initial reload.  As is, encountering an invalid
> > > > root is *guaranteed* to put the task into an infinite loop (albeit a
> > > > breakable loop that honors NEED_RESCHED).
> > > Note: without the newly added is_page_fault_stale() check in patch 4, an invalid
> > > root would not put the task into an infinite loop :) 
> > > 
> > > BTW: As noted in [1], is_page_fault_stale() only checks !mirror roots, and
> > > kvm_mmu_reload() reloads mirror roots only when !mirror roots are also invalid,
> > > since an invalid mirror root was considered impossible. (up to now, no?)
> > > 
> > > [1] https://lore.kernel.org/all/[email protected]
> > 
> > > > Add a WARN to try and detect bugs that break KVM's expectations, along with
> > > > a comment to explain why it should be impossible for the root to be
> > > > invalidated.
> > > And there's already a warning in kvm_tdp_mmu_map():
> > > "KVM_MMU_WARN_ON(!root || root->role.invalid);".
> > > So the warning also seems redundant.
> > 
> > No, KVM_REQ_MMU_FREE_OBSOLETE_ROOTS can be pending even if the current root is
> > valid.  And once the is_page_fault_stale() check comes along, the WARN in
> > kvm_tdp_mmu_map() is effectively unreachable.  The patch ordering is weird, but
> > there wasn't a great solution because adding is_page_fault_stale() first would
> > create an obvious infinite loop.
> Ok. When KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is pending, it is only for direct roots.

No?  KVM_REQ_MMU_FREE_OBSOLETE_ROOTS is also used by the shadow MMU, in
__kvm_mmu_prepare_zap_page() and in FNAME(fetch).  Or did I misunderstand the
question?

> This patch still holds that assumption true, right?
> 
> If so, since mirror roots cannot be invalid after the first reload, any
> subsequent kvm_mmu_reload() calls triggered later in the loop would only reload
> direct roots.
> 
> Is this necessary for kvm_tdp_mmu_map_private_pfn(), which only maps private pfn?
> Or is the purpose of this patch simply to avoid the potential infinite loop
> after patch 4?

Yes, though I would still want this change even without patch 4, as there are no
guarantees that KVM won't gain an equivalent check in kvm_tdp_mmu_map() in the
future, e.g. as hardening.