Re: [PATCH 1/6] KVM: guest_memfd: Add DEFAULT_SHARED flag, reject user page faults if not set

[Sean Christopherson <seanjc@google.com>]

On Thu, Oct 02, 2025, Vishal Annapurve wrote:
> On Thu, Oct 2, 2025, 5:12 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > > >
> > > > If the _only_ user-visible asset that is added is a KVM_CREATE_GUEST_MEMFD flag,
> > > > a CAP is gross overkill.  Even if there are other assets that accompany the new
> > > > flag, there's no reason we couldn't say "this feature exist if XYZ flag is
> > > > supported".
> > > >
> > > > E.g. it's functionally no different than KVM_CAP_VM_TYPES reporting support for
> > > > KVM_X86_TDX_VM also effectively reporting support for a _huge_ number of things
> > > > far beyond being able to create a VM of type KVM_X86_TDX_VM.
> > > >
> > >
> > > What's your opinion about having KVM_CAP_GUEST_MEMFD_MMAP part of
> > > KVM_CAP_GUEST_MEMFD_CAPS i.e. having a KVM cap covering all features
> > > of guest_memfd?
> >
> > I'd much prefer to have both.  Describing flags for an ioctl via a bitmask that
> > doesn't *exactly* match the flags is asking for problems.  At best, it will be
> > confusing.  E.g. we'll probably end up with code like this:
> >
> >         gmem_caps = kvm_check_cap(KVM_CAP_GUEST_MEMFD_CAPS);
> >
> >         if (gmem_caps & KVM_CAP_GUEST_MEMFD_MMAP)
> >                 gmem_flags |= GUEST_MEMFD_FLAG_MMAP;
> >         if (gmem_caps & KVM_CAP_GUEST_MEMFD_INIT_SHARED)
> >                 gmem_flags |= KVM_CAP_GUEST_MEMFD_INIT_SHARED;
> >
> 
> No, I actually meant the userspace can just rely on the cap to assume
> right flags to be available (not necessarily the same flags as cap
> bits).
> 
> i.e. Userspace will do something like:
> gmem_caps = kvm_check_cap(KVM_CAP_GUEST_MEMFD_CAPS);
> 
> if (gmem_caps & KVM_CAP_GUEST_MEMFD_MMAP)
>         gmem_flags |= GUEST_MEMFD_FLAG_MMAP;
> if (gmem_caps & KVM_CAP_GUEST_MEMFD_HUGETLB)
>         gmem_flags |= GUEST_MEMFD_FLAG_HUGETLB | GUEST_MEMFD_FLAG_HUGETLB_2MB;

Yes, that's exactly what I said.  But I goofed when copy+pasted and failed to
do s/KVM_CAP_GUEST_MEMFD_INIT_SHARED/GUEST_MEMFD_FLAG_INIT_SHARED, which is the
type of bug that ideally just can't happen.

Side topic, I'm not at all convinced that this is what we want for KVM's uAPI:

	if (gmem_caps & KVM_CAP_GUEST_MEMFD_HUGETLB)                                  
		gmem_flags |= GUEST_MEMFD_FLAG_HUGETLB | GUEST_MEMFD_FLAG_HUGETLB_2MB;

See https://lore.kernel.org/all/aN_fJEZXo6wkcHOh@google.com.

> Userspace has to anyways assume flag values, userspace just needs to
> know if a particular feature is available.

I don't understand what you mean by "assume flag values".