Re: [PATCH v12 10/18] KVM: x86/mmu: Handle guest page faults for guest_memfd with shared memory

[Fuad Tabba <tabba@google.com>]

Hi Ackerley,

On Fri, 27 Jun 2025 at 16:01, Ackerley Tng <ackerleytng@google.com> wrote:
>
> Ackerley Tng <ackerleytng@google.com> writes:
>
> > [...]
>
> >>> +/*
> >>> + * Returns true if the given gfn's private/shared status (in the CoCo sense) is
> >>> + * private.
> >>> + *
> >>> + * A return value of false indicates that the gfn is explicitly or implicitly
> >>> + * shared (i.e., non-CoCo VMs).
> >>> + */
> >>>  static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
> >>>  {
> >>> -   return IS_ENABLED(CONFIG_KVM_GMEM) &&
> >>> -          kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> >>> +   struct kvm_memory_slot *slot;
> >>> +
> >>> +   if (!IS_ENABLED(CONFIG_KVM_GMEM))
> >>> +           return false;
> >>> +
> >>> +   slot = gfn_to_memslot(kvm, gfn);
> >>> +   if (kvm_slot_has_gmem(slot) && kvm_gmem_memslot_supports_shared(slot)) {
> >>> +           /*
> >>> +            * Without in-place conversion support, if a guest_memfd memslot
> >>> +            * supports shared memory, then all the slot's memory is
> >>> +            * considered not private, i.e., implicitly shared.
> >>> +            */
> >>> +           return false;
> >>
> >> Why!?!?  Just make sure KVM_MEMORY_ATTRIBUTE_PRIVATE is mutually exclusive with
> >> mappable guest_memfd.  You need to do that no matter what.
> >
> > Thanks, I agree that setting KVM_MEMORY_ATTRIBUTE_PRIVATE should be
> > disallowed for gfn ranges whose slot is guest_memfd-only. Missed that
> > out. Where do people think we should check the mutual exclusivity?
> >
> > In kvm_supported_mem_attributes() I'm thiking that we should still allow
> > the use of KVM_MEMORY_ATTRIBUTE_PRIVATE for other non-guest_memfd-only
> > gfn ranges. Or do people think we should just disallow
> > KVM_MEMORY_ATTRIBUTE_PRIVATE for the entire VM as long as one memslot is
> > a guest_memfd-only memslot?
> >
> > If we check mutually exclusivity when handling
> > kvm_vm_set_memory_attributes(), as long as part of the range where
> > KVM_MEMORY_ATTRIBUTE_PRIVATE is requested to be set intersects a range
> > whose slot is guest_memfd-only, the ioctl will return EINVAL.
> >
>
> At yesterday's (2025-06-26) guest_memfd upstream call discussion,
>
> * Fuad brought up a possible use case where within the *same* VM, we
>   want to allow both memslots that supports and does not support mmap in
>   guest_memfd.
> * Shivank suggested a concrete use case for this: the user wants a
>   guest_memfd memslot that supports mmap just so userspace addresses can
>   be used as references for specifying memory policy.
> * Sean then added on that allowing both types of guest_memfd memslots
>   (support and not supporting mmap) will allow the user to have a second
>   layer of protection and ensure that for some memslots, the user
>   expects never to be able to mmap from the memslot.
>
> I agree it will be useful to allow both guest_memfd memslots that
> support and do not support mmap in a single VM.
>
> I think I found an issue with flags, which is that GUEST_MEMFD_FLAG_MMAP
> should not imply that the guest_memfd will provide memory for all guest
> faults within the memslot's gfn range (KVM_MEMSLOT_GMEM_ONLY).
>
> For the use case Shivank raised, if the user wants a guest_memfd memslot
> that supports mmap just so userspace addresses can be used as references
> for specifying memory policy for legacy Coco VMs where shared memory
> should still come from other sources, GUEST_MEMFD_FLAG_MMAP will be set,
> but KVM can't fault shared memory from guest_memfd. Hence,
> GUEST_MEMFD_FLAG_MMAP should not imply KVM_MEMSLOT_GMEM_ONLY.
>
> Thinking forward, if we want guest_memfd to provide (no-mmap) protection
> even for non-CoCo VMs (such that perhaps initial VM image is populated
> and then VM memory should never be mmap-ed at all), we will want
> guest_memfd to be the source of memory even if GUEST_MEMFD_FLAG_MMAP is
> not set.
>
> I propose that we should have a single VM-level flag to solve this (in
> line with Sean's guideline that we should just move towards what we want
> and not support non-existent use cases): something like
> KVM_CAP_PREFER_GMEM.
>
> If KVM_CAP_PREFER_GMEM_MEMORY is set,
>
> * memory for any gfn range in a guest_memfd memslot will be requested
>   from guest_memfd
> * any privacy status queries will also be directed to guest_memfd
> * KVM_MEMORY_ATTRIBUTE_PRIVATE will not be a valid attribute
>
> KVM_CAP_PREFER_GMEM_MEMORY will be orthogonal with no validation on
> GUEST_MEMFD_FLAG_MMAP, which should just purely guard mmap support in
> guest_memfd.
>
> Here's a table that I set up [1]. I believe the proposed
> KVM_CAP_PREFER_GMEM_MEMORY (column 7) lines up with requirements
> (columns 1 to 4) correctly.
>
> [1] https://lpc.events/event/18/contributions/1764/attachments/1409/3710/guest_memfd%20use%20cases%20vs%20guest_memfd%20flags%20and%20privacy%20tracking.pdf

I'm not sure this naming helps. What does "prefer" imply here? If the
caller from user space does not prefer, does it mean that they
mind/oppose?

Regarding the use case Shivank mentioned, mmaping for policy, while
the use case is a valid one, the raison d'être of mmap is to map into
user space (i.e., fault it in). I would argue that if you opt into
mmap, you are doing it to be able to access it. To me, that seems like
something that merits its own flag, rather than mmap. Also, I recall
that we said that later on, with inplace conversion, that won't be
even necessary. In other words, this would also be trying to solve a
problem that we haven't yet encountered and that we have a solution
for anyway.

I think that, unless anyone disagrees, is to go ahead with the names
we discussed in the last meeting. They seem to be the ones that make
the most sense for the upcoming use cases.

Cheers,
/fuad



> > [...]
>