[PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[Yan Zhao]

Add a warning on a !4K level to loudly flag the unexpected condition when
callers pass in a level > 4K before tdh_mem_page_aug() supports huge pages.

The warning makes the unexpected condition more obvious since the SEAMCALL
TDH_MEM_PAGE_AUG does not necessarily fail when the level is above 4K,
while tdh_mem_page_aug() only flushes the cache for a 4K page before huge
page support is added.

Do not drop the "level" parameter instead, as it will be needed once huge
page support is added.

Do not use a stronger BUG_ON() because the cache flush is only needed when
the TDX module exposes a CLFLUSH_BEFORE_ALLOC bit, which has not yet been
observed in any TDX modules.

Reported-by: Kiryl Shutsemau <kas@kernel.org>
Closes: https://lore.kernel.org/all/abvxiuJfK2eM_1UX@thinkstation
Suggested-by: Dave Hansen <dave.hansen@intel.com>
Suggested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 arch/x86/virt/vmx/tdx/tdx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..5c0e91b4aa4f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1722,6 +1722,8 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
 	};
 	u64 ret;
 
+	/* TODO: handle large pages. */
+	WARN_ON_ONCE(level != PG_LEVEL_4K);
 	tdx_clflush_pfn(pfn);
 	ret = seamcall_ret(TDH_MEM_PAGE_AUG, &args);
 

base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
-- 
2.43.2

Re: [PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[Sean Christopherson]

On Tue, Aug 25, 2026, Yan Zhao wrote:
> Add a warning on a !4K level to loudly flag the unexpected condition when
> callers pass in a level > 4K before tdh_mem_page_aug() supports huge pages.
> 
> The warning makes the unexpected condition more obvious since the SEAMCALL
> TDH_MEM_PAGE_AUG does not necessarily fail when the level is above 4K,
> while tdh_mem_page_aug() only flushes the cache for a 4K page before huge
> page support is added.

If the only problem is the lack of CLFLUSH coverage, why not simply fix the
bug?  The changelog spends more effort justifying not fixing a bug than it would
take to fix the bug.

diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 42df8ea464c4..1a83857eb7e6 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1628,9 +1628,9 @@ static void tdx_clflush_page(struct page *page)
 	clflush_cache_range(page_to_virt(page), PAGE_SIZE);
 }
 
-static void tdx_clflush_pfn(kvm_pfn_t pfn)
+static void tdx_clflush_pfn(kvm_pfn_t pfn, enum pg_level level)
 {
-	clflush_cache_range(__va(PFN_PHYS(pfn)), PAGE_SIZE);
+	clflush_cache_range(__va(PFN_PHYS(pfn)), page_level_size(level));
 }
 
 static int pg_level_to_tdx_sept_level(enum pg_level level)
@@ -1670,7 +1670,7 @@ u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *sou
 	};
 	u64 ret;
 
-	tdx_clflush_pfn(pfn);
+	tdx_clflush_pfn(pfn, PG_LEVEL_4K);
 	ret = seamcall_ret(TDH_MEM_PAGE_ADD, &args);
 
 	*ext_err1 = args.rcx;
@@ -1722,7 +1722,7 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
 	};
 	u64 ret;
 
-	tdx_clflush_pfn(pfn);
+	tdx_clflush_pfn(pfn, level);
 	ret = seamcall_ret(TDH_MEM_PAGE_AUG, &args);
 
 	*ext_err1 = args.rcx;

Re: [PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[Edgecombe, Rick P]

On Tue, 2026-08-25 at 10:55 +0800, Yan Zhao wrote:
> Do not drop the "level" parameter instead, as it will be needed once huge
> page support is added.

TDX and coco has so many plans for the future. An abnormal amount of concurrent
thinking. I think everyone agrees we need to wrangle it by doing things
iteratively, and cheating when it comes to uABI.

Since we have TDX huge pages coming so soon, it seems ok to leave it. But I just
wanted to say having the level arg here originally was wrong. So this should not
be precedent for pre-enabling of things years in the future.

Re: [PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[Sean Christopherson]

On Tue, Aug 25, 2026, Rick P Edgecombe wrote:
> On Tue, 2026-08-25 at 10:55 +0800, Yan Zhao wrote:
> > Do not drop the "level" parameter instead, as it will be needed once huge
> > page support is added.
> 
> TDX and coco has so many plans for the future. An abnormal amount of concurrent
> thinking. I think everyone agrees we need to wrangle it by doing things
> iteratively, and cheating when it comes to uABI.
> 
> Since we have TDX huge pages coming so soon, it seems ok to leave it. But I just
> wanted to say having the level arg here originally was wrong. So this should not
> be precedent for pre-enabling of things years in the future.

Eh, I would argue that in this particular case, including @level was ok from a
process perspective, the main issue is the implementation was buggy.  Which is
very arguably _because_ there was no user to validate the code, but given all the
pre-existing KVM MMU code at play, at some point we'd have to insert code to say
"hugepages aren't supported".  E.g. in this specific case, assuming PG_LEVEL_4K
could have been just as bad as having a partially-dead @level.

But nitpicking aside, +1 to not building out unused, untestable infrastructure.

Re: [PATCH] x86/virt/tdx: Warn on !4K level in tdh_mem_page_aug()

[Edgecombe, Rick P]

On Tue, 2026-08-25 at 10:48 -0700, Sean Christopherson wrote:
> > Since we have TDX huge pages coming so soon, it seems ok to leave it. But I
> > just wanted to say having the level arg here originally was wrong. So this
> > should not be precedent for pre-enabling of things years in the future.
> 
> Eh, I would argue that in this particular case, including @level was ok from a
> process perspective, the main issue is the implementation was buggy.  Which is
> very arguably _because_ there was no user to validate the code, but given all
> the pre-existing KVM MMU code at play, at some point we'd have to insert code
> to say "hugepages aren't supported".  E.g. in this specific case, assuming
> PG_LEVEL_4K could have been just as bad as having a partially-dead @level.

These days (not back then), tdx_sept_set_private_spte() is a good spot to put
all the "is this a supported S-EPT operation" checking. Having it in one place
is good.

While it is normal for the kernel to not include dead code, it is also a TDX
process problem. We have a lot of people working together. Some on near things
and some on far things. When you discuss your teammates patch, it can be hard to
not think about how it affects your own work. But then you have a design
influenced by hidden and unstable future projects. We run into this kind of
thing all the time. So the slope is especially slippery. To work around it, we a
firmish rule. The rule(s) is:
 - The design of a feature should be about the current state of upstream, not 
   future work.
 - Except, you can think about long term uABI.

Now, we got some different direction from you regarding DPAMT and TDX huge
pages. You wanted them co-designed. And actually Dave was giving us the opposite
direction to do them one at a time. So, partly why I'm elaborating on how we are
doing this as a default.