Xiaoyao Li <[email protected]> writes: > On 7/29/2026 8:35 AM, Ackerley Tng via B4 Relay wrote: >> From: Sean Christopherson <[email protected]> > > Though the patch order and diff of this patch is adjusted to what looks > in v7, per Sean's request [1], the changelog still looks somewhat > confusing and part incorrect to me. > > [1] https://lore.kernel.org/all/[email protected]/ > >> 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. > > As I commented in v8, the "in anticipation of" thing is not correct. >
What I think was intended here is that in patch 20 of this series, "KVM: Let userspace disable per-VM mem attributes, enable per-gmem attributes", this part -#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +#if defined(CONFIG_KVM_SW_PROTECTED_VM) || \ + defined(CONFIG_KVM_INTEL_TDX) || \ + defined(CONFIG_KVM_AMD_SEV) #define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem) decouples CONFIG_KVM_VM_MEMORY_ATTRIBUTES from kvm_arch_has_private_mem(). I get it now, so CONFIG_KVM_VM_MEMORY_ATTRIBUTES is not actually being decoupled from kvm_supported_mem_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. > > 1. I'm not sure what "memory attributes being supported somewhere" means. > > 2. the kvm_supported_mem_attributes() will be renamed to > kvm_supported_vm_mem_attributes() and it's still under the guard of > CONFIG_KVM_VM_MEMORY_ATTRIBUTES, what's relationship with "guest_memfd > support for memory attributes"? > I think another way to phrase this is that if we don't make this change, say, on the foo architecture where there's no CoCo and no private mem support, kvm_supported_mem_attributes() would return true for the !kvm case, which is over-reporting. How about this, replacing the entire changelog paragraph above: Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based on kvm_arch_has_private_mem being #defined. This improves reporting accuracy by not reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE when kvm_supported_mem_attributes() is called with kvm == NULL. Give architectures full control over overriding the default definition of kvm_arch_has_private_mem() by removing the coupling with CONFIG_KVM_VM_MEMORY_ATTRIBUTES. In a later patch, kvm_arch_has_private_mem() will be defined based on whether architectural features are compiled in, and made orthogonal to CONFIG_KVM_VM_MEMORY_ATTRIBUTES. >> Signed-off-by: Sean Christopherson <[email protected]> >> Reviewed-by: Fuad Tabba <[email protected]> >> Reviewed-by: Binbin Wu <[email protected]> >> Tested-by: Shivank Garg <[email protected]> >> Signed-off-by: Ackerley Tng <[email protected]> >> --- >> include/linux/kvm_host.h | 2 +- >> virt/kvm/kvm_main.c | 2 ++ >> 2 files changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 9f78a466c6f3e..c125d2e8155a7 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 902aa166c9c0c..9501dd8d015d1 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; >> } >>
