On Mon, Apr 17, 2023 at 12:37 PM Zhi Wang <zhi.wang.linux@xxxxxxxxx> wrote: > > On Fri, 7 Apr 2023 20:19:17 +0000 > Sagi Shahar <sagis@xxxxxxxxxx> wrote: > > This patch is actually adding a separate counter for accounting private > tdp mmu page not really introducing a new tdp_mmu_pages list for private > pages. I guess better refine the tittle to reflect what this patch > is doing. Thanks for catching this. tdp_mmu_pages actually used to be a list of pages when I first developed this code but was replaced with a counter in https://lore.kernel.org/all/20221019165618.927057-6-seanjc@xxxxxxxxxx/ > > > tdp_mmu_pages holds all the active pages used by the mmu. When we > > transfer the state during intra-host migration we need to transfer the > > private pages but not the shared ones. > > > Maybe explain a little bit about how the shared one is processed. Guess > one sentence is enough. How about: tdp_mmu_pages holds all the active pages used by the mmu. When we transfer the state during intra-host migration we need to transfer the private pages but not the shared ones. The shared pages are going to be re-faulted as needed on the destination, but that approach doesn't work for private pages which stores information in the secure EPT. > > Keeping them in separate counters makes this transfer more efficient. > > > > Signed-off-by: Sagi Shahar <sagis@xxxxxxxxxx> > > --- > > arch/x86/include/asm/kvm_host.h | 5 ++++- > > arch/x86/kvm/mmu/tdp_mmu.c | 11 +++++++++-- > > 2 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index ae377eec81987..5ed70cd9d74bf 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -1426,9 +1426,12 @@ struct kvm_arch { > > struct task_struct *nx_huge_page_recovery_thread; > > > > #ifdef CONFIG_X86_64 > > - /* The number of TDP MMU pages across all roots. */ > > + /* The number of non-private TDP MMU pages across all roots. */ > > atomic64_t tdp_mmu_pages; > > > > + /* Same as tdp_mmu_pages but only for private pages. */ > > + atomic64_t tdp_private_mmu_pages; > > + > > /* > > * List of struct kvm_mmu_pages being used as roots. > > * All struct kvm_mmu_pages in the list should have > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c > > index 58a236a69ec72..327dee4f6170e 100644 > > --- a/arch/x86/kvm/mmu/tdp_mmu.c > > +++ b/arch/x86/kvm/mmu/tdp_mmu.c > > @@ -44,6 +44,7 @@ void kvm_mmu_uninit_tdp_mmu(struct kvm *kvm) > > destroy_workqueue(kvm->arch.tdp_mmu_zap_wq); > > > > WARN_ON(atomic64_read(&kvm->arch.tdp_mmu_pages)); > > + WARN_ON(atomic64_read(&kvm->arch.tdp_private_mmu_pages)); > > WARN_ON(!list_empty(&kvm->arch.tdp_mmu_roots)); > > > > /* > > @@ -373,13 +374,19 @@ static void handle_changed_spte_dirty_log(struct kvm *kvm, int as_id, gfn_t gfn, > > static void tdp_account_mmu_page(struct kvm *kvm, struct kvm_mmu_page *sp) > > { > > kvm_account_pgtable_pages((void *)sp->spt, +1); > > - atomic64_inc(&kvm->arch.tdp_mmu_pages); > > + if (is_private_sp(sp)) > > + atomic64_inc(&kvm->arch.tdp_private_mmu_pages); > > + else > > + atomic64_inc(&kvm->arch.tdp_mmu_pages); > > } > > > > static void tdp_unaccount_mmu_page(struct kvm *kvm, struct kvm_mmu_page *sp) > > { > > kvm_account_pgtable_pages((void *)sp->spt, -1); > > - atomic64_dec(&kvm->arch.tdp_mmu_pages); > > + if (is_private_sp(sp)) > > + atomic64_dec(&kvm->arch.tdp_private_mmu_pages); > > + else > > + atomic64_dec(&kvm->arch.tdp_mmu_pages); > > } > > > > /** >