Hi Xiaoyao, On Wed, 16 Jul 2025 at 07:11, Xiaoyao Li <xiaoyao.li@xxxxxxxxx> wrote: > > On 7/15/2025 5:33 PM, Fuad Tabba wrote: > > Add a new internal flag, KVM_MEMSLOT_GMEM_ONLY, to the top half of > > memslot->flags. This flag tracks when a guest_memfd-backed memory slot > > supports host userspace mmap operations. It's strictly for KVM's > > internal use. > > I would expect some clarification of why naming it with > KVM_MEMSLOT_GMEM_ONLY, not something like KVM_MEMSLOT_GMEM_MMAP_ENABLED > > There was a patch to check the userspace_addr of the memslot refers to > the same memory as guest memfd[1], but that patch was dropped. Without > the background that when guest memfd is mmapable, userspace doesn't need > to provide separate memory via userspace_addr, it's hard to understand > and accept the name of GMEM_ONLY. The commit message could have clarified this a bit more. Regarding the rationale for the naming, there have been various threads and live discussions in the biweekly guest_memfd meeting . Instead of rehashing the discussion here, I can refer you to a couple [1, 2]. [1] https://docs.google.com/document/d/1M6766BzdY1Lhk7LiR5IqVR8B8mG3cr-cxTxOrAosPOk/edit?tab=t.0#heading=h.a15es1buok51 [2] https://lore.kernel.org/all/aFwChljXL5QJYLM_@xxxxxxxxxx/ Thanks, /fuad > [1] https://lore.kernel.org/all/20250513163438.3942405-9-tabba@xxxxxxxxxx/ > > > This optimization avoids repeatedly checking the underlying guest_memfd > > file for mmap support, which would otherwise require taking and > > releasing a reference on the file for each check. By caching this > > information directly in the memslot, we reduce overhead and simplify the > > logic involved in handling guest_memfd-backed pages for host mappings. > > > > Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx> > > Reviewed-by: Shivank Garg <shivankg@xxxxxxx> > > Acked-by: David Hildenbrand <david@xxxxxxxxxx> > > Suggested-by: David Hildenbrand <david@xxxxxxxxxx> > > Signed-off-by: Fuad Tabba <tabba@xxxxxxxxxx> > > --- > > include/linux/kvm_host.h | 11 ++++++++++- > > virt/kvm/guest_memfd.c | 2 ++ > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > index 9ac21985f3b5..d2218ec57ceb 100644 > > --- a/include/linux/kvm_host.h > > +++ b/include/linux/kvm_host.h > > @@ -54,7 +54,8 @@ > > * used in kvm, other bits are visible for userspace which are defined in > > * include/uapi/linux/kvm.h. > > */ > > -#define KVM_MEMSLOT_INVALID (1UL << 16) > > +#define KVM_MEMSLOT_INVALID (1UL << 16) > > +#define KVM_MEMSLOT_GMEM_ONLY (1UL << 17) > > > > /* > > * Bit 63 of the memslot generation number is an "update in-progress flag", > > @@ -2536,6 +2537,14 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu, > > vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_PRIVATE; > > } > > > > +static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot) > > +{ > > + if (!IS_ENABLED(CONFIG_KVM_GMEM_SUPPORTS_MMAP)) > > + return false; > > + > > + return slot->flags & KVM_MEMSLOT_GMEM_ONLY; > > +} > > + > > #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES > > static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn) > > { > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > > index 07a4b165471d..2b00f8796a15 100644 > > --- a/virt/kvm/guest_memfd.c > > +++ b/virt/kvm/guest_memfd.c > > @@ -592,6 +592,8 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot, > > */ > > WRITE_ONCE(slot->gmem.file, file); > > slot->gmem.pgoff = start; > > + if (kvm_gmem_supports_mmap(inode)) > > + slot->flags |= KVM_MEMSLOT_GMEM_ONLY; > > > > xa_store_range(&gmem->bindings, start, end - 1, slot, GFP_KERNEL); > > filemap_invalidate_unlock(inode->i_mapping); >
