Re: [PATCH v12 00/18] KVM: Mapping guest_memfd backed memory at the host for software protected VMs

[David Hildenbrand <david@redhat.com>]

On 24.06.25 12:25, Fuad Tabba wrote:
> Hi David,
> 
> On Tue, 24 Jun 2025 at 11:16, David Hildenbrand <david@redhat.com> wrote:
>>
>> On 24.06.25 12:02, Fuad Tabba wrote:
>>> Hi,
>>>
>>> Before I respin this, I thought I'd outline the planned changes for
>>> V13, especially since it involves a lot of repainting. I hope that
>>> by presenting this first, we could reduce the number of times I'll
>>> need to respin it.
>>>
>>> In struct kvm_arch: add bool supports_gmem instead of renaming
>>> has_private_mem
>>>
>>> The guest_memfd flag GUEST_MEMFD_FLAG_SUPPORT_SHARED should be
>>> called GUEST_MEMFD_FLAG_MMAP
>>>
>>> The memslot internal flag KVM_MEMSLOT_SUPPORTS_GMEM_SHARED should be
>>> called KVM_MEMSLOT_SUPPORTS_GMEM_MMAP
>>>
>>> kvm_arch_supports_gmem_shared_mem() should be called
>>> kvm_arch_supports_gmem_mmap()
>>>
>>> kvm_gmem_memslot_supports_shared() should be called
>>> kvm_gmem_memslot_supports_mmap()
>>>
>>> kvm_gmem_fault_shared(struct vm_fault *vmf) should be called
>>> kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
>>>
>>> The capability KVM_CAP_GMEM_SHARED_MEM should be called
>>> KVM_CAP_GMEM_MMAP
>>>
>>> The Kconfig CONFIG_KVM_GMEM_SHARED_MEM should be called
>>> CONFIG_KVM_GMEM_SUPPORTS_MMAP
>>
>> Works for me.
>>
>>>
>>> Also, what (unless you disagree) will stay the same as V12:
>>>
>>> Rename CONFIG_KVM_PRIVATE_MEM to CONFIG_KVM_GMEM: Since private
>>> implies gmem, and we will have additional flags for MMAP support
>>
>> Agreed.
>>
>>>
>>> Rename CONFIG_KVM_GENERIC_PRIVATE_MEM to
>>> CONFIG_KVM_GENERIC_GMEM_POPULATE
>>
>> Agreed.
>>
>>>
>>> Rename  kvm_slot_can_be_private() to kvm_slot_has_gmem(): since
>>> private does imply that it has gmem
>>
>> Right. It's a little more tricky in reality at least with this series:
>> without in-place conversion, not all gmem can have private memory. But
>> the places that check kvm_slot_can_be_private() likely only care about
>> if this memslot is backed by gmem.
> 
> Exactly. Reading the code, all the places that check
> kvm_slot_can_be_private() are really checking whether the slot has
> gmem. After this series, if a caller is interested in finding out
> whether a slot can be private could achieve the same effect by
> checking that a gmem slot doesn't support mmap (i.e.,
> kvm_slot_has_gmem() && kvm_arch_supports_gmem_mmap() ). If that
> happens, we can reintroduce kvm_slot_can_be_private() as such.
> 
> Otherwise, I could keep it and already define it as so. What do you think?
> 
>> Sean also raised a "kvm_is_memslot_gmem_only()", how did you end up
>> calling that?
> 
> Good point, I'd missed that. Isn't it true that
> kvm_is_memslot_gmem_only() is synonymous (at least for now) with
> kvm_gmem_memslot_supports_mmap()?

Yes. I think having a simple kvm_is_memslot_gmem_only() helper might 
make fault handling code easier to read, though.

-- 
Cheers,

David / dhildenb