Re: [PATCH] KVM: x86/mmu: Embed direct bits into gpa for KVM_PRE_FAULT_MEMORY

[Edgecombe, Rick P]

On Wed, 2025-06-11 at 12:37 -0700, Sean Christopherson wrote:
> Ugh, and the whole tdp_mmu_get_root_for_fault() handling is broken.
> is_page_fault_stale() only looks at mmu->root.hpa, i.e. could theoretically blow
> up if the shared root is somehow valid but the mirror root is not.  Probably can't
> happen in practice, but it's ugly.

We had some discussion on this root valid/invalid pattern:
https://lore.kernel.org/kvm/d33d00b88707961126a24b19f940de43ba6e6c56.camel@xxxxxxxxx/

It's brittle though.

> 
> Oof, and I've no idea what kvm_tdp_mmu_fast_pf_get_last_sptep() is doing.  It
> says:
> 
> 	/* Fast pf is not supported for mirrored roots  */
> 
> but I don't see anything that actually enforces that.

Functionally, page_fault_can_be_fast() should prevented this with the check of
kvm->arch.has_private_mem. But, yea it's not correct for being readable. The
mirror/external concepts only work if they make sense as independent concepts.
Otherwise it's just naming obfuscation.

> 
> So tdp_mmu_get_root_for_fault() should be a generic kvm_mmu_get_root_for_fault(),
> and tdp_mmu_get_root() simply shouldn't exist.
> 
> As for stuffing the correct GPA, with kvm_mmu_get_root_for_fault() being generic
> and the root holding its gfn modifier, kvm_tdp_map_page() can simply OR in the
> appropriate gfn (and maybe WARN if there's overlap?).