Re: [PATCH v12 00/18] KVM: Mapping guest_memfd backed memory at the host for software protected VMs

[Sean Christopherson <seanjc@google.com>]

On Tue, Jun 24, 2025, Fuad Tabba wrote:
> On Tue, 24 Jun 2025 at 12:44, David Hildenbrand <david@redhat.com> wrote:
> >
> > On 24.06.25 12:25, Fuad Tabba wrote:
> > > Hi David,
> > >
> > > On Tue, 24 Jun 2025 at 11:16, David Hildenbrand <david@redhat.com> wrote:
> > >>
> > >> On 24.06.25 12:02, Fuad Tabba wrote:
> > >>> Hi,
> > >>>
> > >>> Before I respin this, I thought I'd outline the planned changes for
> > >>> V13, especially since it involves a lot of repainting. I hope that
> > >>> by presenting this first, we could reduce the number of times I'll
> > >>> need to respin it.
> > >>>
> > >>> In struct kvm_arch: add bool supports_gmem instead of renaming
> > >>> has_private_mem
> > >>>
> > >>> The guest_memfd flag GUEST_MEMFD_FLAG_SUPPORT_SHARED should be
> > >>> called GUEST_MEMFD_FLAG_MMAP
> > >>>
> > >>> The memslot internal flag KVM_MEMSLOT_SUPPORTS_GMEM_SHARED should be
> > >>> called KVM_MEMSLOT_SUPPORTS_GMEM_MMAP

This one...

> > >>> kvm_arch_supports_gmem_shared_mem() should be called
> > >>> kvm_arch_supports_gmem_mmap()
> > >>>
> > >>> kvm_gmem_memslot_supports_shared() should be called
> > >>> kvm_gmem_memslot_supports_mmap()

...and this one are the only names I don't like.  Explanation below.

> > >>> Rename  kvm_slot_can_be_private() to kvm_slot_has_gmem(): since
> > >>> private does imply that it has gmem
> > >>
> > >> Right. It's a little more tricky in reality at least with this series:
> > >> without in-place conversion, not all gmem can have private memory. But
> > >> the places that check kvm_slot_can_be_private() likely only care about
> > >> if this memslot is backed by gmem.
> > >
> > > Exactly. Reading the code, all the places that check
> > > kvm_slot_can_be_private() are really checking whether the slot has gmem.

Yeah, I'm fine with this change.  There are a few KVM x86 uses where
kvm_slot_can_be_private() is slightly better in a vacuum, but in all but one of
those cases, the check immediately gates a kvm_gmem_xxx() call.  I.e. when
looking at the code as a whole, I think kvm_slot_has_gmem() will be easier for
new readers to understand.

The only outlier is kvm_mmu_max_mapping_level(), but that'll probably get ripped
apart by this series, i.e. I'm guessing kvm_slot_has_gmem() will probably work
out better there too.

> > > After this series, if a caller is interested in finding out whether a
> > > slot can be private could achieve the same effect by checking that a gmem
> > > slot doesn't support mmap (i.e., kvm_slot_has_gmem() &&
> > > kvm_arch_supports_gmem_mmap() ). If that happens, we can reintroduce
> > > kvm_slot_can_be_private() as such.
> > >
> > > Otherwise, I could keep it and already define it as so. What do you think?
> > >
> > >> Sean also raised a "kvm_is_memslot_gmem_only()", how did you end up
> > >> calling that?
> > >
> > > Good point, I'd missed that. Isn't it true that
> > > kvm_is_memslot_gmem_only() is synonymous (at least for now) with
> > > kvm_gmem_memslot_supports_mmap()?
> >
> > Yes. I think having a simple kvm_is_memslot_gmem_only() helper might
> > make fault handling code easier to read, though.

Yep, exactly.  The fact that a memslot is bound to a guest_memfd instance that
supports mmap() isn't actually what KVM cares about.  The important part is that
the userspace_addr in the memslot needs to be ignored when mapping memory into
the guest, because the bound guest_memfd is the single source of truth for guest
mappings.

E.g. userspace could actually point userspace_addr at a completely different
mapping, in which case walking the userspace page tables to get the max mapping
size would be all kinds of wrong.

KVM will still use userspace_addr when access guest memory from within KVM,
but that's not dangerous to the host kernel/KVM, only to the guest (and userspace
is firmly in the TCB for that side of things).

So I think KVM_MEMSLOT_IS_GMEM_ONLY and kvm_is_memslot_gmem_only()?

Those names are technically not entirely true, because as above, there is no
guarantee that userspace_addr actually points at the bound guest_memfd.  But
for all intents and purposes, that will hold true for all non-buggy setups.