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?).
