kvm: Guet_memfd enhancement and fix for KVM_SET_USER_MEMORY_REGION2

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

Posted by Xiaoyao Li 3 months, 3 weeks ago

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

Posted by Philippe Mathieu-Daudé 3 months, 3 weeks ago

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

Posted by Xiaoyao Li 3 months, 3 weeks ago

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);
> ---