Re: [PATCH 3/3] KVM: always make in-place conversion a compile-time constant when possible

[Ackerley Tng]

Paolo Bonzini <pbonzini@xxxxxxxxxx> writes:

> Remove the unnecessary static call indirection and make
> gmem_in_place_conversion a constant if CONFIG_KVM_PRIVATE_MEMATTR_VM
> is false.
>
> Require that CONFIG_KVM_PRIVATE_MEMATTR_VM is y only if
> kvm_arch_has_private_mem is also defined.  This way, the three
> possible cases can be expressed without nested #ifs.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  include/linux/kvm_host.h | 13 +++++--------
>  virt/kvm/kvm_main.c      | 13 +++++++------
>  2 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 09c57d31d153..e7fc94e2910a 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2585,15 +2585,7 @@ bool kvm_arch_post_set_vm_memory_attributes(struct kvm *kvm,
>  #endif  /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
>
>  #ifdef CONFIG_KVM_PRIVATE_MEMATTR_VM
> -static inline bool kvm_vm_is_private_gfn(struct kvm *kvm, gfn_t gfn)
> -{
> -	return kvm_get_vm_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> -}
> -#endif  /* CONFIG_KVM_PRIVATE_MEMATTR_VM */
> -
> -#ifdef kvm_arch_has_private_mem
>  extern bool gmem_in_place_conversion;
> -
>  typedef bool (kvm_is_private_gfn_t)(struct kvm *kvm, gfn_t gfn);
>  DECLARE_STATIC_CALL(__kvm_is_private_gfn, kvm_is_private_gfn_t);
>
> @@ -2601,6 +2593,11 @@ static inline bool kvm_is_private_gfn(struct kvm *kvm, gfn_t gfn)
>  {
>  	return static_call(__kvm_is_private_gfn)(kvm, gfn);
>  }
> +
> +#elif defined kvm_arch_has_private_mem
> +#define gmem_in_place_conversion true
> +#define kvm_is_private_gfn kvm_gmem_is_private_gfn
> +
>  #else
>  #define gmem_in_place_conversion false
>

There's a #endif just below here where the comment says

  #endif /* kvm_arch_has_private_mem */

Not sure if the convention is to align with the first #if
(CONFIG_KVM_PRIVATE_MEMATTR_VM) or the #elif (kvm_arch_has_private_mem)
or something in-between.

Reviewed-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>

>
> [...snip...]
>