Re: [PATCH v8 06/46] KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined

[Sean Christopherson <seanjc@google.com>]

On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > From: Ackerley Tng <ackerleytng@google.com>
> > 
> > Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based
> > on kvm_arch_has_private_mem being #defined in anticipation of decoupling
> > kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
> 
> Well, after this series, kvm_supported_mem_attributes() is renamed to
> kvm_supported_vm_mem_attributes(), and it's still under
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
> 
> > guest_memfd support for memory attributes will be unconditional to avoid
> > yet more macros (all architectures that support guest_memfd are expected to
> > use per-gmem attributes at some point), at which point enumerating support
> > KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being
> > supported _somewhere_ would result in KVM over-reporting support on arm64.
> 
> I don't understand it. This patch only changes the behavior of
> kvm_supported_mem_attributes(), the usage of which is guarded by
> CONFIG_KVM_VM_MEMORY_ATTRIBUTESq. This is config is only visible to x86 due
> to patch 03. How does it affect arm64?

Hrm, yeah, this is messed up.  Ahh, I think Ackerley shuffled things around and
"broke" stuff in the process.  In v7[1] and earlier, the diff was this:

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 091f201251159..68142bc962953 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -722,7 +722,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
 }
 #endif
 
-#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifndef kvm_arch_has_private_mem
 static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
 {
 	return false;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 306153abbafa5..abb9cfa3eb04d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2421,8 +2421,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
 #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
 static u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
+#ifdef kvm_arch_has_private_mem
 	if (!kvm || kvm_arch_has_private_mem(kvm))
 		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+#endif
 
 	return 0;
 }

which makes a *lot* more sense given the changelog (and IMO for the ordering in
general).  In v8 here, Ackerley combined part of a change[2] (that I provided
off-list) with part of this commit, to create patch 4, "KVM: Decouple
kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTESthe".

Ackerley, the cover letter says:

  + Reshuffled the earlier commits that deal with preparing KVM to stop
    seeing VM memory attributes as the only source of attributes.

but there's no explanation for *why* the reshuffling was done.  Reorganizing
code like this at v8 of a series this size is a big "no-no" unless there's a
*really* good reason to do so.  In addition to the resulting confusion, changes
like this invalidate Fuad's Reviewed-by.  And since it's obviously quite difficult
to tease out exactly what changed, it's not realistic to re-review things without
doing a deep audit of the series, which no one wants to do for a series that is/was
so close to being fully ready.  And without such an audit, I can't accept the
patches, because I can't trust that what I am accepting is what I and others have
reviewed.

So, except where there is/was a *need* to shuffle things around relative to v7,
I think we should revert back to the v7 ordering for v9.  And where there is a
need to rework things, each and every one of those needs to be explicitly
documented, because "Reshuffled the earlier commits" is grossly insufficient.

[1] https://lore.kernel.org/all/20260522-gmem-inplace-conversion-v7-3-2f0fae496530@google.com
[2] https://github.com/sean-jc/linux/commit/8a475b1bcf89f1cf776ed9ce7d6bb587aab0d421