KVM gets notified when memory pages changed in the memory backing store.
When userspace allocates the memory with fallocate() or frees memory
with fallocate(FALLOC_FL_PUNCH_HOLE), memory backing store calls into
KVM fallocate/invalidate callbacks respectively. To ensure KVM never
maps both the private and shared variants of a GPA into the guest, in
the fallocate callback, we should zap the existing shared mapping and
in the invalidate callback we should zap the existing private mapping.
In the callbacks, KVM firstly converts the offset range into the
gfn_range and then calls existing kvm_unmap_gfn_range() which will zap
the shared or private mapping. Both callbacks pass in a memslot
reference but we need 'kvm' so add a reference in memslot structure.
Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
---
include/linux/kvm_host.h | 3 ++-
virt/kvm/kvm_main.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9b175aeca63f..186b9b981a65 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -236,7 +236,7 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
#endif
-#ifdef KVM_ARCH_WANT_MMU_NOTIFIER
+#if defined(KVM_ARCH_WANT_MMU_NOTIFIER) || defined(CONFIG_MEMFILE_NOTIFIER)
struct kvm_gfn_range {
struct kvm_memory_slot *slot;
gfn_t start;
@@ -568,6 +568,7 @@ struct kvm_memory_slot {
loff_t private_offset;
struct memfile_pfn_ops *pfn_ops;
struct memfile_notifier notifier;
+ struct kvm *kvm;
};
static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 67349421eae3..52319f49d58a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
#endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
#ifdef CONFIG_MEMFILE_NOTIFIER
+static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
+ pgoff_t start, pgoff_t end)
+{
+ int idx;
+ struct kvm_memory_slot *slot = container_of(notifier,
+ struct kvm_memory_slot,
+ notifier);
+ struct kvm_gfn_range gfn_range = {
+ .slot = slot,
+ .start = start - (slot->private_offset >> PAGE_SHIFT),
+ .end = end - (slot->private_offset >> PAGE_SHIFT),
+ .may_block = true,
+ };
+ struct kvm *kvm = slot->kvm;
+
+ gfn_range.start = max(gfn_range.start, slot->base_gfn);
+ gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
+
+ if (gfn_range.start >= gfn_range.end)
+ return;
+
+ idx = srcu_read_lock(&kvm->srcu);
+ KVM_MMU_LOCK(kvm);
+ kvm_unmap_gfn_range(kvm, &gfn_range);
+ kvm_flush_remote_tlbs(kvm);
+ KVM_MMU_UNLOCK(kvm);
+ srcu_read_unlock(&kvm->srcu, idx);
+}
+
+static struct memfile_notifier_ops kvm_memfile_notifier_ops = {
+ .invalidate = kvm_memfile_notifier_handler,
+ .fallocate = kvm_memfile_notifier_handler,
+};
+
static inline int kvm_memfile_register(struct kvm_memory_slot *slot)
{
+ slot->notifier.ops = &kvm_memfile_notifier_ops;
return memfile_register_notifier(file_inode(slot->private_file),
&slot->notifier,
&slot->pfn_ops);
@@ -1963,6 +1998,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
new->private_file = file;
new->private_offset = mem->flags & KVM_MEM_PRIVATE ?
region_ext->private_offset : 0;
+ new->kvm = kvm;
r = kvm_set_memslot(kvm, old, new, change);
if (!r)
--
2.17.1On Thu, Mar 10, 2022 at 6:11 AM Chao Peng <chao.p.peng@linux.intel.com> wrote:
>
> KVM gets notified when memory pages changed in the memory backing store.
> When userspace allocates the memory with fallocate() or frees memory
> with fallocate(FALLOC_FL_PUNCH_HOLE), memory backing store calls into
> KVM fallocate/invalidate callbacks respectively. To ensure KVM never
> maps both the private and shared variants of a GPA into the guest, in
> the fallocate callback, we should zap the existing shared mapping and
> in the invalidate callback we should zap the existing private mapping.
>
> In the callbacks, KVM firstly converts the offset range into the
> gfn_range and then calls existing kvm_unmap_gfn_range() which will zap
> the shared or private mapping. Both callbacks pass in a memslot
> reference but we need 'kvm' so add a reference in memslot structure.
>
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> include/linux/kvm_host.h | 3 ++-
> virt/kvm/kvm_main.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 9b175aeca63f..186b9b981a65 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -236,7 +236,7 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> #endif
>
> -#ifdef KVM_ARCH_WANT_MMU_NOTIFIER
> +#if defined(KVM_ARCH_WANT_MMU_NOTIFIER) || defined(CONFIG_MEMFILE_NOTIFIER)
> struct kvm_gfn_range {
> struct kvm_memory_slot *slot;
> gfn_t start;
> @@ -568,6 +568,7 @@ struct kvm_memory_slot {
> loff_t private_offset;
> struct memfile_pfn_ops *pfn_ops;
> struct memfile_notifier notifier;
> + struct kvm *kvm;
> };
>
> static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 67349421eae3..52319f49d58a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
>
> #ifdef CONFIG_MEMFILE_NOTIFIER
> +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> + pgoff_t start, pgoff_t end)
> +{
> + int idx;
> + struct kvm_memory_slot *slot = container_of(notifier,
> + struct kvm_memory_slot,
> + notifier);
> + struct kvm_gfn_range gfn_range = {
> + .slot = slot,
> + .start = start - (slot->private_offset >> PAGE_SHIFT),
> + .end = end - (slot->private_offset >> PAGE_SHIFT),
> + .may_block = true,
> + };
> + struct kvm *kvm = slot->kvm;
> +
> + gfn_range.start = max(gfn_range.start, slot->base_gfn);
gfn_range.start seems to be page offset within the file. Should this rather be:
gfn_range.start = slot->base_gfn + min(gfn_range.start, slot->npages);
> + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> +
Similar to previous comment, should this rather be:
gfn_range.end = slot->base_gfn + min(gfn_range.end, slot->npages);
> + if (gfn_range.start >= gfn_range.end)
> + return;
> +
> + idx = srcu_read_lock(&kvm->srcu);
> + KVM_MMU_LOCK(kvm);
> + kvm_unmap_gfn_range(kvm, &gfn_range);
> + kvm_flush_remote_tlbs(kvm);
> + KVM_MMU_UNLOCK(kvm);
> + srcu_read_unlock(&kvm->srcu, idx);
> +}
> +
> +static struct memfile_notifier_ops kvm_memfile_notifier_ops = {
> + .invalidate = kvm_memfile_notifier_handler,
> + .fallocate = kvm_memfile_notifier_handler,
> +};
> +
> static inline int kvm_memfile_register(struct kvm_memory_slot *slot)
> {
> + slot->notifier.ops = &kvm_memfile_notifier_ops;
> return memfile_register_notifier(file_inode(slot->private_file),
> &slot->notifier,
> &slot->pfn_ops);
> @@ -1963,6 +1998,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> new->private_file = file;
> new->private_offset = mem->flags & KVM_MEM_PRIVATE ?
> region_ext->private_offset : 0;
> + new->kvm = kvm;
>
> r = kvm_set_memslot(kvm, old, new, change);
> if (!r)
> --
> 2.17.1
>
On Tue, Apr 19, 2022 at 03:43:56PM -0700, Vishal Annapurve wrote:
> On Thu, Mar 10, 2022 at 6:11 AM Chao Peng <chao.p.peng@linux.intel.com> wrote:
> >
> > KVM gets notified when memory pages changed in the memory backing store.
> > When userspace allocates the memory with fallocate() or frees memory
> > with fallocate(FALLOC_FL_PUNCH_HOLE), memory backing store calls into
> > KVM fallocate/invalidate callbacks respectively. To ensure KVM never
> > maps both the private and shared variants of a GPA into the guest, in
> > the fallocate callback, we should zap the existing shared mapping and
> > in the invalidate callback we should zap the existing private mapping.
> >
> > In the callbacks, KVM firstly converts the offset range into the
> > gfn_range and then calls existing kvm_unmap_gfn_range() which will zap
> > the shared or private mapping. Both callbacks pass in a memslot
> > reference but we need 'kvm' so add a reference in memslot structure.
> >
> > Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> > Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> > ---
> > include/linux/kvm_host.h | 3 ++-
> > virt/kvm/kvm_main.c | 36 ++++++++++++++++++++++++++++++++++++
> > 2 files changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 9b175aeca63f..186b9b981a65 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -236,7 +236,7 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> > int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> > #endif
> >
> > -#ifdef KVM_ARCH_WANT_MMU_NOTIFIER
> > +#if defined(KVM_ARCH_WANT_MMU_NOTIFIER) || defined(CONFIG_MEMFILE_NOTIFIER)
> > struct kvm_gfn_range {
> > struct kvm_memory_slot *slot;
> > gfn_t start;
> > @@ -568,6 +568,7 @@ struct kvm_memory_slot {
> > loff_t private_offset;
> > struct memfile_pfn_ops *pfn_ops;
> > struct memfile_notifier notifier;
> > + struct kvm *kvm;
> > };
> >
> > static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 67349421eae3..52319f49d58a 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> > #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
> >
> > #ifdef CONFIG_MEMFILE_NOTIFIER
> > +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> > + pgoff_t start, pgoff_t end)
> > +{
> > + int idx;
> > + struct kvm_memory_slot *slot = container_of(notifier,
> > + struct kvm_memory_slot,
> > + notifier);
> > + struct kvm_gfn_range gfn_range = {
> > + .slot = slot,
> > + .start = start - (slot->private_offset >> PAGE_SHIFT),
> > + .end = end - (slot->private_offset >> PAGE_SHIFT),
> > + .may_block = true,
> > + };
> > + struct kvm *kvm = slot->kvm;
> > +
> > + gfn_range.start = max(gfn_range.start, slot->base_gfn);
>
> gfn_range.start seems to be page offset within the file. Should this rather be:
> gfn_range.start = slot->base_gfn + min(gfn_range.start, slot->npages);
Right. For start we don't really need care about the uppper bound
here (will check below), so this should be enough:
gfn_range.start = slot->base_gfn + gfn_range.start;
>
> > + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> > +
>
> Similar to previous comment, should this rather be:
> gfn_range.end = slot->base_gfn + min(gfn_range.end, slot->npages);
This is correct.
Thanks,
Chao
>
> > + if (gfn_range.start >= gfn_range.end)
> > + return;
> > +
> > + idx = srcu_read_lock(&kvm->srcu);
> > + KVM_MMU_LOCK(kvm);
> > + kvm_unmap_gfn_range(kvm, &gfn_range);
> > + kvm_flush_remote_tlbs(kvm);
> > + KVM_MMU_UNLOCK(kvm);
> > + srcu_read_unlock(&kvm->srcu, idx);
> > +}
> > +
> > +static struct memfile_notifier_ops kvm_memfile_notifier_ops = {
> > + .invalidate = kvm_memfile_notifier_handler,
> > + .fallocate = kvm_memfile_notifier_handler,
> > +};
> > +
> > static inline int kvm_memfile_register(struct kvm_memory_slot *slot)
> > {
> > + slot->notifier.ops = &kvm_memfile_notifier_ops;
> > return memfile_register_notifier(file_inode(slot->private_file),
> > &slot->notifier,
> > &slot->pfn_ops);
> > @@ -1963,6 +1998,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > new->private_file = file;
> > new->private_offset = mem->flags & KVM_MEM_PRIVATE ?
> > region_ext->private_offset : 0;
> > + new->kvm = kvm;
> >
> > r = kvm_set_memslot(kvm, old, new, change);
> > if (!r)
> > --
> > 2.17.1
> >
On Thu, Mar 10, 2022, Chao Peng wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 67349421eae3..52319f49d58a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
>
> #ifdef CONFIG_MEMFILE_NOTIFIER
> +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> + pgoff_t start, pgoff_t end)
> +{
> + int idx;
> + struct kvm_memory_slot *slot = container_of(notifier,
> + struct kvm_memory_slot,
> + notifier);
> + struct kvm_gfn_range gfn_range = {
> + .slot = slot,
> + .start = start - (slot->private_offset >> PAGE_SHIFT),
> + .end = end - (slot->private_offset >> PAGE_SHIFT),
> + .may_block = true,
> + };
> + struct kvm *kvm = slot->kvm;
> +
> + gfn_range.start = max(gfn_range.start, slot->base_gfn);
> + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> +
> + if (gfn_range.start >= gfn_range.end)
> + return;
> +
> + idx = srcu_read_lock(&kvm->srcu);
> + KVM_MMU_LOCK(kvm);
> + kvm_unmap_gfn_range(kvm, &gfn_range);
> + kvm_flush_remote_tlbs(kvm);
This should check the result of kvm_unmap_gfn_range() and flush only if necessary.
kvm->mmu_notifier_seq needs to be incremented, otherwise KVM will incorrectly
install a SPTE if the mapping is zapped between retrieving the pfn in faultin and
installing it after acquire mmu_lock.
> + KVM_MMU_UNLOCK(kvm);
> + srcu_read_unlock(&kvm->srcu, idx);
> +}
> +
> +static struct memfile_notifier_ops kvm_memfile_notifier_ops = {
> + .invalidate = kvm_memfile_notifier_handler,
> + .fallocate = kvm_memfile_notifier_handler,
> +};
> +
> static inline int kvm_memfile_register(struct kvm_memory_slot *slot)
> {
> + slot->notifier.ops = &kvm_memfile_notifier_ops;
> return memfile_register_notifier(file_inode(slot->private_file),
> &slot->notifier,
> &slot->pfn_ops);
> @@ -1963,6 +1998,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> new->private_file = file;
> new->private_offset = mem->flags & KVM_MEM_PRIVATE ?
> region_ext->private_offset : 0;
> + new->kvm = kvm;
>
> r = kvm_set_memslot(kvm, old, new, change);
> if (!r)
> --
> 2.17.1
>
On Tue, Mar 29, 2022 at 07:23:04PM +0000, Sean Christopherson wrote:
> On Thu, Mar 10, 2022, Chao Peng wrote:
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 67349421eae3..52319f49d58a 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> > #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
> >
> > #ifdef CONFIG_MEMFILE_NOTIFIER
> > +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> > + pgoff_t start, pgoff_t end)
> > +{
> > + int idx;
> > + struct kvm_memory_slot *slot = container_of(notifier,
> > + struct kvm_memory_slot,
> > + notifier);
> > + struct kvm_gfn_range gfn_range = {
> > + .slot = slot,
> > + .start = start - (slot->private_offset >> PAGE_SHIFT),
> > + .end = end - (slot->private_offset >> PAGE_SHIFT),
> > + .may_block = true,
> > + };
> > + struct kvm *kvm = slot->kvm;
> > +
> > + gfn_range.start = max(gfn_range.start, slot->base_gfn);
> > + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> > +
> > + if (gfn_range.start >= gfn_range.end)
> > + return;
> > +
> > + idx = srcu_read_lock(&kvm->srcu);
> > + KVM_MMU_LOCK(kvm);
> > + kvm_unmap_gfn_range(kvm, &gfn_range);
> > + kvm_flush_remote_tlbs(kvm);
>
> This should check the result of kvm_unmap_gfn_range() and flush only if necessary.
Yep.
>
> kvm->mmu_notifier_seq needs to be incremented, otherwise KVM will incorrectly
> install a SPTE if the mapping is zapped between retrieving the pfn in faultin and
> installing it after acquire mmu_lock.
Good catch.
Chao
>
>
> > + KVM_MMU_UNLOCK(kvm);
> > + srcu_read_unlock(&kvm->srcu, idx);
> > +}
> > +
> > +static struct memfile_notifier_ops kvm_memfile_notifier_ops = {
> > + .invalidate = kvm_memfile_notifier_handler,
> > + .fallocate = kvm_memfile_notifier_handler,
> > +};
> > +
> > static inline int kvm_memfile_register(struct kvm_memory_slot *slot)
> > {
> > + slot->notifier.ops = &kvm_memfile_notifier_ops;
> > return memfile_register_notifier(file_inode(slot->private_file),
> > &slot->notifier,
> > &slot->pfn_ops);
> > @@ -1963,6 +1998,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
> > new->private_file = file;
> > new->private_offset = mem->flags & KVM_MEM_PRIVATE ?
> > region_ext->private_offset : 0;
> > + new->kvm = kvm;
> >
> > r = kvm_set_memslot(kvm, old, new, change);
> > if (!r)
> > --
> > 2.17.1
> >
On Thu, Mar 10, 2022 at 10:09:09PM +0800, Chao Peng wrote:
> KVM gets notified when memory pages changed in the memory backing store.
> When userspace allocates the memory with fallocate() or frees memory
> with fallocate(FALLOC_FL_PUNCH_HOLE), memory backing store calls into
> KVM fallocate/invalidate callbacks respectively. To ensure KVM never
> maps both the private and shared variants of a GPA into the guest, in
> the fallocate callback, we should zap the existing shared mapping and
> in the invalidate callback we should zap the existing private mapping.
>
> In the callbacks, KVM firstly converts the offset range into the
> gfn_range and then calls existing kvm_unmap_gfn_range() which will zap
> the shared or private mapping. Both callbacks pass in a memslot
> reference but we need 'kvm' so add a reference in memslot structure.
>
> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
> include/linux/kvm_host.h | 3 ++-
> virt/kvm/kvm_main.c | 36 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 9b175aeca63f..186b9b981a65 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -236,7 +236,7 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> int kvm_async_pf_wakeup_all(struct kvm_vcpu *vcpu);
> #endif
>
> -#ifdef KVM_ARCH_WANT_MMU_NOTIFIER
> +#if defined(KVM_ARCH_WANT_MMU_NOTIFIER) || defined(CONFIG_MEMFILE_NOTIFIER)
> struct kvm_gfn_range {
> struct kvm_memory_slot *slot;
> gfn_t start;
> @@ -568,6 +568,7 @@ struct kvm_memory_slot {
> loff_t private_offset;
> struct memfile_pfn_ops *pfn_ops;
> struct memfile_notifier notifier;
> + struct kvm *kvm;
> };
>
> static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 67349421eae3..52319f49d58a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
>
> #ifdef CONFIG_MEMFILE_NOTIFIER
> +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> + pgoff_t start, pgoff_t end)
> +{
> + int idx;
> + struct kvm_memory_slot *slot = container_of(notifier,
> + struct kvm_memory_slot,
> + notifier);
> + struct kvm_gfn_range gfn_range = {
> + .slot = slot,
> + .start = start - (slot->private_offset >> PAGE_SHIFT),
> + .end = end - (slot->private_offset >> PAGE_SHIFT),
> + .may_block = true,
> + };
> + struct kvm *kvm = slot->kvm;
> +
> + gfn_range.start = max(gfn_range.start, slot->base_gfn);
> + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> +
> + if (gfn_range.start >= gfn_range.end)
> + return;
> +
> + idx = srcu_read_lock(&kvm->srcu);
> + KVM_MMU_LOCK(kvm);
> + kvm_unmap_gfn_range(kvm, &gfn_range);
> + kvm_flush_remote_tlbs(kvm);
> + KVM_MMU_UNLOCK(kvm);
> + srcu_read_unlock(&kvm->srcu, idx);
Should this also invalidate gfn_to_pfn_cache mappings? Otherwise it seems
possible the kernel might end up inadvertantly writing to now-private guest
memory via a now-stale gfn_to_pfn_cache entry.
On Tue, Apr 05, 2022, Michael Roth wrote:
> On Thu, Mar 10, 2022 at 10:09:09PM +0800, Chao Peng wrote:
> > static inline bool kvm_slot_is_private(const struct kvm_memory_slot *slot)
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 67349421eae3..52319f49d58a 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -841,8 +841,43 @@ static int kvm_init_mmu_notifier(struct kvm *kvm)
> > #endif /* CONFIG_MMU_NOTIFIER && KVM_ARCH_WANT_MMU_NOTIFIER */
> >
> > #ifdef CONFIG_MEMFILE_NOTIFIER
> > +static void kvm_memfile_notifier_handler(struct memfile_notifier *notifier,
> > + pgoff_t start, pgoff_t end)
> > +{
> > + int idx;
> > + struct kvm_memory_slot *slot = container_of(notifier,
> > + struct kvm_memory_slot,
> > + notifier);
> > + struct kvm_gfn_range gfn_range = {
> > + .slot = slot,
> > + .start = start - (slot->private_offset >> PAGE_SHIFT),
> > + .end = end - (slot->private_offset >> PAGE_SHIFT),
> > + .may_block = true,
> > + };
> > + struct kvm *kvm = slot->kvm;
> > +
> > + gfn_range.start = max(gfn_range.start, slot->base_gfn);
> > + gfn_range.end = min(gfn_range.end, slot->base_gfn + slot->npages);
> > +
> > + if (gfn_range.start >= gfn_range.end)
> > + return;
> > +
> > + idx = srcu_read_lock(&kvm->srcu);
> > + KVM_MMU_LOCK(kvm);
> > + kvm_unmap_gfn_range(kvm, &gfn_range);
> > + kvm_flush_remote_tlbs(kvm);
> > + KVM_MMU_UNLOCK(kvm);
> > + srcu_read_unlock(&kvm->srcu, idx);
>
> Should this also invalidate gfn_to_pfn_cache mappings? Otherwise it seems
> possible the kernel might end up inadvertantly writing to now-private guest
> memory via a now-stale gfn_to_pfn_cache entry.
Yes. Ideally we'd get these flows to share common code and avoid these goofs.
I tried very briefly but they're just different enough to make it ugly.
© 2016 - 2026 Red Hat, Inc.