[RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

The memory represented by guest_memfd-only memslots
(kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
freely mmap it and access it. The only thing that is preventing dirty page
logging for such memslots is that KVM doesn't allow slots backed by
guest_memfd to have their flags changed; they can only be created and
deleted.

When KVM changes the flags for a memslot, it is possible for one reader
handling a guest fault to observe the old memslot, with the old flags, and
another reader to observe the new memslot, with the new flags. With the way
a guest_memfd file keeps track of the associated memslots, it is impossible
to avoid the WARN_ON_ONCE in __kvm_gmem_get_pfn() when one of the two
memslot pointers that the readers observe doesn't match the memslot pointer
stored in the file bindings.

To get around this, I decided to change the way guest_memfd keeps track of
the associated memslots: instead of an xarray of memslot pointers, use an
xarray which stores the memslot id (id and as_id, to be more precise),
which can be used to search for the memslot in the active memslots array.
Whenever guest_memfd wants to access a memslot, it deferences the
kvm->memslots RCU pointer under the RCU read lock, similar to how guest
faults are handled, or how the MMU notifiers work. All of this is
implemented in patch #1, "KVM: guest_memfd: Use memslot id to keep track of
associated memslots".

Building on that, toggling the KVM_MEM_LOG_DIRTY_PAGES flag for
guest_memfd-only memslots is implemented in patch #2, "KVM: Implement dirty
page logging for guest_memfd-only memslots". This is gated by a KVM
capability because it is a userspace visible change in behaviour.

The capability is also architecture specific, not because this is something
tied to a specific architecture, but because when I was testing the series
on arm64 I realized that the arm64 fault handling code required a minor
change, and I don't know enough about the other architectures to tell if
any changes are needed for them.

Just FYI, this is an RFC so it goes without saying that I'm open to any
suggestions, and I'll redo the whole thing if there's a better solution.

Tested the series using kvmtool on an arm64 machine, with guest_memfd
support added, as well as two command line arguments: --enable-dirty-log
and --disable-dirty-log. The arguments toggle the KVM_MEM_LOG_DIRTY_PAGES
memslot flag, they don't read or otherwise touch the list of dirty pages in
any way. Pushed a branch at [1].

[1] https://gitlab.arm.com/linux-arm/kvmtool-ae/-/tree/guest-memfd-v1-wip4-dirty-page-logging

Alexandru Elisei (3):
  KVM: guest_memfd: Use memslot id to keep track of associated memslots
  KVM: Implement dirty page logging for guest_memfd-only memslots
  KVM: arm64: Allow dirty page logging for guest_memfd-only memslots

 Documentation/virt/kvm/api.rst |   9 ++
 arch/arm64/kvm/arm.c           |  22 +++++
 arch/arm64/kvm/mmu.c           |   3 +-
 include/linux/kvm_host.h       |  15 +++
 include/uapi/linux/kvm.h       |   1 +
 virt/kvm/guest_memfd.c         | 165 ++++++++++++++++++++++++++++++---
 virt/kvm/kvm_main.c            |  44 ++++-----
 virt/kvm/kvm_mm.h              |  11 +++
 8 files changed, 232 insertions(+), 38 deletions(-)


base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
-- 
2.43.0

[RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Alexandru Elisei]

To enable memslot operations, KVM maintains two arrays of memslots, and an
RCU pointer to the active (in use) array. Changes are made first to the
inactive array, and the RCU pointer is updated to point to the inactive
array, which becomes active.

The guest_memfd file maintains an xarray of pointers to memslots that use
it as the memory provider. After the RCU pointer to the active memslots is
updated and until SRCU is synchronized, readers can observe the old or the
new value for the active array, and therefore the old or the new pointer
for a given memslot.  For memslot creation or deletion that is not an issue
for guest_memfd, as readers will either read the same memslot pointer saved
by the guest_memfd file, or a non-existing memslot.

But when changing the flags for a memslot, readers can read two different
and non-NULL memslot pointers. Since there is no easy way to ensure that
the memslot pointer that the guest_memfd stores is consistent with both
views at the same time, modify how the guest_memfd file keeps track of the
associated memslots: instead of storing the pointer directly, store the
memslot id and address space id (as_id), and use that to reach the memslot
in the active list of memslots.

This only changes how guest_memfd keeps track of memslots, userspace is not
allowed to make changes to a memslot yet.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 virt/kvm/guest_memfd.c | 95 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 80 insertions(+), 15 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index db57c5766ab6..43ef8e908aaf 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -25,6 +25,7 @@ struct gmem_file {
 	struct kvm *kvm;
 	struct xarray bindings;
 	struct list_head entry;
+	bool found_memslot;	/* Used for balancing invalidations when punching a hole */
 };
 
 struct gmem_inode {
@@ -43,6 +44,29 @@ static __always_inline struct gmem_inode *GMEM_I(struct inode *inode)
 #define kvm_gmem_for_each_file(f, inode) \
 	list_for_each_entry(f, &GMEM_I(inode)->gmem_file_list, entry)
 
+static void *memslot_to_xa_value(struct kvm_memory_slot *slot)
+{
+	WARN_ON_ONCE(sizeof(slot->as_id) > 16);
+	WARN_ON_ONCE(sizeof(slot->id) > 16);
+	WARN_ON_ONCE(sizeof(slot->as_id) + sizeof(slot->id) > sizeof(unsigned long));
+
+	return xa_mk_value(((unsigned long)slot->as_id) << 16 | (unsigned long)slot->id);
+}
+
+static struct kvm_memory_slot *xa_value_to_memslot(struct kvm *kvm, const void *entry)
+{
+	unsigned long full_id = xa_to_value(entry);
+	u16 as_id = (full_id >> 16) & U16_MAX;
+	short id = full_id & U16_MAX;
+
+	/*
+	 * Do not ignore KVM_MEMSLOT_INVALID memslots, as we want
+	 * ->error_remove_folio(), when it races with memslot deletion, to have
+	 *  unmapped the memory upon completion.
+	 */
+	return id_to_memslot(__kvm_memslots(kvm, as_id), id);
+}
+
 /**
  * folio_file_pfn - like folio_file_page, but return a pfn.
  * @folio: The folio which contains this index.
@@ -157,7 +181,7 @@ static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *in
 	return KVM_FILTER_PRIVATE;
 }
 
-static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
+static bool __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
 					pgoff_t end,
 					enum kvm_gfn_range_filter attr_filter)
 {
@@ -165,9 +189,15 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
 	struct kvm_memory_slot *slot;
 	struct kvm *kvm = f->kvm;
 	unsigned long index;
+	void *entry;
+
+	xa_for_each_range(&f->bindings, index, entry, start, end - 1) {
+		pgoff_t pgoff;
 
-	xa_for_each_range(&f->bindings, index, slot, start, end - 1) {
-		pgoff_t pgoff = slot->gmem.pgoff;
+		slot = xa_value_to_memslot(kvm, entry);
+		if (!slot)
+			continue;
+		pgoff = slot->gmem.pgoff;
 
 		struct kvm_gfn_range gfn_range = {
 			.start = slot->base_gfn + max(pgoff, start) - pgoff,
@@ -192,6 +222,8 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
 
 	if (found_memslot)
 		KVM_MMU_UNLOCK(kvm);
+
+	return found_memslot;
 }
 
 static void kvm_gmem_invalidate_start(struct inode *inode, pgoff_t start,
@@ -199,11 +231,22 @@ static void kvm_gmem_invalidate_start(struct inode *inode, pgoff_t start,
 {
 	enum kvm_gfn_range_filter attr_filter;
 	struct gmem_file *f;
+	struct kvm *kvm;
+	int idx;
 
 	attr_filter = kvm_gmem_get_invalidate_filter(inode);
 
-	kvm_gmem_for_each_file(f, inode)
-		__kvm_gmem_invalidate_start(f, start, end, attr_filter);
+	kvm_gmem_for_each_file(f, inode) {
+		kvm = f->kvm;
+		idx = srcu_read_lock(&kvm->srcu);
+		/*
+		 * This is safe to do because calls to
+		 * kvm_gmem_invalidate_start() are serialized by
+		 * filemap_invalidate_lock().
+		 */
+		f->found_memslot = __kvm_gmem_invalidate_start(f, start, end, attr_filter);
+		srcu_read_unlock(&kvm->srcu, idx);
+	}
 }
 
 static void __kvm_gmem_invalidate_end(struct gmem_file *f, pgoff_t start,
@@ -223,8 +266,11 @@ static void kvm_gmem_invalidate_end(struct inode *inode, pgoff_t start,
 {
 	struct gmem_file *f;
 
-	kvm_gmem_for_each_file(f, inode)
+	kvm_gmem_for_each_file(f, inode) {
+		if (!f->found_memslot)
+			continue;
 		__kvm_gmem_invalidate_end(f, start, end);
+	}
 }
 
 static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
@@ -326,6 +372,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
 	struct kvm_memory_slot *slot;
 	struct kvm *kvm = f->kvm;
 	unsigned long index;
+	void *entry;
 
 	/*
 	 * Prevent concurrent attempts to *unbind* a memslot.  This is the last
@@ -344,17 +391,18 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
 
 	filemap_invalidate_lock(inode->i_mapping);
 
-	xa_for_each(&f->bindings, index, slot)
+	xa_for_each(&f->bindings, index, entry) {
+		slot = xa_value_to_memslot(kvm, entry);
 		WRITE_ONCE(slot->gmem.file, NULL);
+	}
 
 	/*
 	 * All in-flight operations are gone and new bindings can be created.
 	 * Zap all SPTEs pointed at by this file.  Do not free the backing
 	 * memory, as its lifetime is associated with the inode, not the file.
 	 */
-	__kvm_gmem_invalidate_start(f, 0, -1ul,
-				    kvm_gmem_get_invalidate_filter(inode));
-	__kvm_gmem_invalidate_end(f, 0, -1ul);
+	if (__kvm_gmem_invalidate_start(f, 0, -1ul, kvm_gmem_get_invalidate_filter(inode)))
+		__kvm_gmem_invalidate_end(f, 0, -1ul);
 
 	list_del(&f->entry);
 
@@ -498,14 +546,20 @@ static int kvm_gmem_migrate_folio(struct address_space *mapping,
 
 static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *folio)
 {
+	enum kvm_gfn_range_filter attr_filter;
+	struct inode *inode = mapping->host;
+	struct gmem_file *f;
 	pgoff_t start, end;
+	bool found_memslot;
+	struct kvm *kvm;
+	int idx;
 
 	filemap_invalidate_lock_shared(mapping);
 
 	start = folio->index;
 	end = start + folio_nr_pages(folio);
 
-	kvm_gmem_invalidate_start(mapping->host, start, end);
+	attr_filter = kvm_gmem_get_invalidate_filter(inode);
 
 	/*
 	 * Do not truncate the range, what action is taken in response to the
@@ -515,8 +569,19 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol
 	 * at which point KVM can either terminate the VM or propagate the
 	 * error to userspace.
 	 */
+	kvm_gmem_for_each_file(f, inode) {
+		kvm = f->kvm;
+
+		idx = srcu_read_lock(&kvm->srcu);
+		found_memslot = __kvm_gmem_invalidate_start(f, start, end, attr_filter);
+		srcu_read_unlock(&kvm->srcu, idx);
 
-	kvm_gmem_invalidate_end(mapping->host, start, end);
+		if (found_memslot) {
+			KVM_MMU_LOCK(kvm);
+			kvm_mmu_invalidate_end(kvm);
+			KVM_MMU_UNLOCK(kvm);
+		}
+	}
 
 	filemap_invalidate_unlock_shared(mapping);
 
@@ -691,7 +756,7 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	if (kvm_gmem_supports_mmap(inode))
 		slot->flags |= KVM_MEMSLOT_GMEM_ONLY;
 
-	xa_store_range(&f->bindings, start, end - 1, slot, GFP_KERNEL);
+	xa_store_range(&f->bindings, start, end - 1, memslot_to_xa_value(slot), GFP_KERNEL);
 	filemap_invalidate_unlock(inode->i_mapping);
 
 	/*
@@ -765,8 +830,8 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
 		return ERR_PTR(-EFAULT);
 	}
 
-	if (xa_load(&f->bindings, index) != slot) {
-		WARN_ON_ONCE(xa_load(&f->bindings, index));
+	if (xa_load(&f->bindings, index) != memslot_to_xa_value(slot)) {
+		WARN_ON_ONCE(xa_to_value(xa_load(&f->bindings, index)));
 		return ERR_PTR(-EIO);
 	}
 
-- 
2.43.0

[RFC PATCH 2/3] KVM: Implement dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

The entire memory represented by guest_memfd-only memslot is shared and
accessible by userspace. Enable dirty page logging for such memslots, and
allow architectures to advertise support for it with the
KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES capability.

No architecture supports it yet.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 Documentation/virt/kvm/api.rst |  9 +++++
 include/linux/kvm_host.h       | 15 ++++++++
 include/uapi/linux/kvm.h       |  1 +
 virt/kvm/guest_memfd.c         | 70 ++++++++++++++++++++++++++++++++++
 virt/kvm/kvm_main.c            | 44 ++++++++++-----------
 virt/kvm/kvm_mm.h              | 11 ++++++
 6 files changed, 128 insertions(+), 22 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a5f9ee92f43e..5012afe6a9b5 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -9493,6 +9493,15 @@ take care to differentiate between these cases.
 The presence of this capability indicates that the nested KVM guest can
 start in ESA mode.
 
+8.48 KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES
+---------------------------------------------
+
+:Architectures: all
+
+The presence of this capability indicates that memslots backed by a guest_memfd
+file descriptor created with the GUEST_MEMFD_FLAG_MMAP flag can have dirty
+page logging enabled.
+
 9. Known KVM API problems
 =========================
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ab8cfaec82d3..c44e9253eb40 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -56,6 +56,7 @@
  */
 #define KVM_MEMSLOT_INVALID			(1UL << 16)
 #define KVM_MEMSLOT_GMEM_ONLY			(1UL << 17)
+#define MEMSLOT_USER_FLAGS_MASK			0xffff
 
 /*
  * Bit 63 of the memslot generation number is an "update in-progress flag",
@@ -731,6 +732,9 @@ static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
 
 #ifdef CONFIG_KVM_GUEST_MEMFD
 bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm);
+bool kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm);
+int kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *slot,
+			     unsigned int fd, loff_t offset);
 
 static inline u64 kvm_gmem_get_supported_flags(struct kvm *kvm)
 {
@@ -741,6 +745,17 @@ static inline u64 kvm_gmem_get_supported_flags(struct kvm *kvm)
 
 	return flags;
 }
+#else
+static inline bool kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm)
+{
+	return false;
+}
+static inline int kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *slot,
+					   unsigned int fd, loff_t offset)
+{
+	WARN_ON_ONCE(1);
+	return -EIO;
+}
 #endif
 
 #ifndef kvm_arch_has_readonly_mem
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 419011097fa8..5a53e2e19b2f 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -997,6 +997,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_S390_KEYOP 247
 #define KVM_CAP_S390_VSIE_ESAMODE 248
 #define KVM_CAP_S390_HPAGE_2G 249
+#define KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES 250
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 43ef8e908aaf..210bdd76f0aa 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -622,6 +622,11 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
 	return true;
 }
 
+bool __weak kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm)
+{
+	return false;
+}
+
 static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
 {
 	static const char *name = "[kvm-gmem]";
@@ -705,6 +710,66 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
 	return __kvm_gmem_create(kvm, size, flags);
 }
 
+static int __kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *old,
+				      struct file *old_file, unsigned int fd,
+				      loff_t offset)
+{
+	struct file *new_file;
+
+	new_file = fget(fd);
+	if (!new_file)
+		return -EBADF;
+	if (new_file != old_file) {
+		fput(new_file);
+		return -EBADF;
+	}
+	fput(new_file);
+
+	if (old->gmem.pgoff != offset >> PAGE_SHIFT)
+		return -EINVAL;
+
+	return 0;
+}
+
+int kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *old,
+			     unsigned int fd, loff_t offset)
+{
+	CLASS(gmem_get_file, old_file)(old);
+
+	return __kvm_gmem_check_no_change(kvm, old, old_file, fd, offset);
+}
+
+int kvm_gmem_change_flags(struct kvm *kvm, struct kvm_memory_slot *old,
+			  struct kvm_memory_slot *new, unsigned int fd,
+			  loff_t offset)
+{
+	struct gmem_file *old_f;
+	int ret;
+
+	lockdep_assert_held(&kvm->slots_lock);
+
+	if (!kvm_memslot_is_gmem_only(old))
+		return -EINVAL;
+
+	CLASS(gmem_get_file, old_file)(old);
+
+	ret = __kvm_gmem_check_no_change(kvm, old, old_file, fd, offset);
+	if (ret)
+		return ret;
+
+	old_f = old_file->private_data;
+	if (xa_load(&old_f->bindings, old->gmem.pgoff) != memslot_to_xa_value(new)) {
+		WARN_ON_ONCE(xa_to_value(xa_load(&old_f->bindings, old->gmem.pgoff)));
+		return -EIO;
+	}
+
+	new->gmem.file = old->gmem.file;
+	new->gmem.pgoff = old->gmem.pgoff;
+	new->flags |= KVM_MEMSLOT_GMEM_ONLY;
+
+	return 0;
+}
+
 int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 		  unsigned int fd, uoff_t offset)
 {
@@ -734,6 +799,11 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 	if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
 		goto err;
 
+	if (slot->flags & KVM_MEM_LOG_DIRTY_PAGES &&
+	    (!kvm_gmem_supports_mmap(inode) ||
+	     !kvm_arch_supports_gmem_mmap_dirty_logging(kvm)))
+		goto err;
+
 	filemap_invalidate_lock(inode->i_mapping);
 
 	start = offset >> PAGE_SHIFT;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e44c20c04961..af380e8f2b68 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1573,14 +1573,14 @@ static void kvm_replace_memslot(struct kvm *kvm,
 static int check_memory_region_flags(struct kvm *kvm,
 				     const struct kvm_userspace_memory_region2 *mem)
 {
-	u32 valid_flags = KVM_MEM_LOG_DIRTY_PAGES;
+	u32 valid_flags = 0;
 
 	if (IS_ENABLED(CONFIG_KVM_GUEST_MEMFD))
 		valid_flags |= KVM_MEM_GUEST_MEMFD;
 
-	/* Dirty logging private memory is not currently supported. */
-	if (mem->flags & KVM_MEM_GUEST_MEMFD)
-		valid_flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
+	if (!(mem->flags & KVM_MEM_GUEST_MEMFD) ||
+	    kvm_arch_supports_gmem_mmap_dirty_logging(kvm))
+		valid_flags |= KVM_MEM_LOG_DIRTY_PAGES;
 
 	/*
 	 * GUEST_MEMFD is incompatible with read-only memslots, as writes to
@@ -1739,16 +1739,6 @@ static void kvm_commit_memory_region(struct kvm *kvm,
 		 */
 		if (old->dirty_bitmap && !new->dirty_bitmap)
 			kvm_destroy_dirty_bitmap(old);
-
-		/*
-		 * Unbind the guest_memfd instance as needed; the @new slot has
-		 * already created its own binding.  TODO: Drop the WARN when
-		 * dirty logging guest_memfd memslots is supported.  Until then,
-		 * flags-only changes on guest_memfd slots should be impossible.
-		 */
-		if (WARN_ON_ONCE(old->flags & KVM_MEM_GUEST_MEMFD))
-			kvm_gmem_unbind(old);
-
 		/*
 		 * The final quirk.  Free the detached, old slot, but only its
 		 * memory, not any metadata.  Metadata, including arch specific
@@ -2073,22 +2063,27 @@ static int kvm_set_memory_region(struct kvm *kvm,
 		if ((kvm->nr_memslot_pages + npages) < kvm->nr_memslot_pages)
 			return -EINVAL;
 	} else { /* Modify an existing slot. */
-		/* Private memslots are immutable, they can only be deleted. */
-		if (mem->flags & KVM_MEM_GUEST_MEMFD)
-			return -EINVAL;
 		if ((mem->userspace_addr != old->userspace_addr) ||
 		    (npages != old->npages) ||
 		    ((mem->flags ^ old->flags) & (KVM_MEM_READONLY | KVM_MEM_GUEST_MEMFD)))
 			return -EINVAL;
 
-		if (base_gfn != old->base_gfn)
+		if (base_gfn != old->base_gfn) {
 			change = KVM_MR_MOVE;
-		else if (mem->flags != old->flags)
+		} else if (mem->flags != (old->flags & MEMSLOT_USER_FLAGS_MASK)) {
 			change = KVM_MR_FLAGS_ONLY;
-		else /* Nothing to change. */
+		} else if (mem->flags & KVM_MEM_GUEST_MEMFD) {
+			return kvm_gmem_check_no_change(kvm, old, mem->guest_memfd,
+							mem->guest_memfd_offset);
+		} else {
 			return 0;
+		}
 	}
 
+	if (mem->flags & KVM_MEM_GUEST_MEMFD &&
+	    change != KVM_MR_CREATE && change != KVM_MR_FLAGS_ONLY)
+		return -EINVAL;
+
 	if ((change == KVM_MR_CREATE || change == KVM_MR_MOVE) &&
 	    kvm_check_memslot_overlap(slots, id, base_gfn, base_gfn + npages))
 		return -EEXIST;
@@ -2105,7 +2100,12 @@ static int kvm_set_memory_region(struct kvm *kvm,
 	new->flags = mem->flags;
 	new->userspace_addr = mem->userspace_addr;
 	if (mem->flags & KVM_MEM_GUEST_MEMFD) {
-		r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
+		if (change == KVM_MR_CREATE) {
+			r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
+		} else if (change == KVM_MR_FLAGS_ONLY) {
+			r = kvm_gmem_change_flags(kvm, old, new, mem->guest_memfd,
+						  mem->guest_memfd_offset);
+		}
 		if (r)
 			goto out;
 	}
@@ -2117,7 +2117,7 @@ static int kvm_set_memory_region(struct kvm *kvm,
 	return 0;
 
 out_unbind:
-	if (mem->flags & KVM_MEM_GUEST_MEMFD)
+	if ((mem->flags & KVM_MEM_GUEST_MEMFD) && change == KVM_MR_CREATE)
 		kvm_gmem_unbind(new);
 out:
 	kfree(new);
diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h
index 7510ca915dd1..c58dfeb0f3df 100644
--- a/virt/kvm/kvm_mm.h
+++ b/virt/kvm/kvm_mm.h
@@ -77,6 +77,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args);
 int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
 		  unsigned int fd, uoff_t offset);
 void kvm_gmem_unbind(struct kvm_memory_slot *slot);
+int kvm_gmem_change_flags(struct kvm *kvm, struct kvm_memory_slot *old,
+			  struct kvm_memory_slot *new, unsigned int fd,
+			  loff_t offset);
 #else
 static inline int kvm_gmem_init(struct module *module)
 {
@@ -95,6 +98,14 @@ static inline void kvm_gmem_unbind(struct kvm_memory_slot *slot)
 {
 	WARN_ON_ONCE(1);
 }
+static inline int kvm_gmem_change_flags(struct kvm *kvm,
+					struct kvm_memory_slot *old,
+					struct kvm_memory_slot *new,
+					unsigned int fd, loff_t offset)
+{
+	WARN_ON_ONCE(1);
+	return -EIO;
+}
 #endif /* CONFIG_KVM_GUEST_MEMFD */
 
 #endif /* __KVM_MM_H__ */
-- 
2.43.0

[RFC PATCH 3/3] KVM: arm64: Allow dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

Everything is in place to allow dirty page logging for guest_memfd-only
memslots, advertise it to userspace.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/arm.c | 22 ++++++++++++++++++++++
 arch/arm64/kvm/mmu.c |  3 ++-
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 50adfff75be8..e92e5d7b533a 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -393,6 +393,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_COUNTER_OFFSET:
 	case KVM_CAP_ARM_WRITABLE_IMP_ID_REGS:
 	case KVM_CAP_ARM_SEA_TO_USER:
+#ifdef CONFIG_KVM_GUEST_MEMFD
+	case KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES:
+#endif
 		r = 1;
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
@@ -493,6 +496,25 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	return r;
 }
 
+#ifdef CONFIG_KVM_GUEST_MEMFD
+bool kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm)
+{
+	/*
+	 * Protected pKVM VMs don't allow dirty page logging, fail early here
+	 * instead of in kvm_arch_prepare_memory_region().
+	 *
+	 * KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES is not available for a
+	 * protected pKVM VM, and returning false means that
+	 * KVM_SET_USER_MEMORY_REGION2 fails with EINVAL, which is consistent
+	 * with unsupported memslot flags.
+	 */
+	if (!is_protected_kvm_enabled())
+		return true;
+
+	return kvm_pkvm_ext_allowed(kvm, KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES);
+}
+#endif
+
 long kvm_arch_dev_ioctl(struct file *filp,
 			unsigned int ioctl, unsigned long arg)
 {
diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 6c941aaa10c6..f6e6de153ce2 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1648,7 +1648,8 @@ static int gmem_abort(const struct kvm_s2_fault_desc *s2fd)
 		return ret;
 	}
 
-	if (!(s2fd->memslot->flags & KVM_MEM_READONLY))
+	if (!(s2fd->memslot->flags & KVM_MEM_READONLY) &&
+	    (!kvm_slot_dirty_track_enabled(s2fd->memslot) || write_fault))
 		prot |= KVM_PGTABLE_PROT_W;
 
 	if (s2fd->nested)
-- 
2.43.0

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[David Hildenbrand]

On 7/2/26 16:29, Alexandru Elisei wrote:
> To enable memslot operations, KVM maintains two arrays of memslots, and an
> RCU pointer to the active (in use) array. Changes are made first to the
> inactive array, and the RCU pointer is updated to point to the inactive
> array, which becomes active.
> 
> The guest_memfd file maintains an xarray of pointers to memslots that use
> it as the memory provider. After the RCU pointer to the active memslots is
> updated and until SRCU is synchronized, readers can observe the old or the
> new value for the active array, and therefore the old or the new pointer
> for a given memslot.  For memslot creation or deletion that is not an issue
> for guest_memfd, as readers will either read the same memslot pointer saved
> by the guest_memfd file, or a non-existing memslot.
> 
> But when changing the flags for a memslot, readers can read two different
> and non-NULL memslot pointers. Since there is no easy way to ensure that
> the memslot pointer that the guest_memfd stores is consistent with both
> views at the same time, modify how the guest_memfd file keeps track of the
> associated memslots: instead of storing the pointer directly, store the
> memslot id and address space id (as_id), and use that to reach the memslot
> in the active list of memslots.
> 
> This only changes how guest_memfd keeps track of memslots, userspace is not
> allowed to make changes to a memslot yet.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  virt/kvm/guest_memfd.c | 95 +++++++++++++++++++++++++++++++++++-------
>  1 file changed, 80 insertions(+), 15 deletions(-)
> 
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index db57c5766ab6..43ef8e908aaf 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -25,6 +25,7 @@ struct gmem_file {
>  	struct kvm *kvm;
>  	struct xarray bindings;
>  	struct list_head entry;
> +	bool found_memslot;	/* Used for balancing invalidations when punching a hole */

Probabably best to document what it means, not only what it is used for (and
maybe document above the member if you end up with more text).

>  };
>  
>  struct gmem_inode {
> @@ -43,6 +44,29 @@ static __always_inline struct gmem_inode *GMEM_I(struct inode *inode)
>  #define kvm_gmem_for_each_file(f, inode) \
>  	list_for_each_entry(f, &GMEM_I(inode)->gmem_file_list, entry)
>  
> +static void *memslot_to_xa_value(struct kvm_memory_slot *slot)
> +{
> +	WARN_ON_ONCE(sizeof(slot->as_id) > 16);
> +	WARN_ON_ONCE(sizeof(slot->id) > 16);
> +	WARN_ON_ONCE(sizeof(slot->as_id) + sizeof(slot->id) > sizeof(unsigned long));

These can just be BUILD_BUG_ON() I suppose.

> +
> +	return xa_mk_value(((unsigned long)slot->as_id) << 16 | (unsigned long)slot->id);

The latter "(unsigned long)" should not be required.

> +}
> +
> +static struct kvm_memory_slot *xa_value_to_memslot(struct kvm *kvm, const void *entry)
> +{

The following can all be const.

> +	unsigned long full_id = xa_to_value(entry);
> +	u16 as_id = (full_id >> 16) & U16_MAX;

Why the "& U16_MAX" here?

(1) It's an u16

(2) memslot_to_xa_value() never stores anything in there.

> +	short id = full_id & U16_MAX;

Same here. And I wonder why you are not also using an u16 here.

> +
> +	/*
> +	 * Do not ignore KVM_MEMSLOT_INVALID memslots, as we want
> +	 * ->error_remove_folio(), when it races with memslot deletion, to have
> +	 *  unmapped the memory upon completion.
> +	 */
> +	return id_to_memslot(__kvm_memslots(kvm, as_id), id);
> +}
> +
>  /**
>   * folio_file_pfn - like folio_file_page, but return a pfn.
>   * @folio: The folio which contains this index.
> @@ -157,7 +181,7 @@ static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *in
>  	return KVM_FILTER_PRIVATE;
>  }
>  
> -static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
> +static bool __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
>  					pgoff_t end,
>  					enum kvm_gfn_range_filter attr_filter)
>  {
> @@ -165,9 +189,15 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
>  	struct kvm_memory_slot *slot;
>  	struct kvm *kvm = f->kvm;
>  	unsigned long index;
> +	void *entry;
> +
> +	xa_for_each_range(&f->bindings, index, entry, start, end - 1) {
> +		pgoff_t pgoff;
>  
> -	xa_for_each_range(&f->bindings, index, slot, start, end - 1) {
> -		pgoff_t pgoff = slot->gmem.pgoff;
> +		slot = xa_value_to_memslot(kvm, entry);

That now gets more expensive. id_to_memslot() uses a hashtable, but there is
certainly more pointer chasing going on now.

> +		if (!slot)
> +			continue;
> +		pgoff = slot->gmem.pgoff;
>  
>  		struct kvm_gfn_range gfn_range = {
>  			.start = slot->base_gfn + max(pgoff, start) - pgoff,
> @@ -192,6 +222,8 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
>  
>  	if (found_memslot)
>  		KVM_MMU_UNLOCK(kvm);
> +
> +	return found_memslot;
>  }
-- 
Cheers,

David

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Alexandru Elisei]

Hi David,

Thanks for having a look.

On Mon, Jul 06, 2026 at 09:14:59AM +0200, David Hildenbrand wrote:
> On 7/2/26 16:29, Alexandru Elisei wrote:
> > To enable memslot operations, KVM maintains two arrays of memslots, and an
> > RCU pointer to the active (in use) array. Changes are made first to the
> > inactive array, and the RCU pointer is updated to point to the inactive
> > array, which becomes active.
> > 
> > The guest_memfd file maintains an xarray of pointers to memslots that use
> > it as the memory provider. After the RCU pointer to the active memslots is
> > updated and until SRCU is synchronized, readers can observe the old or the
> > new value for the active array, and therefore the old or the new pointer
> > for a given memslot.  For memslot creation or deletion that is not an issue
> > for guest_memfd, as readers will either read the same memslot pointer saved
> > by the guest_memfd file, or a non-existing memslot.
> > 
> > But when changing the flags for a memslot, readers can read two different
> > and non-NULL memslot pointers. Since there is no easy way to ensure that
> > the memslot pointer that the guest_memfd stores is consistent with both
> > views at the same time, modify how the guest_memfd file keeps track of the
> > associated memslots: instead of storing the pointer directly, store the
> > memslot id and address space id (as_id), and use that to reach the memslot
> > in the active list of memslots.
> > 
> > This only changes how guest_memfd keeps track of memslots, userspace is not
> > allowed to make changes to a memslot yet.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  virt/kvm/guest_memfd.c | 95 +++++++++++++++++++++++++++++++++++-------
> >  1 file changed, 80 insertions(+), 15 deletions(-)
> > 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index db57c5766ab6..43ef8e908aaf 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -25,6 +25,7 @@ struct gmem_file {
> >  	struct kvm *kvm;
> >  	struct xarray bindings;
> >  	struct list_head entry;
> > +	bool found_memslot;	/* Used for balancing invalidations when punching a hole */
> 
> Probabably best to document what it means, not only what it is used for (and
> maybe document above the member if you end up with more text).

Sure, this is how I changed it:

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 210bdd76f0aa..3cee64047bce 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -25,7 +25,13 @@ struct gmem_file {
        struct kvm *kvm;
        struct xarray bindings;
        struct list_head entry;
-       bool found_memslot;     /* Used for balancing invalidations when punching a hole */
+       /*
+        * Keeps track of whether memory has been unmapped during secondary MMU
+        * invalidation, to keep the invalidate start and end calls balanced.
+        *
+        * Accessed while holding the inode->i_mapping lock in exclusive mode.
+        */
+       bool memslot_invalidated;
 };

Hopefully the new name is better.

> 
> >  };
> >  
> >  struct gmem_inode {
> > @@ -43,6 +44,29 @@ static __always_inline struct gmem_inode *GMEM_I(struct inode *inode)
> >  #define kvm_gmem_for_each_file(f, inode) \
> >  	list_for_each_entry(f, &GMEM_I(inode)->gmem_file_list, entry)
> >  
> > +static void *memslot_to_xa_value(struct kvm_memory_slot *slot)
> > +{
> > +	WARN_ON_ONCE(sizeof(slot->as_id) > 16);
> > +	WARN_ON_ONCE(sizeof(slot->id) > 16);
> > +	WARN_ON_ONCE(sizeof(slot->as_id) + sizeof(slot->id) > sizeof(unsigned long));
> 
> These can just be BUILD_BUG_ON() I suppose.

That's a good idea, will do.

> 
> > +
> > +	return xa_mk_value(((unsigned long)slot->as_id) << 16 | (unsigned long)slot->id);
> 
> The latter "(unsigned long)" should not be required.

Indeed.

> 
> > +}
> > +
> > +static struct kvm_memory_slot *xa_value_to_memslot(struct kvm *kvm, const void *entry)
> > +{
> 
> The following can all be const.
> 
> > +	unsigned long full_id = xa_to_value(entry);
> > +	u16 as_id = (full_id >> 16) & U16_MAX;
> 
> Why the "& U16_MAX" here?
> 
> (1) It's an u16
> 
> (2) memslot_to_xa_value() never stores anything in there.
> 
> > +	short id = full_id & U16_MAX;
> 
> Same here. And I wonder why you are not also using an u16 here.

I wanted to make it clear that everything should fit in a u16, I think I
wrote this code before the WARN_ON_ONCE in memslot_to_xa_value() above and
I forgot to change it.

I will clean it up.

> 
> > +
> > +	/*
> > +	 * Do not ignore KVM_MEMSLOT_INVALID memslots, as we want
> > +	 * ->error_remove_folio(), when it races with memslot deletion, to have
> > +	 *  unmapped the memory upon completion.
> > +	 */
> > +	return id_to_memslot(__kvm_memslots(kvm, as_id), id);
> > +}
> > +
> >  /**
> >   * folio_file_pfn - like folio_file_page, but return a pfn.
> >   * @folio: The folio which contains this index.
> > @@ -157,7 +181,7 @@ static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *in
> >  	return KVM_FILTER_PRIVATE;
> >  }
> >  
> > -static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
> > +static bool __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
> >  					pgoff_t end,
> >  					enum kvm_gfn_range_filter attr_filter)
> >  {
> > @@ -165,9 +189,15 @@ static void __kvm_gmem_invalidate_start(struct gmem_file *f, pgoff_t start,
> >  	struct kvm_memory_slot *slot;
> >  	struct kvm *kvm = f->kvm;
> >  	unsigned long index;
> > +	void *entry;
> > +
> > +	xa_for_each_range(&f->bindings, index, entry, start, end - 1) {
> > +		pgoff_t pgoff;
> >  
> > -	xa_for_each_range(&f->bindings, index, slot, start, end - 1) {
> > -		pgoff_t pgoff = slot->gmem.pgoff;
> > +		slot = xa_value_to_memslot(kvm, entry);
> 
> That now gets more expensive. id_to_memslot() uses a hashtable, but there is
> certainly more pointer chasing going on now.

Yes, a hashtable search, plus a few more pointer dereferences and an
srcu_read_lock()/unlock() pair is more expensive than accessing the memslot
pointer directly from f->bindings.

Depending on the range, kvm_mmu_unmap_gfn_range() can be quite expensive.
gfn_range->may_block is set, this communicates to the arch code that they
can resched periodically if the range is too big (at least, that's what it
means on arm64, which sleeps every THP sized address range, chosen
empirically as to stop RCU from complaining). Plus, there's also the TLB
flush at the end. My gut feeling is that the extra overhead from how the
memslot is found is insignificant in this case, but I don't have the
numbers to back it up. I can try to get some numbers, if you think that
would be useful?

But that's when there's actually a range to unmap from the secondary MMU.
Let's say there's no range. The invalidations are performed when:

1. The file is closed.
2. Userspace punches a hole in the file.
3. Memory error.

I'm going to assume that the memory error happens very rarely, so it's not
worth to optimize for it.

Userspace closes the file when the VM is being destroyed, does it matter if
it's slightly slower, considering how many other things are usually
unmapped and/or freed during VM teardown? I don't know the answer to that.

As for userspace punching a hole. Two situations here:

(a) No memslots are using the file. I think it's unlikely for userspace to
punch a hole in a file when it's not being used by any VMs (am I wrong?)

(b) There are memslots using the file, but the memory hasn't been mapped
yet by a secondary MMU. In that case, the arch code will still walk the
secondary MMU page tables. Considering that punching a hole also requires a
transition from userspace to the kernel and back, I don't think that the
overhead will be noticeable.

What do you think? Do you think my analysis is correct or have I missed
something?

I'm very willing to consider another approach if you think we can do it
better.

Thanks,
Alex

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Sean Christopherson]

On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> To enable memslot operations, KVM maintains two arrays of memslots, and an
> RCU pointer to the active (in use) array. Changes are made first to the
> inactive array, and the RCU pointer is updated to point to the inactive
> array, which becomes active.
> 
> The guest_memfd file maintains an xarray of pointers to memslots that use
> it as the memory provider. After the RCU pointer to the active memslots is
> updated and until SRCU is synchronized, readers can observe the old or the
> new value for the active array, and therefore the old or the new pointer
> for a given memslot.  For memslot creation or deletion that is not an issue
> for guest_memfd, as readers will either read the same memslot pointer saved
> by the guest_memfd file, or a non-existing memslot.
> 
> But when changing the flags for a memslot, readers can read two different
> and non-NULL memslot pointers. 

And?  Why does that matter?  KVM memslot updates aren't atomic.  Practically
speaking, they _can't_ be made atomic.  Userspace is required to quiesce all
activity that must not observe inconsistent state, i.e. userspace must pause
(stop running) vCPUs when performing a memslot update.

> Since there is no easy way to ensure that the memslot pointer that the
> guest_memfd stores is consistent with both views at the same time, modify how
> the guest_memfd file keeps track of the associated memslots: instead of
> storing the pointer directly, store the memslot id and address space id
> (as_id), and use that to reach the memslot in the active list of memslots.

I don't see how this changes anything.  Readers can still see the old or new
memslot depending on when kvm->memslots[] is derefenced.

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Sean Christopherson]

On Mon, Jul 06, 2026, Alexandru Elisei wrote:
> On Mon, Jul 06, 2026 at 09:14:59AM +0200, David Hildenbrand wrote:
> > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > > index db57c5766ab6..43ef8e908aaf 100644
> > > --- a/virt/kvm/guest_memfd.c
> > > +++ b/virt/kvm/guest_memfd.c
> > > @@ -25,6 +25,7 @@ struct gmem_file {
> > >  	struct kvm *kvm;
> > >  	struct xarray bindings;
> > >  	struct list_head entry;
> > > +	bool found_memslot;	/* Used for balancing invalidations when punching a hole */
> > 
> > Probabably best to document what it means, not only what it is used for (and
> > maybe document above the member if you end up with more text).
> 
> Sure, this is how I changed it:
> 
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 210bdd76f0aa..3cee64047bce 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -25,7 +25,13 @@ struct gmem_file {
>         struct kvm *kvm;
>         struct xarray bindings;
>         struct list_head entry;
> -       bool found_memslot;     /* Used for balancing invalidations when punching a hole */
> +       /*
> +        * Keeps track of whether memory has been unmapped during secondary MMU
> +        * invalidation, to keep the invalidate start and end calls balanced.
> +        *
> +        * Accessed while holding the inode->i_mapping lock in exclusive mode.
> +        */
> +       bool memslot_invalidated;
>  };
> 
> Hopefully the new name is better.

Why add a boolean?  It's not _needed_ to balance updates, or rather it shouldn't
be needed.  filemap_invalidate_lock() prevents modifying bindings while an
invalidation is in-progress.  It might be a performation optimization, but if so,
it's a premature one and belongs in a separate patch.

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Sean Christopherson]

On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> The memory represented by guest_memfd-only memslots
> (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> freely mmap it and access it. The only thing that is preventing dirty page
> logging for such memslots is that KVM doesn't allow slots backed by
> guest_memfd to have their flags changed; they can only be created and
> deleted.

Please (publicly) document *why*  you want to add dirty-logging support.  It's
all but impossible to review new uAPI without knowing the use case.

Re: [RFC PATCH 2/3] KVM: Implement dirty page logging for guest_memfd-only memslots

[Sean Christopherson]

On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> The entire memory represented by guest_memfd-only memslot is shared and
> accessible by userspace.

...

> +8.48 KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES
> +---------------------------------------------
> +
> +:Architectures: all
> +
> +The presence of this capability indicates that memslots backed by a guest_memfd
> +file descriptor created with the GUEST_MEMFD_FLAG_MMAP flag can have dirty
> +page logging enabled.

What does mmap() have to do with anything?  Supporting mmap() doesn't guarantee
the memory is shared, and I can't think of any dependency on memory actually
being mapped into userspace.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 43ef8e908aaf..210bdd76f0aa 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -622,6 +622,11 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
>  	return true;
>  }
>  
> +bool __weak kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm)
> +{
> +	return false;
> +}
> +
>  static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
>  {
>  	static const char *name = "[kvm-gmem]";
> @@ -705,6 +710,66 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
>  	return __kvm_gmem_create(kvm, size, flags);
>  }
>  
> +static int __kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *old,
> +				      struct file *old_file, unsigned int fd,
> +				      loff_t offset)
> +{
> +	struct file *new_file;
> +
> +	new_file = fget(fd);
> +	if (!new_file)
> +		return -EBADF;
> +	if (new_file != old_file) {

There's a TOCTOU issue here, no?  Nothing prevents userspace from deleting and
replacing the old guest_memfd instance between now and when the re-binding
happens.  Ah, no, because the check in kvm_set_memory_region() is only to check
for a "nop" update.  I think we should continue to disallow such "updates", I
can't think of any reasonable use case, and then we can fold this helper into
its sole remaining caller. 

> +		fput(new_file);
> +		return -EBADF;
> +	}
> +	fput(new_file);
> +
> +	if (old->gmem.pgoff != offset >> PAGE_SHIFT)

This can and should be handled in common KVM, not in guest_memfd.  It's a
property of the memslot, not of the gmem instance.

> +		return -EINVAL;
> +
> +	return 0;
> +}
> +int kvm_gmem_change_flags(struct kvm *kvm, struct kvm_memory_slot *old,

Hmm, if we use a separate helper, I think this should be phrased in terms of
commands to guest_memfd, not in terms of why common KVM is making changes.
guest_memfd shouldn't have to care *why* it's being asked to re-bind to a
different memslot.

Alternatively, provide kvm_gmem_commit_memory_region() and pass in a
kvm_mr_change param, but that gets weird since the unbind() case needs to be
handled even without an explicit DELETE.

> @@ -734,6 +799,11 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
>  		goto err;
>  
> +	if (slot->flags & KVM_MEM_LOG_DIRTY_PAGES &&
> +	    (!kvm_gmem_supports_mmap(inode) ||
> +	     !kvm_arch_supports_gmem_mmap_dirty_logging(kvm)))

I think I would rather handle this in kvm_arch_prepare_memory_region().  AFAIK,
arm64 and x86 are the only architectures that support using gmem for private
memory, and so are the only architectures that would need to restrict dirty
logging (TDX needs additional plumbiong).  It'd mean updating x86 at the same
time, but that should be relatively straighforward.

That would mean we couldn't handle the check in check_memory_region_flags(), but
that should be a non-issue, e.g. arm64 and RISC-V already put additional
restrictions on what can regions be dirty-logged.

> @@ -1739,16 +1739,6 @@ static void kvm_commit_memory_region(struct kvm *kvm,
>  		 */
>  		if (old->dirty_bitmap && !new->dirty_bitmap)
>  			kvm_destroy_dirty_bitmap(old);
> -
> -		/*
> -		 * Unbind the guest_memfd instance as needed; the @new slot has
> -		 * already created its own binding.  TODO: Drop the WARN when
> -		 * dirty logging guest_memfd memslots is supported.  Until then,
> -		 * flags-only changes on guest_memfd slots should be impossible.
> -		 */
> -		if (WARN_ON_ONCE(old->flags & KVM_MEM_GUEST_MEMFD))
> -			kvm_gmem_unbind(old);
> -
>  		/*
>  		 * The final quirk.  Free the detached, old slot, but only its
>  		 * memory, not any metadata.  Metadata, including arch specific
> @@ -2073,22 +2063,27 @@ static int kvm_set_memory_region(struct kvm *kvm,
>  		if ((kvm->nr_memslot_pages + npages) < kvm->nr_memslot_pages)
>  			return -EINVAL;
>  	} else { /* Modify an existing slot. */
> -		/* Private memslots are immutable, they can only be deleted. */
> -		if (mem->flags & KVM_MEM_GUEST_MEMFD)
> -			return -EINVAL;
>  		if ((mem->userspace_addr != old->userspace_addr) ||
>  		    (npages != old->npages) ||
>  		    ((mem->flags ^ old->flags) & (KVM_MEM_READONLY | KVM_MEM_GUEST_MEMFD)))
>  			return -EINVAL;
>  
> -		if (base_gfn != old->base_gfn)
> +		if (base_gfn != old->base_gfn) {
>  			change = KVM_MR_MOVE;
> -		else if (mem->flags != old->flags)
> +		} else if (mem->flags != (old->flags & MEMSLOT_USER_FLAGS_MASK)) {
>  			change = KVM_MR_FLAGS_ONLY;
> -		else /* Nothing to change. */
> +		} else if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> +			return kvm_gmem_check_no_change(kvm, old, mem->guest_memfd,
> +							mem->guest_memfd_offset);

As above, just return -EINVAL.

> +		} else {
>  			return 0;
> +		}
>  	}
>  
> +	if (mem->flags & KVM_MEM_GUEST_MEMFD &&
> +	    change != KVM_MR_CREATE && change != KVM_MR_FLAGS_ONLY)

This is a *very* convoluted way of disallowing MOVE.  Handle this above.  E.g.o

		if (base_gfn != old->base_gfn) {
			/* KVM doesn't support moving guest_memfd bindings. */
			if (mem->flags & KVM_MEM_GUEST_MEMFD)
				return -EINVAL;

			change = KVM_MR_MOVE;
		} else if (mem->flags != (old->flags & MEMSLOT_USER_FLAGS_MASK)) {
			if (mem->flags & KVM_MEM_GUEST_MEMFD &&
			    old->gmem.pgoff != mem->guest_memfd_offset)
			change = KVM_MR_FLAGS_ONLY;
		} else if (mem->flags & KVM_MEM_GUEST_MEMFD) {
			return -EINVAL;
		} else {
			return 0;
		}

> +		return -EINVAL;
> +
>  	if ((change == KVM_MR_CREATE || change == KVM_MR_MOVE) &&
>  	    kvm_check_memslot_overlap(slots, id, base_gfn, base_gfn + npages))
>  		return -EEXIST;
> @@ -2105,7 +2100,12 @@ static int kvm_set_memory_region(struct kvm *kvm,
>  	new->flags = mem->flags;
>  	new->userspace_addr = mem->userspace_addr;
>  	if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> -		r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
> +		if (change == KVM_MR_CREATE) {

Curly braces aren't needed.

> +			r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
> +		} else if (change == KVM_MR_FLAGS_ONLY) {
> +			r = kvm_gmem_change_flags(kvm, old, new, mem->guest_memfd,
> +						  mem->guest_memfd_offset);
> +		}
>  		if (r)
>  			goto out;
>  	}
> @@ -2117,7 +2117,7 @@ static int kvm_set_memory_region(struct kvm *kvm,
>  	return 0;
>  
>  out_unbind:
> -	if (mem->flags & KVM_MEM_GUEST_MEMFD)
> +	if ((mem->flags & KVM_MEM_GUEST_MEMFD) && change == KVM_MR_CREATE)
>  		kvm_gmem_unbind(new);

This is wrong.  If kvm_set_memslot() failed, the old memslot needs to be bound
back to the guest_memfd instance.  Hmm, but KVM can't guarantee success.  So
unless there's reason why the bind() can't happen under slots_arch_lock, I think
the way to handle this is to only bind once success is guaranteed.  It'll require
plumbing the fd+offset into kvm_set_memslot().

Or I guess add the "fd" to kvm_memory_slot.gmem?  I kinda like that, because then
we can require that userspace really is just updating flags, and not switching
the fd (to a the same file).


E.g. something like this?

diff --git include/linux/kvm_host.h include/linux/kvm_host.h
index ab8cfaec82d3..82385eb9a82e 100644
--- include/linux/kvm_host.h
+++ include/linux/kvm_host.h
@@ -610,6 +610,7 @@ struct kvm_memory_slot {
                 * reference via kvm_gmem_get_file() is protected by
                 * either kvm->slots_lock or kvm->srcu.
                 */
+               int fd;
                struct file *file;
                pgoff_t pgoff;
        } gmem;
diff --git virt/kvm/kvm_main.c virt/kvm/kvm_main.c
index e44c20c04961..0729e7c94816 100644
--- virt/kvm/kvm_main.c
+++ virt/kvm/kvm_main.c
@@ -1946,6 +1946,20 @@ static int kvm_set_memslot(struct kvm *kvm,
                return r;
        }
 
+       if (new->flags & KVM_MEM_GUEST_MEMFD) {
+               if (change == KVM_MR_CREATE)
+                       r = kvm_gmem_bind(...);
+               else if (WARN_ON_ONCE(change == KVM_MR_MOVE))
+                       r = -EINVAL;
+               else if (change == KVM_MR_FLAGS_ONLY)
+                       r = kvm_gmem_rebind(...);
+
+               if (r) {
+                       mutex_unlock(&kvm->slots_arch_lock);
+                       return r;
+               }
+       }
+
        /*
         * For DELETE and MOVE, the working slot is now active as the INVALID
         * version of the old slot.  MOVE is particularly special as it reuses
@@ -2104,21 +2118,14 @@ static int kvm_set_memory_region(struct kvm *kvm,
        new->npages = npages;
        new->flags = mem->flags;
        new->userspace_addr = mem->userspace_addr;
-       if (mem->flags & KVM_MEM_GUEST_MEMFD) {
-               r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
-               if (r)
-                       goto out;
-       }
+       new->gmem.fd = mem->guest_memfd;
+       new->gmem.pgoff = mem->guest_memfd_offset >> PAGE_SHIFT;
 
        r = kvm_set_memslot(kvm, old, new, change);
        if (r)
-               goto out_unbind;
+               goto out;
 
        return 0;
-
-out_unbind:
-       if (mem->flags & KVM_MEM_GUEST_MEMFD)
-               kvm_gmem_unbind(new);
 out:
        kfree(new);
        return r;

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

Hi Sean,

On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > The memory represented by guest_memfd-only memslots
> > (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> > freely mmap it and access it. The only thing that is preventing dirty page
> > logging for such memslots is that KVM doesn't allow slots backed by
> > guest_memfd to have their flags changed; they can only be created and
> > deleted.
> 
> Please (publicly) document *why*  you want to add dirty-logging support.  It's
> all but impossible to review new uAPI without knowing the use case.

Of course, my mistake, I was so deep in this that I didn't realise that
there might be different perspectives.

My thinking was that since guest_memfd created with GUEST_MEMFD_FLAG_MMAP +
GUEST_MEMFD_FLAG_INIT_SHARED is extremely similar from a userspace point of
view to using an anonymous file (created with memfd_create()), that
supporting dirty page logging and migration would be a natural next step
and would expand the usefulness of guest_memfd. It has nothing to do with
confidential compute.

As to why I'm working on it now, it's because of an arm64 feature that
requires that memory remains mapped at stage 2, called Statistical
Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
the feature to a guest requires that memory remains mapped at stage 2
outside of userspace explicitely unmapping it, and guest_memfd, with the
patch to ignore the MMU notifiers [1], has this property.  I wanted to
expand the functionality of guest_memfd to support migration of virtual
machines when that arm64 feature is exposed to guests.

[1] https://lore.kernel.org/kvmarm/20260625130902.258331-1-alexandru.elisei@arm.com/

Thanks,
Alex

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Alexandru Elisei]

Hi Sean,

On Mon, Jul 06, 2026 at 02:43:23PM -0700, Sean Christopherson wrote:
> On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > To enable memslot operations, KVM maintains two arrays of memslots, and an
> > RCU pointer to the active (in use) array. Changes are made first to the
> > inactive array, and the RCU pointer is updated to point to the inactive
> > array, which becomes active.
> > 
> > The guest_memfd file maintains an xarray of pointers to memslots that use
> > it as the memory provider. After the RCU pointer to the active memslots is
> > updated and until SRCU is synchronized, readers can observe the old or the
> > new value for the active array, and therefore the old or the new pointer
> > for a given memslot.  For memslot creation or deletion that is not an issue
> > for guest_memfd, as readers will either read the same memslot pointer saved
> > by the guest_memfd file, or a non-existing memslot.
> > 
> > But when changing the flags for a memslot, readers can read two different
> > and non-NULL memslot pointers. 
> 
> And?  Why does that matter?  KVM memslot updates aren't atomic.  Practically
> speaking, they _can't_ be made atomic. Userspace is required to quiesce all
> activity that must not observe inconsistent state, i.e. userspace must pause
> (stop running) vCPUs when performing a memslot update.

Is that true when KVM_MEM_LOG_DIRTY_PAGES is toggled for a memslot?

As far as I can tell, KVM today tolerates VCPUs running while the
KVM_MEM_LOG_DIRTY_PAGES flags is being changed for a memslot. And by
tolarate I mean that VCPUs that are running when the flag is changed don't
return an error from KVM_RUN. If changing a memslot while VCPUs are running
were fatal, I would think that KVM would want to take vcpu->mutex for all
VCPUs to keep them from running. Or is it a case of KVM allowing userspace
to shoot themselves in the foot if they really want it?

When the KVM_MEM_LOG_DIRTY_PAGES flags is being changed, VCPUs handling a
guest fault can observe either the old memslot, with the old flags, or the
new memslot, with the flag changed, but they still continue running without
returning an error.

That's not the case for memslots backed by guest_memfd: if the memslot
observed by a VCPU during the flags update does not match to the memslot
pointer that the guest_memfd stores in f->bindings, __kvm_gmem_get_pfn()
prints a kernel warning and returns an error.

I agree that memslot updates aren't atomic and they cannot be made atomic
so I tried to come up with another scheme for keeping track of the memslots
associated with a guest_memfd file, to avoid the situation above.

> 
> > Since there is no easy way to ensure that the memslot pointer that the
> > guest_memfd stores is consistent with both views at the same time, modify how
> > the guest_memfd file keeps track of the associated memslots: instead of
> > storing the pointer directly, store the memslot id and address space id
> > (as_id), and use that to reach the memslot in the active list of memslots.
> 
> I don't see how this changes anything.  Readers can still see the old or new
> memslot depending on when kvm->memslots[] is derefenced.

Indeed, and I'm not even trying to change that. When I'm trying to avoid is
this when handling a guest fault:

slot = gfn_to_memslot(kvm, gfn);
kvm_gmem_get_pfn(kvm, slot, gfn, ..)
  xa_load(&f->bindings, kvm_gmem_get_index(slot, gfn)) != slot

Another way to avoid it would be this:

-       if (xa_load(&f->bindings, index) != slot) {
+       s = xa_load(&f->bindings, index);
+       if (s->id != slot->id || s->as_id != slot->as_id) {
                WARN_ON_ONCE(xa_load(&f->bindings, index));
                return ERR_PTR(-EIO);
        }

and to update the memslot pointer stored by guest_memfd at the same time as
the memslot being updated. Or that check could be dropped entirely, but I'm
not really sure why it's there, my assumption was that it's a sanity check
to make sure there no unexpected race somewhere that left guest_memfd's
bindings inconsistent with KVM's memslots, so I left it.

I'm sure there are yet more ways of allowing flags update to a memslot that
uses guest_memfd.

It would be extremely useful for me if I could get clarity on a few things.

* Is allowing KVM_MEM_LOG_DIRTY_PAGES to be set for a guest_memfd backed
memslot something that is worth pursuing?

* Is the current approach, of storing memslot id and as_id in f->bindings,
acceptable? Should I continue working on it?

* If it's not acceptable, what should a new approach do differently?

Thanks,
Alex

Re: [RFC PATCH 1/3] KVM: guest_memfd: Use memslot id to keep track of associated memslots

[Alexandru Elisei]

Hi Sean,

On Mon, Jul 06, 2026 at 02:46:03PM -0700, Sean Christopherson wrote:
> On Mon, Jul 06, 2026, Alexandru Elisei wrote:
> > On Mon, Jul 06, 2026 at 09:14:59AM +0200, David Hildenbrand wrote:
> > > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > > > index db57c5766ab6..43ef8e908aaf 100644
> > > > --- a/virt/kvm/guest_memfd.c
> > > > +++ b/virt/kvm/guest_memfd.c
> > > > @@ -25,6 +25,7 @@ struct gmem_file {
> > > >  	struct kvm *kvm;
> > > >  	struct xarray bindings;
> > > >  	struct list_head entry;
> > > > +	bool found_memslot;	/* Used for balancing invalidations when punching a hole */
> > > 
> > > Probabably best to document what it means, not only what it is used for (and
> > > maybe document above the member if you end up with more text).
> > 
> > Sure, this is how I changed it:
> > 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 210bdd76f0aa..3cee64047bce 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -25,7 +25,13 @@ struct gmem_file {
> >         struct kvm *kvm;
> >         struct xarray bindings;
> >         struct list_head entry;
> > -       bool found_memslot;     /* Used for balancing invalidations when punching a hole */
> > +       /*
> > +        * Keeps track of whether memory has been unmapped during secondary MMU
> > +        * invalidation, to keep the invalidate start and end calls balanced.
> > +        *
> > +        * Accessed while holding the inode->i_mapping lock in exclusive mode.
> > +        */
> > +       bool memslot_invalidated;
> >  };
> > 
> > Hopefully the new name is better.
> 
> Why add a boolean?  It's not _needed_ to balance updates, or rather it shouldn't
> be needed.  filemap_invalidate_lock() prevents modifying bindings while an
> invalidation is in-progress.  It might be a performation optimization, but if so,
> it's a premature one and belongs in a separate patch.

It's not about bindings being modified. This is the race I am trying to avoid:

CPU0:							CPU1:

KVM_SET_USER_MEMORY_REGION(slot, memory_size=0)
kvm_replace_memslot(kvm, slot, invalid_slot)
kvm_swap_active_memslots(kvm)
kvm_replace_memslot(kvm, invalid_slot, NULL)
kvm_swap_active_memslots(kvm)
							fallocate(guest_memfd, FALLOC_FL_PUNCH_HOLE)
							  __kvm_gmem_invalidate_start()
							    slot = id_to_memslot(__kvm_memslots(kvm, as_id), id);
							    if (slot == NULL) {
								    // Early exit, does not call kvm_mmu_invalidate_start(kvm)
								    return;
							    }
							  __kvm_gmem_invalidate_end()
							    // No corresponding kvm_mmu_invalidate_start(kvm) call
							    kvm_mmu_invalidate_end(kvm)
kvm_replace_memslot(kvm, invalid_slot, NULL);
kvm_free_memslot(kvm, slot)
  kvm_gmem_unbind(slot)

Does that make sense?

Thanks,
Alex

Re: [RFC PATCH 2/3] KVM: Implement dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

Hi Sean,

On Mon, Jul 06, 2026 at 06:29:11PM -0700, Sean Christopherson wrote:
> On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > The entire memory represented by guest_memfd-only memslot is shared and
> > accessible by userspace.
> 
> ...
> 
> > +8.48 KVM_CAP_GUEST_MEMFD_MMAP_LOG_DIRTY_PAGES
> > +---------------------------------------------
> > +
> > +:Architectures: all
> > +
> > +The presence of this capability indicates that memslots backed by a guest_memfd
> > +file descriptor created with the GUEST_MEMFD_FLAG_MMAP flag can have dirty
> > +page logging enabled.
> 
> What does mmap() have to do with anything?  Supporting mmap() doesn't guarantee
> the memory is shared, and I can't think of any dependency on memory actually
> being mapped into userspace.

My bad, it should have been GUEST_MEMFD_FLAG_MMAP +
GUEST_MEMFD_FLAG_INIT_SHARED. I'm not sure what you mean by "dependency on
memory actually being mapped into userspace".

From my point of view, it only makes sense to enable dirty page logging if
the contents of the memory is accessible to userspace, hence I made dirty
page logging depend on userspace having the option to access the memory.
This can only happen if the guest_memfd file is mmap'able and accessible by
userspace. But it doesn't force userspace to actually have the memory
mapped to allow the log dirty pages flag to be set for a guest_memfd backed
memslot. Hm.. now that I think about it, maybe I should have made depend on
guest_memfd also having been created as shared? Though I think that can be
changed with KVM_SET_MEMORY_ATTRIBUTES on x86.

Does that answer your question?

> 
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 43ef8e908aaf..210bdd76f0aa 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -622,6 +622,11 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
> >  	return true;
> >  }
> >  
> > +bool __weak kvm_arch_supports_gmem_mmap_dirty_logging(struct kvm *kvm)
> > +{
> > +	return false;
> > +}
> > +
> >  static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
> >  {
> >  	static const char *name = "[kvm-gmem]";
> > @@ -705,6 +710,66 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
> >  	return __kvm_gmem_create(kvm, size, flags);
> >  }
> >  
> > +static int __kvm_gmem_check_no_change(struct kvm *kvm, struct kvm_memory_slot *old,
> > +				      struct file *old_file, unsigned int fd,
> > +				      loff_t offset)
> > +{
> > +	struct file *new_file;
> > +
> > +	new_file = fget(fd);
> > +	if (!new_file)
> > +		return -EBADF;
> > +	if (new_file != old_file) {
> 
> There's a TOCTOU issue here, no?  Nothing prevents userspace from deleting and
> replacing the old guest_memfd instance between now and when the re-binding
> happens.

When userspace closes a guest_memfd file, for all memslots that use that
file, slot->gmem.file is set to NULL in kvm_gmem_release().
kvm_gmem_release() takes the kvm->slots_lock(), so I don't think changes to
memslots can run concurrently with a guest_memfd instance being closed -
i.e, if a guest_memfd file has been closed then the subsequent memslot
update will see slot->gmem.file = NULL. Or if a memslot update is under
way, slot->gmem.file won't be made NULL until the memslot update completes.

Unless I misunderstood what you were saying.

> Ah, no, because the check in kvm_set_memory_region() is only to check
> for a "nop" update.  I think we should continue to disallow such "updates", I

Sure, I added this here because calling the legacy
KVM_SET_USER_MEMORY_REGION with the same values for the struct
kvm_user_memory_region fields does not return an error.

> can't think of any reasonable use case, and then we can fold this helper into
> its sole remaining caller. 
> 
> > +		fput(new_file);
> > +		return -EBADF;
> > +	}
> > +	fput(new_file);
> > +
> > +	if (old->gmem.pgoff != offset >> PAGE_SHIFT)
> 
> This can and should be handled in common KVM, not in guest_memfd.  It's a
> property of the memslot, not of the gmem instance.

Sure.

> 
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +int kvm_gmem_change_flags(struct kvm *kvm, struct kvm_memory_slot *old,
> 
> Hmm, if we use a separate helper, I think this should be phrased in terms of
> commands to guest_memfd, not in terms of why common KVM is making changes.
> guest_memfd shouldn't have to care *why* it's being asked to re-bind to a
> different memslot.
> 
> Alternatively, provide kvm_gmem_commit_memory_region() and pass in a
> kvm_mr_change param, but that gets weird since the unbind() case needs to be
> handled even without an explicit DELETE.

The way I see it, guest_memfd is not rebinding to a new memslot, since
f->bindings are not changed at any point in the handling of the
KVM_SET_USER_MEMORY_REGION2 call if only the flags are changed.

The function was initially supposed to validate that this indeed is a flag
only change, which means that the guest_memfd fd and file offset are
unchanged.  But then I added the update to new->flags to set the
KVM_MEMSLOT_GMEM_ONLY flag. I guess I could split it into
kvm_gmem_check_flags_update() and kvm_gmem_copy_flags()?

Also, I think the best place for both functions would be in
kvm_set_memory_region():
* That's where the validity of a change to a memslot is checked.
* That's where new->flags is copied from kvm_userspace_memory_region2->flags.

What do you think? Does that make sense?

> 
> > @@ -734,6 +799,11 @@ int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
> >  	if (!PAGE_ALIGNED(offset) || offset + size > i_size_read(inode))
> >  		goto err;
> >  
> > +	if (slot->flags & KVM_MEM_LOG_DIRTY_PAGES &&
> > +	    (!kvm_gmem_supports_mmap(inode) ||
> > +	     !kvm_arch_supports_gmem_mmap_dirty_logging(kvm)))
> 
> I think I would rather handle this in kvm_arch_prepare_memory_region().  AFAIK,
> arm64 and x86 are the only architectures that support using gmem for private
> memory, and so are the only architectures that would need to restrict dirty
> logging (TDX needs additional plumbiong).  It'd mean updating x86 at the same
> time, but that should be relatively straighforward.

Sure, I'll give it a go.

> 
> That would mean we couldn't handle the check in check_memory_region_flags(), but
> that should be a non-issue, e.g. arm64 and RISC-V already put additional
> restrictions on what can regions be dirty-logged.
> 
> > @@ -1739,16 +1739,6 @@ static void kvm_commit_memory_region(struct kvm *kvm,
> >  		 */
> >  		if (old->dirty_bitmap && !new->dirty_bitmap)
> >  			kvm_destroy_dirty_bitmap(old);
> > -
> > -		/*
> > -		 * Unbind the guest_memfd instance as needed; the @new slot has
> > -		 * already created its own binding.  TODO: Drop the WARN when
> > -		 * dirty logging guest_memfd memslots is supported.  Until then,
> > -		 * flags-only changes on guest_memfd slots should be impossible.
> > -		 */
> > -		if (WARN_ON_ONCE(old->flags & KVM_MEM_GUEST_MEMFD))
> > -			kvm_gmem_unbind(old);
> > -
> >  		/*
> >  		 * The final quirk.  Free the detached, old slot, but only its
> >  		 * memory, not any metadata.  Metadata, including arch specific
> > @@ -2073,22 +2063,27 @@ static int kvm_set_memory_region(struct kvm *kvm,
> >  		if ((kvm->nr_memslot_pages + npages) < kvm->nr_memslot_pages)
> >  			return -EINVAL;
> >  	} else { /* Modify an existing slot. */
> > -		/* Private memslots are immutable, they can only be deleted. */
> > -		if (mem->flags & KVM_MEM_GUEST_MEMFD)
> > -			return -EINVAL;
> >  		if ((mem->userspace_addr != old->userspace_addr) ||
> >  		    (npages != old->npages) ||
> >  		    ((mem->flags ^ old->flags) & (KVM_MEM_READONLY | KVM_MEM_GUEST_MEMFD)))
> >  			return -EINVAL;
> >  
> > -		if (base_gfn != old->base_gfn)
> > +		if (base_gfn != old->base_gfn) {
> >  			change = KVM_MR_MOVE;
> > -		else if (mem->flags != old->flags)
> > +		} else if (mem->flags != (old->flags & MEMSLOT_USER_FLAGS_MASK)) {
> >  			change = KVM_MR_FLAGS_ONLY;
> > -		else /* Nothing to change. */
> > +		} else if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> > +			return kvm_gmem_check_no_change(kvm, old, mem->guest_memfd,
> > +							mem->guest_memfd_offset);
> 
> As above, just return -EINVAL.

Sure.

> 
> > +		} else {
> >  			return 0;
> > +		}
> >  	}
> >  
> > +	if (mem->flags & KVM_MEM_GUEST_MEMFD &&
> > +	    change != KVM_MR_CREATE && change != KVM_MR_FLAGS_ONLY)
> 
> This is a *very* convoluted way of disallowing MOVE.  Handle this above.  E.g.o
> 
> 		if (base_gfn != old->base_gfn) {
> 			/* KVM doesn't support moving guest_memfd bindings. */
> 			if (mem->flags & KVM_MEM_GUEST_MEMFD)
> 				return -EINVAL;
> 
> 			change = KVM_MR_MOVE;
> 		} else if (mem->flags != (old->flags & MEMSLOT_USER_FLAGS_MASK)) {
> 			if (mem->flags & KVM_MEM_GUEST_MEMFD &&
> 			    old->gmem.pgoff != mem->guest_memfd_offset)
> 			change = KVM_MR_FLAGS_ONLY;
> 		} else if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> 			return -EINVAL;
> 		} else {
> 			return 0;
> 		}

Sure, looks better.

> 
> > +		return -EINVAL;
> > +
> >  	if ((change == KVM_MR_CREATE || change == KVM_MR_MOVE) &&
> >  	    kvm_check_memslot_overlap(slots, id, base_gfn, base_gfn + npages))
> >  		return -EEXIST;
> > @@ -2105,7 +2100,12 @@ static int kvm_set_memory_region(struct kvm *kvm,
> >  	new->flags = mem->flags;
> >  	new->userspace_addr = mem->userspace_addr;
> >  	if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> > -		r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
> > +		if (change == KVM_MR_CREATE) {
> 
> Curly braces aren't needed.

Sure, I'll remove them, I put them there because the block for
KVM_MR_FLAGS_ONLY, which, even though has only an instruction, spans two
lines.

> 
> > +			r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
> > +		} else if (change == KVM_MR_FLAGS_ONLY) {
> > +			r = kvm_gmem_change_flags(kvm, old, new, mem->guest_memfd,
> > +						  mem->guest_memfd_offset);
> > +		}
> >  		if (r)
> >  			goto out;
> >  	}
> > @@ -2117,7 +2117,7 @@ static int kvm_set_memory_region(struct kvm *kvm,
> >  	return 0;
> >  
> >  out_unbind:
> > -	if (mem->flags & KVM_MEM_GUEST_MEMFD)
> > +	if ((mem->flags & KVM_MEM_GUEST_MEMFD) && change == KVM_MR_CREATE)
> >  		kvm_gmem_unbind(new);
> 
> This is wrong.  If kvm_set_memslot() failed, the old memslot needs to be bound
> back to the guest_memfd instance.  Hmm, but KVM can't guarantee success.  So

The old memslot is still bound to guest_memfd in the sense that f->bindings
still has the same memslot id+as_id assigned to the memslot's gpa range.
There's no rebinding happening when flags are changed because the memslot's
id + as_id are not changed.

I think we might not be on the same page regarding what the previous patch
does, that is where I've changed how guest_memfd keeps track of the
memslots.  Instead of storing a pointer, it now stores the memslot id and
as_id, and uses a search in the active memslots (kvm->memslots) to get the
active memslot by using id and as_id:

  slot = id_to_memslot(__kvm_memslots(kvm, as_id), id)

If userspace punches a hole or kvm_gmem_error_folio() is called while a
memslot flags update is in progress, __kvm_gmem_invalidate_{start,end}()
might observe the old memslot (with the old flags), but I believe that's
ok, because that's also what happens when the MMU notifiers trigger
invalidation during a memslot update.

> unless there's reason why the bind() can't happen under slots_arch_lock, I think
> the way to handle this is to only bind once success is guaranteed.  It'll require
> plumbing the fd+offset into kvm_set_memslot().
> 
> Or I guess add the "fd" to kvm_memory_slot.gmem?  I kinda like that, because then
> we can require that userspace really is just updating flags, and not switching
> the fd (to a the same file).
> 
> 
> E.g. something like this?
> 
> diff --git include/linux/kvm_host.h include/linux/kvm_host.h
> index ab8cfaec82d3..82385eb9a82e 100644
> --- include/linux/kvm_host.h
> +++ include/linux/kvm_host.h
> @@ -610,6 +610,7 @@ struct kvm_memory_slot {
>                  * reference via kvm_gmem_get_file() is protected by
>                  * either kvm->slots_lock or kvm->srcu.
>                  */
> +               int fd;
>                 struct file *file;
>                 pgoff_t pgoff;
>         } gmem;
> diff --git virt/kvm/kvm_main.c virt/kvm/kvm_main.c
> index e44c20c04961..0729e7c94816 100644
> --- virt/kvm/kvm_main.c
> +++ virt/kvm/kvm_main.c
> @@ -1946,6 +1946,20 @@ static int kvm_set_memslot(struct kvm *kvm,
>                 return r;
>         }
>  
> +       if (new->flags & KVM_MEM_GUEST_MEMFD) {
> +               if (change == KVM_MR_CREATE)
> +                       r = kvm_gmem_bind(...);
> +               else if (WARN_ON_ONCE(change == KVM_MR_MOVE))
> +                       r = -EINVAL;
> +               else if (change == KVM_MR_FLAGS_ONLY)
> +                       r = kvm_gmem_rebind(...);
> +
> +               if (r) {
> +                       mutex_unlock(&kvm->slots_arch_lock);
> +                       return r;
> +               }
> +       }
> +
>         /*
>          * For DELETE and MOVE, the working slot is now active as the INVALID
>          * version of the old slot.  MOVE is particularly special as it reuses
> @@ -2104,21 +2118,14 @@ static int kvm_set_memory_region(struct kvm *kvm,
>         new->npages = npages;
>         new->flags = mem->flags;
>         new->userspace_addr = mem->userspace_addr;
> -       if (mem->flags & KVM_MEM_GUEST_MEMFD) {
> -               r = kvm_gmem_bind(kvm, new, mem->guest_memfd, mem->guest_memfd_offset);
> -               if (r)
> -                       goto out;
> -       }
> +       new->gmem.fd = mem->guest_memfd;
> +       new->gmem.pgoff = mem->guest_memfd_offset >> PAGE_SHIFT;
>  
>         r = kvm_set_memslot(kvm, old, new, change);
>         if (r)
> -               goto out_unbind;
> +               goto out;
>  
>         return 0;
> -
> -out_unbind:
> -       if (mem->flags & KVM_MEM_GUEST_MEMFD)
> -               kvm_gmem_unbind(new);
>  out:
>         kfree(new);
>         return r;

I can explore that, for sure, thank you for the review!

Alex

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Sean Christopherson]

On Tue, Jul 07, 2026, Alexandru Elisei wrote:
> Hi Sean,
> 
> On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > > The memory represented by guest_memfd-only memslots
> > > (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> > > freely mmap it and access it. The only thing that is preventing dirty page
> > > logging for such memslots is that KVM doesn't allow slots backed by
> > > guest_memfd to have their flags changed; they can only be created and
> > > deleted.
> > 
> > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > all but impossible to review new uAPI without knowing the use case.
> 
> Of course, my mistake, I was so deep in this that I didn't realise that
> there might be different perspectives.
> 
> My thinking was that since guest_memfd created with GUEST_MEMFD_FLAG_MMAP +
> GUEST_MEMFD_FLAG_INIT_SHARED is extremely similar from a userspace point of
> view to using an anonymous file (created with memfd_create()), that
> supporting dirty page logging and migration would be a natural next step
> and would expand the usefulness of guest_memfd. It has nothing to do with
> confidential compute.

Sure, but just because userspace usage *might* be extremely similar, doesn't mean
the logic is technically solid.  There is simply no requirement that guest_memfd
be mmap()-able.  And that's the one of the main selling points of guest_memfd:
the memory doesn't need to be mapped into userspace in order to map it into the
guest.

> As to why I'm working on it now, it's because of an arm64 feature that
> requires that memory remains mapped at stage 2, called Statistical
> Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> the feature to a guest requires that memory remains mapped at stage 2
> outside of userspace explicitely unmapping it, and guest_memfd, with the
> patch to ignore the MMU notifiers [1], has this property.  I wanted to
> expand the functionality of guest_memfd to support migration of virtual
> machines when that arm64 feature is exposed to guests.

I'm all for adding dirty logging support for guest_memfd, but for SPE I don't
think relying on guest_memfd always being mapped is a good idea.  guest_memfd
is "pinned" purely because adding support for page migration is (very) low
priority for SNP, TDX, and pKVM.  guest_memfd page migration might play nice
with SPE?  Probably depends on whether KVM is forced to do break-before-make?

And at some point guest_memfd may support userspace-driven swap, but I suppose
we can cross that bridge when we come to it.

From a uABI perspective, forcing userspace to use guest_memfd to get access to
something like SPE isn't ideal.  While I have aspirations of using guest_memfd
much more broadly, I don't know that banking on guest_memfd replacing "everything"
is a winning strategy.

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Mark Rutland]

Hi Sean,

Ignoring dirty page logging for the moment, I think you've raised a much
bigger concern.

I've dumped a bit more context below, with a couple of high-level
questions. The important thing for Alexandru and I is whether core folk
are willing to consider some mechanism to ensure that guest PAs are
pinned writeable and never fault (even transiently).

On Tue, Jul 07, 2026 at 10:12:41AM -0700, Sean Christopherson wrote:
> On Tue, Jul 07, 2026, Alexandru Elisei wrote:
> > On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > > On Thu, Jul 02, 2026, Alexandru Elisei wrote:

> > > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > > all but impossible to review new uAPI without knowing the use case.

> > As to why I'm working on it now, it's because of an arm64 feature that
> > requires that memory remains mapped at stage 2, called Statistical
> > Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> > the feature to a guest requires that memory remains mapped at stage 2
> > outside of userspace explicitely unmapping it, and guest_memfd, with the
> > patch to ignore the MMU notifiers [1], has this property.  I wanted to
> > expand the functionality of guest_memfd to support migration of virtual
> > machines when that arm64 feature is exposed to guests.
> 
> I'm all for adding dirty logging support for guest_memfd, but for SPE I don't
> think relying on guest_memfd always being mapped is a good idea.  guest_memfd
> is "pinned" purely because adding support for page migration is (very) low
> priority for SNP, TDX, and pKVM.  guest_memfd page migration might play nice
> with SPE?  Probably depends on whether KVM is forced to do break-before-make?

The key thing for SPE is that any pages that the SPE HW is using must
have a valid writeable end-to-end (VA to real PA) mapping at all times
while the guest is running (and while the host drains buffers). If that
requirement is violated, even transiently, then data is lost and the
guest will see an unexpected fault reported by SPE.

If there's any reason we might (transiently) unmap a leaf entry (or
entire sub-tree) from the stage-2 tables, or might (transiently) remove
write permission, then we can't guarantee SPE will work correctly from
the guest's PoV.

Obviously we can't guarantee that for regular memslots backed by
userspace memory, hence we were hoping we could rely on guest_memfd.

Am I right to understand that we expect (in future) to do things with
guest_memfd that could violate that? If so (and if we're not happy to
have some options to say "always keep this pinned end-to-end no matter
what"), then I think that means that in practice we cannot virtualize
SPE correctly, and Alexandru and I need to go back to our architects.

> And at some point guest_memfd may support userspace-driven swap, but I
> suppose we can cross that bridge when we come to it.

Unfortunately, I think we need to figure out now whether it would be
acceptable to suppress that (or making it mutually exclusive with SPE),
if only to decide whether or not we continue trying to virtualize SPE.

We don't need to figure out all the details; just whether or not that
broad approach would be acceptable, or whether we have to give up on SPE
virtualization.

> From a uABI perspective, forcing userspace to use guest_memfd to get access to
> something like SPE isn't ideal.  While I have aspirations of using guest_memfd
> much more broadly, I don't know that banking on guest_memfd replacing "everything"
> is a winning strategy.

I agree this isn't ideal, and we're certainly not expecting that this is
suitable for all users. We're just trying to get as much functionality
as we practically can with today's SPE hardware.

Thanks,
Mark.

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Sean Christopherson]

On Thu, Jul 09, 2026, Mark Rutland wrote:
> Hi Sean,
> 
> Ignoring dirty page logging for the moment, I think you've raised a much
> bigger concern.
> 
> I've dumped a bit more context below, with a couple of high-level
> questions. The important thing for Alexandru and I is whether core folk
> are willing to consider some mechanism to ensure that guest PAs are
> pinned writeable and never fault (even transiently).
> 
> On Tue, Jul 07, 2026 at 10:12:41AM -0700, Sean Christopherson wrote:
> > On Tue, Jul 07, 2026, Alexandru Elisei wrote:
> > > On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > > > On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> 
> > > > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > > > all but impossible to review new uAPI without knowing the use case.
> 
> > > As to why I'm working on it now, it's because of an arm64 feature that
> > > requires that memory remains mapped at stage 2, called Statistical
> > > Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> > > the feature to a guest requires that memory remains mapped at stage 2
> > > outside of userspace explicitely unmapping it, and guest_memfd, with the
> > > patch to ignore the MMU notifiers [1], has this property.  I wanted to
> > > expand the functionality of guest_memfd to support migration of virtual
> > > machines when that arm64 feature is exposed to guests.
> > 
> > I'm all for adding dirty logging support for guest_memfd, but for SPE I don't
> > think relying on guest_memfd always being mapped is a good idea.  guest_memfd
> > is "pinned" purely because adding support for page migration is (very) low
> > priority for SNP, TDX, and pKVM.  guest_memfd page migration might play nice
> > with SPE?  Probably depends on whether KVM is forced to do break-before-make?
> 
> The key thing for SPE is that any pages that the SPE HW is using must
> have a valid writeable end-to-end (VA to real PA) mapping at all times
> while the guest is running (and while the host drains buffers). If that
> requirement is violated, even transiently, then data is lost and the
> guest will see an unexpected fault reported by SPE.

Well, at least it doesn't crash the host :-D

> If there's any reason we might (transiently) unmap a leaf entry (or
> entire sub-tree) from the stage-2 tables, or might (transiently) remove
> write permission, then we can't guarantee SPE will work correctly from
> the guest's PoV.
> 
> Obviously we can't guarantee that for regular memslots backed by
> userspace memory, hence we were hoping we could rely on guest_memfd.
> 
> Am I right to understand that we expect (in future) to do things with
> guest_memfd that could violate that? If so (and if we're not happy to
> have some options to say "always keep this pinned end-to-end no matter
> what"), 

Yes?  Page migration is the main one that I think will be problematic for arm64,
*unless* CPUs that support SPE don't require break-before-make (no idea if there
are even plans to ever drop the BBM rules).  Because with page migration, unless
KVM temporarily jails all vCPUs in the host while swizzling stage-2 PTEs, there
will be a small window of time where the memory isn't mapped.

I mention page migration because I expect swap/reclaim to be fully userspace
driven, i.e. if userspace crashes/corrupts the guest, the answer will be "well
don't do that".  But (at least some forms of) page migration will likely be
kernel-driven, e.g. to compact movable memory for THP.  And I really don't want
to give arch code the ability to hard-pin specific ranges of memory.

That said, odds are good that we'll end up with per-gmem flags to communicate to
guest_memfd whether or not the gmem instance supports page migration (x86's TDX
and SNP in particular require extra consideration).  So if the anticipated use
cases are fine with all-or-nothing "pinning", or with juggling guest_memfd files
in userspace if a more dynamic setup is desired, then you should be ok?

E.g. if the anticipated use cases are all slice-of-hardware style setups where
the VM will be statically assigned a chunk of memory, then for the most part this
will all Just Work.

> then I think that means that in practice we cannot virtualize
> SPE correctly, and Alexandru and I need to go back to our architects.
> 
> > And at some point guest_memfd may support userspace-driven swap, but I
> > suppose we can cross that bridge when we come to it.
> 
> Unfortunately, I think we need to figure out now whether it would be
> acceptable to suppress that (or making it mutually exclusive with SPE),
> if only to decide whether or not we continue trying to virtualize SPE.

Eh, as above, if userspace pulls a stupid and kills the guest, that's their
problem.  Just make sure the host isn't at risk :-)

> We don't need to figure out all the details; just whether or not that
> broad approach would be acceptable, or whether we have to give up on SPE
> virtualization.
> 
> > From a uABI perspective, forcing userspace to use guest_memfd to get access to
> > something like SPE isn't ideal.  While I have aspirations of using guest_memfd
> > much more broadly, I don't know that banking on guest_memfd replacing "everything"
> > is a winning strategy.
> 
> I agree this isn't ideal, and we're certainly not expecting that this is
> suitable for all users. We're just trying to get as much functionality
> as we practically can with today's SPE hardware.
> 
> Thanks,
> Mark.

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Oliver Upton]

On Tue, Jul 07, 2026 at 05:58:49PM +0100, Alexandru Elisei wrote:
> Hi Sean,
> 
> On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > > The memory represented by guest_memfd-only memslots
> > > (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> > > freely mmap it and access it. The only thing that is preventing dirty page
> > > logging for such memslots is that KVM doesn't allow slots backed by
> > > guest_memfd to have their flags changed; they can only be created and
> > > deleted.
> > 
> > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > all but impossible to review new uAPI without knowing the use case.
> 
> Of course, my mistake, I was so deep in this that I didn't realise that
> there might be different perspectives.
> 
> My thinking was that since guest_memfd created with GUEST_MEMFD_FLAG_MMAP +
> GUEST_MEMFD_FLAG_INIT_SHARED is extremely similar from a userspace point of
> view to using an anonymous file (created with memfd_create()), that
> supporting dirty page logging and migration would be a natural next step
> and would expand the usefulness of guest_memfd. It has nothing to do with
> confidential compute.
> 
> As to why I'm working on it now, it's because of an arm64 feature that
> requires that memory remains mapped at stage 2, called Statistical
> Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> the feature to a guest requires that memory remains mapped at stage 2
> outside of userspace explicitely unmapping it, and guest_memfd, with the
> patch to ignore the MMU notifiers [1], has this property.  I wanted to
> expand the functionality of guest_memfd to support migration of virtual
> machines when that arm64 feature is exposed to guests.

Can you please expand a bit on how you actually expect dirty tracking to
work with SPE? Taking write permission faults seem to be at odds with
"thou shalt not fault".

Seems to me like you'd need hardware dirty state and the "noabort"
flavor of BBML2 to actually get things down to page mappings. Oh, and an
SPE implementation that actually respects write permissions... Is there
even an implementation out there where the stars align?

Fixing rotten architecture would be a more appropriate starting point :-/

Thanks,
Oliver

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Mark Rutland]

Hi Sean,

On Thu, Jul 09, 2026 at 12:01:31PM -0700, Sean Christopherson wrote:
> On Thu, Jul 09, 2026, Mark Rutland wrote:
> The key thing for SPE is that any pages that the SPE HW is using must
> > have a valid writeable end-to-end (VA to real PA) mapping at all times
> > while the guest is running (and while the host drains buffers). If that
> > requirement is violated, even transiently, then data is lost and the
> > guest will see an unexpected fault reported by SPE.
> 
> Well, at least it doesn't crash the host :-D

Indeed! :)

> > If there's any reason we might (transiently) unmap a leaf entry (or
> > entire sub-tree) from the stage-2 tables, or might (transiently) remove
> > write permission, then we can't guarantee SPE will work correctly from
> > the guest's PoV.
> > 
> > Obviously we can't guarantee that for regular memslots backed by
> > userspace memory, hence we were hoping we could rely on guest_memfd.
> > 
> > Am I right to understand that we expect (in future) to do things with
> > guest_memfd that could violate that? If so (and if we're not happy to
> > have some options to say "always keep this pinned end-to-end no matter
> > what"), 
> 
> Yes?  Page migration is the main one that I think will be problematic for arm64,
> *unless* CPUs that support SPE don't require break-before-make (no idea if there
> are even plans to ever drop the BBM rules).  Because with page migration, unless
> KVM temporarily jails all vCPUs in the host while swizzling stage-2 PTEs, there
> will be a small window of time where the memory isn't mapped.

At a high level, if page migration (or any other changes to live stage-2
table entries) will be a thing for guest_memfd, then us Arm folk have to
go and work through the implications of that.

For contemporary HW with SPE, break-before-make will be required in at
least some cases. So unless we have some way to suppress migration, then
SPE virtualization on contemporary hardware is largely dead.

Going forwards, I believe that the intent is that future CPUs will
require BBM in fewer cases, but I suspect it won't disapear entirely.

> I mention page migration because I expect swap/reclaim to be fully userspace
> driven, i.e. if userspace crashes/corrupts the guest, the answer will be "well
> don't do that". 

Ah. I had assumed that if we'd consider kernel-driver migration of
guest_memfd memory, we'd also consider kernel-driven swapping of
guest_memfd memory.

I'm happy in principle with userspace being responsible for avoiding
anything problematic that is userspace-driven.

> But (at least some forms of) page migration will likely be
> kernel-driven, e.g. to compact movable memory for THP.  And I really don't want
> to give arch code the ability to hard-pin specific ranges of memory.
> 
> That said, odds are good that we'll end up with per-gmem flags to communicate to
> guest_memfd whether or not the gmem instance supports page migration (x86's TDX
> and SNP in particular require extra consideration).  So if the anticipated use
> cases are fine with all-or-nothing "pinning", or with juggling guest_memfd files
> in userspace if a more dynamic setup is desired, then you should be ok?
>
> E.g. if the anticipated use cases are all slice-of-hardware style setups where
> the VM will be statically assigned a chunk of memory, then for the most part this
> will all Just Work.

Yeah. I agree (with the caveats you mention). Arm folk need to go figure
out if it's worthwhile to support with all those caveats.

> > then I think that means that in practice we cannot virtualize
> > SPE correctly, and Alexandru and I need to go back to our architects.
> > 
> > > And at some point guest_memfd may support userspace-driven swap, but I
> > > suppose we can cross that bridge when we come to it.
> > 
> > Unfortunately, I think we need to figure out now whether it would be
> > acceptable to suppress that (or making it mutually exclusive with SPE),
> > if only to decide whether or not we continue trying to virtualize SPE.
> 
> Eh, as above, if userspace pulls a stupid and kills the guest, that's their
> problem.  Just make sure the host isn't at risk :-)

:)

FWIW, I agree from the host kernel side!

My concern is that if userspace VMM folk don't want to enforce that (or
want to but find it hard to handle correctly), then practically speaking
we're back to "SPE doesn't work", and from the guest kernel side there's
nothing we can do. As one of the SPE driver maintainers, I don't want to
be in a position where folk will shout at me because some VMM made a
mistake, and where I can't do anything to remedy the situation.

Hence, even if it's userspace's responsibility, we *might* want some way
for userspace to tell the kernel "please don't let me mess up this
specific requirement".

Thanks,
Mark.

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Alexandru Elisei]

Hi Oliver,

On Thu, Jul 09, 2026 at 01:33:30PM -0700, Oliver Upton wrote:
> On Tue, Jul 07, 2026 at 05:58:49PM +0100, Alexandru Elisei wrote:
> > Hi Sean,
> > 
> > On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > > On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > > > The memory represented by guest_memfd-only memslots
> > > > (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> > > > freely mmap it and access it. The only thing that is preventing dirty page
> > > > logging for such memslots is that KVM doesn't allow slots backed by
> > > > guest_memfd to have their flags changed; they can only be created and
> > > > deleted.
> > > 
> > > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > > all but impossible to review new uAPI without knowing the use case.
> > 
> > Of course, my mistake, I was so deep in this that I didn't realise that
> > there might be different perspectives.
> > 
> > My thinking was that since guest_memfd created with GUEST_MEMFD_FLAG_MMAP +
> > GUEST_MEMFD_FLAG_INIT_SHARED is extremely similar from a userspace point of
> > view to using an anonymous file (created with memfd_create()), that
> > supporting dirty page logging and migration would be a natural next step
> > and would expand the usefulness of guest_memfd. It has nothing to do with
> > confidential compute.
> > 
> > As to why I'm working on it now, it's because of an arm64 feature that
> > requires that memory remains mapped at stage 2, called Statistical
> > Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> > the feature to a guest requires that memory remains mapped at stage 2
> > outside of userspace explicitely unmapping it, and guest_memfd, with the
> > patch to ignore the MMU notifiers [1], has this property.  I wanted to
> > expand the functionality of guest_memfd to support migration of virtual
> > machines when that arm64 feature is exposed to guests.
> 
> Can you please expand a bit on how you actually expect dirty tracking to
> work with SPE? Taking write permission faults seem to be at odds with
> "thou shalt not fault".

For the initial posting on the mailing list, my approach was to keep things
as simple as possible: allow memory to be made read-only and then have the
host map the memory as writable at stage 2 when SPE reports a permission
fault, if the buffer happens to be enabled while dirty page logging is also
enabled. Of course, that means that the quality of the profiling data might
be degraded. I was planning to point this out, to start a discussion about
it, but we can talk about it now.

Another approach that builds on that, which I have prototyped locally, is
to have a VCPU exit to userspace when/if the buffer is enabled and there is
at least one memslot with dirty page logging enabled (with a specific
hardware_exit_reason) and then let userspace decide what to do: resume the
VCPU and live with the blackout window, or keep the VCPU stopped, and the
guest gets non-degraded profiling data as the expense of the VCPU making
progress. My hope is that if migration is fast enough, the VCPU not running
for (possibly) the entire duration of the migration process won't be
noticeable, but I might be naive in thinking that.

> 
> Seems to me like you'd need hardware dirty state and the "noabort"
> flavor of BBML2 to actually get things down to page mappings. Oh, and an
> SPE implementation that actually respects write permissions... Is there
> even an implementation out there where the stars align?

According to the Neoverse v3 errata document (MP168), r0p0 is affected,
r0p1 is not affected. A quick google search revealed that Microsoft's
Cobalt 200 CPU is based on Neoverse v3, same for the Arm AGI CPU. Wikipedia
also says that Graviton 5 is based on Neoverse v3. Don't know the exact
revision though, for either of them.

I was planning to add a kernel parameter that permits SPE on the affected
parts, so people who trust their guests* can still use it from a VM. I
don't think migration would be possible in this case because SPE bypasses
the read-only permission at stage 2, and the host would not know to mark
that memory as dirty (for dirty page logging).

* Ignoring migration, guest_memfd is always mapped as writable at stage 2,
so a guest would have to point the buffer at memory that KVM represents
with a KVM_MEM_READONLY memslot, like the CFI flash device used by EDK2.

> 
> Fixing rotten architecture would be a more appropriate starting point :-/

I like to think that the architecture is constantly improving :)

Thanks,
Alex

Re: [RFC PATCH 0/3] KVM: Dirty page logging for guest_memfd-only memslots

[Oliver Upton]

On Fri, Jul 10, 2026 at 11:44:15AM +0100, Alexandru Elisei wrote:
> Hi Oliver,
> 
> On Thu, Jul 09, 2026 at 01:33:30PM -0700, Oliver Upton wrote:
> > On Tue, Jul 07, 2026 at 05:58:49PM +0100, Alexandru Elisei wrote:
> > > Hi Sean,
> > > 
> > > On Mon, Jul 06, 2026 at 05:56:12PM -0700, Sean Christopherson wrote:
> > > > On Thu, Jul 02, 2026, Alexandru Elisei wrote:
> > > > > The memory represented by guest_memfd-only memslots
> > > > > (kvm_memslot_is_gmem_only() is true) is shared with userspace, which can
> > > > > freely mmap it and access it. The only thing that is preventing dirty page
> > > > > logging for such memslots is that KVM doesn't allow slots backed by
> > > > > guest_memfd to have their flags changed; they can only be created and
> > > > > deleted.
> > > > 
> > > > Please (publicly) document *why*  you want to add dirty-logging support.  It's
> > > > all but impossible to review new uAPI without knowing the use case.
> > > 
> > > Of course, my mistake, I was so deep in this that I didn't realise that
> > > there might be different perspectives.
> > > 
> > > My thinking was that since guest_memfd created with GUEST_MEMFD_FLAG_MMAP +
> > > GUEST_MEMFD_FLAG_INIT_SHARED is extremely similar from a userspace point of
> > > view to using an anonymous file (created with memfd_create()), that
> > > supporting dirty page logging and migration would be a natural next step
> > > and would expand the usefulness of guest_memfd. It has nothing to do with
> > > confidential compute.
> > > 
> > > As to why I'm working on it now, it's because of an arm64 feature that
> > > requires that memory remains mapped at stage 2, called Statistical
> > > Profiling Extension (SPE), similar to Intel's PEBS or AMD's IBS. Exposing
> > > the feature to a guest requires that memory remains mapped at stage 2
> > > outside of userspace explicitely unmapping it, and guest_memfd, with the
> > > patch to ignore the MMU notifiers [1], has this property.  I wanted to
> > > expand the functionality of guest_memfd to support migration of virtual
> > > machines when that arm64 feature is exposed to guests.
> > 
> > Can you please expand a bit on how you actually expect dirty tracking to
> > work with SPE? Taking write permission faults seem to be at odds with
> > "thou shalt not fault".
> 
> For the initial posting on the mailing list, my approach was to keep things
> as simple as possible: allow memory to be made read-only and then have the
> host map the memory as writable at stage 2 when SPE reports a permission
> fault, if the buffer happens to be enabled while dirty page logging is also
> enabled. Of course, that means that the quality of the profiling data might
> be degraded. I was planning to point this out, to start a discussion about
> it, but we can talk about it now.

Understanding how this is going to fit together end-to-end seems
relevant since you're asking for UAPI guarantees to enable SPE.

I agree that degrading the profile isn't that big of a deal but the buffer
may not even be in a restartable state after taking a fault. Who (KVM /
VMM) is going to handle rewinding PMBPTR in the case of an incomplete
record? Blaming userspace is always a good start but at the same time the
more rules/gotchas there are around the feature the less likely it seems a
VMM will pick this up.

Assuring that there are no KVM-initiated invalidations for guest_memfd
doesn't seem unreasonable but we need to have an aligned view on how SPE
will actually be enabled in KVM + VMM first.

> Another approach that builds on that, which I have prototyped locally, is
> to have a VCPU exit to userspace when/if the buffer is enabled and there is
> at least one memslot with dirty page logging enabled (with a specific
> hardware_exit_reason) and then let userspace decide what to do: resume the
> VCPU and live with the blackout window, or keep the VCPU stopped, and the
> guest gets non-degraded profiling data as the expense of the VCPU making
> progress.

My take is that there's no amount of idiot-proofing we can do in KVM to
prevent the VMM from messing this up somehow. We can just document the
limitations and let userspace decide to quiesce when dirty logging is
enabled.

> My hope is that if migration is fast enough, the VCPU not running
> for (possibly) the entire duration of the migration process won't be
> noticeable, but I might be naive in thinking that.

Err, for a live upgrade of the VMM or kernel, sure. But an actual copyful
migration going over network could be O(minutes).

> According to the Neoverse v3 errata document (MP168), r0p0 is affected,
> r0p1 is not affected. A quick google search revealed that Microsoft's
> Cobalt 200 CPU is based on Neoverse v3, same for the Arm AGI CPU. Wikipedia
> also says that Graviton 5 is based on Neoverse v3. Don't know the exact
> revision though, for either of them.
> 
> I was planning to add a kernel parameter that permits SPE on the affected
> parts, so people who trust their guests* can still use it from a VM. I
> don't think migration would be possible in this case because SPE bypasses
> the read-only permission at stage 2, and the host would not know to mark
> that memory as dirty (for dirty page logging).
> 
> * Ignoring migration, guest_memfd is always mapped as writable at stage 2,
> so a guest would have to point the buffer at memory that KVM represents
> with a KVM_MEM_READONLY memslot, like the CFI flash device used by EDK2.
> 
> > 
> > Fixing rotten architecture would be a more appropriate starting point :-/
> 
> I like to think that the architecture is constantly improving :)

I'd like to think that too...

Best,
Oliver