Xiaoyao Li <[email protected]> writes:

> On 7/29/2026 8:35 AM, Ackerley Tng via B4 Relay wrote:
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index 33c9830190e2e..89cf922232920 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -893,6 +893,27 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct 
>> kvm_memory_slot *slot,
>>   EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
>>
>>   #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
>> +static bool kvm_range_is_private(struct file *file, pgoff_t index,
>> +                             size_t nr_pages, struct kvm *kvm, gfn_t gfn)
>> +{
>> +    struct inode *inode = file_inode(file);
>> +    pgoff_t last = index + nr_pages - 1;
>> +    struct maple_tree *mt;
>> +    void *entry;
>> +
>> +    if (!gmem_in_place_conversion)
>> +            return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + 
>> nr_pages,
>> +                                                      
>> KVM_MEMORY_ATTRIBUTE_PRIVATE,
>> +                                                      
>> KVM_MEMORY_ATTRIBUTE_PRIVATE);
>> +
>> +    mt = &GMEM_I(inode)->attributes;
>> +    mt_for_each(mt, entry, index, last) {
>> +            if (kvm_gmem_interpret_entry(inode, entry) !=
>> +                KVM_MEMORY_ATTRIBUTE_PRIVATE)
>> +                    return false;
>> +    }
>> +    return true;
>> +}
>>
>>   static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot 
>> *slot,
>>                              struct file *file, gfn_t gfn, struct page 
>> *src_page,
>> @@ -913,9 +934,7 @@ static long __kvm_gmem_populate(struct kvm *kvm, struct 
>> kvm_memory_slot *slot,
>>
>>      folio_unlock(folio);
>>
>> -    if (!kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + 1,
>> -                                            KVM_MEMORY_ATTRIBUTE_PRIVATE,
>> -                                            KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
>> +    if (!kvm_range_is_private(file, index, 1, kvm, gfn)) {
>
> It's checking if a single gfn is private.
>

But with huge page support we'd need to check a range of gfns... I guess
there's the argument of not keeping complexity for the future, but in
this case it's undoing functionality for this series and probably adding
it back later.

> We can just use kvm_mem_is_private()? And it seems can be a separate patch.
>

Do you mean that this refactoring could be a separate patch? I could
refactor out kvm_range_is_private() in a separate patch, then put in the
mt_for_each() check in this patch together as part of all the rest of
the "wiring".

I considered that but it seemed like the refactoring was too small to
separate out into another patch when the mt_for_each() part is going to
be in this patch anyway.

Or, I could add an earlier patch to first replace the call to
kvm_range_has_vm_memory_attributes() with a call to check just 1 gfn,
then this wiring patch would be simpler.

Let me know what y'all think is best. Thanks!

>>              ret = -EINVAL;
>>              goto out_put_folio;
>>      }

Reply via email to