* Re: [PATCH v2 16/21] x86/virt/seamldr: Handle TDX Module update failures
From: Xu Yilun @ 2026-01-15 6:24 UTC (permalink / raw)
To: Chao Gao
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <20251001025442.427697-17-chao.gao@intel.com>
On Tue, Sep 30, 2025 at 07:53:00PM -0700, Chao Gao wrote:
> Failures encountered after a successful module shutdown are unrecoverable,
> e.g., there is no way to restore the old TDX Module.
"e.g." is obscure. To me, the following sentence is explaining the
reason why the failure is not recoverable. Maybe "i.e." or "because"?
^ permalink raw reply
* Re: [PATCH v2 04/21] x86/virt/tdx: Prepare to support P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-01-15 7:25 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aWYVCImRjhIogogF@yilunxu-OptiPlex-7050>
On Tue, Jan 13, 2026 at 05:48:56PM +0800, Xu Yilun wrote:
>> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
>> index e872a411a359..7ad026618a23 100644
>> --- a/arch/x86/include/asm/tdx.h
>> +++ b/arch/x86/include/asm/tdx.h
>> @@ -32,6 +32,11 @@
>> #define TDX_SUCCESS 0ULL
>> #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
>>
>> +/* P-SEAMLDR SEAMCALL leaf function error codes */
>> +#define SEAMLDR_RND_NO_ENTROPY 0x8000000000030001ULL
>> +
>> +#define SEAMLDR_SEAMCALL_MASK _BITUL(63)
>
>If we do want Patch #3 [*], I think no need to expose this bit in
>public, define it in your newly added arch/x86/virt/vmx/tdx/seamcall.h
Makes sense.
>
>[*]: https://lore.kernel.org/all/20251001025442.427697-4-chao.gao@intel.com/
>
>[...]
>
>> +static inline bool sc_need_retry(u64 fn, u64 error_code)
>> +{
>> + if (is_seamldr_call(fn))
>> + return error_code == SEAMLDR_RND_NO_ENTROPY;
>> + else
>> + return error_code == TDX_RND_NO_ENTROPY;
>> +}
>> +
>
>Maybe we can remove this single-use wrapper and integrate it in
>sc_retry?
>
>> static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>> struct tdx_module_args *args)
>> {
>
> u64 retry_code = TDX_RND_NO_ENTROPY;
>
> if (is_seamldr_call(fn))
> retry_code = SEAMLDR_RND_NO_ENTROPY;
>
>> @@ -22,7 +35,7 @@ static __always_inline u64 sc_retry(sc_func_t func, u64 fn,
>>
>> do {
>> ret = func(fn, args);
>> - } while (ret == TDX_RND_NO_ENTROPY && --retry);
>> + } while (sc_need_retry(fn, ret) && --retry);
>
> } while (ret == retry_code && --retry);
>
>Seems more straightforward to me.
I am ok with this change. Will do.
>
>>
>> return ret;
>> }
>> @@ -48,6 +61,17 @@ static inline void seamcall_err_ret(u64 fn, u64 err,
>> args->r9, args->r10, args->r11);
>> }
>>
>> +static inline void seamldr_err(u64 fn, u64 err, struct tdx_module_args *args)
>> +{
>> + /*
>> + * Get the actual leaf number. No need to print the bit used to
>> + * differentiate between P-SEAMLDR and TDX module as the "P-SEAMLDR"
>> + * string in the error message already provides that information.
>> + */
>> + fn &= ~SEAMLDR_SEAMCALL_MASK;
>
>Why we must strip the bit from leaf number? See from P-seamldr SPEC [*],
>all leaf definitions are with this bit, same for your C code:
>
> +/* P-SEAMLDR SEAMCALL leaf function */
> +#define P_SEAMLDR_INFO 0x8000000000000000
>
>So my feeling is, log readability reduces without this bit. We don't
>have to make all developers understand this bit, just expose the whole
>leaf as magic number.
Agree. Printing P-SEAMLDR leaf numbers in hex without removing bit 63 is
better, so the values shown in the logs can match code and spec.
>
>[*] https://cdrdv2.intel.com/v1/dl/getContent/733584
>
>> + pr_err("P-SEAMLDR (%lld) failed: 0x%016llx\n", fn, err);
>
>I see no problem we keep both "P-SEAMLDR" string & SEAMLDR bit printed.
>
>
>And could we handle it the same as sc_retry(), something like:
>
> static inline void seamcall_err(u64 fn, u64 err, struct tdx_module_args *args)
> {
>- pr_err("SEAMCALL (0x%016llx) failed: 0x%016llx\n", fn, err);
>+ char *call = "SEAMCALL";
>+
>+ if (is_seamldr_call(fn))
>+ call = "P-SEAMLDR";
>+
>+ pr_err("%s (0x%016llx) failed: 0x%016llx\n", call, fn, err);
> }
It's doable, but the conditional is unnecessary ...
>
>And the benifit is ...
>
>> +}
>> +
>> static __always_inline int sc_retry_prerr(sc_func_t func,
>> sc_err_func_t err_func,
>> u64 fn, struct tdx_module_args *args)
>> @@ -76,4 +100,7 @@ static __always_inline int sc_retry_prerr(sc_func_t func,
>> #define seamcall_prerr_ret(__fn, __args) \
>> sc_retry_prerr(__seamcall_ret, seamcall_err_ret, (__fn), (__args))
>>
>> +#define seamldr_prerr(__fn, __args) \
>> + sc_retry_prerr(__seamcall, seamldr_err, (__fn), (__args))
>
>... we don't need this definition anymore, just use seamcall_prerr()
Having this seamldr_prerr() (and seamldr_err()) is to eliminate that conditional.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Allow userspace to return errors to guest for MAPGPA
From: Xiaoyao Li @ 2026-01-15 7:47 UTC (permalink / raw)
To: Sagi Shahar, Sean Christopherson
Cc: Paolo Bonzini, Dave Hansen, Kiryl Shutsemau, Rick Edgecombe,
Thomas Gleixner, Borislav Petkov, H. Peter Anvin, x86, kvm,
linux-kernel, linux-coco, Vishal Annapurve
In-Reply-To: <CAAhR5DE=ypkYwqEGEJBZs5A2N9OCVaL_9Jxi5YN5X7rNpKSZTw@mail.gmail.com>
On 1/15/2026 9:21 AM, Sagi Shahar wrote:
> On Wed, Jan 14, 2026 at 9:57 AM Sean Christopherson <seanjc@google.com> wrote:
>>
>> On Wed, Jan 14, 2026, Xiaoyao Li wrote:
>>> On 1/14/2026 8:30 AM, Sagi Shahar wrote:
>>>> From: Vishal Annapurve <vannapurve@google.com>
>>>>
>>>> MAPGPA request from TDX VMs gets split into chunks by KVM using a loop
>>>> of userspace exits until the complete range is handled.
>>>>
>>>> In some cases userspace VMM might decide to break the MAPGPA operation
>>>> and continue it later. For example: in the case of intrahost migration
>>>> userspace might decide to continue the MAPGPA operation after the
>>>> migrration is completed
>>
>> migration
>>
>>>> Allow userspace to signal to TDX guests that the MAPGPA operation should
>>>> be retried the next time the guest is scheduled.
>>
>> To Xiaoyao's point, changes like this either need new uAPI, or a detailed
>> explanation in the changelog of why such uAPI isn't deemed necessary.
>>
>>>> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
>>>> Co-developed-by: Sagi Shahar <sagis@google.com>
>>>> Signed-off-by: Sagi Shahar <sagis@google.com>
>>>> ---
>>>> arch/x86/kvm/vmx/tdx.c | 8 +++++++-
>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
>>>> index 2d7a4d52ccfb..3244064b1a04 100644
>>>> --- a/arch/x86/kvm/vmx/tdx.c
>>>> +++ b/arch/x86/kvm/vmx/tdx.c
>>>> @@ -1189,7 +1189,13 @@ static int tdx_complete_vmcall_map_gpa(struct kvm_vcpu *vcpu)
>>>> struct vcpu_tdx *tdx = to_tdx(vcpu);
>>>> if (vcpu->run->hypercall.ret) {
>>>> - tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
>>>> + if (vcpu->run->hypercall.ret == -EBUSY)
>>>> + tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
>>>> + else if (vcpu->run->hypercall.ret == -EINVAL)
>>>> + tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
>>>> + else
>>>> + return -EINVAL;
>>>
>>> It's incorrect to return -EINVAL here.
>>
>> It's not incorrect, just potentially a breaking change.
>>
>>> The -EINVAL will eventually be
>>> returned to userspace for the VCPU_RUN ioctl. It certainly breaks userspace.
>>
>> It _might_ break userspace. It certainly changes KVM's ABI, but if no userspace
>> actually utilizes the existing ABI, then userspace hasn't been broken.
>>
>> And unless I'm missing something, QEMU _still_ doesn't set hypercall.ret. E.g.
>> see this code in __tdx_map_gpa().
>>
>> /*
>> * In principle this should have been -KVM_ENOSYS, but userspace (QEMU <=9.2)
>> * assumed that vcpu->run->hypercall.ret is never changed by KVM and thus that
>> * it was always zero on KVM_EXIT_HYPERCALL. Since KVM is now overwriting
>> * vcpu->run->hypercall.ret, ensuring that it is zero to not break QEMU.
>> */
>> tdx->vcpu.run->hypercall.ret = 0;
>>
>> AFAICT, QEMU kills the VM if anything goes wrong.
>>
>> So while I initially had the exact same reaction of "this is a breaking change
>> and needs to be opt-in", we might actually be able to get away with just making
>> the change (assuming no other VMMs care, or are willing to change themselves).
>
> Is there a better source of truth for whether QEMU uses hypercall.ret
> or just point to this comment in the commit message.
No version of QEMU touches hypercall.ret, from the source code.
I suggest not mentioning the comment, because it only tells QEMU expects
vcpu->run->hypercall.ret to be 0 on KVM_EXIT_HYPERCALL. What matters is
QEMU never sets vcpu->run->hypercall.ret to a non-zero value after
handling KVM_EXIT_HYPERCALL. I think you can just describe the fact that
QEMU never set vcpu->run->hypercall.ret to a non-zero value in the
commit message.
^ permalink raw reply
* Re: [PATCH v2 05/21] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-01-15 10:12 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Kirill A. Shutemov
In-Reply-To: <aWYntT4AIRrnGvv0@yilunxu-OptiPlex-7050>
On Tue, Jan 13, 2026 at 07:08:37PM +0800, Xu Yilun wrote:
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 58d890fe2100..6b47383d2958 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -1905,6 +1905,16 @@ config INTEL_TDX_HOST
>>
>> If unsure, say N.
>>
>> +config INTEL_TDX_MODULE_UPDATE
>> + bool "Intel TDX module runtime update"
>> + depends on TDX_HOST_SERVICES
>> + help
>> + This enables the kernel to support TDX module runtime update. This
>> + allows the admin to update the TDX module to the same or any newer
>> + version without the need to terminate running TDX guests.
>
>I'm wondering if it is better to put this option in
>drivers/virt/coco/tdx-host. Just as TDX Connect, the
>functionalities/uAPIs are exposed in /sys/devices/faux/tdx_host. Better
>the 2 features could have aligned config pattern. The TDX Connect
>configuration is here:
>
> https://lore.kernel.org/all/20251117022311.2443900-4-yilun.xu@linux.intel.com/
Agreed. TDX Connect and Module update should align in this matter.
I will move this kconfig under drivers/virt/coco/tdx-host.
>
>> +
>> + If unsure, say N.
>> +
>> config EFI
>> bool "EFI runtime service support"
>> depends on ACPI
>> diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile
>> index 90da47eb85ee..26aea3531c36 100644
>> --- a/arch/x86/virt/vmx/tdx/Makefile
>> +++ b/arch/x86/virt/vmx/tdx/Makefile
>> @@ -1,2 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> obj-y += seamcall.o tdx.o
>> +obj-$(CONFIG_INTEL_TDX_MODULE_UPDATE) += seamldr.o
>
>And I'm wondering if we must disable seamldr core helpers if Update
>uAPIs are not selected. TDX core now are expected to expose various
>helpers for different features and is it necessary we have to mask
>in/out all helpers in such a fine granularity? For example we may not
>disable tdh_mem_sept_xx() helpers if KVM_INTEL is not selected.
I would rather keep this. seamldr.c will have other facilities that are for
updates only. It's better to compile them out if there are no kernel users of
them. I would agree with you if we needed to sprinkle a few #ifdef/#endif
throughout the C file, but that isn't the case as the whole file won't be
compiled.
^ permalink raw reply
* Re: [PATCH v2 05/21] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-01-15 10:22 UTC (permalink / raw)
To: Duan, Zhenzhong
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, yilun.xu, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Kirill A. Shutemov
In-Reply-To: <3354385d-b1a0-416d-a3cd-53d515840b1c@intel.com>
>> diff --git a/arch/x86/virt/vmx/tdx/Makefile b/arch/x86/virt/vmx/tdx/Makefile
>> index 90da47eb85ee..26aea3531c36 100644
>> --- a/arch/x86/virt/vmx/tdx/Makefile
>> +++ b/arch/x86/virt/vmx/tdx/Makefile
>> @@ -1,2 +1,3 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> obj-y += seamcall.o tdx.o
>> +obj-$(CONFIG_INTEL_TDX_MODULE_UPDATE) += seamldr.o
>
>Not clear if seamldr will support other features besides TDX module update,
>
>if yes, maybe more general name CONFIG_INTEL_SEAMLDR?
Currently, no other features. So, CONFIG_INTEL_TDX_MODULE_UPDATE should be
good for now. If some new feature emerges, we can add CONFIG_INTEL_SEAMLDR and
make CONFIG_INTEL_TDX_MODULE_UPDATE and new features select it.
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Allow userspace to return errors to guest for MAPGPA
From: Sean Christopherson @ 2026-01-15 16:54 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Sagi Shahar, Paolo Bonzini, Dave Hansen, Kiryl Shutsemau,
Rick Edgecombe, Thomas Gleixner, Borislav Petkov, H. Peter Anvin,
x86, kvm, linux-kernel, linux-coco, Vishal Annapurve
In-Reply-To: <af8bbddc-fcf5-460b-9a6f-1418a0748f37@intel.com>
On Thu, Jan 15, 2026, Xiaoyao Li wrote:
> On 1/15/2026 9:21 AM, Sagi Shahar wrote:
> > On Wed, Jan 14, 2026 at 9:57 AM Sean Christopherson <seanjc@google.com> wrote:
> > > On Wed, Jan 14, 2026, Xiaoyao Li wrote:
> > > > The -EINVAL will eventually be returned to userspace for the VCPU_RUN
> > > > ioctl. It certainly breaks userspace.
> > >
> > > It _might_ break userspace. It certainly changes KVM's ABI, but if no userspace
> > > actually utilizes the existing ABI, then userspace hasn't been broken.
> > >
> > > And unless I'm missing something, QEMU _still_ doesn't set hypercall.ret. E.g.
> > > see this code in __tdx_map_gpa().
> > >
> > > /*
> > > * In principle this should have been -KVM_ENOSYS, but userspace (QEMU <=9.2)
> > > * assumed that vcpu->run->hypercall.ret is never changed by KVM and thus that
> > > * it was always zero on KVM_EXIT_HYPERCALL. Since KVM is now overwriting
> > > * vcpu->run->hypercall.ret, ensuring that it is zero to not break QEMU.
> > > */
> > > tdx->vcpu.run->hypercall.ret = 0;
> > >
> > > AFAICT, QEMU kills the VM if anything goes wrong.
> > >
> > > So while I initially had the exact same reaction of "this is a breaking change
> > > and needs to be opt-in", we might actually be able to get away with just making
> > > the change (assuming no other VMMs care, or are willing to change themselves).
> >
> > Is there a better source of truth for whether QEMU uses hypercall.ret
> > or just point to this comment in the commit message.
>
> No version of QEMU touches hypercall.ret, from the source code.
>
> I suggest not mentioning the comment, because it only tells QEMU expects
> vcpu->run->hypercall.ret to be 0 on KVM_EXIT_HYPERCALL. What matters is QEMU
> never sets vcpu->run->hypercall.ret to a non-zero value after handling
> KVM_EXIT_HYPERCALL. I think you can just describe the fact that QEMU never
> set vcpu->run->hypercall.ret to a non-zero value in the commit message.
+1. We can't _guarantee_ changing the behavior won't break userspace, e.g. in
theory, someone could be running a fork of QEMU in production that explicitly
sets hypercall.ret to some weird value. Or someone could be running a VMM we
don't even know about. I.e. there is no single source of truth, all we can do
is explain why we have high confidence that the ABI change won't break anything.
^ permalink raw reply
* Re: [PATCH v3 0/6] KVM: guest_memfd: Rework preparation/population flows in prep for in-place conversion
From: Sean Christopherson @ 2026-01-15 18:03 UTC (permalink / raw)
To: Sean Christopherson, kvm, Michael Roth
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
vbabka, ashish.kalra, liam.merwick, vannapurve, ackerleytng, aik,
ira.weiny, yan.y.zhao, pankaj.gupta, David Hildenbrand
In-Reply-To: <20260108214622.1084057-1-michael.roth@amd.com>
On Thu, 08 Jan 2026 15:46:16 -0600, Michael Roth wrote:
> This patchset is also available at:
>
> https://github.com/AMDESE/linux/tree/gmem-populate-rework-v3
>
> and is based on top of kvm/next (0499add8efd7)
>
>
> [...]
Applied to kvm-x86 gmem, with the tweaked logic I suggested. Thanks!
[1/6] KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()
https://github.com/kvm-x86/linux/commit/60b590de8b30
[2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
https://github.com/kvm-x86/linux/commit/0726d3e164f1
[3/6] KVM: guest_memfd: Remove preparation tracking
https://github.com/kvm-x86/linux/commit/188349ceb0f0
[4/6] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
https://github.com/kvm-x86/linux/commit/b2e648758038
[5/6] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
https://github.com/kvm-x86/linux/commit/894c3cc35b89
[6/6] KVM: guest_memfd: GUP source pages prior to populating guest memory
https://github.com/kvm-x86/linux/commit/ba375af3d04d
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply
* Re: [PATCH v3 2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Sean Christopherson @ 2026-01-15 19:17 UTC (permalink / raw)
To: Michael Roth
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <20260108214622.1084057-3-michael.roth@amd.com>
On Thu, Jan 08, 2026, Michael Roth wrote:
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index fdaea3422c30..9dafa44838fe 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> mapping_gfp_mask(inode->i_mapping), policy);
> mpol_cond_put(policy);
>
> + /*
> + * External interfaces like kvm_gmem_get_pfn() support dealing
> + * with hugepages to a degree, but internally, guest_memfd currently
> + * assumes that all folios are order-0 and handling would need
> + * to be updated for anything otherwise (e.g. page-clearing
> + * operations).
> + */
> + WARN_ON_ONCE(folio_order(folio));
Gah, this is buggy. __filemap_get_folio_mpol() can return an ERR_PTR(). If that
happens, this WARN will dereference garbage and explode.
And of course I find it _just_ after sending thank yous, *sigh*.
I'll squash to this (after testing):
WARN_ON_ONCE(!IS_ERR(folio) && folio_order(folio));
avoiding a few emails isn't worth having a lurking bug.
^ permalink raw reply
* Re: [PATCH v3 2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
From: Michael Roth @ 2026-01-15 20:42 UTC (permalink / raw)
To: Sean Christopherson
Cc: kvm, linux-coco, linux-mm, linux-kernel, thomas.lendacky,
pbonzini, vbabka, ashish.kalra, liam.merwick, david, vannapurve,
ackerleytng, aik, ira.weiny, yan.y.zhao, pankaj.gupta, Kai Huang
In-Reply-To: <aWk9PusYNW0iADuD@google.com>
On Thu, Jan 15, 2026 at 11:17:18AM -0800, Sean Christopherson wrote:
> On Thu, Jan 08, 2026, Michael Roth wrote:
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index fdaea3422c30..9dafa44838fe 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -151,6 +151,15 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> > mapping_gfp_mask(inode->i_mapping), policy);
> > mpol_cond_put(policy);
> >
> > + /*
> > + * External interfaces like kvm_gmem_get_pfn() support dealing
> > + * with hugepages to a degree, but internally, guest_memfd currently
> > + * assumes that all folios are order-0 and handling would need
> > + * to be updated for anything otherwise (e.g. page-clearing
> > + * operations).
> > + */
> > + WARN_ON_ONCE(folio_order(folio));
>
> Gah, this is buggy. __filemap_get_folio_mpol() can return an ERR_PTR(). If that
> happens, this WARN will dereference garbage and explode.
>
> And of course I find it _just_ after sending thank yous, *sigh*.
>
> I'll squash to this (after testing):
>
> WARN_ON_ONCE(!IS_ERR(folio) && folio_order(folio));
>
> avoiding a few emails isn't worth having a lurking bug.
Thanks for the catch. FWIW I double-checked that kvm_gmem_get_folio() always
returns an ERR_PTR() instead of NULL directly, so the suggested change looks
good to me.
-Mike
^ permalink raw reply
* Re: [PATCH v3 0/6] KVM: guest_memfd: Rework preparation/population flows in prep for in-place conversion
From: Sean Christopherson @ 2026-01-15 21:07 UTC (permalink / raw)
To: kvm, Michael Roth
Cc: linux-coco, linux-mm, linux-kernel, thomas.lendacky, pbonzini,
vbabka, ashish.kalra, liam.merwick, vannapurve, ackerleytng, aik,
ira.weiny, yan.y.zhao, pankaj.gupta, David Hildenbrand
In-Reply-To: <176849903000.720660.2401438098975748028.b4-ty@google.com>
On Thu, Jan 15, 2026, Sean Christopherson wrote:
> On Thu, 08 Jan 2026 15:46:16 -0600, Michael Roth wrote:
> > This patchset is also available at:
> >
> > https://github.com/AMDESE/linux/tree/gmem-populate-rework-v3
> >
> > and is based on top of kvm/next (0499add8efd7)
> >
> >
> > [...]
>
> Applied to kvm-x86 gmem, with the tweaked logic I suggested. Thanks!
>
> [1/6] KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()
> https://github.com/kvm-x86/linux/commit/60b590de8b30
> [2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
> https://github.com/kvm-x86/linux/commit/0726d3e164f1
> [3/6] KVM: guest_memfd: Remove preparation tracking
> https://github.com/kvm-x86/linux/commit/188349ceb0f0
> [4/6] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
> https://github.com/kvm-x86/linux/commit/b2e648758038
> [5/6] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
> https://github.com/kvm-x86/linux/commit/894c3cc35b89
> [6/6] KVM: guest_memfd: GUP source pages prior to populating guest memory
> https://github.com/kvm-x86/linux/commit/ba375af3d04d
New hashes after fixing the IS_ERR() goof:
[1/6] KVM: SVM: Fix a missing kunmap_local() in sev_gmem_post_populate()
https://github.com/kvm-x86/linux/commit/60b590de8b30
[2/6] KVM: guest_memfd: Remove partial hugepage handling from kvm_gmem_populate()
https://github.com/kvm-x86/linux/commit/6538b6221cc2
[3/6] KVM: guest_memfd: Remove preparation tracking
https://github.com/kvm-x86/linux/commit/8622ef05709f
[4/6] KVM: SEV: Document/enforce page-alignment for KVM_SEV_SNP_LAUNCH_UPDATE
https://github.com/kvm-x86/linux/commit/dcbcc2323c80
[5/6] KVM: TDX: Document alignment requirements for KVM_TDX_INIT_MEM_REGION
https://github.com/kvm-x86/linux/commit/189fd1b059a9
[6/6] KVM: guest_memfd: GUP source pages prior to populating guest memory
https://github.com/kvm-x86/linux/commit/2a62345b3052
^ permalink raw reply
* [PATCH v2] KVM: TDX: Allow userspace to return errors to guest for MAPGPA
From: Sagi Shahar @ 2026-01-15 22:52 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Dave Hansen, Kiryl Shutsemau,
Rick Edgecombe
Cc: Thomas Gleixner, Borislav Petkov, H. Peter Anvin, x86, kvm,
linux-kernel, linux-coco, Vishal Annapurve, Sagi Shahar
From: Vishal Annapurve <vannapurve@google.com>
MAPGPA request from TDX VMs gets split into chunks by KVM using a loop
of userspace exits until the complete range is handled.
In some cases userspace VMM might decide to break the MAPGPA operation
and continue it later. For example: in the case of intrahost migration
userspace might decide to continue the MAPGPA operation after the
migration is completed.
Allow userspace to signal to TDX guests that the MAPGPA operation should
be retried the next time the guest is scheduled.
This is potentially a breaking change since if userspace sets
hypercall.ret to a value other than EBUSY or EINVAL an EINVAL error code
will be returned to userspace. As of now QEMU never sets hypercall.ret
to a non-zero value after handling KVM_EXIT_HYPERCALL so this change
should be safe.
Signed-off-by: Vishal Annapurve <vannapurve@google.com>
Co-developed-by: Sagi Shahar <sagis@google.com>
Signed-off-by: Sagi Shahar <sagis@google.com>
---
arch/x86/kvm/vmx/tdx.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 2d7a4d52ccfb..9bd4ffbdfecf 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -1189,7 +1189,13 @@ static int tdx_complete_vmcall_map_gpa(struct kvm_vcpu *vcpu)
struct vcpu_tdx *tdx = to_tdx(vcpu);
if (vcpu->run->hypercall.ret) {
- tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
+ if (vcpu->run->hypercall.ret == EAGAIN)
+ tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_RETRY);
+ else if (vcpu->run->hypercall.ret == EINVAL)
+ tdvmcall_set_return_code(vcpu, TDVMCALL_STATUS_INVALID_OPERAND);
+ else
+ return -EINVAL;
+
tdx->vp_enter_args.r11 = tdx->map_gpa_next;
return 1;
}
--
2.52.0.457.g6b5491de43-goog
^ permalink raw reply related
* CCC TEE I/O Cancelled
From: dan.j.williams @ 2026-01-16 2:40 UTC (permalink / raw)
To: linux-coco
Note that the TEE I/O call in the CCC tonight is cancelled. We will
start that series back up for 2026 next week.
Apologies for the thrash.
^ permalink raw reply
* Re: [PATCH v2 07/21] coco/tdx-host: Expose P-SEAMLDR information via sysfs
From: Chao Gao @ 2026-01-16 3:15 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen
In-Reply-To: <aWb2aW063ADCxUyd@yilunxu-OptiPlex-7050>
On Wed, Jan 14, 2026 at 09:50:33AM +0800, Xu Yilun wrote:
>On Tue, Sep 30, 2025 at 07:52:51PM -0700, Chao Gao wrote:
>> TDX Module updates require userspace to select the appropriate module
>> to load. Expose necessary information to facilitate this decision. Two
>> values are needed:
>>
>> - P-SEAMLDR version: for compatibility checks between TDX Module and
>> P-SEAMLDR
>> - num_remaining_updates: indicates how many updates can be performed
>>
>> Expose them as tdx-host device attributes.
>>
>> Note that P-SEAMLDR sysfs nodes are hidden when INTEL_TDX_MODULE_UPDATE
>> isn't enabled or when P-SEAMLDR isn't loaded by BIOS, both of which
>
>I don't think we need to worry about whether P-SEAMLDR is loaded or not.
>The tdx-host device exists only if TDX Module is loaded, and in turn
>P-SEAMLDR is loaded.
Yes, you are right.
<snip>
>> +static umode_t seamldr_group_is_visible(struct kobject *kobj,
>> + struct attribute *attr, int n)
>> +{
>> + return seamldr_get_info() ? attr->mode : 0;
>
>I feel it is a little wierd here, need some explaination why use
>seamldr_get_info() for visibility. At first glance, I get the impression
>that we don't expose the attributes on 1st seamldr_get_info() failure,
>and if 1st read success we expose the attributes, then we return read
>failure on 2nd seamldr_get_info() failure. That's the motivation I'm
>trying to make the logic simpler.
>
>As you said, the purpose of using seamldr_get_info() here is for the 2
>checks:
>
> 1. If INTEL_TDX_MODULE_UPDATE is selected.
> 2. If P-SEAMLOAD exists.
>
>But P-SEAMLOAD must exist in tdx-host device context. The chain of
>dependency is P-SEAMLOAD->TDX Module->tdx host device.
Indeed, and the suggested changes below look good to me.
Thanks.
^ permalink raw reply
* Re: [PATCH v2 08/21] coco/tdx-host: Implement FW_UPLOAD sysfs ABI for TDX Module updates
From: Chao Gao @ 2026-01-16 3:31 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, H. Peter Anvin, Kirill A. Shutemov
In-Reply-To: <aWcIt9AUMo2N9RKw@yilunxu-OptiPlex-7050>
>> +struct tdx_fw_upload_status {
>> + bool cancel_request;
>> +};
>> +
>> +struct fw_upload *tdx_fwl;
>> +static struct tdx_fw_upload_status tdx_fw_upload_status;
>> +
>> static struct faux_device *fdev;
>
>Make the fdev declaration right before tdx_host_init(), try best to keep
>the update stuff in one bluk.
ok.
>
>[...]
>
>> +static int seamldr_init(struct device *dev)
>> +{
>> + const struct seamldr_info *seamldr_info = seamldr_get_info();
>> + const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
>> + int ret;
>> +
>> + if (!tdx_sysinfo || !seamldr_info)
>> + return -ENXIO;
>> +
>> + if (!tdx_supports_runtime_update(tdx_sysinfo)) {
>> + pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
>> + return -EOPNOTSUPP;
>
>I don't think we fail out the whole tdx-host here. We should skip the
>optional feature if it is not supported to allow other features work.
>E.g. the TDX Module version, the P-SEAMLOAD version, TDX Connect.
Yes. How about making seamldr_init() return void? Then any failure in setting
up TDX module update won't impact other features of the tdx-host device. e.g.,
diff --git a/drivers/virt/coco/tdx-host/tdx-host.c b/drivers/virt/coco/tdx-host/tdx-host.c
index 540f9af7f81c..d653c594bb94 100644
--- a/drivers/virt/coco/tdx-host/tdx-host.c
+++ b/drivers/virt/coco/tdx-host/tdx-host.c
@@ -176,32 +176,22 @@ static const struct fw_upload_ops tdx_fw_ops = {
.cancel = tdx_fw_cancel,
};
-static int seamldr_init(struct device *dev)
+static void seamldr_init(struct device *dev)
{
- const struct seamldr_info *seamldr_info = seamldr_get_info();
const struct tdx_sys_info *tdx_sysinfo = tdx_get_sysinfo();
int ret;
- if (!tdx_sysinfo || !seamldr_info)
- return -ENXIO;
+ if (WARN_ON_ONCE(!tdx_sysinfo))
+ return;
- if (!tdx_supports_runtime_update(tdx_sysinfo)) {
+ if (!tdx_supports_runtime_update(tdx_sysinfo))
pr_info("Current TDX Module cannot be updated. Consider BIOS updates\n");
- return -EOPNOTSUPP;
- }
-
- if (!seamldr_info->num_remaining_updates) {
- pr_info("P-SEAMLDR doesn't support TDX Module updates\n");
- return -EOPNOTSUPP;
- }
tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
&tdx_fw_ops, &tdx_fw_upload_status);
ret = PTR_ERR_OR_ZERO(tdx_fwl);
if (ret)
pr_err("failed to register module uploader %d\n", ret);
-
- return ret;
}
static void seamldr_deinit(void)
@@ -212,8 +202,8 @@ static void seamldr_deinit(void)
static int tdx_host_probe(struct faux_device *fdev)
{
- /* Only support TDX Module updates now. More TDX features could be added here. */
- return seamldr_init(&fdev->dev);
+ seamldr_init(&fdev->dev);
+ return 0;
}
static void tdx_host_remove(struct faux_device *fdev)
>
>> + }
>> +
>> + if (!seamldr_info->num_remaining_updates) {
>> + pr_info("P-SEAMLDR doesn't support TDX Module updates\n");
>> + return -EOPNOTSUPP;
>> + }
>
>Ditto. And keeping num_remaining_updates sysfs node visible and returning 0
>is valuable, it clearly tells why update is impossible and aligns with
>the situation when the user keeps on updating and exhausts the available
>updates.
Makes sense. I will drop this check.
>
>> +
>> + tdx_fwl = firmware_upload_register(THIS_MODULE, dev, "seamldr_upload",
>> + &tdx_fw_ops, &tdx_fw_upload_status);
>> + ret = PTR_ERR_OR_ZERO(tdx_fwl);
>> + if (ret)
>> + pr_err("failed to register module uploader %d\n", ret);
>> +
>> + return ret;
>> +}
>
^ permalink raw reply related
* Re: [PATCH v2 11/21] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-01-16 6:14 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aWc7bq+EiQiME8nl@yilunxu-OptiPlex-7050>
On Wed, Jan 14, 2026 at 02:45:02PM +0800, Xu Yilun wrote:
>> +/* Allocate and populate a seamldr_params */
>> +static struct seamldr_params *alloc_seamldr_params(const void *module, int module_size,
>> + const void *sig, int sig_size)
>> +{
>> + struct seamldr_params *params;
>> + const u8 *ptr;
>> + int i;
>> +
>> + BUILD_BUG_ON(sizeof(struct seamldr_params) != SZ_4K);
>> + if (module_size > SEAMLDR_MAX_NR_MODULE_4KB_PAGES * SZ_4K)
>> + return ERR_PTR(-EINVAL);
>> +
>> + if (!IS_ALIGNED(module_size, SZ_4K) || !IS_ALIGNED(sig_size, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)module, SZ_4K) ||
>> + !IS_ALIGNED((unsigned long)sig, SZ_4K))
>> + return ERR_PTR(-EINVAL);
>> +
>> + /* seamldr_params accepts one 4KB-page for sigstruct */
>> + if (sig_size != SZ_4K)
>
>Why we check both IS_ALIGNED(sig_size, SZ_4K) and sig_size != SZ_4K, I
>assume the former is redundant.
Yes. it is redundant.
But in the next version, I will implement an extension that increases the
sigstruct size limit from a single 4KB page to four 4KB pages, so this will
become:
/* P-SEAMLDR accepts up to 4 4KB pages for sigstruct */
if (sig_size > 4 * SZ_4K)
return ERR_PTR(-EINVAL);
>
>> + return ERR_PTR(-EINVAL);
>> +
>> + params = (struct seamldr_params *)get_zeroed_page(GFP_KERNEL);
>> + if (!params)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + params->scenario = SEAMLDR_SCENARIO_UPDATE;
>> + params->sigstruct_pa = (vmalloc_to_pfn(sig) << PAGE_SHIFT) +
>
>This void * buffer comes from FW_UPLOAD callback, which is a kvmalloc
>buffer, so we do vmalloc_to_pfn() here. But that knowledge resides in
>FW_UPLOAD driver context, the kAPI entry, seamldr_install_module()
>doesn't say that. So could we add the kernel-doc to specify this
>"const u8 *data" at ...
Good suggestion. Will do.
^ permalink raw reply
* Re: SVSM Development Call January 14, 2026
From: Jörg Rödel @ 2026-01-16 8:40 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <tlhepq4dp47rbq3vpzm4qtf65t62fqe26tk736sd3ulcbwcefy@btzt3fsapu5e>
Meeting minutes are here:
https://github.com/coconut-svsm/governance/pull/92
Have fun!
-Joerg
^ permalink raw reply
* [PATCH v6] virt: tdx-guest: Return error for GetQuote failures
From: Kuppuswamy Sathyanarayanan @ 2026-01-16 23:03 UTC (permalink / raw)
To: Kirill A . Shutemov, x86
Cc: Rick Edgecombe, Dave Hansen, Dan Williams, linux-kernel,
linux-coco
Currently, the GetQuote request handler returns explicit errors for
hypercall-level failures and timeouts, but it ignores some VMM
failures (e.g., GET_QUOTE_SERVICE_UNAVAILABLE), for which it returns
success with a zero-length Quote. This makes error handling in
userspace more complex.
The VMM reports failures via the status field in the shared GPA header,
which is inaccessible to userspace because only the Quote payload is
exposed to userspace. Parse the status field in the kernel and return
an error for Quote failures.
This preserves existing ABI behavior as userspace already treats a
zero-length Quote as a failure.
Refer to GHCI specification [1], section "TDG.VP.VMCALL <GetQuote>",
Table 3-10 and Table 3-11 for details on the GPA header and
GetQuote status codes.
Fixes: f4738f56d1dc ("virt: tdx-guest: Add Quote generation support using TSM_REPORTS")
Reported-by: Xiaoyao Li <xiaoyao.li@intel.com>
Closes: https://lore.kernel.org/linux-coco/6bdf569c-684a-4459-af7c-4430691804eb@linux.intel.com/T/#u
Closes: https://github.com/confidential-containers/guest-components/issues/823
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Tested-by: Mikko Ylinen <mikko.ylinen@linux.intel.com>
Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Kai Huang <kai.huang@intel.com>
Link: https://cdrdv2.intel.com/v1/dl/getContent/858626 # [1]
---
Changes since v5:
* Simplified the commit message to focus on the constraint of
GPA header inaccessibility (Dave).
* Reordered commit tags as per kernel process (Dave).
* Refined the commit title to reflect the fix.
* Replaced pr_err() with pr_debug() for error log to avoid dmesg spam.
* Added Link to GHCI spec.
Changes since v4:
* Rebased on top of v6.18-rc1
* Added Tested-by tag from Mikko.
* Added more details in commit log to clarify no user impact and also
link to a related github issue.
* Added error message for the failed case.
Changes since v3:
* Rebased on top of v6.9-rc1
* Added Dan's Reviewed-by tag.
Changes since v2:
* Updated the commit log (Dan)
* Removed pr_err message.
Changes since v1:
* Updated the commit log (Kirill)
drivers/virt/coco/tdx-guest/tdx-guest.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/virt/coco/tdx-guest/tdx-guest.c b/drivers/virt/coco/tdx-guest/tdx-guest.c
index 4e239ec960c9..a3c8f5c19bae 100644
--- a/drivers/virt/coco/tdx-guest/tdx-guest.c
+++ b/drivers/virt/coco/tdx-guest/tdx-guest.c
@@ -304,6 +304,11 @@ static int tdx_report_new_locked(struct tsm_report *report, void *data)
return ret;
}
+ if (quote_buf->status != GET_QUOTE_SUCCESS) {
+ pr_debug("GetQuote request failed, status:%llx\n", quote_buf->status);
+ return -EIO;
+ }
+
buf = kvmemdup(quote_buf->data, quote_buf->out_len, GFP_KERNEL);
if (!buf)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Sean Christopherson @ 2026-01-16 23:17 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, tglx, vannapurve, x86,
yan.y.zhao, xiaoyao.li, binbin.wu, Kirill A. Shutemov
In-Reply-To: <20251121005125.417831-8-rick.p.edgecombe@intel.com>
On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> +/*
> + * Return a page that can be used as TDX private memory
> + * and obtain TDX protections.
Wrap at ~80.
This comment is also misleading, arguably wrong. Because from KVM's perspective,
these APIs are _never_ used to back TDX private memory. They are used only for
control pages, which yeah, I suppose might be encrypted with the guest's private
key, but most readers will interpret "used as TDX private memory" to mean that
these are _the_ source of pages for guest private memory.
> + */
> +struct page *tdx_alloc_page(void)
And in a similar vein, given terminology in other places, maybe call these
tdx_{alloc,free}_control_page()?
> +{
> + struct page *page;
> +
> + page = alloc_page(GFP_KERNEL);
GFP_KERNEL_ACCOUNT, all of these allocations are tied to a VM.
> + if (!page)
> + return NULL;
> +
> + if (tdx_pamt_get(page)) {
> + __free_page(page);
> + return NULL;
> + }
> +
> + return page;
> +}
> +EXPORT_SYMBOL_GPL(tdx_alloc_page);
Note, these can all now be EXPORT_SYMBOL_FOR_KVM.
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Edgecombe, Rick P @ 2026-01-16 23:25 UTC (permalink / raw)
To: seanjc@google.com
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Huang, Kai,
Li, Xiaoyao, Hansen, Dave, Zhao, Yan Y, Wu, Binbin,
kas@kernel.org, linux-kernel@vger.kernel.org, mingo@redhat.com,
pbonzini@redhat.com, tglx@linutronix.de, Yamahata, Isaku,
kirill.shutemov@linux.intel.com, Annapurve, Vishal, Gao, Chao,
bp@alien8.de, x86@kernel.org
In-Reply-To: <aWrHAeMcjDpVnTBp@google.com>
On Fri, 2026-01-16 at 15:17 -0800, Sean Christopherson wrote:
> On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> > +/*
> > + * Return a page that can be used as TDX private memory
> > + * and obtain TDX protections.
>
> Wrap at ~80.
>
> This comment is also misleading, arguably wrong. Because from KVM's perspective,
> these APIs are _never_ used to back TDX private memory. They are used only for
> control pages, which yeah, I suppose might be encrypted with the guest's private
> key, but most readers will interpret "used as TDX private memory" to mean that
> these are _the_ source of pages for guest private memory.
Maybe just drop "private" and call it TDX memory?
>
> > + */
> > +struct page *tdx_alloc_page(void)
>
> And in a similar vein, given terminology in other places, maybe call these
> tdx_{alloc,free}_control_page()?
True, that is the only use today, but also there is nothing control page
specific about the functions themselves. I'm ok changing it, but I'm not sure it
helps that much.
>
> > +{
> > + struct page *page;
> > +
> > + page = alloc_page(GFP_KERNEL);
>
> GFP_KERNEL_ACCOUNT, all of these allocations are tied to a VM.
Yes, thanks.
>
> > + if (!page)
> > + return NULL;
> > +
> > + if (tdx_pamt_get(page)) {
> > + __free_page(page);
> > + return NULL;
> > + }
> > +
> > + return page;
> > +}
> > +EXPORT_SYMBOL_GPL(tdx_alloc_page);
>
> Note, these can all now be EXPORT_SYMBOL_FOR_KVM.
Sure.
^ permalink raw reply
* Re: [PATCH v4 07/16] x86/virt/tdx: Add tdx_alloc/free_page() helpers
From: Dave Hansen @ 2026-01-16 23:40 UTC (permalink / raw)
To: Sean Christopherson, Rick Edgecombe
Cc: bp, chao.gao, isaku.yamahata, kai.huang, kas, kvm, linux-coco,
linux-kernel, mingo, pbonzini, tglx, vannapurve, x86, yan.y.zhao,
xiaoyao.li, binbin.wu, Kirill A. Shutemov
In-Reply-To: <aWrHAeMcjDpVnTBp@google.com>
On 1/16/26 15:17, Sean Christopherson wrote:
>> +struct page *tdx_alloc_page(void)
> And in a similar vein, given terminology in other places, maybe call these
> tdx_{alloc,free}_control_page()?
Ack on the naming, especially if they're never used as normal guest
memory and are *only* for TDX module metadata that the guest never
actually sees.
^ permalink raw reply
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Sean Christopherson @ 2026-01-17 0:53 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, tglx, vannapurve, x86,
yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-12-rick.p.edgecombe@intel.com>
On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> Move mmu_external_spt_cache behind x86 ops.
>
> In the mirror/external MMU concept, the KVM MMU manages a non-active EPT
> tree for private memory (the mirror). The actual active EPT tree the
> private memory is protected inside the TDX module. Whenever the mirror EPT
> is changed, it needs to call out into one of a set of x86 opts that
> implement various update operation with TDX specific SEAMCALLs and other
> tricks. These implementations operate on the TDX S-EPT (the external).
>
> In reality these external operations are designed narrowly with respect to
> TDX particulars. On the surface, what TDX specific things are happening to
> fulfill these update operations are mostly hidden from the MMU, but there
> is one particular area of interest where some details leak through.
>
> The S-EPT needs pages to use for the S-EPT page tables. These page tables
> need to be allocated before taking the mmu lock, like all the rest. So the
> KVM MMU pre-allocates pages for TDX to use for the S-EPT in the same place
> where it pre-allocates the other page tables. It’s not too bad and fits
> nicely with the others.
>
> However, Dynamic PAMT will need even more pages for the same operations.
> Further, these pages will need to be handed to the arch/x86 side which used
> them for DPAMT updates, which is hard for the existing KVM based cache.
> The details living in core MMU code start to add up.
>
> So in preparation to make it more complicated, move the external page
> table cache into TDX code by putting it behind some x86 ops. Have one for
> topping up and one for allocation. Don’t go so far to try to hide the
> existence of external page tables completely from the generic MMU, as they
> are currently stored in their mirror struct kvm_mmu_page and it’s quite
> handy.
>
> To plumb the memory cache operations through tdx.c, export some of
> the functions temporarily. This will be removed in future changes.
>
> Acked-by: Kiryl Shutsemau <kas@kernel.org>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
NAK. I kinda sorta get why you did this? But the pages KVM uses for page tables
are KVM's, not to be mixed with PAMT pages.
Eww. Definitely a hard "no". In tdp_mmu_alloc_sp_for_split(), the allocation
comes from KVM:
if (mirror) {
sp->external_spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
if (!sp->external_spt) {
free_page((unsigned long)sp->spt);
kmem_cache_free(mmu_page_header_cache, sp);
return NULL;
}
}
But then in kvm_tdp_mmu_map(), via kvm_mmu_alloc_external_spt(), the allocation
comes from get_tdx_prealloc_page()
static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
{
struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
if (WARN_ON_ONCE(!page))
return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
return page_address(page);
}
But then regardles of where the page came from, KVM frees it. Seriously.
static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
{
free_page((unsigned long)sp->external_spt); <=====
free_page((unsigned long)sp->spt);
kmem_cache_free(mmu_page_header_cache, sp);
}
Oh, and the hugepage series also fumbles its topup (why there's yet another
topup API, I have no idea).
static int tdx_topup_vm_split_cache(struct kvm *kvm, enum pg_level level)
{
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
struct tdx_prealloc *prealloc = &kvm_tdx->prealloc_split_cache;
int cnt = tdx_min_split_cache_sz(kvm, level);
while (READ_ONCE(prealloc->cnt) < cnt) {
struct page *page = alloc_page(GFP_KERNEL); <==== GFP_KERNEL_ACCOUNT
if (!page)
return -ENOMEM;
spin_lock(&kvm_tdx->prealloc_split_cache_lock);
list_add(&page->lru, &prealloc->page_list);
prealloc->cnt++;
spin_unlock(&kvm_tdx->prealloc_split_cache_lock);
}
return 0;
}
^ permalink raw reply
* Re: [PATCH v4 12/16] x86/virt/tdx: Add helpers to allow for pre-allocating pages
From: Sean Christopherson @ 2026-01-17 1:02 UTC (permalink / raw)
To: Rick Edgecombe
Cc: bp, chao.gao, dave.hansen, isaku.yamahata, kai.huang, kas, kvm,
linux-coco, linux-kernel, mingo, pbonzini, tglx, vannapurve, x86,
yan.y.zhao, xiaoyao.li, binbin.wu
In-Reply-To: <20251121005125.417831-13-rick.p.edgecombe@intel.com>
On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> ---
> v4:
> - Change to GFP_KERNEL_ACCOUNT to match replaced kvm_mmu_memory_cache
> - Add GFP_ATOMIC backup, like kvm_mmu_memory_cache has (Kiryl)
LOL, having fun reinventing kvm_mmu_memory_cache? :-D
> - Explain why not to use mempool (Dave)
> - Tweak local vars to be more reverse christmas tree by deleting some
> that were only added for reasons that go away in this patch anyway
> ---
> arch/x86/include/asm/tdx.h | 43 ++++++++++++++++++++++++++++++++++++-
> arch/x86/kvm/vmx/tdx.c | 21 +++++++++++++-----
> arch/x86/kvm/vmx/tdx.h | 2 +-
> arch/x86/virt/vmx/tdx/tdx.c | 22 +++++++++++++------
> virt/kvm/kvm_main.c | 3 ---
> 5 files changed, 75 insertions(+), 16 deletions(-)
> +/*
> + * Simple structure for pre-allocating Dynamic
> + * PAMT pages outside of locks.
As called out in an earlier patch, it's not just PAMT pages.
> + */
> +struct tdx_prealloc {
> + struct list_head page_list;
> + int cnt;
> +};
> +
> +static inline struct page *get_tdx_prealloc_page(struct tdx_prealloc *prealloc)
> +{
> + struct page *page;
> +
> + page = list_first_entry_or_null(&prealloc->page_list, struct page, lru);
> + if (page) {
> + list_del(&page->lru);
> + prealloc->cnt--;
> + }
> +
> + return page;
> +}
> +
> +static inline int topup_tdx_prealloc_page(struct tdx_prealloc *prealloc, unsigned int min_size)
> +{
> + while (prealloc->cnt < min_size) {
> + struct page *page = alloc_page(GFP_KERNEL_ACCOUNT);
> +
> + if (!page)
> + return -ENOMEM;
> +
> + list_add(&page->lru, &prealloc->page_list);
Huh, TIL that page->lru is fair game for private usage when the page is kernel-
allocated.
> + prealloc->cnt++;
>
> static int tdx_topup_external_fault_cache(struct kvm_vcpu *vcpu, unsigned int cnt)
> {
> - struct vcpu_tdx *tdx = to_tdx(vcpu);
> + struct tdx_prealloc *prealloc = &to_tdx(vcpu)->prealloc;
> + int min_fault_cache_size;
>
> - return kvm_mmu_topup_memory_cache(&tdx->mmu_external_spt_cache, cnt);
> + /* External page tables */
> + min_fault_cache_size = cnt;
> + /* Dynamic PAMT pages (if enabled) */
> + min_fault_cache_size += tdx_dpamt_entry_pages() * PT64_ROOT_MAX_LEVEL;
> +
> + return topup_tdx_prealloc_page(prealloc, min_fault_cache_size);
> }
>
> static void tdx_free_external_fault_cache(struct kvm_vcpu *vcpu)
> {
> struct vcpu_tdx *tdx = to_tdx(vcpu);
> + struct page *page;
>
> - kvm_mmu_free_memory_cache(&tdx->mmu_external_spt_cache);
> + while ((page = get_tdx_prealloc_page(&tdx->prealloc)))
> + __free_page(page);
No. Either put the ownership of the PAMT cache in arch/x86/virt/vmx/tdx/tdx.c
or use kvm_mmu_memory_cache. Don't add a custom caching scheme in KVM.
> /* Number PAMT pages to be provided to TDX module per 2M region of PA */
> -static int tdx_dpamt_entry_pages(void)
> +int tdx_dpamt_entry_pages(void)
> {
> if (!tdx_supports_dynamic_pamt(&tdx_sysinfo))
> return 0;
>
A comment here stating the "common" number of entries would be helper. I have no
clue as to the magnitude. E.g. this could be 2 or it could be 200, I genuinely
have no idea.
^ permalink raw reply
* Re: [PATCH v2 17/21] x86/virt/seamldr: Install a new TDX Module
From: Chao Gao @ 2026-01-19 0:28 UTC (permalink / raw)
To: Xu Yilun
Cc: linux-coco, linux-kernel, x86, reinette.chatre, ira.weiny,
kai.huang, dan.j.williams, sagis, vannapurve, paulmck,
nik.borisov, Farrah Chen, Kirill A. Shutemov, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin
In-Reply-To: <aWiGA7sOPEs+7ifs@yilunxu-OptiPlex-7050>
On Thu, Jan 15, 2026 at 02:15:31PM +0800, Xu Yilun wrote:
>> static int do_seamldr_install_module(void *params)
>> {
>> + struct tdx_module_args args = { .rcx = __pa(params) };
>
>Is it better we put the definition, or at least the value assignment in
>case TDP_CPU_INSTALL? This pattern always appears here for a seamcall
>wrapper but this function is far more complex than that.
>
>And the .rcx = __pa(params) also confuse me a bit. Better we name it
>e.g. seamldr_params which looks reasonable for seamcall arguments.
Sounds good. I will do the following changes:
diff --git a/arch/x86/virt/vmx/tdx/seamldr.c b/arch/x86/virt/vmx/tdx/seamldr.c
index a0b59d6c53c9..f2933c7e3852 100644
--- a/arch/x86/virt/vmx/tdx/seamldr.c
+++ b/arch/x86/virt/vmx/tdx/seamldr.c
@@ -313,10 +313,10 @@ static void print_update_failure_message(void)
* See multi_cpu_stop() from where this multi-cpu state-machine was
* adopted, and the rationale for touch_nmi_watchdog()
*/
-static int do_seamldr_install_module(void *params)
+static int do_seamldr_install_module(void *seamldr_params)
{
- struct tdx_module_args args = { .rcx = __pa(params) };
enum tdp_state newstate, curstate = TDP_START;
+ struct tdx_module_args args = {};
int cpu = smp_processor_id();
bool primary;
int ret = 0;
@@ -336,6 +336,7 @@ static int do_seamldr_install_module(void *params)
ret = tdx_module_shutdown();
break;
case TDP_CPU_INSTALL:
+ args.rcx = __pa(seamldr_params);
scoped_guard(raw_spinlock, &seamldr_lock)
ret = seamldr_call(P_SEAMLDR_INSTALL, &args);
break;
^ permalink raw reply related
* Re: [PATCH v4 11/16] KVM: TDX: Add x86 ops for external spt cache
From: Yan Zhao @ 2026-01-19 2:31 UTC (permalink / raw)
To: Sean Christopherson
Cc: Rick Edgecombe, bp, chao.gao, dave.hansen, isaku.yamahata,
kai.huang, kas, kvm, linux-coco, linux-kernel, mingo, pbonzini,
tglx, vannapurve, x86, xiaoyao.li, binbin.wu
In-Reply-To: <aWrdpZCCDDAffZRM@google.com>
On Fri, Jan 16, 2026 at 04:53:57PM -0800, Sean Christopherson wrote:
> On Thu, Nov 20, 2025, Rick Edgecombe wrote:
> > Move mmu_external_spt_cache behind x86 ops.
> >
> > In the mirror/external MMU concept, the KVM MMU manages a non-active EPT
> > tree for private memory (the mirror). The actual active EPT tree the
> > private memory is protected inside the TDX module. Whenever the mirror EPT
> > is changed, it needs to call out into one of a set of x86 opts that
> > implement various update operation with TDX specific SEAMCALLs and other
> > tricks. These implementations operate on the TDX S-EPT (the external).
> >
> > In reality these external operations are designed narrowly with respect to
> > TDX particulars. On the surface, what TDX specific things are happening to
> > fulfill these update operations are mostly hidden from the MMU, but there
> > is one particular area of interest where some details leak through.
> >
> > The S-EPT needs pages to use for the S-EPT page tables. These page tables
> > need to be allocated before taking the mmu lock, like all the rest. So the
> > KVM MMU pre-allocates pages for TDX to use for the S-EPT in the same place
> > where it pre-allocates the other page tables. It’s not too bad and fits
> > nicely with the others.
> >
> > However, Dynamic PAMT will need even more pages for the same operations.
> > Further, these pages will need to be handed to the arch/x86 side which used
> > them for DPAMT updates, which is hard for the existing KVM based cache.
> > The details living in core MMU code start to add up.
> >
> > So in preparation to make it more complicated, move the external page
> > table cache into TDX code by putting it behind some x86 ops. Have one for
> > topping up and one for allocation. Don’t go so far to try to hide the
> > existence of external page tables completely from the generic MMU, as they
> > are currently stored in their mirror struct kvm_mmu_page and it’s quite
> > handy.
> >
> > To plumb the memory cache operations through tdx.c, export some of
> > the functions temporarily. This will be removed in future changes.
> >
> > Acked-by: Kiryl Shutsemau <kas@kernel.org>
> > Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> > ---
>
> NAK. I kinda sorta get why you did this? But the pages KVM uses for page tables
> are KVM's, not to be mixed with PAMT pages.
>
> Eww. Definitely a hard "no". In tdp_mmu_alloc_sp_for_split(), the allocation
> comes from KVM:
>
> if (mirror) {
> sp->external_spt = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> if (!sp->external_spt) {
> free_page((unsigned long)sp->spt);
> kmem_cache_free(mmu_page_header_cache, sp);
> return NULL;
> }
> }
>
> But then in kvm_tdp_mmu_map(), via kvm_mmu_alloc_external_spt(), the allocation
> comes from get_tdx_prealloc_page()
>
> static void *tdx_alloc_external_fault_cache(struct kvm_vcpu *vcpu)
> {
> struct page *page = get_tdx_prealloc_page(&to_tdx(vcpu)->prealloc);
>
> if (WARN_ON_ONCE(!page))
> return (void *)__get_free_page(GFP_ATOMIC | __GFP_ACCOUNT);
>
> return page_address(page);
> }
>
> But then regardles of where the page came from, KVM frees it. Seriously.
>
> static void tdp_mmu_free_sp(struct kvm_mmu_page *sp)
> {
> free_page((unsigned long)sp->external_spt); <=====
> free_page((unsigned long)sp->spt);
> kmem_cache_free(mmu_page_header_cache, sp);
> }
IMHO, it's by design. I don't see a problem with KVM freeing the sp->external_spt,
regardless of whether it's from:
(1) KVM's mmu cache,
(2) tdp_mmu_alloc_sp_for_split(), or
(3) tdx_alloc_external_fault_cache().
Please correct me if I missed anything.
None of (1)-(3) keeps the pages in list after KVM obtains the pages and maps
them into SPTEs.
So, with SPTEs as the pages' sole consumer, it's perfectly fine for KVM to free
the pages when freeing SPTEs. No?
Also, in the current upstream code, after tdp_mmu_split_huge_pages_root() is
invoked for dirty tracking, some sp->spt are allocated from
tdp_mmu_alloc_sp_for_split(), while others are from kvm_mmu_memory_cache_alloc().
However, tdp_mmu_free_sp() can still free them without any problem.
> Oh, and the hugepage series also fumbles its topup (why there's yet another
> topup API, I have no idea).
Introducing another topup API is because in the hugepage usage, the split occurs
in a non-vCPU context. So, the "kvm_tdx->prealloc_split_cache" is per-VM, since
hugepage cannot reuse tdx_topup_external_fault_cache().
(Please also see the patch log of
"[PATCH v3 19/24] KVM: x86: Introduce per-VM external cache for splitting" [1]).
[1] https://lore.kernel.org/all/20260106102331.25244-1-yan.y.zhao@intel.com.
> static int tdx_topup_vm_split_cache(struct kvm *kvm, enum pg_level level)
> {
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> struct tdx_prealloc *prealloc = &kvm_tdx->prealloc_split_cache;
> int cnt = tdx_min_split_cache_sz(kvm, level);
>
> while (READ_ONCE(prealloc->cnt) < cnt) {
> struct page *page = alloc_page(GFP_KERNEL); <==== GFP_KERNEL_ACCOUNT
Thanks for caching. Will use GFP_KERNEL_ACCOUNT instead.
> if (!page)
> return -ENOMEM;
>
> spin_lock(&kvm_tdx->prealloc_split_cache_lock);
I didn't have tdx_topup_vm_split_cache() reuse topup_tdx_prealloc_page() (as
used by tdx_topup_external_fault_cache()). This is due to the need to add the
spinlock kvm_tdx->prealloc_split_cache_lock to protect page enqueuing and
dequeuing. (I've pasted the explanation of the per-VM external cache for
splitting and its lock protections from [2] below as an "Appendix" for your
convenience).
If we can have the split for huge pages not reuse
tdp_mmu_split_huge_pages_root() (i.e., create a specific interface for
kvm_split_cross_boundary_leafs() instead of reusing
tdp_mmu_split_huge_pages_root(), as I asked about in [3]), we can preallocate
enough pages and hold mmu_lock without releasing it for memory allocation. Then,
this spinlock kvm_tdx->prealloc_split_cache_lock would not be needed.
[2] https://lore.kernel.org/all/20260106101646.24809-1-yan.y.zhao@intel.com
[3] https://lore.kernel.org/all/aW2Iwpuwoyod8eQc@yzhao56-desk.sh.intel.com
> list_add(&page->lru, &prealloc->page_list);
> prealloc->cnt++;
> spin_unlock(&kvm_tdx->prealloc_split_cache_lock);
> }
>
> return 0;
> }
Appendix:
Explanation of the per-VM external cache for splitting huge pages from the
cover letter of huge pages v3 [2]:
"
9. DPAMT
Currently, DPAMT's involvement with TDX huge page is limited to page
splitting.
As shown in the following call stack, DPAMT pages used by splitting are
pre-allocated and queued in the per-VM external split cache. They are
dequeued and consumed in tdx_sept_split_private_spte().
kvm_split_cross_boundary_leafs
kvm_tdp_mmu_gfn_range_split_cross_boundary_leafs
tdp_mmu_split_huge_pages_root
(*) 1) tdp_mmu_alloc_sp_for_split()
+-----2.1) need_topup_external_split_cache(): check if enough pages in
| the external split cache. Go to 3 if pages are enough.
| +--2.2) topup_external_split_cache(): preallocate/enqueue pages in
| | the external split cache.
| | 3) tdp_mmu_split_huge_page
| | tdp_mmu_link_sp
| | tdp_mmu_iter_set_spte
| |(**) tdp_mmu_set_spte
| | split_external_spte
| | kvm_x86_call(split_external_spte)
| | tdx_sept_split_private_spte
| | 3.1) BLOCK, TRACK
+--+-------------------3.2) Dequeue PAMT pages from the external split
| | cache for the new sept page
| | 3.3) PAMT_ADD for the new sept page
+--+-------------------3.4) Dequeue PAMT pages from the external split
cache for the 2MB guest private memory.
3.5) DEMOTE.
3.6) Update PAMT refcount of the 2MB guest private
memory.
(*) The write mmu_lock is held across the checking of enough pages in
cache in step 2.1 and the page dequeuing in steps 3.2 and 3.4, so
it's ensured that dequeuing has enough pages in cache.
(**) A spinlock prealloc_split_cache_lock is used inside the TDX's cache
implementation to protect page enqueuing in step 2.2 and page
dequeuing in steps 3.2 and 3.4.
"
^ permalink raw reply
* Re: [PATCH v2 1/1] PCI/IDE: Fix using wrong VF ID for RID range calculation
From: Xu Yilun @ 2026-01-19 2:21 UTC (permalink / raw)
To: Li Ming; +Cc: helgaas, dan.j.williams, linux-pci, linux-coco, linux-kernel
In-Reply-To: <20260114111455.550984-1-ming.li@zohomail.com>
On Wed, Jan 14, 2026 at 07:14:55PM +0800, Li Ming wrote:
> When allocate a new IDE stream for a PCI device in SR-IOV case, the RID
> range of the new IDE stream should cover all VFs of the device. VF ID
> range of a PCI device is [0, num_VFs - 1], so should use (num_VFs - 1)
> as the last VF's ID.
>
> Fixes: 1e4d2ff3ae45 ("PCI/IDE: Add IDE establishment helpers")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
^ 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