Re: [PATCH v3 09/49] kvm: Enable KVM_SET_USER_MEMORY_REGION2 for memslot

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 3/20/24 09:39, Michael Roth wrote:
> +    if (cap_user_memory2 == -1) {
> +        cap_user_memory2 = kvm_check_extension(s, KVM_CAP_USER_MEMORY2);
> +    }
> +
> +    if (!cap_user_memory2 && slot->guest_memfd >= 0) {
> +        error_report("%s, KVM doesn't support KVM_CAP_USER_MEMORY2,"
> +                     " which is required by guest memfd!", __func__);
> +        exit(1);
> +    }

It's easier and more robust (for error reporting purposes) to check both
KVM_CAP_GUEST_MEMFD and KVM_CAP_USER_MEMORY2 at once in the earlier
patches:

-    kvm_guest_memfd_supported = kvm_check_extension(s, KVM_CAP_GUEST_MEMFD);
+    kvm_guest_memfd_supported =
+        kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
+        kvm_check_extension(s, KVM_CAP_USER_MEMORY2);

since KVM cannot really support guest_memfd if it cannot then use it
to create private memory slots.

And then, this one can be changed to also use kvm_guest_memfd_supported:

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index e83429b31eb..afcf6f87045 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -284,19 +284,8 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
 {
     KVMState *s = kvm_state;
     struct kvm_userspace_memory_region2 mem;
-    static int cap_user_memory2 = -1;
     int ret;

-    if (cap_user_memory2 == -1) {
-        cap_user_memory2 = kvm_check_extension(s, KVM_CAP_USER_MEMORY2);
-    }
-
-    if (!cap_user_memory2 && slot->guest_memfd >= 0) {
-        error_report("%s, KVM doesn't support KVM_CAP_USER_MEMORY2,"
-                     " which is required by guest memfd!", __func__);
-        exit(1);
-    }
-
     mem.slot = slot->slot | (kml->as_id << 16);
     mem.guest_phys_addr = slot->start_addr;
     mem.userspace_addr = (unsigned long)slot->ram;
@@ -309,7 +298,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
          * value. This is needed based on KVM commit 75d61fbc. */
         mem.memory_size = 0;

-        if (cap_user_memory2) {
+        if (kvm_guest_memfd_supported) {
             ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, &mem);
         } else {
             ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
@@ -319,7 +308,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
         }
     }
     mem.memory_size = slot->memory_size;
-    if (cap_user_memory2) {
+    if (kvm_guest_memfd_supported) {
         ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION2, &mem);
     } else {
         ret = kvm_vm_ioctl(s, KVM_SET_USER_MEMORY_REGION, &mem);
@@ -331,7 +320,7 @@ err:
                               mem.userspace_addr, mem.guest_memfd,
                               mem.guest_memfd_offset, ret);
     if (ret < 0) {
-        if (cap_user_memory2) {
+        if (kvm_guest_memfd_supported) {
                 error_report("%s: KVM_SET_USER_MEMORY_REGION2 failed, slot=%d,"
                         " start=0x%" PRIx64 ", size=0x%" PRIx64 ","
                         " flags=0x%" PRIx32 ", guest_memfd=%" PRId32 ","
@@ -501,6 +490,7 @@ static int kvm_mem_flags(MemoryRegion *mr)
         flags |= KVM_MEM_READONLY;
     }
     if (memory_region_has_guest_memfd(mr)) {
+        assert(kvm_guest_memfd_supported);
         flags |= KVM_MEM_GUEST_MEMFD;
     }
     return flags;