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

Implement kvm_gmem_get_memory_attributes() for guest_memfd to allow the KVM core and architecture code to query per-GFN memory attributes.

kvm_gmem_get_memory_attributes() finds the memory slot for a given GFN and queries the guest_memfd file's to determine if the page is marked as private.

If vm_memory_attributes is not enabled, there is no shared/private tracking at the VM level. Install the guest_memfd implementation as long as guest_memfd is enabled to give guest_memfd a chance to respond on attributes.

guest_memfd should look up attributes regardless of whether this memslot is gmem-only since attributes are now tracked by gmem regardless of whether mmap() is enabled.

Signed-off-by: Sean Christopherson seanjc@google.com Co-developed-by: Ackerley Tng ackerleytng@google.com Signed-off-by: Ackerley Tng ackerleytng@google.com --- include/linux/kvm_host.h | 2 ++ virt/kvm/guest_memfd.c | 29 +++++++++++++++++++++++++++++ virt/kvm/kvm_main.c | 3 +++ 3 files changed, 34 insertions(+)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 512febf47c265..b8418cc5851f1 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -2543,6 +2543,8 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm, struct kvm_gfn_range *range); #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */

+unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn); + #ifdef CONFIG_KVM_GUEST_MEMFD int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot, gfn_t gfn, kvm_pfn_t *pfn, struct page **page, diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 26cec833766c3..f62facc3ab776 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -518,6 +518,35 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma) return 0; }

+unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn) +{ + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn); + + /* + * If this gfn has no associated memslot, there's no chance of the gfn + * being backed by private memory, since guest_memfd must be used for + * private memory, and guest_memfd must be associated with some memslot. + */ + if (!slot) + return 0; + + CLASS(gmem_get_file, file)(slot); + if (!file) + return false; + + /* + * Don't take the filemap invalidation lock, as temporarily acquiring + * that lock wouldn't provide any meaningful protection. The caller + * _must_ protect consumption of private vs. shared by checking + * mmu_invalidate_retry_gfn() under mmu_lock. + */ + guard(rcu)(); + + return kvm_gmem_get_attributes(file_inode(file), + kvm_gmem_get_index(slot, gfn)); +} +EXPORT_SYMBOL_GPL(kvm_gmem_get_memory_attributes); + static struct file_operations kvm_gmem_fops = { .mmap = kvm_gmem_mmap, .open = generic_file_open, diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 6c29770dfa7c8..c73ebdb73070e 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2660,6 +2660,9 @@ static void kvm_init_memory_attributes(void) if (vm_memory_attributes) static_call_update(__kvm_get_memory_attributes, kvm_get_vm_memory_attributes); + else if (IS_ENABLED(CONFIG_KVM_GUEST_MEMFD)) + static_call_update(__kvm_get_memory_attributes, + kvm_gmem_get_memory_attributes); else static_call_update(__kvm_get_memory_attributes, (void *)__static_call_return0);

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);

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,

For shared to private conversions, if refcounts on any of the folios within the range are elevated, fail the conversion with -EAGAIN.

At the point of shared to private conversion, all folios in range are also unmapped. The filemap_invalidate_lock() is held, so no faulting can occur. Hence, from that point on, only transient refcounts can be taken on the folios associated with that guest_memfd.

Hence, it is safe to do the conversion from shared to private.

After conversion is complete, refcounts may become elevated, but that is fine since users of transient refcounts don't actually access memory.

For private to shared conversions, there are no refcount checks. any transient refcounts are expected to drop their refcounts soon. The conversion process will spin waiting for these transient refcounts to go away.

Signed-off-by: Ackerley Tng ackerleytng@google.com Co-developed-by: Sean Christopherson seanjc@google.com Signed-off-by: Sean Christopherson seanjc@google.com --- Documentation/virt/kvm/api.rst | 48 +++++++- include/linux/kvm_host.h | 10 ++ include/uapi/linux/kvm.h | 9 +- virt/kvm/Kconfig | 2 +- virt/kvm/guest_memfd.c | 197 ++++++++++++++++++++++++++++++--- virt/kvm/kvm_main.c | 15 +-- 6 files changed, 250 insertions(+), 31 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index a812769d79bf6..156ee69fa9067 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -117,7 +117,7 @@ description: x86 includes both i386 and x86_64.

Type: - system, vm, or vcpu. + system, vm, vcpu or guest_memfd.

Parameters: what parameters are accepted by the ioctl. @@ -6518,11 +6518,22 @@ the capability to be present. ---------------------------------

:Capability: KVM_CAP_MEMORY_ATTRIBUTES2 -:Architectures: x86 -:Type: vm ioctl +:Architectures: all +:Type: vm, guest_memfd ioctl :Parameters: struct kvm_memory_attributes2 (in/out) :Returns: 0 on success, <0 on error

+Errors: + + ========== =============================================================== + EINVAL The specified `offset` or `size` were invalid (e.g. not + page aligned, causes an overflow, or size is zero). + EFAULT The parameter address was invalid. + EAGAIN Some page within requested range had unexpected refcounts. The + offset of the page will be returned in `error_offset`. + ENOMEM Ran out of memory trying to track private/shared state + ========== =============================================================== + 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 @@ -6533,15 +6544,42 @@ Attribute values are shared with KVM_SET_MEMORY_ATTRIBUTES. ::

struct kvm_memory_attributes2 { - __u64 address; + /* in */ + union { + __u64 address; + __u64 offset; + }; __u64 size; __u64 attributes; __u64 flags; - __u64 reserved[4]; + /* out */ + __u64 error_offset; + __u64 reserved[3]; };

#define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)

+Set attributes for a range of offsets within a guest_memfd to +KVM_MEMORY_ATTRIBUTE_PRIVATE to limit the specified guest_memfd backed +memory range for guest_use. Even if KVM_CAP_GUEST_MEMFD_MMAP is +supported, after a successful call to set +KVM_MEMORY_ATTRIBUTE_PRIVATE, the requested range will not be mappable +into host userspace and will only be mappable by the guest. + +To allow the range to be mappable into host userspace again, call +KVM_SET_MEMORY_ATTRIBUTES2 on the guest_memfd again with +KVM_MEMORY_ATTRIBUTE_PRIVATE unset. + +If this ioctl returns -EAGAIN, the offset of the page with unexpected +refcounts will be returned in `error_offset`. This can occur if there +are transient refcounts on the pages, taken by other parts of the +kernel. + +Userspace is expected to figure out how to remove all known refcounts +on the shared pages, such as refcounts taken by get_user_pages(), and +try the ioctl again. A possible source of these long term refcounts is +if the guest_memfd memory was pinned in IOMMU page tables. + See also: :ref: `KVM_SET_MEMORY_ATTRIBUTES`.

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index b48632ee242b3..962055309084c 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -2515,6 +2515,16 @@ static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot) }

#ifdef CONFIG_KVM_MEMORY_ATTRIBUTES +static inline 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; +} + 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);

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index c300e38c7c9cd..cdb00866efe49 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -964,6 +964,7 @@ struct kvm_enable_cap { #define KVM_CAP_ARM_CACHEABLE_PFNMAP_SUPPORTED 243 #define KVM_CAP_GUEST_MEMFD_FLAGS 244 #define KVM_CAP_MEMORY_ATTRIBUTES2 245 +#define KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES 246

struct kvm_irq_routing_irqchip { __u32 irqchip; @@ -1622,11 +1623,15 @@ struct kvm_pre_fault_memory { #define KVM_SET_MEMORY_ATTRIBUTES2 _IOWR(KVMIO, 0xd6, struct kvm_memory_attributes2)

struct kvm_memory_attributes2 { - __u64 address; + union { + __u64 address; + __u64 offset; + }; __u64 size; __u64 attributes; __u64 flags; - __u64 reserved[4]; + __u64 error_offset; + __u64 reserved[3]; };

#endif /* __LINUX_KVM_H */ diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 395996977fe5a..b3473aec4d24d 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -117,7 +117,7 @@ config KVM_VM_MEMORY_ATTRIBUTES bool

config KVM_GUEST_MEMFD - depends on KVM_GENERIC_MMU_NOTIFIER + select KVM_MEMORY_ATTRIBUTES select XARRAY_MULTI bool

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index dab2b3ce78bc8..5ec38e9395d22 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -73,6 +73,21 @@ static bool kvm_gmem_is_shared_mem(struct inode *inode, pgoff_t index) return !kvm_gmem_is_private_mem(inode, index); }

+static bool kvm_gmem_range_has_attributes(struct maple_tree *mt, + pgoff_t index, size_t nr_pages, + u64 attributes) +{ + pgoff_t end = index + nr_pages - 1; + void *entry; + + mt_for_each(mt, entry, index, end) { + if (xa_to_value(entry) != attributes) + return false; + } + + return true; +} + static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot, pgoff_t index, struct folio *folio) { @@ -219,10 +234,12 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)

static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *inode) { - if (GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED) - return KVM_FILTER_SHARED; - - return KVM_FILTER_PRIVATE; + /* + * TODO: Limit invalidations based on the to-be-invalidated range, i.e. + * invalidate shared/private if and only if there can possibly be + * such mappings. + */ + return KVM_FILTER_SHARED | KVM_FILTER_PRIVATE; }

static void __kvm_gmem_invalidate_begin(struct gmem_file *f, pgoff_t start, @@ -583,11 +600,172 @@ unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn) } EXPORT_SYMBOL_GPL(kvm_gmem_get_memory_attributes);

+static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start, + size_t nr_pages, pgoff_t *err_index) +{ + struct address_space *mapping = inode->i_mapping; + const int filemap_get_folios_refcount = 1; + pgoff_t last = start + nr_pages - 1; + struct folio_batch fbatch; + bool safe = true; + int i; + + folio_batch_init(&fbatch); + while (safe && filemap_get_folios(mapping, &start, last, &fbatch)) { + + for (i = 0; i < folio_batch_count(&fbatch); ++i) { + struct folio *folio = fbatch.folios[i]; + + if (folio_ref_count(folio) != + folio_nr_pages(folio) + filemap_get_folios_refcount) { + safe = false; + *err_index = folio->index; + break; + } + } + + folio_batch_release(&fbatch); + } + + return safe; +} + +/* + * Preallocate memory for attributes to be stored on a maple tree, pointed to + * by mas. Adjacent ranges with attributes identical to the new attributes + * will be merged. Also sets mas's bounds up for storing attributes. + * + * This maintains the invariant that ranges with the same attributes will + * always be merged. + */ +static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes, + pgoff_t start, size_t nr_pages) +{ + pgoff_t end = start + nr_pages; + pgoff_t last = end - 1; + void *entry; + + /* Try extending range. entry is NULL on overflow/wrap-around. */ + mas_set_range(mas, end, end); + entry = mas_find(mas, end); + if (entry && xa_to_value(entry) == attributes) + last = mas->last; + + mas_set_range(mas, start - 1, start - 1); + entry = mas_find(mas, start - 1); + if (entry && xa_to_value(entry) == attributes) + start = mas->index; + + mas_set_range(mas, start, last); + return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL); +} + +static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start, + size_t nr_pages, uint64_t attrs, + pgoff_t *err_index) +{ + struct address_space *mapping = inode->i_mapping; + struct gmem_inode *gi = GMEM_I(inode); + pgoff_t end = start + nr_pages; + struct maple_tree *mt; + struct ma_state mas; + int r; + + mt = &gi->attributes; + + filemap_invalidate_lock(mapping); + + mas_init(&mas, mt, start); + + if (kvm_gmem_range_has_attributes(mt, start, nr_pages, attrs)) + goto done; + + r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages); + if (r) { + *err_index = start; + goto out; + } + + unmap_mapping_pages(mapping, start, nr_pages, false); + + if (!kvm_gmem_is_safe_for_conversion(inode, start, nr_pages, err_index)) { + mas_destroy(&mas); + r = -EAGAIN; + goto out; + } + + kvm_gmem_invalidate_begin(inode, start, end); + + mas_store_prealloc(&mas, xa_mk_value(attrs)); + + kvm_gmem_invalidate_end(inode, start, end); +done: + r = 0; +out: + filemap_invalidate_unlock(mapping); + return r; +} + +static long kvm_gmem_set_attributes(struct file *file, void __user *argp) +{ + struct gmem_file *f = file->private_data; + struct inode *inode = file_inode(file); + struct kvm_memory_attributes2 attrs; + pgoff_t err_index; + size_t nr_pages; + pgoff_t index; + int r; + + if (copy_from_user(&attrs, argp, sizeof(attrs))) + return -EFAULT; + + if (attrs.flags) + return -EINVAL; + if (attrs.attributes & ~kvm_supported_mem_attributes(f->kvm)) + return -EINVAL; + if (attrs.size == 0 || attrs.offset + attrs.size < attrs.offset) + return -EINVAL; + if (!PAGE_ALIGNED(attrs.offset) || !PAGE_ALIGNED(attrs.offset)) + return -EINVAL; + + if (attrs.offset > inode->i_size || + attrs.offset + attrs.size > inode->i_size) + return -EINVAL; + + nr_pages = attrs.size >> PAGE_SHIFT; + index = attrs.offset >> PAGE_SHIFT; + r = __kvm_gmem_set_attributes(inode, index, nr_pages, attrs.attributes, + &err_index); + if (r) { + attrs.error_offset = err_index << PAGE_SHIFT; + + if (copy_to_user(argp, &attrs, sizeof(attrs))) + return -EFAULT; + } + + return r; +} + +static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl, + unsigned long arg) +{ + switch (ioctl) { + case KVM_SET_MEMORY_ATTRIBUTES2: + if (vm_memory_attributes) + return -ENOTTY; + + return kvm_gmem_set_attributes(file, (void __user *)arg); + default: + return -ENOTTY; + } +} + static struct file_operations kvm_gmem_fops = { .mmap = kvm_gmem_mmap, .open = generic_file_open, .release = kvm_gmem_release, .fallocate = kvm_gmem_fallocate, + .unlocked_ioctl = kvm_gmem_ioctl, };

static int kvm_gmem_migrate_folio(struct address_space *mapping, @@ -943,20 +1121,13 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn); 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; + return kvm_gmem_range_has_attributes(&gi->attributes, index, nr_pages, + KVM_MEMORY_ATTRIBUTE_PRIVATE); }

long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages, diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index dd84b377e46db..3506a2f2be041 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2435,16 +2435,6 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm, #endif /* CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */

#ifdef CONFIG_KVM_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; -} - #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES static unsigned long kvm_get_vm_memory_attributes(struct kvm *kvm, gfn_t gfn) { @@ -4971,6 +4961,11 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg) return 1; case KVM_CAP_GUEST_MEMFD_FLAGS: return kvm_gmem_get_supported_flags(kvm); + case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES: + if (vm_memory_attributes) + return 0; + + return kvm_supported_mem_attributes(kvm); #endif default: break;

From: Sean Christopherson seanjc@google.com

Make vm_memory_attributes a module parameter so that userspace can disable the use of memory attributes on the VM level.

To avoid inconsistencies in the way memory attributes are tracked in KVM and guest_memfd, the vm_memory_attributes module_param is made read-only (0444).

Make CONFIG_KVM_VM_MEMORY_ATTRIBUTES selectable, only for (CoCo) VM types that might use vm_memory_attributes.

Signed-off-by: Sean Christopherson seanjc@google.com [Drop compile-time check for CONFIG_KVM_VM_MEMORY_ATTRIBUTES in kvm_gmem_range_is_private() since vm_memory_attributes, if defined false with a macro, should elide generation of the if block anyway] Signed-off-by: Ackerley Tng ackerleytng@google.com --- arch/x86/kvm/Kconfig | 13 +++++++++---- virt/kvm/kvm_main.c | 1 + 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 49c7709e3d895..5cd5046c542ba 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -82,13 +82,20 @@ config KVM_WERROR

config KVM_VM_MEMORY_ATTRIBUTES select KVM_MEMORY_ATTRIBUTES - bool + depends on KVM_SW_PROTECTED_VM || KVM_INTEL_TDX || KVM_AMD_SEV + bool "Enable per-VM memory attributes (for CoCo VMs)" + help + Enable support for per-VM memory attributes, which are deprecated in + favor of tracking memory attributes in guest_memfd. Select this if + you need to run CoCo VMs using a VMM that doesn't support guest_memfd + memory attributes. + + If unsure, say N.

config KVM_SW_PROTECTED_VM bool "Enable support for KVM software-protected VMs" depends on EXPERT depends on KVM_X86 && X86_64 - select KVM_VM_MEMORY_ATTRIBUTES help Enable support for KVM software-protected VMs. Currently, software- protected VMs are purely a development and testing vehicle for @@ -138,7 +145,6 @@ config KVM_INTEL_TDX bool "Intel Trust Domain Extensions (TDX) support" default y depends on INTEL_TDX_HOST - select KVM_VM_MEMORY_ATTRIBUTES select HAVE_KVM_ARCH_GMEM_POPULATE help Provides support for launching Intel Trust Domain Extensions (TDX) @@ -162,7 +168,6 @@ config KVM_AMD_SEV depends on KVM_AMD && X86_64 depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m) select ARCH_HAS_CC_PLATFORM - select KVM_VM_MEMORY_ATTRIBUTES select HAVE_KVM_ARCH_GMEM_PREPARE select HAVE_KVM_ARCH_GMEM_INVALIDATE select HAVE_KVM_ARCH_GMEM_POPULATE diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 3506a2f2be041..7680c868fd6e1 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -104,6 +104,7 @@ module_param(allow_unsafe_mappings, bool, 0444); #ifdef CONFIG_KVM_MEMORY_ATTRIBUTES #ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES bool vm_memory_attributes = true; +module_param(vm_memory_attributes, bool, 0444); #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));

From: Sean Christopherson seanjc@google.com

Rename local variables and function parameters for the guest memory file descriptor and its offset to use a "gmem_" prefix instead of "guest_memfd_".

No functional change intended.

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

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index e35c65a173606..8b714270cf381 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -912,7 +912,7 @@ void vm_set_user_memory_region(struct kvm_vm *vm, uint32_t slot, uint32_t flags,

int __vm_set_user_memory_region2(struct kvm_vm *vm, uint32_t slot, uint32_t flags, uint64_t gpa, uint64_t size, void *hva, - uint32_t guest_memfd, uint64_t guest_memfd_offset) + uint32_t gmem_fd, uint64_t gmem_offset) { struct kvm_userspace_memory_region2 region = { .slot = slot, @@ -920,8 +920,8 @@ int __vm_set_user_memory_region2(struct kvm_vm *vm, uint32_t slot, uint32_t flag .guest_phys_addr = gpa, .memory_size = size, .userspace_addr = (uintptr_t)hva, - .guest_memfd = guest_memfd, - .guest_memfd_offset = guest_memfd_offset, + .guest_memfd = gmem_fd, + .guest_memfd_offset = gmem_offset, };

TEST_REQUIRE_SET_USER_MEMORY_REGION2(); @@ -931,10 +931,10 @@ int __vm_set_user_memory_region2(struct kvm_vm *vm, uint32_t slot, uint32_t flag

void vm_set_user_memory_region2(struct kvm_vm *vm, uint32_t slot, uint32_t flags, uint64_t gpa, uint64_t size, void *hva, - uint32_t guest_memfd, uint64_t guest_memfd_offset) + uint32_t gmem_fd, uint64_t gmem_offset) { int ret = __vm_set_user_memory_region2(vm, slot, flags, gpa, size, hva, - guest_memfd, guest_memfd_offset); + gmem_fd, gmem_offset);

TEST_ASSERT(!ret, "KVM_SET_USER_MEMORY_REGION2 failed, errno = %d (%s)", errno, strerror(errno)); @@ -944,7 +944,7 @@ 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 guest_memfd, uint64_t guest_memfd_offset) + int gmem_fd, uint64_t gmem_offset) { int ret; struct userspace_mem_region *region; @@ -1027,12 +1027,12 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, region->mmap_size += alignment;

if (flags & KVM_MEM_GUEST_MEMFD) { - if (guest_memfd < 0) { - uint32_t guest_memfd_flags = 0; + if (gmem_fd < 0) { + uint32_t gmem_flags = 0;

- TEST_ASSERT(!guest_memfd_offset, + TEST_ASSERT(!gmem_offset, "Offset must be zero when creating new guest_memfd"); - guest_memfd = vm_create_guest_memfd(vm, mem_size, guest_memfd_flags); + gmem_fd = vm_create_guest_memfd(vm, mem_size, gmem_flags); } else { /* * Install a unique fd for each memslot so that the fd @@ -1040,11 +1040,11 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type, * needing to track if the fd is owned by the framework * or by the caller. */ - guest_memfd = kvm_dup(guest_memfd); + gmem_fd = kvm_dup(gmem_fd); }

- region->region.guest_memfd = guest_memfd; - region->region.guest_memfd_offset = guest_memfd_offset; + region->region.guest_memfd = gmem_fd; + region->region.guest_memfd_offset = gmem_offset; } else { region->region.guest_memfd = -1; }

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);