From: Sean Christopherson seanjc@google.com

Start plumbing in guest_memfd support for in-place private<=>shared conversions by tracking attributes via a maple tree. KVM currently tracks private vs. shared attributes on a per-VM basis, which made sense when a guest_memfd _only_ supported private memory, but tracking per-VM simply can't work for in-place conversions as the shareability of a given page needs to be per-gmem_inode, not per-VM.

Use the filemap invalidation lock to protect the maple tree, as taking the lock for read when faulting in memory (for userspace or the guest) isn't expected to result in meaningful contention, and using a separate lock would add significant complexity (avoid deadlock is quite difficult).

Signed-off-by: Sean Christopherson seanjc@google.com Co-developed-by: Ackerley Tng ackerleytng@google.com Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Vishal Annapurve vannapurve@google.com Signed-off-by: Vishal Annapurve vannapurve@google.com Co-developed-by: Fuad Tabba tabba@google.com Signed-off-by: Fuad Tabba tabba@google.com --- virt/kvm/guest_memfd.c | 119 +++++++++++++++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 16 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index b22caa8b530ab..26cec833766c3 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -4,6 +4,7 @@ #include <linux/falloc.h> #include <linux/fs.h> #include <linux/kvm_host.h> +#include <linux/maple_tree.h> #include <linux/mempolicy.h> #include <linux/pseudo_fs.h> #include <linux/pagemap.h> @@ -32,6 +33,7 @@ struct gmem_inode { struct inode vfs_inode;

u64 flags; + struct maple_tree attributes; };

static __always_inline struct gmem_inode *GMEM_I(struct inode *inode) @@ -54,6 +56,23 @@ static inline kvm_pfn_t folio_file_pfn(struct folio *folio, pgoff_t index) return folio_pfn(folio) + (index & (folio_nr_pages(folio) - 1)); }

+static u64 kvm_gmem_get_attributes(struct inode *inode, pgoff_t index) +{ + void *entry = mtree_load(&GMEM_I(inode)->attributes, index); + + return WARN_ON_ONCE(!entry) ? 0 : xa_to_value(entry); +} + +static bool kvm_gmem_is_private_mem(struct inode *inode, pgoff_t index) +{ + return kvm_gmem_get_attributes(inode, index) & KVM_MEMORY_ATTRIBUTE_PRIVATE; +} + +static bool kvm_gmem_is_shared_mem(struct inode *inode, pgoff_t index) +{ + return !kvm_gmem_is_private_mem(inode, index); +} + static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot, pgoff_t index, struct folio *folio) { @@ -415,10 +434,13 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf) if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) return VM_FAULT_SIGBUS;

- if (!(GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED)) - return VM_FAULT_SIGBUS; + filemap_invalidate_lock_shared(inode->i_mapping); + if (kvm_gmem_is_shared_mem(inode, vmf->pgoff)) + folio = kvm_gmem_get_folio(inode, vmf->pgoff); + else + folio = ERR_PTR(-EACCES); + filemap_invalidate_unlock_shared(inode->i_mapping);

- folio = kvm_gmem_get_folio(inode, vmf->pgoff); if (IS_ERR(folio)) { if (PTR_ERR(folio) == -EAGAIN) return VM_FAULT_RETRY; @@ -572,6 +594,46 @@ bool __weak kvm_arch_supports_gmem_init_shared(struct kvm *kvm) return true; }

+static int kvm_gmem_init_inode(struct inode *inode, loff_t size, u64 flags) +{ + struct gmem_inode *gi = GMEM_I(inode); + MA_STATE(mas, &gi->attributes, 0, (size >> PAGE_SHIFT) - 1); + u64 attrs; + int r; + + 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)); + + gi->flags = flags; + + mt_set_external_lock(&gi->attributes, + &inode->i_mapping->invalidate_lock); + + /* + * Store default attributes for the entire gmem instance. Ensuring every + * index is represented in the maple tree at all times simplifies the + * conversion and merging logic. + */ + attrs = gi->flags & GUEST_MEMFD_FLAG_INIT_SHARED ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE; + + /* + * Acquire the invalidation lock purely to make lockdep happy. There + * should be no races at this time since the inode hasn't yet been fully + * created. + */ + filemap_invalidate_lock(inode->i_mapping); + r = mas_store_gfp(&mas, xa_mk_value(attrs), GFP_KERNEL); + filemap_invalidate_unlock(inode->i_mapping); + + return r; +} + static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) { static const char *name = "[kvm-gmem]"; @@ -602,16 +664,9 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) goto err_fops; }

- 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)); - - GMEM_I(inode)->flags = flags; + err = kvm_gmem_init_inode(inode, size, flags); + if (err) + goto err_inode;

file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR, &kvm_gmem_fops); if (IS_ERR(file)) { @@ -798,9 +853,13 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot, if (!file) return -EFAULT;

+ filemap_invalidate_lock_shared(file_inode(file)->i_mapping); + folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order); - if (IS_ERR(folio)) - return PTR_ERR(folio); + if (IS_ERR(folio)) { + r = PTR_ERR(folio); + goto out; + }

if (!is_prepared) r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio); @@ -812,6 +871,8 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot, else folio_put(folio);

+out: + filemap_invalidate_unlock_shared(file_inode(file)->i_mapping); return r; } EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn); @@ -925,13 +986,39 @@ static struct inode *kvm_gmem_alloc_inode(struct super_block *sb)

mpol_shared_policy_init(&gi->policy, NULL);

+ /* + * Memory attributes are protected the filemap invalidation lock, but + * the lock structure isn't available at this time. Immediately mark + * maple tree as using external locking so that accessing the tree + * before its fully initialized results in NULL pointer dereferences + * and not more subtle bugs. + */ + mt_init_flags(&gi->attributes, MT_FLAGS_LOCK_EXTERN); + gi->flags = 0; return &gi->vfs_inode; }

static void kvm_gmem_destroy_inode(struct inode *inode) { - mpol_free_shared_policy(&GMEM_I(inode)->policy); + struct gmem_inode *gi = GMEM_I(inode); + + mpol_free_shared_policy(&gi->policy); + + /* + * Note! Checking for an empty tree is functionally necessary to avoid + * explosions if the tree hasn't been initialized, i.e. if the inode is + * being destroyed before guest_memfd can set the external lock. + */ + if (!mtree_empty(&gi->attributes)) { + /* + * Acquire the invalidation lock purely to make lockdep happy, + * the inode is unreachable at this point. + */ + filemap_invalidate_lock(inode->i_mapping); + __mt_destroy(&gi->attributes); + filemap_invalidate_unlock(inode->i_mapping); + } }

static void kvm_gmem_free_inode(struct inode *inode) -- 2.51.0.858.gf9c4a03a3a-goog

From: Sean Christopherson seanjc@google.com

Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based on kvm_arch_has_private_mem being #defined in anticipation of decoupling kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES. guest_memfd support for memory attributes will be unconditional to avoid yet more macros (all architectures that support guest_memfd are expect to user per-gmem attributes at some point), at which point enumerating support KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being supported _somewhere_ would result in KVM over-reporting support on arm64.

Signed-off-by: Sean Christopherson seanjc@google.com --- include/linux/kvm_host.h | 2 +- virt/kvm/kvm_main.c | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index fddb373fcbaaf..21bf30e8d3cc1 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -721,7 +721,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu) } #endif

-#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +#ifndef kvm_arch_has_private_mem static inline bool kvm_arch_has_private_mem(struct kvm *kvm) { return false; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f73047ea4333e..591795a3fa124 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2428,8 +2428,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm, #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES static u64 kvm_supported_mem_attributes(struct kvm *kvm) { +#ifdef kvm_arch_has_private_mem if (!kvm || kvm_arch_has_private_mem(kvm)) return KVM_MEMORY_ATTRIBUTE_PRIVATE; +#endif

return 0; }

From: Sean Christopherson seanjc@google.com

Introduce the basic infrastructure to allow per-VM memory attribute tracking to be disabled. This will be built-upon in a later patch, where a module param can disable per-VM memory attribute tracking.

Split the Kconfig option into a base KVM_MEMORY_ATTRIBUTES and the existing KVM_VM_MEMORY_ATTRIBUTES. The base option provides the core plumbing, while the latter enables the full per-VM tracking via an xarray and the associated ioctls.

kvm_get_memory_attributes() now performs a static call that either looks up kvm->mem_attr_array with CONFIG_KVM_VM_MEMORY_ATTRIBUTES is enabled, or just returns 0 otherwise. The static call can be patched depending on whether per-VM tracking is enabled by the CONFIG.

No functional change intended.

Signed-off-by: Sean Christopherson seanjc@google.com --- arch/x86/include/asm/kvm_host.h | 2 +- include/linux/kvm_host.h | 23 ++++++++++------- virt/kvm/Kconfig | 6 ++++- virt/kvm/kvm_main.c | 44 ++++++++++++++++++++++++++++++++- 4 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index efb0b2e1808d5..197b28ae0e28c 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -2301,7 +2301,7 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level, int tdp_max_root_level, int tdp_huge_page_level);

-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES #define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem) #endif

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 21bf30e8d3cc1..512febf47c265 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -2514,19 +2514,15 @@ static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot) return slot->flags & KVM_MEMSLOT_GMEM_ONLY; }

-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES +typedef unsigned long (kvm_get_memory_attributes_t)(struct kvm *kvm, gfn_t gfn); +DECLARE_STATIC_CALL(__kvm_get_memory_attributes, kvm_get_memory_attributes_t); + static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn) { - return xa_to_value(xa_load(&kvm->mem_attr_array, gfn)); + return static_call(__kvm_get_memory_attributes)(kvm, gfn); }

-bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, - unsigned long mask, unsigned long attrs); -bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm, - struct kvm_gfn_range *range); -bool kvm_arch_post_set_memory_attributes(struct kvm *kvm, - struct kvm_gfn_range *range); - static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) { return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE; @@ -2536,6 +2532,15 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) { return false; } +#endif + +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, + unsigned long mask, unsigned long attrs); +bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm, + struct kvm_gfn_range *range); +bool kvm_arch_post_set_memory_attributes(struct kvm *kvm, + struct kvm_gfn_range *range); #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */

#ifdef CONFIG_KVM_GUEST_MEMFD diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 9dd7873114b59..395996977fe5a 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -108,10 +108,14 @@ config KVM_MMU_LOCKLESS_AGING depends on KVM_GENERIC_MMU_NOTIFIER bool

-config KVM_VM_MEMORY_ATTRIBUTES +config KVM_MEMORY_ATTRIBUTES depends on KVM_GENERIC_MMU_NOTIFIER bool

+config KVM_VM_MEMORY_ATTRIBUTES + select KVM_MEMORY_ATTRIBUTES + bool + config KVM_GUEST_MEMFD depends on KVM_GENERIC_MMU_NOTIFIER select XARRAY_MULTI diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 591795a3fa124..6c29770dfa7c8 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -101,6 +101,17 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(halt_poll_ns_shrink); static bool allow_unsafe_mappings; module_param(allow_unsafe_mappings, bool, 0444);

+#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +static bool vm_memory_attributes = true; +#else +#define vm_memory_attributes false +#endif +DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, kvm_get_memory_attributes_t); +EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes)); +EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_TRAMP(__kvm_get_memory_attributes)); +#endif + /* * Ordering of locks: * @@ -2425,7 +2436,7 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm, } #endif /* CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */

-#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES static u64 kvm_supported_mem_attributes(struct kvm *kvm) { #ifdef kvm_arch_has_private_mem @@ -2436,6 +2447,12 @@ static u64 kvm_supported_mem_attributes(struct kvm *kvm) return 0; }

+#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES +static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn) +{ + return xa_to_value(xa_load(&kvm->mem_attr_array, gfn)); +} + /* * Returns true if _all_ gfns in the range [@start, @end) have attributes * such that the bits in @mask match @attrs. @@ -2632,7 +2649,24 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,

return kvm_vm_set_mem_attributes(kvm, start, end, attrs->attributes); } +#else /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */ +static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn) +{ + BUILD_BUG_ON(1); +} #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */ +static void kvm_init_memory_attributes(void) +{ + if (vm_memory_attributes) + static_call_update(__kvm_get_memory_attributes, + kvm_get_vm_memory_attributes); + else + static_call_update(__kvm_get_memory_attributes, + (void *)__static_call_return0); +} +#else /* CONFIG_KVM_MEMORY_ATTRIBUTES */ +static void kvm_init_memory_attributes(void) { } +#endif /* CONFIG_KVM_MEMORY_ATTRIBUTES */

struct kvm_memory_slot *gfn_to_memslot(struct kvm *kvm, gfn_t gfn) { @@ -4925,6 +4959,9 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) return 1; #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES case KVM_CAP_MEMORY_ATTRIBUTES: + if (!vm_memory_attributes) + return 0; + return kvm_supported_mem_attributes(kvm); #endif #ifdef CONFIG_KVM_GUEST_MEMFD @@ -5331,6 +5368,10 @@ static long kvm_vm_ioctl(struct file *filp, case KVM_SET_MEMORY_ATTRIBUTES: { struct kvm_memory_attributes attrs;

+ r = -ENOTTY; + if (!vm_memory_attributes) + goto out; + r = -EFAULT; if (copy_from_user(&attrs, argp, sizeof(attrs))) goto out; @@ -6513,6 +6554,7 @@ int kvm_init(unsigned vcpu_size, unsigned vcpu_align, struct module *module) kvm_preempt_ops.sched_in = kvm_sched_in; kvm_preempt_ops.sched_out = kvm_sched_out;

+ kvm_init_memory_attributes(); kvm_init_debug();

r = kvm_vfio_ops_init();

Update the guest_memfd populate() flow to pull memory attributes from the gmem instance instead of the VM when KVM is not configured to track shared/private status in the VM.

Rename the per-VM API to make it clear that it retrieves per-VM attributes, i.e. is not suitable for use outside of flows that are specific to generic per-VM attributes.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- arch/x86/kvm/mmu/mmu.c | 2 +- include/linux/kvm_host.h | 5 ++++- virt/kvm/guest_memfd.c | 26 +++++++++++++++++++++++--- virt/kvm/kvm_main.c | 8 +++----- 4 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index e4542b37b0db6..52189853cf4ab 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -7897,7 +7897,7 @@ static bool hugepage_has_attrs(struct kvm *kvm, struct kvm_memory_slot *slot, const unsigned long end = start + KVM_PAGES_PER_HPAGE(level);

if (level == PG_LEVEL_2M) - return kvm_range_has_memory_attributes(kvm, start, end, ~0, attrs); + return kvm_range_has_vm_memory_attributes(kvm, start, end, ~0, attrs);

for (gfn = start; gfn < end; gfn += KVM_PAGES_PER_HPAGE(level - 1)) { if (hugepage_test_mixed(slot, gfn, level - 1) || diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index b8418cc5851f1..b48632ee242b3 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -2535,12 +2535,15 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn) #endif

#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES -bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, +extern bool vm_memory_attributes; +bool kvm_range_has_vm_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, unsigned long mask, unsigned long attrs); bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm, struct kvm_gfn_range *range); bool kvm_arch_post_set_memory_attributes(struct kvm *kvm, struct kvm_gfn_range *range); +#else +#define vm_memory_attributes false #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */

unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn); diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index f62facc3ab776..855e682041311 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -907,10 +907,30 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot, EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);

#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE +static bool kvm_gmem_range_is_private(struct gmem_inode *gi, pgoff_t index, + size_t nr_pages, struct kvm *kvm, gfn_t gfn) +{ + pgoff_t end = index + nr_pages - 1; + void *entry; + + if (vm_memory_attributes) + return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages, + KVM_MEMORY_ATTRIBUTE_PRIVATE, + KVM_MEMORY_ATTRIBUTE_PRIVATE); + + mt_for_each(&gi->attributes, entry, index, end) { + if (xa_to_value(entry) != attributes) + return false; + } + + return true; +} + long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages, kvm_gmem_populate_cb post_populate, void *opaque) { struct kvm_memory_slot *slot; + struct gmem_inode *gi; void __user *p;

int ret = 0, max_order; @@ -929,6 +949,8 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long if (!file) return -EFAULT;

+ gi = GMEM_I(file_inode(file)); + filemap_invalidate_lock(file->f_mapping);

npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages); @@ -962,9 +984,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long (npages - i) < (1 << max_order));

ret = -EINVAL; - while (!kvm_range_has_memory_attributes(kvm, gfn, gfn + (1 << max_order), - KVM_MEMORY_ATTRIBUTE_PRIVATE, - KVM_MEMORY_ATTRIBUTE_PRIVATE)) { + while (!kvm_gmem_range_is_private(gi, index, 1 << max_order, kvm, gfn)) { if (!max_order) goto put_folio_and_exit; max_order--; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index c73ebdb73070e..35166754a22b4 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -103,9 +103,7 @@ module_param(allow_unsafe_mappings, bool, 0444);

#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES -static bool vm_memory_attributes = true; -#else -#define vm_memory_attributes false +bool vm_memory_attributes = true; #endif DEFINE_STATIC_CALL_RET0(__kvm_get_memory_attributes, kvm_get_memory_attributes_t); EXPORT_SYMBOL_FOR_KVM_INTERNAL(STATIC_CALL_KEY(__kvm_get_memory_attributes)); @@ -2457,7 +2455,7 @@ static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn) * Returns true if _all_ gfns in the range [@start, @end) have attributes * such that the bits in @mask match @attrs. */ -bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, +bool kvm_range_has_vm_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end, unsigned long mask, unsigned long attrs) { XA_STATE(xas, &kvm->mem_attr_array, start); @@ -2591,7 +2589,7 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end, mutex_lock(&kvm->slots_lock);

/* Nothing to do if the entire range has the desired attributes. */ - if (kvm_range_has_memory_attributes(kvm, start, end, ~0, attributes)) + if (kvm_range_has_vm_memory_attributes(kvm, start, end, ~0, attributes)) goto out_unlock;

/*

Introduce a "version 2" of KVM_SET_MEMORY_ATTRIBUTES to support returning information back to userspace.

This new ioctl and structure will, in a later patch, be shared as a guest_memfd ioctl, where the padding in the new kvm_memory_attributes2 structure will be for writing the response from the guest_memfd ioctl to userspace.

A new ioctl is necessary for these reasons:

1. KVM_SET_MEMORY_ATTRIBUTES is currently a write-only ioctl and does not allow userspace to read fields. There's nothing in code (yet?) that validates this, but using _IOWR for consistency would be prudent.

2. KVM_SET_MEMORY_ATTRIBUTES, when used as a guest_memfd ioctl, will need an additional field to provide userspace with more error details.

Alternatively, a completely new ioctl could be defined, unrelated to KVM_SET_MEMORY_ATTRIBUTES, but using the same ioctl number and struct for the vm and guest_memfd ioctls streamlines the interface for userspace. In addition, any memory attributes, implemented on the vm or guest_memfd ioctl, can be easily shared with the other.

Suggested-by: Sean Christopherson seanjc@google.com Signed-off-by: Ackerley Tng ackerleytng@google.com --- Documentation/virt/kvm/api.rst | 32 +++++++++++++++++++++++++++++++ include/uapi/linux/kvm.h | 12 ++++++++++++ virt/kvm/kvm_main.c | 35 +++++++++++++++++++++++++++++++--- 3 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index 754b662a453c3..a812769d79bf6 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -6355,6 +6355,8 @@ S390: Returns -EINVAL if the VM has the KVM_VM_S390_UCONTROL flag set. Returns -EINVAL if called on a protected VM.

+.. _KVM_SET_MEMORY_ATTRIBUTES: + 4.141 KVM_SET_MEMORY_ATTRIBUTES -------------------------------

@@ -6512,6 +6514,36 @@ the capability to be present.

`flags` must currently be zero.

+4.144 KVM_SET_MEMORY_ATTRIBUTES2 +--------------------------------- + +:Capability: KVM_CAP_MEMORY_ATTRIBUTES2 +:Architectures: x86 +:Type: vm ioctl +:Parameters: struct kvm_memory_attributes2 (in/out) +:Returns: 0 on success, <0 on error + +KVM_SET_MEMORY_ATTRIBUTES2 is an extension to +KVM_SET_MEMORY_ATTRIBUTES that supports returning (writing) values to +userspace. The original (pre-extension) fields are shared with +KVM_SET_MEMORY_ATTRIBUTES identically. + +Attribute values are shared with KVM_SET_MEMORY_ATTRIBUTES. + +:: + + struct kvm_memory_attributes2 { + __u64 address; + __u64 size; + __u64 attributes; + __u64 flags; + __u64 reserved[4]; + }; + + #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3) + +See also: :ref: `KVM_SET_MEMORY_ATTRIBUTES`. +

.. _kvm_run:

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 52f6000ab0208..c300e38c7c9cd 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -963,6 +963,7 @@ struct kvm_enable_cap { #define KVM_CAP_RISCV_MP_STATE_RESET 242 #define KVM_CAP_ARM_CACHEABLE_PFNMAP_SUPPORTED 243 #define KVM_CAP_GUEST_MEMFD_FLAGS 244 +#define KVM_CAP_MEMORY_ATTRIBUTES2 245

struct kvm_irq_routing_irqchip { __u32 irqchip; @@ -1617,4 +1618,15 @@ struct kvm_pre_fault_memory { __u64 padding[5]; };

+/* Available with KVM_CAP_MEMORY_ATTRIBUTES2 */ +#define KVM_SET_MEMORY_ATTRIBUTES2 _IOWR(KVMIO, 0xd6, struct kvm_memory_attributes2) + +struct kvm_memory_attributes2 { + __u64 address; + __u64 size; + __u64 attributes; + __u64 flags; + __u64 reserved[4]; +}; + #endif /* __LINUX_KVM_H */ diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 35166754a22b4..dd84b377e46db 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2621,7 +2621,7 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end, return r; } static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm, - struct kvm_memory_attributes *attrs) + struct kvm_memory_attributes2 *attrs) { gfn_t start, end;

@@ -4959,6 +4959,7 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) case KVM_CAP_DEVICE_CTRL: return 1; #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES + case KVM_CAP_MEMORY_ATTRIBUTES2: case KVM_CAP_MEMORY_ATTRIBUTES: if (!vm_memory_attributes) return 0; @@ -5184,6 +5185,14 @@ do { \ sizeof_field(struct kvm_userspace_memory_region2, field)); \ } while (0)

+#define SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(field) \ +do { \ + BUILD_BUG_ON(offsetof(struct kvm_set_memory_attributes, field) != \ + offsetof(struct kvm_set_memory_attributes2, field)); \ + BUILD_BUG_ON(sizeof_field(struct kvm_set_memory_attributes, field) != \ + sizeof_field(struct kvm_set_memory_attributes2, field)); \ +} while (0) + static long kvm_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) { @@ -5366,15 +5375,35 @@ static long kvm_vm_ioctl(struct file *filp, } #endif /* CONFIG_HAVE_KVM_IRQ_ROUTING */ #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES + case KVM_SET_MEMORY_ATTRIBUTES2: case KVM_SET_MEMORY_ATTRIBUTES: { - struct kvm_memory_attributes attrs; + struct kvm_memory_attributes2 attrs; + unsigned long size; + + if (ioctl == KVM_SET_MEMORY_ATTRIBUTES) { + /* + * Fields beyond struct kvm_userspace_memory_region shouldn't be + * accessed, but avoid leaking kernel memory in case of a bug. + */ + memset(&mem, 0, sizeof(mem)); + size = sizeof(struct kvm_set_memory_attributes); + } else { + size = sizeof(struct kvm_set_memory_attributes2); + } + + /* Ensure the common parts of the two structs are identical. */ + SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(slot); + SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(flags); + SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(guest_phys_addr); + SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(memory_size); + SANITY_CHECK_MEMORY_ATTRIBUTES_FIELD(userspace_addr);

r = -ENOTTY; if (!vm_memory_attributes) goto out;

r = -EFAULT; - if (copy_from_user(&attrs, argp, sizeof(attrs))) + if (copy_from_user(&attrs, argp, size)) goto out;

r = kvm_vm_ioctl_set_mem_attributes(kvm, &attrs);

guest_memfd folios don't care about accessed flags since the memory is unevictable and there is no storage to write back to, hence, cleanup the allocation path by not setting FGP_ACCESSED.

Signed-off-by: Ackerley Tng ackerleytng@google.com [sean: split to separate patch, write changelog] Signed-off-by: Sean Christopherson seanjc@google.com --- virt/kvm/guest_memfd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 855e682041311..2a9e9220a48aa 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -167,14 +167,13 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index) * 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); + folio = filemap_lock_folio(inode->i_mapping, index); if (!IS_ERR(folio)) return folio;

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, + FGP_LOCK | FGP_CREAT, mapping_gfp_mask(inode->i_mapping), policy); mpol_cond_put(policy);

filemap_add_folio(), called from filemap_grab_folio(), adds folios to an LRU list. This is unnecessary for guest_memfd, which does not participate in swapping.

In addition, the LRU list takes a reference count on the folio. With shared-to-private memory conversions for KVM guests dependent on folio refcounts, this extra reference can cause conversions to fail due to unexpected refcounts.

Rework kvm_gmem_get_folio() to manually allocate and insert the folio into the page cache without placing it on the LRU. This is done by calling __filemap_add_folio() directly.

The folio is then marked unevictable to avoid participation in swapping. The ->free_folio() handler is modified to unset the unevictable flag when the folio is released from guest_memfd.

This change ensures that LRU lists no longer take refcounts on guest_memfd folios, significantly reducing the chance of elevated refcounts during conversion.

To facilitate this, __filemap_add_folio is exported for KVM's use.

Signed-off-by: Ackerley Tng ackerleytng@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- mm/filemap.c | 1 + mm/memcontrol.c | 2 ++ virt/kvm/guest_memfd.c | 60 +++++++++++++++++++++++++++++++++--------- 3 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c index 03f223be575ca..60c7c95bbd7e6 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -954,6 +954,7 @@ noinline int __filemap_add_folio(struct address_space *mapping, return xas_error(&xas); } ALLOW_ERROR_INJECTION(__filemap_add_folio, ERRNO); +EXPORT_SYMBOL_FOR_MODULES(__filemap_add_folio, "kvm");

int filemap_add_folio(struct address_space *mapping, struct folio *folio, pgoff_t index, gfp_t gfp) diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 8dd7fbed5a942..fe8629414d0a9 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -4721,6 +4721,7 @@ int __mem_cgroup_charge(struct folio *folio, struct mm_struct *mm, gfp_t gfp)

return ret; } +EXPORT_SYMBOL_FOR_MODULES(__mem_cgroup_charge, "kvm");

/** * mem_cgroup_charge_hugetlb - charge the memcg for a hugetlb folio @@ -4893,6 +4894,7 @@ void __mem_cgroup_uncharge(struct folio *folio) uncharge_folio(folio, &ug); uncharge_batch(&ug); } +EXPORT_SYMBOL_FOR_MODULES(__mem_cgroup_uncharge, "kvm");

void __mem_cgroup_uncharge_folios(struct folio_batch *folios) { diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 2a9e9220a48aa..dab2b3ce78bc8 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -148,6 +148,41 @@ static struct mempolicy *kvm_gmem_get_folio_policy(struct gmem_inode *gi, #endif }

+static struct folio *__kvm_gmem_get_folio(struct address_space *mapping, + pgoff_t index, + struct mempolicy *policy) +{ + const gfp_t gfp = mapping_gfp_mask(mapping); + struct folio *folio; + int err; + + folio = filemap_lock_folio(mapping, index); + if (!IS_ERR(folio)) + return folio; + + folio = filemap_alloc_folio(gfp, 0, policy); + if (!folio) + return ERR_PTR(-ENOMEM); + + err = mem_cgroup_charge(folio, NULL, gfp); + if (err) + goto err_put; + + __folio_set_locked(folio); + + err = __filemap_add_folio(mapping, folio, index, gfp, NULL); + if (err) { + __folio_clear_locked(folio); + goto err_put; + } + + return folio; + +err_put: + folio_put(folio); + return ERR_PTR(err); +} + /* * Returns a locked folio on success. The caller is responsible for * setting the up-to-date flag before the memory is mapped into the guest. @@ -160,6 +195,7 @@ static struct mempolicy *kvm_gmem_get_folio_policy(struct gmem_inode *gi, static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index) { /* TODO: Support huge pages. */ + struct address_space *mapping = inode->i_mapping; struct mempolicy *policy; struct folio *folio;

@@ -167,16 +203,17 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index) * Fast-path: See if folio is already present in mapping to avoid * policy_lookup. */ - folio = filemap_lock_folio(inode->i_mapping, index); + folio = filemap_lock_folio(mapping, index); if (!IS_ERR(folio)) return folio;

policy = kvm_gmem_get_folio_policy(GMEM_I(inode), index); - folio = __filemap_get_folio_mpol(inode->i_mapping, index, - FGP_LOCK | FGP_CREAT, - mapping_gfp_mask(inode->i_mapping), policy); - mpol_cond_put(policy);

+ do { + folio = __kvm_gmem_get_folio(mapping, index, policy); + } while (IS_ERR(folio) && PTR_ERR(folio) == -EEXIST); + + mpol_cond_put(policy); return folio; }

@@ -588,24 +625,21 @@ static int kvm_gmem_error_folio(struct address_space *mapping, struct folio *fol return MF_DELAYED; }

-#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE static void kvm_gmem_free_folio(struct folio *folio) { - struct page *page = folio_page(folio, 0); - kvm_pfn_t pfn = page_to_pfn(page); - int order = folio_order(folio); + folio_clear_unevictable(folio);

- kvm_arch_gmem_invalidate(pfn, pfn + (1ul << order)); -} +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE + kvm_arch_gmem_invalidate(folio_pfn(folio), + folio_pfn(folio) + folio_nr_pages(folio)); #endif +}

static const struct address_space_operations kvm_gmem_aops = { .dirty_folio = noop_dirty_folio, .migrate_folio = kvm_gmem_migrate_folio, .error_remove_folio = kvm_gmem_error_folio, -#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE .free_folio = kvm_gmem_free_folio, -#endif };

static int kvm_gmem_setattr(struct mnt_idmap *idmap, struct dentry *dentry,

From: Sean Christopherson seanjc@google.com

Now that guest_memfd supports tracking private vs. shared within gmem itself, allow userspace to specify INIT_SHARED on a guest_memfd instance for x86 Confidential Computing (CoCo) VMs, so long as per-VM attributes are disabled, i.e. when it's actually possible for a guest_memfd instance to contain shared memory.

Signed-off-by: Sean Christopherson seanjc@google.com --- arch/x86/kvm/x86.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5e38c4c9cf63c..4ad451982380e 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -13941,14 +13941,13 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu) }

#ifdef CONFIG_KVM_GUEST_MEMFD -/* - * KVM doesn't yet support initializing guest_memfd memory as shared for VMs - * with private memory (the private vs. shared tracking needs to be moved into - * guest_memfd). - */ bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm) { - return !kvm_arch_has_private_mem(kvm); + /* + * INIT_SHARED isn't supported if the memory attributes are per-VM, + * in which case guest_memfd can _only_ be used for private memory. + */ + return !vm_memory_attributes || !kvm_arch_has_private_mem(kvm); }

#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE

From: Sean Christopherson seanjc@google.com

Bury KVM_VM_MEMORY_ATTRIBUTES in x86 to discourage other architectures from adding support for per-VM memory attributes, because tracking private vs. shared memory on a per-VM basis is now deprecated in favor of tracking on a per-guest_memfd basis, and no other memory attributes are on the horizon.

This will also allow modifying KVM_VM_MEMORY_ATTRIBUTES to be user-selectable (in x86) without creating weirdness in KVM's Kconfigs. Now that guest_memfd support memory attributes, it's entirely possible to run x86 CoCo VMs without support for KVM_VM_MEMORY_ATTRIBUTES.

Leave the code itself in common KVM so that it's trivial to undo this change if new per-VM attributes do come along.

Signed-off-by: Sean Christopherson seanjc@google.com --- arch/x86/kvm/Kconfig | 4 ++++ virt/kvm/Kconfig | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index acb03b45ba050..49c7709e3d895 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -80,6 +80,10 @@ config KVM_WERROR

If in doubt, say "N".

+config KVM_VM_MEMORY_ATTRIBUTES + select KVM_MEMORY_ATTRIBUTES + bool + config KVM_SW_PROTECTED_VM bool "Enable support for KVM software-protected VMs" depends on EXPERT diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index b3473aec4d24d..72b19813e5412 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -112,10 +112,6 @@ config KVM_MEMORY_ATTRIBUTES depends on KVM_GENERIC_MMU_NOTIFIER bool

-config KVM_VM_MEMORY_ATTRIBUTES - select KVM_MEMORY_ATTRIBUTES - bool - config KVM_GUEST_MEMFD select KVM_MEMORY_ATTRIBUTES select XARRAY_MULTI

From: Sean Christopherson seanjc@google.com

When adding a memslot associated a guest_memfd instance, create/dup the guest_memfd before creating the "normal" backing file. This will allow dup'ing the gmem fd as the normal fd when guest_memfd supports mmap(), i.e. to make guest_memfd the _only_ backing source for the memslot.

Signed-off-by: Sean Christopherson seanjc@google.com --- tools/testing/selftests/kvm/lib/kvm_util.c | 45 +++++++++++----------- 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index eef6de9a7f4b1..e35c65a173606 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1026,6 +1026,29 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, if (alignment > 1) region->mmap_size += alignment;

+ if (flags & KVM_MEM_GUEST_MEMFD) { + if (guest_memfd < 0) { + uint32_t guest_memfd_flags = 0; + + TEST_ASSERT(!guest_memfd_offset, + "Offset must be zero when creating new guest_memfd"); + guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags); + } else { + /* + * Install a unique fd for each memslot so that the fd + * can be closed when the region is deleted without + * needing to track if the fd is owned by the framework + * or by the caller. + */ + guest_memfd = kvm_dup(guest_memfd); + } + + region->region.guest_memfd = guest_memfd; + region->region.guest_memfd_offset = guest_memfd_offset; + } else { + region->region.guest_memfd = -1; + } + region->fd = -1; if (backing_src_is_shared(src_type)) region->fd = kvm_memfd_alloc(region->mmap_size, @@ -1055,28 +1078,6 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,

region->backing_src_type = src_type;

- if (flags & KVM_MEM_GUEST_MEMFD) { - if (guest_memfd < 0) { - uint32_t guest_memfd_flags = 0; - TEST_ASSERT(!guest_memfd_offset, - "Offset must be zero when creating new guest_memfd"); - guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags); - } else { - /* - * Install a unique fd for each memslot so that the fd - * can be closed when the region is deleted without - * needing to track if the fd is owned by the framework - * or by the caller. - */ - guest_memfd = kvm_dup(guest_memfd); - } - - region->region.guest_memfd = guest_memfd; - region->region.guest_memfd_offset = guest_memfd_offset; - } else { - region->region.guest_memfd = -1; - } - region->unused_phy_pages = sparsebit_alloc(); if (vm_arch_has_protected_memory(vm)) region->protected_phy_pages = sparsebit_alloc();

From: Sean Christopherson seanjc@google.com

Accept gmem_flags in vm_mem_add() to be able to create a guest_memfd within vm_mem_add().

When vm_mem_add() is used to set up a guest_memfd for a memslot, set up the provided (or created) gmem_fd as the fd for the user memory region. This makes it available to be mmap()-ed from just like fds from other memory sources. mmap() from guest_memfd using the provided gmem_flags and gmem_offset.

Add a kvm_slot_to_fd() helper to provide convenient access to the file descriptor of a memslot.

Update existing callers of vm_mem_add() to pass 0 for gmem_flags to preserve existing behavior.

Signed-off-by: Sean Christopherson seanjc@google.com [For guest_memfds, mmap() using gmem_offset instead of 0 all the time.] Signed-off-by: Ackerley Tng ackerleytng@google.com --- tools/testing/selftests/kvm/include/kvm_util.h | 7 ++++++- tools/testing/selftests/kvm/lib/kvm_util.c | 18 ++++++++++-------- .../kvm/x86/private_mem_conversions_test.c | 2 +- 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 45159638d5dde..de8ae9be19067 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -678,7 +678,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, uint32_t flags); void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, uint64_t gpa, uint32_t slot, uint64_t npages, uint32_t flags, - int guest_memfd_fd, uint64_t guest_memfd_offset); + int gmem_fd, uint64_t gmem_offset, uint64_t gmem_flags);

#ifndef vm_arch_has_protected_memory static inline bool vm_arch_has_protected_memory(struct kvm_vm *vm) @@ -711,6 +711,11 @@ void *addr_gva2hva(struct kvm_vm *vm, vm_vaddr_t gva); vm_paddr_t addr_hva2gpa(struct kvm_vm *vm, void *hva); void *addr_gpa2alias(struct kvm_vm *vm, vm_paddr_t gpa);

+static inline int kvm_slot_to_fd(struct kvm_vm *vm, uint32_t slot) +{ + return memslot2region(vm, slot)->fd; +} + #ifndef vcpu_arch_put_guest #define vcpu_arch_put_guest(mem, val) do { (mem) = (val); } while (0) #endif diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 8b714270cf381..19c0445c0b296 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -944,12 +944,13 @@ void vm_set_user_memory_region2(struct kvm_vm *vm, uint32_t slot, uint32_t flags /* FIXME: This thing needs to be ripped apart and rewritten. */ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, uint64_t gpa, uint32_t slot, uint64_t npages, uint32_t flags, - int gmem_fd, uint64_t gmem_offset) + int gmem_fd, uint64_t gmem_offset, uint64_t gmem_flags) { int ret; struct userspace_mem_region *region; size_t backing_src_pagesz = get_backing_src_pagesz(src_type); size_t mem_size = npages * vm->page_size; + off_t mmap_offset; size_t alignment;

TEST_REQUIRE_SET_USER_MEMORY_REGION2(); @@ -1028,8 +1029,6 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,

if (flags & KVM_MEM_GUEST_MEMFD) { if (gmem_fd < 0) { - uint32_t gmem_flags = 0; - TEST_ASSERT(!gmem_offset, "Offset must be zero when creating new guest_memfd"); gmem_fd = vm_create_guest_memfd(vm, mem_size, gmem_flags); @@ -1050,13 +1049,16 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, }

region->fd = -1; - if (backing_src_is_shared(src_type)) + if (flags & KVM_MEM_GUEST_MEMFD && gmem_flags & GUEST_MEMFD_FLAG_MMAP) + region->fd = kvm_dup(gmem_fd); + else if (backing_src_is_shared(src_type)) region->fd = kvm_memfd_alloc(region->mmap_size, src_type == VM_MEM_SRC_SHARED_HUGETLB);

- region->mmap_start = kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE, - vm_mem_backing_src_alias(src_type)->flag, - region->fd); + mmap_offset = flags & KVM_MEM_GUEST_MEMFD ? gmem_offset : 0; + region->mmap_start = __kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE, + vm_mem_backing_src_alias(src_type)->flag, + region->fd, mmap_offset);

TEST_ASSERT(!is_backing_src_hugetlb(src_type) || region->mmap_start == align_ptr_up(region->mmap_start, backing_src_pagesz), @@ -1117,7 +1119,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm, uint64_t gpa, uint32_t slot, uint64_t npages, uint32_t flags) { - vm_mem_add(vm, src_type, gpa, slot, npages, flags, -1, 0); + vm_mem_add(vm, src_type, gpa, slot, npages, flags, -1, 0, 0); }

/* diff --git a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c index 1969f4ab9b280..41f6b38f04071 100644 --- a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c +++ b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c @@ -399,7 +399,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t for (i = 0; i < nr_memslots; i++) vm_mem_add(vm, src_type, BASE_DATA_GPA + slot_size * i, BASE_DATA_SLOT + i, slot_size / vm->page_size, - KVM_MEM_GUEST_MEMFD, memfd, slot_size * i); + KVM_MEM_GUEST_MEMFD, memfd, slot_size * i, 0);

for (i = 0; i < nr_vcpus; i++) { uint64_t gpa = BASE_DATA_GPA + i * per_cpu_size;

Update KVM selftest framework to use KVM_SET_MEMORY_ATTRIBUTES2 and the accompanying struct kvm_memory_attributes2.

Signed-off-by: Ackerley Tng ackerleytng@google.com --- tools/testing/selftests/kvm/include/kvm_util.h | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index de8ae9be19067..019ffcec4510f 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -394,24 +394,30 @@ static inline void vm_enable_cap(struct kvm_vm *vm, uint32_t cap, uint64_t arg0) vm_ioctl(vm, KVM_ENABLE_CAP, &enable_cap); }

+#define TEST_REQUIRE_SET_MEMORY_ATTRIBUTES2() \ + __TEST_REQUIRE(kvm_has_cap(KVM_CAP_MEMORY_ATTRIBUTES2), \ + "KVM selftests now require KVM_SET_MEMORY_ATTRIBUTES2") + static inline void vm_set_memory_attributes(struct kvm_vm *vm, uint64_t gpa, uint64_t size, uint64_t attributes) { - struct kvm_memory_attributes attr = { + struct kvm_memory_attributes2 attr = { .attributes = attributes, .address = gpa, .size = size, .flags = 0, };

+ TEST_REQUIRE_SET_MEMORY_ATTRIBUTES2(); + /* - * KVM_SET_MEMORY_ATTRIBUTES overwrites _all_ attributes. These flows + * KVM_SET_MEMORY_ATTRIBUTES2 overwrites _all_ attributes. These flows * need significant enhancements to support multiple attributes. */ TEST_ASSERT(!attributes || attributes == KVM_MEMORY_ATTRIBUTE_PRIVATE, "Update me to support multiple attributes!");

- vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &attr); + vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES2, &attr); }

Add a selftest for the guest_memfd memory attribute conversion ioctls. The test starts the guest_memfd as all-private (the default state), and verifies the basic flow of converting a single page to shared and then back to private.

Add infrastructure that supports extensions to other conversion flow tests. This infrastructure will be used in upcoming patches for other conversion tests.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- tools/testing/selftests/kvm/Makefile.kvm | 1 + .../kvm/guest_memfd_conversions_test.c | 207 ++++++++++++++++++ 2 files changed, 208 insertions(+) create mode 100644 tools/testing/selftests/kvm/guest_memfd_conversions_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index 148d427ff24be..ddc1bdd51b834 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -141,6 +141,7 @@ TEST_GEN_PROGS_x86 += access_tracking_perf_test TEST_GEN_PROGS_x86 += coalesced_io_test TEST_GEN_PROGS_x86 += dirty_log_perf_test TEST_GEN_PROGS_x86 += guest_memfd_test +TEST_GEN_PROGS_x86 += guest_memfd_conversions_test TEST_GEN_PROGS_x86 += hardware_disable_test TEST_GEN_PROGS_x86 += memslot_modification_stress_test TEST_GEN_PROGS_x86 += memslot_perf_test diff --git a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c new file mode 100644 index 0000000000000..e0370e92e1b24 --- /dev/null +++ b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c @@ -0,0 +1,207 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2024, Google LLC. + */ +#include <sys/mman.h> +#include <unistd.h> + +#include <linux/align.h> +#include <linux/kvm.h> +#include <linux/sizes.h> + +#include "kvm_util.h" +#include "kselftest_harness.h" +#include "test_util.h" +#include "ucall_common.h" + +FIXTURE(gmem_conversions) { + struct kvm_vcpu *vcpu; + int gmem_fd; + /* HVA of the first byte of the memory mmap()-ed from gmem_fd. */ + char *mem; +}; + +typedef FIXTURE_DATA(gmem_conversions) test_data_t; + +FIXTURE_SETUP(gmem_conversions) { } + +static uint64_t page_size; + +static void guest_do_rmw(void); +#define GUEST_MEMFD_SHARING_TEST_GVA 0x90000000ULL + +/* + * Defer setup until the individual test is invoked so that tests can specify + * the number of pages and flags for the guest_memfd instance. + */ +static void gmem_conversions_do_setup(test_data_t *t, int nr_pages, + int gmem_flags) +{ + const struct vm_shape shape = { + .mode = VM_MODE_DEFAULT, + .type = KVM_X86_SW_PROTECTED_VM, + }; + /* + * Use high GPA above APIC_DEFAULT_PHYS_BASE to avoid clashing with + * APIC_DEFAULT_PHYS_BASE. + */ + const uint64_t gpa = SZ_4G; + const uint32_t slot = 1; + struct kvm_vm *vm; + + vm = __vm_create_shape_with_one_vcpu(shape, &t->vcpu, nr_pages, guest_do_rmw); + + vm_mem_add(vm, VM_MEM_SRC_SHMEM, gpa, slot, nr_pages, + KVM_MEM_GUEST_MEMFD, -1, 0, gmem_flags); + + t->gmem_fd = kvm_slot_to_fd(vm, slot); + t->mem = addr_gpa2hva(vm, gpa); + virt_map(vm, GUEST_MEMFD_SHARING_TEST_GVA, gpa, nr_pages); +} + +static void gmem_conversions_do_teardown(test_data_t *t) +{ + /* No need to close gmem_fd, it's owned by the VM structure. */ + kvm_vm_free(t->vcpu->vm); +} + +FIXTURE_TEARDOWN(gmem_conversions) +{ + gmem_conversions_do_teardown(self); +} + +/* + * In these test definition macros, __nr_pages and nr_pages is used to set up + * the total number of pages in the guest_memfd under test. This will be + * available in the test definitions as nr_pages. + */ + +#define __GMEM_CONVERSION_TEST(test, __nr_pages, flags) \ +static void __gmem_conversions_##test(test_data_t *t, int nr_pages); \ + \ +TEST_F(gmem_conversions, test) \ +{ \ + gmem_conversions_do_setup(self, __nr_pages, flags); \ + __gmem_conversions_##test(self, __nr_pages); \ +} \ +static void __gmem_conversions_##test(test_data_t *t, int nr_pages) \ + +#define GMEM_CONVERSION_TEST(test, __nr_pages, flags) \ + __GMEM_CONVERSION_TEST(test, __nr_pages, (flags) | GUEST_MEMFD_FLAG_MMAP) + +#define __GMEM_CONVERSION_TEST_INIT_PRIVATE(test, __nr_pages) \ + GMEM_CONVERSION_TEST(test, __nr_pages, 0) + +#define GMEM_CONVERSION_TEST_INIT_PRIVATE(test) \ + __GMEM_CONVERSION_TEST_INIT_PRIVATE(test, 1) + +struct guest_check_data { + void *mem; + char expected_val; + char write_val; +}; +static struct guest_check_data guest_data; + +static void guest_do_rmw(void) +{ + for (;;) { + char *mem = READ_ONCE(guest_data.mem); + + GUEST_ASSERT_EQ(READ_ONCE(*mem), READ_ONCE(guest_data.expected_val)); + WRITE_ONCE(*mem, READ_ONCE(guest_data.write_val)); + + GUEST_SYNC(0); + } +} + +static void run_guest_do_rmw(struct kvm_vcpu *vcpu, loff_t pgoff, + char expected_val, char write_val) +{ + struct ucall uc; + int r; + + guest_data.mem = (void *)GUEST_MEMFD_SHARING_TEST_GVA + pgoff * page_size; + guest_data.expected_val = expected_val; + guest_data.write_val = write_val; + sync_global_to_guest(vcpu->vm, guest_data); + + for (;;) { + r = __vcpu_run(vcpu); + if (!r && get_ucall(vcpu, &uc) == UCALL_PRINTF) { + REPORT_GUEST_PRINTF(uc); + continue; + } + if (r == -1 && errno == EINTR) + continue; + break; + } + + TEST_ASSERT_EQ(r, 0); + + switch (get_ucall(vcpu, &uc)) { + case UCALL_ABORT: + REPORT_GUEST_ASSERT(uc); + case UCALL_SYNC: + break; + case UCALL_PRINTF: + default: + TEST_FAIL("Unexpected ucall %lu", uc.cmd); + } +} + +static void host_do_rmw(char *mem, loff_t pgoff, char expected_val, + char write_val) +{ + TEST_ASSERT_EQ(READ_ONCE(mem[pgoff * page_size]), expected_val); + WRITE_ONCE(mem[pgoff * page_size], write_val); +} + +static void test_private(test_data_t *t, loff_t pgoff, char starting_val, + char write_val) +{ + TEST_EXPECT_SIGBUS(WRITE_ONCE(t->mem[pgoff * page_size], write_val)); + run_guest_do_rmw(t->vcpu, pgoff, starting_val, write_val); + TEST_EXPECT_SIGBUS(READ_ONCE(t->mem[pgoff * page_size])); +} + +static void test_convert_to_private(test_data_t *t, loff_t pgoff, + char starting_val, char write_val) +{ + gmem_set_private(t->gmem_fd, pgoff * page_size, page_size); + test_private(t, pgoff, starting_val, write_val); +} + +static void test_shared(test_data_t *t, loff_t pgoff, char starting_val, + char host_write_val, char write_val) +{ + host_do_rmw(t->mem, pgoff, starting_val, host_write_val); + run_guest_do_rmw(t->vcpu, pgoff, host_write_val, write_val); + TEST_ASSERT_EQ(READ_ONCE(t->mem[pgoff * page_size]), write_val); +} + +static void test_convert_to_shared(test_data_t *t, loff_t pgoff, + char starting_val, char host_write_val, + char write_val) +{ + gmem_set_shared(t->gmem_fd, pgoff * page_size, page_size); + test_shared(t, pgoff, starting_val, host_write_val, write_val); +} + +GMEM_CONVERSION_TEST_INIT_PRIVATE(init_private) +{ + test_private(t, 0, 0, 'A'); + test_convert_to_shared(t, 0, 'A', 'B', 'C'); + test_convert_to_private(t, 0, 'C', 'E'); +} + + +int main(int argc, char *argv[]) +{ + TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM)); + TEST_REQUIRE(kvm_check_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) & + KVM_MEMORY_ATTRIBUTE_PRIVATE); + + page_size = getpagesize(); + + return test_harness_run(argc, argv); +}

The existing guest_memfd conversion tests only use single-page memory regions. This provides no coverage for multi-page guest_memfd objects, specifically whether KVM correctly handles the page index for conversion operations. An incorrect implementation could, for example, always operate on the first page regardless of the index provided.

Add a new test case to verify that conversions between private and shared memory correctly target the specified page within a multi-page guest_memfd.

To support this test, add a new GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED macro that handles setting up and tearing down the VM for each page iteration. The teardown logic is adjusted to prevent a double-free in this new scenario.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- .../kvm/guest_memfd_conversions_test.c | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c index d57e66ee11310..54e7deec992d4 100644 --- a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c @@ -63,6 +63,9 @@ static void gmem_conversions_do_teardown(test_data_t *t) { /* No need to close gmem_fd, it's owned by the VM structure. */ kvm_vm_free(t->vcpu->vm); + + /* NULL this out to avoid second free on full teardown in multipage tests. */ + t->vcpu->vm = NULL; }

FIXTURE_TEARDOWN(gmem_conversions) @@ -101,6 +104,29 @@ static void __gmem_conversions_##test(test_data_t *t, int nr_pages) \ #define GMEM_CONVERSION_TEST_INIT_SHARED(test) \ __GMEM_CONVERSION_TEST_INIT_SHARED(test, 1)

+/* + * Repeats test over nr_pages in a guest_memfd of size nr_pages, providing each + * test iteration with test_page, the index of the page under test in + * guest_memfd. test_page takes values 0..(nr_pages - 1) inclusive. + */ +#define GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED(test, __nr_pages) \ +static void __gmem_conversions_multipage_##test(test_data_t *t, int nr_pages, \ + const int test_page); \ + \ +TEST_F(gmem_conversions, test) \ +{ \ + const uint64_t flags = GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_INIT_SHARED; \ + int i; \ + \ + for (i = 0; i < __nr_pages; ++i) { \ + gmem_conversions_do_setup(self, __nr_pages, flags); \ + __gmem_conversions_multipage_##test(self, __nr_pages, i); \ + gmem_conversions_do_teardown(self); \ + } \ +} \ +static void __gmem_conversions_multipage_##test(test_data_t *t, int nr_pages, \ + const int test_page) + struct guest_check_data { void *mem; char expected_val; @@ -207,6 +233,36 @@ GMEM_CONVERSION_TEST_INIT_SHARED(init_shared) test_convert_to_shared(t, 0, 'C', 'D', 'E'); }

+/* + * Test indexing of pages within guest_memfd, using test data that is a multiple + * of page index. + */ +GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED(indexing, 4) +{ + int i; + + /* + * Start with the highest index, to catch any errors when, perhaps, the + * first page is returned even for the last index. + */ + for (i = nr_pages - 1; i >= 0; --i) + test_shared(t, i, 0, i, i * 2); + + for (i = 0; i < nr_pages; ++i) { + if (i == test_page) + test_convert_to_private(t, i, i * 2, i * 4); + else + test_shared(t, i, i * 2, i * 3, i * 4); + } + + for (i = 0; i < nr_pages; ++i) { + if (i == test_page) + test_convert_to_shared(t, i, i * 4, i * 5, i * 6); + else + test_shared(t, i, i * 4, i * 5, i * 6); + } +} + int main(int argc, char *argv[]) { TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));

Add a guest_memfd selftest to verify that memory conversions work correctly with allocated folios in different layouts.

By iterating through which pages are initially faulted, the test covers various layouts of contiguous allocated and unallocated regions, exercising conversion with different range layouts.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- .../kvm/guest_memfd_conversions_test.c | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c index 3b222009227c3..b42b1b27cb727 100644 --- a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c @@ -277,6 +277,36 @@ GMEM_CONVERSION_TEST_INIT_PRIVATE(before_allocation_private) test_convert_to_shared(t, 0, 0, 'A', 'B'); }

+/* + * Test that when some of the folios in the conversion range are allocated, + * conversion requests are handled correctly in guest_memfd. Vary the ranges + * allocated before conversion, using test_page, to cover various layouts of + * contiguous allocated and unallocated regions. + */ +GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED(unallocated_folios, 8) +{ + const int second_page_to_fault = 4; + int i; + + /* + * Fault 2 of the pages to test filemap range operations except when + * test_page == second_page_to_fault. + */ + host_do_rmw(t->mem, test_page, 0, 'A'); + if (test_page != second_page_to_fault) + host_do_rmw(t->mem, second_page_to_fault, 0, 'A'); + + gmem_set_private(t->gmem_fd, 0, nr_pages * page_size); + for (i = 0; i < nr_pages; ++i) { + char expected = (i == test_page || i == second_page_to_fault) ? 'A' : 0; + + test_private(t, i, expected, 'B'); + } + + for (i = 0; i < nr_pages; ++i) + test_convert_to_shared(t, i, 'B', 'C', 'D'); +} + int main(int argc, char *argv[]) { TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));

Add a test to verify that deallocating a page in a guest memfd region via fallocate() with FALLOC_FL_PUNCH_HOLE does not alter the shared or private status of the corresponding memory range.

When a page backing a guest memfd mapping is deallocated, e.g., by punching a hole or truncating the file, and then subsequently faulted back in, the new page must inherit the correct shared/private status tracked by guest_memfd.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- .../selftests/kvm/guest_memfd_conversions_test.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)

diff --git a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c index 43efe4af1403c..907d415d72315 100644 --- a/tools/testing/selftests/kvm/guest_memfd_conversions_test.c +++ b/tools/testing/selftests/kvm/guest_memfd_conversions_test.c @@ -10,6 +10,7 @@ #include <linux/sizes.h>

#include "kvm_util.h" +#include "kvm_syscalls.h" #include "kselftest_harness.h" #include "test_util.h" #include "ucall_common.h" @@ -316,6 +317,19 @@ GMEM_CONVERSION_MULTIPAGE_TEST_INIT_SHARED(unallocated_folios, 8) test_convert_to_shared(t, i, 'B', 'C', 'D'); }

+/* Truncation should not affect shared/private status. */ +GMEM_CONVERSION_TEST_INIT_SHARED(truncate) +{ + host_do_rmw(t->mem, 0, 0, 'A'); + kvm_fallocate(t->gmem_fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, 0, page_size); + host_do_rmw(t->mem, 0, 0, 'A'); + + test_convert_to_private(t, 0, 'A', 'B'); + + kvm_fallocate(t->gmem_fd, FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE, 0, page_size); + test_private(t, 0, 0, 'A'); +} + int main(int argc, char *argv[]) { TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));

The private_mem_conversions_test resets shared memory to an initial pattern at the end of each test iteration. This reset is currently performed before the (re)mapping pages as shared.

FALLOC_FL_PUNCH_HOLE indirectly zeroes memory, since old folios were released and new folios are zeroed. This "clobbers" the intended initial pattern, leaving the memory as all-zeroes for the next iteration.

Move the memset() to occur after the hole-punch operation to ensure the memory is correctly re-initialized with the desired pattern. While at it, update the memset() to reset the entire data region, not just the portion used in the last loop, to provide a fully clean slate for the next iteration.

This was not observed before because guest_memfd was only used for private memory, hence shared memory contents were not zeroed by the hole punch operation.

Opportunistically add a test/check that truncation zeroes memory.

Fixes: 43f623f350ce1 ("KVM: selftests: Add x86-only selftest for private memory conversions") Signed-off-by: Ackerley Tng ackerleytng@google.com --- .../selftests/kvm/x86/private_mem_conversions_test.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c index 41f6b38f04071..814187d06fcca 100644 --- a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c +++ b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c @@ -202,15 +202,20 @@ static void guest_test_explicit_conversion(uint64_t base_gpa, bool do_fallocate) guest_sync_shared(gpa, size, p3, p4); memcmp_g(gpa, p4, size);

- /* Reset the shared memory back to the initial pattern. */ - memset((void *)gpa, init_p, size); - /* * Free (via PUNCH_HOLE) *all* private memory so that the next * iteration starts from a clean slate, e.g. with respect to * whether or not there are pages/folios in guest_mem. */ guest_map_shared(base_gpa, PER_CPU_DATA_SIZE, true); + + /* + * Test that fallocate(PUNCH_HOLE) because hole-punching zeroes + * memory, then reset the entire block back to the initial + * pattern for the next GUEST_STAGE. + */ + memcmp_g(base_gpa, 0, PER_CPU_DATA_SIZE); + memset((void *)base_gpa, init_p, PER_CPU_DATA_SIZE); } }

Introduce a new helper, kvm_gpa_to_guest_memfd(), to find the guest_memfd-related details of a memory region that contains a given guest physical address (GPA).

The function returns the file descriptor for the memfd, the offset into the file that corresponds to the GPA, and the number of bytes remaining in the region from that GPA.

kvm_gpa_to_guest_memfd() was factored out from vm_guest_mem_fallocate(); refactor vm_guest_mem_fallocate() to use the new helper.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- .../testing/selftests/kvm/include/kvm_util.h | 3 ++ tools/testing/selftests/kvm/lib/kvm_util.c | 34 ++++++++++++------- 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index dd26a41106fae..e9c2696770cf0 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -404,6 +404,9 @@ static inline void vm_enable_cap(struct kvm_vm *vm, uint32_t cap, uint64_t arg0) vm_ioctl(vm, KVM_ENABLE_CAP, &enable_cap); }

+int kvm_gpa_to_guest_memfd(struct kvm_vm *vm, vm_paddr_t gpa, off_t *fd_offset, + uint64_t *nr_bytes); + #define TEST_REQUIRE_SET_MEMORY_ATTRIBUTES2() \ __TEST_REQUIRE(kvm_has_cap(KVM_CAP_MEMORY_ATTRIBUTES2), \ "KVM selftests now require KVM_SET_MEMORY_ATTRIBUTES2") diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index c9c59f3ecd14f..cb73566fdf153 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1244,27 +1244,19 @@ void vm_guest_mem_fallocate(struct kvm_vm *vm, uint64_t base, uint64_t size, bool punch_hole) { const int mode = FALLOC_FL_KEEP_SIZE | (punch_hole ? FALLOC_FL_PUNCH_HOLE : 0); - struct userspace_mem_region *region; uint64_t end = base + size; uint64_t gpa, len; off_t fd_offset; - int ret; + int fd, ret;

for (gpa = base; gpa < end; gpa += len) { - uint64_t offset; + fd = kvm_gpa_to_guest_memfd(vm, gpa, &fd_offset, &len); + len = min(end - gpa, len);

- region = userspace_mem_region_find(vm, gpa, gpa); - TEST_ASSERT(region && region->region.flags & KVM_MEM_GUEST_MEMFD, - "Private memory region not found for GPA 0x%lx", gpa); - - offset = gpa - region->region.guest_phys_addr; - fd_offset = region->region.guest_memfd_offset + offset; - len = min_t(uint64_t, end - gpa, region->region.memory_size - offset); - - ret = fallocate(region->region.guest_memfd, mode, fd_offset, len); + ret = fallocate(fd, mode, fd_offset, len); TEST_ASSERT(!ret, "fallocate() failed to %s at %lx (len = %lu), fd = %d, mode = %x, offset = %lx", punch_hole ? "punch hole" : "allocate", gpa, len, - region->region.guest_memfd, mode, fd_offset); + fd, mode, fd_offset); } }

@@ -1673,6 +1665,22 @@ void *addr_gpa2alias(struct kvm_vm *vm, vm_paddr_t gpa) return (void *) ((uintptr_t) region->host_alias + offset); }

+int kvm_gpa_to_guest_memfd(struct kvm_vm *vm, vm_paddr_t gpa, off_t *fd_offset, + uint64_t *nr_bytes) +{ + struct userspace_mem_region *region; + vm_paddr_t gpa_offset; + + region = userspace_mem_region_find(vm, gpa, gpa); + TEST_ASSERT(region && region->region.flags & KVM_MEM_GUEST_MEMFD, + "guest_memfd memory region not found for GPA 0x%lx", gpa); + + gpa_offset = gpa - region->region.guest_phys_addr; + *fd_offset = region->region.guest_memfd_offset + gpa_offset; + *nr_bytes = region->region.memory_size - gpa_offset; + return region->fd; +} + /* Create an interrupt controller chip for the specified VM. */ void vm_create_irqchip(struct kvm_vm *vm) {

From: Sean Christopherson seanjc@google.com

Check that a valid fd provided to mmap() must be accompanied by MAP_SHARED.

With an invalid fd (usually used for anonymous mappings), there are no constraints on mmap() flags.

Add this check to make sure that when a guest_memfd is used as region->fd, the flag provided to mmap() will include MAP_SHARED.

Signed-off-by: Sean Christopherson seanjc@google.com [Rephrase assertion message.] Signed-off-by: Ackerley Tng ackerleytng@google.com --- tools/testing/selftests/kvm/lib/kvm_util.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index cb73566fdf153..8603bd5c705ed 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -1057,6 +1057,9 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, region->fd = kvm_memfd_alloc(region->mmap_size, src_type == VM_MEM_SRC_SHARED_HUGETLB);

+ TEST_ASSERT(region->fd == -1 || backing_src_is_shared(src_type), + "A valid fd provided to mmap() must be accompanied by MAP_SHARED."); + mmap_offset = flags & KVM_MEM_GUEST_MEMFD ? gmem_offset : 0; region->mmap_start = __kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE, vm_mem_backing_src_alias(src_type)->flag,

Update the private memory conversions selftest to also test conversions that are done "in-place" via per-guest_memfd memory attributes. In-place conversions require the host to be able to mmap() the guest_memfd so that the host and guest can share the same backing physical memory.

This includes several updates, that are conditioned on the system supporting per-guest_memfd attributes (kvm_has_gmem_attributes):

1. Set up guest_memfd requesting MMAP and INIT_SHARED.

2. With in-place conversions, the host's mapping points directly to the guest's memory. When the guest converts a region to private, host access to that region is blocked. Update the test to expect a SIGBUS when attempting to access the host virtual address (HVA) of private memory.

3. Use vm_mem_set_memory_attributes(), which chooses how to set memory attributes based on whether kvm_has_gmem_attributes.

Restrict the test to using VM_MEM_SRC_SHMEM because guest_memfd's required mmap() flags and page sizes happens to align with those of VM_MEM_SRC_SHMEM. As long as VM_MEM_SRC_SHMEM is used for src_type, vm_mem_add() works as intended.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- .../kvm/x86/private_mem_conversions_test.c | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c index 814187d06fcca..6730923af830c 100644 --- a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c +++ b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c @@ -309,8 +309,8 @@ static void handle_exit_hypercall(struct kvm_vcpu *vcpu) vm_guest_mem_fallocate(vm, gpa, size, map_shared);

if (set_attributes) - vm_set_memory_attributes(vm, gpa, size, - map_shared ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE); + vm_mem_set_memory_attributes(vm, gpa, size, + map_shared ? 0 : KVM_MEMORY_ATTRIBUTE_PRIVATE); run->hypercall.ret = 0; }

@@ -354,8 +354,20 @@ static void *__test_mem_conversions(void *__vcpu) size_t nr_bytes = min_t(size_t, vm->page_size, size - i); uint8_t *hva = addr_gpa2hva(vm, gpa + i);

- /* In all cases, the host should observe the shared data. */ - memcmp_h(hva, gpa + i, uc.args[3], nr_bytes); + /* + * When using per-guest_memfd memory attributes, + * i.e. in-place conversion, host accesses will + * point at guest memory and should SIGBUS when + * guest memory is private. When using per-VM + * attributes, i.e. separate backing for shared + * vs. private, the host should always observe + * the shared data. + */ + if (kvm_has_gmem_attributes && + uc.args[0] == SYNC_PRIVATE) + TEST_EXPECT_SIGBUS(READ_ONCE(*hva)); + else + memcmp_h(hva, gpa + i, uc.args[3], nr_bytes);

/* For shared, write the new pattern to guest memory. */ if (uc.args[0] == SYNC_SHARED) @@ -384,6 +396,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t const size_t slot_size = memfd_size / nr_memslots; struct kvm_vcpu *vcpus[KVM_MAX_VCPUS]; pthread_t threads[KVM_MAX_VCPUS]; + uint64_t gmem_flags; struct kvm_vm *vm; int memfd, i;

@@ -399,12 +412,17 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, uint32_t

vm_enable_cap(vm, KVM_CAP_EXIT_HYPERCALL, (1 << KVM_HC_MAP_GPA_RANGE));

- memfd = vm_create_guest_memfd(vm, memfd_size, 0); + if (kvm_has_gmem_attributes) + gmem_flags = GUEST_MEMFD_FLAG_MMAP | GUEST_MEMFD_FLAG_INIT_SHARED; + else + gmem_flags = 0; + + memfd = vm_create_guest_memfd(vm, memfd_size, gmem_flags);

for (i = 0; i < nr_memslots; i++) vm_mem_add(vm, src_type, BASE_DATA_GPA + slot_size * i, BASE_DATA_SLOT + i, slot_size / vm->page_size, - KVM_MEM_GUEST_MEMFD, memfd, slot_size * i, 0); + KVM_MEM_GUEST_MEMFD, memfd, slot_size * i, gmem_flags);

for (i = 0; i < nr_vcpus; i++) { uint64_t gpa = BASE_DATA_GPA + i * per_cpu_size; @@ -454,17 +472,29 @@ static void usage(const char *cmd)

int main(int argc, char *argv[]) { - enum vm_mem_backing_src_type src_type = DEFAULT_VM_MEM_SRC; + enum vm_mem_backing_src_type src_type; uint32_t nr_memslots = 1; uint32_t nr_vcpus = 1; int opt;

TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));

+ src_type = kvm_has_gmem_attributes ? VM_MEM_SRC_SHMEM : + DEFAULT_VM_MEM_SRC; + while ((opt = getopt(argc, argv, "hm:s:n:")) != -1) { switch (opt) { case 's': src_type = parse_backing_src_type(optarg); + if (kvm_has_gmem_attributes && src_type != VM_MEM_SRC_SHMEM) { + printf("Overriding mem_type to %s to test in-place conversions\n", + vm_mem_backing_src_alias(VM_MEM_SRC_SHMEM)->name); + /* + * Use VM_MEM_SRC_SHMEM, whose size and mmap flags + * align with those of guest_memfd. + */ + src_type = VM_MEM_SRC_SHMEM; + } break; case 'n': nr_vcpus = atoi_positive("nr_vcpus", optarg);

From: Sean Christopherson seanjc@google.com

Skip setting memory to private in the pre-fault memory test when using per-gmem memory attributes, as memory is initialized to private by default for guest_memfd, and using vm_mem_set_private() on a guest_memfd instance requires creating guest_memfd with GUEST_MEMFD_FLAG_MMAP (which is totally doable, but would need to be conditional and is ultimately unnecessary).

Signed-off-by: Sean Christopherson seanjc@google.com --- tools/testing/selftests/kvm/pre_fault_memory_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/pre_fault_memory_test.c b/tools/testing/selftests/kvm/pre_fault_memory_test.c index 6db75946a4f89..6bb5e52f6d948 100644 --- a/tools/testing/selftests/kvm/pre_fault_memory_test.c +++ b/tools/testing/selftests/kvm/pre_fault_memory_test.c @@ -188,7 +188,7 @@ static void __test_pre_fault_memory(unsigned long vm_type, bool private) private ? KVM_MEM_GUEST_MEMFD : 0); virt_map(vm, gva, gpa, TEST_NPAGES);

- if (private) + if (!kvm_has_gmem_attributes && private) vm_mem_set_private(vm, gpa, TEST_SIZE);

pre_fault_memory(vcpu, gpa, 0, SZ_2M, 0, private);