Re: [PATCH v15 01/21] KVM: Rename CONFIG_KVM_PRIVATE_MEM to CONFIG_KVM_GMEM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 17, 2025, Fuad Tabba wrote:
> Rename the Kconfig option CONFIG_KVM_PRIVATE_MEM to CONFIG_KVM_GMEM. 

Please name this CONFIG_KVM_GUEST_MEMFD.  I'm a-ok using gmem as the namespace
for functions/macros/variables, but there's zero reason to shorten things like
Kconfigs.

> @@ -719,10 +719,10 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
>  #endif
>  
>  /*
> - * Arch code must define kvm_arch_has_private_mem if support for private memory
> - * is enabled.
> + * Arch code must define kvm_arch_has_private_mem if support for guest_memfd is
> + * enabled.

This is undesirable, and the comment is flat out wrong.  As evidenced by the lack
of a #define in arm64, arch does NOT need to #define kvm_arch_has_private_mem if
CONFIG_KVM_GUEST_MEMFD=y.  It "works" because the sole caller to kvm_arch_has_private_mem()
is guarded by CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES=y, and that's never selected
by arm64.

I.e. this needs to key off of CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES=y, not off of
CONFIG_KVM_GUEST_MEMFD=y.  And I would just drop the comment altogether at that
point, because it's all quite self-explanatory:

#ifndef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
{
	return false;
}
#endif


>   */
> -#if !defined(kvm_arch_has_private_mem) && !IS_ENABLED(CONFIG_KVM_PRIVATE_MEM)
> +#if !defined(kvm_arch_has_private_mem) && !IS_ENABLED(CONFIG_KVM_GMEM)
>  static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
>  {
>  	return false;
> @@ -2527,7 +2527,7 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
>  
>  static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
>  {
> -	return IS_ENABLED(CONFIG_KVM_PRIVATE_MEM) &&
> +	return IS_ENABLED(CONFIG_KVM_GMEM) &&

And this is equally wrong.  The existing code checked CONFIG_KVM_PRIVATE_MEM,
because memory obviously can't be private if private memory is unsupported.

But that logic chain doesn't work as well for guest_memfd.  In a way, this is a
weird semantic change, e.g. it changes from "select guest_memfd if private memory
is supported" to "allow private memory if guest_memfd is select".   The former
existed because compiling in support for guest_memfd when it coulnd't possibly
be used was wasteful, but even then it was somewhat superfluous.

The latter is an arbitrary requirement that probably shouldn't exist, and if we
did want to make it a hard requirement, should be expressed in the Kconfig
dependency, not here.

TL;DR: drop the IS_ENABLED(CONFIG_KVM_GMEM) check.




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux