| Message ID | 20230407201921.2703758-2-sagis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add TDX intra host migration support | expand |
On Fri, 7 Apr 2023 20:19:17 +0000 Sagi Shahar <sagis@google.com> 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. > 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. > Keeping them in separate counters makes this transfer more efficient. > > Signed-off-by: Sagi Shahar <sagis@google.com> > --- > 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); > } > > /**
On Mon, Apr 17, 2023 at 12:37 PM Zhi Wang <zhi.wang.linux@gmail.com> wrote: > > On Fri, 7 Apr 2023 20:19:17 +0000 > Sagi Shahar <sagis@google.com> 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@google.com/ > > > 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@google.com> > > --- > > 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); > > } > > > > /** >
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); } /**
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. Keeping them in separate counters makes this transfer more efficient. Signed-off-by: Sagi Shahar <sagis@google.com> --- arch/x86/include/asm/kvm_host.h | 5 ++++- arch/x86/kvm/mmu/tdp_mmu.c | 11 +++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-)