* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-01 21:08 UTC (permalink / raw)
To: K Prateek Nayak, Jethro Beekman, tglx, mingo, bp, dave.hansen,
x86, hpa, seanjc, peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <8477525d-55ad-4fc4-b7c6-05bab3d7a861@amd.com>
Hi Prateek,
On 7/1/2026 11:39 AM, K Prateek Nayak wrote:
> Hello Jethro,
>
> On 7/1/2026 3:10 PM, Jethro Beekman wrote:
>> I don't believe my concern has been addressed
>>
>> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/
>
> Quoting your question:
>
>> I think this is too broad. If I have a hypervisor that supports SNP
>> virtualization, a (non-confidential) L1 guest running Linux should
>> still support CPU hotplug while also running confidential L2 guests.
>
> Ashish, Tom, correct me if I'm wrong, but I don't think KVM exposes SNP
> support to L1, at least as per
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/cpuid.c?h=v7.2-rc1#n1221
> and only SNP initialization disables hotplug - not the other variants.
>
> L1, running a confidential guest (SEV/SEV-ES) should still be able to
> support hotplug since it doesn't go through SNP init. Only the base
> hypervisor can setup the RMP tables and go through snp_prepare().
>
> Also bsp_determine_snp() should clear CC_ATTR_HOST_SEV_SNP if it
> detects X86_FEATURE_HYPERVISOR so I don't see how this can be a
> problem for hotplug in L1.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/amd.c?h=v7.2-rc1#n368
>
bsp_determine_snp() only sets CC_ATTR_HOST_SEV_SNP when X86_FEATURE_HYPERVISOR is clear:
if (!cpu_has(c, X86_FEATURE_HYPERVISOR) &&
(ZEN3 || ZEN4 || RMPREAD) && snp_probe_rmptable_info())
cc_platform_set(CC_ATTR_HOST_SEV_SNP);
else {
setup_clear_cpu_cap(X86_FEATURE_SEV_SNP);
cc_platform_clear(CC_ATTR_HOST_SEV_SNP);
}
So Linux running as an L1 guest (HYPERVISOR set) never has CC_ATTR_HOST_SEV_SNP.
And both hotplug-disable sites sit behind that flag:
- snp_prepare() is only called from __sev_snp_init_locked(), which returns -ENODEV early if !cc_platform_has(CC_ATTR_HOST_SEV_SNP).
- snp_rmptable_init() bails (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP))) before its kexec one-shot disable.
So an L1 guest can't reach the disable at all; only the bare-metal host that programs the RMP does.
An L1 running SEV/SEV-ES guests never goes through SNP host init, so it's hotplug is unaffected and KVM doesn't expose SNP to L1.
So there's no impact on L1 hotplug currently.
Thanks,
Ashish
^ permalink raw reply
* Re: [PATCH v2 1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE
From: Ackerley Tng @ 2026-07-01 21:15 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco, linux-kernel,
Sashiko Bot, Joerg Roedel, Yan Zhao
In-Reply-To: <20260630213711.479692-2-seanjc@google.com>
Sean Christopherson <seanjc@google.com> writes:
> From: Joerg Roedel <joerg.roedel@amd.com>
>
> Explicitly reject a NULL userspace virtual address for the source page of
> SNP_LAUNCH_UPDATE instead of relying on the post-populate callback to do
> the check, and don't WARN on failure, as the scenario is blatantly user-
> triggerable, as reported by Sashiko. Waiting until post-populate to check
> the address "works", but makes it unnecessarily difficult to see that KVM's
> ABI is to disallow a NULL source page for non-ZERO pages.
>
> Note, several existing VMMs pass a valid userspace address for the ZERO
> case, i.e. KVM can't *require* the userspace address to be NULL for ZERO
> pages, at least not without breaking userspace.
>
> Fixes: dee5a47cc7a4 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command")
> Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/20260611125849.9ED631F00893@smtp.kernel.org
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/svm/sev.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 74fb15551e83..621a2eaa58f2 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> int level;
> int ret;
>
> - if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> - return -EINVAL;
> -
> 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",
> @@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
> return -EINVAL;
>
> - src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
> -
> - if (!PAGE_ALIGNED(src))
> + if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
> + src = NULL;
> + else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
> return -EINVAL;
> + else
> + src = u64_to_user_ptr(params.uaddr);
>
I think separating validation from src assignment logic is better, like
if (!PAGE_ALIGNED(params.uaddr))
return -EINVAL;
if (!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
return -EINVAL;
src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
u64_to_user_ptr(params.uaddr);
This version is also slightly stricter, since it enforces that
params.uaddr is aligned even if params.type is ZERO. (unless the
intention is to really not care about uaddr and permit an unaligned
uaddr?)
And then with conversions I'll add
if (!gmem_in_place_conversion &&
!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
Just a suggestion:
Reviewed-by: Ackerley Tng <ackerleytng@google.com>
> npages = params.len / PAGE_SIZE;
>
> --
> 2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply
* Re: [PATCH v2 1/2] KVM: SEV: Explicitly disallow NULL user address for SNP_LAUNCH_UPDATE
From: Sean Christopherson @ 2026-07-01 21:22 UTC (permalink / raw)
To: Ackerley Tng
Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
Yan Zhao
In-Reply-To: <CAEvNRgEK+LH+M32LrxFSbN3Nf9sYtAVkr2c2vYq5FHzJmJqL7g@mail.gmail.com>
On Wed, Jul 01, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 74fb15551e83..621a2eaa58f2 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2330,9 +2330,6 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > int level;
> > int ret;
> >
> > - if (WARN_ON_ONCE(sev_populate_args->type != KVM_SEV_SNP_PAGE_TYPE_ZERO && !src_page))
> > - return -EINVAL;
> > -
> > 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",
> > @@ -2421,10 +2418,12 @@ static int snp_launch_update(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > params.type != KVM_SEV_SNP_PAGE_TYPE_CPUID))
> > return -EINVAL;
> >
> > - src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL : u64_to_user_ptr(params.uaddr);
> > -
> > - if (!PAGE_ALIGNED(src))
> > + if (params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO)
> > + src = NULL;
> > + else if (!params.uaddr || !PAGE_ALIGNED(params.uaddr))
> > return -EINVAL;
> > + else
> > + src = u64_to_user_ptr(params.uaddr);
> >
>
> I think separating validation from src assignment logic is better, like
>
> if (!PAGE_ALIGNED(params.uaddr))
> return -EINVAL;
>
> if (!params.uaddr && params.type != KVM_SEV_SNP_PAGE_TYPE_ZERO)
> return -EINVAL;
>
> src = params.type == KVM_SEV_SNP_PAGE_TYPE_ZERO ? NULL :
> u64_to_user_ptr(params.uaddr);
>
> This version is also slightly stricter, since it enforces that
> params.uaddr is aligned even if params.type is ZERO.
That's technically an ABI change since KVM fully ignored params.uaddr before
this, i.e. is a potentially breaking change. As noted in the changelog, KVM
should have required the address to be '0', but unfortunately we can't do that
without breaking userspace.
I highly doubt requiring the address to be page-aligned would break userspace,
but KVM also gains nothing from such a requirement, and from an ABI perspective,
placing restrictions on a value that is completely ignored is rather bizarre.
> (unless the intention is to really not care about uaddr and permit an
> unaligned uaddr?)
I don't think there was any intention (which is the whole problem).
^ permalink raw reply
* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: Kalra, Ashish @ 2026-07-01 21:25 UTC (permalink / raw)
To: Jethro Beekman, tglx, mingo, bp, dave.hansen, x86, hpa, seanjc,
peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, KPrateek.Nayak, Tycho.Andersen,
Nathan.Fontenot, ackerleytng, jackyli, pgonda, rientjes, jacobhxu,
xin, pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen,
darwi, linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <80f3f279-d70e-44d7-a179-c52068115e46@fortanix.com>
On 7/1/2026 4:40 AM, Jethro Beekman wrote:
> Hi Ashish,
>
> I don't believe my concern has been addressed
>
> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/
>
> --
The disable tracks SNP_INIT, not "SNP" in general: SNP_INIT requires SnpEn to be set on all present CPUs, and a CPU brought online afterward wouldn't have it, so the kernel that runs SNP_INIT must keep its CPU set stable. Today the only kernel that runs SNP_INIT is the bare-metal host, so a plain L1 guest keeps full CPU hotplug.
Concretely, the path is gated by CC_ATTR_HOST_SEV_SNP, which bsp_determine_snp() sets only when X86_FEATURE_HYPERVISOR is clear and clears otherwise
(as Prateek pointed out). So a Linux L1 guest never has it set, never reaches snp_prepare()/snp_rmptable_init(), and keeps CPU hotplug —
including while running SEV/SEV-ES confidential L2 guests. Only SNP initialization disables hotplug; the other SEV variants don't. And KVM doesn't expose
SNP to L1, so an L1 can't be an SNP host today in any case.
On the nested scenario you raised: if SNP-guest-as-L2 support is added, an L1 acting as an SNP host would run a *virtualized* SNP_INIT. A faithful virtualization carries the same constraint as physical SNP_INIT — all present (v)CPUs must be SnpEn — so that L1 would have the same (v)CPU-hotplug-disable requirement, just over its virtual CPUs, and this same code would apply at that level. So the disable isn't too broad; it correctly tracks SNP_INIT. It simply doesn't apply to a plain L1 guest today, because such a guest isn't running SNP_INIT.
Thanks,
Ashish
> Jethro Beekman | CTO | Fortanix
>
> On 2026-06-30 20:11, Ashish Kalra wrote:
>> From: Ashish Kalra <ashish.kalra@amd.com>
>>
>> While SNP is active, every memory write is checked against the RMP to
>> protect SEV-SNP guest memory. A core performs these RMP checks only once
>> SNP has been initialized via SNP_INIT and the SNP-enable bit in SYSCFG is
>> set on that core; the firmware requires the SNP-enable bit to be set on
>> every present CPU before SNP initialization. A core that is not
>> SNP-enabled and not SNP-initialized performs no RMP checks at all, so
>> there is no valid configuration with SNP active and any CPU exempt from
>> RMP checks.
>>
>> The firmware determines which CPUs are present from the processor and the
>> BIOS/UEFI configuration (e.g. SMT disabled in the BIOS) and enumerates
>> them at SNP init; it is not aware of the OS bringing CPUs online or
>> offline afterwards. SNP_INIT fails unless SnpEn is set on all CPUs, so a
>> CPU that is offline at SNP init does not have SnpEn set, SNP_INIT fails,
>> and there can be no SNP guest memory. OS CPU hotplug can thus diverge
>> from the firmware's expectations and break SNP.
>>
>> Tie CPU hotplug to the SNP-enable bit: disable it in snp_prepare() before
>> SNP is enabled, and re-enable it in snp_shutdown() once the firmware has
>> disabled SNP. If snp_prepare() fails before enabling SNP it re-enables
>> hotplug itself; once SNP is enabled hotplug stays disabled, including
>> across a failed SNP_INIT and across the legacy SNP_SHUTDOWN_EX path, both
>> of which leave SNP enabled. A kexec target that boots with SNP already
>> enabled disables hotplug once in snp_rmptable_init(), since snp_prepare()
>> bails when SNP is already enabled.
>>
>> This also keeps the CPU set stable for the asynchronous RMPOPT scan added
>> later in this series, and ensures cpus_read_lock() in the scan is
>> uncontended.
>>
>> Suggested-by: Thomas Lendacky <thomas.lendacky@amd.com>
>> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
>> ---
>> arch/x86/virt/svm/sev.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/arch/x86/virt/svm/sev.c b/arch/x86/virt/svm/sev.c
>> index dab6e1c290bc..04a58ac4339c 100644
>> --- a/arch/x86/virt/svm/sev.c
>> +++ b/arch/x86/virt/svm/sev.c
>> @@ -535,6 +535,15 @@ int snp_prepare(void)
>>
>> clear_rmp();
>>
>> + /*
>> + * Disable CPU hotplug before enabling SNP, so no CPU can come online
>> + * without SnpEn while SNP is enabled; it is re-enabled in snp_shutdown()
>> + * once SNP is disabled. Must be before cpus_read_lock():
>> + * cpu_hotplug_disable() takes cpu_add_remove_lock, which nests above
>> + * cpu_hotplug_lock.
>> + */
>> + cpu_hotplug_disable();
>> +
>> cpus_read_lock();
>>
>> if (!cpumask_equal(cpu_online_mask, cpu_present_mask)) {
>> @@ -560,6 +569,10 @@ int snp_prepare(void)
>> unlock:
>> cpus_read_unlock();
>>
>> + /* Re-enable CPU hotplug; SnpEn was never set. */
>> + if (ret)
>> + cpu_hotplug_enable();
>> +
>> return ret;
>> }
>> EXPORT_SYMBOL_FOR_MODULES(snp_prepare, "ccp");
>> @@ -587,6 +600,13 @@ void snp_shutdown(void)
>>
>> rmpopt_cleanup();
>>
>> + /*
>> + * Re-enable CPU hotplug now that the firmware has disabled SNP; CPU
>> + * hotplug is not re-enabled for a legacy SNP shutdown. After
>> + * rmpopt_cleanup() so RMPOPT_BASE is cleared with hotplug still disabled.
>> + */
>> + cpu_hotplug_enable();
>> +
>> clear_rmp();
>> on_each_cpu(mfd_reconfigure, NULL, 1);
>> }
>> @@ -645,6 +665,8 @@ EXPORT_SYMBOL_FOR_MODULES(snp_setup_rmpopt, "ccp");
>> */
>> int __init snp_rmptable_init(void)
>> {
>> + u64 val;
>> +
>> if (WARN_ON_ONCE(!cc_platform_has(CC_ATTR_HOST_SEV_SNP)))
>> return -ENOSYS;
>>
>> @@ -654,6 +676,15 @@ int __init snp_rmptable_init(void)
>> if (!setup_rmptable())
>> return -ENOSYS;
>>
>> + /*
>> + * On a kexec boot SNP may already be enabled (legacy firmware leaves
>> + * SnpEn set across shutdown), in which case snp_prepare() bails without
>> + * disabling CPU hotplug, so disable it here.
>> + */
>> + rdmsrq(MSR_AMD64_SYSCFG, val);
>> + if (val & MSR_AMD64_SYSCFG_SNP_EN)
>> + cpu_hotplug_disable();
>> +
>> /*
>> * Setting crash_kexec_post_notifiers to 'true' to ensure that SNP panic
>> * notifier is invoked to do SNP IOMMU shutdown before kdump.
>
^ permalink raw reply
* Re: [PATCH v2 08/17] x86/virt/tdx: Prepare Quote buffer during extension bringup
From: Dave Hansen @ 2026-07-01 19:56 UTC (permalink / raw)
To: Xu Yilun, x86, kvm, linux-coco, linux-kernel
Cc: djbw, kas, rick.p.edgecombe, yilun.xu, xiaoyao.li, sohil.mehta,
adrian.hunter, kishen.maloor, tony.lindgren, peter.fang, baolu.lu,
zhenzhong.duan, dave.hansen, seanjc
In-Reply-To: <20260618081355.3253581-9-yilun.xu@linux.intel.com>
On 6/18/26 01:13, Xu Yilun wrote:
> For simplicity, let all guests share a global buffer. Build the buffer's
> HPA_LINKED_LIST at Quoting extension bringup. This saves a bunch of
> va-to-pa conversions at runtime.
va-to-pa conversions are not expensive. Even for vmalloc() memory.If
I don't like the global buffer. It just generally seems like a bad idea.
It needs locking, it wastes memory when nobody is doing quotes, etc..
Is there a way to do this without vmalloc()?
Could userspace provide the memory and then the kernel just does a gup
to keep it in place while the TDX module is writing to it? Or, could it
just be allocated:
void *buffer = vmalloc(size);
mutex_lock();
// talk to TDX module
mutex_unlock();
vfree(buffer);
return ret;
That actually has some nice properties. It doesn't require locking for
the buffer. It also penalizes folks doing lots of quotes. If you hammer
on quoting, you hammer on the memory allocator and TLB flushing code and
slow yourself down more. If you hammer on it from multiple threads, it
gets even worse.
That seems like a feature not a bug.
^ permalink raw reply
* Re: [RFC PATCH 08/15] x86/virt/tdx: Add interface to check Quoting availability
From: Peter Fang @ 2026-07-02 0:19 UTC (permalink / raw)
To: Nikolay Borisov
Cc: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, linux-coco,
linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <3cf04184-c34a-4d49-926f-5791f12cd74e@suse.com>
On Wed, Jul 01, 2026 at 02:25:05PM +0300, Nikolay Borisov wrote:
> > diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> > index 9d04293394d7..b305fa5aab5c 100644
> > --- a/arch/x86/virt/vmx/tdx/tdx.c
> > +++ b/arch/x86/virt/vmx/tdx/tdx.c
> > @@ -1213,6 +1213,21 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
> > return page_to_phys(td->tdr_page);
> > }
> > +/**
> > + * tdx_quote_enabled() - Check whether TDX Quoting extension is available
> > + *
> > + * Return: %true if the Quoting extension is available, otherwise %false.
> > + */
> > +bool tdx_quote_enabled(void)
>
> nit: Probably rename the function to tdx_quoting_ext_enabled or
> tdx_quote_ext_enabled, so it's abundantly clear it's about an extension and
> not the quoting functionality in general.
Makes sense to me. I'll fix that. Thanks.
>
>
> > +{
> > + /*
> > + * No need for locking here. The quote buffer is initialized as part of
> > + * core TDX bringup, which comes before KVM is ready for userspace.
> > + */
> > + return !!quote_data.buf;
>
> While this works it feels a bit like a hack, perhaps have a static boolean
> variable being set by the init code which is simply returned by this
> function.
I'll fix this as well.
>
> > +}
> > +EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
> > +
> > #define HPAS_PER_PAGE (PAGE_SIZE / sizeof(u64))
> > static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)
>
^ permalink raw reply
* Re: [PATCH v6 03/20] dma-direct: use DMA_ATTR_CC_SHARED in alloc/free paths
From: Alexey Kardashevskiy @ 2026-07-02 0:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Aneesh Kumar K.V (Arm), iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Catalin Marinas,
Jiri Pirko, Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun,
linuxppc-dev, linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Jiri Pirko,
Michael Kelley, Cheloha, Scott
In-Reply-To: <20260630160242.GI7525@ziepe.ca>
On 1/7/26 02:02, Jason Gunthorpe wrote:
> On Thu, Jun 18, 2026 at 12:39:21PM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 18/6/26 01:41, Jason Gunthorpe wrote:
>>> On Wed, Jun 17, 2026 at 10:50:39AM +1000, Alexey Kardashevskiy wrote:
>>>>> @@ -193,16 +193,31 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>>>>> dma_addr_t *dma_handle, gfp_t gfp, unsigned long attrs)
>>>>> {
>>>>> bool remap = false, set_uncached = false;
>>>>> - bool mark_mem_decrypt = true;
>>>>> + bool mark_mem_decrypt = false;
>>>>> struct page *page;
>>>>> void *ret;
>>>>> + /*
>>>>> + * DMA_ATTR_CC_SHARED is not a caller-visible dma_alloc_*()
>>>>> + * attribute. The direct allocator uses it internally after it has
>>>>> + * decided that the backing pages must be shared/decrypted, so the
>>>>> + * rest of the allocation path can consistently select DMA addresses,
>>>>> + * choose compatible pools and restore encryption on free.
>>>>
>>>> Why this limit?
>>>>
>>>> Context: I am looking for a memory pool for a few shared pages (to
>>>> do some guest<->host communication), SWIOTLB seems like the right
>>>> fit but swiotlb_alloc() is not exported and
>>>> dma_direct_alloc(DMA_ATTR_CC_SHARED) is not allowed. Thanks,
>>>
>>> Then setup your struct device so that the DMA API knows the
>>> guest<->host channel requires unecrypted and it will work correctly.
>>>
>>> I think this is a reasonable API to use for that, and I was just
>>> advocating that hyperv should be using it too.
>>>
>>> But it all relies on a properly setup struct device.
>>
>> Sounds good but how do I do that in practice?
>
> I think we haven't got there yet, I understood Dan's plan was to add a
> bit in the struct device that signals if the device must be
> unencrypted or can support all memory.
>
> Currently the dma api assumes all devices must have unencrypted by
> default so it should be fine already, shouldn't it?
It assumes that DMA returns a handle for shared memory but the state of that memory is not guaranteed beforehand. I dislike 2M page mashing.
>> not externally available so I'll have to trick the DMA layer into
>> using SWIOTLB (which is still all shared, right?) as I specifically
>> want to skip page conversions. Setting low DMA mask won't guarantee
>> that the DMA layer won't allocate a page outside of SWIOTLB and
>> convert it. Manually do
>
> Why so particular?
aahhh I missed "pre-". I need a way to get pre-shared pages for my sev-guest activities.
> Any address that satisifies the constraints should
> be good enough?
Any address can be already shared (from swiotlb) or not (from outside swiotlb). The latter will may trigger 2M page smashing and this is what I want to avoid. Makes better sense now? Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v6 00/20] dma-mapping: Use DMA_ATTR_CC_SHARED through direct, pool and swiotlb paths
From: Alexey Kardashevskiy @ 2026-07-02 1:04 UTC (permalink / raw)
To: Jason Gunthorpe, Aneesh Kumar K.V
Cc: Catalin Marinas, iommu, linux-arm-kernel, linux-kernel,
linux-coco, Robin Murphy, Marek Szyprowski, Will Deacon,
Marc Zyngier, Steven Price, Suzuki K Poulose, Jiri Pirko,
Mostafa Saleh, Petr Tesarik, Dan Williams, Xu Yilun, linuxppc-dev,
linux-s390, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), Alexander Gordeev,
Gerald Schaefer, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, Sven Schnelle, x86, Lendacky, Thomas
In-Reply-To: <20260630174216.GK7525@ziepe.ca>
On 1/7/26 03:42, Jason Gunthorpe wrote:
> On Mon, Jun 29, 2026 at 12:16:30PM +0530, Aneesh Kumar K.V wrote:
>>>> Thinking about this more, I guess we should mark the swiotlb as
>>>> cc_shared only with CC_ATTR_GUEST_MEM_ENCRYPT instead of
>>>> CC_ATTR_MEM_ENCRYPT as we have below.
>>>
>>> The name cc_shared should be used for GUEST scenarios only.
>>>
>>> I guess there is some merit in keeping swiotlb using "decrypted" to
>>> mean it usinig pgprot_decrypted and set_memory_decyped() which AMD
>>> gives meaning to on both host and guest.
>>
>> Are you suggesting to change the struct io_tlb_mem::cc_shared back to
>> struct io_tlb_mem::unencrypted?.
>
> Yes
>
>>> IDK what AMD should do on the host by default. I guess it should setup
>>> a swiotlb pool of low dma addrs "unencrypted", but not "cc_shared"?
>>>
>>
>> If by low DMA address you mean using an address with the C-bit
>> cleared.
>
> Yes
>
>> The current code already does this and uses the swiotlb pool correctly
>> on SME.
>
> Well, through the force_dma_unencrypted() hack...
>
>> The challenge arises when we want to force SWIOTLB
>> bouncing even for devices that can handle encrypted DMA addresses (more
>> on that below). For such a config force_dma_uencrypted(dev) will return
>> false and swiotlb will be marked cc_shared/decrypted = true; This trip
>> the new check we added.
>
> Yes, because cc_shared (guest) and unencrypted (host) are very
> different things and we've mixed them:
>
>> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>
> I'm aruging force_dma_unencrypted should mean cc_shared and be
> guest_only, but the SME hack breaks this.
>
>> We can also do
>>
>> if (cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT)) {
>> /* swiotlb pool is incorrect for this device */
>> if (unlikely(mem->cc_shared != force_dma_unencrypted(dev)))
>> return (phys_addr_t)DMA_MAPPING_ERROR;
>>
>> /* Force attrs to match the kind of memory in the pool */
>> if (mem->cc_shared)
>> *attrs |= DMA_ATTR_CC_SHARED;
>> else
>> *attrs &= ~DMA_ATTR_CC_SHARED;
>> } else {
>> /*
>> * Host memory encryption where device requires an
>> * unencrypted dma_addr_t due to dma mask limit
>> */
>> if (force_dma_unencrypted(dev))
>> *attrs |= DMA_ATTR_CC_SHARED;
>> else
>> *attrs &= ~DMA_ATTR_CC_SHARED;
>> }
>
> If we do this I would like to split the force_dma_.. functions into
> guest and host, ie force_dma_cc_shared() and force_host_decrypted()
imho force_dma_unencrypted() should not look at the mask at all (the mask should tell the DMA layer to use swiotlb, encrypted or not), instead, when we set up swiotlb - we could make it unencrypted if iommu=pt, otherwise encrypted (although this means IOMMU and defeats the purpose of swiotlb). But at least this patchset has enough plumbing to have swiotlb encrypted, right?
> To make it clear there are two very different things here.
>
>> Here I see value in having DMA_ATTR_UNENCRYPTED. The question is do we
>> need to split this into two flags and introduce the resulting code
>> duplication.
>
> The external flag name should be DMA_ATTR_CC_SHARED and only used on
> CC guest. Internally that turns into using set_memory_decrypted()
> which works on guest and host for AMD. I don't know how to make the
> host only case clearer and still keep the code efficient..
>
>>> The dma api has to detect, after the driver sets the dma limit, that
>>> none of system memory is usable when:
>>> - The direct path is being used
>>> - phys to dma for 0 is outside the dma limit
>>>
>>> Then it should assume the arch has setup a swiotlb pool for it to use
>>> to fix the high memory problem.
>>>
>>> Similar hackery would be needed in the dma alloc path to know that
>>> decrypted can be used to fix the high memory problem like for GUEST.
>>>
>>> I guess some 'dev_cannot_reach_memory(dev)' sort of test in a
>>> few key places? Setup with a static branch to be a nop on everything
>>> but AMD, compiled out on every other arch.
>>>
>>
>> If we are not able to reach the memory because of the memory encryption
>> bit, then isn't dev_cannot_reach_memory(dev) the same as
>> force_dma_unencrypted(dev)? If so, that is how it is already done.
>
> Sort of yes, but it is properly named to its purpose and not confused
> with what should be a guest-only function.
>
>> x86/dma: Disable forced SWIOTLB bouncing for SME IOMMU passthrough
>
> Maybe as a crutch to get this series merged..
feels okay but I do not really know the true meaning of "swiotlb=force" so adding Tom to the thread. Thanks,
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Binbin Wu @ 2026-07-02 1:12 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
Yan Zhao, Ackerley Tng
In-Reply-To: <akVKgY6JK9im5aDZ@google.com>
On 7/2/2026 1:12 AM, Sean Christopherson wrote:
> On Wed, Jul 01, 2026, Binbin Wu wrote:
>> On 7/1/2026 5:37 AM, Sean Christopherson wrote:
>>> Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
>>> pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
>>> consistent between TDX and SNP (for LAUNCH_UPDATE). EOPNOTSUPP was chosen
>>> to be a forward-looking error code for when guest_memfd supports in-place
>>> conversion, but even when in-place conversion comes along, it's an awkward
>>> error code as KVM is deliberately choosing to disallow virtual address '0',
>>> which is technically a legal userspace address. I.e. it's not so much a
>>> lack of support as it is that KVM reserves address '0' to simplify KVM's
>>> internal implementation.
>>
>> Nit:
>> Do you think it's worth calling this out in the documentation?
>
> Yes, though that can be done separate since this series doesn't change ABI.
> E.g. we can probably do it opportunistically as part of the in-place conversion
> series?
Yes, make sense.
^ permalink raw reply
* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Xiaoyao Li @ 2026-07-02 2:32 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Kiryl Shutsemau
Cc: Dave Hansen, Rick Edgecombe, kvm, x86, linux-coco, linux-kernel,
Sashiko Bot, Joerg Roedel, Yan Zhao, Ackerley Tng
In-Reply-To: <20260630213711.479692-3-seanjc@google.com>
On 7/1/2026 5:37 AM, Sean Christopherson wrote:
> Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
> pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
> consistent between TDX and SNP (for LAUNCH_UPDATE). EOPNOTSUPP was chosen
> to be a forward-looking error code for when guest_memfd supports in-place
> conversion, but even when in-place conversion comes along, it's an awkward
> error code as KVM is deliberately choosing to disallow virtual address '0',
> which is technically a legal userspace address. I.e. it's not so much a
> lack of support as it is that KVM reserves address '0' to simplify KVM's
> internal implementation.
>
> Opportunistically move the check so that it's co-located with the other
> checks on the userspace address, and so that it's more obvious that a NULL
> source address is explicitly disallowed.
>
> Fixes: 2a62345b3052 ("KVM: guest_memfd: GUP source pages prior to populating guest memory")
> Cc: Yan Zhao <yan.y.zhao@intel.com>
> Cc: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/vmx/tdx.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ffe9d0db58c5..b0ec054732b9 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -3198,9 +3198,6 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
> return -EIO;
>
> - if (!src_page)
> - return -EOPNOTSUPP;
> -
> kvm_tdx->page_add_src = src_page;
> ret = kvm_tdp_mmu_map_private_pfn(arg->vcpu, gfn, pfn);
> kvm_tdx->page_add_src = NULL;
> @@ -3247,8 +3244,8 @@ static int tdx_vcpu_init_mem_region(struct kvm_vcpu *vcpu, struct kvm_tdx_cmd *c
> if (copy_from_user(®ion, u64_to_user_ptr(cmd->data), sizeof(region)))
> return -EFAULT;
>
> - if (!PAGE_ALIGNED(region.source_addr) || !PAGE_ALIGNED(region.gpa) ||
> - !region.nr_pages ||
> + if (!PAGE_ALIGNED(region.source_addr) || !region.source_addr ||
> + !PAGE_ALIGNED(region.gpa) || !region.nr_pages ||
> region.gpa + (region.nr_pages << PAGE_SHIFT) <= region.gpa ||
> !vt_is_tdx_private_gpa(kvm, region.gpa) ||
> !vt_is_tdx_private_gpa(kvm, region.gpa + (region.nr_pages << PAGE_SHIFT) - 1))
^ permalink raw reply
* Re: [RFC PATCH v5 30/45] x86/virt/tdx: Add API to demote a 2MB mapping to 512 4KB mappings
From: Yan Zhao @ 2026-07-02 3:34 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: kvm@vger.kernel.org, Li, Xiaoyao, linux-coco@lists.linux.dev,
Huang, Kai, dave.hansen@linux.intel.com, kas@kernel.org,
seanjc@google.com, binbin.wu@linux.intel.com, pbonzini@redhat.com,
mingo@redhat.com, isaku.yamahata@intel.com,
ackerleytng@google.com, linux-kernel@vger.kernel.org,
tglx@kernel.org, sagis@google.com, bp@alien8.de,
Annapurve, Vishal, x86@kernel.org
In-Reply-To: <212aa1eee0c19fe9cabc1ab8cc2b217f1ee0b7ca.camel@intel.com>
On Wed, Jul 01, 2026 at 12:35:34AM +0800, Edgecombe, Rick P wrote:
> On Tue, 2026-06-30 at 10:36 +0800, Yan Zhao wrote:
> > > It feels kind of hacky. The refcount stuff is already a bit hard to follow.
> > After demotion, there would be 512 4KB mappings for this 2MB range.
> > Then, in later tdx_sept_remove_leaf_spte(), tdx_pamt_put() would be invoked
> > for 512 times. That's why we need to set the refcount to 512 after a
> > successful
> > demotion here.
>
> Oops, I thought I deleted it before I sent the mail. After thinking more, it
> seems reasonable. But I didn't check it thoroughly with the other flows yet.
The new wrapper implementation is as below.
I adjusted the pamt_refcount part and made TDX huge page rely on DPAMT.
Will add the following justification for the dependency on DPAMT:
A TDX module supporting non-interruptible DEMOTE usually also supports DPAMT.
Having TDX huge page rely on DPAMT as well simplifies the implementation.
u64 tdh_mem_page_demote(struct tdx_td *td, u64 gpa, enum pg_level level, u64 pfn,
struct page *new_sp, struct tdx_pamt_cache *pamt_cache,
u64 *ext_err1, u64 *ext_err2)
{
struct tdx_module_args args = {
.rcx = gpa | pg_level_to_tdx_sept_level(level),
.rdx = tdx_tdr_pa(td),
.r8 = page_to_phys(new_sp),
};
atomic_t *pamt_refcount = tdx_find_pamt_refcount(pfn);
struct page *pamt_pages[TDX_DPAMT_ENTRY_PAGE_CNT];
u64 ret;
if (!tdx_huge_page_demote_uninterruptible(&tdx_sysinfo) ||
!tdx_supports_dynamic_pamt(&tdx_sysinfo) || level != PG_LEVEL_2M)
return TDX_SW_ERROR;
if (alloc_pamt_array(pamt_pages, pamt_cache))
return TDX_SW_ERROR;
/* Flush the new S-EPT page to be added */
tdx_clflush_page(new_sp);
/* PAMT page pair to track the 2MB guest memory */
args.r12 = page_to_phys(pamt_pages[0]);
args.r13 = page_to_phys(pamt_pages[1]);
spin_lock(&pamt_lock);
/*
* Before demotion, the 2MB guest memory range is tracked in a
* statically allocated pamt 2MB entry, so the pamt_refcount of the 2MB
* range should be 0.
*/
if (atomic_read(pamt_refcount)) {
WARN_ON_ONCE(1);
ret = TDX_SW_ERROR;
goto out;
}
ret = seamcall_saved_ret(TDH_MEM_PAGE_DEMOTE, &args);
if (ret)
goto out;
/* After a sucessful demotion from 2MB to 4KB, set the refcount to 512.
* Subsequent removal of each 4KB mapping will decrease the refcount by
* 1.
*/
atomic_set(pamt_refcount, PTRS_PER_PMD);
out:
spin_unlock(&pamt_lock);
if (ret) {
free_pamt_array(pamt_pages);
*ext_err1 = args.rcx;
*ext_err2 = args.rdx;
}
return ret;
}
^ permalink raw reply
* [PATCH] KVM: x86/tdx: Do not print error message on non-present feature
From: Jiri Slaby (SUSE) @ 2026-07-02 4:32 UTC (permalink / raw)
To: seanjc
Cc: linux-kernel, Jiri Slaby (SUSE), Dan Williams, Chao Gao,
Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Kiryl Shutsemau, Rick Edgecombe, linux-coco, kvm
Before commit 165e77353831 below, the kernel did not print anything when
X86_FEATURE_TDX_HOST_PLATFORM was not present in a cpu.
After the commit, this is emitted to the log, with the error level:
virt/tdx: TDX not supported by the host platform
This is disturbing as it is not an error running CPUs without the
feature.
Drop the error message completely to revert to the state before
165e77353831.
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
Fixes: 165e77353831 ("KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init")
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Chao Gao <chao.gao@intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Sean Christopherson <seanjc@google.com>
--
Cc: Thomas Gleixner <tglx@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Kiryl Shutsemau <kas@kernel.org>
Cc: Rick Edgecombe <rick.p.edgecombe@intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-coco@lists.linux.dev
Cc: kvm@vger.kernel.org
---
arch/x86/virt/vmx/tdx/tdx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..bdfc3bf12a9c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1228,10 +1228,8 @@ static __init int tdx_enable(void)
enum cpuhp_state state;
int ret;
- if (!cpu_feature_enabled(X86_FEATURE_TDX_HOST_PLATFORM)) {
- pr_err("TDX not supported by the host platform\n");
+ if (!cpu_feature_enabled(X86_FEATURE_TDX_HOST_PLATFORM))
return -ENODEV;
- }
if (!cpu_feature_enabled(X86_FEATURE_XSAVE)) {
pr_err("XSAVE is required for TDX\n");
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v6 04/11] x86/virt/tdx: Allocate ref counts for Dynamic PAMT memory
From: Binbin Wu @ 2026-07-02 7:20 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-5-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> The PAMT memory holds metadata for all possible TDX protected memory. Each
> physical address range is covered by PAMT entries at three levels (1GB,
> 2MB, 4KB). With Dynamic PAMT, the 4KB range of PAMT is allocated on
> demand. The kernel supplies the TDX module with page pairs to store the
> 4KB entries, which cover 2MB of host physical memory. The kernel must
> provide this page pair before using pages from the range for TDX. If this
> is not done, SEAMCALLs that give the pages to be protected by the TDX module
> will fail.
>
> Allocate reference counters for every 2MB range to track TDX memory usage.
> This can be used to handle concurrent get/put callers, in order to
> accurately determine when the dynamic 4KB level of Dynamic PAMT needs to
> be allocated and when it can be freed.
>
> This allocation will currently consume 2 MB for every 1 TB of address
> space from 0 to max_pfn. The allocation size will depend on how the RAM is
> physically laid out. In a worst case scenario where the entire 52-bit
> address space is covered this would be 8GB. Then the DPAMT refcount
> allocations could hypothetically cause the savings from Dynamic PAMT to go
> negative on exotic platforms with sparse, small amounts of memory.
Nit:
Do you think it make sense to print the memory size allocated for
DPAMT refcounts in the kernel log?
>
> Future changes could reduce this refcount overhead to be only allocating
> refcounts for physical ranges that contain memory that TDX can use.
> However, this is left for future work.
>
> Assisted-by: Sashiko:claude-opus-4-6 GitHub Copilot:claude-opus-4-6 Sashiko:claude-opus-4-6
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 05/11] x86/virt/tdx: Handle concurrent callers in tdx_pamt_get/put()
From: Binbin Wu @ 2026-07-02 7:39 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-6-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> tdx_pamt_get()/tdx_pamt_put() unconditionally add or remove Dynamic PAMT
> backing for the 2MB region covering the passed pfn. However, multiple
> callers can concurrently operate on 4KB pages that fall within the same
> 2MB region. When this happens only one Dynamic PAMT page pair needs to be
> installed to cover the 2MB range. And when one page is freed, the Dynamic
> PAMT backing cannot be freed until all pages in the range are no longer in
> use. Make the helpers handle these races internally.
>
> Use the per-2MB refcounts from previous changes to track how many 4KB
> pages are in use within each region. Gate the actual Dynamic PAMT add and
> remove on refcount transitions (0->1 and 1->0). Serialize the refcount
> check and SEAMCALL with a global spinlock so the read-decide-act sequence
> is atomic. This also avoids TDX module BUSY errors, as Dynamic PAMT add
> and remove SEAMCALLs take an internal TDX module locks at 2MB granularity,
> so simultaneous attempts on the same region would conflict.
>
> The lock is global and heavyweight. Use simple conditional logic to keep
> correctness obvious. This will be optimized in a later change.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Co-developed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
One nit below.
[...]
>
> @@ -2057,10 +2061,26 @@ static int tdx_pamt_get(kvm_pfn_t pfn)
> if (ret)
> return ret;
>
> - tdx_status = tdh_phymem_pamt_add(pfn, pamt_pages);
> - if (tdx_status != TDX_SUCCESS) {
> - ret = -EIO;
> - goto out_free;
> + pamt_refcount = tdx_find_pamt_refcount(pfn);
tdx_find_pamt_refcount() is tagged as __maybe_unused in the previous patch.
I think the tag should be removed.
[...]
^ permalink raw reply
* Re: [PATCH v6 07/11] KVM: TDX: Allocate PAMT memory for TD and vCPU control structures
From: Binbin Wu @ 2026-07-02 8:55 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang,
Kirill A. Shutemov
In-Reply-To: <20260526023515.288829-8-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
>
> Use control page helpers for allocating and freeing TD control structures,
> such these operations can work for Dynamic PAMT.
^
Nit:
It appears to be missing the word "that".
>
> The TDX module tracks some state for each page of physical memory that it
> might use. It calls this state the PAMT. It includes separate state for
> each page size a physical page could be utilized at within the TDX module
> (1GB, 2MB, 4KB). In Dynamic PAMT, only the 4KB page size state is
> allocated dynamically. So the kernel must install PAMT backing for each 4KB
> page before gifting it to the TDX module, and tear it down after the page
> is reclaimed.
>
> TD-scoped control pages (TDR, TDCS) and vCPU-scoped control pages (TDVPR,
> TDCX) are all handed to the TDX module at 4KB page size and are therefore
> subject to this requirement. Replace the raw alloc_page()/__free_page()
> calls for these pages with tdx_alloc/free_control_page().
>
> Switching between special Dynamic PAMT operations or normal page
> alloc/free operations is handled internally in
> tdx_alloc/free_control_page(). So don't check for Dynamic PAMT around these
> calls. Just call them unconditionally. Similarly, drop the NULL checks
> before freeing, as tdx_free_control_page() handles NULL internally.
>
> No functional change intended when Dynamic PAMT is not in use.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> [sean: handle alloc+free+reclaim in one patch]
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [Rick: enhance log]
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [PATCH v6 08/11] x86/tdx: Add APIs to support Dynamic PAMT ops from KVM's fault path
From: Binbin Wu @ 2026-07-02 9:32 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, dave.hansen, hpa, kas, kvm, linux-coco, linux-doc,
linux-kernel, mingo, nik.borisov, pbonzini, seanjc, tglx,
vannapurve, x86, chao.gao, yan.y.zhao, kai.huang
In-Reply-To: <20260526023515.288829-9-rick.p.edgecombe@intel.com>
On 5/26/2026 10:35 AM, Rick Edgecombe wrote:
> When handling an EPT violation, KVM holds a spinlock while manipulating
> the EPT. Before entering the spinlock it doesn't know how many EPT page
> tables will need to be installed or whether a huge page will be used. For
> this reason it allocates a worst case number of page tables that it might
> need as part of servicing the EPT violation.
>
> Under Dynamic PAMT these pre-allocated pages will potentially need to have
> Dynamic PAMT backing pages installed for them. KVM already has helpers to
> manage topping up page caches before taking the MMU lock, but they cannot be
> passed from KVM to arch/x86 code.
>
> The problem of how and when to install the DPAMT backing pages for the
> pages given to the TDX module during the fault path has had a lot of
> design attempts.
> - Extracting KVM's MMU caches requires too much inlined code added to
> headers.
> - A few varieties of installing Dynamic PAMT backing when allocating the
> S-EPT page tables. [0][1]
> - Using mempool_t to transfer the pages between KVM and arch/x86 doesn't
> work because it is the component is designed more around maintaining a
^^^^^
Nit: extra "it is"
> pool of pages, rather than topping up a continually drained cache.
>
> So don't do these as they all had various problems. Instead just create a
> small simple data structure to use for handing a pre-allocated list of
> pages between KVM and arch/x86 code. Model this on KVM's existing MMU
> memory caches.
>
> Add a tdx_pamt_cache arg to tdx_pamt_get() so it can draw pages from a
> cache when needed. Not all DPAMT page installations will happen under
> spinlock, for example control pages. So have tdx_pamt_get() maintain the
> existing behavior of allocating from the page allocator when NULL is
> passed for the struct tdx_pamt_cache arg. This prevents excess allocations
> for cases where it can be avoided.
>
> Export the new helpers for KVM.
>
> Assisted-by: GitHub Copilot:claude-opus-4-6 Claude:claude-opus-4-7
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> Link: https://lore.kernel.org/kvm/de05853257e9cc66998101943f78a4b7e6e3d741.camel@intel.com/ [0]
> Link: https://lore.kernel.org/kvm/aYprxnSHKHUtk7pt@google.com/ [1]
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply
* Re: [RFC PATCH 06/15] x86/virt/tdx: Initialize Quoting extension during bringup
From: Peter Fang @ 2026-07-02 9:48 UTC (permalink / raw)
To: Edgecombe, Rick P
Cc: djbw@kernel.org, Xu, Yilun, x86@kernel.org, kas@kernel.org,
yilun.xu@linux.intel.com, Duan, Zhenzhong,
linux-kernel@vger.kernel.org, Li, Xiaoyao, Mehta, Sohil,
kvm@vger.kernel.org, linux-coco@lists.linux.dev,
baolu.lu@linux.intel.com
In-Reply-To: <d862b418cdbc9b55c453038184b7a438f34e4da8.camel@intel.com>
On Mon, Jun 29, 2026 at 11:11:27AM -0700, Edgecombe, Rick P wrote:
> On Sun, 2026-06-14 at 00:50 -0700, Peter Fang wrote:
> > > Is this micro-optimization worth it? What are the classes of quote-init
> > > failures vs just make the policy be anything in the module must init.
> >
> > Since there is a fallback option to do the Quoting in userspace, I think
> > it is probably not worth shooting down TDX entirely over quote-init
> > failures.
> >
> > The quote-init failures can come from:
> >
> > 1. Quoting init SEAMCALL failures, which look pretty opaque to the
> > kernel and there's not much it can do about it.
> > 2. Quoting buffer allocation failures, which *are* understood by the
> > kernel, and it could maybe try something else. Right now, we just
> > treat it the same as 1.
> >
> > This is helpful because I think the question of "what if the Quoting
> > extension fails" has come up enough times that it warrants some
> > explanation in the patch log. Thanks.
>
> I thought we were going to do:
> - If quoting is not supported, don't try to init quoting, and don't fail init
> over it.
> - If quoting is supported, but fails to init. Just fail TDX initialization for
> simplicity.
>
> This is what we are doing for the other features.
Hm this is simpler. The "fallback" scenario is probably a stretch right
now. I'll make this change. Thanks.
^ permalink raw reply
* Re: [PATCH 00/32] x86/msr: Drop 32-bit MSR interfaces
From: Ingo Molnar @ 2026-07-02 10:07 UTC (permalink / raw)
To: Sean Christopherson
Cc: Arnd Bergmann, Juergen Gross, linux-kernel, linux-pm,
linux-edac@vger.kernel.org, x86, linux-acpi, kvm, linux-coco,
linux-pci, virtualization, linux-ide, dri-devel, linux-fbdev,
linux-crypto, open list:GPIO SUBSYSTEM, linux-hyperv, linux-hwmon,
linux-perf-users, linux-mtd, platform-driver-x86,
Rafael J . Wysocki, Daniel Lezcano, Zhang Rui,
lukasz.luba@arm.com, Jason Baron, Borislav Petkov, Tony Luck,
Yazen Ghannam, Len Brown, Pavel Machek, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Kirill A. Shutemov, Rick Edgecombe, Pu Wen, Bjorn Helgaas,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list,
Viresh Kumar, Reinette Chatre, Dave Martin, James Morse,
Babu Moger, Tony W Wang-oc, Damien Le Moal, Niklas Cassel,
Dave Airlie, Helge Deller, linux-geode, Olivia Mackall,
Herbert Xu, Linus Walleij, Bartosz Golaszewski,
Greg Kroah-Hartman, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Guenter Roeck, Peter Zijlstra,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Josh Poimboeuf, Pawan Gupta, Vitaly Kuznetsov,
Andy Lutomirski, Boris Ostrovsky, Huang Rui, Mario Limonciello,
Perry Yuan, K Prateek Nayak, srinivas.pandruvada@linux.intel.com,
Artem Bityutskiy, Artem Bityutskiy, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Ashok Raj, Hans de Goede,
Ilpo Järvinen, Rajneesh Bhardwaj, David E Box, xen-devel
In-Reply-To: <akQR9YMtMHReJTfB@google.com>
* Sean Christopherson <seanjc@google.com> wrote:
> > Note that the individual patches are IMO significantly easier to review
> > through the actual 32-bit => 64-bit variable assignment changes done
> > in isolation (which sometimes include minor cleanups), while
> > the Coccinelle semantic patch:
> >
> > { a(b,c) => c = a(b) }
> >
> > which changes both the function signature and the order of terms as
> > well, is just a single add-on treewide patch.
>
> Is the plan for subsystem maintainers to pick up the relevant patches,
> and then do the treewide change one release cycle later?
I'll try to keep the patches in a single tree (tip:x86/msr)
in the hope of not prolonging the pain two cycles - but it's
of course fine for maintainers to pick up the patches too
(most of them are standalone), we'll sort it all out in the end.
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 11/15] KVM: TDX: Factor out userspace return path from tdx_get_quote()
From: Nikolay Borisov @ 2026-07-02 10:08 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-12-yilun.xu@linux.intel.com>
On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
>
> Separate the logic that returns GetQuote to userspace so that
> tdx_get_quote() can be extended to support in-kernel quote generation.
>
> No functional change intended.
>
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/kvm/vmx/tdx.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index ed12805bbb44..9f7c39e0d4b5 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -1524,6 +1524,20 @@ static int tdx_complete_simple(struct kvm_vcpu *vcpu)
> return 1;
> }
>
> +static int tdx_get_quote_user(struct kvm_vcpu *vcpu, u64 gpa, u64 size)
> +{
nit: Function name is misleading, imo appropriate name should have
"fill"/"init" in the name as what it does is to fill the run->tdx
structure.
tdx_fill_quote_user/tdx_init_quote_user
> + vcpu->run->exit_reason = KVM_EXIT_TDX;
> + vcpu->run->tdx.flags = 0;
> + vcpu->run->tdx.nr = TDVMCALL_GET_QUOTE;
> + vcpu->run->tdx.get_quote.ret = TDVMCALL_STATUS_SUBFUNC_UNSUPPORTED;
> + vcpu->run->tdx.get_quote.gpa = gpa;
> + vcpu->run->tdx.get_quote.size = size;
> +
> + vcpu->arch.complete_userspace_io = tdx_complete_simple;
> +
> + return 0;
> +}
> +
<snip>
^ permalink raw reply
* Re: [PATCH 1/2] x86/tdx: Add helper to query maximum TD Quote size
From: Xiaoyao Li @ 2026-07-02 10:16 UTC (permalink / raw)
To: Peter Fang
Cc: Dave Hansen, Kiryl Shutsemau, Rick Edgecombe,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin, linux-kernel, linux-coco,
kvm
In-Reply-To: <20260623044411.GB923079@pedri>
On 6/23/2026 12:44 PM, Peter Fang wrote:
> On Fri, Jun 12, 2026 at 10:25:03PM +0800, Xiaoyao Li wrote:
>>>
>>> Assisted-by: Claude:claude-opus-4-7
>>> Assisted-by: GitHub Copilot:gpt-5.4
>>> Signed-off-by: Peter Fang <peter.fang@intel.com>
>>
>> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
>
> Thanks for the review Xiaoyao!
>
>>
>> I have another nit other than Kiryl's
>>
>>> +u32 tdx_get_max_quote_size(void)
>>> +{
>>> + u64 val, ret;
>>> +
>>> + ret = tdg_vm_rd(TDCS_QUOTE_MAX_SIZE, &val);
>>> +
>>> + return ret ? 0 : (u32)val;
>>> +}
>>> +EXPORT_SYMBOL_GPL(tdx_get_max_quote_size);
>>
>> Do we need to start to use
>>
>> EXPORT_SYMBOL_FOR_MODULES(tdx_get_max_quote_size, "tdx-guest") ?
>>
>
> This makes sense. But can we use a follow-up patch to improve this file
> later? Right now there are only EXPORT_SYMBOL_GPL() usages, so using
> EXPORT_SYMBOL_FOR_MODULES() here might look inconsistent.
If maintainers is going to merge the series as-is, then fine.
If we will have a v2, I don't see why cannot update to use
EXPORT_SYMBOL_FOR_MODULES().
The existing ones can be updated in a separate patch before or after.
^ permalink raw reply
* Re: [PATCH v8 17/46] KVM: guest_memfd: Advertise KVM_SET_MEMORY_ATTRIBUTES2 ioctl
From: Xiaoyao Li @ 2026-07-02 10:51 UTC (permalink / raw)
To: Sean Christopherson
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <akU7uAKb5E_Tklx-@google.com>
On 7/2/2026 12:09 AM, Sean Christopherson wrote:
> On Wed, Jul 01, 2026, Xiaoyao Li wrote:
>> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
>>> @@ -4969,6 +4973,11 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
>>> return 1;
>>> case KVM_CAP_GUEST_MEMFD_FLAGS:
>>> return kvm_gmem_get_supported_flags(kvm);
>>> + case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES:
>>> + if (!gmem_in_place_conversion || !kvm_supports_private_mem(kvm))
>>> + return 0;
>>> +
>>> + return KVM_MEMORY_ATTRIBUTE_PRIVATE;
>>> #endif
>>> default:
>>> break;
>>
>> this looks inconsistent with the
>>
>> case KVM_SET_MEMORY_ATTRIBUTES2:
>> if (!gmem_in_place_conversion)
>> return -ENOTTY;
>>
>> Well, the check of
>>
>> if (!kvm_arch_has_private_mem(f->kvm))
>> return -EINVAL;
>>
>> is buried in the following kvm_gmem_set_attributes(). How about moving of
>> kvm_arch_has_private_mem() check to put it along with
>> gmem_in_place_conversion check in kvm_gmem_ioctl() in Patch 13?
>
> Me confused, patch 13 already adds the kvm_arch_has_private_mem() in
> kvm_gmem_set_attributes().
I wanted to make the check in KVM_SET_MEMORY_ATTRIBUTES2 in Patch 13
like something below:
case KVM_SET_MEMORY_ATTRIBUTES2:
if (!gmem_in_place_conversion || !kvm_arch_has_private_mem(f->kvm))
return -EXXX;
and finally, with the introduction of kvm_supports_private_mem() in this
patch, it becomes:
case KVM_SET_MEMORY_ATTRIBUTES2:
if (!gmem_in_place_conversion || !kvm_supports_private_mem(f->kvm))
return -EXXX;
So that the guard for KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES and
KVM_SET_MEMORY_ATTRIBUTES2 is consistent.
> That said, the ordering here is wonky and misleading. A cursory read of the series
> would make one think that waiting to advertise KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES
> makes it safe/ok for KVM to plumb in support for KVM_SET_MEMORY_ATTRIBUTES2 over
> multiple patches. But that's not actually true, because the ioctl becomes live
> the instant the code exists, userspace doesn't need to wait for KVM to formally
> advertise support.
>
> To further confuse matters, it is actually safe/ok to iteratively add support,
> because it's all effectively dead code until "Let userspace disable per-VM mem
> attributes, enable per-gmem attributes".
yeah, before patch 24, gmem_in_place_conversion is always false. So
KVM_SET_MEMORY_ATTRIBUTES2 on gmem fd always return -ENOTTY.
> So, I think we should go a step further than what I think Xiaoyao is suggesting,
> and fully squash patch 17 into patch 13. That way the reader doesn't have to jump
> through as many mental hoops to piece together what is happening. It'll obviously
> be a bigger patch, but should be easier to review/understand overall.
>
> Oh, and that combined patch should carve out error_offset straightaway, so that
> the full uAPI can be reviewed in a single patch.
It sounds good.
^ permalink raw reply
* Re: [PATCH 00/32] x86/msr: Drop 32-bit MSR interfaces
From: Juergen Gross @ 2026-07-02 11:03 UTC (permalink / raw)
To: Ingo Molnar, Sean Christopherson
Cc: Arnd Bergmann, linux-kernel, linux-pm, linux-edac@vger.kernel.org,
x86, linux-acpi, kvm, linux-coco, linux-pci, virtualization,
linux-ide, dri-devel, linux-fbdev, linux-crypto,
open list:GPIO SUBSYSTEM, linux-hyperv, linux-hwmon,
linux-perf-users, linux-mtd, platform-driver-x86,
Rafael J . Wysocki, Daniel Lezcano, Zhang Rui,
lukasz.luba@arm.com, Jason Baron, Borislav Petkov, Tony Luck,
Yazen Ghannam, Len Brown, Pavel Machek, Thomas Gleixner,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Kirill A. Shutemov, Rick Edgecombe, Pu Wen, Bjorn Helgaas,
Ajay Kaher, Alexey Makhalov, Broadcom internal kernel review list,
Viresh Kumar, Reinette Chatre, Dave Martin, James Morse,
Babu Moger, Tony W Wang-oc, Damien Le Moal, Niklas Cassel,
Dave Airlie, Helge Deller, linux-geode, Olivia Mackall,
Herbert Xu, Linus Walleij, Bartosz Golaszewski,
Greg Kroah-Hartman, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Guenter Roeck, Peter Zijlstra,
Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter,
James Clark, Josh Poimboeuf, Pawan Gupta, Vitaly Kuznetsov,
Andy Lutomirski, Boris Ostrovsky, Huang Rui, Mario Limonciello,
Perry Yuan, K Prateek Nayak, srinivas.pandruvada@linux.intel.com,
Artem Bityutskiy, Artem Bityutskiy, Miquel Raynal,
Richard Weinberger, Vignesh Raghavendra, Ashok Raj, Hans de Goede,
Ilpo Järvinen, Rajneesh Bhardwaj, David E Box, xen-devel
In-Reply-To: <akY4U0jUZm4HOGZ_@gmail.com>
[-- Attachment #1.1.1: Type: text/plain, Size: 1367 bytes --]
On 02.07.26 12:07, Ingo Molnar wrote:
>
> * Sean Christopherson <seanjc@google.com> wrote:
>
>>> Note that the individual patches are IMO significantly easier to review
>>> through the actual 32-bit => 64-bit variable assignment changes done
>>> in isolation (which sometimes include minor cleanups), while
>>> the Coccinelle semantic patch:
>>>
>>> { a(b,c) => c = a(b) }
>>>
>>> which changes both the function signature and the order of terms as
>>> well, is just a single add-on treewide patch.
>>
>> Is the plan for subsystem maintainers to pick up the relevant patches,
>> and then do the treewide change one release cycle later?
>
> I'll try to keep the patches in a single tree (tip:x86/msr)
> in the hope of not prolonging the pain two cycles - but it's
> of course fine for maintainers to pick up the patches too
> (most of them are standalone), we'll sort it all out in the end.
Ingo, would you be fine with me posting patch updates just as replies to the
original patch emails? This would speed things up, as I wouldn't need to wait
for more review input of all the patches before sending out new versions.
As the patches are (mostly) standalone, this should not cause any weird
problems.
The last two patches might need updates, but those can be applied only after
the rest has been accepted anyway.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply
* Re: [PATCH v14 00/44] arm64: Support for Arm CCA in KVM
From: Kohei Enju @ 2026-07-02 13:49 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
Gavin Shan, Shanker Donthineni, Alper Gun, Aneesh Kumar K . V,
Emi Kisanuki, Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <8c0e6a32-2de2-4b94-9fe2-e32f5e9bcc4a@arm.com>
On 07/01 11:53, Steven Price wrote:
> On 01/07/2026 03:15, Kohei Enju wrote:
> > On 05/13 14:17, Steven Price wrote:
> >> This series adds support for running protected VMs using KVM under the
> >> Arm Confidential Compute Architecture (CCA).
> >>
> >> This is rebased on v7.1-rc1, but still targets RMM v2.0-bet1[1].
> >>
> >> The major updates from v13 remain but have been more fully implemented:
> >> the RMM uses the host's page size, range based RMI APIs mean we don't
> >> have to break everything down to base page sizes, the GIC state is
> >> passed via system registers, and the uAPI has been simplified.
> >>
> >> The main changes since v13 are:
> >>
> >> * The RMI definitions and wrappers have been fully updated for RMM
> >> v2.0-bet1. In particular the temporary RMM v1.0 SMC compatibility
> >> patch has been dropped.
> >>
> >> * The PSCI completion ioctl has been removed. RMM v2.0-bet1 still
> >> requires the host to provide the target REC for PSCI calls which
> >> name another vCPU, but KVM now performs the RMI PSCI completion
> >> automatically before entering the REC again. Userspace no longer
> >> needs to issue KVM_ARM_VCPU_RMI_PSCI_COMPLETE. A future spec should
> >> remove the need for the host to provide the MPIDR mapping.
> >>
> >> * The generic RMI init, RMM configuration, GPT setup,
> >> delegate/undelegate helpers and SRO infrastructure have moved out of
> >> KVM into arch/arm64/kernel/rmi.c. RMI is expected to be used by
> >> features outside KVM, so this code should be available even when KVM
> >> is not built.
> >>
> >> * RMI_GRANULE_TRACKING_GET has been updated to work on a range, this
> >> allows it to work when the region is not aligned to the tracking
> >> size. Solves the problem reported by Mathieu[2].
> >>
> >> * SRO support has been moved earlier in the series and improved. It
> >> provides a cleaner way for the host to provide the RMM with the extra
> >> memory it requires. However support is still incomplete where the
> >> TF-RMM code does not yet implement it. This is noted by FIXMEs in the
> >> code.
> >>
> >> * The ARM VM type encoding has been reworked to coexist with the
> >> upstream pKVM KVM_VM_TYPE_ARM_PROTECTED bit.
> >>
> >> * The private-memory documentation now notes that arm64 uses
> >> KVM_CAP_MEMORY_ATTRIBUTES.
> >>
> >> * PMU support is dropped for now. It will be added later in a separate
> >> series. Similarly for selecting the hash algorithm and RPV.
> >
> > Hi Steven,
>
> Hi,
>
> > Is there any plan to add support for selecting the MEC policy (shared or
> > private)? We have been working on adding support for this on top of your
> > series. If this is not already in the works, we may upstream our
> > implementation later.
>
> I've been trying to focus on getting the minimum useful series
> upstreamed before looking at additional features (such as hash
> algorithm, MEC policy etc). If you've already got support then yes
> please do upstream it later when we've got this series landed.
Thank you for the clarification. That makes sense.
I agree that the non-RFC patch should wait until your series has landed.
Before that, however, I'd like to send an RFC patch to gather feedback
on the design from the community while your series is still under
review. I don't expect the RFC to interfere with the upstreaming of your
series.
Thanks,
Kohei.
>
> Thanks,
> Steve
>
> > Thanks,
> > Kohei
> >
> >>
> >> There are also the usual rebase updates and smaller fixes, including
> >> changes to the RMM v2.0-bet1 range APIs, removal of REC auxiliary
> >> granule handling, fixes to the address range descriptor encoding, and
> >> cleanups around realm stage-2 teardown.
> >>
> >> Stateful RMI Operations
> >> -----------------------
> >>
> >> The RMM v2.0 spec introduces Stateful RMI Operations (SROs), which allow
> >> the RMM to complete an operation over several SMC calls while requesting
> >> or returning memory to the host. This allows interrupts to be handled in
> >> the middle of an operation and lets the RMM dynamically allocate memory
> >> for internal tracking purposes. For example, RMI_REC_CREATE no longer
> >> needs auxiliary granules to be provided up front, and can instead
> >> request memory during the operation.
> >>
> >> This series includes the generic SRO infrastructure in
> >> arch/arm64/kernel/rmi.c and uses it for REC create/destroy. The other
> >> cases are not yet used by TF-RMM and a future revision will be needed to
> >> finish those paths in Linux.
> >>
> >> This series is based on v7.1-rc1. It is also available as a git
> >> repository:
> >>
> >> https://gitlab.arm.com/linux-arm/linux-cca cca-host/v14
> >>
> >> Work in progress changes for kvmtool are available from the git
> >> repository below:
> >>
> >> https://gitlab.arm.com/linux-arm/kvmtool-cca cca/v12
> >>
> >> The TF-RMM has not yet merged the RMM v2.0 support, so you will need to
> >> use a branch with RMM v2.0-bet1 support. At the time of writing the
> >> following branch is being used:
> >>
> >> https://git.trustedfirmware.org/TF-RMM/tf-rmm.git topics/rmm-v2.0-poc_2
> >> (tested on commit 3340667a291a)
> >>
> >> There is a kvm-unit-test branch which has been updated to support the
> >> attestation used in RMMv2.0 available here:
> >>
> >> https://gitlab.arm.com/linux-arm/kvm-unit-tests-cca cca/v4
> >>
> >> [1] https://developer.arm.com/documentation/den0137/2-0bet1/
> >> [2] https://lore.kernel.org/all/acrj-cKphy4hJsEG@p14s/
>
>
^ permalink raw reply
* Re: [PATCH] KVM: x86/tdx: Do not print error message on non-present feature
From: Kiryl Shutsemau @ 2026-07-02 14:07 UTC (permalink / raw)
To: Jiri Slaby (SUSE)
Cc: seanjc, linux-kernel, Dan Williams, Chao Gao, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H. Peter Anvin, Rick Edgecombe, linux-coco, kvm
In-Reply-To: <20260702043204.81741-1-jirislaby@kernel.org>
On Thu, Jul 02, 2026 at 06:32:04AM +0200, Jiri Slaby (SUSE) wrote:
> Before commit 165e77353831 below, the kernel did not print anything when
> X86_FEATURE_TDX_HOST_PLATFORM was not present in a cpu.
>
> After the commit, this is emitted to the log, with the error level:
> virt/tdx: TDX not supported by the host platform
>
> This is disturbing as it is not an error running CPUs without the
> feature.
>
> Drop the error message completely to revert to the state before
> 165e77353831.
>
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
> Fixes: 165e77353831 ("KVM: x86/tdx: Do VMXON and TDX-Module initialization during subsys init")
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Chao Gao <chao.gao@intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Sean Christopherson <seanjc@google.com>
Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply
* [PATCH] x86/virt/tdx: Formalize SEAMCALL version encoding support
From: Xu Yilun @ 2026-07-02 14:46 UTC (permalink / raw)
To: x86, linux-kernel
Cc: kas, rick.p.edgecombe, dave.hansen, dave.hansen, yilun.xu,
yilun.xu, chao.gao, djbw, linux-coco, peter.fang
TDX uses the SEAMCALL instruction to invoke various TDX module
functions. Just like the syscall, a SEAMCALL specifies the operation
using a function number and parameters. Moreover, TDX also uses SEAMCALL
versions to extend the functionalities of existing SEAMCALLs while
keeping backward compatibility. Unlike syscall versions that assign
brand new numbers, TDX segments the function number into a basic
function number field and a version field. Together, they encode the new
function number.
An existing SEAMCALL (TDH.VP.INIT) helper is already using the version
field. However, having the caller pack the version into the function
number open-codes the ABI layout, making the SEAMCALL helper definition
obscure and error prone.
Add a version field in struct tdx_module_args, so that most existing
SEAMCALL helpers get a default "version == 0" behavior without code
churn, while callers requiring extended functionalities can specify the
version descriptively. As an internal implementation detail, encode the
tdx_module_args.version in the function number before calling into
assembly code.
Two alternative schemes were considered:
1. Define versioned macros like TDH_VP_INIT_V0, TDH_VP_INIT_V1, etc.
However, this breaks naming consistency unless all existing stable
function macros are changed to TDH_XXX_V0.
2. Add an explicit 'version' parameter to the base seamcall() API. This
unnecessarily forces all stable SEAMCALL helpers to pass a
meaningless '0' argument. Additionally, the magic '0' or '1' values
at caller sites are not descriptive.
Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
---
arch/x86/include/asm/shared/tdx.h | 2 ++
arch/x86/virt/vmx/tdx/seamcall_internal.h | 19 ++++++++++++++++++-
arch/x86/virt/vmx/tdx/tdx.h | 8 --------
arch/x86/virt/vmx/tdx/tdx.c | 5 +++--
4 files changed, 23 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/shared/tdx.h b/arch/x86/include/asm/shared/tdx.h
index f20e91d7ac35..b9aac2de233a 100644
--- a/arch/x86/include/asm/shared/tdx.h
+++ b/arch/x86/include/asm/shared/tdx.h
@@ -143,6 +143,8 @@ struct tdx_module_args {
u64 rbx;
u64 rdi;
u64 rsi;
+ /* ABI version, encoded in rax */
+ u8 version;
};
/* Used to communicate with the TDX module */
diff --git a/arch/x86/virt/vmx/tdx/seamcall_internal.h b/arch/x86/virt/vmx/tdx/seamcall_internal.h
index be5f446467df..7002e41cddad 100644
--- a/arch/x86/virt/vmx/tdx/seamcall_internal.h
+++ b/arch/x86/virt/vmx/tdx/seamcall_internal.h
@@ -11,6 +11,7 @@
#ifndef _X86_VIRT_SEAMCALL_INTERNAL_H
#define _X86_VIRT_SEAMCALL_INTERNAL_H
+#include <linux/bitfield.h>
#include <linux/printk.h>
#include <linux/types.h>
#include <asm/archrandom.h>
@@ -23,6 +24,22 @@ u64 __seamcall_saved_ret(u64 fn, struct tdx_module_args *args);
typedef u64 (*sc_func_t)(u64 fn, struct tdx_module_args *args);
+/*
+ * SEAMCALL leaf:
+ *
+ * Bit 15:0 Leaf number
+ * Bit 23:16 Version number
+ */
+#define SEAMCALL_VERSION_MASK GENMASK_U64(23, 16)
+
+static __always_inline u64 __seamcall_encode_fn(sc_func_t func, u64 fn,
+ struct tdx_module_args *args)
+{
+ FIELD_MODIFY(SEAMCALL_VERSION_MASK, &fn, args->version);
+
+ return func(fn, args);
+}
+
static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
struct tdx_module_args *args)
{
@@ -39,7 +56,7 @@ static __always_inline u64 __seamcall_dirty_cache(sc_func_t func, u64 fn,
*/
this_cpu_write(cache_state_incoherent, true);
- return func(fn, args);
+ return __seamcall_encode_fn(func, fn, args);
}
static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index bdfd0e1e337a..63e3acfb5d0c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -50,14 +50,6 @@
#define TDH_SYS_UPDATE 53
#define TDH_SYS_DISABLE 69
-/*
- * SEAMCALL leaf:
- *
- * Bit 15:0 Leaf number
- * Bit 23:16 Version number
- */
-#define TDX_VERSION_SHIFT 16
-
/* TDX page types */
#define PT_NDA 0x0
#define PT_RSVD 0x1
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..7a89e29b118c 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1910,10 +1910,11 @@ u64 tdh_vp_init(struct tdx_vp *vp, u64 initial_rcx, u32 x2apicid)
.rcx = vp->tdvpr_pa,
.rdx = initial_rcx,
.r8 = x2apicid,
+ /* apicid requires version == 1. */
+ .version = 1,
};
- /* apicid requires version == 1. */
- return seamcall(TDH_VP_INIT | (1ULL << TDX_VERSION_SHIFT), &args);
+ return seamcall(TDH_VP_INIT, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_vp_init);
--
2.25.1
^ permalink raw reply related
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