| Message ID | 20250605153800.557144-9-tabba@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | KVM: Mapping guest_memfd backed memory at the host for software protected VMs | expand |
On Fri, 6 Jun 2025 at 10:55, David Hildenbrand <david@redhat.com> wrote: > > On 06.06.25 11:30, Fuad Tabba wrote: > > Hi David, > > > > On Fri, 6 Jun 2025 at 10:12, David Hildenbrand <david@redhat.com> wrote: > >> > >> On 05.06.25 17:37, Fuad Tabba wrote: > >>> This patch enables support for shared memory in guest_memfd, including > >>> mapping that memory from host userspace. > >>> > >>> This functionality is gated by the KVM_GMEM_SHARED_MEM Kconfig option, > >>> and enabled for a given instance by the GUEST_MEMFD_FLAG_SUPPORT_SHARED > >>> flag at creation time. > >>> > >>> Co-developed-by: Ackerley Tng <ackerleytng@google.com> > >>> Signed-off-by: Ackerley Tng <ackerleytng@google.com> > >>> Signed-off-by: Fuad Tabba <tabba@google.com> > >>> --- > >> > >> [...] > >> > >>> +static bool kvm_gmem_supports_shared(struct inode *inode) > >>> +{ > >>> + u64 flags; > >>> + > >>> + if (!IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)) > >>> + return false; > >>> + > >>> + flags = (u64)inode->i_private; > >> > >> Can probably do above > >> > >> const u64 flags = (u64)inode->i_private; > >> > > > > Ack. > > > >>> + > >>> + return flags & GUEST_MEMFD_FLAG_SUPPORT_SHARED; > >>> +} > >>> + > >>> +static vm_fault_t kvm_gmem_fault_shared(struct vm_fault *vmf) > >>> +{ > >>> + struct inode *inode = file_inode(vmf->vma->vm_file); > >>> + struct folio *folio; > >>> + vm_fault_t ret = VM_FAULT_LOCKED; > >>> + > >>> + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) > >>> + return VM_FAULT_SIGBUS; > >>> + > >>> + folio = kvm_gmem_get_folio(inode, vmf->pgoff); > >>> + if (IS_ERR(folio)) { > >>> + int err = PTR_ERR(folio); > >>> + > >>> + if (err == -EAGAIN) > >>> + return VM_FAULT_RETRY; > >>> + > >>> + return vmf_error(err); > >>> + } > >>> + > >>> + if (WARN_ON_ONCE(folio_test_large(folio))) { > >>> + ret = VM_FAULT_SIGBUS; > >>> + goto out_folio; > >>> + } > >>> + > >>> + if (!folio_test_uptodate(folio)) { > >>> + clear_highpage(folio_page(folio, 0)); > >>> + kvm_gmem_mark_prepared(folio); > >>> + } > >>> + > >>> + vmf->page = folio_file_page(folio, vmf->pgoff); > >>> + > >>> +out_folio: > >>> + if (ret != VM_FAULT_LOCKED) { > >>> + folio_unlock(folio); > >>> + folio_put(folio); > >>> + } > >>> + > >>> + return ret; > >>> +} > >>> + > >>> +static const struct vm_operations_struct kvm_gmem_vm_ops = { > >>> + .fault = kvm_gmem_fault_shared, > >>> +}; > >>> + > >>> +static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma) > >>> +{ > >>> + if (!kvm_gmem_supports_shared(file_inode(file))) > >>> + return -ENODEV; > >>> + > >>> + if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) != > >>> + (VM_SHARED | VM_MAYSHARE)) { > >>> + return -EINVAL; > >>> + } > >>> + > >>> + vma->vm_ops = &kvm_gmem_vm_ops; > >>> + > >>> + return 0; > >>> +} > >>> + > >>> static struct file_operations kvm_gmem_fops = { > >>> + .mmap = kvm_gmem_mmap, > >>> .open = generic_file_open, > >>> .release = kvm_gmem_release, > >>> .fallocate = kvm_gmem_fallocate, > >>> @@ -428,6 +500,7 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) > >>> } > >>> > >>> file->f_flags |= O_LARGEFILE; > >>> + allow_write_access(file); > >> > >> Why is that required? > >> > >> As the docs mention, it must be paired with a previous deny_write_access(). > >> > >> ... and I don't find similar usage anywhere else. > > > > This is to address Gavin's concern [*] regarding MADV_COLLAPSE, which > > isn't an issue until hugepage support is enabled. Should we wait until > > we have hugepage support? > > If we keep this, we *definitely* need a comment why we do something > nobody else does. > > But I don't think allow_write_access() would ever be the way we want to > fence off MADV_COLLAPSE. :) Maybe AS_INACCESSIBLE or sth. like that > could fence it off in file_thp_enabled(). > > Fortunately, CONFIG_READ_ONLY_THP_FOR_FS might vanish at some point ... > so I've been told. > > So if it's not done for secretmem for now or others, we also shouldn't > be doing it for now I think. I'll remove it. Thanks! /fuad > -- > Cheers, > > David / dhildenb >
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 80371475818f..640ce714cfb2 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -729,6 +729,19 @@ static inline bool kvm_arch_supports_gmem(struct kvm *kvm) } #endif +/* + * Returns true if this VM supports shared mem in guest_memfd. + * + * Arch code must define kvm_arch_supports_gmem_shared_mem if support for + * guest_memfd is enabled. + */ +#if !defined(kvm_arch_supports_gmem_shared_mem) +static inline bool kvm_arch_supports_gmem_shared_mem(struct kvm *kvm) +{ + return false; +} +#endif + #ifndef kvm_arch_has_readonly_mem static inline bool kvm_arch_has_readonly_mem(struct kvm *kvm) { diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index b6ae8ad8934b..c2714c9d1a0e 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -1566,6 +1566,7 @@ struct kvm_memory_attributes { #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3) #define KVM_CREATE_GUEST_MEMFD _IOWR(KVMIO, 0xd4, struct kvm_create_guest_memfd) +#define GUEST_MEMFD_FLAG_SUPPORT_SHARED (1ULL << 0) struct kvm_create_guest_memfd { __u64 size; diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig index 559c93ad90be..e90884f74404 100644 --- a/virt/kvm/Kconfig +++ b/virt/kvm/Kconfig @@ -128,3 +128,7 @@ config HAVE_KVM_ARCH_GMEM_PREPARE config HAVE_KVM_ARCH_GMEM_INVALIDATE bool depends on KVM_GMEM + +config KVM_GMEM_SHARED_MEM + select KVM_GMEM + bool diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 6db515833f61..7a158789d1df 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -312,7 +312,79 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn) return gfn - slot->base_gfn + slot->gmem.pgoff; } +static bool kvm_gmem_supports_shared(struct inode *inode) +{ + u64 flags; + + if (!IS_ENABLED(CONFIG_KVM_GMEM_SHARED_MEM)) + return false; + + flags = (u64)inode->i_private; + + return flags & GUEST_MEMFD_FLAG_SUPPORT_SHARED; +} + +static vm_fault_t kvm_gmem_fault_shared(struct vm_fault *vmf) +{ + struct inode *inode = file_inode(vmf->vma->vm_file); + struct folio *folio; + vm_fault_t ret = VM_FAULT_LOCKED; + + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) + return VM_FAULT_SIGBUS; + + folio = kvm_gmem_get_folio(inode, vmf->pgoff); + if (IS_ERR(folio)) { + int err = PTR_ERR(folio); + + if (err == -EAGAIN) + return VM_FAULT_RETRY; + + return vmf_error(err); + } + + if (WARN_ON_ONCE(folio_test_large(folio))) { + ret = VM_FAULT_SIGBUS; + goto out_folio; + } + + if (!folio_test_uptodate(folio)) { + clear_highpage(folio_page(folio, 0)); + kvm_gmem_mark_prepared(folio); + } + + vmf->page = folio_file_page(folio, vmf->pgoff); + +out_folio: + if (ret != VM_FAULT_LOCKED) { + folio_unlock(folio); + folio_put(folio); + } + + return ret; +} + +static const struct vm_operations_struct kvm_gmem_vm_ops = { + .fault = kvm_gmem_fault_shared, +}; + +static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma) +{ + if (!kvm_gmem_supports_shared(file_inode(file))) + return -ENODEV; + + if ((vma->vm_flags & (VM_SHARED | VM_MAYSHARE)) != + (VM_SHARED | VM_MAYSHARE)) { + return -EINVAL; + } + + vma->vm_ops = &kvm_gmem_vm_ops; + + return 0; +} + static struct file_operations kvm_gmem_fops = { + .mmap = kvm_gmem_mmap, .open = generic_file_open, .release = kvm_gmem_release, .fallocate = kvm_gmem_fallocate, @@ -428,6 +500,7 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags) } file->f_flags |= O_LARGEFILE; + allow_write_access(file); inode = file->f_inode; WARN_ON(file->f_mapping != inode->i_mapping); @@ -463,6 +536,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args) u64 flags = args->flags; u64 valid_flags = 0; + if (kvm_arch_supports_gmem_shared_mem(kvm)) + valid_flags |= GUEST_MEMFD_FLAG_SUPPORT_SHARED; + if (flags & ~valid_flags) return -EINVAL;