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

[Sean Christopherson <seanjc@google.com>]

On Wed, Jun 18, 2025, David Hildenbrand wrote:
> 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:
> > > > > 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].

Another advantage of granular flags that comes to mind: WRITE (and READ) could
be withdrawn after populating memory, e.g. to harden against unexpected accesses
once the VM has been initialized.

And FWIW, I'm pretty sure it's only MMAP that *needs* userspace to opt-in.  If it
weren't for the change in memslot behavior, i.e. to always look at the guest_memfd
fd and ignore the hva, then MMAP wouldn't need a userspace opt-in.  Though we
might *want* an opt-in, e.g. for hardening purposes.

> 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.

...

> > 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? :)

...

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

Probably stating the obvious, but yes, I am.

> > 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.

Yep, that essentially what I'm advocating.

> I could envision support for non-private memory even without mmap() support,
> how useful that might be, I don't know.

It _could_ be very useful, e.g. to have very strong confidence that nothing in
userspace can accidentally clobber guest memory.  The problem is that reality gets
in the way, and so unfortunately I don't see this idea ever coming to fruition
(though I really, really like the concept).

> But that's why I was arguing that we mmap() is just one way to consume
> non-private memory.

I agree that mmap() is just one way to interact with non-private memory, but
in addition to wanting to avoid having to name "non-private memory", I also want
to avoid bundling all of those ways together.  I.e. I want to start with the bare
minimum and add functionality if/when it's needed.  Partly so that we don't have
to spend much time thinking about the unsupported methods, but mostly because
adding functionality is almost always way easier than taking it away.