Re: [PATCH v4 04/10] KVM: guest_memfd: Add KVM capability to check if guest_memfd is shared

[David Hildenbrand <david@redhat.com>]

On 06.03.25 16:48, Ackerley Tng wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 28.02.25 18:22, Fuad Tabba wrote:
>>> Hi Peter,
>>>
>>> On Fri, 28 Feb 2025 at 08:24, Peter Xu <peterx@redhat.com> wrote:
>>>>
>>>> On Tue, Feb 18, 2025 at 05:24:54PM +0000, Fuad Tabba wrote:
>>>>> Add the KVM capability KVM_CAP_GMEM_SHARED_MEM, which indicates
>>>>> that the VM supports shared memory in guest_memfd, or that the
>>>>> host can create VMs that support shared memory. Supporting shared
>>>>> memory implies that memory can be mapped when shared with the
>>>>> host.
>>>>>
>>>>> Signed-off-by: Fuad Tabba <tabba@google.com>
>>>>> ---
>>>>>    include/uapi/linux/kvm.h | 1 +
>>>>>    virt/kvm/kvm_main.c      | 4 ++++
>>>>>    2 files changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>>>>> index 45e6d8fca9b9..117937a895da 100644
>>>>> --- a/include/uapi/linux/kvm.h
>>>>> +++ b/include/uapi/linux/kvm.h
>>>>> @@ -929,6 +929,7 @@ struct kvm_enable_cap {
>>>>>    #define KVM_CAP_PRE_FAULT_MEMORY 236
>>>>>    #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237
>>>>>    #define KVM_CAP_X86_GUEST_MODE 238
>>>>> +#define KVM_CAP_GMEM_SHARED_MEM 239
>>>>
>>>> I think SHARED_MEM is ok.  Said that, to me the use case in this series is
>>>> more about "in-place" rather than "shared".
>>>>
>>>> In comparison, what I'm recently looking at is a "more" shared mode of
>>>> guest-memfd where it works almost like memfd.  So all pages will be shared
>>>> there.
>>>>
>>>> That helps me e.g. for the N:1 kvm binding issue I mentioned in another
>>>> email (in one of my relies in previous version), in which case I want to
>>>> enable gmemfd folios to be mapped more than once in a process.
>>>>
>>>> That'll work there as long as it's fully shared, because all things can be
>>>> registered in the old VA way, then there's no need to have N:1 restriction.
>>>> IOW, gmemfd will still rely on mmu notifier for tearing downs, and the
>>>> gmem->bindings will always be empty.
>>>>
>>>> So if this one would be called "in-place", then I'll have my use case as
>>>> "shared".
>>>
>>> I understand what you mean. The naming here is to be consistent with
>>> the rest of the series. I don't really have a strong opinion. It means
>>> SHARED_IN_PLACE, but then that would be a mouthful. :)
>>
>> I'll note that Patrick is also driving it in "all shared" mode for his
>> direct-map removal series IIRC.
>>
>> So we would have
>>
>> a) All private
>> b) Mixing of private and shared (incl conversion)
>> c) All shared
>>
>> "IN_PLACE" might be the wrong angle to look at it.
> 
> How about something like "supports_mmap" or "mmap_capable"?
> 
> So like
> 
> + KVM_CAP_GMEM_MMAP
> + CONFIG_KVM_GMEM_MMAP_CAPABLE
> + kvm_arch_gmem_mmap_capable()
> 
> I'm just trying to avoid the use of shared, which could already mean
> 
> + shared between processes
> + shared between guest and host

The reason I tried to avoid "MMAP" is that once we support read/write of 
non-private memory, the "mmap" is a bit too specific. Similarly 
"faultable". Hmmm

-- 
Cheers,

David / dhildenb