On Fri, 2025-05-16 at 12:01 +0800, Yan Zhao wrote: > > Maybe we should rename nx_huge_page_workaround_enabled to something more > > generic > > and do the is_mirror logic in kvm_mmu_do_page_fault() when setting it. It > > should > > shrink the diff and centralize the logic. > Hmm, I'm reluctant to rename nx_huge_page_workaround_enabled, because > > (1) Invoking disallowed_hugepage_adjust() for mirror root is to disable page > promotion for TDX private memory, so is only applied to TDP MMU. > (2) nx_huge_page_workaround_enabled is used specifically for nx huge pages. > fault->huge_page_disallowed = fault->exec && fault- > >nx_huge_page_workaround_enabled; Oh, good point. > > if (fault->huge_page_disallowed) > account_nx_huge_page(vcpu->kvm, sp, > fault->req_level >= it.level); > > sp->nx_huge_page_disallowed = fault->huge_page_disallowed. > > Affecting fault->huge_page_disallowed would impact > sp->nx_huge_page_disallowed as well and would disable huge pages entirely. > > So, we still need to keep nx_huge_page_workaround_enabled. > > If we introduce a new flag fault->disable_hugepage_adjust, and set it in > kvm_mmu_do_page_fault(), we would also need to invoke > tdp_mmu_get_root_for_fault() there as well. > > Checking for mirror root for non-TDX VMs is not necessary, and the invocation > of > tdp_mmu_get_root_for_fault() seems redundant with the one in > kvm_tdp_mmu_map(). Also true. What about a wrapper for MMU code to check instead of fault- >nx_huge_page_workaround_enabled then? Also, why not check is_mirror_sp() in disallowed_hugepage_adjust() instead of passing in an is_mirror arg? There must be a way to have it fit in better with disallowed_hugepage_adjust() without adding so much open coded boolean logic.
