Date:   Fri, 17 Mar 2023 14:37:30 +0800
From:   Yan Zhao <yan.y.zhao@intel.com>
To:     Sean Christopherson <seanjc@google.com>
CC:     Paolo Bonzini <pbonzini@redhat.com>,
        Zhenyu Wang <zhenyuw@linux.intel.com>,
        Zhi Wang <zhi.a.wang@intel.com>, <kvm@vger.kernel.org>,
        <intel-gvt-dev@lists.freedesktop.org>,
        <intel-gfx@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
        Ben Gardon <bgardon@google.com>
Subject: Re: [PATCH v2 12/27] KVM: x86/mmu: Don't bounce through page-track
 mechanism for guest PTEs
Message-ID: <ZBQKqpWToZT2ca2D@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20230311002258.852397-1-seanjc@google.com>
 <20230311002258.852397-13-seanjc@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230311002258.852397-13-seanjc@google.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3aMuCUOF/31s4opsSiuUb5uGOICY+Ak4m2koIDI6XrlLDxBjTqJBEkXtD4Aa?=
 =?us-ascii?Q?COgHw3+1kcm4rQHRrohg2PGIBT+3ATQttHMSFKtIPOCMyi6jo+jlPCieNuOt?=
 =?us-ascii?Q?2xEg+CsAsInGMsNA/FSNzRQYm33mzbM8OG0NonDsvDHFl1A2QDSq2kw9JCeY?=
 =?us-ascii?Q?/R0m2sc91FPzCL6Vv1KgHzjDT3DTRai68Snj1Ihk31wDvTWNr5okEvcUCFDL?=
 =?us-ascii?Q?voyZVWEPAjjxr6Y1NtXR629p0851dv3VvyF4xIBSq2mEBEQWCKRUbsFkG8+s?=
 =?us-ascii?Q?m0T3Sti6ZX9dNMUra6ks17LyTJ1pMpiXChS3uxe55bBBgcaEZQd3LsaUkJSz?=
 =?us-ascii?Q?0duiSzNrTdV0ihjXZ1JacsUTq2KCXrLDv9pRzEebA3kfPzLNSPKMDeYBbUBJ?=
 =?us-ascii?Q?udc/TdA3mgyq7cy6agTenfggyBjxVXu04xSdu/KWklYYwHVKj8PE8gBQqEmO?=
 =?us-ascii?Q?pjk2sEqDKSXsZ8k5IdLJ8p8j/eTQIN2dDyUHODsS/k98t9I8WoOvkrMlPmD4?=
 =?us-ascii?Q?YREM/mWVxdLx4tpEaOWVp+O+J+4EO1wX5l+medEZdx5BIX3hE7WAODqDXyuC?=
 =?us-ascii?Q?g2oD7ARsZp5XmKWdJecvnqIWdjKnjEI8HC2ks/Y1xuKo8M10GljEFyhQmTXP?=
 =?us-ascii?Q?mC1qjiYhnAbkXpdOrn0TwC51Ho835c4CYKBahfaUO0ahTQ43rHbDGISYXTPW?=
 =?us-ascii?Q?xtA0vgfUOgihluxaFW7VciEb1NjTu/gR3Eh6iXMBr2O6nEl7WfxoEZFT9twF?=
 =?us-ascii?Q?0FcuN+b6SgDBqIE4Iq1kox6yFhwzSu7BSzqvJOraTaYexfdZdcYQzfp2jtZq?=
 =?us-ascii?Q?MjDtI7gUPNsPUdvMcYYjY7YvOH4CETYj+6E+knC7EU2k/HffIf7NniDSI7aJ?=
 =?us-ascii?Q?MyZO22LaWeRdDBmrseponQlxHJgiVWldxQ66hpHYxQJo4bACt2N99PTv+muy?=
 =?us-ascii?Q?ZBm/wFOlCXQFZQN9XPR/6fF1BZnx3iWIwk4vrtZ9vuTV1gfxRhDwXgXIde3I?=
 =?us-ascii?Q?A6JGi1ONuny5wmYjGV9OUrf7xEw4VNuIOuFp2RLD+cIH36+r77B5M5zlXKOe?=
 =?us-ascii?Q?TCQ0TXz1TlcGhsUT8amGJBmUTEr9WD9nJmr06pEUpGxik8TxLGqzK51Yjeco?=
 =?us-ascii?Q?dHkS5pDAsWTgh6JjZ1Nsp7OqdgBtum+wPtdKzxV541jJm6VL+ZYY+nw2rMxJ?=
 =?us-ascii?Q?WKdjjoJX7raarYnu4wd40I54RM1ewUdJvPCSdzB3wLNHerOP5qDPT2iM1ejU?=
 =?us-ascii?Q?9KUX7UUYd7u6ia2TWmpcyk9n4s65Af2O0bgPPtBESayW3v7rwpywAp7QkQqU?=
 =?us-ascii?Q?Cbousm4kstkmRfxbPM1HUPG12SdtL00Yno5jpthKJvAjaq9HsO1mIM5RCmMA?=
 =?us-ascii?Q?3FKA3WcJ2u8AAa0mm7gJo+ZUPLSsDajqSfXGPJ/iTV5SmENHYQHVmNMxEhwx?=
 =?us-ascii?Q?/w8jWIDOf9ta36Luyud8WBeq1XuMS20EoaWL9rqFyw2vHrt+WC4pfTFEv/1a?=
 =?us-ascii?Q?o1fi1/w8WvxFRwnhy0RDE9L569+pMd3mXT9V6mKvaM7ssyCZKJDMeym6ukZN?=
 =?us-ascii?Q?dNQhRrYv0phVy/gGzwvHTuTS7b+0zGScWHXvImBMmGZ6QeKIByRhSFaVO07u?=
 =?us-ascii?Q?jg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2d98ce96-f42f-47a8-d391-08db26b586d9
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2023 07:02:09.8289
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xo9LLsR+7EUhQcjMFcFM74JT+L4odTkl7SUDVklY6bu/Q4P48tTyT4+b9VN4oRA0Zqp/lBtS9HPj+rmkXETT6g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5627
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>

On Fri, Mar 10, 2023 at 04:22:43PM -0800, Sean Christopherson wrote:
> Don't use the generic page-track mechanism to handle writes to guest PTEs
> in KVM's MMU.  KVM's MMU needs access to information that should not be
> exposed to external page-track users, e.g. KVM needs (for some definitions
> of "need") the vCPU to query the current paging mode, whereas external
> users, i.e. KVMGT, have no ties to the current vCPU and so should never
> need the vCPU.
> 
> Moving away from the page-track mechanism will allow dropping use of the
> page-track mechanism for KVM's own MMU, and will also allow simplifying
> and cleaning up the page-track APIs.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  1 -
>  arch/x86/kvm/mmu.h              |  2 ++
>  arch/x86/kvm/mmu/mmu.c          | 13 ++-----------
>  arch/x86/kvm/mmu/page_track.c   |  2 ++
>  4 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 17281d6825c9..1a4225237564 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1265,7 +1265,6 @@ struct kvm_arch {
>  	 * create an NX huge page (without hanging the guest).
>  	 */
>  	struct list_head possible_nx_huge_pages;
> -	struct kvm_page_track_notifier_node mmu_sp_tracker;
>  	struct kvm_page_track_notifier_head track_notifier_head;
>  	/*
>  	 * Protects marking pages unsync during page faults, as TDP MMU page
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 168c46fd8dd1..b8bde42f6037 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -119,6 +119,8 @@ void kvm_mmu_unload(struct kvm_vcpu *vcpu);
>  void kvm_mmu_free_obsolete_roots(struct kvm_vcpu *vcpu);
>  void kvm_mmu_sync_roots(struct kvm_vcpu *vcpu);
>  void kvm_mmu_sync_prev_roots(struct kvm_vcpu *vcpu);
> +void kvm_mmu_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> +			 int bytes);
>  
>  static inline int kvm_mmu_reload(struct kvm_vcpu *vcpu)
>  {
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 409dabec69df..4f2f83d8322e 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5603,9 +5603,8 @@ static u64 *get_written_sptes(struct kvm_mmu_page *sp, gpa_t gpa, int *nspte)
>  	return spte;
>  }
>  
> -static void kvm_mmu_pte_write(struct kvm_vcpu *vcpu, gpa_t gpa,
> -			      const u8 *new, int bytes,
> -			      struct kvm_page_track_notifier_node *node)
> +void kvm_mmu_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
> +			 int bytes)
>  {
>  	gfn_t gfn = gpa >> PAGE_SHIFT;
>  	struct kvm_mmu_page *sp;
> @@ -6088,7 +6087,6 @@ static bool kvm_has_zapped_obsolete_pages(struct kvm *kvm)
>  
>  int kvm_mmu_init_vm(struct kvm *kvm)
>  {
> -	struct kvm_page_track_notifier_node *node = &kvm->arch.mmu_sp_tracker;
>  	int r;
>  
>  	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
> @@ -6102,9 +6100,6 @@ int kvm_mmu_init_vm(struct kvm *kvm)
>  			return r;
>  	}
>  
> -	node->track_write = kvm_mmu_pte_write;
> -	kvm_page_track_register_notifier(kvm, node);
> -
>  	kvm->arch.split_page_header_cache.kmem_cache = mmu_page_header_cache;
>  	kvm->arch.split_page_header_cache.gfp_zero = __GFP_ZERO;
>  
> @@ -6125,10 +6120,6 @@ static void mmu_free_vm_memory_caches(struct kvm *kvm)
>  
>  void kvm_mmu_uninit_vm(struct kvm *kvm)
>  {
> -	struct kvm_page_track_notifier_node *node = &kvm->arch.mmu_sp_tracker;
> -
> -	kvm_page_track_unregister_notifier(kvm, node);
> -
>  	if (tdp_mmu_enabled)
>  		kvm_mmu_uninit_tdp_mmu(kvm);
>  
> diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c
> index e739dcc3375c..f39f190ad4ae 100644
> --- a/arch/x86/kvm/mmu/page_track.c
> +++ b/arch/x86/kvm/mmu/page_track.c
> @@ -274,6 +274,8 @@ void kvm_page_track_write(struct kvm_vcpu *vcpu, gpa_t gpa, const u8 *new,
>  		if (n->track_write)
>  			n->track_write(vcpu, gpa, new, bytes, n);
>  	srcu_read_unlock(&head->track_srcu, idx);
> +
> +	kvm_mmu_track_write(vcpu, gpa, new, bytes);
>  }
>  
>  /*
> -- 
> 2.40.0.rc1.284.g88254d51c5-goog
>