* SVSM Development Call February 25, 2026
From: Jörg Rödel @ 2026-02-24 21:02 UTC (permalink / raw)
To: coconut-svsm, linux-coco
Hi,
Here is the call for agenda items for this weeks SVSM development call. Please
send any agenda items you have in mind as a reply to this email or raise them
in the meeting.
We will use the LF Zoom instance. Details of the meeting can be found in our
governance repository at:
https://github.com/coconut-svsm/governance
The link to the COCONUT-SVSM calendar is:
https://zoom-lfx.platform.linuxfoundation.org/meetings/coconut-svsm?view=week
The meeting will be recorded and the recording eventually published.
Regards,
Jörg
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Sean Christopherson @ 2026-02-24 20:42 UTC (permalink / raw)
To: Rick P Edgecombe
Cc: binbin.wu@linux.intel.com, kvm@vger.kernel.org,
changyuanl@google.com, Binbin Wu, x86@kernel.org, kas@kernel.org,
Xiaoyao Li, hpa@zytor.com, mingo@redhat.com, bp@alien8.de,
pbonzini@redhat.com, tglx@kernel.org, Isaku Yamahata,
linux-kernel@vger.kernel.org, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com
In-Reply-To: <d6820308325d5f8fee7918996ef98ab3f7b6ce6d.camel@intel.com>
On Tue, Feb 24, 2026, Rick P Edgecombe wrote:
> On Tue, 2026-02-24 at 08:03 -0800, Sean Christopherson wrote:
> > > But adding the consistency check here would cause compatibility issue.
> > > Generally, if a new CPUID indexed function is added for some new CPU and
> > > the TDX module reports it, KVM versions without the CPUID function in
> > > the list will trigger the warning.
> >
> > IMO, that's a good thing and working as intended. WARNs aren't inherently
> > evil. While the goal is to be WARN-free, in this case triggering the WARN if
> > the TDX Module is updated (or new silicon arrives) is desirable, because it
> > alerts us to that new behavior, so that we can go update KVM.
> >
> > But we should "fix" 0x23 and 0x24 before landing this patch.
>
> Would we backport those changes then? I would usually think that if the TDX
> module updates in such a way that triggers a warning in the kernel then it's a
> TDX module bug.
To stable@? No, I don't think see any reason to do that.
> I'm still not clear on the impact of this one, but assuming it's not too
> serious, could we discuss the WIP CPUID bit TDX arch stuff in PUCK before doing
> the change?
Sure, I don't see a rush on the patch.
> We were initially focusing on the problem of CPUID bits that affect host state,
> but then recently were discussing how many other categories of potential
> problems we should worry about at this point. So it would be good to understand
> the impact here.
>
> If this warn is a trend towards doubling back on the initial decision to expose
> the CPUID interface to userspace,
Maybe I'm missing something, but I think you're reading into the WARN waaaay too
much. I suggested it purely as a paranoid guard against the TDX Module doing
something bizarre and/or the kernel fat-fingering a CPUID function. I.e. there's
no ulterior motive here, unless maybe Changyuan is planning world domination or
something. :-D
> which I think is still doable and worth considering as an alternative, then
> this also affects how we would want the TDX module changes to work.
^ permalink raw reply
* Re: [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: Jason Gunthorpe @ 2026-02-24 19:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: John Stultz, dri-devel, linaro-mm-sig, iommu, linux-media,
sumit.semwal, benjamin.gaignard, Brian.Starkey, tjmercier,
christian.koenig, m.szyprowski, robin.murphy, leon, sean.anderson,
ptesarik, catalin.marinas, aneesh.kumar, suzuki.poulose,
steven.price, thomas.lendacky, john.allen, ashish.kalra,
suravee.suthikulpanit, linux-coco
In-Reply-To: <5z6d2etfr24oscoxhk3samf2bbhtcz6hymf65cow76omagsplf@6gdaev2perkk>
On Tue, Feb 24, 2026 at 09:32:01AM +0100, Jiri Pirko wrote:
> >Should there be some global list of leaked decrypted pages such that
> >the mm subsystem could try again later to recover these?
>
> swiotlb does the same non-recovery leakage. I belive is it not worth
> implementing this at this time,
Yeah, I agree
Looking at the callers the purpose of the return code is to trigger
the memory leak because there is no way to recover from this. We have
no idea when in future the hypervisor might permit the operation and
we have no way to keep track of the memory until it does.
It is not a great API design at all, it only makes sense from the
hypervisor perspective where it can run out of memory trying to do
these changes..
Jason
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Edgecombe, Rick P @ 2026-02-24 18:45 UTC (permalink / raw)
To: seanjc@google.com, binbin.wu@linux.intel.com
Cc: kvm@vger.kernel.org, changyuanl@google.com, Wu, Binbin,
x86@kernel.org, kas@kernel.org, Li, Xiaoyao, hpa@zytor.com,
mingo@redhat.com, bp@alien8.de, pbonzini@redhat.com,
tglx@kernel.org, Yamahata, Isaku, linux-kernel@vger.kernel.org,
linux-coco@lists.linux.dev, dave.hansen@linux.intel.com
In-Reply-To: <aZ3LxD5XMepnU8jh@google.com>
On Tue, 2026-02-24 at 08:03 -0800, Sean Christopherson wrote:
> > But adding the consistency check here would cause compatibility issue.
> > Generally, if a new CPUID indexed function is added for some new CPU and
> > the TDX module reports it, KVM versions without the CPUID function in
> > the list will trigger the warning.
>
> IMO, that's a good thing and working as intended. WARNs aren't inherently
> evil. While the goal is to be WARN-free, in this case triggering the WARN if
> the TDX Module is updated (or new silicon arrives) is desirable, because it
> alerts us to that new behavior, so that we can go update KVM.
>
> But we should "fix" 0x23 and 0x24 before landing this patch.
Would we backport those changes then? I would usually think that if the TDX
module updates in such a way that triggers a warning in the kernel then it's a
TDX module bug.
I'm still not clear on the impact of this one, but assuming it's not too
serious, could we discuss the WIP CPUID bit TDX arch stuff in PUCK before doing
the change?
We were initially focusing on the problem of CPUID bits that affect host state,
but then recently were discussing how many other categories of potential
problems we should worry about at this point. So it would be good to understand
the impact here.
If this warn is a trend towards doubling back on the initial decision to expose
the CPUID interface to userspace, which I think is still doable and worth
considering as an alternative, then this also affects how we would want the TDX
module changes to work.
^ permalink raw reply
* Re: [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Jim Mattson @ 2026-02-24 18:29 UTC (permalink / raw)
To: Dave Hansen
Cc: Kim Phillips, linux-kernel, kvm, linux-coco, x86,
Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta, stable
In-Reply-To: <6b3b0c86-99eb-406d-b88d-3d71613bef9e@intel.com>
On Tue, Feb 24, 2026 at 10:23 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 2/24/26 10:01, Kim Phillips wrote:
> > @@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
> > if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
> > spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
> > spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
> > - !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> > + !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
> > + boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
> > pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
> > spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
> > }
>
> Didn't we agree to just use the "Intel feature" name?
Aren't they quite different? IIRC, IBRS_ENHANCED protects host
userspace from guest indirect branch steering (i.e. VMSCAPE style
attacks), but AUTOIBRS does not.
^ permalink raw reply
* Re: [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Dave Hansen @ 2026-02-24 18:22 UTC (permalink / raw)
To: Kim Phillips, linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta, stable
In-Reply-To: <20260224180157.725159-2-kim.phillips@amd.com>
On 2/24/26 10:01, Kim Phillips wrote:
> @@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
> if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
> spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
> - !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
> + !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
> + boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
> pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
> spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
> }
Didn't we agree to just use the "Intel feature" name? See this existing
code:
> /*
> * AMD's AutoIBRS is equivalent to Intel's eIBRS - use the Intel feature
> * flag and protect from vendor-specific bugs via the whitelist.
> *
> * Don't use AutoIBRS when SNP is enabled because it degrades host
> * userspace indirect branch performance.
> */
> if ((x86_arch_cap_msr & ARCH_CAP_IBRS_ALL) ||
> (cpu_has(c, X86_FEATURE_AUTOIBRS) &&
> !cpu_feature_enabled(X86_FEATURE_SEV_SNP))) {
> setup_force_cpu_cap(X86_FEATURE_IBRS_ENHANCED);
> if (!cpu_matches(cpu_vuln_whitelist, NO_EIBRS_PBRSB) &&
> !(x86_arch_cap_msr & ARCH_CAP_PBRSB_NO))
> setup_force_cpu_bug(X86_BUG_EIBRS_PBRSB);
> }
You're probably not seeing X86_FEATURE_IBRS_ENHANCED because it doesn't
get forced under SNP.
^ permalink raw reply
* [PATCH 3/3] KVM: SEV: Add support for SNP BTB Isolation
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest. CPU hardware tracks each
guest's BTB entries and can flush the BTB if it has been determined
to be contaminated with any prediction information originating outside
the particular guest's context.
To mitigate possible performance penalties incurred by these flushes,
it is recommended that the hypervisor runs with SPEC_CTRL[IBRS] set.
Note that using Automatic IBRS is not an equivalent option here, since
it behaves differently when SEV-SNP is active. See commit acaa4b5c4c85
("x86/speculation: Do not enable Automatic IBRS if SEV-SNP is enabled")
for more details.
Indicate support for BTB Isolation in sev_supported_vmsa_features,
bit 7.
SNP-active guests can enable (BTB) Isolation through SEV_Status
bit 9 (SNPBTBIsolation).
For more info, refer to page 615, Section 15.36.17 "Side-Channel
Protection", AMD64 Architecture Programmer's Manual Volume 2: System
Programming Part 2, Pub. 24593 Rev. 3.42 - March 2024 (see Link).
Link: https://bugzilla.kernel.org/attachment.cgi?id=306250
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index edde36097ddc..2038461c1316 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -305,6 +305,7 @@ static_assert((X2AVIC_4K_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AV
#define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
+#define SVM_SEV_FEAT_BTB_ISOLATION BIT(7)
#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ea515cf41168..3c0278871114 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -3167,6 +3167,9 @@ void __init sev_hardware_setup(void)
if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
+
+ if (sev_snp_enabled)
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_BTB_ISOLATION;
}
void sev_hardware_unsetup(void)
--
2.43.0
^ permalink raw reply related
* [PATCH 2/3] cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
This is to prepare to allow legacy IBRS toggling on AMD systems,
where the BTB Isolation SEV-SNP feature can use it to optimize the
quick VM exit to re-entry path.
There is no reason this wasn't allowed in the first place, therefore
adding the cc: stable and Fixes: tags.
Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/kernel/cpu/bugs.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index 4eefbff4b19a..67eff5fba629 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2154,11 +2154,6 @@ static void __init spectre_v2_select_mitigation(void)
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
- if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) {
- pr_err("IBRS selected but not Intel CPU. Switching to AUTO select\n");
- spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
- }
-
if (spectre_v2_cmd == SPECTRE_V2_CMD_IBRS && !boot_cpu_has(X86_FEATURE_IBRS)) {
pr_err("IBRS selected but CPU doesn't have IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
@@ -2247,7 +2242,7 @@ static void __init spectre_v2_apply_mitigation(void)
pr_err(SPECTRE_V2_EIBRS_EBPF_MSG);
if (spectre_v2_in_ibrs_mode(spectre_v2_enabled)) {
- if (boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
+ if (boot_cpu_has(X86_FEATURE_AUTOIBRS) && spectre_v2_enabled != SPECTRE_V2_IBRS) {
msr_set_bit(MSR_EFER, _EFER_AUTOIBRS);
} else {
x86_spec_ctrl_base |= SPEC_CTRL_IBRS;
--
2.43.0
^ permalink raw reply related
* [PATCH 1/3] cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips, stable
In-Reply-To: <20260224180157.725159-1-kim.phillips@amd.com>
The original commit that added support for Automatic IBRS neglected
to amend a condition to include AUTOIBRS in addition to the
X86_FEATURE_IBRS_ENHANCED check. Fix that, and another couple
of minor outliers.
Fixes: e7862eda309e ("x86/cpu: Support AMD Automatic IBRS")
Reported-by: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Borislav Petkov (AMD) <bp@alien8.de>
Cc: stable@kernel.org
Signed-off-by: Kim Phillips <kim.phillips@amd.com>
---
arch/x86/kernel/cpu/bugs.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index d0a2847a4bb0..4eefbff4b19a 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -2136,7 +2136,8 @@ static void __init spectre_v2_select_mitigation(void)
if ((spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS ||
spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_LFENCE ||
spectre_v2_cmd == SPECTRE_V2_CMD_EIBRS_RETPOLINE) &&
- !boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ !(boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS))) {
pr_err("EIBRS selected but CPU doesn't have Enhanced or Automatic IBRS. Switching to AUTO select\n");
spectre_v2_cmd = SPECTRE_V2_CMD_AUTO;
}
@@ -2182,7 +2183,8 @@ static void __init spectre_v2_select_mitigation(void)
break;
fallthrough;
case SPECTRE_V2_CMD_FORCE:
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED)) {
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS)) {
spectre_v2_enabled = SPECTRE_V2_EIBRS;
break;
}
@@ -2262,7 +2264,8 @@ static void __init spectre_v2_apply_mitigation(void)
case SPECTRE_V2_IBRS:
setup_force_cpu_cap(X86_FEATURE_KERNEL_IBRS);
- if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED))
+ if (boot_cpu_has(X86_FEATURE_IBRS_ENHANCED) ||
+ boot_cpu_has(X86_FEATURE_AUTOIBRS))
pr_warn(SPECTRE_V2_IBRS_PERF_MSG);
break;
--
2.43.0
^ permalink raw reply related
* [PATCH 0/3] KVM: SEV: Add support for BTB Isolation
From: Kim Phillips @ 2026-02-24 18:01 UTC (permalink / raw)
To: linux-kernel, kvm, linux-coco, x86
Cc: Sean Christopherson, Paolo Bonzini, K Prateek Nayak,
Nikunj A Dadhania, Tom Lendacky, Michael Roth, Borislav Petkov,
Borislav Petkov, Naveen Rao, David Kaplan, Pawan Gupta,
Kim Phillips
This feature ensures SNP guest Branch Target Buffers (BTBs) are not
affected by context outside that guest.
The first patch fixes a longstanding bug where users couldn't select
Automatic IBRS on AMD machines using spectre_v2=eibrs on the kcmdline.
The second patch fixes another longstanding bug where users couldn't
select legacy / toggling SPEC_CTRL[IBRS] on AMD systems, which may
be used by users of the BTB Isolation feature.
The third patch adds support for the feature by adding it to the
supported features bitmask.
Based on git://git.kernel.org/pub/scm/virt/kvm/kvm.git next,
currently b1195183ed42 (tag: tags/kvm-7.0-1, kvm/queue, kvm/next).
This series also available here:
https://github.com/AMDESE/linux/tree/btb-isol-latest
Advance qemu bits (to add btb-isol=on/off switch) available here:
https://github.com/AMDESE/qemu/tree/btb-isol-latest
Qemu bits will be posted upstream once kernel bits are merged.
They depend on Naveen Rao's "target/i386: SEV: Add support for
enabling VMSA SEV features":
https://lore.kernel.org/qemu-devel/cover.1761648149.git.naveen@kernel.org/
Kim Phillips (3):
cpu/bugs: Fix selecting Automatic IBRS using spectre_v2=eibrs
cpu/bugs: Allow spectre_v2=ibrs on x86 vendors other than Intel
KVM: SEV: Add support for SNP BTB Isolation
arch/x86/include/asm/svm.h | 1 +
arch/x86/kernel/cpu/bugs.c | 16 +++++++---------
arch/x86/kvm/svm/sev.c | 3 +++
3 files changed, 11 insertions(+), 9 deletions(-)
base-commit: b1195183ed42f1522fae3fe44ebee3af437aa000
--
2.43.0
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Sean Christopherson @ 2026-02-24 16:03 UTC (permalink / raw)
To: Binbin Wu
Cc: Rick P Edgecombe, Xiaoyao Li, changyuanl@google.com,
pbonzini@redhat.com, Binbin Wu, Isaku Yamahata, bp@alien8.de,
x86@kernel.org, kas@kernel.org, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
tglx@kernel.org, kvm@vger.kernel.org, linux-coco@lists.linux.dev
In-Reply-To: <fd3b58fd-a450-471a-89a3-541c3f88c874@linux.intel.com>
On Tue, Feb 24, 2026, Binbin Wu wrote:
> On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
> >> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >> index 2d7a4d52ccfb4..0c524f9a94a6c 100644
> >> --- a/arch/x86/kvm/vmx/tdx.c
> >> +++ b/arch/x86/kvm/vmx/tdx.c
> >> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
> >> kvm_cpuid_entry2 *entry, unsigned char i
> >> entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
> >> entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
> >>
> >> - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
> >> + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
> >> entry->index = 0;
> >> + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> >
> > There are two callers of this. One is already zeroed, and the other has
> > stack garbage in flags. But that second caller doesn't look at the
> > flags so it is harmless. Maybe it would be simpler and clearer to just
> > zero init the entry struct in that caller. Then you don't need to clear
> > it here. Or alternatively set flags to zero above, and then add
> > KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> > single bit in a field of garbage, which seems weird.
+1, td_init_cpuid_entry2() should initialize flags to '0' and then set
KVM_CPUID_FLAG_SIGNIFCANT_INDEX as appropriate.
> >> + } else {
> >> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> >> + }
> >>
> >> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
> >> + !!(entry->flags &
> >> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
> >
> > It warns on leaf 0x23 for me. Is it intentional?
>
> I guess because the list in cpuid_function_is_indexed() is hard-coded
> and 0x23 is not added into the list yet.
Yeah, I was anticipating that we'd run afoul of leaves that aren't known to
the kernel. FWIW, it looks like 0x24 is also indexed.
> It's fine for existing KVM code because cpuid_function_is_indexed() is
> only used to check that if a CPUID entry is queried without index, it
> shouldn't be included in the indexed list.
>
> But adding the consistency check here would cause compatibility issue.
> Generally, if a new CPUID indexed function is added for some new CPU and
> the TDX module reports it, KVM versions without the CPUID function in
> the list will trigger the warning.
IMO, that's a good thing and working as intended. WARNs aren't inherently evil.
While the goal is to be WARN-free, in this case triggering the WARN if the TDX
Module is updated (or new silicon arrives) is desirable, because it alerts us to
that new behavior, so that we can go update KVM.
But we should "fix" 0x23 and 0x24 before landing this patch.
^ permalink raw reply
* Re: [PATCH 1/2] firmware: smccc: add timeout, touch wdt
From: Andre Przywara @ 2026-02-24 10:58 UTC (permalink / raw)
To: Vedashree Vidwans, salman.nabi, sudeep.holla, lpieralisi,
mark.rutland, trilokkumar.soni
Cc: ardb, chao.gao, linux-arm-kernel, linux-coco, linux-kernel,
sdonthineni, vsethi, vwadekar
In-Reply-To: <20260210224023.2341728-2-vvidwans@nvidia.com>
Hi Veda,
On 2/10/26 23:40, Vedashree Vidwans wrote:
> Enhance PRIME/ACTIVATION functions to touch watchdog and implement
> timeout mechanism. This update ensures that any potential hangs are
> detected promptly and that the LFA process is allocated sufficient
> execution time before the watchdog timer expires. These changes improve
> overall system reliability by reducing the risk of undetected process
> stalls and unexpected watchdog resets.
Many thanks for that, I think it's a very good idea to take care of the
watchdog and to avoid an infinite loop in the AGAIN case.
I have some comments about some details below ....
> Signed-off-by: Vedashree Vidwans <vvidwans@nvidia.com>
> ---
> drivers/firmware/smccc/lfa_fw.c | 40 +++++++++++++++++++++++++++++++++
> 1 file changed, 40 insertions(+)
>
> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
> index da6b54fe1685..b0ace6fc8dac 100644
> --- a/drivers/firmware/smccc/lfa_fw.c
> +++ b/drivers/firmware/smccc/lfa_fw.c
> @@ -17,6 +17,9 @@
> #include <linux/array_size.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
> +#include <linux/nmi.h>
> +#include <linux/ktime.h>
> +#include <linux/delay.h>
>
> #undef pr_fmt
> #define pr_fmt(fmt) "Arm LFA: " fmt
> @@ -37,6 +40,14 @@
> #define LFA_PRIME_CALL_AGAIN BIT(0)
> #define LFA_ACTIVATE_CALL_AGAIN BIT(0)
>
> +/* Prime loop limits, TODO: tune after testing */
> +#define LFA_PRIME_BUDGET_US 30000000 /* 30s cap */
> +#define LFA_PRIME_POLL_DELAY_US 10 /* 10us between polls */
> +
> +/* Activation loop limits, TODO: tune after testing */
> +#define LFA_ACTIVATE_BUDGET_US 20000000 /* 20s cap */
> +#define LFA_ACTIVATE_POLL_DELAY_US 10 /* 10us between polls */
> +
> /* LFA return values */
> #define LFA_SUCCESS 0
> #define LFA_NOT_SUPPORTED 1
> @@ -219,6 +230,7 @@ static int call_lfa_activate(void *data)
> struct image_props *attrs = data;
> struct arm_smccc_1_2_regs args = { 0 };
> struct arm_smccc_1_2_regs res = { 0 };
> + ktime_t end = ktime_add_us(ktime_get(), LFA_ACTIVATE_BUDGET_US);
>
> args.a0 = LFA_1_0_FN_ACTIVATE;
> args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> @@ -232,6 +244,8 @@ static int call_lfa_activate(void *data)
> args.a2 = !(attrs->cpu_rendezvous_forced || attrs->cpu_rendezvous);
>
> for (;;) {
> + /* Touch watchdog, ACTIVATE shouldn't take longer than watchdog_thresh */
> + touch_nmi_watchdog();
> arm_smccc_1_2_invoke(&args, &res);
>
> if ((long)res.a0 < 0) {
> @@ -241,6 +255,15 @@ static int call_lfa_activate(void *data)
> }
> if (!(res.a1 & LFA_ACTIVATE_CALL_AGAIN))
> break; /* ACTIVATE successful */
> +
> + /* SMC returned with call_again flag set */
> + if (ktime_before(ktime_get(), end)) {
> + udelay(LFA_ACTIVATE_POLL_DELAY_US);
I don't think we should wait here at all, and definitely not with
udelay: https://docs.kernel.org/timers/delay_sleep_functions.html
Instead we should move the "call again" (and timeout) mechanism out of
this function, into activate_fw_image(), so that we exit the
stop_machine(). Otherwise we would still block everything. Doing it
there, where we should be preemptible, would give the kernel a chance to
do some housekeeping. If there is nothing for the kernel to do, then I
think it's fine to immediately call lfa_activate() again, after a
cond_resched(), for instance.
> + continue;
> + }
> +
> + pr_err("ACTIVATE for image %s timed out", attrs->image_name);
> + return -ETIMEDOUT;
> }
>
> return res.a0;
> @@ -290,6 +313,7 @@ static int prime_fw_image(struct image_props *attrs)
> {
> struct arm_smccc_1_2_regs args = { 0 };
> struct arm_smccc_1_2_regs res = { 0 };
> + ktime_t end = ktime_add_us(ktime_get(), LFA_PRIME_BUDGET_US);
> int ret;
>
> mutex_lock(&lfa_lock);
> @@ -317,6 +341,8 @@ static int prime_fw_image(struct image_props *attrs)
> args.a0 = LFA_1_0_FN_PRIME;
> args.a1 = attrs->fw_seq_id; /* fw_seq_id under consideration */
> for (;;) {
> + /* Touch watchdog, PRIME shouldn't take longer than watchdog_thresh */
> + touch_nmi_watchdog();
> arm_smccc_1_2_invoke(&args, &res);
>
> if ((long)res.a0 < 0) {
> @@ -328,6 +354,20 @@ static int prime_fw_image(struct image_props *attrs)
> }
> if (!(res.a1 & LFA_PRIME_CALL_AGAIN))
> break; /* PRIME successful */
> +
> + /* SMC returned with call_again flag set */
> + if (ktime_before(ktime_get(), end)) {
> + udelay(LFA_PRIME_POLL_DELAY_US);
same comment here, please no udelay().
This should also avoid the discussion about the exact values of the
sleep periods.
I'd just have one generous timeout (a few seconds, basically what your
BUDGET values do above), to avoid looping forever in case of a firmware
bug, for instance.
Cheers,
Andre
> + continue;
> + }
> +
> + pr_err("LFA_PRIME for image %s timed out", attrs->image_name);
> + mutex_unlock(&lfa_lock);
> +
> + ret = lfa_cancel(attrs);
> + if (ret != 0)
> + return ret;
> + return -ETIMEDOUT;
> }
>
> mutex_unlock(&lfa_lock);
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Huang, Kai @ 2026-02-24 10:49 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, bp@alien8.de,
kas@kernel.org, mingo@redhat.com, Chatre, Reinette, Weiny, Ira,
seanjc@google.com, Verma, Vishal L, nik.borisov@suse.com,
binbin.wu@linux.intel.com, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
linux-kernel@vger.kernel.org, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0+j0ohYdJlCACn@intel.com>
On Tue, 2026-02-24 at 14:00 +0800, Chao Gao wrote:
> On Mon, Feb 23, 2026 at 05:25:53PM +0800, Huang, Kai wrote:
> >
> > >
> > > +/*
> > > + * During a TDX Module update, all CPUs start from TDP_START and progress
> >
> > Nit: start from TDP_START or TDP_START + 1 ?
>
> TDP_START. See:
>
> +static int do_seamldr_install_module(void *params)
> +{
> + enum tdp_state newstate, curstate = TDP_START;
> ^^^^^^^^^^^^^^^^^^^^
>
> >
> > The code below says:
> >
> > + set_target_state(TDP_START + 1);
>
> set_target_state() sets a global target (or next) state for all CPUs. Each CPU
> compares its current state to the target. If they don't match, the CPU performs
> the required task and then acks the state.
>
> The global target state must be reset at the start of each update to trigger
> the do-while loop in do_seamldr_install_module().
OK thanks for clarification.
>
> > + ret = stop_machine_cpuslocked(do_seamldr_install_module, params,
> > cpu_online_mask);
> >
> > > + * to TDP_DONE. Each state is associated with certain work. For some
> > > + * states, just one CPU needs to perform the work, while other CPUs just
> > > + * wait during those states.
> > > + */
> > > +enum tdp_state {
> > > + TDP_START,
> > > + TDP_DONE,
> > > +};
> >
> > Nit: just curious, what does "TDP" mean?
> >
> > Maybe something more obvious?
>
> It stands for TD Preserving. Since this term isn't commonly used outside
> Intel, "TDX Module updates" is clearer. I'll change this enum to:
>
> enum module_update_state {
> MODULE_UPDATE_START,
> MODULE_UPDATE_SHUTDOWN,
> MODULE_UPDATE_CPU_INSTALL,
> MODULE_UPDATE_CPU_INIT,
> MODULE_UPDATE_RUN_UPDATE,
> MODULE_UPDATE_DONE,
> };
Thanks.
>
> >
> > > +
> > > +static struct {
> > > + enum tdp_state state;
> > > + atomic_t thread_ack;
> > > +} tdp_data;
> > > +
> > > +static void set_target_state(enum tdp_state state)
> > > +{
> > > + /* Reset ack counter. */
> > > + atomic_set(&tdp_data.thread_ack, num_online_cpus());
> > > + /* Ensure thread_ack is updated before the new state */
> >
> > Nit: perhaps add "so that ..." part to the comment?
>
> how about:
>
> /*
> * Ensure thread_ack is updated before the new state.
> * Otherwise, other CPUs may see the new state and ack
> * it before thread_ack is reset. An ack before reset
> * is effectively lost, causing the system to wait
> * forever for thread_ack to become zero.
> */
>
LGTM.
> >
> > > + smp_wmb();
> > > + WRITE_ONCE(tdp_data.state, state);
> > > +}
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Huang, Kai @ 2026-02-24 10:46 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, dave.hansen@linux.intel.com, bp@alien8.de,
kas@kernel.org, mingo@redhat.com, Chatre, Reinette, Weiny, Ira,
seanjc@google.com, Verma, Vishal L, nik.borisov@suse.com,
binbin.wu@linux.intel.com, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
linux-kernel@vger.kernel.org, paulmck@kernel.org, tglx@kernel.org,
yilun.xu@linux.intel.com, x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ00DQ2YwcwfgQtP@intel.com>
On Tue, 2026-02-24 at 13:15 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 06:31:24AM +0800, Huang, Kai wrote:
> > On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > > P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
> > > update requests. This structure contains physical addresses pointing to
> > > the module binary and its signature file (or sigstruct), along with an
> > > update scenario field.
> > >
> > > TDX Modules are distributed in the tdx_blob format defined at [1]. A
> > > tdx_blob contains a header, sigstruct, and module binary. This is also
> > > the format supplied by the userspace to the kernel.
> > >
> > > Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
> > > accordingly. This structure will be passed to P-SEAMLDR to initiate the
> > > update.
> > >
> > > Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
> > > a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
> > > Specification" will be published separately. The kernel does not
> > > validate P-SEAMLDR compatibility (for example, whether it supports 4KB
> > > or 16KB sigstruct);
> > >
> >
> > Nit:
> >
> > This sounds like the kernel can validate but chooses not to. But I thought
> > the fact is the kernel cannot validate because there's no P-SEAMLDR ABI to
> > enumerate such compatibility?
>
> Emm, the kernel could validate this by parsing mapping_file.json, but the
> complexity wouldn't be worth it.
Oh making kernel parse JSON file is beyond my imagination, but I see you
have a point here :-)
I think my real comment is the sentence
The kernel does not validate ...
only describes what does the kernel do today, which is not the case here.
Instead, we are making a design choice here, so I think the sentence should
at least be something like:
Don't make the kernel validate ...
>
> >
> > > userspace must ensure the P-SEAMLDR version is
> > > compatible with the selected TDX Module by checking the minimum
> > > P-SEAMLDR version requirements at [2].
> > >
> > > Signed-off-by: Chao Gao <chao.gao@intel.com>
> > > Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
> > > Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
> > > Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
> > >
> >
> > Nit:
> >
> > As mentioned in v3, can the link be considered as "stable", e.g., won't
> > disappear couple of years later?
>
> I'm not sure when this link will be outdated, but we'll definitely have a TDX
> Module release repository with a blob_structure.txt file describing the format.
>
> >
> > Not sure we should just have a documentation patch for 'tdx_blob' layout. I
> > suspect the content won't be changed in the future anyway, at least for
> > foreseeable future, given you have already updated the sigstruct part.
> >
> > We can include the links to the actual doc too, and if necessarily, point
> > out the links may get updated in the future. We can actually update the
> > links if they are in some doc.
>
> Regarding the documentation patch, I don't see the value in adding one. It
> would just mirror the code and become outdated when 'tdx_blob' layout is
> updated.
Sure.
>
> If the concern is that tdx_blob layout changes could cause incompatibilities,
> that's not the kernel's responsibility to prevent; the kernel has no control
> over external format changes.
No that's not the main concern.
>
> If the issue is simply that links may become outdated, that's a common problem.
> We can address this by referring to blob_structure.txt in the "Intel TDX Module
> Binaries Repository" and dropping the specific link. For example:
>
> TDX Modules are distributed in the tdx_blob format defined in
> blob_structure.txt from the "Intel TDX Module Binaries Repository". A
> tdx_blob contains a header, sigstruct, and module binary. This is also the
> format supplied by the userspace to the kernel.
I think I prefer this instead of using the Links.
My concern is the links in the changelog won't be stable. If that is
acceptable, then that's fine too.
But in the patch 23, you will update the doc anyway, so I think we can just
provide the link there (you already mentioned the repo link there anyway).
>
> >
> > [...]
> >
> > > +/*
> > > + * Intel TDX Module blob. Its format is defined at:
> > > + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
>
> I will drop this link as well.
>
I am fine keeping it here. We need a link "somewhere in _this_ patch" to
review the code I think.
It's in the comment so we can change in the future if it changes.
^ permalink raw reply
* Re: [PATCH v4 05/24] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Huang, Kai @ 2026-02-24 10:30 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0ULTpWJpGjOKLU@intel.com>
On Tue, 2026-02-24 at 10:59 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 05:36:33PM +0800, Huang, Kai wrote:
> >
> > > +int seamldr_get_info(struct seamldr_info *seamldr_info)
> > > +{
> > > + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
> >
> > Should we have a comment for slow_virt_to_phys()? This patch alone doesn't
> > really tell where is the memory from.
>
> How about:
>
> /*
> * Use slow_virt_to_phys() since @seamldr_info may be allocated on
> * the stack.
> */
>
> I was hesitant to add a comment since most existing slow_virt_to_phys() usage
> lacks comments.
Perhaps this is because in these existing usages "where the memory comes
from" and the "use of slow_virt_to_phys()" are closely together so no
comment is needed?
(disclaimer: I was looking at kvm_register_steal_time().)
So I am fine with either way -- feel free to ignore.
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Huang, Kai @ 2026-02-24 10:25 UTC (permalink / raw)
To: Gao, Chao
Cc: tony.lindgren@linux.intel.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, tglx@kernel.org, dave.hansen@linux.intel.com,
bp@alien8.de, kas@kernel.org, Chatre, Reinette, mingo@redhat.com,
Weiny, Ira, seanjc@google.com, Verma, Vishal L,
nik.borisov@suse.com, binbin.wu@linux.intel.com, hpa@zytor.com,
Annapurve, Vishal, Chen, Farrah, Duan, Zhenzhong,
sagis@google.com, linux-kernel@vger.kernel.org,
paulmck@kernel.org, Edgecombe, Rick P, yilun.xu@linux.intel.com,
x86@kernel.org, Williams, Dan J
In-Reply-To: <aZ0Nnay7ygKeXmuC@intel.com>
>
> >
> > But I don't know why do you even need to talk about NP-SEAMLDR.
>
> I included this because Dave had some confusion about NP-SEAMLDR [1], so I
> wanted to clarify it.
>
> [1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/
I thought that was under assumption both NP-SEAMLDR and P-SEAMLDR are SEAM
software (which is why both of them are mentioned). But only P-SEAMLDR is,
so I thought we can skip NP-SEAMLDR.
>
> And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
> helpful to clarify the difference. This follows Dave's earlier suggestion to
> explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].
>
> [2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/
>
Sure. If you feel that helps.
[...]
>
> > > + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
> > > + * interact with P-SEAMLDR simultaneously.
> > > + */
> > > +static DEFINE_RAW_SPINLOCK(seamldr_lock);
> > > +
> > > +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
> > > +{
> > > + /*
> > > + * Serialize P-SEAMLDR calls and disable interrupts as the calls
> > > + * can be made from IRQ context.
> > > + */
> > > + guard(raw_spinlock_irqsave)(&seamldr_lock);
> >
> > Why do you need to disable IRQ? A plain raw_spinlock should work with both
> > cases where seamldr_call() is called from IRQ disabled context and normal
> > task context?
>
> No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
> fires while a task context already holds the lock, and the interrupt handler
> also tries to acquire the same lock.
I thought that's not possible to happen because during module update we have
a machine state to serialize these P-SEAMLDR SEAMCALLs.
But I agree making it IRQ safe is the simplest way so that we don't need to
worry about the deadlock.
Sorry about the noise.
^ permalink raw reply
* Re: [PATCH v4 03/24] coco/tdx-host: Expose TDX Module version
From: Huang, Kai @ 2026-02-24 10:18 UTC (permalink / raw)
To: Gao, Chao
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
dave.hansen@linux.intel.com, kas@kernel.org, seanjc@google.com,
Chatre, Reinette, Weiny, Ira, binbin.wu@linux.intel.com,
Verma, Vishal L, nik.borisov@suse.com,
linux-kernel@vger.kernel.org, sagis@google.com, Annapurve, Vishal,
Duan, Zhenzhong, Edgecombe, Rick P, tony.lindgren@linux.intel.com,
paulmck@kernel.org, yilun.xu@linux.intel.com, x86@kernel.org,
Williams, Dan J
In-Reply-To: <aZ0Gm5/xpBnhOeod@intel.com>
On Tue, 2026-02-24 at 10:02 +0800, Chao Gao wrote:
> On Fri, Feb 20, 2026 at 08:40:13AM +0800, Huang, Kai wrote:
> > On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
> > > For TDX Module updates, userspace needs to select compatible update
> > > versions based on the current module version. This design delegates
> > > module selection complexity to userspace because TDX Module update
> > > policies are complex and version series are platform-specific.
> > >
> > > For example, the 1.5.x series is for certain platform generations, while
> > > the 2.0.x series is intended for others. And TDX Module 1.5.x may be
> > > updated to 1.5.y but not to 1.5.y+1.
> > >
> > > Expose the TDX Module version to userspace via sysfs to aid module
> > > selection. Since the TDX faux device will drive module updates, expose
> > > the version as its attribute.
> > >
> > > One bonus of exposing TDX Module version via sysfs is: TDX Module
> > > version information remains available even after dmesg logs are cleared.
> > >
> > > == Background ==
> > >
> > > The "faux device + device attribute" approach compares to other update
> > > mechanisms as follows:
> >
> > This "faux device + device attribute" approach seems to be a wider design
> > choice instead of how to expose module version (which is the scope of this
> > patch). Overall, shouldn't this be in the changelog of the previous patch
> > which actually introduces "faux device" (albeit no attribute is introduced
> > in that patch)?
> ,
> Yes, it's mentioned briefly in the previous patch:
>
> """
> Create a virtual device not only to align with other implementations but
> also to make it easier to
>
> - expose metadata (e.g., TDX module version, seamldr version etc) to
> the userspace as device attributes
>
> ...
> """
>
> The previous patch doesn't provide details for version information
> exposure, as version attributes are just one of several purposes for the
> virtual device.
>
> >
> > >
> > > 1. AMD SEV leverages an existing PCI device for the PSP to expose
> > > metadata. TDX uses a faux device as it doesn't have PCI device
> > > in its architecture.
> >
> > E.g., this sounds to justify "why to use faux device for TDX", but not "to
> > expose module version via faux device attributes".
>
> This provides additional context as suggested by Dave:
>
> https://lore.kernel.org/kvm/aa3f026b-ad69-4070-8433-8950e5250edb@intel.com/
>
> Dave asked:
>
> """
> What are other CPU vendors doing for this? SEV? CCA? S390? How are their
> firmware versions exposed? What about other things in the Intel world
> like CPU microcode or the billion other chunks of firmware? ...
> """
I fully agree with this. We need justification of why we need to expose TDX
module version to somewhere in /sysfs, and the choice of that somewhere is
the faux device attributes.
But my interpretation is Dave is asking to provide such justification in
general, but not specifically in _this_ patch.
In this patch, you have already adequately put why to expose version info
via /sysfs. The "background" is really explaining why to choose "faux
device" as the /sysfs entry.
But you have already made the choice to use faux device (and mentioned
exposing version is one purpose) in the previous patch, so to me the
"background" part is a bit weird to be here, but not in previous patch.
But I also see there's some connection here -- and anyway this is just my
interpretation, so feel free to ignore :-)
^ permalink raw reply
* Re: [PATCH] KVM: TDX: Set SIGNIFCANT_INDEX flag for supported CPUIDs
From: Binbin Wu @ 2026-02-24 8:50 UTC (permalink / raw)
To: Edgecombe, Rick P, Li, Xiaoyao, changyuanl@google.com,
pbonzini@redhat.com, seanjc@google.com, Wu, Binbin,
Yamahata, Isaku
Cc: bp@alien8.de, x86@kernel.org, kas@kernel.org, hpa@zytor.com,
mingo@redhat.com, linux-kernel@vger.kernel.org,
dave.hansen@linux.intel.com, tglx@kernel.org, kvm@vger.kernel.org,
linux-coco@lists.linux.dev
In-Reply-To: <213d614fe73e183a230c8f4e0c8fa1cc3d45df39.camel@intel.com>
On 2/24/2026 9:57 AM, Edgecombe, Rick P wrote:
> +binbin
>
> On Mon, 2026-02-23 at 13:43 -0800, Changyuan Lyu wrote:
>> Set the KVM_CPUID_FLAG_SIGNIFCANT_INDEX flag in the kvm_cpuid_entry2
>> structures returned by KVM_TDX_CAPABILITIES if the CPUID is indexed.
>> This ensures consistency with the CPUID entries returned by
>> KVM_GET_SUPPORTED_CPUID.
>>
>> Additionally, add a WARN_ON_ONCE() to verify that the TDX module's
>> reported entries align with KVM's expectations regarding indexed
>> CPUID functions.
>>
>> Suggested-by: Sean Christopherson <seanjc@google.com>
>> Signed-off-by: Changyuan Lyu <changyuanl@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 2d7a4d52ccfb4..0c524f9a94a6c 100644
>> --- a/arch/x86/kvm/vmx/tdx.c
>> +++ b/arch/x86/kvm/vmx/tdx.c
>> @@ -172,9 +172,15 @@ static void td_init_cpuid_entry2(struct
>> kvm_cpuid_entry2 *entry, unsigned char i
>> entry->ecx = (u32)td_conf->cpuid_config_values[idx][1];
>> entry->edx = td_conf->cpuid_config_values[idx][1] >> 32;
>>
>> - if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF)
>> + if (entry->index == KVM_TDX_CPUID_NO_SUBLEAF) {
>> entry->index = 0;
>> + entry->flags &= ~KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>
> There are two callers of this. One is already zeroed, and the other has
> stack garbage in flags. But that second caller doesn't look at the
> flags so it is harmless. Maybe it would be simpler and clearer to just
> zero init the entry struct in that caller. Then you don't need to clear
> it here. Or alternatively set flags to zero above, and then add
> KVM_CPUID_FLAG_SIGNIFCANT_INDEX if needed. Rather than manipulating a
> single bit in a field of garbage, which seems weird.
>
>> + } else {
>> + entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>> + }
>>
>> + WARN_ON_ONCE(cpuid_function_is_indexed(entry->function) !=
>> + !!(entry->flags &
>> KVM_CPUID_FLAG_SIGNIFCANT_INDEX));
>
> It warns on leaf 0x23 for me. Is it intentional?
I guess because the list in cpuid_function_is_indexed() is hard-coded
and 0x23 is not added into the list yet.
It's fine for existing KVM code because cpuid_function_is_indexed() is
only used to check that if a CPUID entry is queried without index, it
shouldn't be included in the indexed list.
But adding the consistency check here would cause compatibility issue.
Generally, if a new CPUID indexed function is added for some new CPU and
the TDX module reports it, KVM versions without the CPUID function in
the list will trigger the warning.
>
> This warning kind of begs the question of how how much consistency
> there should be between KVM_TDX_CAPABILITIES and
> KVM_GET_SUPPORTED_CPUID. There was quite a bit of debate on this and in
> the end we moved forward with a solution that did the bare minimum
> consistency checking.
>
> We actually have been looking at some potential TDX module changes to
> fix the deficiencies from not enforcing the consistency. But didn't
> consider this pattern. Can you explain more about the failure mode?
>
>> /*
>> * The TDX module doesn't allow configuring the guest phys
>> addr bits
>> * (EAX[23:16]). However, KVM uses it as an interface to
>> the userspace
>> --
>
^ permalink raw reply
* Re: [PATCH v2 2/2] dma-buf: heaps: system: add system_cc_decrypted heap for explicitly decrypted memory
From: Jiri Pirko @ 2026-02-24 8:32 UTC (permalink / raw)
To: John Stultz
Cc: dri-devel, linaro-mm-sig, iommu, linux-media, sumit.semwal,
benjamin.gaignard, Brian.Starkey, tjmercier, christian.koenig,
m.szyprowski, robin.murphy, jgg, leon, sean.anderson, ptesarik,
catalin.marinas, aneesh.kumar, suzuki.poulose, steven.price,
thomas.lendacky, john.allen, ashish.kalra, suravee.suthikulpanit,
linux-coco
In-Reply-To: <CANDhNCp94KG06P_7ivMTNA27qEM9g8ox-h3b_tZ=v6e-25xJ3g@mail.gmail.com>
Mon, Feb 23, 2026 at 07:33:07PM +0100, jstultz@google.com wrote:
>On Mon, Feb 23, 2026 at 1:51 AM Jiri Pirko <jiri@resnulli.us> wrote:
>>
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> Add a new "system_cc_decrypted" dma-buf heap to allow userspace to
>> allocate decrypted (shared) memory for confidential computing (CoCo)
>> VMs.
>>
>> On CoCo VMs, guest memory is encrypted by default. The hardware uses an
>> encryption bit in page table entries (C-bit on AMD SEV, "shared" bit on
>> Intel TDX) to control whether a given memory access is encrypted or
>> decrypted. The kernel's direct map is set up with encryption enabled,
>> so pages returned by alloc_pages() are encrypted in the direct map
>> by default. To make this memory usable for devices that do not support
>> DMA to encrypted memory (no TDISP support), it has to be explicitly
>> decrypted. A couple of things are needed to properly handle
>> decrypted memory for the dma-buf use case:
>>
>> - set_memory_decrypted() on the direct map after allocation:
>> Besides clearing the encryption bit in the direct map PTEs, this
>> also notifies the hypervisor about the page state change. On free,
>> the inverse set_memory_encrypted() must be called before returning
>> pages to the allocator. If re-encryption fails, pages
>> are intentionally leaked to prevent decrypted memory from being
>> reused as private.
>>
>> - pgprot_decrypted() for userspace and kernel virtual mappings:
>> Any new mapping of the decrypted pages, be it to userspace via
>> mmap or to kernel vmalloc space via vmap, creates PTEs independent
>> of the direct map. These must also have the encryption bit cleared,
>> otherwise accesses through them would see encrypted (garbage) data.
>>
>> - DMA_ATTR_CC_DECRYPTED for DMA mapping:
>> Since the pages are already decrypted, the DMA API needs to be
>> informed via DMA_ATTR_CC_DECRYPTED so it can map them correctly
>> as unencrypted for device access.
>>
>> On non-CoCo VMs, the system_cc_decrypted heap is not registered
>> to prevent misuse by userspace that does not understand
>> the security implications of explicitly decrypted memory.
>>
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>
>Thanks for reworking this! I've not reviewed it super closely, but I
>believe it resolves my objection on your first version.
>
>Few nits/questions below.
>
>> @@ -296,6 +345,14 @@ static void system_heap_dma_buf_release(struct dma_buf *dmabuf)
>> for_each_sgtable_sg(table, sg, i) {
>> struct page *page = sg_page(sg);
>>
>> + /*
>> + * Intentionally leak pages that cannot be re-encrypted
>> + * to prevent decrypted memory from being reused.
>> + */
>> + if (buffer->decrypted &&
>> + system_heap_set_page_encrypted(page))
>> + continue;
>> +
>
>What are the conditions where this would fail? How much of an edge
>case is this? I fret this opens a DoS vector if one is able to
>allocate from this heap and then stress the system when doing the
>free.
From what I can see, the failure of set_memory_encrypted() is quite
rare. Don't see any real DoS scenario for this. All the failures seems
to be either theoretical (sanity checks, malicious VMM) or concurrent
kexec execution in case of x86/pat.
>
>Should there be some global list of leaked decrypted pages such that
>the mm subsystem could try again later to recover these?
swiotlb does the same non-recovery leakage. I belive is it not worth
implementing this at this time,
>
>> diff --git a/include/linux/dma-heap.h b/include/linux/dma-heap.h
>> index 648328a64b27..d97b668413c1 100644
>> --- a/include/linux/dma-heap.h
>> +++ b/include/linux/dma-heap.h
>> @@ -10,6 +10,7 @@
>> #define _DMA_HEAPS_H
>>
>> #include <linux/types.h>
>> +#include <uapi/linux/dma-heap.h>
>>
>> struct dma_heap;
>>
>> diff --git a/include/uapi/linux/dma-heap.h b/include/uapi/linux/dma-heap.h
>> index a4cf716a49fa..ab95bb355ed5 100644
>> --- a/include/uapi/linux/dma-heap.h
>> +++ b/include/uapi/linux/dma-heap.h
>> @@ -18,8 +18,7 @@
>> /* Valid FD_FLAGS are O_CLOEXEC, O_RDONLY, O_WRONLY, O_RDWR */
>> #define DMA_HEAP_VALID_FD_FLAGS (O_CLOEXEC | O_ACCMODE)
>>
>> -/* Currently no heap flags */
>> -#define DMA_HEAP_VALID_HEAP_FLAGS (0ULL)
>> +#define DMA_HEAP_VALID_HEAP_FLAGS (0)
>>
>> /**
>> * struct dma_heap_allocation_data - metadata passed from userspace for
>
>Are these header changes still necessary?
Oops, leftovers. Will remove.
Thanks!
>
>thanks
>-john
^ permalink raw reply
* Re: [PATCH v4 20/24] x86/virt/tdx: Enable TDX Module runtime updates
From: Chao Gao @ 2026-02-24 6:02 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <5bd8500eba9a8e83491c02ae84f81b55ac09dacb.camel@intel.com>
On Mon, Feb 23, 2026 at 01:09:10PM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> All pieces of TDX Module runtime updates are in place. Enable it if it
>> is supported.
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> ---
>> v4:
>> - s/BIT/BIT_ULL [Tony]
>> ---
>> arch/x86/include/asm/tdx.h | 5 ++++-
>> arch/x86/virt/vmx/tdx/tdx.h | 3 ---
>> 2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
>> index ffadbf64d0c1..ad62a7be0443 100644
>> --- a/arch/x86/include/asm/tdx.h
>> +++ b/arch/x86/include/asm/tdx.h
>> @@ -32,6 +32,9 @@
>> #define TDX_SUCCESS 0ULL
>> #define TDX_RND_NO_ENTROPY 0x8000020300000000ULL
>>
>> +/* Bit definitions of TDX_FEATURES0 metadata field */
>> +#define TDX_FEATURES0_TD_PRESERVING BIT_ULL(1)
>> +#define TDX_FEATURES0_NO_RBP_MOD BIT_ULL(18)
>> #ifndef __ASSEMBLER__
>>
>> #include <uapi/asm/mce.h>
>> @@ -105,7 +108,7 @@ const struct tdx_sys_info *tdx_get_sysinfo(void);
>>
>> static inline bool tdx_supports_runtime_update(const struct tdx_sys_info *sysinfo)
>> {
>> - return false; /* To be enabled when kernel is ready */
>> + return sysinfo->features.tdx_features0 & TDX_FEATURES0_TD_PRESERVING;
>> }
>>
>> int tdx_guest_keyid_alloc(void);
>> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
>> index d1807a476d3b..749f4d74cb2c 100644
>> --- a/arch/x86/virt/vmx/tdx/tdx.h
>> +++ b/arch/x86/virt/vmx/tdx/tdx.h
>> @@ -88,9 +88,6 @@ struct tdmr_info {
>> DECLARE_FLEX_ARRAY(struct tdmr_reserved_area, reserved_areas);
>> } __packed __aligned(TDMR_INFO_ALIGNMENT);
>>
>> -/* Bit definitions of TDX_FEATURES0 metadata field */
>> -#define TDX_FEATURES0_NO_RBP_MOD BIT(18)
>> -
>>
>
>Nit:
>
>Strictly speaking, moving this "NO_RBP_MOD" isn't required to "enable TDX
>module runtime updates". So I think it's better to call out in changelog
>that this is trying to centralize the bit definitions.
Sure. Will do.
^ permalink raw reply
* Re: [PATCH v4 11/24] x86/virt/seamldr: Introduce skeleton for TDX Module updates
From: Chao Gao @ 2026-02-24 6:00 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <14ee337df2983edb3677e3929d31e54374a1762e.camel@intel.com>
On Mon, Feb 23, 2026 at 05:25:53PM +0800, Huang, Kai wrote:
>
>>
>> +/*
>> + * During a TDX Module update, all CPUs start from TDP_START and progress
>
>Nit: start from TDP_START or TDP_START + 1 ?
TDP_START. See:
+static int do_seamldr_install_module(void *params)
+{
+ enum tdp_state newstate, curstate = TDP_START;
^^^^^^^^^^^^^^^^^^^^
>
>The code below says:
>
>+ set_target_state(TDP_START + 1);
set_target_state() sets a global target (or next) state for all CPUs. Each CPU
compares its current state to the target. If they don't match, the CPU performs
the required task and then acks the state.
The global target state must be reset at the start of each update to trigger
the do-while loop in do_seamldr_install_module().
>+ ret = stop_machine_cpuslocked(do_seamldr_install_module, params,
>cpu_online_mask);
>
>> + * to TDP_DONE. Each state is associated with certain work. For some
>> + * states, just one CPU needs to perform the work, while other CPUs just
>> + * wait during those states.
>> + */
>> +enum tdp_state {
>> + TDP_START,
>> + TDP_DONE,
>> +};
>
>Nit: just curious, what does "TDP" mean?
>
>Maybe something more obvious?
It stands for TD Preserving. Since this term isn't commonly used outside
Intel, "TDX Module updates" is clearer. I'll change this enum to:
enum module_update_state {
MODULE_UPDATE_START,
MODULE_UPDATE_SHUTDOWN,
MODULE_UPDATE_CPU_INSTALL,
MODULE_UPDATE_CPU_INIT,
MODULE_UPDATE_RUN_UPDATE,
MODULE_UPDATE_DONE,
};
>
>> +
>> +static struct {
>> + enum tdp_state state;
>> + atomic_t thread_ack;
>> +} tdp_data;
>> +
>> +static void set_target_state(enum tdp_state state)
>> +{
>> + /* Reset ack counter. */
>> + atomic_set(&tdp_data.thread_ack, num_online_cpus());
>> + /* Ensure thread_ack is updated before the new state */
>
>Nit: perhaps add "so that ..." part to the comment?
how about:
/*
* Ensure thread_ack is updated before the new state.
* Otherwise, other CPUs may see the new state and ack
* it before thread_ack is reset. An ack before reset
* is effectively lost, causing the system to wait
* forever for thread_ack to become zero.
*/
>
>> + smp_wmb();
>> + WRITE_ONCE(tdp_data.state, state);
>> +}
>> +
>> +/* Last one to ack a state moves to the next state. */
>> +static void ack_state(void)
>> +{
>> + if (atomic_dec_and_test(&tdp_data.thread_ack))
>> + set_target_state(tdp_data.state + 1);
>> +}
>> +
>> +/*
>> + * See multi_cpu_stop() from where this multi-cpu state-machine was
>> + * adopted, and the rationale for touch_nmi_watchdog()
>> + */
>
>Nit: add a period to the end of the sentence.
>
>(btw, I found using period or not isn't consistent even among the 'one-line-
>sentence' comments, maybe you want to make that consistent.)
Will do. Thanks for this suggestion.
^ permalink raw reply
* Re: [PATCH v4 10/24] x86/virt/seamldr: Allocate and populate a module update request
From: Chao Gao @ 2026-02-24 5:15 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <1aa733f9066dd85c1d4f880c5c48b40c76d518c7.camel@intel.com>
On Fri, Feb 20, 2026 at 06:31:24AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> P-SEAMLDR uses the SEAMLDR_PARAMS structure to describe TDX Module
>> update requests. This structure contains physical addresses pointing to
>> the module binary and its signature file (or sigstruct), along with an
>> update scenario field.
>>
>> TDX Modules are distributed in the tdx_blob format defined at [1]. A
>> tdx_blob contains a header, sigstruct, and module binary. This is also
>> the format supplied by the userspace to the kernel.
>>
>> Parse the tdx_blob format and populate a SEAMLDR_PARAMS structure
>> accordingly. This structure will be passed to P-SEAMLDR to initiate the
>> update.
>>
>> Note that the sigstruct_pa field in SEAMLDR_PARAMS has been extended to
>> a 4-element array. The updated "SEAM Loader (SEAMLDR) Interface
>> Specification" will be published separately. The kernel does not
>> validate P-SEAMLDR compatibility (for example, whether it supports 4KB
>> or 16KB sigstruct);
>>
>
>Nit:
>
>This sounds like the kernel can validate but chooses not to. But I thought
>the fact is the kernel cannot validate because there's no P-SEAMLDR ABI to
>enumerate such compatibility?
Emm, the kernel could validate this by parsing mapping_file.json, but the
complexity wouldn't be worth it.
>
>> userspace must ensure the P-SEAMLDR version is
>> compatible with the selected TDX Module by checking the minimum
>> P-SEAMLDR version requirements at [2].
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/blob_structure.txt # [1]
>> Link: https://github.com/intel/confidential-computing.tdx.tdx-module.binaries/blob/main/mapping_file.json # [2]
>>
>
>Nit:
>
>As mentioned in v3, can the link be considered as "stable", e.g., won't
>disappear couple of years later?
I'm not sure when this link will be outdated, but we'll definitely have a TDX
Module release repository with a blob_structure.txt file describing the format.
>
>Not sure we should just have a documentation patch for 'tdx_blob' layout. I
>suspect the content won't be changed in the future anyway, at least for
>foreseeable future, given you have already updated the sigstruct part.
>
>We can include the links to the actual doc too, and if necessarily, point
>out the links may get updated in the future. We can actually update the
>links if they are in some doc.
Regarding the documentation patch, I don't see the value in adding one. It
would just mirror the code and become outdated when 'tdx_blob' layout is
updated.
If the concern is that tdx_blob layout changes could cause incompatibilities,
that's not the kernel's responsibility to prevent; the kernel has no control
over external format changes.
If the issue is simply that links may become outdated, that's a common problem.
We can address this by referring to blob_structure.txt in the "Intel TDX Module
Binaries Repository" and dropping the specific link. For example:
TDX Modules are distributed in the tdx_blob format defined in
blob_structure.txt from the "Intel TDX Module Binaries Repository". A
tdx_blob contains a header, sigstruct, and module binary. This is also the
format supplied by the userspace to the kernel.
>
>[...]
>
>> +/*
>> + * Intel TDX Module blob. Its format is defined at:
>> + * https://github.com/intel/tdx-module-binaries/blob/main/blob_structure.txt
I will drop this link as well.
>> + *
>> + * Note this structure differs from the reference above: the two variable-length
>> + * fields "@sigstruct" and "@module" are represented as a single "@data" field
>> + * here and split programmatically using the offset_of_module value.
>> + */
>> +struct tdx_blob {
>> + u16 version;
>> + u16 checksum;
>> + u32 offset_of_module;
>> + u8 signature[8];
>> + u32 length;
>> + u32 resv0;
>> + u64 resv1[509];
>> + u8 data[];
>> +} __packed;
>
>Nit:
>
>It appeared you said you will s/resv/rsvd in v3.
>
>I don't quite mind if other people are fine with 'resv'. Or you can spell
>out 'reserved' in full to match the one in 'struct seamldr_params' above.
Sorry, I missed this feedback. I'll use "reserved".
I even updated "len" to "length" and changed the index to start from 0 (to match
blob_structure.txt) but somehow missed updating "resv."
^ permalink raw reply
* Re: [PATCH v4 05/24] x86/virt/seamldr: Retrieve P-SEAMLDR information
From: Chao Gao @ 2026-02-24 2:59 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org, Chen, Farrah,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <88141072be073896990f87b2b4c33bdd99f38b29.camel@intel.com>
On Fri, Feb 20, 2026 at 05:36:33PM +0800, Huang, Kai wrote:
>
>> +int seamldr_get_info(struct seamldr_info *seamldr_info)
>> +{
>> + struct tdx_module_args args = { .rcx = slow_virt_to_phys(seamldr_info) };
>
>Should we have a comment for slow_virt_to_phys()? This patch alone doesn't
>really tell where is the memory from.
How about:
/*
* Use slow_virt_to_phys() since @seamldr_info may be allocated on
* the stack.
*/
I was hesitant to add a comment since most existing slow_virt_to_phys() usage
lacks comments.
>
>Btw, it it were me, I would just merge this patch with the next one. Then
>it's clear the memory comes from tdx-host module's stack. The merged patch
>won't be too big to review either (IMHO). You can then have this
>seamldr_get_info() and its user together in one patch, with one changelog to
>tell the full story.
>
>But just my 2cents, feel free to ignore.
I'm fine with this. But let's see what others think about merging the patches.
>
>> +
>> + return seamldr_call(P_SEAMLDR_INFO, &args);
>> +}
>> +EXPORT_SYMBOL_FOR_MODULES(seamldr_get_info, "tdx-host");
>> --
>> 2.47.3
^ permalink raw reply
* Re: [PATCH v4 04/24] x86/virt/seamldr: Introduce a wrapper for P-SEAMLDR SEAMCALLs
From: Chao Gao @ 2026-02-24 2:31 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org, Chen, Farrah,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
mingo@redhat.com, Weiny, Ira, hpa@zytor.com, Annapurve, Vishal,
sagis@google.com, Duan, Zhenzhong, Edgecombe, Rick P,
paulmck@kernel.org, tglx@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J, bp@alien8.de
In-Reply-To: <2683dff7a7950c57aa7a73584d86cf1b34bcfc07.camel@intel.com>
On Fri, Feb 20, 2026 at 09:12:29AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> The TDX architecture uses the "SEAMCALL" instruction to communicate with
>> SEAM mode software. Right now, the only SEAM mode software that the kernel
>> communicates with is the TDX module. But, there is actually another
>> component that runs in SEAM mode but it is separate from the TDX module:
>> the persistent SEAM loader or "P-SEAMLDR". Right now, the only component
>> that communicates with it is the BIOS which loads the TDX module itself at
>> boot. But, to support updating the TDX module, the kernel now needs to be
>> able to talk to it.
>>
>> P-SEAMLDR SEAMCALLs differ from TDX Module SEAMCALLs in areas such as
>> concurrency requirements. Add a P-SEAMLDR wrapper to handle these
>> differences and prepare for implementing concrete functions.
>>
>> Note that unlike P-SEAMLDR, there is also a non-persistent SEAM loader
>> ("NP-SEAMLDR"). This is an authenticated code module (ACM) that is not
>> callable at runtime. Only BIOS launches it to load P-SEAMLDR at boot;
>
>[...]
>
>> the kernel does not interact with it.
>
>Nit:
>
>Again, to me this only describes what does the kernel do today. It doesn't
>describe what the kernel needs to do for runtime updating.
>
>Maybe it can just be something like:
>
> The kernel does not need to interact with it for runtime update.
I am fine with this. Will do.
>
>But I don't know why do you even need to talk about NP-SEAMLDR.
I included this because Dave had some confusion about NP-SEAMLDR [1], so I
wanted to clarify it.
[1]: https://lore.kernel.org/kvm/aXt0+lRvpvf5knKP@intel.com/
And, since NP-SEAMLDR and P-SEAMLDR have similar names, I thought it would be
helpful to clarify the difference. This follows Dave's earlier suggestion to
explain SEAM_INFO and SEAM_SEAMINFO SEAMCALLs for clarity [2].
[2]: https://lore.kernel.org/kvm/b2e2fd5e-8aff-4eda-a648-9ae9f8234d25@intel.com/
>
>>
>> For details of P-SEAMLDR SEAMCALLs, see Intel® Trust Domain CPU
>> Architectural Extensions, Revision 343754-002, Chapter 2.3 "INSTRUCTION
>> SET REFERENCE".
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Tested-by: Farrah Chen <farrah.chen@intel.com>
>> Link: https://cdrdv2.intel.com/v1/dl/getContent/733582 # [1]
>>
>
>[...]
>
>> + * Serialize P-SEAMLDR calls since the hardware only allows a single CPU to
>> + * interact with P-SEAMLDR simultaneously.
>> + */
>> +static DEFINE_RAW_SPINLOCK(seamldr_lock);
>> +
>> +static __maybe_unused int seamldr_call(u64 fn, struct tdx_module_args *args)
>> +{
>> + /*
>> + * Serialize P-SEAMLDR calls and disable interrupts as the calls
>> + * can be made from IRQ context.
>> + */
>> + guard(raw_spinlock_irqsave)(&seamldr_lock);
>
>Why do you need to disable IRQ? A plain raw_spinlock should work with both
>cases where seamldr_call() is called from IRQ disabled context and normal
>task context?
No, that's not safe. Without _irqsave, a deadlock can occur if an interrupt
fires while a task context already holds the lock, and the interrupt handler
also tries to acquire the same lock.
>
>> + return seamcall_prerr(fn, args);
>> +}
^ permalink raw reply
* Re: [PATCH v4 03/24] coco/tdx-host: Expose TDX Module version
From: Chao Gao @ 2026-02-24 2:02 UTC (permalink / raw)
To: Huang, Kai
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev,
linux-kernel@vger.kernel.org, x86@kernel.org,
dave.hansen@linux.intel.com, tony.lindgren@linux.intel.com,
binbin.wu@linux.intel.com, seanjc@google.com, kas@kernel.org,
Chatre, Reinette, Verma, Vishal L, nik.borisov@suse.com,
Weiny, Ira, Annapurve, Vishal, sagis@google.com, Duan, Zhenzhong,
Edgecombe, Rick P, paulmck@kernel.org, yilun.xu@linux.intel.com,
Williams, Dan J
In-Reply-To: <3a8feb5470bd964e421969918b5553259abdd493.camel@intel.com>
On Fri, Feb 20, 2026 at 08:40:13AM +0800, Huang, Kai wrote:
>On Thu, 2026-02-12 at 06:35 -0800, Chao Gao wrote:
>> For TDX Module updates, userspace needs to select compatible update
>> versions based on the current module version. This design delegates
>> module selection complexity to userspace because TDX Module update
>> policies are complex and version series are platform-specific.
>>
>> For example, the 1.5.x series is for certain platform generations, while
>> the 2.0.x series is intended for others. And TDX Module 1.5.x may be
>> updated to 1.5.y but not to 1.5.y+1.
>>
>> Expose the TDX Module version to userspace via sysfs to aid module
>> selection. Since the TDX faux device will drive module updates, expose
>> the version as its attribute.
>>
>> One bonus of exposing TDX Module version via sysfs is: TDX Module
>> version information remains available even after dmesg logs are cleared.
>>
>> == Background ==
>>
>> The "faux device + device attribute" approach compares to other update
>> mechanisms as follows:
>
>This "faux device + device attribute" approach seems to be a wider design
>choice instead of how to expose module version (which is the scope of this
>patch). Overall, shouldn't this be in the changelog of the previous patch
>which actually introduces "faux device" (albeit no attribute is introduced
>in that patch)?
Yes, it's mentioned briefly in the previous patch:
"""
Create a virtual device not only to align with other implementations but
also to make it easier to
- expose metadata (e.g., TDX module version, seamldr version etc) to
the userspace as device attributes
...
"""
The previous patch doesn't provide details for version information
exposure, as version attributes are just one of several purposes for the
virtual device.
>
>>
>> 1. AMD SEV leverages an existing PCI device for the PSP to expose
>> metadata. TDX uses a faux device as it doesn't have PCI device
>> in its architecture.
>
>E.g., this sounds to justify "why to use faux device for TDX", but not "to
>expose module version via faux device attributes".
This provides additional context as suggested by Dave:
https://lore.kernel.org/kvm/aa3f026b-ad69-4070-8433-8950e5250edb@intel.com/
Dave asked:
"""
What are other CPU vendors doing for this? SEV? CCA? S390? How are their
firmware versions exposed? What about other things in the Intel world
like CPU microcode or the billion other chunks of firmware? ...
"""
>
>>
>> 2. Microcode uses per-CPU virtual devices to report microcode revisions
>> because CPUs can have different revisions. But, there is only a
>> single TDX Module, so exposing the TDX Module version through a global
>> TDX faux device is appropriate
>
>This is related to exposing module version, but to me "there's only a single
>TDX module" is also more like a justification to use "one faux device",
>which should belong to changelog of previous patch too.
The previous patch already includes this justification:
"""
A faux device is used as for TDX because the TDX module is singular within
the system ...
"""
>
>With "there's only a single TDX module" being said in previous patch
>changelog, I think we can safely deduce that there's only "one module
>version" but not per-cpu (thus I don't think we even need to call this out
>in _this_ patch).
>
>>
>> 3. ARM's CCA implementation isn't in-tree yet, but will likely follow a
>> similar faux device approach [1], though it's unclear whether they need
>> to expose firmware version information
>
>Again, I don't feel "follow a similar faux device approach" for ARM CCA
>should be a justification of "exposing module version via faux attributes".
>It should be a justification of "using faux device for TDX".
Agreed. I repeated this information here under "== Background ==" to give
broader context for the overall approach.
>
>>
>> Signed-off-by: Chao Gao <chao.gao@intel.com>
>> Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
>> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
>> Reviewed-by: Xu Yilun <yilun.xu@linux.intel.com>
>> Link: https://lore.kernel.org/all/2025073035-bulginess-rematch-b92e@gregkh/ # [1]
>>
>
>[...]
>
>> +Description: (RO) Report the version of the loaded TDX Module. The TDX Module
>> + version is formatted as x.y.z, where "x" is the major version,
>> + "y" is the minor version and "z" is the update version. Versions
>> + are used for bug reporting, TDX Module updates and etc.
> ^
>
>Nit: No need to use "and" before "etc".
Thanks. Will fix this.
^ 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