Re: [PATCH v8 08/13] KVM: guest_memfd: Allow host to map guest_memfd() pages

[Ira Weiny <ira.weiny@xxxxxxxxx>]

Fuad Tabba wrote:
> Add support for mmap() and fault() for guest_memfd backed memory
> in the host for VMs that support in-place conversion between
> shared and private. To that end, this patch adds the ability to
> check whether the VM type supports in-place conversion, and only
> allows mapping its memory if that's the case.
> 
> This patch introduces the configuration option KVM_GMEM_SHARED_MEM,
> which enables support for in-place shared memory.
> 
> It also introduces the KVM capability KVM_CAP_GMEM_SHARED_MEM, which
> indicates that the host can create VMs that support shared memory.
> Supporting shared memory implies that memory can be mapped when shared
> with the host.
> 
> Signed-off-by: Fuad Tabba <tabba@xxxxxxxxxx>
> ---
>  include/linux/kvm_host.h | 15 ++++++-
>  include/uapi/linux/kvm.h |  1 +
>  virt/kvm/Kconfig         |  5 +++
>  virt/kvm/guest_memfd.c   | 92 ++++++++++++++++++++++++++++++++++++++++
>  virt/kvm/kvm_main.c      |  4 ++
>  5 files changed, 116 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 9419fb99f7c2..f3af6bff3232 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -729,6 +729,17 @@ static inline bool kvm_arch_supports_gmem(struct kvm *kvm)
>  }
>  #endif
>  
> +/*
> + * Arch code must define kvm_arch_gmem_supports_shared_mem if support for
> + * private memory is enabled and it supports in-place shared/private conversion.
> + */
> +#if !defined(kvm_arch_gmem_supports_shared_mem) && !IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)

Perhaps the bots already caught this?

I just tried enabling KVM_GMEM_SHARED_MEM on x86 with this patch and it fails with:

|| In file included from arch/x86/kvm/../../../virt/kvm/binary_stats.c:8:
|| ./include/linux/kvm_host.h: In function ‘kvm_mem_from_gmem’:
include/linux/kvm_host.h|2530 col 13| error: implicit declaration of function ‘kvm_arch_gmem_supports_shared_mem’ [-Wimplicit-function-declaration]
||  2530 |         if (kvm_arch_gmem_supports_shared_mem(kvm))
||       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|| make[4]: *** Waiting for unfinished jobs....


I think the predicate on !CONFIG_KVM_GMEM_SHARED_MEM is wrong.

Shouldn't this always default off?  I __think__ this then gets enabled in
11/13?

IOW

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f3af6bff3232..577674e95c09 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -733,7 +733,7 @@ static inline bool kvm_arch_supports_gmem(struct kvm *kvm)
  * Arch code must define kvm_arch_gmem_supports_shared_mem if support for
  * private memory is enabled and it supports in-place shared/private conversion.
  */
-#if !defined(kvm_arch_gmem_supports_shared_mem) && !IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)
+#if !defined(kvm_arch_gmem_supports_shared_mem)
 static inline bool kvm_arch_gmem_supports_shared_mem(struct kvm *kvm)
 {
        return false;