* [PATCH 02/10] KVM/x86: Use bool for the err parameter of kvm_complete_insn_gp()
From: Juergen Gross @ 2025-12-05 7:45 UTC (permalink / raw)
To: linux-kernel, x86, kvm, linux-coco
Cc: Juergen Gross, Sean Christopherson, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe
In-Reply-To: <20251205074537.17072-1-jgross@suse.com>
The err parameter of kvm_complete_insn_gp() is just a flag, so bool
is the appropriate type for it.
Just converting the function itself for now is fine, as the implicit
conversion from int to bool is doing the right thing. Most callers
will be changed later to use bool, too.
As vmx_complete_emulated_msr is defined to kvm_complete_insn_gp, the
same parameter modification should be applied to
vt_complete_emulated_msr(), tdx_complete_emulated_msr() and
svm_complete_emulated_msr().
No change of functionality intended.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/include/asm/kvm_host.h | 4 ++--
arch/x86/kvm/svm/svm.c | 2 +-
arch/x86/kvm/vmx/main.c | 2 +-
arch/x86/kvm/vmx/tdx.c | 2 +-
arch/x86/kvm/vmx/tdx.h | 2 +-
arch/x86/kvm/x86.c | 2 +-
6 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 48598d017d6f..2b289708b56b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1930,7 +1930,7 @@ struct kvm_x86_ops {
void (*migrate_timers)(struct kvm_vcpu *vcpu);
void (*recalc_intercepts)(struct kvm_vcpu *vcpu);
- int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, int err);
+ int (*complete_emulated_msr)(struct kvm_vcpu *vcpu, bool err);
void (*vcpu_deliver_sipi_vector)(struct kvm_vcpu *vcpu, u8 vector);
@@ -2409,7 +2409,7 @@ bool kvm_arch_can_dequeue_async_page_present(struct kvm_vcpu *vcpu);
extern bool kvm_find_async_pf_gfn(struct kvm_vcpu *vcpu, gfn_t gfn);
int kvm_skip_emulated_instruction(struct kvm_vcpu *vcpu);
-int kvm_complete_insn_gp(struct kvm_vcpu *vcpu, int err);
+int kvm_complete_insn_gp(struct kvm_vcpu *vcpu, bool err);
void __user *__x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa,
u32 size);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9d29b2e7e855..f354cf0b6c1c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2777,7 +2777,7 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
return 0;
}
-static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
+static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, bool err)
{
struct vcpu_svm *svm = to_svm(vcpu);
if (!err || !sev_es_guest(vcpu->kvm) || WARN_ON_ONCE(!svm->sev_es.ghcb))
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index 0eb2773b2ae2..2f1b9a75fe47 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -202,7 +202,7 @@ static void vt_recalc_intercepts(struct kvm_vcpu *vcpu)
vmx_recalc_intercepts(vcpu);
}
-static int vt_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
+static int vt_complete_emulated_msr(struct kvm_vcpu *vcpu, bool err)
{
if (is_td_vcpu(vcpu))
return tdx_complete_emulated_msr(vcpu, err);
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 0a49c863c811..6b99c8dbd8cc 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -2028,7 +2028,7 @@ static int tdx_handle_ept_violation(struct kvm_vcpu *vcpu)
return ret;
}
-int tdx_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
+int tdx_complete_emulated_msr(struct kvm_vcpu *vcpu, bool err)
{
if (err) {
tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h
index ca39a9391db1..19a066868d45 100644
--- a/arch/x86/kvm/vmx/tdx.h
+++ b/arch/x86/kvm/vmx/tdx.h
@@ -174,7 +174,7 @@ static __always_inline void td_##lclass##_clearbit##bits(struct vcpu_tdx *tdx, \
bool tdx_interrupt_allowed(struct kvm_vcpu *vcpu);
-int tdx_complete_emulated_msr(struct kvm_vcpu *vcpu, int err);
+int tdx_complete_emulated_msr(struct kvm_vcpu *vcpu, bool err);
TDX_BUILD_TDVPS_ACCESSORS(16, VMCS, vmcs);
TDX_BUILD_TDVPS_ACCESSORS(32, VMCS, vmcs);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c9c2aa6f4705..f7c84d9ea9de 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -950,7 +950,7 @@ void kvm_requeue_exception(struct kvm_vcpu *vcpu, unsigned int nr,
}
EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_requeue_exception);
-int kvm_complete_insn_gp(struct kvm_vcpu *vcpu, int err)
+int kvm_complete_insn_gp(struct kvm_vcpu *vcpu, bool err)
{
if (err)
kvm_inject_gp(vcpu, 0);
--
2.51.0
^ permalink raw reply related
* [PATCH 00/10] KVM: Avoid literal numbers as return values
From: Juergen Gross @ 2025-12-05 7:45 UTC (permalink / raw)
To: linux-kernel, kvm, x86, linux-coco
Cc: Juergen Gross, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, Vitaly Kuznetsov,
David Woodhouse, Paul Durrant
This series is the first part of replacing the use of literal numbers
(0 and 1) as return values with either true/false or with defines.
This work is a prelude of getting rid of the magic value "1" for
"return to guest". I started in x86 KVM host code doing that and soon
stumbled over lots of other use cases of the magic "1" as return value,
especially in MSR emulation where a comment even implied this "1" was
due to the "return to guest" semantics.
A detailed analysis of all related code paths revealed that there was
indeed a rather clean interface between the functions using the MSR
emulation "1" and those using the "return to guest" "1".
A few functions just using "0" and "1" instead of bool are changed,
tooi (patches 1-4).
The rest of the series is cleaning up the MSR emulation code by using
new proper defines for return values 0 and 1.
The whole series should not result in any functional change.
Juergen Gross (10):
KVM: Switch coalesced_mmio_in_range() to return bool
KVM/x86: Use bool for the err parameter of kvm_complete_insn_gp()
KVM/x86: Let x86_emulate_ops.set_cr() return a bool
KVM/x86: Let x86_emulate_ops.set_dr() return a bool
KVM/x86: Add KVM_MSR_RET_* defines for values 0 and 1
KVM/x86: Use defines for APIC related MSR emulation
KVM/x86: Use defines for Hyper-V related MSR emulation
KVM/x86: Use defines for VMX related MSR emulation
KVM/x86: Use defines for SVM related MSR emulation
KVM/x86: Use defines for common related MSR emulation
arch/x86/include/asm/kvm_host.h | 14 +-
arch/x86/kvm/emulate.c | 2 +-
arch/x86/kvm/hyperv.c | 110 +++++++-------
arch/x86/kvm/kvm_emulate.h | 4 +-
arch/x86/kvm/lapic.c | 48 +++----
arch/x86/kvm/mtrr.c | 12 +-
arch/x86/kvm/pmu.c | 12 +-
arch/x86/kvm/smm.c | 2 +-
arch/x86/kvm/svm/pmu.c | 12 +-
arch/x86/kvm/svm/svm.c | 54 +++----
arch/x86/kvm/vmx/main.c | 2 +-
arch/x86/kvm/vmx/nested.c | 18 +--
arch/x86/kvm/vmx/pmu_intel.c | 20 +--
arch/x86/kvm/vmx/tdx.c | 18 +--
arch/x86/kvm/vmx/tdx.h | 2 +-
arch/x86/kvm/vmx/vmx.c | 122 ++++++++--------
arch/x86/kvm/x86.c | 246 ++++++++++++++++----------------
arch/x86/kvm/x86.h | 10 +-
arch/x86/kvm/xen.c | 14 +-
virt/kvm/coalesced_mmio.c | 14 +-
20 files changed, 372 insertions(+), 364 deletions(-)
--
2.51.0
^ permalink raw reply
* Re: [PATCH 1/3] KVM: guest_memfd: Remove preparation tracking
From: Yan Zhao @ 2025-12-05 3:54 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, seanjc, vbabka, ashish.kalra, liam.merwick, david,
vannapurve, ackerleytng, aik, ira.weiny
In-Reply-To: <20251203134717.475azsubs26pj2x4@amd.com>
On Wed, Dec 03, 2025 at 07:47:17AM -0600, Michael Roth wrote:
> On Tue, Dec 02, 2025 at 05:17:20PM +0800, Yan Zhao wrote:
> > On Mon, Dec 01, 2025 at 05:44:47PM -0600, Michael Roth wrote:
> > > On Tue, Nov 25, 2025 at 11:13:25AM +0800, Yan Zhao wrote:
> > > > On Fri, Nov 21, 2025 at 06:43:14AM -0600, Michael Roth wrote:
> > > > > On Thu, Nov 20, 2025 at 05:12:55PM +0800, Yan Zhao wrote:
> > > > > > On Thu, Nov 13, 2025 at 05:07:57PM -0600, Michael Roth wrote:
> > > > > > > @@ -797,19 +782,25 @@ 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)) {
> > > > > > > + unsigned long i, nr_pages = folio_nr_pages(folio);
> > > > > > > +
> > > > > > > + for (i = 0; i < nr_pages; i++)
> > > > > > > + clear_highpage(folio_page(folio, i));
> > > > > > > + folio_mark_uptodate(folio);
> > > > > > Here, the entire folio is cleared only when the folio is not marked uptodate.
> > > > > > Then, please check my questions at the bottom
> > > > >
> > > > > Yes, in this patch at least where I tried to mirror the current logic. I
> > > > > would not be surprised if we need to rework things for inplace/hugepage
> > > > > support though, but decoupling 'preparation' from the uptodate flag is
> > > > > the main goal here.
> > > > Could you elaborate a little why the decoupling is needed if it's not for
> > > > hugepage?
> > >
> > > For instance, for in-place conversion:
> > >
> > > 1. initial allocation: clear, set uptodate, fault in as private
> > > 2. private->shared: call invalidate hook, fault in as shared
> > > 3. shared->private: call prep hook, fault in as private
> > >
> > > Here, 2/3 need to track where the current state is shared/private in
> > > order to make appropriate architecture-specific changes (e.g. RMP table
> > > updates). But we want to allow for non-destructive in-place conversion,
> > > where a page is 'uptodate', but not in the desired shared/private state.
> > > So 'uptodate' becomes a separate piece of state, which is still
> > > reasonable for gmem to track in the current 4K-only implementation, and
> > > provides for a reasonable approach to upstreaming in-place conversion,
> > > which isn't far off for either SNP or TDX.
> > To me, "1. initial allocation: clear, set uptodate" is more appropriate to
> > be done in kvm_gmem_get_folio(), instead of in kvm_gmem_get_pfn().
>
> The downside is that preallocating originally involved just
> preallocating, and zero'ing would happen lazily during fault time. (or
> upfront via KVM_PRE_FAULT_MEMORY).
>
> But in the context of the hugetlb RFC, it certainly looks cleaner to do
> it there, since it could be done before any potential splitting activity,
> so then all the tail pages can inherit that initial uptodate flag.
>
> We'd probably want to weigh that these trade-offs based on how it will
> affect hugepages, but that would be clearer in the context of a new
> posting of hugepage support on top of these changes. So I think it's
> better to address that change as a follow-up so we can consider it with
> more context.
>
> >
> > With it, below looks reasonable to me.
> > > For hugepages, we'll have other things to consider, but those things are
> > > probably still somewhat far off, and so we shouldn't block steps toward
> > > in-place conversion based on uncertainty around hugepages. I think it's
> > > gotten enough attention at least that we know it *can* work, e.g. even
> > > if we take the inefficient/easy route of zero'ing the whole folio on
> > > initial access, setting it uptodate, and never doing anything with
> > > uptodate again, it's still a usable implementation.
>
> To me as well. But in the context of this series, against kvm/next, it
> creates userspace-visible changes to pre-allocation behavior that we
> can't justify in the context of this series alone, so as above I think
> that's something to save for hugepage follow-up.
>
> FWIW though, I ended up taking this approach for the hugetlb-based test
> branch, to address the fact (after you reminded me) that I wasn't fully
> zero'ing folio's in the kvm_gmem_populate() path:
>
> https://github.com/AMDESE/linux/commit/253fb30b076d81232deba0b02db070d5bc2b90b5
>
> So maybe for your testing you could do similar. For newer hugepage
> support I'll likely do similar, but I don't think any of this reasoning
> or changes makes sense to people reviewing this series without already
> being aware of hugepage plans/development, so that's why I'm trying to
> keep this series more focused on in-place conversion enablement, because
> hugepage plans might be massively reworked for next posting based on LPC
> talks and changes in direction mentioned in recent guest_memfd calls and
> we are basically just hand-waving about what it will look like at this
> point while blocking other efforts.
>
Got it. Thanks for the explanation!
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Yan Zhao @ 2025-12-05 3:38 UTC (permalink / raw)
To: Michael Roth
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
thomas.lendacky@amd.com, pbonzini@redhat.com, seanjc@google.com,
vbabka@suse.cz, ashish.kalra@amd.com, liam.merwick@oracle.com,
david@redhat.com, Annapurve, Vishal, ackerleytng@google.com,
aik@amd.com, Weiny, Ira
In-Reply-To: <20251203142648.trx6sslxvxr26yzd@amd.com>
On Wed, Dec 03, 2025 at 10:26:48PM +0800, Michael Roth wrote:
> Look at your
> changelog for the change above, for instance: it has no relevance in the
> context of this series. What do I put in its place? Bug reports about
> my experimental tree? It's just not the right place to try to justify
> these changes.
The following diff is reasonable to this series(if npages is up to 2MB),
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -878,11 +878,10 @@ 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),
+ while (!IS_ALIGNED(gfn, 1 << max_order) || (npages - i) < (1 << max_order) ||
+ !kvm_range_has_memory_attributes(kvm, gfn, gfn + (1 << max_order),
KVM_MEMORY_ATTRIBUTE_PRIVATE,
KVM_MEMORY_ATTRIBUTE_PRIVATE)) {
if (!max_order)
because:
1. kmalloc_array() + GUP 2MB src pages + returning -ENOMEM in "Hunk 1" is a
waste if max_order is always 0.
2. If we allow max_order > 0, then we must remove the WARN_ON().
3. When start_gfn is not 2MB aligned, just allocating 4KB src page each round is
enough (as in Sean's sketch patch).
Hunk 1: -------------------------------------------------------------------
src_npages = IS_ALIGNED((unsigned long)src, PAGE_SIZE) ? npages : npages + 1;
src_pages = kmalloc_array(src_npages, sizeof(struct page *), GFP_KERNEL);
if (!src_pages)
return -ENOMEM;
ret = get_user_pages_fast((unsigned long)src, src_npages, 0, src_pages);
if (ret < 0)
return ret;
if (ret != src_npages)
return -ENOMEM;
Hunk 2: -------------------------------------------------------------------
for (i = 0; i < npages; i += (1 << max_order)) {
...
folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
(npages - i) < (1 << max_order));
ret = post_populate(kvm, gfn, pfn, src ? &src_pages[i] : NULL,
src_offset, max_order, opaque);
...
}
^ permalink raw reply
* Re: [PATCH] virt: Fix Kconfig warning when selecting TSM without VIRT_DRIVERS
From: dan.j.williams @ 2025-12-05 1:33 UTC (permalink / raw)
To: Nathan Chancellor, Dan Williams
Cc: Jonathan Cameron, linux-pci, linux-coco, linux-kernel,
kernel test robot, Nathan Chancellor
In-Reply-To: <20251203-fix-pci-tsm-select-tsm-warning-v1-1-c3959c1cb110@kernel.org>
Nathan Chancellor wrote:
> After commit 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link
> Encryption"), there is a Kconfig warning when selecting CONFIG_TSM
> without CONFIG_VIRT_DRIVERS:
>
> WARNING: unmet direct dependencies detected for TSM
> Depends on [n]: VIRT_DRIVERS [=n]
> Selected by [y]:
> - PCI_TSM [=y] && PCI [=y]
>
> CONFIG_TSM is defined in drivers/virt/coco/Kconfig but this Kconfig is
> only sourced when CONFIG_VIRT_DRIVERS is enabled. Since this symbol is
> hidden with no dependencies, it should be available without a symbol
> that just enables a menu.
>
> Move the sourcing of drivers/virt/coco/Kconfig outside of
> CONFIG_VIRT_DRIVERS and wrap the other source statements in
> drivers/virt/coco/Kconfig with CONFIG_VIRT_DRIVERS to ensure users do
> not get any additional prompts while ensuring CONFIG_TSM is always
> available to select. This complements commit 110c155e8a68 ("drivers/virt:
> Drop VIRT_DRIVERS build dependency"), which addressed the build issue
> that this Kconfig warning was pointing out.
>
> Fixes: 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link Encryption")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202511140712.NubhamPy-lkp@intel.com/
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
This looks good to me Nathan, thanks. I will include it for the v6.19 update.
^ permalink raw reply
* Re: [PATCH v5] virt: tdx-guest: Handle GetQuote request error code
From: Sathyanarayanan Kuppuswamy @ 2025-12-05 0:20 UTC (permalink / raw)
To: Dave Hansen, Kirill A . Shutemov
Cc: Rick Edgecombe, Dave Hansen, Dan Williams, x86, linux-kernel,
linux-coco
In-Reply-To: <baf27817-e8db-4521-872a-1f1d32cf0a7e@intel.com>
Hi Dave,
On 12/3/25 10:16 AM, Dave Hansen wrote:
> On 12/3/25 10:04, Sathyanarayanan Kuppuswamy wrote:
>> Got it. How about the following version?
> Still way too wordy and flowery for my taste.
>
>
Thanks for the feedback. I have trimmed the changelog to a
strict problem and solution format and removed the narrative
wording.
virt: tdx-guest: Return explicit errors for GetQuote failures
Some VMM reported GetQuote failures are currently returned to userspace
as success with a zero length Quote, which makes failure detection
ambiguous.
The VMM failure status is reported in the shared GPA header and is not
visible to userspace. Parse the status in the kernel and return
standard error codes for these failures.
This preserves existing ABI behavior. Userspace already treats a zero
length Quote as failure. It now also receives explicit error codes.
Refer to the GHCI specification v1.5 March 2023, sec titled
TDG.VP.VMCALL<GetQuote>, Table 3-10 and Table 3-11 for details on the
Quote header and status codes.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
^ permalink raw reply
* CCC Kernel Sig Thursday night (PST) calls cancelled through New Years
From: dan.j.williams @ 2025-12-04 21:42 UTC (permalink / raw)
To: linux-coco; +Cc: srutherford
FYI, the Confidential Computing Consortium meeting titled "Linux Kernel
SIG IOMMUFD Meeting" is cancelled through New Years. We have Plumbers
next week to have productive discussions on next steps, then expect most
folks to disappear for the holidays.
See you at Plumbers next week, either in person or remote!
^ permalink raw reply
* Re: [PATCH 0/3] x86/sev: Carve out the SVSM support code
From: Borislav Petkov @ 2025-12-04 15:06 UTC (permalink / raw)
To: Tom Lendacky; +Cc: Borislav Petkov, linux-coco, X86 ML, LKML
In-Reply-To: <e4571015-5b76-426f-9e12-0181df9ab1f7@amd.com>
On Thu, Dec 04, 2025 at 08:53:01AM -0600, Tom Lendacky wrote:
> Quick testing doesn't show any issues.
>
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Much appreciated, thanks!
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 1/3] x86/sev: Move the internal header
From: Borislav Petkov @ 2025-12-04 14:57 UTC (permalink / raw)
To: Tom Lendacky; +Cc: Borislav Petkov, linux-coco, X86 ML, LKML
In-Reply-To: <740dd1aa-a1b8-46d5-8b9a-ce03bed50dc0@amd.com>
On Thu, Dec 04, 2025 at 08:18:42AM -0600, Tom Lendacky wrote:
> > +#include "../coco/sev/internal.h"
>
> Shouldn't this be "../../coco/sev/internal.h" ?
>
> What is strange is that it works with either.
Pure luck :-\
gcc cmdline is:
gcc -Wp,-MMD,arch/x86/boot/startup/.sev-startup.o.d -nostdinc
-I./arch/x86/include...
so the include path becomes:
# 1 "./arch/x86/include/../coco/sev/internal.h" 1
which is
$ readlink -f arch/x86/include/../coco/sev/internal.h
/mnt/kernel/kernel/linux/arch/x86/coco/sev/internal.h
And /mnt/kernel is my kernel-sources-containing SSD :-)
In any case, good catch!
I'll do the "../../" so that it is perfectly clear and future
potential include directives changes do not break this.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 0/3] x86/sev: Carve out the SVSM support code
From: Tom Lendacky @ 2025-12-04 14:53 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
In-Reply-To: <20251204124809.31783-1-bp@kernel.org>
On 12/4/25 06:48, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
>
> Hi,
>
> I've been meaning to do this for a while now but didn't have a good idea how
> to do it nicely. Using the internal header makes it almost trivial.
Quick testing doesn't show any issues.
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
>
> Thx.
>
> Borislav Petkov (AMD) (3):
> x86/sev: Move the internal header
> x86/sev: Add internal header guards
> x86/sev: Carve out the SVSM code into a separate compilation unit
>
> arch/x86/boot/startup/sev-startup.c | 3 +-
> arch/x86/coco/sev/Makefile | 2 +-
> arch/x86/coco/sev/core.c | 380 +-----------------
> .../sev-internal.h => coco/sev/internal.h} | 32 ++
> arch/x86/coco/sev/noinstr.c | 3 +-
> arch/x86/coco/sev/svsm.c | 362 +++++++++++++++++
> arch/x86/coco/sev/vc-handle.c | 3 +-
> 7 files changed, 403 insertions(+), 382 deletions(-)
> rename arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} (75%)
> create mode 100644 arch/x86/coco/sev/svsm.c
>
^ permalink raw reply
* Re: [PATCH 1/3] x86/sev: Move the internal header
From: Tom Lendacky @ 2025-12-04 14:18 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
In-Reply-To: <20251204124809.31783-2-bp@kernel.org>
On 12/4/25 06:48, Borislav Petkov wrote:
> From: "Borislav Petkov (AMD)" <bp@alien8.de>
>
> Move the internal header out of the usual include/asm/ include path
> because having an "internal" header there doesn't really make it so
> - quite the opposite.
>
> So move where it belongs and make it really internal.
>
> No functional changes.
>
> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
> ---
> arch/x86/boot/startup/sev-startup.c | 3 ++-
> arch/x86/coco/sev/core.c | 3 ++-
> arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} | 0
> arch/x86/coco/sev/noinstr.c | 3 ++-
> arch/x86/coco/sev/vc-handle.c | 3 ++-
> 5 files changed, 8 insertions(+), 4 deletions(-)
> rename arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} (100%)
>
> diff --git a/arch/x86/boot/startup/sev-startup.c b/arch/x86/boot/startup/sev-startup.c
> index 09725428d3e6..1115214429fd 100644
> --- a/arch/x86/boot/startup/sev-startup.c
> +++ b/arch/x86/boot/startup/sev-startup.c
> @@ -27,7 +27,6 @@
> #include <asm/cpu_entry_area.h>
> #include <asm/stacktrace.h>
> #include <asm/sev.h>
> -#include <asm/sev-internal.h>
> #include <asm/insn-eval.h>
> #include <asm/fpu/xcr.h>
> #include <asm/processor.h>
> @@ -41,6 +40,8 @@
> #include <asm/cpuid/api.h>
> #include <asm/cmdline.h>
>
> +#include "../coco/sev/internal.h"
Shouldn't this be "../../coco/sev/internal.h" ?
What is strange is that it works with either.
Thanks,
Tom
> +
> /* Include code shared with pre-decompression boot stage */
> #include "sev-shared.c"
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index 9ae3b11754e6..4e618e596267 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -31,7 +31,6 @@
> #include <asm/cpu_entry_area.h>
> #include <asm/stacktrace.h>
> #include <asm/sev.h>
> -#include <asm/sev-internal.h>
> #include <asm/insn-eval.h>
> #include <asm/fpu/xcr.h>
> #include <asm/processor.h>
> @@ -46,6 +45,8 @@
> #include <asm/cmdline.h>
> #include <asm/msr.h>
>
> +#include "internal.h"
> +
> /* Bitmap of SEV features supported by the hypervisor */
> u64 sev_hv_features __ro_after_init;
> SYM_PIC_ALIAS(sev_hv_features);
> diff --git a/arch/x86/include/asm/sev-internal.h b/arch/x86/coco/sev/internal.h
> similarity index 100%
> rename from arch/x86/include/asm/sev-internal.h
> rename to arch/x86/coco/sev/internal.h
> diff --git a/arch/x86/coco/sev/noinstr.c b/arch/x86/coco/sev/noinstr.c
> index b527eafb6312..9d94aca4a698 100644
> --- a/arch/x86/coco/sev/noinstr.c
> +++ b/arch/x86/coco/sev/noinstr.c
> @@ -16,7 +16,8 @@
> #include <asm/msr.h>
> #include <asm/ptrace.h>
> #include <asm/sev.h>
> -#include <asm/sev-internal.h>
> +
> +#include "internal.h"
>
> static __always_inline bool on_vc_stack(struct pt_regs *regs)
> {
> diff --git a/arch/x86/coco/sev/vc-handle.c b/arch/x86/coco/sev/vc-handle.c
> index f08c7505ed82..43f264afd590 100644
> --- a/arch/x86/coco/sev/vc-handle.c
> +++ b/arch/x86/coco/sev/vc-handle.c
> @@ -23,7 +23,6 @@
> #include <asm/init.h>
> #include <asm/stacktrace.h>
> #include <asm/sev.h>
> -#include <asm/sev-internal.h>
> #include <asm/insn-eval.h>
> #include <asm/fpu/xcr.h>
> #include <asm/processor.h>
> @@ -35,6 +34,8 @@
> #include <asm/apic.h>
> #include <asm/cpuid/api.h>
>
> +#include "internal.h"
> +
> static enum es_result vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
> unsigned long vaddr, phys_addr_t *paddr)
> {
^ permalink raw reply
* [PATCH 3/3] x86/sev: Carve out the SVSM code into a separate compilation unit
From: Borislav Petkov @ 2025-12-04 12:48 UTC (permalink / raw)
To: Tom Lendacky; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
In-Reply-To: <20251204124809.31783-1-bp@kernel.org>
From: "Borislav Petkov (AMD)" <bp@alien8.de>
Move the SVSM-related machinery into a separate compilation unit in
order to keep sev/core.c slim and "on-topic".
No functional changes.
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
arch/x86/coco/sev/Makefile | 2 +-
arch/x86/coco/sev/core.c | 377 -----------------------------------
arch/x86/coco/sev/internal.h | 29 +++
arch/x86/coco/sev/svsm.c | 362 +++++++++++++++++++++++++++++++++
4 files changed, 392 insertions(+), 378 deletions(-)
create mode 100644 arch/x86/coco/sev/svsm.c
diff --git a/arch/x86/coco/sev/Makefile b/arch/x86/coco/sev/Makefile
index 3b8ae214a6a6..fb8ffedfc8b0 100644
--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
-obj-y += core.o noinstr.o vc-handle.o
+obj-y += core.o noinstr.o vc-handle.o svsm.o
# Clang 14 and older may fail to respect __no_sanitize_undefined when inlining
UBSAN_SANITIZE_noinstr.o := n
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 4e618e596267..379e0c09c7f3 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -55,40 +55,6 @@ SYM_PIC_ALIAS(sev_hv_features);
u64 sev_secrets_pa __ro_after_init;
SYM_PIC_ALIAS(sev_secrets_pa);
-/* For early boot SVSM communication */
-struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
-SYM_PIC_ALIAS(boot_svsm_ca_page);
-
-/*
- * SVSM related information:
- * During boot, the page tables are set up as identity mapped and later
- * changed to use kernel virtual addresses. Maintain separate virtual and
- * physical addresses for the CAA to allow SVSM functions to be used during
- * early boot, both with identity mapped virtual addresses and proper kernel
- * virtual addresses.
- */
-u64 boot_svsm_caa_pa __ro_after_init;
-SYM_PIC_ALIAS(boot_svsm_caa_pa);
-
-DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
-DEFINE_PER_CPU(u64, svsm_caa_pa);
-
-static inline struct svsm_ca *svsm_get_caa(void)
-{
- if (sev_cfg.use_cas)
- return this_cpu_read(svsm_caa);
- else
- return rip_rel_ptr(&boot_svsm_ca_page);
-}
-
-static inline u64 svsm_get_caa_pa(void)
-{
- if (sev_cfg.use_cas)
- return this_cpu_read(svsm_caa_pa);
- else
- return boot_svsm_caa_pa;
-}
-
/* AP INIT values as documented in the APM2 section "Processor Initialization State" */
#define AP_INIT_CS_LIMIT 0xffff
#define AP_INIT_DS_LIMIT 0xffff
@@ -218,95 +184,6 @@ static u64 __init get_jump_table_addr(void)
return ret;
}
-static int svsm_perform_ghcb_protocol(struct ghcb *ghcb, struct svsm_call *call)
-{
- struct es_em_ctxt ctxt;
- u8 pending = 0;
-
- vc_ghcb_invalidate(ghcb);
-
- /*
- * Fill in protocol and format specifiers. This can be called very early
- * in the boot, so use rip-relative references as needed.
- */
- ghcb->protocol_version = ghcb_version;
- ghcb->ghcb_usage = GHCB_DEFAULT_USAGE;
-
- ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_SNP_RUN_VMPL);
- ghcb_set_sw_exit_info_1(ghcb, 0);
- ghcb_set_sw_exit_info_2(ghcb, 0);
-
- sev_es_wr_ghcb_msr(__pa(ghcb));
-
- svsm_issue_call(call, &pending);
-
- if (pending)
- return -EINVAL;
-
- switch (verify_exception_info(ghcb, &ctxt)) {
- case ES_OK:
- break;
- case ES_EXCEPTION:
- vc_forward_exception(&ctxt);
- fallthrough;
- default:
- return -EINVAL;
- }
-
- return svsm_process_result_codes(call);
-}
-
-static int svsm_perform_call_protocol(struct svsm_call *call)
-{
- struct ghcb_state state;
- unsigned long flags;
- struct ghcb *ghcb;
- int ret;
-
- flags = native_local_irq_save();
-
- if (sev_cfg.ghcbs_initialized)
- ghcb = __sev_get_ghcb(&state);
- else if (boot_ghcb)
- ghcb = boot_ghcb;
- else
- ghcb = NULL;
-
- do {
- ret = ghcb ? svsm_perform_ghcb_protocol(ghcb, call)
- : __pi_svsm_perform_msr_protocol(call);
- } while (ret == -EAGAIN);
-
- if (sev_cfg.ghcbs_initialized)
- __sev_put_ghcb(&state);
-
- native_local_irq_restore(flags);
-
- return ret;
-}
-
-static inline void __pval_terminate(u64 pfn, bool action, unsigned int page_size,
- int ret, u64 svsm_ret)
-{
- WARN(1, "PVALIDATE failure: pfn: 0x%llx, action: %u, size: %u, ret: %d, svsm_ret: 0x%llx\n",
- pfn, action, page_size, ret, svsm_ret);
-
- sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
-}
-
-static void svsm_pval_terminate(struct svsm_pvalidate_call *pc, int ret, u64 svsm_ret)
-{
- unsigned int page_size;
- bool action;
- u64 pfn;
-
- pfn = pc->entry[pc->cur_index].pfn;
- action = pc->entry[pc->cur_index].action;
- page_size = pc->entry[pc->cur_index].page_size;
-
- __pval_terminate(pfn, action, page_size, ret, svsm_ret);
-}
-
static void pval_pages(struct snp_psc_desc *desc)
{
struct psc_entry *e;
@@ -343,152 +220,6 @@ static void pval_pages(struct snp_psc_desc *desc)
}
}
-static u64 svsm_build_ca_from_pfn_range(u64 pfn, u64 pfn_end, bool action,
- struct svsm_pvalidate_call *pc)
-{
- struct svsm_pvalidate_entry *pe;
-
- /* Nothing in the CA yet */
- pc->num_entries = 0;
- pc->cur_index = 0;
-
- pe = &pc->entry[0];
-
- while (pfn < pfn_end) {
- pe->page_size = RMP_PG_SIZE_4K;
- pe->action = action;
- pe->ignore_cf = 0;
- pe->rsvd = 0;
- pe->pfn = pfn;
-
- pe++;
- pfn++;
-
- pc->num_entries++;
- if (pc->num_entries == SVSM_PVALIDATE_MAX_COUNT)
- break;
- }
-
- return pfn;
-}
-
-static int svsm_build_ca_from_psc_desc(struct snp_psc_desc *desc, unsigned int desc_entry,
- struct svsm_pvalidate_call *pc)
-{
- struct svsm_pvalidate_entry *pe;
- struct psc_entry *e;
-
- /* Nothing in the CA yet */
- pc->num_entries = 0;
- pc->cur_index = 0;
-
- pe = &pc->entry[0];
- e = &desc->entries[desc_entry];
-
- while (desc_entry <= desc->hdr.end_entry) {
- pe->page_size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
- pe->action = e->operation == SNP_PAGE_STATE_PRIVATE;
- pe->ignore_cf = 0;
- pe->rsvd = 0;
- pe->pfn = e->gfn;
-
- pe++;
- e++;
-
- desc_entry++;
- pc->num_entries++;
- if (pc->num_entries == SVSM_PVALIDATE_MAX_COUNT)
- break;
- }
-
- return desc_entry;
-}
-
-static void svsm_pval_pages(struct snp_psc_desc *desc)
-{
- struct svsm_pvalidate_entry pv_4k[VMGEXIT_PSC_MAX_ENTRY];
- unsigned int i, pv_4k_count = 0;
- struct svsm_pvalidate_call *pc;
- struct svsm_call call = {};
- unsigned long flags;
- bool action;
- u64 pc_pa;
- int ret;
-
- /*
- * This can be called very early in the boot, use native functions in
- * order to avoid paravirt issues.
- */
- flags = native_local_irq_save();
-
- /*
- * The SVSM calling area (CA) can support processing 510 entries at a
- * time. Loop through the Page State Change descriptor until the CA is
- * full or the last entry in the descriptor is reached, at which time
- * the SVSM is invoked. This repeats until all entries in the descriptor
- * are processed.
- */
- call.caa = svsm_get_caa();
-
- pc = (struct svsm_pvalidate_call *)call.caa->svsm_buffer;
- pc_pa = svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
-
- /* Protocol 0, Call ID 1 */
- call.rax = SVSM_CORE_CALL(SVSM_CORE_PVALIDATE);
- call.rcx = pc_pa;
-
- for (i = 0; i <= desc->hdr.end_entry;) {
- i = svsm_build_ca_from_psc_desc(desc, i, pc);
-
- do {
- ret = svsm_perform_call_protocol(&call);
- if (!ret)
- continue;
-
- /*
- * Check if the entry failed because of an RMP mismatch (a
- * PVALIDATE at 2M was requested, but the page is mapped in
- * the RMP as 4K).
- */
-
- if (call.rax_out == SVSM_PVALIDATE_FAIL_SIZEMISMATCH &&
- pc->entry[pc->cur_index].page_size == RMP_PG_SIZE_2M) {
- /* Save this entry for post-processing at 4K */
- pv_4k[pv_4k_count++] = pc->entry[pc->cur_index];
-
- /* Skip to the next one unless at the end of the list */
- pc->cur_index++;
- if (pc->cur_index < pc->num_entries)
- ret = -EAGAIN;
- else
- ret = 0;
- }
- } while (ret == -EAGAIN);
-
- if (ret)
- svsm_pval_terminate(pc, ret, call.rax_out);
- }
-
- /* Process any entries that failed to be validated at 2M and validate them at 4K */
- for (i = 0; i < pv_4k_count; i++) {
- u64 pfn, pfn_end;
-
- action = pv_4k[i].action;
- pfn = pv_4k[i].pfn;
- pfn_end = pfn + 512;
-
- while (pfn < pfn_end) {
- pfn = svsm_build_ca_from_pfn_range(pfn, pfn_end, action, pc);
-
- ret = svsm_perform_call_protocol(&call);
- if (ret)
- svsm_pval_terminate(pc, ret, call.rax_out);
- }
- }
-
- native_local_irq_restore(flags);
-}
-
static void pvalidate_pages(struct snp_psc_desc *desc)
{
struct psc_entry *e;
@@ -1589,56 +1320,6 @@ static int __init report_snp_info(void)
}
arch_initcall(report_snp_info);
-static void update_attest_input(struct svsm_call *call, struct svsm_attest_call *input)
-{
- /* If (new) lengths have been returned, propagate them up */
- if (call->rcx_out != call->rcx)
- input->manifest_buf.len = call->rcx_out;
-
- if (call->rdx_out != call->rdx)
- input->certificates_buf.len = call->rdx_out;
-
- if (call->r8_out != call->r8)
- input->report_buf.len = call->r8_out;
-}
-
-int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
- struct svsm_attest_call *input)
-{
- struct svsm_attest_call *ac;
- unsigned long flags;
- u64 attest_call_pa;
- int ret;
-
- if (!snp_vmpl)
- return -EINVAL;
-
- local_irq_save(flags);
-
- call->caa = svsm_get_caa();
-
- ac = (struct svsm_attest_call *)call->caa->svsm_buffer;
- attest_call_pa = svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
-
- *ac = *input;
-
- /*
- * Set input registers for the request and set RDX and R8 to known
- * values in order to detect length values being returned in them.
- */
- call->rax = call_id;
- call->rcx = attest_call_pa;
- call->rdx = -1;
- call->r8 = -1;
- ret = svsm_perform_call_protocol(call);
- update_attest_input(call, input);
-
- local_irq_restore(flags);
-
- return ret;
-}
-EXPORT_SYMBOL_GPL(snp_issue_svsm_attest_req);
-
static int snp_issue_guest_request(struct snp_guest_req *req)
{
struct snp_req_data *input = &req->input;
@@ -1703,64 +1384,6 @@ static int snp_issue_guest_request(struct snp_guest_req *req)
return ret;
}
-/**
- * snp_svsm_vtpm_probe() - Probe if SVSM provides a vTPM device
- *
- * Check that there is SVSM and that it supports at least TPM_SEND_COMMAND
- * which is the only request used so far.
- *
- * Return: true if the platform provides a vTPM SVSM device, false otherwise.
- */
-static bool snp_svsm_vtpm_probe(void)
-{
- struct svsm_call call = {};
-
- /* The vTPM device is available only if a SVSM is present */
- if (!snp_vmpl)
- return false;
-
- call.caa = svsm_get_caa();
- call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
-
- if (svsm_perform_call_protocol(&call))
- return false;
-
- /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
- return call.rcx_out & BIT_ULL(8);
-}
-
-/**
- * snp_svsm_vtpm_send_command() - Execute a vTPM operation on SVSM
- * @buffer: A buffer used to both send the command and receive the response.
- *
- * Execute a SVSM_VTPM_CMD call as defined by
- * "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00
- *
- * All command request/response buffers have a common structure as specified by
- * the following table:
- * Byte Size In/Out Description
- * Offset (Bytes)
- * 0x000 4 In Platform command
- * Out Platform command response size
- *
- * Each command can build upon this common request/response structure to create
- * a structure specific to the command. See include/linux/tpm_svsm.h for more
- * details.
- *
- * Return: 0 on success, -errno on failure
- */
-int snp_svsm_vtpm_send_command(u8 *buffer)
-{
- struct svsm_call call = {};
-
- call.caa = svsm_get_caa();
- call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
- call.rcx = __pa(buffer);
-
- return svsm_perform_call_protocol(&call);
-}
-EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
-
static struct platform_device sev_guest_device = {
.name = "sev-guest",
.id = -1,
diff --git a/arch/x86/coco/sev/internal.h b/arch/x86/coco/sev/internal.h
index af991f1da095..039326b5c799 100644
--- a/arch/x86/coco/sev/internal.h
+++ b/arch/x86/coco/sev/internal.h
@@ -66,6 +66,9 @@ extern u64 boot_svsm_caa_pa;
enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt *ctxt);
void vc_forward_exception(struct es_em_ctxt *ctxt);
+void svsm_pval_pages(struct snp_psc_desc *desc);
+int svsm_perform_call_protocol(struct svsm_call *call);
+bool snp_svsm_vtpm_probe(void);
static inline u64 sev_es_rd_ghcb_msr(void)
{
@@ -87,4 +90,30 @@ enum es_result sev_es_ghcb_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt
u64 get_hv_features(void);
const struct snp_cpuid_table *snp_cpuid_get_table(void);
+
+static inline struct svsm_ca *svsm_get_caa(void)
+{
+ if (sev_cfg.use_cas)
+ return this_cpu_read(svsm_caa);
+ else
+ return rip_rel_ptr(&boot_svsm_ca_page);
+}
+
+static inline u64 svsm_get_caa_pa(void)
+{
+ if (sev_cfg.use_cas)
+ return this_cpu_read(svsm_caa_pa);
+ else
+ return boot_svsm_caa_pa;
+}
+
+static inline void __pval_terminate(u64 pfn, bool action, unsigned int page_size,
+ int ret, u64 svsm_ret)
+{
+ WARN(1, "PVALIDATE failure: pfn: 0x%llx, action: %u, size: %u, ret: %d, svsm_ret: 0x%llx\n",
+ pfn, action, page_size, ret, svsm_ret);
+
+ sev_es_terminate(SEV_TERM_SET_LINUX, GHCB_TERM_PVALIDATE);
+}
+
#endif /* __X86_COCO_SEV_INTERNAL_H__ */
diff --git a/arch/x86/coco/sev/svsm.c b/arch/x86/coco/sev/svsm.c
new file mode 100644
index 000000000000..2acf4a76afe7
--- /dev/null
+++ b/arch/x86/coco/sev/svsm.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SVSM support code
+ */
+
+#include <linux/types.h>
+
+#include <asm/sev.h>
+
+#include "internal.h"
+
+/* For early boot SVSM communication */
+struct svsm_ca boot_svsm_ca_page __aligned(PAGE_SIZE);
+SYM_PIC_ALIAS(boot_svsm_ca_page);
+
+/*
+ * SVSM related information:
+ * During boot, the page tables are set up as identity mapped and later
+ * changed to use kernel virtual addresses. Maintain separate virtual and
+ * physical addresses for the CAA to allow SVSM functions to be used during
+ * early boot, both with identity mapped virtual addresses and proper kernel
+ * virtual addresses.
+ */
+u64 boot_svsm_caa_pa __ro_after_init;
+SYM_PIC_ALIAS(boot_svsm_caa_pa);
+
+DEFINE_PER_CPU(struct svsm_ca *, svsm_caa);
+DEFINE_PER_CPU(u64, svsm_caa_pa);
+
+static int svsm_perform_ghcb_protocol(struct ghcb *ghcb, struct svsm_call *call)
+{
+ struct es_em_ctxt ctxt;
+ u8 pending = 0;
+
+ vc_ghcb_invalidate(ghcb);
+
+ /*
+ * Fill in protocol and format specifiers. This can be called very early
+ * in the boot, so use rip-relative references as needed.
+ */
+ ghcb->protocol_version = ghcb_version;
+ ghcb->ghcb_usage = GHCB_DEFAULT_USAGE;
+
+ ghcb_set_sw_exit_code(ghcb, SVM_VMGEXIT_SNP_RUN_VMPL);
+ ghcb_set_sw_exit_info_1(ghcb, 0);
+ ghcb_set_sw_exit_info_2(ghcb, 0);
+
+ sev_es_wr_ghcb_msr(__pa(ghcb));
+
+ svsm_issue_call(call, &pending);
+
+ if (pending)
+ return -EINVAL;
+
+ switch (verify_exception_info(ghcb, &ctxt)) {
+ case ES_OK:
+ break;
+ case ES_EXCEPTION:
+ vc_forward_exception(&ctxt);
+ fallthrough;
+ default:
+ return -EINVAL;
+ }
+
+ return svsm_process_result_codes(call);
+}
+
+int svsm_perform_call_protocol(struct svsm_call *call)
+{
+ struct ghcb_state state;
+ unsigned long flags;
+ struct ghcb *ghcb;
+ int ret;
+
+ flags = native_local_irq_save();
+
+ if (sev_cfg.ghcbs_initialized)
+ ghcb = __sev_get_ghcb(&state);
+ else if (boot_ghcb)
+ ghcb = boot_ghcb;
+ else
+ ghcb = NULL;
+
+ do {
+ ret = ghcb ? svsm_perform_ghcb_protocol(ghcb, call)
+ : __pi_svsm_perform_msr_protocol(call);
+ } while (ret == -EAGAIN);
+
+ if (sev_cfg.ghcbs_initialized)
+ __sev_put_ghcb(&state);
+
+ native_local_irq_restore(flags);
+
+ return ret;
+}
+
+static u64 svsm_build_ca_from_pfn_range(u64 pfn, u64 pfn_end, bool action,
+ struct svsm_pvalidate_call *pc)
+{
+ struct svsm_pvalidate_entry *pe;
+
+ /* Nothing in the CA yet */
+ pc->num_entries = 0;
+ pc->cur_index = 0;
+
+ pe = &pc->entry[0];
+
+ while (pfn < pfn_end) {
+ pe->page_size = RMP_PG_SIZE_4K;
+ pe->action = action;
+ pe->ignore_cf = 0;
+ pe->rsvd = 0;
+ pe->pfn = pfn;
+
+ pe++;
+ pfn++;
+
+ pc->num_entries++;
+ if (pc->num_entries == SVSM_PVALIDATE_MAX_COUNT)
+ break;
+ }
+
+ return pfn;
+}
+
+static int svsm_build_ca_from_psc_desc(struct snp_psc_desc *desc, unsigned int desc_entry,
+ struct svsm_pvalidate_call *pc)
+{
+ struct svsm_pvalidate_entry *pe;
+ struct psc_entry *e;
+
+ /* Nothing in the CA yet */
+ pc->num_entries = 0;
+ pc->cur_index = 0;
+
+ pe = &pc->entry[0];
+ e = &desc->entries[desc_entry];
+
+ while (desc_entry <= desc->hdr.end_entry) {
+ pe->page_size = e->pagesize ? RMP_PG_SIZE_2M : RMP_PG_SIZE_4K;
+ pe->action = e->operation == SNP_PAGE_STATE_PRIVATE;
+ pe->ignore_cf = 0;
+ pe->rsvd = 0;
+ pe->pfn = e->gfn;
+
+ pe++;
+ e++;
+
+ desc_entry++;
+ pc->num_entries++;
+ if (pc->num_entries == SVSM_PVALIDATE_MAX_COUNT)
+ break;
+ }
+
+ return desc_entry;
+}
+
+static void svsm_pval_terminate(struct svsm_pvalidate_call *pc, int ret, u64 svsm_ret)
+{
+ unsigned int page_size;
+ bool action;
+ u64 pfn;
+
+ pfn = pc->entry[pc->cur_index].pfn;
+ action = pc->entry[pc->cur_index].action;
+ page_size = pc->entry[pc->cur_index].page_size;
+
+ __pval_terminate(pfn, action, page_size, ret, svsm_ret);
+}
+
+void svsm_pval_pages(struct snp_psc_desc *desc)
+{
+ struct svsm_pvalidate_entry pv_4k[VMGEXIT_PSC_MAX_ENTRY];
+ unsigned int i, pv_4k_count = 0;
+ struct svsm_pvalidate_call *pc;
+ struct svsm_call call = {};
+ unsigned long flags;
+ bool action;
+ u64 pc_pa;
+ int ret;
+
+ /*
+ * This can be called very early in the boot, use native functions in
+ * order to avoid paravirt issues.
+ */
+ flags = native_local_irq_save();
+
+ /*
+ * The SVSM calling area (CA) can support processing 510 entries at a
+ * time. Loop through the Page State Change descriptor until the CA is
+ * full or the last entry in the descriptor is reached, at which time
+ * the SVSM is invoked. This repeats until all entries in the descriptor
+ * are processed.
+ */
+ call.caa = svsm_get_caa();
+
+ pc = (struct svsm_pvalidate_call *)call.caa->svsm_buffer;
+ pc_pa = svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+ /* Protocol 0, Call ID 1 */
+ call.rax = SVSM_CORE_CALL(SVSM_CORE_PVALIDATE);
+ call.rcx = pc_pa;
+
+ for (i = 0; i <= desc->hdr.end_entry;) {
+ i = svsm_build_ca_from_psc_desc(desc, i, pc);
+
+ do {
+ ret = svsm_perform_call_protocol(&call);
+ if (!ret)
+ continue;
+
+ /*
+ * Check if the entry failed because of an RMP mismatch (a
+ * PVALIDATE at 2M was requested, but the page is mapped in
+ * the RMP as 4K).
+ */
+
+ if (call.rax_out == SVSM_PVALIDATE_FAIL_SIZEMISMATCH &&
+ pc->entry[pc->cur_index].page_size == RMP_PG_SIZE_2M) {
+ /* Save this entry for post-processing at 4K */
+ pv_4k[pv_4k_count++] = pc->entry[pc->cur_index];
+
+ /* Skip to the next one unless at the end of the list */
+ pc->cur_index++;
+ if (pc->cur_index < pc->num_entries)
+ ret = -EAGAIN;
+ else
+ ret = 0;
+ }
+ } while (ret == -EAGAIN);
+
+ if (ret)
+ svsm_pval_terminate(pc, ret, call.rax_out);
+ }
+
+ /* Process any entries that failed to be validated at 2M and validate them at 4K */
+ for (i = 0; i < pv_4k_count; i++) {
+ u64 pfn, pfn_end;
+
+ action = pv_4k[i].action;
+ pfn = pv_4k[i].pfn;
+ pfn_end = pfn + 512;
+
+ while (pfn < pfn_end) {
+ pfn = svsm_build_ca_from_pfn_range(pfn, pfn_end, action, pc);
+
+ ret = svsm_perform_call_protocol(&call);
+ if (ret)
+ svsm_pval_terminate(pc, ret, call.rax_out);
+ }
+ }
+
+ native_local_irq_restore(flags);
+}
+
+static void update_attest_input(struct svsm_call *call, struct svsm_attest_call *input)
+{
+ /* If (new) lengths have been returned, propagate them up */
+ if (call->rcx_out != call->rcx)
+ input->manifest_buf.len = call->rcx_out;
+
+ if (call->rdx_out != call->rdx)
+ input->certificates_buf.len = call->rdx_out;
+
+ if (call->r8_out != call->r8)
+ input->report_buf.len = call->r8_out;
+}
+
+int snp_issue_svsm_attest_req(u64 call_id, struct svsm_call *call,
+ struct svsm_attest_call *input)
+{
+ struct svsm_attest_call *ac;
+ unsigned long flags;
+ u64 attest_call_pa;
+ int ret;
+
+ if (!snp_vmpl)
+ return -EINVAL;
+
+ local_irq_save(flags);
+
+ call->caa = svsm_get_caa();
+
+ ac = (struct svsm_attest_call *)call->caa->svsm_buffer;
+ attest_call_pa = svsm_get_caa_pa() + offsetof(struct svsm_ca, svsm_buffer);
+
+ *ac = *input;
+
+ /*
+ * Set input registers for the request and set RDX and R8 to known
+ * values in order to detect length values being returned in them.
+ */
+ call->rax = call_id;
+ call->rcx = attest_call_pa;
+ call->rdx = -1;
+ call->r8 = -1;
+ ret = svsm_perform_call_protocol(call);
+ update_attest_input(call, input);
+
+ local_irq_restore(flags);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(snp_issue_svsm_attest_req);
+
+/**
+ * snp_svsm_vtpm_send_command() - Execute a vTPM operation on SVSM
+ * @buffer: A buffer used to both send the command and receive the response.
+ *
+ * Execute a SVSM_VTPM_CMD call as defined by
+ * "Secure VM Service Module for SEV-SNP Guests" Publication # 58019 Revision: 1.00
+ *
+ * All command request/response buffers have a common structure as specified by
+ * the following table:
+ * Byte Size In/Out Description
+ * Offset (Bytes)
+ * 0x000 4 In Platform command
+ * Out Platform command response size
+ *
+ * Each command can build upon this common request/response structure to create
+ * a structure specific to the command. See include/linux/tpm_svsm.h for more
+ * details.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int snp_svsm_vtpm_send_command(u8 *buffer)
+{
+ struct svsm_call call = {};
+
+ call.caa = svsm_get_caa();
+ call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
+ call.rcx = __pa(buffer);
+
+ return svsm_perform_call_protocol(&call);
+}
+EXPORT_SYMBOL_GPL(snp_svsm_vtpm_send_command);
+
+/**
+ * snp_svsm_vtpm_probe() - Probe if SVSM provides a vTPM device
+ *
+ * Check that there is SVSM and that it supports at least TPM_SEND_COMMAND
+ * which is the only request used so far.
+ *
+ * Return: true if the platform provides a vTPM SVSM device, false otherwise.
+ */
+bool snp_svsm_vtpm_probe(void)
+{
+ struct svsm_call call = {};
+
+ /* The vTPM device is available only if a SVSM is present */
+ if (!snp_vmpl)
+ return false;
+
+ call.caa = svsm_get_caa();
+ call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
+
+ if (svsm_perform_call_protocol(&call))
+ return false;
+
+ /* Check platform commands contains TPM_SEND_COMMAND - platform command 8 */
+ return call.rcx_out & BIT_ULL(8);
+}
--
2.51.0
^ permalink raw reply related
* [PATCH 2/3] x86/sev: Add internal header guards
From: Borislav Petkov @ 2025-12-04 12:48 UTC (permalink / raw)
To: Tom Lendacky; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
In-Reply-To: <20251204124809.31783-1-bp@kernel.org>
From: "Borislav Petkov (AMD)" <bp@alien8.de>
All headers need guards ifdeffery.
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
arch/x86/coco/sev/internal.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/coco/sev/internal.h b/arch/x86/coco/sev/internal.h
index c58c47c68ab6..af991f1da095 100644
--- a/arch/x86/coco/sev/internal.h
+++ b/arch/x86/coco/sev/internal.h
@@ -1,4 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __X86_COCO_SEV_INTERNAL_H__
+#define __X86_COCO_SEV_INTERNAL_H__
#define DR7_RESET_VALUE 0x400
@@ -85,3 +87,4 @@ enum es_result sev_es_ghcb_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt
u64 get_hv_features(void);
const struct snp_cpuid_table *snp_cpuid_get_table(void);
+#endif /* __X86_COCO_SEV_INTERNAL_H__ */
--
2.51.0
^ permalink raw reply related
* [PATCH 1/3] x86/sev: Move the internal header
From: Borislav Petkov @ 2025-12-04 12:48 UTC (permalink / raw)
To: Tom Lendacky; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
In-Reply-To: <20251204124809.31783-1-bp@kernel.org>
From: "Borislav Petkov (AMD)" <bp@alien8.de>
Move the internal header out of the usual include/asm/ include path
because having an "internal" header there doesn't really make it so
- quite the opposite.
So move where it belongs and make it really internal.
No functional changes.
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
---
arch/x86/boot/startup/sev-startup.c | 3 ++-
arch/x86/coco/sev/core.c | 3 ++-
arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} | 0
arch/x86/coco/sev/noinstr.c | 3 ++-
arch/x86/coco/sev/vc-handle.c | 3 ++-
5 files changed, 8 insertions(+), 4 deletions(-)
rename arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} (100%)
diff --git a/arch/x86/boot/startup/sev-startup.c b/arch/x86/boot/startup/sev-startup.c
index 09725428d3e6..1115214429fd 100644
--- a/arch/x86/boot/startup/sev-startup.c
+++ b/arch/x86/boot/startup/sev-startup.c
@@ -27,7 +27,6 @@
#include <asm/cpu_entry_area.h>
#include <asm/stacktrace.h>
#include <asm/sev.h>
-#include <asm/sev-internal.h>
#include <asm/insn-eval.h>
#include <asm/fpu/xcr.h>
#include <asm/processor.h>
@@ -41,6 +40,8 @@
#include <asm/cpuid/api.h>
#include <asm/cmdline.h>
+#include "../coco/sev/internal.h"
+
/* Include code shared with pre-decompression boot stage */
#include "sev-shared.c"
diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 9ae3b11754e6..4e618e596267 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -31,7 +31,6 @@
#include <asm/cpu_entry_area.h>
#include <asm/stacktrace.h>
#include <asm/sev.h>
-#include <asm/sev-internal.h>
#include <asm/insn-eval.h>
#include <asm/fpu/xcr.h>
#include <asm/processor.h>
@@ -46,6 +45,8 @@
#include <asm/cmdline.h>
#include <asm/msr.h>
+#include "internal.h"
+
/* Bitmap of SEV features supported by the hypervisor */
u64 sev_hv_features __ro_after_init;
SYM_PIC_ALIAS(sev_hv_features);
diff --git a/arch/x86/include/asm/sev-internal.h b/arch/x86/coco/sev/internal.h
similarity index 100%
rename from arch/x86/include/asm/sev-internal.h
rename to arch/x86/coco/sev/internal.h
diff --git a/arch/x86/coco/sev/noinstr.c b/arch/x86/coco/sev/noinstr.c
index b527eafb6312..9d94aca4a698 100644
--- a/arch/x86/coco/sev/noinstr.c
+++ b/arch/x86/coco/sev/noinstr.c
@@ -16,7 +16,8 @@
#include <asm/msr.h>
#include <asm/ptrace.h>
#include <asm/sev.h>
-#include <asm/sev-internal.h>
+
+#include "internal.h"
static __always_inline bool on_vc_stack(struct pt_regs *regs)
{
diff --git a/arch/x86/coco/sev/vc-handle.c b/arch/x86/coco/sev/vc-handle.c
index f08c7505ed82..43f264afd590 100644
--- a/arch/x86/coco/sev/vc-handle.c
+++ b/arch/x86/coco/sev/vc-handle.c
@@ -23,7 +23,6 @@
#include <asm/init.h>
#include <asm/stacktrace.h>
#include <asm/sev.h>
-#include <asm/sev-internal.h>
#include <asm/insn-eval.h>
#include <asm/fpu/xcr.h>
#include <asm/processor.h>
@@ -35,6 +34,8 @@
#include <asm/apic.h>
#include <asm/cpuid/api.h>
+#include "internal.h"
+
static enum es_result vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt *ctxt,
unsigned long vaddr, phys_addr_t *paddr)
{
--
2.51.0
^ permalink raw reply related
* [PATCH 0/3] x86/sev: Carve out the SVSM support code
From: Borislav Petkov @ 2025-12-04 12:48 UTC (permalink / raw)
To: Tom Lendacky; +Cc: linux-coco, X86 ML, LKML, Borislav Petkov (AMD)
From: "Borislav Petkov (AMD)" <bp@alien8.de>
Hi,
I've been meaning to do this for a while now but didn't have a good idea how
to do it nicely. Using the internal header makes it almost trivial.
Thx.
Borislav Petkov (AMD) (3):
x86/sev: Move the internal header
x86/sev: Add internal header guards
x86/sev: Carve out the SVSM code into a separate compilation unit
arch/x86/boot/startup/sev-startup.c | 3 +-
arch/x86/coco/sev/Makefile | 2 +-
arch/x86/coco/sev/core.c | 380 +-----------------
.../sev-internal.h => coco/sev/internal.h} | 32 ++
arch/x86/coco/sev/noinstr.c | 3 +-
arch/x86/coco/sev/svsm.c | 362 +++++++++++++++++
arch/x86/coco/sev/vc-handle.c | 3 +-
7 files changed, 403 insertions(+), 382 deletions(-)
rename arch/x86/{include/asm/sev-internal.h => coco/sev/internal.h} (75%)
create mode 100644 arch/x86/coco/sev/svsm.c
--
2.51.0
^ permalink raw reply
* SVSM Development Call December 3rd, 2025
From: Jörg Rödel @ 2025-12-04 8:42 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
I completely forgot to send out a notice for the SVSM call yesterday, sorry for
that. I am all the more pleased about the numerous attendance despite my mistake.
Anyway, the meeting minutes from yesterdays call are now posted in this PR:
https://github.com/coconut-svsm/governance/pull/89
Please note that next week there will be no meeting due to a lot of people,
including myself, being at LPC. This means that there is one more meeting this
year on December 17th before we start into the holiday break.
Regards,
Joerg
^ permalink raw reply
* [PATCH] coco/guest: Remove unneeded selection of CRYPTO
From: Eric Biggers @ 2025-12-04 5:55 UTC (permalink / raw)
To: Dan Williams, linux-coco; +Cc: Eric Biggers
All that's needed here is CRYPTO_HASH_INFO. It used to be the case that
CRYPTO_HASH_INFO was visible only when CRYPTO, but that was fixed by
commit aacb37f597d0 ("lib/crypto: hash_info: Move hash_info.c into
lib/crypto/"). Now CRYPTO_HASH_INFO can be selected directly.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
drivers/virt/coco/guest/Kconfig | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/virt/coco/guest/Kconfig b/drivers/virt/coco/guest/Kconfig
index 3d5e1d05bf34..da570dc4bd48 100644
--- a/drivers/virt/coco/guest/Kconfig
+++ b/drivers/virt/coco/guest/Kconfig
@@ -11,7 +11,6 @@ config TSM_REPORTS
tristate
config TSM_MEASUREMENTS
select TSM_GUEST
select CRYPTO_HASH_INFO
- select CRYPTO
bool
base-commit: b2c27842ba853508b0da00187a7508eb3a96c8f7
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] crypto/ccp: Fix CONFIG_PCI=n build
From: Alexey Kardashevskiy @ 2025-12-04 0:27 UTC (permalink / raw)
To: Dan Williams, linux-crypto
Cc: linux-kernel, linux-coco, kernel test robot, Tom Lendacky,
John Allen
In-Reply-To: <20251203031948.2471431-1-dan.j.williams@intel.com>
On 3/12/25 14:19, Dan Williams wrote:
> It turns out that the PCI driver for ccp is unconditionally built into the
> kernel in the CONFIG_PCI=y case. This means that the new SEV-TIO support
> needs an explicit dependency on PCI to avoid build errors when
> CONFIG_CRYPTO_DEV_SP_PSP=y and CONFIG_PCI=n.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: http://lore.kernel.org/202512030743.6pVPA4sx-lkp@intel.com
> Cc: Alexey Kardashevskiy <aik@amd.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: John Allen <john.allen@amd.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/crypto/ccp/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/ccp/Kconfig b/drivers/crypto/ccp/Kconfig
> index e2b127f0986b..f16a0f611317 100644
> --- a/drivers/crypto/ccp/Kconfig
> +++ b/drivers/crypto/ccp/Kconfig
> @@ -39,7 +39,7 @@ config CRYPTO_DEV_SP_PSP
> bool "Platform Security Processor (PSP) device"
> default y
> depends on CRYPTO_DEV_CCP_DD && X86_64 && AMD_IOMMU
> - select PCI_TSM
> + select PCI_TSM if PCI
Acked-by: Alexey Kardashevskiy <aik@amd.com>
> help
> Provide support for the AMD Platform Security Processor (PSP).
> The PSP is a dedicated processor that provides support for key
>
> base-commit: f7ae6d4ec6520a901787cbab273983e96d8516da
> prerequisite-patch-id: 085ed7fc143cfcfd0418527cfad03db88d4b64ec
> prerequisite-patch-id: c1d1a6d802b3b4bfffb9f45fc5ac6a9a1b5e361d
> prerequisite-patch-id: 44c6ea6fb683418ae67ff3efdb0c07fda013e6b2
> prerequisite-patch-id: 407daf59d54ecebcb7fefd22a5b5833e03c038e4
oh it can do this too now, cool :) Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-03 23:48 UTC (permalink / raw)
To: Vishal Annapurve
Cc: Yan Zhao, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, ackerleytng, aik, ira.weiny
In-Reply-To: <CAGtprH8gznGJ6VObk8aShBn_XnhwDoUzyzTkaDAe+MyiNsJ-NA@mail.gmail.com>
On Sun, Nov 30, 2025 at 05:44:31PM -0800, Vishal Annapurve wrote:
> On Fri, Nov 21, 2025 at 5:02 AM Michael Roth <michael.roth@amd.com> wrote:
> >
> > >
> > > Increasing GMEM_GUP_NPAGES to (1UL << PUD_ORDER) is probabaly not a good idea.
> > >
> > > Given both TDX/SNP map at 4KB granularity, why not just invoke post_populate()
> > > per 4KB while removing the max_order from post_populate() parameters, as done
> > > in Sean's sketch patch [1]?
> >
> > That's an option too, but SNP can make use of 2MB pages in the
> > post-populate callback so I don't want to shut the door on that option
> > just yet if it's not too much of a pain to work in. Given the guest BIOS
> > lives primarily in 1 or 2 of these 2MB regions the benefits might be
> > worthwhile, and SNP doesn't have a post-post-populate promotion path
> > like TDX (at least, not one that would help much for guest boot times)
>
> Given the small initial payload size, do you really think optimizing
> for setting up huge page-aligned RMP entries is worthwhile?
Missed this reply earlier.
It could be, but would probably be worthwhile to do some performance
analysis before considering that so we can simplify in the meantime.
> The code becomes somewhat complex when trying to get this scenario
> working and IIUC it depends on userspace-passed initial payload
> regions aligning to the huge page size. What happens if userspace
> tries to trigger snp_launch_update() for two unaligned regions within
> the same huge page?
We'd need to gate the order that we pass to post-populate callback
according to each individual call. For 2MB pages we'd end up with
4K behavior. For 1GB pages, there's some potential of using 2MB
order for each region if gpa/dst/len are aligned, but without the
buddy-style 1G->2M-4K splitting stuff, we'd likely need to split
to 4K at some point and then the 2MB RMP entry would get PSMASH'd
to 4K anyway. But maybe the 1GB could remain intact for long enough
to get through a decent portion of OVMF boot before we end up
creating a mixed range... not sure.
But yes, this also seems like functionality that's premature to
prep for, so just locking it in at 4K is probably best for now.
-Mike
>
> What Sean suggested earlier[1] seems relatively simpler to maintain.
>
> [1] https://lore.kernel.org/kvm/aHEwT4X0RcfZzHlt@google.com/
>
> >
> > Thanks,
> >
> > Mike
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-03 23:12 UTC (permalink / raw)
To: FirstName LastName
Cc: ackerleytng, aik, ashish.kalra, david, ira.weiny, kvm,
liam.merwick, linux-coco, linux-kernel, linux-mm, pbonzini,
seanjc, thomas.lendacky, vbabka, yan.y.zhao
In-Reply-To: <20251203205910.1137445-1-vannapurve@google.com>
On Wed, Dec 03, 2025 at 08:59:10PM +0000, FirstName LastName wrote:
> > >
> > > > but it makes a lot more sense to make those restrictions and changes in
> > > > the context of hugepage support, rather than this series which is trying
> > > > very hard to not do hugepage enablement, but simply keep what's partially
> > > > there intact while reworking other things that have proven to be
> > > > continued impediments to both in-place conversion and hugepage
> > > > enablement.
> > > Not sure how fixing the warning in this series could impede hugepage enabling :)
> > >
> > > But if you prefer, I don't mind keeping it locally for longer.
> >
> > It's the whole burden of needing to anticipate hugepage design, while it
> > is in a state of potentially massive flux just before LPC, in order to
> > make tiny incremental progress toward enabling in-place conversion,
> > which is something I think we can get upstream much sooner. Look at your
> > changelog for the change above, for instance: it has no relevance in the
> > context of this series. What do I put in its place? Bug reports about
> > my experimental tree? It's just not the right place to try to justify
> > these changes.
> >
> > And most of this weirdness stems from the fact that we prematurely added
> > partial hugepage enablement to begin with. Let's not repeat these mistakes,
> > and address changes in the proper context where we know they make sense.
> >
> > I considered stripping out the existing hugepage support as a pre-patch
> > to avoid leaving these uncertainties in place while we are reworking
> > things, but it felt like needless churn. But that's where I'm coming
>
> I think simplifying this implementation to handle populate at 4K pages is worth
> considering at this stage and we could optimize for huge page granularity
> population in future based on the need.
That's probably for the best, after all. Though I think a separate
pre-patch to remove the hugepage stuff would be cleaner, as it
obfuscates the GUP changes quite a bit, which are already pretty subtle
as-is.
I'll plan to do this for the next spin, if there are no objections
raised in the meantime.
>
> 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.
> Extending Sean's suggestion earlier, compile tested only.
Thanks!
-Mike
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index f59c65abe3cf..224e79ab8f86 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2267,66 +2267,56 @@ 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,
> + struct page *src_page, void *opaque)
> {
> struct sev_gmem_populate_args *sev_populate_args = opaque;
> struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> - int n_private = 0, ret, i;
> - int npages = (1 << order);
> - gfn_t gfn;
> + 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;
>
> - 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;
> - }
> + struct sev_data_snp_launch_update fw_args = {0};
> + bool assigned = false;
> + int level;
>
> - if (src) {
> - void *vaddr = kmap_local_pfn(pfn + i);
> + 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 err;
> + }
>
> - if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
> - ret = -EFAULT;
> - goto err;
> - }
> - kunmap_local(vaddr);
> - }
> + if (src_page) {
> + void *vaddr = kmap_local_pfn(pfn);
>
> - ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> - sev_get_asid(kvm), true);
> - if (ret)
> - goto err;
> + memcpy(vaddr, page_address(src_page), PAGE_SIZE);
> + kunmap_local(vaddr);
> + }
>
> - n_private++;
> + ret = rmp_make_private(pfn, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> + sev_get_asid(kvm), true);
> + if (ret)
> + goto err;
>
> - 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;
> + 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;
>
> - 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;
> - }
> + 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;
>
> return 0;
>
> fw_err:
> /*
> * 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 specially.
> *
> * Additionally, when invalid CPUID function entries are detected,
> * firmware writes the expected values into the page and leaves it
> @@ -2336,25 +2326,18 @@ 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 (!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);
> -
> - if (copy_to_user(src + i * PAGE_SIZE, vaddr, PAGE_SIZE))
> - pr_debug("Failed to write CPUID page back to userspace\n");
> + void *vaddr = kmap_local_pfn(pfn);
>
> + memcpy(page_address(src_page), vaddr, PAGE_SIZE);
> 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);
> + pr_debug("%s: exiting with error ret %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..acdcb802d9f2 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, int order, 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;
> + int ret;
>
> 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;
>
> @@ -3156,11 +3143,9 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> * mmu_notifier events can't reach S-EPT entries, and KVM's internal
> * zapping flows are mutually exclusive with S-EPT mappings.
> */
> - for (i = 0; i < PAGE_SIZE; i += TDX_EXTENDMR_CHUNKSIZE) {
> - err = tdh_mr_extend(&kvm_tdx->td, gpa + i, &entry, &level_state);
> - if (TDX_BUG_ON_2(err, TDH_MR_EXTEND, entry, level_state, kvm))
> - return -EIO;
> - }
> + err = tdh_mr_extend(&kvm_tdx->td, gpa, &entry, &level_state);
> + if (TDX_BUG_ON_2(err, TDH_MR_EXTEND, entry, level_state, kvm))
> + return -EIO;
>
> return 0;
> }
> @@ -3196,38 +3181,26 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
> return -EINVAL;
>
> ret = 0;
> - while (region.nr_pages) {
> - if (signal_pending(current)) {
> - ret = -EINTR;
> - break;
> - }
> -
> - arg = (struct tdx_gmem_post_populate_arg) {
> - .vcpu = vcpu,
> - .flags = cmd->flags,
> - };
> - gmem_ret = kvm_gmem_populate(kvm, gpa_to_gfn(region.gpa),
> - u64_to_user_ptr(region.source_addr),
> - 1, tdx_gmem_post_populate, &arg);
> - if (gmem_ret < 0) {
> - ret = gmem_ret;
> - break;
> - }
> + arg = (struct tdx_gmem_post_populate_arg) {
> + .vcpu = vcpu,
> + .flags = cmd->flags,
> + };
> + gmem_ret = kvm_gmem_populate(kvm, gpa_to_gfn(region.gpa),
> + u64_to_user_ptr(region.source_addr),
> + region.nr_pages, tdx_gmem_post_populate, &arg);
> + if (gmem_ret < 0)
> + ret = gmem_ret;
>
> - if (gmem_ret != 1) {
> - ret = -EIO;
> - break;
> - }
> + if (gmem_ret != region.nr_pages)
> + ret = -EIO;
>
> - region.source_addr += PAGE_SIZE;
> - region.gpa += PAGE_SIZE;
> - region.nr_pages--;
> + if (gmem_ret >= 0) {
> + region.source_addr += gmem_ret * PAGE_SIZE;
> + region.gpa += gmem_ret * PAGE_SIZE;
>
> - cond_resched();
> + if (copy_to_user(u64_to_user_ptr(cmd->data), ®ion, sizeof(region)))
> + ret = -EFAULT;
> }
> -
> - if (copy_to_user(u64_to_user_ptr(cmd->data), ®ion, sizeof(region)))
> - ret = -EFAULT;
> return ret;
> }
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index d93f75b05ae2..263e75f90e91 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);
> + struct page *src_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 2e62bf882aa8..550dc818748b 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -85,7 +85,6 @@ static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slo
> 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;
>
> /*
> @@ -794,7 +793,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> return PTR_ERR(folio);
>
> if (!folio_test_uptodate(folio)) {
> - clear_huge_page(&folio->page, 0, folio_nr_pages(folio));
> + clear_highpage(folio_page(folio, 0));
> folio_mark_uptodate(folio);
> }
>
> @@ -812,13 +811,54 @@ 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 gmem_inode *gi;
> + struct folio *folio;
> + int ret, max_order;
> + kvm_pfn_t pfn;
> +
> + gi = GMEM_I(file_inode(file));
> +
> + filemap_invalidate_lock(file->f_mapping);
> +
> + folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> + 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 page *src_aligned_page = NULL;
> struct kvm_memory_slot *slot;
> + struct page *src_page = NULL;
> void __user *p;
> -
> - int ret = 0, max_order;
> + int ret = 0;
> long i;
>
> lockdep_assert_held(&kvm->slots_lock);
> @@ -834,52 +874,50 @@ 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);
> + if (src && !PAGE_ALIGNED(src)) {
> + src_page = alloc_page(GFP_KERNEL_ACCOUNT);
> + if (!src_page)
> + return -ENOMEM;
> + }
>
> npages = min_t(ulong, slot->npages - (start_gfn - slot->base_gfn), npages);
> - for (i = 0; i < npages; i += (1 << max_order)) {
> - struct folio *folio;
> - gfn_t gfn = start_gfn + i;
> - pgoff_t index = kvm_gmem_get_index(slot, gfn);
> - kvm_pfn_t pfn;
> -
> + for (i = 0; i < npages; i++) {
> if (signal_pending(current)) {
> ret = -EINTR;
> break;
> }
>
> - folio = __kvm_gmem_get_pfn(file, slot, index, &pfn, &max_order);
> - if (IS_ERR(folio)) {
> - ret = PTR_ERR(folio);
> - break;
> - }
> -
> - folio_unlock(folio);
> - WARN_ON(!IS_ALIGNED(gfn, 1 << max_order) ||
> - (npages - i) < (1 << max_order));
> + p = src ? src + i * PAGE_SIZE : NULL;
>
> - 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 (p) {
> + if (src_page) {
> + if (copy_from_user(page_address(src_page), p, PAGE_SIZE)) {
> + ret = -EFAULT;
> + break;
> + }
> + src_aligned_page = src_page;
> + } else {
> + ret = get_user_pages((unsigned long)p, 1, 0, &src_aligned_page);
> + if (ret < 0)
> + break;
> + if (ret != 1) {
> + ret = -ENOMEM;
> + break;
> + }
> + }
> }
>
> - p = src ? src + i * PAGE_SIZE : NULL;
> - ret = post_populate(kvm, gfn, pfn, p, max_order, opaque);
> - if (!ret)
> - folio_mark_uptodate(folio);
> + ret = __kvm_gmem_populate(kvm, slot, file, start_gfn + i, src_aligned_page,
> + post_populate, opaque);
> + if (p && !src_page)
> + put_page(src_aligned_page);
>
> -put_folio_and_exit:
> - folio_put(folio);
> if (ret)
> break;
> }
>
> - filemap_invalidate_unlock(file->f_mapping);
> -
> + if (src_page)
> + __free_page(src_page);
> return ret && !i ? ret : i;
> }
> EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_populate);
> --
> 2.52.0.177.g9f829587af-goog
>
^ permalink raw reply
* Re: [PATCH 3/3] KVM: guest_memfd: GUP source pages prior to populating guest memory
From: Michael Roth @ 2025-12-03 23:07 UTC (permalink / raw)
To: Ira Weiny
Cc: Yan Zhao, kvm, linux-coco, linux-mm, linux-kernel,
thomas.lendacky, pbonzini, seanjc, vbabka, ashish.kalra,
liam.merwick, david, vannapurve, ackerleytng, aik
In-Reply-To: <6930a5242dd1b_307bf1002@iweiny-mobl.notmuch>
On Wed, Dec 03, 2025 at 03:01:24PM -0600, Ira Weiny wrote:
> Michael Roth wrote:
> > On Wed, Dec 03, 2025 at 10:46:27AM +0800, Yan Zhao wrote:
> > > On Mon, Dec 01, 2025 at 04:13:55PM -0600, Michael Roth wrote:
> > > > On Mon, Nov 24, 2025 at 05:31:46PM +0800, Yan Zhao wrote:
> > > > > On Fri, Nov 21, 2025 at 07:01:44AM -0600, Michael Roth wrote:
> > > > > > On Thu, Nov 20, 2025 at 05:11:48PM +0800, Yan Zhao wrote:
> > > > > > > On Thu, Nov 13, 2025 at 05:07:59PM -0600, Michael Roth wrote:
>
> [snip]
>
> > > > > > > > ---
> > > > > > > > arch/x86/kvm/svm/sev.c | 40 ++++++++++++++++++++++++++------------
> > > > > > > > arch/x86/kvm/vmx/tdx.c | 21 +++++---------------
> > > > > > > > include/linux/kvm_host.h | 3 ++-
> > > > > > > > virt/kvm/guest_memfd.c | 42 ++++++++++++++++++++++++++++++++++------
> > > > > > > > 4 files changed, 71 insertions(+), 35 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > > > > > index 0835c664fbfd..d0ac710697a2 100644
> > > > > > > > --- a/arch/x86/kvm/svm/sev.c
> > > > > > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > > > > > @@ -2260,7 +2260,8 @@ struct sev_gmem_populate_args {
> > > > > > > > };
> > > > > > > >
> > > > > > > > static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pfn,
> > > > > > > > - void __user *src, int order, void *opaque)
> > > > > > > > + struct page **src_pages, loff_t src_offset,
> > > > > > > > + int order, void *opaque)
> > > > > > > > {
> > > > > > > > struct sev_gmem_populate_args *sev_populate_args = opaque;
> > > > > > > > struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
> > > > > > > > @@ -2268,7 +2269,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> > > > > > > > int npages = (1 << order);
> > > > > > > > gfn_t gfn;
> > > > > > > >
> > > > > > > > - 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_pages))
> > > > > > > > return -EINVAL;
> > > > > > > >
> > > > > > > > for (gfn = gfn_start, i = 0; gfn < gfn_start + npages; gfn++, i++) {
> > > > > > > > @@ -2284,14 +2285,21 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> > > > > > > > goto err;
> > > > > > > > }
> > > > > > > >
> > > > > > > > - if (src) {
> > > > > > > > - void *vaddr = kmap_local_pfn(pfn + i);
> > > > > > > > + if (src_pages) {
> > > > > > > > + void *src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i]));
> > > > > > > > + void *dst_vaddr = kmap_local_pfn(pfn + i);
> > > > > > > >
> > > > > > > > - if (copy_from_user(vaddr, src + i * PAGE_SIZE, PAGE_SIZE)) {
> > > > > > > > - ret = -EFAULT;
> > > > > > > > - goto err;
> > > > > > > > + memcpy(dst_vaddr, src_vaddr + src_offset, PAGE_SIZE - src_offset);
> > > > > > > > + kunmap_local(src_vaddr);
> > > > > > > > +
> > > > > > > > + if (src_offset) {
> > > > > > > > + src_vaddr = kmap_local_pfn(page_to_pfn(src_pages[i + 1]));
> > > > > > > > +
> > > > > > > > + memcpy(dst_vaddr + PAGE_SIZE - src_offset, src_vaddr, src_offset);
> > > > > > > > + kunmap_local(src_vaddr);
> > > > > > > IIUC, src_offset is the src's offset from the first page. e.g.,
> > > > > > > src could be 0x7fea82684100, with src_offset=0x100, while npages could be 512.
> > > > > > >
> > > > > > > Then it looks like the two memcpy() calls here only work when npages == 1 ?
> > > > > >
> > > > > > src_offset ends up being the offset into the pair of src pages that we
> > > > > > are using to fully populate a single dest page with each iteration. So
> > > > > > if we start at src_offset, read a page worth of data, then we are now at
> > > > > > src_offset in the next src page and the loop continues that way even if
> > > > > > npages > 1.
> > > > > >
> > > > > > If src_offset is 0 we never have to bother with straddling 2 src pages so
> > > > > > the 2nd memcpy is skipped on every iteration.
> > > > > >
> > > > > > That's the intent at least. Is there a flaw in the code/reasoning that I
> > > > > > missed?
> > > > > Oh, I got you. SNP expects a single src_offset applies for each src page.
> > > > >
> > > > > So if npages = 2, there're 4 memcpy() calls.
> > > > >
> > > > > src: |---------|---------|---------| (VA contiguous)
> > > > > ^ ^ ^
> > > > > | | |
> > > > > dst: |---------|---------| (PA contiguous)
> > > > >
> > > > >
> > > > > I previously incorrectly thought kvm_gmem_populate() should pass in src_offset
> > > > > as 0 for the 2nd src page.
> > > > >
> > > > > Would you consider checking if params.uaddr is PAGE_ALIGNED() in
> > > > > snp_launch_update() to simplify the design?
> > > >
> > > > This was an option mentioned in the cover letter and during PUCK. I am
> > > > not opposed if that's the direction we decide, but I also don't think
> > > > it makes big difference since:
> > > >
> > > > int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > > > struct page **src_pages, loff_t src_offset,
> > > > int order, void *opaque);
> > > >
> > > > basically reduces to Sean's originally proposed:
> > > >
> > > > int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > > > struct page *src_pages, int order,
> > > > void *opaque);
> > >
> > > Hmm, the requirement of having each copy to dst_page account for src_offset
> > > (which actually results in 2 copies) is quite confusing. I initially thought the
> > > src_offset only applied to the first dst_page.
> >
> > What I'm wondering though is if I'd done a better job of documenting
> > this aspect, e.g. with the following comment added above
> > kvm_gmem_populate_cb:
> >
> > /*
> > * ...
> > * 'src_pages': array of GUP'd struct page pointers corresponding to
> > * the pages that store the data that is to be copied
> > * into the HPA corresponding to 'pfn'
> > * 'src_offset': byte offset, relative to the first page in the array
> > * of pages pointed to by 'src_pages', to begin copying
> > * the data from.
> > *
> > * NOTE: if the caller of kvm_gmem_populate() enforces that 'src' is
> > * page-aligned, then 'src_offset' will always be zero, and src_pages
> > * will contain only 1 page to copy from, beginning at byte offset 0.
> > * In this case, callers can assume src_offset is 0.
> > */
> > int (*kvm_gmem_populate_cb)(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > struct page **src_pages, loff_t src_offset,
> > int order, void *opaque);
> >
> > could the confusion have been avoided, or is it still unwieldly?
> >
> > I don't mind that users like SNP need to deal with the extra bits, but
> > I'm hoping for users like TDX it isn't too cludgy.
>
> FWIW I don't think the TDX code was a problem. I was trying to review the
> SNP code for correctness and it was confusing enough that I was concerned
> the investment is not worth the cost.
I think it would only be worth it if we have some reasonable indication
that someone is using SNP_LAUNCH_UPDATE with un-aligned 'uaddr'/'src'
parameter, or anticipate that a future architecture would rely on such
a thing.
I don't *think* there is, but if that guess it wrong then someone out
there will be very grumpy. I'm not sure what the threshold is for
greenlighting a userspace API change like that though, so I'd prefer
to weasel out of that responsibility by assuming we need to support
non-page-aligned src until the maintainers tell me it's
okay/warranted. :)
>
> I'll re-iterate that the in-place conversion _use_ _case_ requires user
> space to keep the 'source' (ie the page) aligned because it is all getting
> converted anyway. So I'm not seeing a good use case for supporting this.
> But Vishal seemed to think there was so...
I think Sean wanted to leave open the possibility of using a src that
isn't necessarily the same page as the destination. In this series, it
is actually not possible to use 'src' at all if the src/dst are the
same, since that means that src would have been marked with
KVM_MEMORY_ATTRIBUTE_PRIVATE in advance of calling kvm_gmem_populate(),
in which case GUP would trigger the SIGBUS handling in
kvm_gmem_fault_user_mapping(). But I consider that a feature, since
it's more efficient to let userspace initialize it in advance, prior
to marking it as PRIVATE and calling whatever ioctl triggers
kvm_gmem_populate(), and it gets naturally enforced with that existing
checks in kvm_gmem_populate(). So, if src==dst, userspace would need
to pass src==0
>
> Given this potential use case; the above comment is more clear.
>
> FWIW, I think this is going to get even more complex if the src/dest page
> sizes are miss-matched. But that algorithm can be reviewed at that time,
> not now.
>
> > >
> > > This will also cause kvm_gmem_populate() to allocate 1 more src_npages than
> > > npages for dst pages.
> >
> > That's more of a decision on the part of userspace deciding to use
> > non-page-aligned 'src' pointer to begin with.
>
> IIRC this is where I think there might be an issue with the code. The
> code used PAGE_SIZE for the memcpy's. Is it clear that user space must
> have a buffer >= PAGE_SIZE when src_offset != 0?
>
> I did not see that check; and/or I was not clear how that works.
Yes, for SNP_LAUNCH_UPDATE at least, it is documented that the 'len' must
be non-0 and aligned to 4K increments, and that's enforced in
snp_launch_update() handler. I don't quite remember why we didn't just
make it a 'npages' argument but I remember there being some reasoning
for that.
>
> [snip]
>
> > > > > >
> > > > > > This was necessarily chosen in prep for hugepages, but more about my
> > > > > > unease at letting userspace GUP arbitrarilly large ranges. PMD_ORDER
> > > > > > happens to align with 2MB hugepages while seeming like a reasonable
> > > > > > batching value so that's why I chose it.
> > > > > >
> > > > > > Even with 1GB support, I wasn't really planning to increase it. SNP
> > > > > > doesn't really make use of RMP sizes >2MB, and it sounds like TDX
> > > > > > handles promotion in a completely different path. So atm I'm leaning
> > > > > > toward just letting GMEM_GUP_NPAGES be the cap for the max page size we
> > > > > > support for kvm_gmem_populate() path and not bothering to change it
> > > > > > until a solid use-case arises.
> > > > > The problem is that with hugetlb-based guest_memfd, the folio itself could be
> > > > > of 1GB, though SNP and TDX can force mapping at only 4KB.
> > > >
> > > > If TDX wants to unload handling of page-clearing to its per-page
> > > > post-populate callback and tie that its shared/private tracking that's
> > > > perfectly fine by me.
> > > >
> > > > *How* TDX tells gmem it wants this different behavior is a topic for a
> > > > follow-up patchset, Vishal suggested kernel-internal flags to
> > > > kvm_gmem_create(), which seemed reasonable to me. In that case, uptodate
> > > Not sure which flag you are referring to with "Vishal suggested kernel-internal
> > > flags to kvm_gmem_create()".
> > >
> > > However, my point is that when the backend folio is 1GB in size (leading to
> > > max_order being PUD_ORDER), even if SNP only maps at 2MB to RMP, it may hit the
> > > warning of "!IS_ALIGNED(gfn, 1 << max_order)".
> >
> > I think I've had to remove that warning every time I start working on
> > some new spin of THP/hugetlbfs-based SNP. I'm not objecting to that. But it's
> > obvious there, in those contexts, and I can explain exactly why it's being
> > removed.
> >
> > It's not obvious in this series, where all we have are hand-wavy thoughts
> > about what hugepages will look like. For all we know we might decide that
> > kvm_gmem_populate() path should just pre-split hugepages to make all the
> > logic easier, or we decide to lock it in at 4K-only and just strip all the
> > hugepage stuff out.
>
> Yea don't do that.
>
> > I don't really know, and this doesn't seem like the place
> > to try to hash all that out when nothing in this series will cause this
> > existing WARN_ON to be tripped.
>
> Agreed.
>
>
> [snip]
>
> >
> > >
> > > > but it makes a lot more sense to make those restrictions and changes in
> > > > the context of hugepage support, rather than this series which is trying
> > > > very hard to not do hugepage enablement, but simply keep what's partially
> > > > there intact while reworking other things that have proven to be
> > > > continued impediments to both in-place conversion and hugepage
> > > > enablement.
> > > Not sure how fixing the warning in this series could impede hugepage enabling :)
> > >
> > > But if you prefer, I don't mind keeping it locally for longer.
> >
> > It's the whole burden of needing to anticipate hugepage design, while it
> > is in a state of potentially massive flux just before LPC, in order to
> > make tiny incremental progress toward enabling in-place conversion,
> > which is something I think we can get upstream much sooner. Look at your
> > changelog for the change above, for instance: it has no relevance in the
> > context of this series. What do I put in its place? Bug reports about
> > my experimental tree? It's just not the right place to try to justify
> > these changes.
> >
> > And most of this weirdness stems from the fact that we prematurely added
> > partial hugepage enablement to begin with. Let's not repeat these mistakes,
> > and address changes in the proper context where we know they make sense.
> >
> > I considered stripping out the existing hugepage support as a pre-patch
> > to avoid leaving these uncertainties in place while we are reworking
> > things, but it felt like needless churn. But that's where I'm coming
> > from with this series: let's get in-place conversion landed, get the API
> > fleshed out, get it working, and then re-assess hugepages with all these
> > common/intersecting bits out of the way. If we can remove some obstacles
> > for hugepages as part of that, great, but that is not the main intent
> > here.
>
> I'd like to second what Mike is saying here. The entire discussion about
> hugepage support is premature for this series.
Yah, maybe a clean slate, removing the existing hugepage bits as Vishal
is suggesting, is the best way to free ourselves to address these things
incrementally without the historical baggage.
-Mike
>
> Ira
>
> [snip]
^ permalink raw reply
* [PATCH] virt: Fix Kconfig warning when selecting TSM without VIRT_DRIVERS
From: Nathan Chancellor @ 2025-12-03 22:25 UTC (permalink / raw)
To: Dan Williams
Cc: Jonathan Cameron, linux-pci, linux-coco, linux-kernel,
kernel test robot, Nathan Chancellor
After commit 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link
Encryption"), there is a Kconfig warning when selecting CONFIG_TSM
without CONFIG_VIRT_DRIVERS:
WARNING: unmet direct dependencies detected for TSM
Depends on [n]: VIRT_DRIVERS [=n]
Selected by [y]:
- PCI_TSM [=y] && PCI [=y]
CONFIG_TSM is defined in drivers/virt/coco/Kconfig but this Kconfig is
only sourced when CONFIG_VIRT_DRIVERS is enabled. Since this symbol is
hidden with no dependencies, it should be available without a symbol
that just enables a menu.
Move the sourcing of drivers/virt/coco/Kconfig outside of
CONFIG_VIRT_DRIVERS and wrap the other source statements in
drivers/virt/coco/Kconfig with CONFIG_VIRT_DRIVERS to ensure users do
not get any additional prompts while ensuring CONFIG_TSM is always
available to select. This complements commit 110c155e8a68 ("drivers/virt:
Drop VIRT_DRIVERS build dependency"), which addressed the build issue
that this Kconfig warning was pointing out.
Fixes: 3225f52cde56 ("PCI/TSM: Establish Secure Sessions and Link Encryption")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202511140712.NubhamPy-lkp@intel.com/
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
drivers/virt/Kconfig | 4 ++--
drivers/virt/coco/Kconfig | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index d8c848cf09a6..52eb7e4ba71f 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -47,6 +47,6 @@ source "drivers/virt/nitro_enclaves/Kconfig"
source "drivers/virt/acrn/Kconfig"
-source "drivers/virt/coco/Kconfig"
-
endif
+
+source "drivers/virt/coco/Kconfig"
diff --git a/drivers/virt/coco/Kconfig b/drivers/virt/coco/Kconfig
index bb0c6d6ddcc8..df1cfaf26c65 100644
--- a/drivers/virt/coco/Kconfig
+++ b/drivers/virt/coco/Kconfig
@@ -3,6 +3,7 @@
# Confidential computing related collateral
#
+if VIRT_DRIVERS
source "drivers/virt/coco/efi_secret/Kconfig"
source "drivers/virt/coco/pkvm-guest/Kconfig"
@@ -14,6 +15,7 @@ source "drivers/virt/coco/tdx-guest/Kconfig"
source "drivers/virt/coco/arm-cca-guest/Kconfig"
source "drivers/virt/coco/guest/Kconfig"
+endif
config TSM
bool
---
base-commit: 4be423572da1f4c11f45168e3fafda870ddac9f8
change-id: 20251203-fix-pci-tsm-select-tsm-warning-5dd724dc74e0
Best regards,
--
Nathan Chancellor <nathan@kernel.org>
^ permalink raw reply related
* Re: [PATCH v4] page_alloc: allow migration of smaller hugepages during contig_alloc
From: Gregory Price @ 2025-12-03 21:50 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Frank van der Linden, Johannes Weiner, linux-mm, kernel-team,
linux-kernel, akpm, vbabka, surenb, mhocko, jackmanb, ziy, kas,
dave.hansen, rick.p.edgecombe, muchun.song, osalvador, x86,
linux-coco, kvm, Wei Yang, David Rientjes, Joshua Hahn
In-Reply-To: <f6b159c1-e07c-489e-ab9b-4d77551877f0@kernel.org>
On Wed, Dec 03, 2025 at 09:14:44PM +0100, David Hildenbrand (Red Hat) wrote:
> On 12/3/25 21:09, Gregory Price wrote:
> > On Wed, Dec 03, 2025 at 08:43:29PM +0100, David Hildenbrand (Red Hat) wrote:
> > > On 12/3/25 19:01, Frank van der Linden wrote:
> >
> > Worth noting that because this check really only applies to gigantic
> > page *reservation* (not faulting), this isn't necessarily incurred in a
> > time critical path. So, maybe i'm biased here, the reliability increase
> > feels like a win even if the operation can take a very long time under
> > memory pressure scenarios (which seems like an outliar anyway).
>
> Not sure I understand correctly. I think the fix from Mel was the right
> thing to do.
>
> It does not make sense to try migrating a 1GB page when allocating a 1GB
> page. Ever.
>
Oh yeah I agree, this patch doesn't allow that either.
I was just saying his patch's restriction of omitting all HugeTLB
(including 2MB) was more aggressive than needed.
I.e. allowing movement of 2MB pages to increase reliability is (arguably)
worth the potential long-runtime that doing so may produce (because we no
longer filter out regions with 2MB pages).
tl;dr: just re-iterating the theory of this patch.
~Gregory
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Dave Hansen @ 2025-12-03 21:40 UTC (permalink / raw)
To: Edgecombe, Rick P, nik.borisov@suse.com, kas@kernel.org
Cc: kvm@vger.kernel.org, Li, Xiaoyao, Huang, Kai,
linux-coco@lists.linux.dev, Zhao, Yan Y, Wu, Binbin,
linux-kernel@vger.kernel.org, seanjc@google.com, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <e0eb7ab13b715c7509a310bbc043f78898002442.camel@intel.com>
On 12/3/25 13:39, Edgecombe, Rick P wrote:
> So I take it you are ok to leave the general approach, but worth trying to
> remove the dynamic sizing.
Yeah, that sounds fine.
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Edgecombe, Rick P @ 2025-12-03 21:39 UTC (permalink / raw)
To: Hansen, Dave, nik.borisov@suse.com, kas@kernel.org
Cc: kvm@vger.kernel.org, Li, Xiaoyao, Huang, Kai,
linux-coco@lists.linux.dev, Zhao, Yan Y, Wu, Binbin,
linux-kernel@vger.kernel.org, seanjc@google.com, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
Annapurve, Vishal, Gao, Chao, bp@alien8.de, x86@kernel.org
In-Reply-To: <408079db-c488-492e-b6e7-063dea3cb861@intel.com>
On Wed, 2025-12-03 at 12:13 -0800, Dave Hansen wrote:
> BTW, I *DO* expect the DPAMT pages to be mostly allocated forever. Maybe
> I'm just a pessimist, but you can't get them back for compaction or
> reclaim, so they're basically untouchable. Sure, if you kill all the TDX
> guests you get them back, but that's a very different kind of kernel
> memory from stuff that's truly reclaimable under pressure.
If we want to improve it (later) we could topup via a single higher order
allocation. So like if we need 15 pages we could round up to an order-4, then
hand them out individually. But I suspect someone would want to see some proof
before that kind of tweak.
So I take it you are ok to leave the general approach, but worth trying to
remove the dynamic sizing.
^ 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