Sean Christopherson seanjc@google.com writes:
On Thu, Oct 23, 2025, Ackerley Tng wrote:
Sean Christopherson seanjc@google.com writes:
On Wed, Oct 22, 2025, Ackerley Tng wrote:
Ackerley Tng ackerleytng@google.com writes:
Found another issue with KVM_CAP_MEMORY_ATTRIBUTES2.
KVM_CAP_MEMORY_ATTRIBUTES2 was defined to do the same thing as KVM_CAP_MEMORY_ATTRIBUTES, but that's wrong since KVM_CAP_MEMORY_ATTRIBUTES2 should indicate the presence of KVM_SET_MEMORY_ATTRIBUTES2 and struct kvm_memory_attributes2.
No? If no attributes are supported, whether or not KVM_SET_MEMORY_ATTRIBUTES2 exists is largely irrelevant.
That's true.
We can even provide the same -ENOTTY errno by checking that _any_ attributes are supported, i.e. so that doing KVM_SET_MEMORY_ATTRIBUTES2 on KVM without any support whatsoever fails in the same way that KVM with code support but no attributes fails.
IIUC KVM_SET_MEMORY_ATTRIBUTES doesn't fail with -ENOTTY now when there are no valid attributes.
Even if there's no valid attributes (as in kvm_supported_mem_attributes() returns 0), it's possible to call KVM_SET_MEMORY_ATTRIBUTES with .attributes set to 0, which will be a no-op, but will return 0.
I think this is kind of correct behavior since .attributes = 0 is actually a valid expression for "I want this range to be shared", and for a VM that doesn't support private memory, it's a valid expression.
The other way that there are "no attributes" would be if there are no /VM/ attributes, in which case KVM_SET_MEMORY_ATTRIBUTES, sent to as a vm ioctl, will return -ENOTTY.
Ya, this is what I was trying to say with "_any_ attributes are supported". I.e. by "any" I meant "any attributes in KVM for VMs vs. gmems", not "any attributes for this specific VM/gmem instance".
[...snip...]
I've been thinking more about this:
#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES case KVM_CAP_MEMORY_ATTRIBUTES2: case KVM_CAP_MEMORY_ATTRIBUTES: if (!vm_memory_attributes) return 0;
return kvm_supported_mem_attributes(kvm); #endif
And the purpose of adding KVM_CAP_MEMORY_ATTRIBUTES2 is that KVM_CAP_MEMORY_ATTRIBUTES2 tells userspace that KVM_SET_MEMORY_ATTRIBUTES2 is available iff there are valid attributes.
(So there's still a purpose)
Without valid attributes, userspace can't tell if it should use KVM_SET_MEMORY_ATTRIBUTES or the 2 version.
I also added KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES, which tells userspace the valid attributes when calling KVM_SET_MEMORY_ATTRIBUTES2 on a guest_memfd:
#ifdef CONFIG_KVM_GUEST_MEMFD case KVM_CAP_GUEST_MEMFD: return 1; case KVM_CAP_GUEST_MEMFD_FLAGS: return kvm_gmem_get_supported_flags(kvm); case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES: if (vm_memory_attributes) return 0;
return kvm_supported_mem_attributes(kvm); #endif
So to set memory attributes, userspace should
if (kvm_check_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) > 0) use KVM_SET_MEMORY_ATTRIBUTES2 with guest_memfd else if (kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES2) > 0) use KVM_SET_MEMORY_ATTRIBUTES2 with VM fd else if (kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) > 0) use KVM_SET_MEMORY_ATTRIBUTES with VM fd else can't set memory attributes
Something like that?
In selftests there's this, when KVM_SET_USER_MEMORY_REGION2 was introduced:
#define TEST_REQUIRE_SET_USER_MEMORY_REGION2() \ __TEST_REQUIRE(kvm_has_cap(KVM_CAP_USER_MEMORY2), \ "KVM selftests now require KVM_SET_USER_MEMORY_REGION2 (introduced in v6.8)")
But looks like there's no direct equivalent for the introduction of KVM_SET_MEMORY_ATTRIBUTES2?
The closest would be to add a TEST_REQUIRE_VALID_ATTRIBUTES() which checks KVM_CAP_MEMORY_ATTRIBUTES2 or KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES before making the vm or guest_memfd ioctl respsectively.