Re: [PATCH v12 08/18] KVM: guest_memfd: Allow host to map guest_memfd pages

[David Hildenbrand <david@redhat.com>]

On 18.06.25 02:40, Sean Christopherson wrote:
> On Mon, Jun 16, 2025, David Hildenbrand wrote:
>> On 16.06.25 16:16, Fuad Tabba wrote:
>>> On Mon, 16 Jun 2025 at 15:03, David Hildenbrand <david@redhat.com> wrote:
>>>>>> IMO, GUEST_MEMFD_FLAG_SHAREABLE would be more appropriate.  But even that is
>>>>>> weird to me.  For non-CoCo VMs, there is no concept of shared vs. private.  What's
>>>>>> novel and notable is that the memory is _mappable_.  Yeah, yeah, pKVM's use case
>>>>>> is to share memory, but that's a _use case_, not the property of guest_memfd that
>>>>>> is being controlled by userspace.
>>>>>>
>>>>>> And kvm_gmem_memslot_supports_shared() is even worse.  It's simply that the
>>>>>> memslot is bound to a mappable guest_memfd instance, it's that the guest_memfd
>>>>>> instance is the _only_ entry point to the memslot.
>>>>>>
>>>>>> So my vote would be "GUEST_MEMFD_FLAG_MAPPABLE", and then something like
>>>>>
>>>>> If we are going to change this; FLAG_MAPPABLE is not clear to me either.
>>>>> The guest can map private memory, right?  I see your point about shared
>>>>> being overloaded with file shared but it would not be the first time a
>>>>> term is overloaded.  kvm_slot_has_gmem() does makes a lot of sense.
>>>>>
>>>>> If it is going to change; how about GUEST_MEMFD_FLAG_USER_MAPPABLE?
>>>>
>>>> If "shared" is not good enough terminology ...
>>>>
>>>> ... can we please just find a way to name what this "non-private" memory
>>>> is called?
> 
> guest_memfd?  Not trying to be cheeky, I genuinely don't understand the need
> to come up with a different name.  Before CoCo came along, I can't think of a
> single time where we felt the need to describe guest memory.  There have been
> *many* instances of referring to the underlying backing store (e.g. HugeTLB vs.
> THP), and many instances where we've needed to talk about the types of mappings
> for guest memory, but I can't think of any cases where describing the state of
> guest memory itself was ever necessary or even useful.
 >   >>>> That something is mappable into $whatever is not the right
>>>> way to look at this IMHO.
> 
> Why not?  Honest question.  USER_MAPPABLE is very literal, but I think it's the
> right granularity.  E.g. we _could_ support read()/write()/etc, but it's not
> clear to me that we need/want to.  And so why bundle those under SHARED, or any
> other one-size-fits-all flag?

Let's take a step back. There are various ways to look at this:


1) Indicate support for guest_memfd operations:

"GUEST_MEMFD_FLAG_MMAP": we support the mmap() operation
"GUEST_MEMFD_FLAG_WRITE": we support the write() operation
"GUEST_MEMFD_FLAG_READ": we support the read() operation
...
"GUEST_MEMFD_FLAG_UFFD": we support userfaultfd operations


Absolutely fine with me. In this series, we'd be advertising 
GUEST_MEMFD_FLAG_MMAP. Because we support the mmap operation.

If the others are ever required remains to be seen [1].


2) Indicating the mmap mapping type (support for MMAP flags)

As you write below, one could indicate that we support 
"mmap(MAP_SHARED)" vs "mmap(MAP_PRIVATE)".

I don't think that's required for now, as MAP_SHARED is really the 
default that anything that supports mmap() supports. If someone ever 
needs MAP_PRIVATE (CoW) support they can add such a flag 
(GUEST_MEMFD_FLAG_MMAP_MAP_PRIVATE). I doubt we want that, but who knows.

As expressed elsewhere, the mmap mapping type was never what the 
"SHARED" in KVM_GMEM_SHARED_MEM implied.


3) *guest-memfd specific* memory access characteristics

"private (non-accessible, private, secure, protected, ...) vs. 
"non-private".

Traditionally, all was memory in guest-memfd was private, now we will 
make guest_memfd also support non-private memory. As this memory is 
"inaccessible" from a host point of view, any access to read/write it 
(fault it into user page tables, read(), write(), etc) will fail.



Mempolicy support wanted to support mmap() without that, though [2], 
which was one of the reasons I agreed that exposing the access 
characteristics (that affect what you can actually mmap() ) made sense.

In the last upstream meeting we agreed that we will not do that, but 
rather built up on MMAP+support for non-private memory support.


[1] 
https://lore.kernel.org/kvm/20250303130838.28812-1-kalyazin@amazon.com/T/
[2] 
https://lore.kernel.org/linux-mm/20250408112402.181574-1-shivankg@amd.com/

[...]

>>>> I'll further note that in the doc of KVM_SET_USER_MEMORY_REGION2 we talk
>>>> about "private" vs "shared" memory ... so that would have to be improved
>>>> as well.
>>>
>>> To add to what David just wrote, V1 of this series used the term
>>> "mappable" [1].  After a few discussions, I thought the consensus was
>>> that "shared" was a more accurate description --- i.e., mappability
>>> was a side effect of it being shared with the host.
> 
> As I mentioned in the other thread with respect to sharing between other
> entities, simply SHARED doesn't provide sufficient granularity.  HOST_SHAREABLE
> gets us closer, but I still don't like that because it implies the memory is
> 100% shareable, e.g. can be accessed just like normal memory.
> 
> And for non-CoCo x86 VMs, sharing with host userspace isn't even necessarily the
> goal, i.e. "sharing" is a side effect of needing to allow mmap() so that KVM can
> continue to function.

Does mmap() support imply "support for non-private" memory or does 
"support for non-private" imply mmap() support? :)

In this series we went for the latter. If I got you correctly, you argue 
for the former.

Maybe both things should simply be separated.

> 
>>> One could argue that non-CoCo VMs have no concept of "shared" vs
>>> "private".
> 
> I am that one :-)

Well, if the concept of "private" does not exist, I'd argue everything 
is "non-private" :)

> 
>> A different way of looking at it is, non-CoCo VMs have
>>> their state as shared by default.
> 
> Eh, there has to be another state for there to be a default.
> 
>> All memory of these VMs behaves similar to other memory-based shared memory
>> backends (memfd, shmem) in the system, yes. You can map it into multiple
>> processes and use it like shmem/memfd.
> 
> Ya, but that's more because guest_memfd only supports MAP_SHARED, versus KVM
> really wanting to truly share the memory with the entire system.
 > > Of course, that's also an argument to some extent against 
USER_MAPPABLE, because
> that name assumes we'll never want to support MAP_PRIVATE.  But letting userspace
> MAP_PRIVATE guest_memfd would completely defeat the purpose of guest_memfd, so
> unless I'm forgetting a wrinkle with MAP_PRIVATE vs. MAP_SHARED, that's an
> assumption I'm a-ok making.

So, first important question, are we okay with adding:

"GUEST_MEMFD_FLAG_MMAP": we support the mmap() operation

> 
> If we are really dead set on having SHARED in the name, it could be
> GUEST_MEMFD_FLAG_USER_MAPPABLE_SHARED or GUEST_MEMFD_FLAG_USER_MAP_SHARED?  But
> to me that's _too_ specific and again somewhat confusing given the unfortunate
> private vs. shared usage in CoCo-land.  And just playing the odds, I'm fine taking
> a risk of ending up with GUEST_MEMFD_FLAG_USER_MAPPABLE_PRIVATE or whatever,
> because I think that is comically unlikely to happen.

I think in addition to GUEST_MEMFD_FLAG_MMAP we want something to 
express "this is not your old guest_memfd that only supports private 
memory". And that's what I am struggling with.

Now, if you argue "support for mmap() implies support for non-private 
memory", I'm probably okay for that.

I could envision support for non-private memory even without mmap() 
support, how useful that might be, I don't know. But that's why I was 
arguing that we mmap() is just one way to consume non-private memory.

-- 
Cheers,

David / dhildenb