[PATCH 0/3] kvm: Guet_memfd enhancement and fix for KVM_SET_USER_MEMORY_REGION2

[PATCH 0/3] kvm: Guet_memfd enhancement and fix for KVM_SET_USER_MEMORY_REGION2

[Xiaoyao Li]

Patch 1 changes to check guest memfd CAP on VM instance.

Patch 2 is a fix for KVM_SET_USER_MEMORY_REGION2.

Patch 3 is an enhancement for guest_memfd_offset.

Xiaoyao Li (3):
  accel/kvm: Switch to check KVM_CAP_GUEST_MEMFD and
    KVM_CAP_USER_MEMORY2 on VM
  accel/kvm: Zero out mem explicitly in kvm_set_user_memory_region()
  accel/kvm: Set mem.guest_memfd_offset only when guest_memfd is valid

 accel/kvm/kvm-all.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH 1/3] accel/kvm: Switch to check KVM_CAP_GUEST_MEMFD and KVM_CAP_USER_MEMORY2 on VM

[Xiaoyao Li]

It returns more accruate result on checking KVM_CAP_GUEST_MEMFD and
KVM_CAP_USER_MEMORY2 on VM instance instead of on KVM platform.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 accel/kvm/kvm-all.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 890d5ea9f865..14d47246ca63 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2776,8 +2776,8 @@ static int kvm_init(AccelState *as, MachineState *ms)
 
     kvm_supported_memory_attributes = kvm_vm_check_extension(s, KVM_CAP_MEMORY_ATTRIBUTES);
     kvm_guest_memfd_supported =
-        kvm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
-        kvm_check_extension(s, KVM_CAP_USER_MEMORY2) &&
+        kvm_vm_check_extension(s, KVM_CAP_GUEST_MEMFD) &&
+        kvm_vm_check_extension(s, KVM_CAP_USER_MEMORY2) &&
         (kvm_supported_memory_attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE);
     kvm_pre_fault_memory_supported = kvm_vm_check_extension(s, KVM_CAP_PRE_FAULT_MEMORY);
 
-- 
2.43.0

[PATCH 2/3] accel/kvm: Zero out mem explicitly in kvm_set_user_memory_region()

[Xiaoyao Li]

Zero out the entire mem explicitly before it's used, to ensure the unused
feilds (pad1, pad2) are all zeros. Otherwise, it might cause problem when
the pad fields are extended by future KVM.

Fixes: ce5a983233b4 ("kvm: Enable KVM_SET_USER_MEMORY_REGION2 for memslot")
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 accel/kvm/kvm-all.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 14d47246ca63..4f4c30fc84b2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -361,6 +361,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
     struct kvm_userspace_memory_region2 mem;
     int ret;
 
+    memset(&mem, 0, sizeof(mem));
     mem.slot = slot->slot | (kml->as_id << 16);
     mem.guest_phys_addr = slot->start_addr;
     mem.userspace_addr = (unsigned long)slot->ram;
-- 
2.43.0

Re: [PATCH 2/3] accel/kvm: Zero out mem explicitly in kvm_set_user_memory_region()

[Philippe Mathieu-Daudé]

On 23/7/25 09:09, Xiaoyao Li wrote:
> Zero out the entire mem explicitly before it's used, to ensure the unused
> feilds (pad1, pad2) are all zeros. Otherwise, it might cause problem when
> the pad fields are extended by future KVM.
> 
> Fixes: ce5a983233b4 ("kvm: Enable KVM_SET_USER_MEMORY_REGION2 for memslot")
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   accel/kvm/kvm-all.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 14d47246ca63..4f4c30fc84b2 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -361,6 +361,7 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
>       struct kvm_userspace_memory_region2 mem;

Or:

         struct kvm_userspace_memory_region2 mem = { };

Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>       int ret;
>   
> +    memset(&mem, 0, sizeof(mem));
>       mem.slot = slot->slot | (kml->as_id << 16);
>       mem.guest_phys_addr = slot->start_addr;
>       mem.userspace_addr = (unsigned long)slot->ram;

[PATCH 3/3] accel/kvm: Set mem.guest_memfd_offset only when guest_memfd is valid

[Xiaoyao Li]

Current QEMU unconditionally sets the mmem.guest_memfd_offset in
kvm_set_user_memory_region(), which leads to the trace of kvm_set_user_memory
look as:

kvm_set_user_memory AddrSpace#0 Slot#4 flags=0x2 gpa=0xe0000 size=0x20000 ua=0x7f5840de0000 guest_memfd=-1 guest_memfd_offset=0x3e0000 ret=0

It's confusing that the guest_memfd_offset has a non-zero value while
the guest_memfd is invalid (-1).

Change to only set guest_memfd_offset when guest_memfd is valid.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 accel/kvm/kvm-all.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 4f4c30fc84b2..2b6fbf58a127 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -367,7 +367,9 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
     mem.userspace_addr = (unsigned long)slot->ram;
     mem.flags = slot->flags;
     mem.guest_memfd = slot->guest_memfd;
-    mem.guest_memfd_offset = slot->guest_memfd_offset;
+    if (slot->guest_memfd >= 0) {
+        mem.guest_memfd_offset = slot->guest_memfd_offset;
+    }
 
     if (slot->memory_size && !new && (mem.flags ^ slot->old_flags) & KVM_MEM_READONLY) {
         /* Set the slot size to 0 before setting the slot to the desired
-- 
2.43.0

Re: [PATCH 3/3] accel/kvm: Set mem.guest_memfd_offset only when guest_memfd is valid

[Philippe Mathieu-Daudé]

On 23/7/25 09:09, Xiaoyao Li wrote:
> Current QEMU unconditionally sets the mmem.guest_memfd_offset in
> kvm_set_user_memory_region(), which leads to the trace of kvm_set_user_memory
> look as:
> 
> kvm_set_user_memory AddrSpace#0 Slot#4 flags=0x2 gpa=0xe0000 size=0x20000 ua=0x7f5840de0000 guest_memfd=-1 guest_memfd_offset=0x3e0000 ret=0
> 
> It's confusing that the guest_memfd_offset has a non-zero value while
> the guest_memfd is invalid (-1).
> 
> Change to only set guest_memfd_offset when guest_memfd is valid.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>   accel/kvm/kvm-all.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 4f4c30fc84b2..2b6fbf58a127 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -367,7 +367,9 @@ static int kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
>       mem.userspace_addr = (unsigned long)slot->ram;
>       mem.flags = slot->flags;
>       mem.guest_memfd = slot->guest_memfd;
> -    mem.guest_memfd_offset = slot->guest_memfd_offset;
> +    if (slot->guest_memfd >= 0) {
> +        mem.guest_memfd_offset = slot->guest_memfd_offset;
> +    }

IMHO the issue is in when the KVMSlot is filled in kvm_set_phys_mem():

-- >8 --
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 890d5ea9f86..631f14b996a 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1595,7 +1595,8 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
          mem->ram = ram;
          mem->flags = kvm_mem_flags(mr);
          mem->guest_memfd = mr->ram_block->guest_memfd;
-        mem->guest_memfd_offset = (uint8_t*)ram - mr->ram_block->host;
+        mem->guest_memfd_offset = mr->ram_block->guest_memfd
+                                ? (uint8_t*)ram - mr->ram_block->host : 0;

          kvm_slot_init_dirty_bitmap(mem);
          err = kvm_set_user_memory_region(kml, mem, true);
---

Re: [PATCH 3/3] accel/kvm: Set mem.guest_memfd_offset only when guest_memfd is valid

[Xiaoyao Li]

On 7/23/2025 8:06 PM, Philippe Mathieu-Daudé wrote:
> On 23/7/25 09:09, Xiaoyao Li wrote:
>> Current QEMU unconditionally sets the mmem.guest_memfd_offset in
>> kvm_set_user_memory_region(), which leads to the trace of 
>> kvm_set_user_memory
>> look as:
>>
>> kvm_set_user_memory AddrSpace#0 Slot#4 flags=0x2 gpa=0xe0000 
>> size=0x20000 ua=0x7f5840de0000 guest_memfd=-1 
>> guest_memfd_offset=0x3e0000 ret=0
>>
>> It's confusing that the guest_memfd_offset has a non-zero value while
>> the guest_memfd is invalid (-1).
>>
>> Change to only set guest_memfd_offset when guest_memfd is valid.
>>
>> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
>> ---
>>   accel/kvm/kvm-all.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
>> index 4f4c30fc84b2..2b6fbf58a127 100644
>> --- a/accel/kvm/kvm-all.c
>> +++ b/accel/kvm/kvm-all.c
>> @@ -367,7 +367,9 @@ static int 
>> kvm_set_user_memory_region(KVMMemoryListener *kml, KVMSlot *slot, boo
>>       mem.userspace_addr = (unsigned long)slot->ram;
>>       mem.flags = slot->flags;
>>       mem.guest_memfd = slot->guest_memfd;
>> -    mem.guest_memfd_offset = slot->guest_memfd_offset;
>> +    if (slot->guest_memfd >= 0) {
>> +        mem.guest_memfd_offset = slot->guest_memfd_offset;
>> +    }
> 
> IMHO the issue is in when the KVMSlot is filled in kvm_set_phys_mem():

Agreed, fix it in the origin。

> -- >8 --
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 890d5ea9f86..631f14b996a 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -1595,7 +1595,8 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
>           mem->ram = ram;
>           mem->flags = kvm_mem_flags(mr);
>           mem->guest_memfd = mr->ram_block->guest_memfd;
> -        mem->guest_memfd_offset = (uint8_t*)ram - mr->ram_block->host;
> +        mem->guest_memfd_offset = mr->ram_block->guest_memfd
> +                                ? (uint8_t*)ram - mr->ram_block->host : 0;

it should be

	mr->ram_block->guest_memfd >= 0 ? (uint8_t*)ram - mr->ram_block->host : 0;

I will use the fixed version in v2.

Thanks!

>           kvm_slot_init_dirty_bitmap(mem);
>           err = kvm_set_user_memory_region(kml, mem, true);
> ---