* Re: [PATCH v2 4/5] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
From: Vishal Annapurve @ 2025-12-16 0:19 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-5-michael.roth@amd.com>
On Mon, Dec 15, 2025 at 7:36 AM Michael Roth <michael.roth@amd.com> wrote:
>
> Since it was never possible to use a non-PAGE_SIZE-aligned @source_addr,
> go ahead and document this as a requirement. This is in preparation for
> enforcing page-aligned @source_addr for all architectures in
> guest_memfd.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
> Documentation/virt/kvm/x86/intel-tdx.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/virt/kvm/x86/intel-tdx.rst b/Documentation/virt/kvm/x86/intel-tdx.rst
> index 5efac62c92c7..6a222e9d0954 100644
> --- a/Documentation/virt/kvm/x86/intel-tdx.rst
> +++ b/Documentation/virt/kvm/x86/intel-tdx.rst
> @@ -156,7 +156,7 @@ KVM_TDX_INIT_MEM_REGION
> :Returns: 0 on success, <0 on error
>
> Initialize @nr_pages TDX guest private memory starting from @gpa with userspace
> -provided data from @source_addr.
> +provided data from @source_addr. @source_addr must be PAGE_SIZE-aligned.
Reviewed-By: Vishal Annapurve <vannapurve@google.com>
^ permalink raw reply
* Re: [PATCH v2 5/5] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Vishal Annapurve @ 2025-12-16 0:18 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-6-michael.roth@amd.com>
On Mon, Dec 15, 2025 at 7:36 AM Michael Roth <michael.roth@amd.com> wrote:
>
> Currently the post-populate callbacks handle copying source pages into
> private GPA ranges backed by guest_memfd, where kvm_gmem_populate()
> acquires the filemap invalidate lock, then calls a post-populate
> callback which may issue a get_user_pages() on the source pages prior to
> copying them into the private GPA (e.g. TDX).
>
> This will not be compatible with in-place conversion, where the
> userspace page fault path will attempt to acquire filemap invalidate
> lock while holding the mm->mmap_lock, leading to a potential ABBA
> deadlock[1].
>
> Address this by hoisting the GUP above the filemap invalidate lock so
> that these page faults path can be taken early, prior to acquiring the
> filemap invalidate lock.
>
> It's not currently clear whether this issue is reachable with the
> current implementation of guest_memfd, which doesn't support in-place
> conversion, however it does provide a consistent mechanism to provide
> stable source/target PFNs to callbacks rather than punting to
> vendor-specific code, which allows for more commonality across
> architectures, which may be worthwhile even without in-place conversion.
>
> As part of this change, also begin enforcing that the 'src' argument to
> kvm_gmem_populate() must be page-aligned, as this greatly reduces the
> complexity around how the post-populate callbacks are implemented, and
> since no current in-tree users support using a non-page-aligned 'src'
> argument.
>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
Tested-By: Vishal Annapurve <vannapurve@google.com>
^ permalink raw reply
* Re: [PATCH v2 1/5] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Sean Christopherson @ 2025-12-16 0:18 UTC (permalink / raw)
To: Vishal Annapurve
Cc: Michael Roth, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, vbabka, ashish.kalra, liam.merwick,
david, ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <CAGtprH95s5wL1=rSSpG7Cmj5HhJOftwJY7CP27WE-qmq7hr+XA@mail.gmail.com>
On Mon, Dec 15, 2025, Vishal Annapurve wrote:
> On Mon, Dec 15, 2025 at 7:35 AM Michael Roth <michael.roth@amd.com> wrote:
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index fdaea3422c30..9dafa44838fe 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> > mapping_gfp_mask(inode->i_mapping), policy);
> > mpol_cond_put(policy);
> >
> > + /*
> > + * External interfaces like kvm_gmem_get_pfn() support dealing
> > + * with hugepages to a degree, but internally, guest_memfd currently
> > + * assumes that all folios are order-0 and handling would need
> > + * to be updated for anything otherwise (e.g. page-clearing
> > + * operations).
> > + */
> > + WARN_ON_ONCE(folio_order(folio));
>
> I am not sure if this WARN_ON adds any value. i.e. The current code
> can't hit it.
The current code _shouldn't_ hit it.
> This note concerns future efforts to add hugepage support and could be
> omitted altogether from the current implementation.
IMO, this is a good use of WARN_ON_ONCE(). It documents guest_memfd's assumptions
and/or limitations, which is extremely helpful to readers/contributors that aren't
familiar with guest_memfd and/or its history of hugepage support.
^ permalink raw reply
* Re: [PATCH v2 3/5] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
From: Vishal Annapurve @ 2025-12-16 0:15 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-4-michael.roth@amd.com>
On Mon, Dec 15, 2025 at 7:35 AM Michael Roth <michael.roth@amd.com> wrote:
>
> In the past, KVM_SEV_SNP_LAUNCH_UPDATE accepted a non-page-aligned
> 'uaddr' parameter to copy data from, but continuing to support this with
> new functionality like in-place conversion and hugepages in the pipeline
> has proven to be more trouble than it is worth, since there are no known
> users that have been identified who use a non-page-aligned 'uaddr'
> parameter.
>
> Rather than locking guest_memfd into continuing to support this, go
> ahead and document page-alignment as a requirement and begin enforcing
> this in the handling function.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
Reviewed-By: Vishal Annapurve <vannapurve@google.com>
^ permalink raw reply
* Re: [PATCH v2 2/5] KVM: guest_memfd: Remove preparation tracking
From: Vishal Annapurve @ 2025-12-16 0:13 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-3-michael.roth@amd.com>
On Mon, Dec 15, 2025 at 7:35 AM Michael Roth <michael.roth@amd.com> wrote:
>
> guest_memfd currently uses the folio uptodate flag to track:
>
> 1) whether or not a page has been cleared before initial usage
> 2) whether or not the architecture hooks have been issued to put the
> page in a private state as defined by the architecture
>
> In practice, 2) is only actually being tracked for SEV-SNP VMs, and
> there do not seem to be any plans/reasons that would suggest this will
> change in the future, so this additional tracking/complexity is not
> really providing any general benefit to guest_memfd users. Future plans
> around in-place conversion and hugepage support, where the per-folio
> uptodate flag is planned to be used purely to track the initial clearing
> of folios, whereas conversion operations could trigger multiple
> transitions between 'prepared' and 'unprepared' and thus need separate
> tracking, will make the burden of tracking this information within
> guest_memfd even more complex, since preparation generally happens
> during fault time, on the "read-side" of any global locks that might
> protect state tracked by guest_memfd, and so may require more complex
> locking schemes to allow for concurrent handling of page faults for
> multiple vCPUs where the "preparedness" state tracked by guest_memfd
> might need to be updated as part of handling the fault.
>
> Instead of keeping this current/future complexity within guest_memfd for
> what is essentially just SEV-SNP, just drop the tracking for 2) and have
> the arch-specific preparation hooks get triggered unconditionally on
> every fault so the arch-specific hooks can check the preparation state
> directly and decide whether or not a folio still needs additional
> preparation. In the case of SEV-SNP, the preparation state is already
> checked again via the preparation hooks to avoid double-preparation, so
> nothing extra needs to be done to update the handling of things there.
>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
Reviewed-By: Vishal Annapurve <vannapurve@google.com>
Tested-By: Vishal Annapurve <vannapurve@google.com>
^ permalink raw reply
* Re: [PATCH v2 1/5] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Vishal Annapurve @ 2025-12-16 0:12 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-2-michael.roth@amd.com>
On Mon, Dec 15, 2025 at 7:35 AM Michael Roth <michael.roth@amd.com> wrote:
>
> kvm_gmem_populate(), and the associated post-populate callbacks, have
> some limited support for dealing with guests backed by hugepages by
> passing the order information along to each post-populate callback and
> iterating through the pages passed to kvm_gmem_populate() in
> hugepage-chunks.
>
> However, guest_memfd doesn't yet support hugepages, and in most cases
> additional changes in the kvm_gmem_populate() path would also be needed
> to actually allow for this functionality.
>
> This makes the existing code unecessarily complex, and makes changes
> difficult to work through upstream due to theoretical impacts on
> hugepage support that can't be considered properly without an actual
> hugepage implementation to reference. So for now, remove what's there
> so changes for things like in-place conversion can be
> implemented/reviewed more efficiently.
>
> Suggested-by: Vishal Annapurve <vannapurve@google.com>
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Michael Roth <michael.roth@amd.com>
Tested-By: Vishal Annapurve <vannapurve@google.com>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index fdaea3422c30..9dafa44838fe 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> mapping_gfp_mask(inode->i_mapping), policy);
> mpol_cond_put(policy);
>
> + /*
> + * External interfaces like kvm_gmem_get_pfn() support dealing
> + * with hugepages to a degree, but internally, guest_memfd currently
> + * assumes that all folios are order-0 and handling would need
> + * to be updated for anything otherwise (e.g. page-clearing
> + * operations).
> + */
> + WARN_ON_ONCE(folio_order(folio));
I am not sure if this WARN_ON adds any value. i.e. The current code
can't hit it. This note concerns future efforts to add hugepage
support and could be omitted altogether from the current
implementation.
> +
> return folio;
> }
>
^ permalink raw reply
* [PATCH v2 5/5] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-1-michael.roth@amd.com>
Currently the post-populate callbacks handle copying source pages into
private GPA ranges backed by guest_memfd, where kvm_gmem_populate()
acquires the filemap invalidate lock, then calls a post-populate
callback which may issue a get_user_pages() on the source pages prior to
copying them into the private GPA (e.g. TDX).
This will not be compatible with in-place conversion, where the
userspace page fault path will attempt to acquire filemap invalidate
lock while holding the mm->mmap_lock, leading to a potential ABBA
deadlock[1].
Address this by hoisting the GUP above the filemap invalidate lock so
that these page faults path can be taken early, prior to acquiring the
filemap invalidate lock.
It's not currently clear whether this issue is reachable with the
current implementation of guest_memfd, which doesn't support in-place
conversion, however it does provide a consistent mechanism to provide
stable source/target PFNs to callbacks rather than punting to
vendor-specific code, which allows for more commonality across
architectures, which may be worthwhile even without in-place conversion.
As part of this change, also begin enforcing that the 'src' argument to
kvm_gmem_populate() must be page-aligned, as this greatly reduces the
complexity around how the post-populate callbacks are implemented, and
since no current in-tree users support using a non-page-aligned 'src'
argument.
Suggested-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Co-developed-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
arch/x86/kvm/svm/sev.c | 32 ++++++++-------
arch/x86/kvm/vmx/tdx.c | 15 +------
include/linux/kvm_host.h | 4 +-
virt/kvm/guest_memfd.c | 84 +++++++++++++++++++++++++++-------------
4 files changed, 77 insertions(+), 58 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 90c512ca24a9..11ae008aec8a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2268,7 +2268,7 @@ struct sev_gmem_populate_args {
};
static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
- void __user *src, void *opaque)
+ struct page *src_page, void *opaque)
{
struct sev_gmem_populate_args *sev_populate_args = opaque;
struct sev_data_snp_launch_update fw_args = {0};
@@ -2277,7 +2277,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
int level;
int ret;
- if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src))
+ if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
return -EINVAL;
ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
@@ -2288,14 +2288,14 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
goto out;
}
- if (src) {
- void *vaddr = kmap_local_pfn(pfn);
+ if (src_page) {
+ void *src_vaddr = kmap_local_pfn(page_to_pfn(src_page));
+ void *dst_vaddr = kmap_local_pfn(pfn);
- if (copy_from_user(vaddr, src, PAGE_SIZE)) {
- ret = -EFAULT;
- goto out;
- }
- kunmap_local(vaddr);
+ memcpy(dst_vaddr, src_vaddr, PAGE_SIZE);
+
+ kunmap_local(src_vaddr);
+ kunmap_local(dst_vaddr);
}
ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
@@ -2325,17 +2325,19 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
if (ret && !snp_page_reclaim(kvm, pfn) &&
sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
- void *vaddr = kmap_local_pfn(pfn);
+ void *src_vaddr = kmap_local_pfn(page_to_pfn(src_page));
+ void *dst_vaddr = kmap_local_pfn(pfn);
- if (copy_to_user(src, vaddr, PAGE_SIZE))
- pr_debug("Failed to write CPUID page back to userspace\n");
+ memcpy(src_vaddr, dst_vaddr, PAGE_SIZE);
- kunmap_local(vaddr);
+ kunmap_local(src_vaddr);
+ kunmap_local(dst_vaddr);
}
out:
- pr_debug("%s: exiting with return code %d (fw_error %d)\n",
- __func__, ret, sev_populate_args->fw_error);
+ if (ret)
+ pr_debug("%s: error updating GFN %llx, return code %d (fw_error %d)\n",
+ __func__, gfn, ret, sev_populate_args->fw_error);
return ret;
}
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 4fb042ce8ed1..3eb597c0e79f 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3118,34 +3118,21 @@ struct tdx_gmem_post_populate_arg {
};
static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
- void __user *src, void *_arg)
+ struct page *src_page, void *_arg)
{
struct tdx_gmem_post_populate_arg *arg = _arg;
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
u64 err, entry, level_state;
gpa_t gpa = gfn_to_gpa(gfn);
- struct page *src_page;
int ret, i;
if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
return -EIO;
- /*
- * Get the source page if it has been faulted in. Return failure if the
- * source page has been swapped out or unmapped in primary memory.
- */
- ret = get_user_pages_fast((unsigned long)src, 1, 0, &src_page);
- if (ret < 0)
- return ret;
- if (ret != 1)
- return -ENOMEM;
-
kvm_tdx->page_add_src = src_page;
ret = kvm_tdp_mmu_map_private_pfn(arg->vcpu, gfn, pfn);
kvm_tdx->page_add_src = NULL;
- put_page(src_page);
-
if (ret || !(arg->flags & KVM_TDX_MEASURE_MEMORY_REGION))
return ret;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1d0cee72e560..49c0cfe24fd8 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2566,7 +2566,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
* @gfn: starting GFN to be populated
* @src: userspace-provided buffer containing data to copy into GFN range
* (passed to @post_populate, and incremented on each iteration
- * if not NULL)
+ * if not NULL). Must be page-aligned.
* @npages: number of pages to copy from userspace-buffer
* @post_populate: callback to issue for each gmem page that backs the GPA
* range
@@ -2581,7 +2581,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
* Returns the number of pages that were populated.
*/
typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
- void __user *src, void *opaque);
+ struct page *page, void *opaque);
long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
kvm_gmem_populate_cb post_populate, void *opaque);
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 8b1248f42aae..18ae59b92257 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -820,12 +820,48 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
+
+static long __kvm_gmem_populate(struct kvm *kvm, struct kvm_memory_slot *slot,
+ struct file *file, gfn_t gfn, struct page *src_page,
+ kvm_gmem_populate_cb post_populate, void *opaque)
+{
+ pgoff_t index = kvm_gmem_get_index(slot, gfn);
+ struct folio *folio;
+ kvm_pfn_t pfn;
+ int ret;
+
+ filemap_invalidate_lock(file->f_mapping);
+
+ folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
+ if (IS_ERR(folio)) {
+ ret = PTR_ERR(folio);
+ goto out_unlock;
+ }
+
+ folio_unlock(folio);
+
+ if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
+ KVM_MEMORY_ATTRIBUTE_PRIVATE,
+ KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
+ ret = -EINVAL;
+ goto out_put_folio;
+ }
+
+ ret = post_populate(kvm, gfn, pfn, src_page, opaque);
+ if (!ret)
+ folio_mark_uptodate(folio);
+
+out_put_folio:
+ folio_put(folio);
+out_unlock:
+ filemap_invalidate_unlock(file->f_mapping);
+ return ret;
+}
+
long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long npages,
kvm_gmem_populate_cb post_populate, void *opaque)
{
struct kvm_memory_slot *slot;
- void __user *p;
-
int ret = 0;
long i;
@@ -834,6 +870,9 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
if (WARN_ON_ONCE(npages <= 0))
return -EINVAL;
+ if (WARN_ON_ONCE(!PAGE_ALIGNED(src)))
+ return -EINVAL;
+
slot = gfn_to_memslot(kvm, start_gfn);
if (!kvm_slot_has_gmem(slot))
return -EINVAL;
@@ -842,47 +881,38 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
if (!file)
return -EFAULT;
- filemap_invalidate_lock(file->f_mapping);
-
npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
for (i = 0; i < npages; i++) {
- struct folio *folio;
- gfn_t gfn = start_gfn + i;
- pgoff_t index = kvm_gmem_get_index(slot, gfn);
- kvm_pfn_t pfn;
+ struct page *src_page = NULL;
+ void __user *p;
if (signal_pending(current)) {
ret = -EINTR;
break;
}
- folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
- if (IS_ERR(folio)) {
- ret = PTR_ERR(folio);
- break;
- }
+ p = src ? src + i * PAGE_SIZE : NULL;
- folio_unlock(folio);
+ if (p) {
+ ret = get_user_pages_fast((unsigned long)p, 1, 0, &src_page);
+ if (ret < 0)
+ break;
+ if (ret != 1) {
+ ret = -ENOMEM;
+ break;
+ }
+ }
- ret = -EINVAL;
- if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
- KVM_MEMORY_ATTRIBUTE_PRIVATE,
- KVM_MEMORY_ATTRIBUTE_PRIVATE))
- goto put_folio_and_exit;
+ ret = __kvm_gmem_populate(kvm, slot, file, start_gfn + i, src_page,
+ post_populate, opaque);
- p = src ? src + i * PAGE_SIZE : NULL;
- ret = post_populate(kvm, gfn, pfn, p, opaque);
- if (!ret)
- folio_mark_uptodate(folio);
+ if (src_page)
+ put_page(src_page);
-put_folio_and_exit:
- folio_put(folio);
if (ret)
break;
}
- filemap_invalidate_unlock(file->f_mapping);
-
return ret && !i ? ret : i;
}
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_populate);
--
2.25.1
^ permalink raw reply related
* [PATCH v2 4/5] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-1-michael.roth@amd.com>
Since it was never possible to use a non-PAGE_SIZE-aligned @source_addr,
go ahead and document this as a requirement. This is in preparation for
enforcing page-aligned @source_addr for all architectures in
guest_memfd.
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
Documentation/virt/kvm/x86/intel-tdx.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/virt/kvm/x86/intel-tdx.rst b/Documentation/virt/kvm/x86/intel-tdx.rst
index 5efac62c92c7..6a222e9d0954 100644
--- a/Documentation/virt/kvm/x86/intel-tdx.rst
+++ b/Documentation/virt/kvm/x86/intel-tdx.rst
@@ -156,7 +156,7 @@ KVM_TDX_INIT_MEM_REGION
:Returns: 0 on success, <0 on error
Initialize @nr_pages TDX guest private memory starting from @gpa with userspace
-provided data from @source_addr.
+provided data from @source_addr. @source_addr must be PAGE_SIZE-aligned.
Note, before calling this sub command, memory attribute of the range
[gpa, gpa + nr_pages] needs to be private. Userspace can use
--
2.25.1
^ permalink raw reply related
* [PATCH v2 3/5] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-1-michael.roth@amd.com>
In the past, KVM_SEV_SNP_LAUNCH_UPDATE accepted a non-page-aligned
'uaddr' parameter to copy data from, but continuing to support this with
new functionality like in-place conversion and hugepages in the pipeline
has proven to be more trouble than it is worth, since there are no known
users that have been identified who use a non-page-aligned 'uaddr'
parameter.
Rather than locking guest_memfd into continuing to support this, go
ahead and document page-alignment as a requirement and begin enforcing
this in the handling function.
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
Documentation/virt/kvm/x86/amd-memory-encryption.rst | 2 +-
arch/x86/kvm/svm/sev.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
index 1ddb6a86ce7f..5a88d0197cb3 100644
--- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
+++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
@@ -523,7 +523,7 @@ Returns: 0 on success, < 0 on error, -EAGAIN if caller should retry
struct kvm_sev_snp_launch_update {
__u64 gfn_start; /* Guest page number to load/encrypt data into. */
- __u64 uaddr; /* Userspace address of data to be loaded/encrypted. */
+ __u64 uaddr; /* 4k-aligned address of data to be loaded/encrypted. */
__u64 len; /* 4k-aligned length in bytes to copy into guest memory.*/
__u8 type; /* The type of the guest pages being initialized. */
__u8 pad0;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 362c6135401a..90c512ca24a9 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2366,6 +2366,11 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
return -EINVAL;
+ src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
+
+ if (!PAGE_ALIGNED(src))
+ return -EINVAL;
+
npages = params.len / PAGE_SIZE;
/*
@@ -2397,7 +2402,6 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
sev_populate_args.sev_fd = argp->sev_fd;
sev_populate_args.type = params.type;
- src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
count = kvm_gmem_populate(kvm, params.gfn_start, src, npages,
sev_gmem_post_populate, &sev_populate_args);
--
2.25.1
^ permalink raw reply related
* [PATCH v2 2/5] KVM: guest_memfd: Remove preparation tracking
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-1-michael.roth@amd.com>
guest_memfd currently uses the folio uptodate flag to track:
1) whether or not a page has been cleared before initial usage
2) whether or not the architecture hooks have been issued to put the
page in a private state as defined by the architecture
In practice, 2) is only actually being tracked for SEV-SNP VMs, and
there do not seem to be any plans/reasons that would suggest this will
change in the future, so this additional tracking/complexity is not
really providing any general benefit to guest_memfd users. Future plans
around in-place conversion and hugepage support, where the per-folio
uptodate flag is planned to be used purely to track the initial clearing
of folios, whereas conversion operations could trigger multiple
transitions between 'prepared' and 'unprepared' and thus need separate
tracking, will make the burden of tracking this information within
guest_memfd even more complex, since preparation generally happens
during fault time, on the "read-side" of any global locks that might
protect state tracked by guest_memfd, and so may require more complex
locking schemes to allow for concurrent handling of page faults for
multiple vCPUs where the "preparedness" state tracked by guest_memfd
might need to be updated as part of handling the fault.
Instead of keeping this current/future complexity within guest_memfd for
what is essentially just SEV-SNP, just drop the tracking for 2) and have
the arch-specific preparation hooks get triggered unconditionally on
every fault so the arch-specific hooks can check the preparation state
directly and decide whether or not a folio still needs additional
preparation. In the case of SEV-SNP, the preparation state is already
checked again via the preparation hooks to avoid double-preparation, so
nothing extra needs to be done to update the handling of things there.
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
virt/kvm/guest_memfd.c | 44 ++++++++++++------------------------------
1 file changed, 12 insertions(+), 32 deletions(-)
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 9dafa44838fe..8b1248f42aae 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -76,11 +76,6 @@ static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slo
return 0;
}
-static inline void kvm_gmem_mark_prepared(struct folio *folio)
-{
- folio_mark_uptodate(folio);
-}
-
/*
* Process @folio, which contains @gfn, so that the guest can use it.
* The folio must be locked and the gfn must be contained in @slot.
@@ -90,13 +85,7 @@ static inline void kvm_gmem_mark_prepared(struct folio *folio)
static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
gfn_t gfn, struct folio *folio)
{
- unsigned long nr_pages, i;
pgoff_t index;
- int r;
-
- nr_pages = folio_nr_pages(folio);
- for (i = 0; i < nr_pages; i++)
- clear_highpage(folio_page(folio, i));
/*
* Preparing huge folios should always be safe, since it should
@@ -114,11 +103,8 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
WARN_ON(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
index = kvm_gmem_get_index(slot, gfn);
index = ALIGN_DOWN(index, folio_nr_pages(folio));
- r = __kvm_gmem_prepare_folio(kvm, slot, index, folio);
- if (!r)
- kvm_gmem_mark_prepared(folio);
- return r;
+ return __kvm_gmem_prepare_folio(kvm, slot, index, folio);
}
/*
@@ -429,7 +415,7 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
if (!folio_test_uptodate(folio)) {
clear_highpage(folio_page(folio, 0));
- kvm_gmem_mark_prepared(folio);
+ folio_mark_uptodate(folio);
}
vmf->page = folio_file_page(folio, vmf->pgoff);
@@ -766,7 +752,7 @@ void kvm_gmem_unbind(struct kvm_memory_slot *slot)
static struct folio *__kvm_gmem_get_pfn(struct file *file,
struct kvm_memory_slot *slot,
pgoff_t index, kvm_pfn_t *pfn,
- bool *is_prepared, int *max_order)
+ int *max_order)
{
struct file *slot_file = READ_ONCE(slot->gmem.file);
struct gmem_file *f = file->private_data;
@@ -796,7 +782,6 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
if (max_order)
*max_order = 0;
- *is_prepared = folio_test_uptodate(folio);
return folio;
}
@@ -806,19 +791,22 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
{
pgoff_t index = kvm_gmem_get_index(slot, gfn);
struct folio *folio;
- bool is_prepared = false;
int r = 0;
CLASS(gmem_get_file, file)(slot);
if (!file)
return -EFAULT;
- folio = __kvm_gmem_get_pfn(file, slot, index, pfn, &is_prepared, max_order);
+ folio = __kvm_gmem_get_pfn(file, slot, index, pfn, max_order);
if (IS_ERR(folio))
return PTR_ERR(folio);
- if (!is_prepared)
- r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
+ if (!folio_test_uptodate(folio)) {
+ clear_highpage(folio_page(folio, 0));
+ folio_mark_uptodate(folio);
+ }
+
+ r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
folio_unlock(folio);
@@ -861,7 +849,6 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
struct folio *folio;
gfn_t gfn = start_gfn + i;
pgoff_t index = kvm_gmem_get_index(slot, gfn);
- bool is_prepared = false;
kvm_pfn_t pfn;
if (signal_pending(current)) {
@@ -869,19 +856,12 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
break;
}
- folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, NULL);
+ folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, NULL);
if (IS_ERR(folio)) {
ret = PTR_ERR(folio);
break;
}
- if (is_prepared) {
- folio_unlock(folio);
- folio_put(folio);
- ret = -EEXIST;
- break;
- }
-
folio_unlock(folio);
ret = -EINVAL;
@@ -893,7 +873,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
p = src ? src + i * PAGE_SIZE : NULL;
ret = post_populate(kvm, gfn, pfn, p, opaque);
if (!ret)
- kvm_gmem_mark_prepared(folio);
+ folio_mark_uptodate(folio);
put_folio_and_exit:
folio_put(folio);
--
2.25.1
^ permalink raw reply related
* [PATCH v2 1/5] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
In-Reply-To: <20251215153411.3613928-1-michael.roth@amd.com>
kvm_gmem_populate(), and the associated post-populate callbacks, have
some limited support for dealing with guests backed by hugepages by
passing the order information along to each post-populate callback and
iterating through the pages passed to kvm_gmem_populate() in
hugepage-chunks.
However, guest_memfd doesn't yet support hugepages, and in most cases
additional changes in the kvm_gmem_populate() path would also be needed
to actually allow for this functionality.
This makes the existing code unecessarily complex, and makes changes
difficult to work through upstream due to theoretical impacts on
hugepage support that can't be considered properly without an actual
hugepage implementation to reference. So for now, remove what's there
so changes for things like in-place conversion can be
implemented/reviewed more efficiently.
Suggested-by: Vishal Annapurve <vannapurve@google.com>
Co-developed-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Signed-off-by: Michael Roth <michael.roth@amd.com>
---
arch/x86/kvm/svm/sev.c | 94 ++++++++++++++++------------------------
arch/x86/kvm/vmx/tdx.c | 2 +-
include/linux/kvm_host.h | 2 +-
virt/kvm/guest_memfd.c | 30 +++++++------
4 files changed, 56 insertions(+), 72 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f59c65abe3cf..362c6135401a 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2267,66 +2267,52 @@ struct sev_gmem_populate_args {
int fw_error;
};
-static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pfn,
- void __user *src, int order, void *opaque)
+static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
+ void __user *src, void *opaque)
{
struct sev_gmem_populate_args *sev_populate_args = opaque;
+ struct sev_data_snp_launch_update fw_args = {0};
struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
- int n_private = 0, ret, i;
- int npages = (1 << order);
- gfn_t gfn;
+ bool assigned = false;
+ int level;
+ int ret;
if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src))
return -EINVAL;
- for (gfn = gfn_start, i = 0; gfn < gfn_start + npages; gfn++, i++) {
- struct sev_data_snp_launch_update fw_args = {0};
- bool assigned = false;
- int level;
-
- ret = snp_lookup_rmpentry((u64)pfn + i, &assigned, &level);
- if (ret || assigned) {
- pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
- __func__, gfn, ret, assigned);
- ret = ret ? -EINVAL : -EEXIST;
- goto err;
- }
+ ret = snp_lookup_rmpentry((u64)pfn, &assigned, &level);
+ if (ret || assigned) {
+ pr_debug("%s: Failed to ensure GFN 0x%llx RMP entry is initial shared state, ret: %d assigned: %d\n",
+ __func__, gfn, ret, assigned);
+ ret = ret ? -EINVAL : -EEXIST;
+ goto out;
+ }
- if (src) {
- void *vaddr = kmap_local_pfn(pfn + i);
+ if (src) {
+ void *vaddr = kmap_local_pfn(pfn);
- if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
- ret = -EFAULT;
- goto err;
- }
- kunmap_local(vaddr);
+ if (copy_from_user(vaddr, src, PAGE_SIZE)) {
+ ret = -EFAULT;
+ goto out;
}
-
- ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
- sev_get_asid(kvm), true);
- if (ret)
- goto err;
-
- n_private++;
-
- fw_args.gctx_paddr = __psp_pa(sev->snp_context);
- fw_args.address = __sme_set(pfn_to_hpa(pfn + i));
- fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
- fw_args.page_type = sev_populate_args->type;
-
- ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
- &fw_args, &sev_populate_args->fw_error);
- if (ret)
- goto fw_err;
+ kunmap_local(vaddr);
}
- return 0;
+ ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
+ sev_get_asid(kvm), true);
+ if (ret)
+ goto out;
+
+ fw_args.gctx_paddr = __psp_pa(sev->snp_context);
+ fw_args.address = __sme_set(pfn_to_hpa(pfn));
+ fw_args.page_size = PG_LEVEL_TO_RMP(PG_LEVEL_4K);
+ fw_args.page_type = sev_populate_args->type;
-fw_err:
+ ret = __sev_issue_cmd(sev_populate_args->sev_fd, SEV_CMD_SNP_LAUNCH_UPDATE,
+ &fw_args, &sev_populate_args->fw_error);
/*
* If the firmware command failed handle the reclaim and cleanup of that
- * PFN specially vs. prior pages which can be cleaned up below without
- * needing to reclaim in advance.
+ * PFN before reporting an error.
*
* Additionally, when invalid CPUID function entries are detected,
* firmware writes the expected values into the page and leaves it
@@ -2336,26 +2322,20 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
* information to provide information on which CPUID leaves/fields
* failed CPUID validation.
*/
- if (!snp_page_reclaim(kvm, pfn + i) &&
+ if (ret && !snp_page_reclaim(kvm, pfn) &&
sev_populate_args->type == KVM_SEV_SNP_PAGE_TYPE_CPUID &&
sev_populate_args->fw_error == SEV_RET_INVALID_PARAM) {
- void *vaddr = kmap_local_pfn(pfn + i);
+ void *vaddr = kmap_local_pfn(pfn);
- if (copy_to_user(src + i * PAGE_SIZE, vaddr, PAGE_SIZE))
+ if (copy_to_user(src, vaddr, PAGE_SIZE))
pr_debug("Failed to write CPUID page back to userspace\n");
kunmap_local(vaddr);
}
- /* pfn + i is hypervisor-owned now, so skip below cleanup for it. */
- n_private--;
-
-err:
- pr_debug("%s: exiting with error ret %d (fw_error %d), restoring %d gmem PFNs to shared.\n",
- __func__, ret, sev_populate_args->fw_error, n_private);
- for (i = 0; i < n_private; i++)
- kvm_rmp_make_shared(kvm, pfn + i, PG_LEVEL_4K);
-
+out:
+ pr_debug("%s: exiting with return code %d (fw_error %d)\n",
+ __func__, ret, sev_populate_args->fw_error);
return ret;
}
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2d7a4d52ccfb..4fb042ce8ed1 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3118,7 +3118,7 @@ struct tdx_gmem_post_populate_arg {
};
static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
- void __user *src, int order, void *_arg)
+ void __user *src, void *_arg)
{
struct tdx_gmem_post_populate_arg *arg = _arg;
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d93f75b05ae2..1d0cee72e560 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2581,7 +2581,7 @@ int kvm_arch_gmem_prepare(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn, int max_ord
* Returns the number of pages that were populated.
*/
typedef int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
- void __user *src, int order, void *opaque);
+ void __user *src, void *opaque);
long kvm_gmem_populate(struct kvm *kvm, gfn_t gfn, void __user *src, long npages,
kvm_gmem_populate_cb post_populate, void *opaque);
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index fdaea3422c30..9dafa44838fe 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
mapping_gfp_mask(inode->i_mapping), policy);
mpol_cond_put(policy);
+ /*
+ * External interfaces like kvm_gmem_get_pfn() support dealing
+ * with hugepages to a degree, but internally, guest_memfd currently
+ * assumes that all folios are order-0 and handling would need
+ * to be updated for anything otherwise (e.g. page-clearing
+ * operations).
+ */
+ WARN_ON_ONCE(folio_order(folio));
+
return folio;
}
@@ -829,7 +838,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
struct kvm_memory_slot *slot;
void __user *p;
- int ret = 0, max_order;
+ int ret = 0;
long i;
lockdep_assert_held(&kvm->slots_lock);
@@ -848,7 +857,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
filemap_invalidate_lock(file->f_mapping);
npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
- for (i = 0; i < npages; i += (1 << max_order)) {
+ for (i = 0; i < npages; i++) {
struct folio *folio;
gfn_t gfn = start_gfn + i;
pgoff_t index = kvm_gmem_get_index(slot, gfn);
@@ -860,7 +869,7 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
break;
}
- folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, &max_order);
+ folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &is_prepared, NULL);
if (IS_ERR(folio)) {
ret = PTR_ERR(folio);
break;
@@ -874,20 +883,15 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
}
folio_unlock(folio);
- WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
- (npages - i) < (1 << max_order));
ret = -EINVAL;
- while (!kvm_range_has_memory_attributes(kvm, gfn, gfn + (1 << max_order),
- KVM_MEMORY_ATTRIBUTE_PRIVATE,
- KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
- if (!max_order)
- goto put_folio_and_exit;
- max_order--;
- }
+ if (!kvm_range_has_memory_attributes(kvm, gfn, gfn + 1,
+ KVM_MEMORY_ATTRIBUTE_PRIVATE,
+ KVM_MEMORY_ATTRIBUTE_PRIVATE))
+ goto put_folio_and_exit;
p = src ? src + i * PAGE_SIZE : NULL;
- ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
+ ret = post_populate(kvm, gfn, pfn, p, opaque);
if (!ret)
kvm_gmem_mark_prepared(folio);
--
2.25.1
^ permalink raw reply related
* [PATCH v2 0/5] KVM: guest_memfd: Rework preparation/population flows in prep for in-place conversion
From: Michael Roth @ 2025-12-15 15:34 UTC (permalink / raw)
To: kvm
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
seanjc, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao
This patchset is also available at:
https://github.com/AMDESE/linux/tree/gmem-populate-rework-v2
and is based on top of kvm/next (e0c26d47def7)
Overview
--------
Yan previously posted a series[1] that reworked kvm_gmem_populate() to deal
with potential locking issues that might arise once in-place conversion
support[2] is added for guest_memfd. To quote Yan's original summary of the
issues:
(1)
In Michael's series "KVM: gmem: 2MB THP support and preparedness tracking
changes" [4], kvm_gmem_get_pfn() was modified to rely on the filemap
invalidation lock for protecting its preparedness tracking. Similarly, the
in-place conversion version of guest_memfd series by Ackerly also requires
kvm_gmem_get_pfn() to acquire filemap invalidation lock [5].
kvm_gmem_get_pfn
filemap_invalidate_lock_shared(file_inode(file)->i_mapping);
However, since kvm_gmem_get_pfn() is called by kvm_tdp_map_page(), which is
in turn invoked within kvm_gmem_populate() in TDX, a deadlock occurs on the
filemap invalidation lock.
(2)
Moreover, in step 2, get_user_pages_fast() may acquire mm->mmap_lock,
resulting in the following lock sequence in tdx_vcpu_init_mem_region():
- filemap invalidation lock --> mm->mmap_lock
However, in future code, the shared filemap invalidation lock will be held
in kvm_gmem_fault_shared() (see [6]), leading to the lock sequence:
- mm->mmap_lock --> filemap invalidation lock
This creates an AB-BA deadlock issue.
Sean has since then addressed (1) with his series[3] that avoids relying on
calling kvm_gmem_get_pfn() within the TDX post-populate callback to re-fetch
the PFN that was passed to it.
This series aims to address (2), which is still outstanding, and does so based
heavily on Sean's suggested approach[4] of hoisting the get_user_pages_fast()
out of the TDX post-populate callback so that it can be called prior to taking
the filemap invalidate lock so that the ABBA deadlock is no longer possible.
As preperation for this change, all the partial enablement for hugepages in
the kvm_gmem_populate() path is stripped out so that it can be better
considered once hugepage support is actually in place and code/design can be
kept simpler in the meantime.
It additionally removes 'preparation' tracking from guest_memfd, which would
similarly complicate locking considerations in the context of in-place
conversion (and even moreso in the context of hugepage support). This has
been discussed during both the guest_memfd calls and PUCK calls, and so far
no strong objections have been given, so hopefully that particular change
isn't too controversial.
Some items worth noting/discussing
----------------------------------
(A) While one of the aims of this rework is to implement things such that
a separate source address can still be passed to kvm_gmem_populate()
even though the gmem pages can be populated in-place from userspace
beforehand, issues still arise if the source address itself has the
KVM_MEMORY_ATTRIBUTE_PRIVATE attribute set, e.g. if source/target
addresses are the same page. One line of reasoning would be to
conclude that KVM_MEMORY_ATTRIBUTE_PRIVATE implies that it cannot
be used as the source of a GUP/copy_from_user(), and thus cases like
source==target are naturally disallowed. Thus userspace has no choice
but to populate pages in-place *prior* to setting the
KVM_MEMORY_ATTRIBUTE_PRIVATE attribute (as kvm_gmem_populate()
requires), and passing in NULL for the source such that the GUP can
be skipped (otherwise, it will trigger the shared memory fault path,
which will then SIGBUS because it will see that it is faulting in
pages for which KVM_MEMORY_ATTRIBUTE_PRIVATE is set).
While workable, this would at the very least involve documentation
updates to KVM_TDX_INIT_MEM_REGION/KVM_SEV_SNP_LAUNCH_UPDATE to cover
these soon-to-be-possible scenarios. Ira posted a patch separately
that demonstrates how a NULL source could be safely handled within
the TDX post-populate callback[5].
Known issues / TODO
-------------------
- Compile-tested only for the TDX bits (testing/feedback welcome!)
Changes since RFC v1
--------------------
- and a prep patch to remove partial hugepage enablement in
kvm_gmem_populate() to simplify things until a hugepage implementation
can actually make use of it (Yan, Ira, Vishal, Sean)
- begin retroactively enforcing that source pages must be page-aligned
so that kvm_gmem_populate() callbacks can be simplified. add a patch
to update SNP user-facing documentation to mention this.
- drop handling for GUP'ing multiple pages before issuing callbacks.
This will only be needed for potentially for hugepages, and it must
simpler to handle per-page in the meantime. (Yan, Vishal)
- make sure TDX actually builds (Ira, Yan)
Thanks,
Mike
[1] https://lore.kernel.org/kvm/20250703062641.3247-1-yan.y.zhao@intel.com/
[2] https://lore.kernel.org/kvm/cover.1760731772.git.ackerleytng@google.com/
[3] https://lore.kernel.org/kvm/20251030200951.3402865-1-seanjc@google.com/
[4] https://lore.kernel.org/kvm/aHEwT4X0RcfZzHlt@google.com/
[5] https://lore.kernel.org/kvm/20251105-tdx-init-in-place-v1-1-1196b67d0423@intel.com/
----------------------------------------------------------------
Michael Roth (5):
KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
KVM: guest_memfd: Remove preparation tracking
KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
KVM: guest_memfd: GUP source pages prior to populating guest memory
.../virt/kvm/x86/amd-memory-encryption.rst | 2 +-
Documentation/virt/kvm/x86/intel-tdx.rst | 2 +-
arch/x86/kvm/svm/sev.c | 108 +++++++---------
arch/x86/kvm/vmx/tdx.c | 15 +--
include/linux/kvm_host.h | 4 +-
virt/kvm/guest_memfd.c | 140 +++++++++++----------
6 files changed, 129 insertions(+), 142 deletions(-)
^ permalink raw reply
* [PATCH] coco/guest: Constify struct configfs_item_operations and configfs_group_operations
From: Christophe JAILLET @ 2025-12-14 9:39 UTC (permalink / raw)
To: Dan Williams
Cc: linux-kernel, kernel-janitors, Christophe JAILLET, linux-coco
'struct configfs_item_operations' and 'configfs_group_operations' are not
modified in this driver.
Constifying these structures moves some data to a read-only section, so
increases overall security, especially when the structure holds some
function pointers.
On a x86_64, with allmodconfig:
Before:
======
text data bss dec hex filename
13784 6864 128 20776 5128 drivers/virt/coco/guest/report.o
After:
=====
text data bss dec hex filename
14040 6608 128 20776 5128 drivers/virt/coco/guest/report.o
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Compile tested only.
This change is possible since commits f2f36500a63b and f7f78098690d.
---
drivers/virt/coco/guest/report.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/virt/coco/guest/report.c b/drivers/virt/coco/guest/report.c
index d3d18fc22bc2..77f8dc3ca088 100644
--- a/drivers/virt/coco/guest/report.c
+++ b/drivers/virt/coco/guest/report.c
@@ -376,7 +376,7 @@ static void tsm_report_item_release(struct config_item *cfg)
kfree(state);
}
-static struct configfs_item_operations tsm_report_item_ops = {
+static const struct configfs_item_operations tsm_report_item_ops = {
.release = tsm_report_item_release,
};
@@ -406,7 +406,7 @@ static bool tsm_report_is_bin_visible(struct config_item *item,
return provider.ops->report_bin_attr_visible(n);
}
-static struct configfs_group_operations tsm_report_attr_group_ops = {
+static const struct configfs_group_operations tsm_report_attr_group_ops = {
.is_visible = tsm_report_is_visible,
.is_bin_visible = tsm_report_is_bin_visible,
};
@@ -443,7 +443,7 @@ static void tsm_report_drop_item(struct config_group *group, struct config_item
atomic_dec(&provider.count);
}
-static struct configfs_group_operations tsm_report_group_ops = {
+static const struct configfs_group_operations tsm_report_group_ops = {
.make_item = tsm_report_make_item,
.drop_item = tsm_report_drop_item,
};
--
2.52.0
^ permalink raw reply related
* Reminder: COCONUT-SVSM BoF at LPC on Sat, 3pm
From: Jörg Rödel @ 2025-12-13 1:02 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi everyone,
Here is a quick reminder that we will have a BoF session for COCONUT-SVSM today
at the Linux Plumbers Conference in Tokyo. The BoF is scheduled on Main Hall B
at 3pm Japan timezone. This is Friday, 10pm PST (US west coast), Saturday 1am
ET (US East Coast), and Saturday 7am MET (Europe).
Thanks to the fantastic organizers the session is in an AV-capable room, so you
can join it via BBB. At the given time, go to
https://meet.lpc.events/
and click the 'Join Session' button on the COCONUT-SVSM BoF session. Right now
there is no Live stream of the room, but that might change. So if you are not
registered for LPC virtual attendance it is worth checking out
https://www.youtube.com/@LinuxPlumbersConference
for a live stream of Main Hall B. I will also check on our COCONUT Matrix chat
room for remote feedback and questions on the session.
Hope to see you all there.
Regards,
Joerg
^ permalink raw reply
* Re: [PATCH v2 3/7] KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init
From: Sean Christopherson @ 2025-12-12 18:56 UTC (permalink / raw)
To: Chao Gao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Dan Williams
In-Reply-To: <aTfKeNMIiF8NCRlO@intel.com>
On Tue, Dec 09, 2025, Chao Gao wrote:
> On Fri, Dec 05, 2025 at 05:10:50PM -0800, Sean Christopherson wrote:
> > static int __init __tdx_bringup(void)
> > {
> > const struct tdx_sys_info_td_conf *td_conf;
> >@@ -3417,34 +3362,18 @@ static int __init __tdx_bringup(void)
> > }
> > }
> >
> >- /*
> >- * Enabling TDX requires enabling hardware virtualization first,
> >- * as making SEAMCALLs requires CPU being in post-VMXON state.
> >- */
> >- r = kvm_enable_virtualization();
> >- if (r)
> >- return r;
> >-
> >- cpus_read_lock();
> >- r = __do_tdx_bringup();
> >- cpus_read_unlock();
> >-
> >- if (r)
> >- goto tdx_bringup_err;
> >-
> >- r = -EINVAL;
> > /* Get TDX global information for later use */
> > tdx_sysinfo = tdx_get_sysinfo();
> >- if (WARN_ON_ONCE(!tdx_sysinfo))
> >- goto get_sysinfo_err;
> >+ if (!tdx_sysinfo)
> >+ return -EINVAL;
>
> ...
>
> >- /*
> >- * Ideally KVM should probe whether TDX module has been loaded
> >- * first and then try to bring it up. But TDX needs to use SEAMCALL
> >- * to probe whether the module is loaded (there is no CPUID or MSR
> >- * for that), and making SEAMCALL requires enabling virtualization
> >- * first, just like the rest steps of bringing up TDX module.
> >- *
> >- * So, for simplicity do everything in __tdx_bringup(); the first
> >- * SEAMCALL will return -ENODEV when the module is not loaded. The
> >- * only complication is having to make sure that initialization
> >- * SEAMCALLs don't return TDX_SEAMCALL_VMFAILINVALID in other
> >- * cases.
> >- */
> > r = __tdx_bringup();
> >- if (r) {
> >- /*
> >- * Disable TDX only but don't fail to load module if the TDX
> >- * module could not be loaded. No need to print message saying
> >- * "module is not loaded" because it was printed when the first
> >- * SEAMCALL failed. Don't bother unwinding the S-EPT hooks or
> >- * vm_size, as kvm_x86_ops have already been finalized (and are
> >- * intentionally not exported). The S-EPT code is unreachable,
> >- * and allocating a few more bytes per VM in a should-be-rare
> >- * failure scenario is a non-issue.
> >- */
> >- if (r == -ENODEV)
> >- goto success_disable_tdx;
>
> Previously, loading kvm-intel.ko (with tdx=1) would succeed even if there was
> no TDX module loaded by BIOS. IIUC, the behavior changes here; the lack of TDX
> module becomes fatal and kvm-intel.ko loading would fail.
>
> Is this intentional?
Nope, definitely not intentional. I think this as fixup?
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index d0161dc3d184..4e0372f12e6d 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -3365,7 +3365,7 @@ static int __init __tdx_bringup(void)
/* Get TDX global information for later use */
tdx_sysinfo = tdx_get_sysinfo();
if (!tdx_sysinfo)
- return -EINVAL;
+ return -ENODEV;
/* Check TDX module and KVM capabilities */
if (!tdx_get_supported_attrs(&tdx_sysinfo->td_conf) ||
@@ -3470,8 +3470,20 @@ int __init tdx_bringup(void)
}
r = __tdx_bringup();
- if (r)
- enable_tdx = 0;
+ if (r) {
+ /*
+ * Disable TDX only but don't fail to load module if the TDX
+ * module could not be loaded. No need to print message saying
+ * "module is not loaded" because it was printed when the first
+ * SEAMCALL failed. Don't bother unwinding the S-EPT hooks or
+ * vm_size, as kvm_x86_ops have already been finalized (and are
+ * intentionally not exported). The S-EPT code is unreachable,
+ * and allocating a few more bytes per VM in a should-be-rare
+ * failure scenario is a non-issue.
+ */
+ if (r == -ENODEV)
+ goto success_disable_tdx;
+ }
return r;
^ permalink raw reply related
* Re: [RFC PATCH 5/5] firmware: smccc: lfa: refresh fw details
From: Matt Ochs @ 2025-12-12 15:37 UTC (permalink / raw)
To: Vedashree Vidwans
Cc: salman.nabi@arm.com, sudeep.holla@arm.com, andre.przywara@arm.com,
lpieralisi@kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, Varun Wadekar, Shanker Donthineni,
Vikram Sethi, linux-coco@lists.linux.dev, ardb@kernel.org,
linux-arm-kernel@lists.infradead.org, chao.gao@intel.com
In-Reply-To: <20251208221319.1524888-6-vvidwans@nvidia.com>
> On Dec 8, 2025, at 16:13, Vedashree Vidwans <vvidwans@nvidia.com> wrote:
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 24916fc53420..334090708405 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
...
> + /*
> + * Pass 2:
> + * If current FW components number is more than previous list, add new component nodes.
> + */
> + for (node_idx; node_idx < num_of_components; node_idx++) {
drivers/firmware/smccc/lfa_fw.c: In function ‘update_fw_images_tree’:
drivers/firmware/smccc/lfa_fw.c:554:9: warning: statement with no effect [-Wunused-value]
554 | for (node_idx; node_idx < num_of_components; node_idx++) {
| ^~~
Please drop “node_idx” from the initializer statement.
^ permalink raw reply
* Re: [RFC PATCH 4/5] firmware: smccc: register as platform driver
From: Matt Ochs @ 2025-12-12 15:31 UTC (permalink / raw)
To: Vedashree Vidwans
Cc: salman.nabi@arm.com, sudeep.holla@arm.com, andre.przywara@arm.com,
lpieralisi@kernel.org, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, Varun Wadekar, Shanker Donthineni,
Vikram Sethi, linux-coco@lists.linux.dev, ardb@kernel.org,
linux-arm-kernel@lists.infradead.org, chao.gao@intel.com
In-Reply-To: <20251208221319.1524888-5-vvidwans@nvidia.com>
> On Dec 8, 2025, at 16:13, Vedashree Vidwans <vvidwans@nvidia.com> wrote:
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index 0e420cefa260..24916fc53420 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
...
> +
> +static int __init lfa_probe(struct platform_device *pdev)
> +{
WARNING: modpost: vmlinux: section mismatch in reference: lfa_driver+0x0 (section: .data) -> lfa_probe (section: .init.text)
__init is not needed here, please remove.
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-11 22:07 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: David Hildenbrand (Red Hat), linux-mm, linux-coco, linux-efi, x86,
linux-kernel, tglx, mingo, bp, dave.hansen, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <uc6yz23havsg2cdgtk3fgku7xr4gj2ykse7lxitcs4eh2fw4vo@hyavdm3ovdfh>
On 12/11/25 9:00 AM, Kiryl Shutsemau wrote:
> On Tue, Dec 09, 2025 at 03:36:09PM -0600, Pratik R. Sampat wrote:
>>> Agreed, I think Kiryl was hinting at pre-allocated bitmaps as well.
>>>
>>> Since, the overhead to do this upfront is fairly minimal, that should
>>> certainly simplify things and have very little to no meddling with the
>>> original EFI struct.
>>>
>>
>> Taking another look at this suggestion, I think there may be more to it
>> than I previously thought. Parsing e820 tables to know what the range
>> are for allocating the bitmap to cover hotplug may be difficult. For e.g
>>
>> [ 0.000000] efi: mem110: [Unaccepted <snip>]
>> range=[0x0000000100000000-0x000000017fffffff] (2048MB)
>> [ 0.000000] efi: mem111: [Reserved <snip>]
>> range=[0x000000fd00000000-0x000000ffffffffff] (12288MB)
>>
>> Parsing of the ACPI SRAT seems to be the one that gives us useful ranges
>> to base the upfront bitmap allocation on. e.g.
>> ...
>> [ 0.018357] ACPI: SRAT: Node 0 PXM 0 [mem 0x100000000-0x17fffffff]
>> [ 0.018781] ACPI: SRAT: Node 0 PXM 0 [mem 0x180000000-0x2ffffffff]
>> hotplug
>> This is also where max_possible_pfn gets updated to reflect this range.
>
> Do I understand correctly that EFI memory map doesn't mention hot plug
> range at all, but SRAT does?
>
> That's a mess. I thought, all hotpluggable range supposed to be declared
> in the memory map.
Not an EFI expert by a long shot, but seems so.
EFI_MEMORY_HOT_PLUGGABLE attribute does exist for hot-removable regions
of memory that must not be used for allocation during the boot context.
However, I am unclear if this in principle is also supposed to span
the entire range or just the cold-plugged regions of memory.
>
> I wounder if it is what BIOS provides, or is it result of EFI memmap
> cleanup by kernel? I see we are doing bunch of them, like in
> efi_remove_e820_mmio().
>
>> One potential solution could be to parse the SRAT during unaccepted
>> memory bitmap allocation in the EFI stub. However, this would fragment
>> the implementation by duplicating the SRAT parsing. Alternatively, we
>> could keep the current approach of dynamically allocating the bitmap on
>> hotplug or I could also replace the entire memblock_reserved unaccepted
>> table like Kiryl suggested if we must absolutely avoid changing the
>> unaccepted structure?
>
> Other possible option would be to accept all memory on hotplug and don't
> touch the bitmap at all. It might be not that bad: it doesn't block boot.
> We can think of a better solution later, if needed.
>
Absolutely, accepting memory as soon as it's added is easy.
Benchmarking it's effects may be a little tricky since unlike measuring
boot-time in eager vs lazy we may have to find representative workloads
to measure how much overheads accepting memory up-front adds.
Thanks
--Pratik
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Kiryl Shutsemau @ 2025-12-11 15:00 UTC (permalink / raw)
To: Pratik R. Sampat
Cc: David Hildenbrand (Red Hat), linux-mm, linux-coco, linux-efi, x86,
linux-kernel, tglx, mingo, bp, dave.hansen, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <a7ac1f74-32a6-41d1-82da-b338c127fb2e@amd.com>
On Tue, Dec 09, 2025 at 03:36:09PM -0600, Pratik R. Sampat wrote:
> > Agreed, I think Kiryl was hinting at pre-allocated bitmaps as well.
> >
> > Since, the overhead to do this upfront is fairly minimal, that should
> > certainly simplify things and have very little to no meddling with the
> > original EFI struct.
> >
>
> Taking another look at this suggestion, I think there may be more to it
> than I previously thought. Parsing e820 tables to know what the range
> are for allocating the bitmap to cover hotplug may be difficult. For e.g
>
> [ 0.000000] efi: mem110: [Unaccepted <snip>]
> range=[0x0000000100000000-0x000000017fffffff] (2048MB)
> [ 0.000000] efi: mem111: [Reserved <snip>]
> range=[0x000000fd00000000-0x000000ffffffffff] (12288MB)
>
> Parsing of the ACPI SRAT seems to be the one that gives us useful ranges
> to base the upfront bitmap allocation on. e.g.
> ...
> [ 0.018357] ACPI: SRAT: Node 0 PXM 0 [mem 0x100000000-0x17fffffff]
> [ 0.018781] ACPI: SRAT: Node 0 PXM 0 [mem 0x180000000-0x2ffffffff]
> hotplug
> This is also where max_possible_pfn gets updated to reflect this range.
Do I understand correctly that EFI memory map doesn't mention hot plug
range at all, but SRAT does?
That's a mess. I thought, all hotpluggable range supposed to be declared
in the memory map.
I wounder if it is what BIOS provides, or is it result of EFI memmap
cleanup by kernel? I see we are doing bunch of them, like in
efi_remove_e820_mmio().
> One potential solution could be to parse the SRAT during unaccepted
> memory bitmap allocation in the EFI stub. However, this would fragment
> the implementation by duplicating the SRAT parsing. Alternatively, we
> could keep the current approach of dynamically allocating the bitmap on
> hotplug or I could also replace the entire memblock_reserved unaccepted
> table like Kiryl suggested if we must absolutely avoid changing the
> unaccepted structure?
Other possible option would be to accept all memory on hotplug and don't
touch the bitmap at all. It might be not that bad: it doesn't block boot.
We can think of a better solution later, if needed.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* Re: [PATCH v4 06/16] x86/virt/tdx: Improve PAMT refcounts allocation for sparse memory
From: Edgecombe, Rick P @ 2025-12-11 0:07 UTC (permalink / raw)
To: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, seanjc@google.com, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
nik.borisov@suse.com, linux-kernel@vger.kernel.org,
Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
Cc: kirill.shutemov@linux.intel.com
In-Reply-To: <69a2dee2-f6a5-4c1b-9daa-8c32ff7c3956@suse.com>
On Thu, 2025-11-27 at 09:36 +0200, Nikolay Borisov wrote:
> I agree with your analysis but this needs to be described not only in
> the commit message but also as a code comment because you intentionally
> omit locking since that particular pte (at that point) can only have a
> single user so no race conditions are possible.
While commenting and writing up the log reasoning for why this is safe, I ended
up revisiting why we need to do all this manual PTE modification in the first
place.
In Kiryl's v2 he had the vmalloc area allocated with VM_IOREMAP. Kai asked why,
and seemingly as a result, it was changed to VM_SPARSE. It turns out the
VM_SPARSE flag is a newer thing for dealing with sparsely populated vmalloc
address space mappings, exactly like we have here. It comes with some helpers
for mapping and unmapping sparse vmalloc ranges. So I played around with these a
bit.
The current apply_to_page_range() version is optimally efficient, but the
allocation is a one-time operation, and the freeing is actually only called in
an error path. I'm not sure it needs to be optimal.
If you use the helpers to populate, you pretty much just need to allocate all
the pages up front, and then call vm_area_map_pages() to map them. To unmap (the
error path), the code pattern is to iterate through the mapping a page at a
time, fetch the page with vmalloc_to_page(), unmap and free the page. At most
this is 2,097,152 iterations. Not fast, but not going to hang boot either.
So rather than the try to justify the locking, I'm thinking to go with a
stupider, simpler mapping/unmapping method that uses the ready made VM_SPARSE
helpers.
I hate to change things at this point, but I think the discussed reasoning is
going to beg the question of why it needs to be complicated.
^ permalink raw reply
* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: Sean Christopherson @ 2025-12-10 14:20 UTC (permalink / raw)
To: dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <6939242dcfff1_20cb5100c3@dwillia2-mobl4.notmuch>
On Wed, Dec 10, 2025, dan.j.williams@intel.com wrote:
> Sean Christopherson wrote:
> > On Sat, Dec 06, 2025, dan.j.williams@intel.com wrote:
> > I don't think we need anything at this time. INTEL_TDX_HOST depends on KVM_INTEL,
> > and so without a user that needs VMXON without KVM_INTEL, I think we're good as-is.
> >
> > config INTEL_TDX_HOST
> > bool "Intel Trust Domain Extensions (TDX) host support"
> > depends on CPU_SUP_INTEL
> > depends on X86_64
> > depends on KVM_INTEL
>
> ...but INTEL_TDX_HOST, it turns out, does not have any functional
> dependencies on KVM_INTEL. At least, not since I last checked. Yes, it
> would be silly and result in dead code today to do a build with:
>
> CONFIG_INTEL_TDX_HOST=y
> CONFIG_KVM_INTEL=n
>
> However, when the TDX Connect support arrives you could have:
>
> CONFIG_INTEL_TDX_HOST=y
> CONFIG_KVM_INTEL=n
> CONFIG_TDX_HOST_SERVICES=y
>
> Where "TDX Host Services" is a driver for PCIe Link Encryption and TDX
> Module update. Whether such configuration freedom has any practical
> value is a separate question.
>
> I am ok if the answer is, "wait until someone shows up who really wants
> PCIe Link Encryption without KVM".
Ya, that's my answer. At the very least, wait until TDX_HOST_SERVICES comes
along.
^ permalink raw reply
* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: dan.j.williams @ 2025-12-10 7:41 UTC (permalink / raw)
To: Sean Christopherson, dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <aTiAKG4TlKcZnJnn@google.com>
Sean Christopherson wrote:
> On Sat, Dec 06, 2025, dan.j.williams@intel.com wrote:
> > Sean Christopherson wrote:
> > > @@ -694,9 +696,6 @@ static void drop_user_return_notifiers(void)
> > > kvm_on_user_return(&msrs->urn);
> > > }
> > >
> > > -__visible bool kvm_rebooting;
> > > -EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_rebooting);
> >
> > ...a short stay for this symbol in kvm/x86.c? It raises my curiosity why
> > patch1 is separate.
>
> Because it affects non-x86 architectures. It should be a complete nop, but I
> wanted to isolate what I could.
Ok.
[..]
> > > +static cpu_emergency_virt_cb __rcu *kvm_emergency_callback;
> >
> > Hmm, why kvm_ and not virt_?
>
> I was trying to capture that this callback can _only_ be used by KVM, because
> KVM is the only in-tree hypervisor. That's also why the exports are only for
> KVM (and will use EXPORT_SYMBOL_FOR_KVM() when I post the next version).
Oh, true, that makes sense.
> > [..]
> > > +#if IS_ENABLED(CONFIG_KVM_INTEL)
> > > +static DEFINE_PER_CPU(struct vmcs *, root_vmcs);
> >
> > Perhaps introduce a CONFIG_INTEL_VMX for this? For example, KVM need not
> > be enabled if all one wants to do is use TDX to setup PCIe Link
> > Encryption. ...or were you expecting?
> >
> > #if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(...<other VMX users>...)
>
> I don't think we need anything at this time. INTEL_TDX_HOST depends on KVM_INTEL,
> and so without a user that needs VMXON without KVM_INTEL, I think we're good as-is.
>
> config INTEL_TDX_HOST
> bool "Intel Trust Domain Extensions (TDX) host support"
> depends on CPU_SUP_INTEL
> depends on X86_64
> depends on KVM_INTEL
...but INTEL_TDX_HOST, it turns out, does not have any functional
dependencies on KVM_INTEL. At least, not since I last checked. Yes, it
would be silly and result in dead code today to do a build with:
CONFIG_INTEL_TDX_HOST=y
CONFIG_KVM_INTEL=n
However, when the TDX Connect support arrives you could have:
CONFIG_INTEL_TDX_HOST=y
CONFIG_KVM_INTEL=n
CONFIG_TDX_HOST_SERVICES=y
Where "TDX Host Services" is a driver for PCIe Link Encryption and TDX
Module update. Whether such configuration freedom has any practical
value is a separate question.
I am ok if the answer is, "wait until someone shows up who really wants
PCIe Link Encryption without KVM".
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Sean Christopherson @ 2025-12-10 1:30 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Michael Roth, FirstName LastName, ackerleytng, aik, ashish.kalra,
david, ira.weiny, kvm, liam.merwick, linux-coco, linux-kernel,
linux-mm, pbonzini, thomas.lendacky, yan.y.zhao
In-Reply-To: <5649e224-bb6a-4b63-bb27-5541216df0b6@suse.cz>
On Mon, Dec 08, 2025, Vlastimil Babka wrote:
> On 12/4/25 00:12, Michael Roth wrote:
> > On Wed, Dec 03, 2025 at 08:59:10PM +0000, FirstName LastName wrote:
> >>
> >> e.g. 4K page based population logic will keep things simple and can be
> >> further simplified if we can add PAGE_ALIGNED(params.uaddr) restriction.
> >
> > I'm still hesitant to pull the trigger on retroactively enforcing
> > page-aligned uaddr for SNP, but if the maintainers are good with it then
> > no objection from me.
>
> IMHO it would be for the best. If there are no known users that would break,
> it's worth trying. The "do not break userspace" rule isn't about eliminating
> any theoretical possibility, but indeed about known breakages (and reacting
> appropriately to reports about previously unknown breakages). Perhaps any
> such users would be also willing to adjust and not demand a revert.
+1. This code is already crazy complex, we should jump at any simplification
possible. Especially since we expect in-place conversion to dominate usage in
the future, and in-place conversion is incompatible with an unaligned source.
^ permalink raw reply
* Re: [RFC PATCH 2/4] mm: Add support for unaccepted memory hotplug
From: Pratik R. Sampat @ 2025-12-09 21:36 UTC (permalink / raw)
To: David Hildenbrand (Red Hat), linux-mm, linux-coco, linux-efi, x86,
linux-kernel
Cc: tglx, mingo, bp, dave.hansen, kas, ardb, akpm, osalvador,
thomas.lendacky, michael.roth
In-Reply-To: <89fde0fd-57c4-4146-82fc-a4c1a56e74ec@amd.com>
On 12/1/25 1:35 PM, Pratik R. Sampat wrote:
>
>
> On 12/1/25 12:36 PM, David Hildenbrand (Red Hat) wrote:
>> On 12/1/25 18:21, Pratik R. Sampat wrote:
>>>
>>>
>>> On 11/28/25 3:32 AM, David Hildenbrand (Red Hat) wrote:
>>>> On 11/25/25 18:57, Pratik R. Sampat wrote:
>>>>> The unaccepted memory structure currently only supports accepting memory
>>>>> present at boot time. The unaccepted table uses a fixed-size bitmap
>>>>> reserved in memblock based on the initial memory layout, preventing
>>>>> dynamic addition of memory ranges after boot. This causes guest
>>>>> termination when memory is hot-added in a secure virtual machine due to
>>>>> accessing pages that have not transitioned to private before use.
>>>>>
>>>>> Extend the unaccepted memory framework to handle hotplugged memory by
>>>>> dynamically managing the unaccepted bitmap. Allocate a new bitmap when
>>>>> hotplugged ranges exceed the reserved bitmap capacity and switch to
>>>>> kernel-managed allocation.
>>>>>
>>>>> Hotplugged memory also follows the same acceptance policy using the
>>>>> accept_memory=[eager|lazy] kernel parameter to accept memory either
>>>>> up-front when added or before first use.
>>>>>
>>>>> Signed-off-by: Pratik R. Sampat <prsampat@amd.com>
>>>>> ---
>>>>> arch/x86/boot/compressed/efi.h | 1 +
>>>>> .../firmware/efi/libstub/unaccepted_memory.c | 1 +
>>>>> drivers/firmware/efi/unaccepted_memory.c | 83 +++++++++++++++++++
>>>>> include/linux/efi.h | 1 +
>>>>> include/linux/mm.h | 11 +++
>>>>> mm/memory_hotplug.c | 7 ++
>>>>> mm/page_alloc.c | 2 +
>>>>> 7 files changed, 106 insertions(+)
>>>>>
>>>>> diff --git a/arch/x86/boot/compressed/efi.h b/arch/x86/boot/compressed/efi.h
>>>>> index 4f7027f33def..a220a1966cae 100644
>>>>> --- a/arch/x86/boot/compressed/efi.h
>>>>> +++ b/arch/x86/boot/compressed/efi.h
>>>>> @@ -102,6 +102,7 @@ struct efi_unaccepted_memory {
>>>>> u32 unit_size;
>>>>> u64 phys_base;
>>>>> u64 size;
>>>>> + bool mem_reserved;
>>>>> unsigned long *bitmap;
>>>>> };
>>>>> diff --git a/drivers/firmware/efi/libstub/unaccepted_memory.c b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>>> index c1370fc14555..b16bd61c12bf 100644
>>>>> --- a/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>>> +++ b/drivers/firmware/efi/libstub/unaccepted_memory.c
>>>>> @@ -83,6 +83,7 @@ efi_status_t allocate_unaccepted_bitmap(__u32 nr_desc,
>>>>> unaccepted_table->unit_size = EFI_UNACCEPTED_UNIT_SIZE;
>>>>> unaccepted_table->phys_base = unaccepted_start;
>>>>> unaccepted_table->size = bitmap_size;
>>>>> + unaccepted_table->mem_reserved = true;
>>>>> memset(unaccepted_table->bitmap, 0, bitmap_size);
>>>>> status = efi_bs_call(install_configuration_table,
>>>>> diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c
>>>>> index 4479aad258f8..8537812346e2 100644
>>>>> --- a/drivers/firmware/efi/unaccepted_memory.c
>>>>> +++ b/drivers/firmware/efi/unaccepted_memory.c
>>>>> @@ -218,6 +218,89 @@ bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size)
>>>>> return ret;
>>>>> }
>>>>> +static int extend_unaccepted_bitmap(phys_addr_t mem_range_start,
>>>>> + unsigned long mem_range_size)
>>>>> +{
>>>>> + struct efi_unaccepted_memory *unacc_tbl;
>>>>> + unsigned long *old_bitmap, *new_bitmap;
>>>>> + phys_addr_t start, end, mem_range_end;
>>>>> + u64 phys_base, size, unit_size;
>>>>> + unsigned long flags;
>>>>> +
>>>>> + unacc_tbl = efi_get_unaccepted_table();
>>>>> + if (!unacc_tbl || !unacc_tbl->unit_size)
>>>>> + return -EIO;
>>>>> +
>>>>> + unit_size = unacc_tbl->unit_size;
>>>>> + phys_base = unacc_tbl->phys_base;
>>>>> +
>>>>> + mem_range_end = round_up(mem_range_start + mem_range_size, unit_size);
>>>>> + size = DIV_ROUND_UP(mem_range_end - phys_base, unit_size * BITS_PER_BYTE);
>>>>> +
>>>>> + /* Translate to offsets from the beginning of the bitmap */
>>>>> + start = mem_range_start - phys_base;
>>>>> + end = mem_range_end - phys_base;
>>>>> +
>>>>> + old_bitmap = efi_get_unaccepted_bitmap();
>>>>> + if (!old_bitmap)
>>>>> + return -EIO;
>>>>> +
>>>>> + /* If the bitmap is already large enough, just set the bits */
>>>>> + if (unacc_tbl->size >= size) {
>>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>>> + bitmap_set(old_bitmap, start / unit_size, (end - start) / unit_size);
>>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>>> +
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + /* Reserved memblocks cannot be extended so allocate a new bitmap */
>>>>> + if (unacc_tbl->mem_reserved) {
>>>>> + new_bitmap = kzalloc(size, GFP_KERNEL);
>>>>> + if (!new_bitmap)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>>> + memcpy(new_bitmap, old_bitmap, unacc_tbl->size);
>>>>> + unacc_tbl->mem_reserved = false;
>>>>> + free_reserved_area(old_bitmap, old_bitmap + unacc_tbl->size, -1, NULL);
>>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>>> + } else {
>>>>> + new_bitmap = krealloc(old_bitmap, size, GFP_KERNEL);
>>>>> + if (!new_bitmap)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + /* Zero the bitmap from the range it was extended from */
>>>>> + memset(new_bitmap + unacc_tbl->size, 0, size - unacc_tbl->size);
>>>>> + }
>>>>> +
>>>>> + bitmap_set(new_bitmap, start / unit_size, (end - start) / unit_size);
>>>>> +
>>>>> + spin_lock_irqsave(&unaccepted_memory_lock, flags);
>>>>> + unacc_tbl->size = size;
>>>>> + unacc_tbl->bitmap = (unsigned long *)__pa(new_bitmap);
>>>>> + spin_unlock_irqrestore(&unaccepted_memory_lock, flags);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +int accept_hotplug_memory(phys_addr_t mem_range_start, unsigned long mem_range_size)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (!IS_ENABLED(CONFIG_UNACCEPTED_MEMORY))
>>>>> + return 0;
>>>>> +
>>>>> + ret = extend_unaccepted_bitmap(mem_range_start, mem_range_size);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + if (!mm_lazy_accept_enabled())
>>>>> + accept_memory(mem_range_start, mem_range_size);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> #ifdef CONFIG_PROC_VMCORE
>>>>> static bool unaccepted_memory_vmcore_pfn_is_ram(struct vmcore_cb *cb,
>>>>> unsigned long pfn)
>>>>> diff --git a/include/linux/efi.h b/include/linux/efi.h
>>>>> index a74b393c54d8..1021eb78388f 100644
>>>>> --- a/include/linux/efi.h
>>>>> +++ b/include/linux/efi.h
>>>>> @@ -545,6 +545,7 @@ struct efi_unaccepted_memory {
>>>>> u32 unit_size;
>>>>> u64 phys_base;
>>>>> u64 size;
>>>>> + bool mem_reserved;
>>>>> unsigned long *bitmap;
>>>>> };
>>>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>>>> index 1ae97a0b8ec7..bb43876e6c47 100644
>>>>> --- a/include/linux/mm.h
>>>>> +++ b/include/linux/mm.h
>>>>> @@ -4077,6 +4077,9 @@ int set_anon_vma_name(unsigned long addr, unsigned long size,
>>>>> bool range_contains_unaccepted_memory(phys_addr_t start, unsigned long size);
>>>>> void accept_memory(phys_addr_t start, unsigned long size);
>>>>> +int accept_hotplug_memory(phys_addr_t mem_range_start,
>>>>> + unsigned long mem_range_size);
>>>>> +bool mm_lazy_accept_enabled(void);
>>>>> #else
>>>>> @@ -4090,6 +4093,14 @@ static inline void accept_memory(phys_addr_t start, unsigned long size)
>>>>> {
>>>>> }
>>>>> +static inline int accept_hotplug_memory(phys_addr_t mem_range_start,
>>>>> + unsigned long mem_range_size)
>>>>> +{
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static inline bool mm_lazy_accept_enabled(void) { return false; }
>>>>> +
>>>>> #endif
>>>>> static inline bool pfn_is_unaccepted_memory(unsigned long pfn)
>>>>> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
>>>>> index 74318c787715..bf8086682b66 100644
>>>>> --- a/mm/memory_hotplug.c
>>>>> +++ b/mm/memory_hotplug.c
>>>>> @@ -1581,6 +1581,13 @@ int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
>>>>> if (!strcmp(res->name, "System RAM"))
>>>>> firmware_map_add_hotplug(start, start + size, "System RAM");
>>>>> + ret = accept_hotplug_memory(start, size);
>>>>
>>>> What makes this special that we have to have "hotplug_memory" as part of the name?
>>>>
>>>> Staring at the helper itself, there isn't anything really hotplug specific happening in there except extending the bitmap, maybe?
>>>>
>>>
>>> Right, we are extending the original bitmap and initializing a structure
>>> to track state as well. I added the hotplug_memory keyword without
>>> much thought, since I didn't see anyone else attempting to extend these
>>> structures.
>>>
>>> That said, I agree the name is awkward. I could either come up with
>>> something different, or we could eliminate the parent function
>>> entirely and call extend_unaccepted_bitmap() + accept_memory() directly
>>> from add_memory_resource(). Similarly, we could do the same to
>>> s/unaccept_hotplug_memory/unaccept_memory too.
>>
>> BTW, can't we allocate the bitmap based on maximum memory in the system as indicated by e820 (which includes to-maybe-be-hotplugged-ranges) and not do this allocation during hotplug events?
>>
>> If you search for max_possible_pfn / max_pfn I think you should find what I mean.
>>
>> Then it would be a simple accept_memory().
>>
>
> Agreed, I think Kiryl was hinting at pre-allocated bitmaps as well.
>
> Since, the overhead to do this upfront is fairly minimal, that should
> certainly simplify things and have very little to no meddling with the
> original EFI struct.
>
Taking another look at this suggestion, I think there may be more to it
than I previously thought. Parsing e820 tables to know what the range
are for allocating the bitmap to cover hotplug may be difficult. For e.g
[ 0.000000] efi: mem110: [Unaccepted <snip>]
range=[0x0000000100000000-0x000000017fffffff] (2048MB)
[ 0.000000] efi: mem111: [Reserved <snip>]
range=[0x000000fd00000000-0x000000ffffffffff] (12288MB)
Parsing of the ACPI SRAT seems to be the one that gives us useful ranges
to base the upfront bitmap allocation on. e.g.
...
[ 0.018357] ACPI: SRAT: Node 0 PXM 0 [mem 0x100000000-0x17fffffff]
[ 0.018781] ACPI: SRAT: Node 0 PXM 0 [mem 0x180000000-0x2ffffffff]
hotplug
This is also where max_possible_pfn gets updated to reflect this range.
One potential solution could be to parse the SRAT during unaccepted
memory bitmap allocation in the EFI stub. However, this would fragment
the implementation by duplicating the SRAT parsing. Alternatively, we
could keep the current approach of dynamically allocating the bitmap on
hotplug or I could also replace the entire memblock_reserved unaccepted
table like Kiryl suggested if we must absolutely avoid changing the
unaccepted structure?
--Pratik
^ permalink raw reply
* Re: [PATCH v2 2/7] KVM: x86: Extract VMXON and EFER.SVME enablement to kernel
From: Sean Christopherson @ 2025-12-09 20:01 UTC (permalink / raw)
To: dan.j.williams
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Kiryl Shutsemau, Paolo Bonzini, linux-kernel, linux-coco, kvm,
Chao Gao
In-Reply-To: <69352b2239a33_1b2e100d2@dwillia2-mobl4.notmuch>
On Sat, Dec 06, 2025, dan.j.williams@intel.com wrote:
> Sean Christopherson wrote:
> > @@ -694,9 +696,6 @@ static void drop_user_return_notifiers(void)
> > kvm_on_user_return(&msrs->urn);
> > }
> >
> > -__visible bool kvm_rebooting;
> > -EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_rebooting);
>
> ...a short stay for this symbol in kvm/x86.c? It raises my curiosity why
> patch1 is separate.
Because it affects non-x86 architectures. It should be a complete nop, but I
wanted to isolate what I could.
> Patch1 looked like the start of a series of incremental conversions, patch2
> is a combo move. I am ok either way, just questioning consistency. I.e. if
> combo move then patch1 folds in here, if incremental, perhaps split out other
> combo conversions like emergency_disable_virtualization_cpu()? The aspect of
> "this got moved twice in the same patchset" is what poked me.
Yeah, I got lazy to a large extent. I'm not super optimistic that we won't end
up with one big "move all this stuff" patch, but I agree it doesn't need to be
_this_ big.
> [..]
> > diff --git a/arch/x86/virt/hw.c b/arch/x86/virt/hw.c
> > new file mode 100644
> > index 000000000000..986e780cf438
> > --- /dev/null
> > +++ b/arch/x86/virt/hw.c
> > @@ -0,0 +1,340 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/errno.h>
> > +#include <linux/kvm_types.h>
> > +#include <linux/list.h>
> > +#include <linux/percpu.h>
> > +
> > +#include <asm/perf_event.h>
> > +#include <asm/processor.h>
> > +#include <asm/virt.h>
> > +#include <asm/vmx.h>
> > +
> > +static int x86_virt_feature __ro_after_init;
> > +
> > +__visible bool virt_rebooting;
> > +EXPORT_SYMBOL_GPL(virt_rebooting);
> > +
> > +static DEFINE_PER_CPU(int, virtualization_nr_users);
> > +
> > +static cpu_emergency_virt_cb __rcu *kvm_emergency_callback;
>
> Hmm, why kvm_ and not virt_?
I was trying to capture that this callback can _only_ be used by KVM, because
KVM is the only in-tree hypervisor. That's also why the exports are only for
KVM (and will use EXPORT_SYMBOL_FOR_KVM() when I post the next version).
> [..]
> > +#if IS_ENABLED(CONFIG_KVM_INTEL)
> > +static DEFINE_PER_CPU(struct vmcs *, root_vmcs);
>
> Perhaps introduce a CONFIG_INTEL_VMX for this? For example, KVM need not
> be enabled if all one wants to do is use TDX to setup PCIe Link
> Encryption. ...or were you expecting?
>
> #if IS_ENABLED(CONFIG_KVM_INTEL) || IS_ENABLED(...<other VMX users>...)
I don't think we need anything at this time. INTEL_TDX_HOST depends on KVM_INTEL,
and so without a user that needs VMXON without KVM_INTEL, I think we're good as-is.
config INTEL_TDX_HOST
bool "Intel Trust Domain Extensions (TDX) host support"
depends on CPU_SUP_INTEL
depends on X86_64
depends on KVM_INTEL
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox