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

On Mon, Aug 17, 2026 at 06:37:39AM -0700, Sean Christopherson wrote:
> 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?
Oh, I forgot about the shadow MMU. By "direct roots", I was referring to
"!mirror roots".
kvm_tdp_mmu_map_private_pfn() is currently solely called by TDX. So, even when a
vCPU has KVM_REQ_MMU_FREE_OBSOLETE_ROOTS pending, it should only be caused by
invalidating !mirror roots.
Therefore, I didn't understand why kvm_tdp_mmu_map_private_pfn() cares about
invalidating a !mirror root, which cannot be the same root to be mapped in
kvm_tdp_mmu_map().

> > 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.
Ok.