Re: [PATCH gmem 2/6] KVM: guest_memfd: Only call kvm_arch_gmem_prepare hook if necessary

[Binbin Wu <binbin.wu@linux.intel.com>]

On 3/30/2024 5:24 AM, Michael Roth wrote:
> It has been reported that the internal workings of
> kvm_gmem_prepare_folio() incurs noticeable overhead for large guests
> even for platforms where kvm_arch_gmem_prepare() is a no-op.
>
> Provide a new kvm_arch_gmem_prepare_needed() hook so that architectures
> that set CONFIG_HAVE_KVM_GMEM_PREPARE can still opt-out of issuing the
> kvm_arch_gmem_prepare() callback

Just wondering which part has big impact on performance,
the issue of kvm_arch_gmem_prepare() callback or the preparation code for
the kvm_arch_gmem_prepare()?


> if the particular KVM instance doesn't
> require any sort of special preparation of its gmem pages prior to use.
>
> Link: https://lore.kernel.org/lkml/20240228202906.GB10568@ls.amr.corp.intel.com/
> Suggested-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>   include/linux/kvm_host.h |  1 +
>   virt/kvm/guest_memfd.c   | 10 ++++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 2f5074eff958..5b8308b5e4af 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2466,6 +2466,7 @@ static inline int kvm_gmem_undo_get_pfn(struct kvm *kvm,
>   
>   #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
>   int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_order);
> +bool kvm_arch_gmem_prepare_needed(struct kvm *kvm);
>   #endif
>   
>   #ifdef CONFIG_HAVE_KVM_GMEM_INVALIDATE
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 74e19170af8a..4ce0056d1149 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -13,6 +13,13 @@ struct kvm_gmem {
>   	struct list_head entry;
>   };
>   
> +#ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
> +bool __weak kvm_arch_gmem_prepare_needed(struct kvm *kvm)
> +{
> +	return false;
> +}
> +#endif
> +
>   static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct folio *folio)
>   {
>   #ifdef CONFIG_HAVE_KVM_GMEM_PREPARE
> @@ -27,6 +34,9 @@ static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct fol
>   		gfn_t gfn;
>   		int rc;
>   
> +		if (!kvm_arch_gmem_prepare_needed(kvm))
> +			continue;

Can multiple gmems (if any) bound to the same inode's address space 
belong to different kvm instances?
If not, just return here?

> +
>   		slot = xa_load(&gmem->bindings, index);
>   		if (!slot)
>   			continue;