Message ID | 20211223123011.41044-7-chao.p.peng@linux.intel.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | KVM: mm: fd-based approach for supporting KVM guest private memory | expand |
On Thu, Dec 23, 2021, Chao Peng wrote: > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > index 03b2ce34e7f4..86655cd660ca 100644 > --- a/arch/x86/kvm/Kconfig > +++ b/arch/x86/kvm/Kconfig > @@ -46,6 +46,7 @@ config KVM > select SRCU > select INTERVAL_TREE > select HAVE_KVM_PM_NOTIFIER if PM > + select MEMFD_OPS MEMFD_OPS is a weird Kconfig name given that it's not just memfd() that can implement the ops. > help > Support hosting fully virtualized guest machines using hardware > virtualization extensions. You will need a fairly recent > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 3bd875f9669f..21f8b1880723 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -806,6 +806,12 @@ static inline void kvm_irqfd_exit(void) > { > } > #endif > + > +int kvm_memfd_register(struct kvm *kvm, struct kvm_memory_slot *slot); > +void kvm_memfd_unregister(struct kvm_memory_slot *slot); > +long kvm_memfd_get_pfn(struct kvm_memory_slot *slot, gfn_t gfn, int *order); > +void kvm_memfd_put_pfn(kvm_pfn_t pfn); > + > int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, > struct module *module); > void kvm_exit(void); > diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm > index ffdcad3cc97a..8842128d8429 100644 > --- a/virt/kvm/Makefile.kvm > +++ b/virt/kvm/Makefile.kvm > @@ -5,7 +5,7 @@ > > KVM ?= ../../../virt/kvm > > -kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o > +kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o $(KVM)/memfd.o This should be kvm-$(CONFIG_MEMFD_OPS) += $(KVM)/memfd.o with stubs provided in a header file as needed. I also really dislike naming KVM's file memfd.c, though I don't have a good alternative off the top of my head. > kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o > kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o > kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o > +#ifdef CONFIG_MEMFD_OPS > +static const struct memfd_pfn_ops *memfd_ops; memfd_ops needs to be associated with the slot, e.g. userspace should be able to map multiple types of a backing stores into a single VM. This doesn't even allow that for multiple VMs, and there are all kinds of ordering issues. > +void kvm_memfd_unregister(struct kvm_memory_slot *slot) > +{ > +#ifdef CONFIG_MEMFD_OPS > + if (slot->file) { > + fput(slot->file); Needs to actually unregister. > + slot->file = NULL; > + } > +#endif > +} > -- > 2.17.1 >
On 12/23/21 19:34, Sean Christopherson wrote: >> select HAVE_KVM_PM_NOTIFIER if PM >> + select MEMFD_OPS > MEMFD_OPS is a weird Kconfig name given that it's not just memfd() that can > implement the ops. > Or, it's kvm that implements them to talk to memfd? Paolo
On Thu, Dec 23, 2021 at 06:34:22PM +0000, Sean Christopherson wrote: > On Thu, Dec 23, 2021, Chao Peng wrote: > > diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig > > index 03b2ce34e7f4..86655cd660ca 100644 > > --- a/arch/x86/kvm/Kconfig > > +++ b/arch/x86/kvm/Kconfig > > @@ -46,6 +46,7 @@ config KVM > > select SRCU > > select INTERVAL_TREE > > select HAVE_KVM_PM_NOTIFIER if PM > > + select MEMFD_OPS > > MEMFD_OPS is a weird Kconfig name given that it's not just memfd() that can > implement the ops. > > > help > > Support hosting fully virtualized guest machines using hardware > > virtualization extensions. You will need a fairly recent > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > > index 3bd875f9669f..21f8b1880723 100644 > > --- a/include/linux/kvm_host.h > > +++ b/include/linux/kvm_host.h > > @@ -806,6 +806,12 @@ static inline void kvm_irqfd_exit(void) > > { > > } > > #endif > > + > > +int kvm_memfd_register(struct kvm *kvm, struct kvm_memory_slot *slot); > > +void kvm_memfd_unregister(struct kvm_memory_slot *slot); > > +long kvm_memfd_get_pfn(struct kvm_memory_slot *slot, gfn_t gfn, int *order); > > +void kvm_memfd_put_pfn(kvm_pfn_t pfn); > > + > > int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, > > struct module *module); > > void kvm_exit(void); > > diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm > > index ffdcad3cc97a..8842128d8429 100644 > > --- a/virt/kvm/Makefile.kvm > > +++ b/virt/kvm/Makefile.kvm > > @@ -5,7 +5,7 @@ > > > > KVM ?= ../../../virt/kvm > > > > -kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o > > +kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o $(KVM)/memfd.o > > This should be > > kvm-$(CONFIG_MEMFD_OPS) += $(KVM)/memfd.o > > with stubs provided in a header file as needed. I also really dislike naming KVM's > file memfd.c, though I don't have a good alternative off the top of my head. > > > kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o > > kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o > > kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o > > > > +#ifdef CONFIG_MEMFD_OPS > > +static const struct memfd_pfn_ops *memfd_ops; > > memfd_ops needs to be associated with the slot, e.g. userspace should be able to > map multiple types of a backing stores into a single VM. I considered this but gave up as I'm not so confident that we will support other memory backends than memfd in the forthcoming future. >This doesn't even allow > that for multiple VMs, and there are all kinds of ordering issues. Current memfd kAPI actually returns the same set of callback pointer for all the VMs. It supports multiple VMs via callback parameter inode, assume one inode can be associated with only one VM. > > > +void kvm_memfd_unregister(struct kvm_memory_slot *slot) > > +{ > > +#ifdef CONFIG_MEMFD_OPS > > + if (slot->file) { > > + fput(slot->file); > > Needs to actually unregister. Good catch, thanks. > > > + slot->file = NULL; > > + } > > +#endif > > +} > > -- > > 2.17.1 > >
On Thu, Dec 23, 2021 at 06:34:22PM +0000, Sean Christopherson wrote: > On Thu, Dec 23, 2021, Chao Peng wrote: > > > > -kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o > > +kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o $(KVM)/memfd.o > > This should be > > kvm-$(CONFIG_MEMFD_OPS) += $(KVM)/memfd.o > > with stubs provided in a header file as needed. I also really dislike naming KVM's > file memfd.c, though I don't have a good alternative off the top of my head. Is memory-backend.c better? if we end up introducing the callback definition in KVM we can call it CONFIG_KVM_MEMORY_BACKEDN_OPS. Chao
On Fri, Dec 24, 2021 at 12:09:47AM +0100, Paolo Bonzini wrote: > On 12/23/21 19:34, Sean Christopherson wrote: > > > select HAVE_KVM_PM_NOTIFIER if PM > > > + select MEMFD_OPS > > MEMFD_OPS is a weird Kconfig name given that it's not just memfd() that can > > implement the ops. > > > > Or, it's kvm that implements them to talk to memfd? The only thing is VFIO may also use the same set of callbacks, as discussed in the v2. But I think that's fine. Chao > > Paolo
On Fri, Dec 24, 2021, Chao Peng wrote: > On Fri, Dec 24, 2021 at 12:09:47AM +0100, Paolo Bonzini wrote: > > On 12/23/21 19:34, Sean Christopherson wrote: > > > > select HAVE_KVM_PM_NOTIFIER if PM > > > > + select MEMFD_OPS > > > MEMFD_OPS is a weird Kconfig name given that it's not just memfd() that can > > > implement the ops. > > > > > > > Or, it's kvm that implements them to talk to memfd? > > The only thing is VFIO may also use the same set of callbacks, as > discussed in the v2. But I think that's fine. I'm objecting to assuming that KVM is talking to memfd. KVM shouldn't know or care what is sitting behind the fd, KVM only cares whether or not the backing store provides the necessary APIs. I also think that the API as whole should be abstracted from memfd. It's mostly cosmectic, e.g. tweak the struct and Kconfig name. I don't really care if it's initially dependent on MEMFD_CREATE, I just don't want to end up with an API and KVM implementation that implies there's something fundamentally special about memfd.
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig index 03b2ce34e7f4..86655cd660ca 100644 --- a/arch/x86/kvm/Kconfig +++ b/arch/x86/kvm/Kconfig @@ -46,6 +46,7 @@ config KVM select SRCU select INTERVAL_TREE select HAVE_KVM_PM_NOTIFIER if PM + select MEMFD_OPS help Support hosting fully virtualized guest machines using hardware virtualization extensions. You will need a fairly recent diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 3bd875f9669f..21f8b1880723 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -806,6 +806,12 @@ static inline void kvm_irqfd_exit(void) { } #endif + +int kvm_memfd_register(struct kvm *kvm, struct kvm_memory_slot *slot); +void kvm_memfd_unregister(struct kvm_memory_slot *slot); +long kvm_memfd_get_pfn(struct kvm_memory_slot *slot, gfn_t gfn, int *order); +void kvm_memfd_put_pfn(kvm_pfn_t pfn); + int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align, struct module *module); void kvm_exit(void); diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm index ffdcad3cc97a..8842128d8429 100644 --- a/virt/kvm/Makefile.kvm +++ b/virt/kvm/Makefile.kvm @@ -5,7 +5,7 @@ KVM ?= ../../../virt/kvm -kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o +kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o $(KVM)/memfd.o kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o diff --git a/virt/kvm/memfd.c b/virt/kvm/memfd.c new file mode 100644 index 000000000000..662393a76782 --- /dev/null +++ b/virt/kvm/memfd.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * memfd.c: routines for fd based guest memory + * Copyright (c) 2021, Intel Corporation. + * + * Author: + * Chao Peng <chao.p.peng@linux.intel.com> + */ + +#include <linux/kvm_host.h> +#include <linux/memfd.h> + +#ifdef CONFIG_MEMFD_OPS +static const struct memfd_pfn_ops *memfd_ops; + +static void memfd_invalidate_page_range(struct inode *inode, void *owner, + pgoff_t start, pgoff_t end) +{ +} + +static void memfd_fallocate(struct inode *inode, void *owner, + pgoff_t start, pgoff_t end) +{ +} + +static bool memfd_get_owner(void *owner) +{ + return kvm_get_kvm_safe(owner); +} + +static void memfd_put_owner(void *owner) +{ + kvm_put_kvm(owner); +} + +static const struct memfd_falloc_notifier memfd_notifier = { + .invalidate_page_range = memfd_invalidate_page_range, + .fallocate = memfd_fallocate, + .get_owner = memfd_get_owner, + .put_owner = memfd_put_owner, +}; +#endif + +long kvm_memfd_get_pfn(struct kvm_memory_slot *slot, gfn_t gfn, int *order) +{ +#ifdef CONFIG_MEMFD_OPS + pgoff_t index = gfn - slot->base_gfn + (slot->ofs >> PAGE_SHIFT); + + return memfd_ops->get_lock_pfn(slot->file->f_inode, index, order); +#else + return -EOPNOTSUPP; +#endif +} + +void kvm_memfd_put_pfn(kvm_pfn_t pfn) +{ +#ifdef CONFIG_MEMFD_OPS + memfd_ops->put_unlock_pfn(pfn); +#endif +} + +int kvm_memfd_register(struct kvm *kvm, struct kvm_memory_slot *slot) +{ +#ifdef CONFIG_MEMFD_OPS + int ret; + struct fd fd = fdget(slot->fd); + + if (!fd.file) + return -EINVAL; + + ret = memfd_register_falloc_notifier(fd.file->f_inode, kvm, + &memfd_notifier, &memfd_ops); + if (ret) + return ret; + + slot->file = fd.file; + return 0; +#else + return -EOPNOTSUPP; +#endif +} + +void kvm_memfd_unregister(struct kvm_memory_slot *slot) +{ +#ifdef CONFIG_MEMFD_OPS + if (slot->file) { + fput(slot->file); + slot->file = NULL; + } +#endif +}