My apologies for the super late feedback. None of this is critical (mechanical things that can be cleaned up after the fact), so if there's any urgency to getting this series into 6.18, just ignore it.

On Wed, Aug 27, 2025, Ackerley Tng wrote:

Shivank Garg shivankg@amd.com writes: @@ -463,11 +502,70 @@ bool __weak kvm_arch_supports_gmem_mmap(struct kvm *kvm) return true; }

+static struct inode *kvm_gmem_inode_create(const char *name, loff_t size,

• 			   u64 flags)
  

+{

• struct inode *inode;
• inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL);
• if (IS_ERR(inode))

• return inode;
  

• inode->i_private = (void *)(unsigned long)flags;
• inode->i_op = &kvm_gmem_iops;
• inode->i_mapping->a_ops = &kvm_gmem_aops;
• inode->i_mode |= S_IFREG;
• inode->i_size = size;
• mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
• mapping_set_inaccessible(inode->i_mapping);
• /* Unmovable mappings are supposed to be marked unevictable as well. */
• WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
• return inode;

+}

+static struct file *kvm_gmem_inode_create_getfile(void *priv, loff_t size,

• 				  u64 flags)
  

+{

• static const char *name = "[kvm-gmem]";
• struct inode *inode;
• struct file *file;
• int err;
• err = -ENOENT;
• /* __fput() will take care of fops_put(). */
• if (!fops_get(&kvm_gmem_fops))

• goto err;
  

• inode = kvm_gmem_inode_create(name, size, flags);
• if (IS_ERR(inode)) {

• err = PTR_ERR(inode);
  

• goto err_fops_put;
  

• }
• file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,

• 		 &kvm_gmem_fops);
  

• if (IS_ERR(file)) {

• err = PTR_ERR(file);
  

• goto err_put_inode;
  

• }
• file->f_flags |= O_LARGEFILE;
• file->private_data = priv;
• return file;

+err_put_inode:

• iput(inode);

+err_fops_put:

• fops_put(&kvm_gmem_fops);

+err:

• return ERR_PTR(err);

+}

I don't see any reason to add two helpers. It requires quite a bit more lines of code due to adding more error paths and local variables, and IMO doesn't make the code any easier to read.

Passing in "gmem" as @priv is especially ridiculous, as it adds code and obfuscates what file->private_data is set to.

I get the sense that the code was written to be a "replacement" for common APIs, but that is nonsensical (no pun intended).

static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) {

• const char *anon_name = "[kvm-gmem]"; struct kvm_gmem *gmem;
• struct inode *inode; struct file *file; int fd, err;

@@ -481,32 +579,16 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) goto err_fd; }

• file = anon_inode_create_getfile(anon_name, &kvm_gmem_fops, gmem,

• 			 O_RDWR, NULL);
  

• file = kvm_gmem_inode_create_getfile(gmem, size, flags); if (IS_ERR(file)) { err = PTR_ERR(file); goto err_gmem; }

• file->f_flags |= O_LARGEFILE;
• inode = file->f_inode;
• WARN_ON(file->f_mapping != inode->i_mapping);
• inode->i_private = (void *)(unsigned long)flags;
• inode->i_op = &kvm_gmem_iops;
• inode->i_mapping->a_ops = &kvm_gmem_aops;
• inode->i_mode |= S_IFREG;
• inode->i_size = size;
• mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
• mapping_set_inaccessible(inode->i_mapping);
• /* Unmovable mappings are supposed to be marked unevictable as well. */
• WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
• kvm_get_kvm(kvm); gmem->kvm = kvm; xa_init(&gmem->bindings);
• list_add(&gmem->entry, &inode->i_mapping->i_private_list);

• list_add(&gmem->entry, &file_inode(file)->i_mapping->i_private_list);

I don't understand this change? Isn't file_inode(file) == inode?

Compile tested only, and again not critical, but it's -40 LoC...

--- include/uapi/linux/magic.h | 1 + virt/kvm/guest_memfd.c | 75 ++++++++++++++++++++++++++++++++------ virt/kvm/kvm_main.c | 7 +++- virt/kvm/kvm_mm.h | 9 +++-- 4 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h index bb575f3ab45e..638ca21b7a90 100644 --- a/include/uapi/linux/magic.h +++ b/include/uapi/linux/magic.h @@ -103,5 +103,6 @@ #define DEVMEM_MAGIC 0x454d444d /* "DMEM" */ #define SECRETMEM_MAGIC 0x5345434d /* "SECM" */ #define PID_FS_MAGIC 0x50494446 /* "PIDF" */ +#define GUEST_MEMFD_MAGIC 0x474d454d /* "GMEM" */

#endif /* __LINUX_MAGIC_H__ */ diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 08a6bc7d25b6..73c9791879d5 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -1,12 +1,16 @@ // SPDX-License-Identifier: GPL-2.0 +#include <linux/anon_inodes.h> #include <linux/backing-dev.h> #include <linux/falloc.h> +#include <linux/fs.h> #include <linux/kvm_host.h> +#include <linux/pseudo_fs.h> #include <linux/pagemap.h> -#include <linux/anon_inodes.h>

#include "kvm_mm.h"

+static struct vfsmount *kvm_gmem_mnt; + struct kvm_gmem { struct kvm *kvm; struct xarray bindings; @@ -385,9 +389,45 @@ static struct file_operations kvm_gmem_fops = { .fallocate = kvm_gmem_fallocate, };

-void kvm_gmem_init(struct module *module) +static int kvm_gmem_init_fs_context(struct fs_context *fc) +{ + if (!init_pseudo(fc, GUEST_MEMFD_MAGIC)) + return -ENOMEM; + + fc->s_iflags |= SB_I_NOEXEC; + fc->s_iflags |= SB_I_NODEV; + + return 0; +} + +static struct file_system_type kvm_gmem_fs = { + .name = "guest_memfd", + .init_fs_context = kvm_gmem_init_fs_context, + .kill_sb = kill_anon_super, +}; + +static int kvm_gmem_init_mount(void) +{ + kvm_gmem_mnt = kern_mount(&kvm_gmem_fs); + + if (IS_ERR(kvm_gmem_mnt)) + return PTR_ERR(kvm_gmem_mnt); + + kvm_gmem_mnt->mnt_flags |= MNT_NOEXEC; + return 0; +} + +int kvm_gmem_init(struct module *module) { kvm_gmem_fops.owner = module; + + return kvm_gmem_init_mount(); +} + +void kvm_gmem_exit(void) +{ + kern_unmount(kvm_gmem_mnt); + kvm_gmem_mnt = NULL; }

static int kvm_gmem_migrate_folio(struct address_space *mapping, @@ -465,7 +505,7 @@ bool __weak kvm_arch_supports_gmem_mmap(struct kvm *kvm)

static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) { - const char *anon_name = "[kvm-gmem]"; + static const char *name = "[kvm-gmem]"; struct kvm_gmem *gmem; struct inode *inode; struct file *file; @@ -481,17 +521,17 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) goto err_fd; }

- file = anon_inode_create_getfile(anon_name, &kvm_gmem_fops, gmem, - O_RDWR, NULL); - if (IS_ERR(file)) { - err = PTR_ERR(file); + /* __fput() will take care of fops_put(). */ + if (!fops_get(&kvm_gmem_fops)) { + err = -ENOENT; goto err_gmem; }

- file->f_flags |= O_LARGEFILE; - - inode = file->f_inode; - WARN_ON(file->f_mapping != inode->i_mapping); + inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL); + if (IS_ERR(inode)) { + err = PTR_ERR(inode); + goto err_fops; + }

inode->i_private = (void *)(unsigned long)flags; inode->i_op = &kvm_gmem_iops; @@ -503,6 +543,15 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) /* Unmovable mappings are supposed to be marked unevictable as well. */ WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));

+ file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR, &kvm_gmem_fops); + if (IS_ERR(file)) { + err = PTR_ERR(file); + goto err_inode; + } + + file->f_flags |= O_LARGEFILE; + file->private_data = gmem; + kvm_get_kvm(kvm); gmem->kvm = kvm; xa_init(&gmem->bindings); @@ -511,6 +560,10 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) fd_install(fd, file); return fd;

+err_inode: + iput(inode); +err_fops: + fops_put(&kvm_gmem_fops); err_gmem: kfree(gmem); err_fd: diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 18f29ef93543..301d48d6e00d 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -6489,7 +6489,9 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) if (WARN_ON_ONCE(r)) goto err_vfio;

- kvm_gmem_init(module); + r = kvm_gmem_init(module); + if (r) + goto err_gmem;

r = kvm_init_virtualization(); if (r) @@ -6510,6 +6512,8 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) err_register: kvm_uninit_virtualization(); err_virt: + kvm_gmem_exit(); +err_gmem: kvm_vfio_ops_exit(); err_vfio: kvm_async_pf_deinit(); @@ -6541,6 +6545,7 @@ void kvm_exit(void) for_each_possible_cpu(cpu) free_cpumask_var(per_cpu(cpu_kick_mask, cpu)); kmem_cache_destroy(kvm_vcpu_cache); + kvm_gmem_exit(); kvm_vfio_ops_exit(); kvm_async_pf_deinit(); kvm_irqfd_exit(); diff --git a/virt/kvm/kvm_mm.h b/virt/kvm/kvm_mm.h index 31defb08ccba..9fcc5d5b7f8d 100644 --- a/virt/kvm/kvm_mm.h +++ b/virt/kvm/kvm_mm.h @@ -68,17 +68,18 @@ static inline void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, #endif /* HAVE_KVM_PFNCACHE */

#ifdef CONFIG_KVM_GUEST_MEMFD -void kvm_gmem_init(struct module *module); +int kvm_gmem_init(struct module *module); +void kvm_gmem_exit(void); 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, loff_t offset); void kvm_gmem_unbind(struct kvm_memory_slot *slot); #else -static inline void kvm_gmem_init(struct module *module) +static inline int kvm_gmem_init(struct module *module) { - + return 0; } - +static inline void kvm_gmem_exit(void) {}; static inline int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot, unsigned int fd, loff_t offset)

base-commit: d133892dddd6607de651b7e32510359a6af97c4c --

On 25.09.25 13:44, Garg, Shivank wrote:

On 9/25/2025 8:20 AM, Sean Christopherson wrote:

...

My apologies for the super late feedback. None of this is critical (mechanical things that can be cleaned up after the fact), so if there's any urgency to getting this series into 6.18, just ignore it.

On Wed, Aug 27, 2025, Ackerley Tng wrote:

...

Shivank Garg shivankg@amd.com writes: @@ -463,11 +502,70 @@ bool __weak kvm_arch_supports_gmem_mmap(struct kvm *kvm) return true; }

+static struct inode *kvm_gmem_inode_create(const char *name, loff_t size,

• 			   u64 flags)
  

+{

• struct inode *inode;
• inode = anon_inode_make_secure_inode(kvm_gmem_mnt->mnt_sb, name, NULL);
• if (IS_ERR(inode))

• return inode;
  

• inode->i_private = (void *)(unsigned long)flags;
• inode->i_op = &kvm_gmem_iops;
• inode->i_mapping->a_ops = &kvm_gmem_aops;
• inode->i_mode |= S_IFREG;
• inode->i_size = size;
• mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
• mapping_set_inaccessible(inode->i_mapping);
• /* Unmovable mappings are supposed to be marked unevictable as well. */
• WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
• return inode;

+}

+static struct file *kvm_gmem_inode_create_getfile(void *priv, loff_t size,

• 				  u64 flags)
  

+{

• static const char *name = "[kvm-gmem]";
• struct inode *inode;
• struct file *file;
• int err;
• err = -ENOENT;
• /* __fput() will take care of fops_put(). */
• if (!fops_get(&kvm_gmem_fops))

• goto err;
  

• inode = kvm_gmem_inode_create(name, size, flags);
• if (IS_ERR(inode)) {

• err = PTR_ERR(inode);
  

• goto err_fops_put;
  

• }
• file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,

• 		 &kvm_gmem_fops);
  

• if (IS_ERR(file)) {

• err = PTR_ERR(file);
  

• goto err_put_inode;
  

• }
• file->f_flags |= O_LARGEFILE;
• file->private_data = priv;
• return file;

+err_put_inode:

• iput(inode);

+err_fops_put:

• fops_put(&kvm_gmem_fops);

+err:

• return ERR_PTR(err);

+}

I don't see any reason to add two helpers. It requires quite a bit more lines of code due to adding more error paths and local variables, and IMO doesn't make the code any easier to read.

Passing in "gmem" as @priv is especially ridiculous, as it adds code and obfuscates what file->private_data is set to.

I get the sense that the code was written to be a "replacement" for common APIs, but that is nonsensical (no pun intended).

...

static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) {

• const char *anon_name = "[kvm-gmem]"; struct kvm_gmem *gmem;
• struct inode *inode; struct file *file; int fd, err;

@@ -481,32 +579,16 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) goto err_fd; }

• file = anon_inode_create_getfile(anon_name, &kvm_gmem_fops, gmem,

• 			 O_RDWR, NULL);
  

• file = kvm_gmem_inode_create_getfile(gmem, size, flags); if (IS_ERR(file)) { err = PTR_ERR(file); goto err_gmem; }

• file->f_flags |= O_LARGEFILE;
• inode = file->f_inode;
• WARN_ON(file->f_mapping != inode->i_mapping);
• inode->i_private = (void *)(unsigned long)flags;
• inode->i_op = &kvm_gmem_iops;
• inode->i_mapping->a_ops = &kvm_gmem_aops;
• inode->i_mode |= S_IFREG;
• inode->i_size = size;
• mapping_set_gfp_mask(inode->i_mapping, GFP_HIGHUSER);
• mapping_set_inaccessible(inode->i_mapping);
• /* Unmovable mappings are supposed to be marked unevictable as well. */
• WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
• kvm_get_kvm(kvm); gmem->kvm = kvm; xa_init(&gmem->bindings);
• list_add(&gmem->entry, &inode->i_mapping->i_private_list);

• list_add(&gmem->entry, &file_inode(file)->i_mapping->i_private_list);

I don't understand this change? Isn't file_inode(file) == inode?

Compile tested only, and again not critical, but it's -40 LoC...

Thanks. I did functional testing and it works fine.

I can queue this instead. I guess I can reuse the patch description and add Sean as author + add his SOB (if he agrees).

Let me take a look at the patch later in more detail.

On Wed, Aug 27, 2025, Shivank Garg wrote:

Add dedicated inode structure (kvm_gmem_inode_info) and slab-allocated inode cache for guest memory backing, similar to how shmem handles inodes.

This adds the necessary allocation/destruction functions and prepares for upcoming guest_memfd NUMA policy support changes.

Signed-off-by: Shivank Garg shivankg@amd.com

virt/kvm/guest_memfd.c | 70 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 6c66a0974055..356947d36a47 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -17,6 +17,15 @@ struct kvm_gmem { struct list_head entry; }; +struct kvm_gmem_inode_info {

What about naming this simply gmem_inode?

• struct inode vfs_inode;

+};

+static inline struct kvm_gmem_inode_info *KVM_GMEM_I(struct inode *inode)

And then GMEM_I()?

And then (in a later follow-up if we target this for 6.18, or as a prep patch if we push this out to 6.19), rename kvm_gmem to gmem_file?

That would make guest_memfd look a bit more like other filesystems, and I don't see a need to preface the local structures and helpers with "kvm_", e.g. GMEM_I() is analogous to x86's to_vmx() and to_svm().

As for renaming kvm_gmem => gmem_file, I wandered back into this code via Ackerley's in-place conversion series, and it took me a good long while to remember the roles of files vs. inodes in gmem. That's probably a sign that the code needs clarification given that I wrote the original code. :-)

Leveraging an old discussion[*], my thought is to get to this:

/* * A guest_memfd instance can be associated multiple VMs, each with its own * "view" of the underlying physical memory. * * The gmem's inode is effectively the raw underlying physical storage, and is * used to track properties of the physical memory, while each gmem file is * effectively a single VM's view of that storage, and is used to track assets * specific to its associated VM, e.g. memslots=>gmem bindings. */ struct gmem_file { struct kvm *kvm; struct xarray bindings; struct list_head entry; };

struct gmem_inode { struct shared_policy policy; struct inode vfs_inode; };

[*] https://lore.kernel.org/all/ZLGiEfJZTyl7M8mS@google.com

On Thu, Sep 25, 2025, Sean Christopherson wrote:

On Wed, Aug 27, 2025, Shivank Garg wrote:

...

@@ -26,6 +28,9 @@ static inline struct kvm_gmem_inode_info *KVM_GMEM_I(struct inode *inode) return container_of(inode, struct kvm_gmem_inode_info, vfs_inode); } +static struct mempolicy *kvm_gmem_get_pgoff_policy(struct kvm_gmem_inode_info *info,

• 				   pgoff_t index);
  

/**

• folio_file_pfn - like folio_file_page, but return a pfn.
• @folio: The folio which contains this index.

@@ -112,7 +117,25 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot, static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index) { /* TODO: Support huge pages. */

• return filemap_grab_folio(inode->i_mapping, index);

• struct mempolicy *policy;
• struct folio *folio;
• /*

• * Fast-path: See if folio is already present in mapping to avoid
  

• * policy_lookup.
  

• */
  

• folio = __filemap_get_folio(inode->i_mapping, index,

• 		    FGP_LOCK | FGP_ACCESSED, 0);
  

• if (!IS_ERR(folio))

• return folio;
  

• policy = kvm_gmem_get_pgoff_policy(KVM_GMEM_I(inode), index);
• folio = __filemap_get_folio_mpol(inode->i_mapping, index,

• 			 FGP_LOCK | FGP_ACCESSED | FGP_CREAT,
  

• 			 mapping_gfp_mask(inode->i_mapping), policy);
  

• mpol_cond_put(policy);
• return folio;

} static void kvm_gmem_invalidate_begin(struct kvm_gmem *gmem, pgoff_t start, @@ -372,8 +395,45 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf) return ret; } +#ifdef CONFIG_NUMA +static int kvm_gmem_set_policy(struct vm_area_struct *vma, struct mempolicy *mpol) +{

• struct inode *inode = file_inode(vma->vm_file);
• return mpol_set_shared_policy(&KVM_GMEM_I(inode)->policy, vma, mpol);

+}

+static struct mempolicy *kvm_gmem_get_policy(struct vm_area_struct *vma,

• 			     unsigned long addr, pgoff_t *pgoff)
  

+{

• struct inode *inode = file_inode(vma->vm_file);
• *pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT);
• return mpol_shared_policy_lookup(&KVM_GMEM_I(inode)->policy, *pgoff);

+}

+static struct mempolicy *kvm_gmem_get_pgoff_policy(struct kvm_gmem_inode_info *info,

• 				   pgoff_t index)
  

I keep reading this is "page offset policy", as opposed to "policy given a page offset". Another oddity that is confusing is that this helper explicitly does get_task_policy(current), while kvm_gmem_get_policy() lets the caller do that. The end result is the same, but I think it would be helpful for gmem to be internally consistent.

If we have kvm_gmem_get_policy() use this helper, then we can kill two birds with one stone:

static struct mempolicy *__kvm_gmem_get_policy(struct gmem_inode *gi, pgoff_t index) { struct mempolicy *mpol;

mpol = mpol_shared_policy_lookup(&gi->policy, index); return mpol ? mpol : get_task_policy(current); }

static struct mempolicy *kvm_gmem_get_policy(struct vm_area_struct *vma, unsigned long addr, pgoff_t *pgoff) { *pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT);

return __kvm_gmem_get_policy(GMEM_I(file_inode(vma->vm_file)), *pgoff);

Argh!!!!! This breaks the selftest because do_get_mempolicy() very specifically falls back to the default_policy, NOT to the current task's policy. That is *exactly* the type of subtle detail that needs to be commented, because there's no way some random KVM developer is going to know that returning NULL here is important with respect to get_mempolicy() ABI.

On a happier note, I'm very glad you wrote a testcase :-)

I've got this as fixup-to-the-fixup:

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index e796cc552a96..61130a52553f 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -114,8 +114,8 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot, return r; }

-static struct mempolicy *__kvm_gmem_get_policy(struct gmem_inode *gi, - pgoff_t index) +static struct mempolicy *kvm_gmem_get_folio_policy(struct gmem_inode *gi, + pgoff_t index) { #ifdef CONFIG_NUMA struct mempolicy *mpol; @@ -151,7 +151,7 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index) if (!IS_ERR(folio)) return folio;

- policy = __kvm_gmem_get_policy(GMEM_I(inode), index); + policy = kvm_gmem_get_folio_policy(GMEM_I(inode), index); folio = __filemap_get_folio_mpol(inode->i_mapping, index, FGP_LOCK | FGP_ACCESSED | FGP_CREAT, mapping_gfp_mask(inode->i_mapping), policy); @@ -431,9 +431,18 @@ static int kvm_gmem_set_policy(struct vm_area_struct *vma, struct mempolicy *mpo static struct mempolicy *kvm_gmem_get_policy(struct vm_area_struct *vma, unsigned long addr, pgoff_t *pgoff) { + struct inode *inode = file_inode(vma->vm_file); + *pgoff = vma->vm_pgoff + ((addr - vma->vm_start) >> PAGE_SHIFT);

- return __kvm_gmem_get_policy(GMEM_I(file_inode(vma->vm_file)), *pgoff); + /* + * Note! Directly return whatever the lookup returns, do NOT return + * the current task's policy as is done when looking up the policy for + * a specific folio. Kernel ABI for get_mempolicy() is to return + * MPOL_DEFAULT when there is no defined policy, not whatever the + * default policy resolves to. + */ + return mpol_shared_policy_lookup(&GMEM_I(inode)->policy, *pgoff); } #endif /* CONFIG_NUMA */

On Wed, Aug 27, 2025, Shivank Garg wrote:

Add tests for NUMA memory policy binding and NUMA aware allocation in guest_memfd. This extends the existing selftests by adding proper validation for:

• KVM GMEM set_policy and get_policy() vm_ops functionality using mbind() and get_mempolicy()
• NUMA policy application before and after memory allocation

These tests help ensure NUMA support for guest_memfd works correctly.

Signed-off-by: Shivank Garg shivankg@amd.com

tools/testing/selftests/kvm/Makefile.kvm | 1 + .../testing/selftests/kvm/guest_memfd_test.c | 121 ++++++++++++++++++ 2 files changed, 122 insertions(+)

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index 90f03f00cb04..c46cef2a7cd7 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -275,6 +275,7 @@ pgste-option = $(call try-run, echo 'int main(void) { return 0; }' | \ $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste) LDLIBS += -ldl +LDLIBS += -lnuma

Hrm, this is going to be very annoying. I don't have libnuma-dev installed on any of my <too many> systems, and I doubt I'm alone. Installing the package is trivial, but I'm a little wary of foisting that requirement on all KVM developers and build bots.

I'd be especially curious what ARM and RISC-V think, as NUMA is likely a bit less prevelant there.

LDFLAGS += -pthread $(no-pie-option) $(pgste-option) LIBKVM_C := $(filter %.c,$(LIBKVM)) diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c index b3ca6737f304..9640d04ec293 100644 --- a/tools/testing/selftests/kvm/guest_memfd_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_test.c @@ -7,6 +7,8 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <numa.h> +#include <numaif.h> #include <errno.h> #include <stdio.h> #include <fcntl.h> @@ -19,6 +21,7 @@ #include <sys/mman.h> #include <sys/types.h> #include <sys/stat.h> +#include <sys/syscall.h> #include "kvm_util.h" #include "test_util.h" @@ -72,6 +75,122 @@ static void test_mmap_supported(int fd, size_t page_size, size_t total_size) TEST_ASSERT(!ret, "munmap() should succeed."); } +#define TEST_REQUIRE_NUMA_MULTIPLE_NODES() \

• TEST_REQUIRE(numa_available() != -1 && numa_max_node() >= 1)

Using TEST_REQUIRE() here will result in skipping the _entire_ test. Ideally this test would use fixtures so that each testcase can run in a child process and thus can use TEST_REQUIRE(), but that's a conversion for another day.

Easiest thing would probably be to turn this into a common helper and then bail early.

diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c index 9640d04ec293..6acb186e5300 100644 --- a/tools/testing/selftests/kvm/guest_memfd_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_test.c @@ -7,7 +7,6 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> -#include <numa.h> #include <numaif.h> #include <errno.h> #include <stdio.h> @@ -75,9 +74,6 @@ static void test_mmap_supported(int fd, size_t page_size, size_t total_size) TEST_ASSERT(!ret, "munmap() should succeed."); }

-#define TEST_REQUIRE_NUMA_MULTIPLE_NODES() \ - TEST_REQUIRE(numa_available() != -1 && numa_max_node() >= 1) - static void test_mbind(int fd, size_t page_size, size_t total_size) { unsigned long nodemask = 1; /* nid: 0 */ @@ -87,7 +83,8 @@ static void test_mbind(int fd, size_t page_size, size_t total_size) char *mem; int ret;

- TEST_REQUIRE_NUMA_MULTIPLE_NODES(); + if (!is_multi_numa_node_system()) + return;

mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); TEST_ASSERT(mem != MAP_FAILED, "mmap for mbind test should succeed"); @@ -136,7 +133,8 @@ static void test_numa_allocation(int fd, size_t page_size, size_t total_size) char *mem; int ret, i;

- TEST_REQUIRE_NUMA_MULTIPLE_NODES(); + if (!is_multi_numa_node_system()) + return;

/* Clean slate: deallocate all file space, if any */ ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, total_size); diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 23a506d7eca3..d7051607e6bf 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -21,6 +21,7 @@ #include <sys/eventfd.h> #include <sys/ioctl.h>

+#include <numa.h> #include <pthread.h>

#include "kvm_util_arch.h" @@ -633,6 +634,11 @@ static inline bool is_smt_on(void) return false; }

+static inline bool is_multi_numa_node_system(void) +{ + return numa_available() != -1 && numa_max_node() >= 1; +} + void vm_create_irqchip(struct kvm_vm *vm);

static inline int __vm_create_guest_memfd(struct kvm_vm *vm, uint64_t size,

On 9/26/2025 1:01 PM, David Hildenbrand wrote:

On 25.09.25 23:35, Sean Christopherson wrote:

...

On Wed, Aug 27, 2025, Shivank Garg wrote:

...

Add tests for NUMA memory policy binding and NUMA aware allocation in guest_memfd. This extends the existing selftests by adding proper validation for:

• KVM GMEM set_policy and get_policy() vm_ops functionality using

mbind() and get_mempolicy()

• NUMA policy application before and after memory allocation

These tests help ensure NUMA support for guest_memfd works correctly.

Signed-off-by: Shivank Garg shivankg@amd.com

tools/testing/selftests/kvm/Makefile.kvm      |   1 +   .../testing/selftests/kvm/guest_memfd_test.c  | 121 ++++++++++++++++++   2 files changed, 122 insertions(+)

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index 90f03f00cb04..c46cef2a7cd7 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -275,6 +275,7 @@ pgste-option = $(call try-run, echo 'int main(void) { return 0; }' | \       $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste)     LDLIBS += -ldl +LDLIBS += -lnuma

Hrm, this is going to be very annoying.  I don't have libnuma-dev installed on any of my <too many> systems, and I doubt I'm alone.  Installing the package is trivial, but I'm a little wary of foisting that requirement on all KVM developers and build bots.

I'd be especially curious what ARM and RISC-V think, as NUMA is likely a bit less prevelant there.

We unconditionally use it in the mm tests for ksm and migration tests, so it's not particularly odd to require it here as well.

What we do with liburing in mm selftests is to detect presence at compile time and essentially make the tests behave differently based on availability (see check_config.sh).

I have an alternative that drops libnuma entirely. If this approach looks reasonable, could we potentially factor these out into a common test utility for other selftests that currently depend on libnuma?

What are your thoughts on this?

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index c46cef2a7cd7..90f03f00cb04 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -275,7 +275,6 @@ pgste-option = $(call try-run, echo 'int main(void) { return 0; }' | \ $(CC) -Werror -Wl$(comma)--s390-pgste -x c - -o "$$TMP",-Wl$(comma)--s390-pgste)

LDLIBS += -ldl -LDLIBS += -lnuma LDFLAGS += -pthread $(no-pie-option) $(pgste-option)

LIBKVM_C := $(filter %.c,$(LIBKVM)) diff --git a/tools/testing/selftests/kvm/guest_memfd_test.c b/tools/testing/selftests/kvm/guest_memfd_test.c index 9640d04ec293..12ce91950c44 100644 --- a/tools/testing/selftests/kvm/guest_memfd_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_test.c @@ -7,8 +7,6 @@ #include <stdlib.h> #include <string.h> #include <unistd.h> -#include <numa.h> -#include <numaif.h> #include <errno.h> #include <stdio.h> #include <fcntl.h> @@ -75,9 +73,6 @@ static void test_mmap_supported(int fd, size_t page_size, size_t total_size) TEST_ASSERT(!ret, "munmap() should succeed."); }

-#define TEST_REQUIRE_NUMA_MULTIPLE_NODES() \ - TEST_REQUIRE(numa_available() != -1 && numa_max_node() >= 1) - static void test_mbind(int fd, size_t page_size, size_t total_size) { unsigned long nodemask = 1; /* nid: 0 */ @@ -87,7 +82,8 @@ static void test_mbind(int fd, size_t page_size, size_t total_size) char *mem; int ret;

- TEST_REQUIRE_NUMA_MULTIPLE_NODES(); + if (!is_multi_numa_node_system()) + return;

mem = mmap(NULL, total_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); TEST_ASSERT(mem != MAP_FAILED, "mmap for mbind test should succeed"); @@ -136,7 +132,8 @@ static void test_numa_allocation(int fd, size_t page_size, size_t total_size) char *mem; int ret, i;

- TEST_REQUIRE_NUMA_MULTIPLE_NODES(); + if (!is_multi_numa_node_system()) + return;

/* Clean slate: deallocate all file space, if any */ ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, total_size); diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 23a506d7eca3..ba4c316f4fef 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -12,6 +12,7 @@ #include "linux/list.h" #include <linux/kernel.h> #include <linux/kvm.h> +#include <linux/mempolicy.h> #include "linux/rbtree.h" #include <linux/types.h>

@@ -20,6 +21,7 @@

#include <sys/eventfd.h> #include <sys/ioctl.h> +#include <sys/syscall.h>

#include <pthread.h>

@@ -633,6 +635,50 @@ static inline bool is_smt_on(void) return false; }

+#include <dirent.h> +static int numa_max_node(void) +{ + DIR *d; + struct dirent *de; + int max_node = 0; + + d = opendir("/sys/devices/system/node"); + if (!d) { + /* No NUMA support or no nodes found, assume single node */ + return 0; + } + + while ((de = readdir(d)) != NULL) { + int node_id; + char *endptr; + + if (strncmp(de->d_name, "node", 4) != 0) + continue; + + node_id = strtol(de->d_name + 4, &endptr, 10); + if (*endptr != '\0') + continue; + + if (node_id > max_node) + max_node = node_id; + } + closedir(d); + + return max_node; +} + +static int numa_available(void) +{ + if (syscall(__NR_get_mempolicy, NULL, NULL, 0, 0, 0) < 0 && (errno == ENOSYS || errno == EPERM)) + return -1; + return 0; +} + +static inline bool is_multi_numa_node_system(void) +{ + return numa_available() != -1 && numa_max_node() >= 1; +} + void vm_create_irqchip(struct kvm_vm *vm);

static inline int __vm_create_guest_memfd(struct kvm_vm *vm, uint64_t size,