From: Michael Roth <michael.roth@amd.com>
TODO: add a CONFIG option that can be to completely skip arch
invalidation loop and avoid __weak references for arch/platforms that
don't need an additional invalidation hook.
In some cases, like with SEV-SNP, guest memory needs to be updated in a
platform-specific manner before it can be safely freed back to the host.
Add hooks to wire up handling of this sort when freeing memory in
response to FALLOC_FL_PUNCH_HOLE operations.
Also issue invalidations of all allocated pages when releasing the gmem
file so that the pages are not left in an unusable state when they get
freed back to the host.
Signed-off-by: Michael Roth <michael.roth@amd.com>
Link: https://lore.kernel.org/r/20230612042559.375660-3-michael.roth@amd.com
---
Changes v4 -> v5:
- Fix compile issue by adding static inline when gmem is disabled
Changes v2 -> v3:
- Newly added
---
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/x86.c | 6 +++++
include/linux/kvm_host.h | 3 +++
virt/kvm/guest_mem.c | 42 ++++++++++++++++++++++++++++++
5 files changed, 53 insertions(+)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 439ba4beb5af..48f043de2ec0 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -134,6 +134,7 @@ KVM_X86_OP(complete_emulated_msr)
KVM_X86_OP(vcpu_deliver_sipi_vector)
KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
+KVM_X86_OP_OPTIONAL(gmem_invalidate)
#undef KVM_X86_OP
#undef KVM_X86_OP_OPTIONAL
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2bc42f2887fa..17e78f9f2d17 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1735,6 +1735,7 @@ struct kvm_x86_ops {
int (*gmem_prepare)(struct kvm *kvm, struct kvm_memory_slot *slot,
kvm_pfn_t pfn, gfn_t gfn, u8 *max_level);
+ void (*gmem_invalidate)(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end);
};
struct kvm_x86_nested_ops {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index de195ad83ec0..b54818d02cb1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13274,6 +13274,12 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
+#ifdef CONFIG_KVM_PRIVATE_MEM
+void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end)
+{
+ static_call_cond(kvm_x86_gmem_invalidate)(kvm, start, end);
+}
+#endif
int kvm_spec_ctrl_test_value(u64 value)
{
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 091bc89ae805..349b0bf81fa5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2358,6 +2358,7 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
#ifdef CONFIG_KVM_PRIVATE_MEM
int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
gfn_t gfn, kvm_pfn_t *pfn, int *max_order);
+void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end);
#else
static inline int kvm_gmem_get_pfn(struct kvm *kvm,
struct kvm_memory_slot *slot, gfn_t gfn,
@@ -2366,6 +2367,8 @@ static inline int kvm_gmem_get_pfn(struct kvm *kvm,
KVM_BUG_ON(1, kvm);
return -EIO;
}
+
+static inline void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end) { }
#endif /* CONFIG_KVM_PRIVATE_MEM */
#endif
diff --git a/virt/kvm/guest_mem.c b/virt/kvm/guest_mem.c
index ed03f1d12172..342d2938716a 100644
--- a/virt/kvm/guest_mem.c
+++ b/virt/kvm/guest_mem.c
@@ -127,6 +127,46 @@ static void kvm_gmem_invalidate_end(struct kvm_gmem *gmem, pgoff_t start,
KVM_MMU_UNLOCK(kvm);
}
+void __weak kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end)
+{
+}
+
+/* Handle arch-specific hooks needed before releasing guarded pages. */
+static void kvm_gmem_issue_arch_invalidate(struct kvm *kvm, struct file *file,
+ pgoff_t start, pgoff_t end)
+{
+ pgoff_t file_end = i_size_read(file_inode(file)) >> PAGE_SHIFT;
+ pgoff_t index = start;
+
+ end = min(end, file_end);
+
+ while (index < end) {
+ struct folio *folio;
+ unsigned int order;
+ struct page *page;
+ kvm_pfn_t pfn;
+
+ folio = __filemap_get_folio(file->f_mapping, index,
+ FGP_LOCK, 0);
+ if (!folio) {
+ index++;
+ continue;
+ }
+
+ page = folio_file_page(folio, index);
+ pfn = page_to_pfn(page);
+ order = folio_order(folio);
+
+ kvm_arch_gmem_invalidate(kvm, pfn, pfn + min((1ul << order), end - index));
+
+ index = folio_next_index(folio);
+ folio_unlock(folio);
+ folio_put(folio);
+
+ cond_resched();
+ }
+}
+
static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
{
struct list_head *gmem_list = &inode->i_mapping->private_list;
@@ -143,6 +183,7 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
list_for_each_entry(gmem, gmem_list, entry)
kvm_gmem_invalidate_begin(gmem, start, end);
+ kvm_gmem_issue_arch_invalidate(kvm, file, start, end);
truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
list_for_each_entry(gmem, gmem_list, entry)
@@ -253,6 +294,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
* memory, as its lifetime is associated with the inode, not the file.
*/
kvm_gmem_invalidate_begin(gmem, 0, -1ul);
+ kvm_gmem_issue_arch_invalidate(gmem->kvm, file, 0, -1ul);
kvm_gmem_invalidate_end(gmem, 0, -1ul);
list_del(&gmem->entry);
--
2.25.1On 2023-08-15 at 10:18:53 -0700, isaku.yamahata@intel.com wrote:
> From: Michael Roth <michael.roth@amd.com>
>
> TODO: add a CONFIG option that can be to completely skip arch
> invalidation loop and avoid __weak references for arch/platforms that
> don't need an additional invalidation hook.
>
> In some cases, like with SEV-SNP, guest memory needs to be updated in a
> platform-specific manner before it can be safely freed back to the host.
> Add hooks to wire up handling of this sort when freeing memory in
> response to FALLOC_FL_PUNCH_HOLE operations.
>
> Also issue invalidations of all allocated pages when releasing the gmem
> file so that the pages are not left in an unusable state when they get
> freed back to the host.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> Link: https://lore.kernel.org/r/20230612042559.375660-3-michael.roth@amd.com
>
[...]
> +/* Handle arch-specific hooks needed before releasing guarded pages. */
> +static void kvm_gmem_issue_arch_invalidate(struct kvm *kvm, struct file *file,
> + pgoff_t start, pgoff_t end)
> +{
> + pgoff_t file_end = i_size_read(file_inode(file)) >> PAGE_SHIFT;
> + pgoff_t index = start;
> +
> + end = min(end, file_end);
> +
> + while (index < end) {
> + struct folio *folio;
> + unsigned int order;
> + struct page *page;
> + kvm_pfn_t pfn;
> +
> + folio = __filemap_get_folio(file->f_mapping, index,
> + FGP_LOCK, 0);
> + if (!folio) {
> + index++;
> + continue;
> + }
> +
> + page = folio_file_page(folio, index);
> + pfn = page_to_pfn(page);
> + order = folio_order(folio);
> +
> + kvm_arch_gmem_invalidate(kvm, pfn, pfn + min((1ul << order), end - index));
Observed an issue there.
The valid page may not point to the first page in the folio, then the
range [pfn, pfn + (1ul << order)) expands to the next folio. This makes
a part of the pages be invalidated again when loop to the next folio.
On TDX, it causes TDH_PHYMEM_PAGE_WBINVD failed.
> +
> + index = folio_next_index(folio);
> + folio_unlock(folio);
> + folio_put(folio);
> +
> + cond_resched();
> + }
> +}
My fix would be:
diff --git a/virt/kvm/guest_mem.c b/virt/kvm/guest_mem.c
index e629782d73d5..3665003c3746 100644
--- a/virt/kvm/guest_mem.c
+++ b/virt/kvm/guest_mem.c
@@ -155,7 +155,7 @@ static void kvm_gmem_issue_arch_invalidate(struct kvm *kvm, struct inode *inode,
while (index < end) {
struct folio *folio;
- unsigned int order;
+ pgoff_t ntails;
struct page *page;
kvm_pfn_t pfn;
@@ -168,9 +168,9 @@ static void kvm_gmem_issue_arch_invalidate(struct kvm *kvm, struct inode *inode,
page = folio_file_page(folio, index);
pfn = page_to_pfn(page);
- order = folio_order(folio);
+ ntails = folio_nr_pages(folio) - folio_page_idx(folio, page);
- kvm_arch_gmem_invalidate(kvm, pfn, pfn + min((1ul << order), end - index));
+ kvm_arch_gmem_invalidate(kvm, pfn, pfn + min(ntails, end - index));
index = folio_next_index(folio);
folio_unlock(folio);
Thanks,
Yilun
On Tue, Aug 15, 2023 at 10:18:53AM -0700,
isaku.yamahata@intel.com wrote:
> From: Michael Roth <michael.roth@amd.com>
>
> TODO: add a CONFIG option that can be to completely skip arch
> invalidation loop and avoid __weak references for arch/platforms that
> don't need an additional invalidation hook.
>
> In some cases, like with SEV-SNP, guest memory needs to be updated in a
> platform-specific manner before it can be safely freed back to the host.
> Add hooks to wire up handling of this sort when freeing memory in
> response to FALLOC_FL_PUNCH_HOLE operations.
>
> Also issue invalidations of all allocated pages when releasing the gmem
> file so that the pages are not left in an unusable state when they get
> freed back to the host.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> Link: https://lore.kernel.org/r/20230612042559.375660-3-michael.roth@amd.com
Somehow I used the old one. Here is the updated one. The change is the argument
for kvm_gmem_issue_arch_invalidate() is struct inode instead of struct file.
From e14483943e2ab6d8a0e4d00ea903509595847aa9 Mon Sep 17 00:00:00 2001
Message-Id: <e14483943e2ab6d8a0e4d00ea903509595847aa9.1692218085.git.isaku.yamahata@intel.com>
In-Reply-To: <cover.1692218085.git.isaku.yamahata@intel.com>
References: <cover.1692218085.git.isaku.yamahata@intel.com>
From: Michael Roth <michael.roth@amd.com>
Date: Sun, 11 Jun 2023 23:25:10 -0500
Subject: [PATCH 6/8] KVM: gmem, x86: Add gmem hook for invalidating private
memory
TODO: add a CONFIG option that can be to completely skip arch
invalidation loop and avoid __weak references for arch/platforms that
don't need an additional invalidation hook.
In some cases, like with SEV-SNP, guest memory needs to be updated in a
platform-specific manner before it can be safely freed back to the host.
Add hooks to wire up handling of this sort when freeing memory in
response to FALLOC_FL_PUNCH_HOLE operations.
Also issue invalidations of all allocated pages when releasing the gmem
file so that the pages are not left in an unusable state when they get
freed back to the host.
Signed-off-by: Michael Roth <michael.roth@amd.com>
Link: https://lore.kernel.org/r/20230612042559.375660-3-michael.roth@amd.com
---
Changes:
- Use struct inode instead of struct file.
Changes v4 -> v5:
- Fix compile issue by adding static inline when gmem is disabled
Changes v2 -> v3:
- Newly added
KVM: guest_mem: fix kvm_gmem_issue_arch_invalidate()
Now kvm_gmem_issue_arch_invalidate() takes struct inode instead of
struct file. Adjust argument.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/x86.c | 6 +++++
include/linux/kvm_host.h | 3 +++
virt/kvm/guest_mem.c | 42 ++++++++++++++++++++++++++++++
5 files changed, 53 insertions(+)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 439ba4beb5af..48f043de2ec0 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -134,6 +134,7 @@ KVM_X86_OP(complete_emulated_msr)
KVM_X86_OP(vcpu_deliver_sipi_vector)
KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
KVM_X86_OP_OPTIONAL_RET0(gmem_prepare)
+KVM_X86_OP_OPTIONAL(gmem_invalidate)
#undef KVM_X86_OP
#undef KVM_X86_OP_OPTIONAL
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2bc42f2887fa..17e78f9f2d17 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1735,6 +1735,7 @@ struct kvm_x86_ops {
int (*gmem_prepare)(struct kvm *kvm, struct kvm_memory_slot *slot,
kvm_pfn_t pfn, gfn_t gfn, u8 *max_level);
+ void (*gmem_invalidate)(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end);
};
struct kvm_x86_nested_ops {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index de195ad83ec0..b54818d02cb1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -13274,6 +13274,12 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
}
EXPORT_SYMBOL_GPL(kvm_arch_no_poll);
+#ifdef CONFIG_KVM_PRIVATE_MEM
+void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end)
+{
+ static_call_cond(kvm_x86_gmem_invalidate)(kvm, start, end);
+}
+#endif
int kvm_spec_ctrl_test_value(u64 value)
{
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 091bc89ae805..349b0bf81fa5 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2358,6 +2358,7 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
#ifdef CONFIG_KVM_PRIVATE_MEM
int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
gfn_t gfn, kvm_pfn_t *pfn, int *max_order);
+void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end);
#else
static inline int kvm_gmem_get_pfn(struct kvm *kvm,
struct kvm_memory_slot *slot, gfn_t gfn,
@@ -2366,6 +2367,8 @@ static inline int kvm_gmem_get_pfn(struct kvm *kvm,
KVM_BUG_ON(1, kvm);
return -EIO;
}
+
+static inline void kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end) { }
#endif /* CONFIG_KVM_PRIVATE_MEM */
#endif
diff --git a/virt/kvm/guest_mem.c b/virt/kvm/guest_mem.c
index ed03f1d12172..13d6dab08f87 100644
--- a/virt/kvm/guest_mem.c
+++ b/virt/kvm/guest_mem.c
@@ -127,6 +127,46 @@ static void kvm_gmem_invalidate_end(struct kvm_gmem *gmem, pgoff_t start,
KVM_MMU_UNLOCK(kvm);
}
+void __weak kvm_arch_gmem_invalidate(struct kvm *kvm, kvm_pfn_t start, kvm_pfn_t end)
+{
+}
+
+/* Handle arch-specific hooks needed before releasing guarded pages. */
+static void kvm_gmem_issue_arch_invalidate(struct kvm *kvm, struct inode *inode,
+ pgoff_t start, pgoff_t end)
+{
+ pgoff_t file_end = i_size_read(inode) >> PAGE_SHIFT;
+ pgoff_t index = start;
+
+ end = min(end, file_end);
+
+ while (index < end) {
+ struct folio *folio;
+ unsigned int order;
+ struct page *page;
+ kvm_pfn_t pfn;
+
+ folio = __filemap_get_folio(inode->i_mapping, index,
+ FGP_LOCK, 0);
+ if (!folio) {
+ index++;
+ continue;
+ }
+
+ page = folio_file_page(folio, index);
+ pfn = page_to_pfn(page);
+ order = folio_order(folio);
+
+ kvm_arch_gmem_invalidate(kvm, pfn, pfn + min((1ul << order), end - index));
+
+ index = folio_next_index(folio);
+ folio_unlock(folio);
+ folio_put(folio);
+
+ cond_resched();
+ }
+}
+
static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
{
struct list_head *gmem_list = &inode->i_mapping->private_list;
@@ -143,6 +183,7 @@ static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
list_for_each_entry(gmem, gmem_list, entry)
kvm_gmem_invalidate_begin(gmem, start, end);
+ kvm_gmem_issue_arch_invalidate(gmem->kvm, inode, start, end);
truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
list_for_each_entry(gmem, gmem_list, entry)
@@ -253,6 +294,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
* memory, as its lifetime is associated with the inode, not the file.
*/
kvm_gmem_invalidate_begin(gmem, 0, -1ul);
+ kvm_gmem_issue_arch_invalidate(gmem->kvm, file_inode(file), 0, -1ul);
kvm_gmem_invalidate_end(gmem, 0, -1ul);
list_del(&gmem->entry);
--
2.25.1
--
Isaku Yamahata <isaku.yamahata@linux.intel.com>
Hi,
kernel test robot noticed the following build errors:
[auto build test ERROR on 89b6a7b873d72280e85976bbb8fe4998b2ababa8]
url: https://github.com/intel-lab-lkp/linux/commits/isaku-yamahata-intel-com/KVM-gmem-Make-kvm_gmem_bind-return-EBADF-on-wrong-fd/20230816-012315
base: 89b6a7b873d72280e85976bbb8fe4998b2ababa8
patch link: https://lore.kernel.org/r/8c9f0470ba6e5dc122f3f4e37c4dcfb6fb97b184.1692119201.git.isaku.yamahata%40intel.com
patch subject: [PATCH 6/8] KVM: gmem, x86: Add gmem hook for invalidating private memory
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230816/202308160801.jwbys3HI-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230816/202308160801.jwbys3HI-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308160801.jwbys3HI-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/x86/kvm/../../../virt/kvm/guest_mem.c: In function 'kvm_gmem_punch_hole':
>> arch/x86/kvm/../../../virt/kvm/guest_mem.c:186:40: error: 'kvm' undeclared (first use in this function)
186 | kvm_gmem_issue_arch_invalidate(kvm, file, start, end);
| ^~~
arch/x86/kvm/../../../virt/kvm/guest_mem.c:186:40: note: each undeclared identifier is reported only once for each function it appears in
>> arch/x86/kvm/../../../virt/kvm/guest_mem.c:186:45: error: 'file' undeclared (first use in this function)
186 | kvm_gmem_issue_arch_invalidate(kvm, file, start, end);
| ^~~~
vim +/kvm +186 arch/x86/kvm/../../../virt/kvm/guest_mem.c
169
170 static long kvm_gmem_punch_hole(struct inode *inode, loff_t offset, loff_t len)
171 {
172 struct list_head *gmem_list = &inode->i_mapping->private_list;
173 pgoff_t start = offset >> PAGE_SHIFT;
174 pgoff_t end = (offset + len) >> PAGE_SHIFT;
175 struct kvm_gmem *gmem;
176
177 /*
178 * Bindings must stable across invalidation to ensure the start+end
179 * are balanced.
180 */
181 filemap_invalidate_lock(inode->i_mapping);
182
183 list_for_each_entry(gmem, gmem_list, entry)
184 kvm_gmem_invalidate_begin(gmem, start, end);
185
> 186 kvm_gmem_issue_arch_invalidate(kvm, file, start, end);
187 truncate_inode_pages_range(inode->i_mapping, offset, offset + len - 1);
188
189 list_for_each_entry(gmem, gmem_list, entry)
190 kvm_gmem_invalidate_end(gmem, start, end);
191
192 filemap_invalidate_unlock(inode->i_mapping);
193
194 return 0;
195 }
196
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.