On Fri, Aug 29, 2025 at 09:16:47AM +0800, Yan Zhao wrote: > On Thu, Aug 28, 2025 at 12:40:20PM -0700, Sean Christopherson wrote: > > On Thu, Aug 28, 2025, Yan Zhao wrote: > > > On Thu, Aug 28, 2025 at 09:26:50AM +0800, Edgecombe, Rick P wrote: > > > > On Wed, 2025-08-27 at 17:54 -0700, Rick Edgecombe wrote: > > > > > > > > > > > > Then, what about setting > > > > > > .max_level = PG_LEVEL_4K, > > > > > > directly? > > > > > > > > > > > > Otherwise, the "(KVM_BUG_ON(level != PG_LEVEL_4K, kvm)" would be triggered > > > > > > in > > > > > > tdx_sept_set_private_spte(). > > > > > > > > > > Yes this fails to boot a TD. With max_level = PG_LEVEL_4K it passes the full > > > > > tests. I don't think it's ideal to encode PAGE.ADD details here though. > > > > > > > > > > But I'm not immediately clear what is going wrong. The old struct > > > > > kvm_page_fault > > > > > looks pretty similar. Did you root cause it? > > > > > > > > Oh, duh. Because we are passing in the PFN now so it can't know the size. So > > > > it's not about PAGE.ADD actually. > > > Right, it's because the previous kvm_tdp_map_page() updates fault->max_level in > > > kvm_mmu_faultin_pfn_private() by checking the private_max_mapping_level hook. > > > > > > However, private_max_mapping_level() skips the faultin step and goes straight > > > to kvm_tdp_mmu_map(). > > > > > > > Sill, how about calling the function kvm_tdp_mmu_map_private_pfn_4k(), or > > > > passing in the level? > > > Looks [1] can also address this issue. Not sure which one Sean prefers. > > > > > > [1] https://lore.kernel.org/all/20250729225455.670324-15-seanjc@xxxxxxxxxx > > > > That won't fix this issue though, becuase @fault will be valid and so max_level > Ah, right, I missed that you composed a fault... FWIW: after reviewing it again, I think [1] is still able update the max_level to 4KB. The flow with a valid @fault: kvm_mmu_hugepage_adjust kvm_mmu_max_mapping_level kvm_max_private_mapping_level kvm_x86_call(gmem_max_mapping_level)(kvm, pfn); > > will still be KVM_MAX_HUGEPAGE_LEVEL. Which is by design, the intent in that > > flow is that KVM should have gotten the level when getting the pfn from gmem. > > > > IIUC, this particular flow _must_ map at 4KiB, so I think forcing PG_LEVEL_4k is > > the right solution. > Forcing PG_LEVEL_4k looks good to me. > I was worried that SEV might want to use higher levels.
