Re: [PATCH v3 4/6] KVM: guest_memfd: add support for userfaultfd minor

[Nikita Kalyazin]

On 10/06/2025 23:25, Peter Xu wrote:
> On Fri, Apr 04, 2025 at 03:43:50PM +0000, Nikita Kalyazin wrote:
> > Add support for sending a pagefault event if userfaultfd is registered.
> > Only page minor event is currently supported.
> >
> > Signed-off-by: Nikita Kalyazin <kalyazin@xxxxxxxxxx>
> > ---
> >   virt/kvm/guest_memfd.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index fbf89e643add..096d89e7282d 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -4,6 +4,9 @@
> >   #include <linux/kvm_host.h>
> >   #include <linux/pagemap.h>
> >   #include <linux/anon_inodes.h>
> > +#ifdef CONFIG_KVM_PRIVATE_MEM
> > +#include <linux/userfaultfd_k.h>
> > +#endif /* CONFIG_KVM_PRIVATE_MEM */
> >
> >   #include "kvm_mm.h"
> >
> > @@ -380,6 +383,13 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
> >                kvm_gmem_mark_prepared(folio);
> >        }
> >
> > +     if (userfaultfd_minor(vmf->vma) &&
> > +         !(vmf->flags & FAULT_FLAG_USERFAULT_CONTINUE)) {
> > +             folio_unlock(folio);
> > +             filemap_invalidate_unlock_shared(inode->i_mapping);
> > +             return handle_userfault(vmf, VM_UFFD_MINOR);
> > +     }
> > +
>
> Hmm, does guest-memfd (when with your current approach) at least needs to
> define the new can_userfault() hook?
>
> Meanwhile, we have some hard-coded lines so far, like:
>
> mfill_atomic():
>          if (!vma_is_shmem(dst_vma) &&
>              uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE))
>                  goto out_unlock;
>
> I thought it would fail guest-memfd already on a CONTINUE request, and it
> doesn't seem to be touched yet in this series.
>
> I'm not yet sure how the test worked out without hitting things like it.
> Highly likely I missed something.  Some explanations would be welcomed..

Yes, I realised that I'd failed to post this part soon after I sent the 
series, but I refrained from sending a new version because the upstream 
consensus was to review/merge the mmap support in guest_memfd [1] before 
continuing to build on top of it.  This is the missed part I planned to 
include in the next version.  Sorry for the confusion.

diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h
index 64551e8a55fb..080437fa7eab 100644
--- a/include/linux/userfaultfd_k.h
+++ b/include/linux/userfaultfd_k.h
@@ -221,8 +221,10 @@ static inline bool vma_can_userfault(struct 
vm_area_struct *vma,
 	if (vm_flags & VM_DROPPABLE)
 		return false;

-	if (!vma->vm_ops->can_userfault ||
-	    !vma->vm_ops->can_userfault(vma, VM_UFFD_MINOR))
+	if ((vm_flags & VM_UFFD_MINOR) &&
+	     (!vma->vm_ops ||
+	      !vma->vm_ops->can_userfault ||
+	      !vma->vm_ops->can_userfault(vma, VM_UFFD_MINOR)))
 		return false;

 	/*
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 0aa82c968e16..638360a78561 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -788,7 +788,9 @@ static __always_inline ssize_t mfill_atomic(struct 
userfaultfd_ctx *ctx,
 		return  mfill_atomic_hugetlb(ctx, dst_vma, dst_start,
 					     src_start, len, flags);

-	can_userfault = dst_vma->vm_ops->can_userfault &&
+	can_userfault =
+	    dst_vma->vm_ops &&
+	    dst_vma->vm_ops->can_userfault &&
 	    dst_vma->vm_ops->can_userfault(dst_vma, __VM_UFFD_FLAGS);

 	if (!vma_is_anonymous(dst_vma) && !can_userfault)
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 91ee5dd91c31..202b12dc4b6f 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -420,8 +420,15 @@ static vm_fault_t kvm_gmem_fault(struct vm_fault *vmf)
 	return ret;
 }

+static bool kvm_gmem_can_userfault(struct vm_area_struct *vma,
+				   unsigned long vm_flags)
+{
+	return vm_flags & VM_UFFD_MINOR;
+}
+
 static const struct vm_operations_struct kvm_gmem_vm_ops = {
-	.fault = kvm_gmem_fault,
+	.fault         = kvm_gmem_fault,
+	.can_userfault = kvm_gmem_can_userfault,
 };

 static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)


[1] https://lore.kernel.org/kvm/20250605153800.557144-1-tabba@xxxxxxxxxx/

> Thanks,
>
> >        vmf->page = folio_file_page(folio, vmf->pgoff);
> >
> >   out_folio:
> > --
> > 2.47.1
>
> --
> Peter Xu