* [PATCH 0/2] Drop unneeded cache flushing
@ 2026-09-22 20:52 Rick Edgecombe
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
0 siblings, 2 replies; 13+ messages in thread
From: Rick Edgecombe @ 2026-09-22 20:52 UTC (permalink / raw)
To: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
Cc: rick.p.edgecombe
Hi TDX developers,
Can I get a round of review before we ask Dave and Sean to look at it?
In the early days of TDX it was assumed the HW would need some cache
flushing help around transitioning memory between KeyIDs, and so TDX arch
was designed to specify how the VMM needed to help the module perform
this. But it turned out the HW was able to handle these issues without SW
help. For some reason the TDX arch around performing the flushes remained.
Now the TDX arch is getting fixed and Linux can remove the code that was
helping perform this unneeded cache flushing. The changes will appear in
a future documentation update.
Especially, please consider if the explanation is clear enough for
maintainers and future git archaeologists.
As some more context, while this is nice cleanup as is, merging these
patches will let us drop 2 patches out of the TDX huge pages series. Then
that series doesn't need to modify these flushes to work with TDX huge
pages. It also addresses Kiryl's comment about the cache flushing in
tdh_mem_page_aug() not respecting the 'level' arg, or warning about that
non-4KB sizes are not supported.[0]
Based on v7.3-rc1.
Thanks,
Rick
[0] https://lore.kernel.org/lkml/20260825025533.25663-1-yan.y.zhao@intel.com/#t
Rick Edgecombe (2):
x86/virt/tdx: Drop tdx_clflush_*()
x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
arch/x86/include/asm/tdx.h | 2 --
arch/x86/kvm/vmx/tdx.c | 14 -----------
arch/x86/virt/vmx/tdx/tdx.c | 49 -------------------------------------
arch/x86/virt/vmx/tdx/tdx.h | 1 -
4 files changed, 66 deletions(-)
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-22 20:52 [PATCH 0/2] Drop unneeded cache flushing Rick Edgecombe
@ 2026-09-22 20:52 ` Rick Edgecombe
2026-09-23 5:25 ` Yan Zhao
` (2 more replies)
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
1 sibling, 3 replies; 13+ messages in thread
From: Rick Edgecombe @ 2026-09-22 20:52 UTC (permalink / raw)
To: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
Cc: rick.p.edgecombe
The TDX module exposes a bit (CLFLUSH_BEFORE_ALLOC) to say whether it
requires some memory to be flushed from the cache before being handed
to the TDX module.
During the initial TDX upstreaming, it was decided it would be simplest
to just always flush in these cases, rather than checking for the bit and
having conditional logic. So currently the kernel flushes in several
places when handing memory to the TDX module.
In practice this bit has never been set by any TDX module, so flushing
is unneeded. Going forward the TDX docs will be updated to clarify that
CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
So make code even simpler by just deleting all the CLFLUSH_BEFORE_ALLOC
handling.
AI was used to review the change.
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/virt/vmx/tdx/tdx.c | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 1b9ff749dd8e0..8f9d3588f200f 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1619,22 +1619,6 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
return page_to_phys(td->tdr_page);
}
-/*
- * The TDX module exposes a CLFLUSH_BEFORE_ALLOC bit to specify whether
- * a CLFLUSH of pages is required before handing them to the TDX module.
- * Be conservative and make the code simpler by doing the CLFLUSH
- * unconditionally.
- */
-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)
-{
- clflush_cache_range(__va(PFN_PHYS(pfn)), PAGE_SIZE);
-}
-
static int pg_level_to_tdx_sept_level(enum pg_level level)
{
WARN_ON_ONCE(level == PG_LEVEL_NONE);
@@ -1656,7 +1640,6 @@ u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
.rdx = tdx_tdr_pa(td),
};
- tdx_clflush_page(tdcs_page);
return seamcall(TDH_MNG_ADDCX, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_mng_addcx);
@@ -1672,7 +1655,6 @@ u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *sou
};
u64 ret;
- tdx_clflush_pfn(pfn);
ret = seamcall_ret(TDH_MEM_PAGE_ADD, &args);
*ext_err1 = args.rcx;
@@ -1692,7 +1674,6 @@ u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level,
};
u64 ret;
- tdx_clflush_page(page);
ret = seamcall_ret(TDH_MEM_SEPT_ADD, &args);
*ext_err1 = args.rcx;
@@ -1709,7 +1690,6 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
.rdx = vp->tdvpr_pa,
};
- tdx_clflush_page(tdcx_page);
return seamcall(TDH_VP_ADDCX, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_vp_addcx);
@@ -1724,7 +1704,6 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
};
u64 ret;
- tdx_clflush_pfn(pfn);
ret = seamcall_ret(TDH_MEM_PAGE_AUG, &args);
*ext_err1 = args.rcx;
@@ -1769,7 +1748,6 @@ u64 tdh_mng_create(struct tdx_td *td, u16 hkid)
.rdx = hkid,
};
- tdx_clflush_page(td->tdr_page);
return seamcall(TDH_MNG_CREATE, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_mng_create);
@@ -1781,7 +1759,6 @@ u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp)
.rdx = tdx_tdr_pa(td),
};
- tdx_clflush_page(vp->tdvpr_page);
return seamcall(TDH_VP_CREATE, &args);
}
EXPORT_SYMBOL_FOR_KVM(tdh_vp_create);
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
2026-09-22 20:52 [PATCH 0/2] Drop unneeded cache flushing Rick Edgecombe
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
@ 2026-09-22 20:52 ` Rick Edgecombe
2026-09-23 2:18 ` Xiaoyao Li
` (2 more replies)
1 sibling, 3 replies; 13+ messages in thread
From: Rick Edgecombe @ 2026-09-22 20:52 UTC (permalink / raw)
To: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
Cc: rick.p.edgecombe
In the early days of TDX it was expected that pages being reclaimed from
the TDX module would need any cacheline from their private KeyID alias
flushed. The TDX docs describe doing this in a couple cases, for example
reclaiming the TDR memory:
Flush MODIFIED cache lines: this is required to avoid corruption due to
cache line aliasing. Note that all cache lines for all other TD pages
must have been flushed before the TDR page was reclaimed.
For a similar reasons as why the CLFLUSH_BEFORE_ALLOC features0 bit was
never actually set in any TDX module, this flush of the private KeyID
cachelines turned out to not be needed by the final TDX solution. However,
the TDX docs were never updated to reflect that.
Since private KeyID virtual mappings are reserved for TDX use, the TDX
module has to do the clflush itself. The TDH.PHYMEM.PAGE.WBINVD SEAMCALL
leaf was created for this purpose.
So drop the kernel code that calls this SEAMCALL and the two helpers that
facilitate it. With the removal of tdh_phymem_page_wbinvd_tdr(),
tdx_global_keyid no longer needs to be kept around as a global variable.
It just needs to be selected and provided to the TDX module during initial
setup. However, don't churn the code to remove the global variable because
it fits neatly with the other KeyID partitioning global data. It is also
foreseeable that this would be used in the future for other SEAMCALLs that
require the passing the global KeyID.
AI was used to review the change.
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/tdx.h | 2 --
arch/x86/kvm/vmx/tdx.c | 14 --------------
arch/x86/virt/vmx/tdx/tdx.c | 26 --------------------------
arch/x86/virt/vmx/tdx/tdx.h | 1 -
4 files changed, 43 deletions(-)
diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index 89e97d5761d89..39e3c1f8a48e1 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -174,8 +174,6 @@ u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64
u64 tdh_mem_track(struct tdx_td *tdr);
u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
u64 tdh_phymem_cache_wb(bool resume);
-u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td);
-u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn);
#else
static inline void tdx_init(void) { }
static inline u32 tdx_get_nr_guest_keyids(void) { return 0; }
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index b272c20586a74..51327b5d5784a 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -550,7 +550,6 @@ void tdx_mmu_release_hkid(struct kvm *kvm)
static void tdx_reclaim_td_control_pages(struct kvm *kvm)
{
struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
- u64 err;
int i;
/*
@@ -578,15 +577,6 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
if (__tdx_reclaim_page(kvm_tdx->td.tdr_page))
return;
- /*
- * Use a SEAMCALL to ask the TDX module to flush the cache based on the
- * KeyID. TDX module may access TDR while operating on TD (Especially
- * when it is reclaiming TDCS).
- */
- err = tdh_phymem_page_wbinvd_tdr(&kvm_tdx->td);
- if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
- return;
-
tdx_quirk_reset_paddr(page_to_phys(kvm_tdx->td.tdr_page), PAGE_SIZE);
__free_page(kvm_tdx->td.tdr_page);
@@ -1814,10 +1804,6 @@ static int tdx_sept_remove_leaf_spte(struct kvm *kvm, gfn_t gfn,
if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_REMOVE, entry, level_state, kvm))
return -EIO;
- err = tdh_phymem_page_wbinvd_hkid((u16)kvm_tdx->hkid, pfn);
- if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
- return -EIO;
-
tdx_quirk_reset_paddr(PFN_PHYS(pfn), PAGE_SIZE);
return 0;
}
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index 8f9d3588f200f..21f7fce38aa46 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1956,32 +1956,6 @@ u64 tdh_phymem_cache_wb(bool resume)
}
EXPORT_SYMBOL_FOR_KVM(tdh_phymem_cache_wb);
-static inline u64 mk_keyed_paddr(u16 hkid, kvm_pfn_t pfn)
-{
- /* KeyID bits are just above the physical address bits. */
- return PFN_PHYS(pfn) | ((u64)hkid << boot_cpu_data.x86_phys_bits);
-}
-
-u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
-{
- struct tdx_module_args args = {};
-
- args.rcx = mk_keyed_paddr(tdx_global_keyid, page_to_pfn(td->tdr_page));
-
- return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
-}
-EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_tdr);
-
-u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn)
-{
- struct tdx_module_args args = {};
-
- args.rcx = mk_keyed_paddr(hkid, pfn);
-
- return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
-}
-EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
-
void tdx_sys_disable(void)
{
struct tdx_module_args args = {};
diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
index bdfd0e1e337ac..3289be59d14fc 100644
--- a/arch/x86/virt/vmx/tdx/tdx.h
+++ b/arch/x86/virt/vmx/tdx/tdx.h
@@ -43,7 +43,6 @@
#define TDH_SYS_TDMR_INIT 36
#define TDH_MEM_TRACK 38
#define TDH_PHYMEM_CACHE_WB 40
-#define TDH_PHYMEM_PAGE_WBINVD 41
#define TDH_VP_WR 43
#define TDH_SYS_CONFIG 45
#define TDH_SYS_SHUTDOWN 52
--
2.55.0
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
@ 2026-09-23 2:18 ` Xiaoyao Li
2026-09-23 16:31 ` Edgecombe, Rick P
2026-09-23 5:41 ` Yan Zhao
2026-09-23 7:09 ` Xu Yilun
2 siblings, 1 reply; 13+ messages in thread
From: Xiaoyao Li @ 2026-09-23 2:18 UTC (permalink / raw)
To: Rick Edgecombe, pbonzini, dave.hansen, seanjc, x86, kas,
adrian.hunter, yan.y.zhao, binbin.wu, chao.gao, linux-kernel,
kvm
On 9/23/2026 4:52 AM, Rick Edgecombe wrote:
> In the early days of TDX it was expected that pages being reclaimed from
> the TDX module would need any cacheline from their private KeyID alias
> flushed. The TDX docs describe doing this in a couple cases, for example
> reclaiming the TDR memory:
> Flush MODIFIED cache lines: this is required to avoid corruption due to
> cache line aliasing. Note that all cache lines for all other TD pages
> must have been flushed before the TDR page was reclaimed.
>
> For a similar reasons as why the CLFLUSH_BEFORE_ALLOC features0 bit was
> never actually set in any TDX module, this flush of the private KeyID
> cachelines turned out to not be needed by the final TDX solution. However,
> the TDX docs were never updated to reflect that.
>
> Since private KeyID virtual mappings are reserved for TDX use, the TDX
> module has to do the clflush itself. The TDH.PHYMEM.PAGE.WBINVD SEAMCALL
> leaf was created for this purpose.
>
> So drop the kernel code that calls this SEAMCALL and the two helpers that
> facilitate it. With the removal of tdh_phymem_page_wbinvd_tdr(),
> tdx_global_keyid no longer needs to be kept around as a global variable.
> It just needs to be selected and provided to the TDX module during initial
> setup. However, don't churn the code to remove the global variable because
> it fits neatly with the other KeyID partitioning global data. It is also
> foreseeable that this would be used in the future for other SEAMCALLs that
> require the passing the global KeyID.
What about TDH.PHYMEM.CACHE.WB? Can it be dropped as well?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
@ 2026-09-23 5:25 ` Yan Zhao
2026-09-23 7:30 ` Binbin Wu
2026-09-23 7:47 ` Tony Lindgren
2026-09-23 11:58 ` Nikolay Borisov
2 siblings, 1 reply; 13+ messages in thread
From: Yan Zhao @ 2026-09-23 5:25 UTC (permalink / raw)
To: Rick Edgecombe
Cc: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
Just some nits to the commit log. Free free to ignore.
On Tue, Sep 22, 2026 at 01:52:14PM -0700, Rick Edgecombe wrote:
> The TDX module exposes a bit (CLFLUSH_BEFORE_ALLOC) to say whether it
> requires some memory to be flushed from the cache before being handed
Drop "some"?
> to the TDX module.
>
> During the initial TDX upstreaming, it was decided it would be simplest
> to just always flush in these cases, rather than checking for the bit and
"in these cases" is ambiguous.
> having conditional logic. So currently the kernel flushes in several
> places when handing memory to the TDX module.
>
> In practice this bit has never been set by any TDX module, so flushing
> is unneeded. Going forward the TDX docs will be updated to clarify that
"Going forward, ...".
> CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
>
> So make code even simpler by just deleting all the CLFLUSH_BEFORE_ALLOC
> handling.
>
> AI was used to review the change.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
> ---
> arch/x86/virt/vmx/tdx/tdx.c | 23 -----------------------
> 1 file changed, 23 deletions(-)
>
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 1b9ff749dd8e0..8f9d3588f200f 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1619,22 +1619,6 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
> return page_to_phys(td->tdr_page);
> }
>
> -/*
> - * The TDX module exposes a CLFLUSH_BEFORE_ALLOC bit to specify whether
> - * a CLFLUSH of pages is required before handing them to the TDX module.
> - * Be conservative and make the code simpler by doing the CLFLUSH
> - * unconditionally.
> - */
> -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)
> -{
> - clflush_cache_range(__va(PFN_PHYS(pfn)), PAGE_SIZE);
> -}
> -
> static int pg_level_to_tdx_sept_level(enum pg_level level)
> {
> WARN_ON_ONCE(level == PG_LEVEL_NONE);
> @@ -1656,7 +1640,6 @@ u64 tdh_mng_addcx(struct tdx_td *td, struct page *tdcs_page)
> .rdx = tdx_tdr_pa(td),
> };
>
> - tdx_clflush_page(tdcs_page);
> return seamcall(TDH_MNG_ADDCX, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mng_addcx);
> @@ -1672,7 +1655,6 @@ u64 tdh_mem_page_add(struct tdx_td *td, u64 gpa, kvm_pfn_t pfn, struct page *sou
> };
> u64 ret;
>
> - tdx_clflush_pfn(pfn);
> ret = seamcall_ret(TDH_MEM_PAGE_ADD, &args);
>
> *ext_err1 = args.rcx;
> @@ -1692,7 +1674,6 @@ u64 tdh_mem_sept_add(struct tdx_td *td, u64 gpa, enum pg_level level,
> };
> u64 ret;
>
> - tdx_clflush_page(page);
> ret = seamcall_ret(TDH_MEM_SEPT_ADD, &args);
>
> *ext_err1 = args.rcx;
> @@ -1709,7 +1690,6 @@ u64 tdh_vp_addcx(struct tdx_vp *vp, struct page *tdcx_page)
> .rdx = vp->tdvpr_pa,
> };
>
> - tdx_clflush_page(tdcx_page);
> return seamcall(TDH_VP_ADDCX, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_vp_addcx);
> @@ -1724,7 +1704,6 @@ u64 tdh_mem_page_aug(struct tdx_td *td, u64 gpa, enum pg_level level,
> };
> u64 ret;
>
> - tdx_clflush_pfn(pfn);
> ret = seamcall_ret(TDH_MEM_PAGE_AUG, &args);
>
> *ext_err1 = args.rcx;
> @@ -1769,7 +1748,6 @@ u64 tdh_mng_create(struct tdx_td *td, u16 hkid)
> .rdx = hkid,
> };
>
> - tdx_clflush_page(td->tdr_page);
> return seamcall(TDH_MNG_CREATE, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_mng_create);
> @@ -1781,7 +1759,6 @@ u64 tdh_vp_create(struct tdx_td *td, struct tdx_vp *vp)
> .rdx = tdx_tdr_pa(td),
> };
>
> - tdx_clflush_page(vp->tdvpr_page);
> return seamcall(TDH_VP_CREATE, &args);
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_vp_create);
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
2026-09-23 2:18 ` Xiaoyao Li
@ 2026-09-23 5:41 ` Yan Zhao
2026-09-23 7:09 ` Xu Yilun
2 siblings, 0 replies; 13+ messages in thread
From: Yan Zhao @ 2026-09-23 5:41 UTC (permalink / raw)
To: Rick Edgecombe
Cc: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
On Tue, Sep 22, 2026 at 01:52:15PM -0700, Rick Edgecombe wrote:
> In the early days of TDX it was expected that pages being reclaimed from
> the TDX module would need any cacheline from their private KeyID alias
> flushed. The TDX docs describe doing this in a couple cases, for example
> reclaiming the TDR memory:
> Flush MODIFIED cache lines: this is required to avoid corruption due to
> cache line aliasing. Note that all cache lines for all other TD pages
> must have been flushed before the TDR page was reclaimed.
Is the spec quote necessary?
The mention of "Note that all cache lines for all other TD pages must have been
flushed before the TDR page was reclaimed" seems irrelevant to this patch.
> For a similar reasons as why the CLFLUSH_BEFORE_ALLOC features0 bit was
s/reasons/reason
> never actually set in any TDX module, this flush of the private KeyID
> cachelines turned out to not be needed by the final TDX solution. However,
> the TDX docs were never updated to reflect that.
>
> Since private KeyID virtual mappings are reserved for TDX use, the TDX
> module has to do the clflush itself. The TDH.PHYMEM.PAGE.WBINVD SEAMCALL
> leaf was created for this purpose.
Should this paragraph appear before the last one "For a similar..."?
> So drop the kernel code that calls this SEAMCALL and the two helpers that
> facilitate it. With the removal of tdh_phymem_page_wbinvd_tdr(),
> tdx_global_keyid no longer needs to be kept around as a global variable.
> It just needs to be selected and provided to the TDX module during initial
> setup. However, don't churn the code to remove the global variable because
> it fits neatly with the other KeyID partitioning global data. It is also
> foreseeable that this would be used in the future for other SEAMCALLs that
> require the passing the global KeyID.
>
> AI was used to review the change.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
For the patch implementation:
Reviewed-by: Yan Zhao <yan.y.zhao@intel.com>
> ---
> arch/x86/include/asm/tdx.h | 2 --
> arch/x86/kvm/vmx/tdx.c | 14 --------------
> arch/x86/virt/vmx/tdx/tdx.c | 26 --------------------------
> arch/x86/virt/vmx/tdx/tdx.h | 1 -
> 4 files changed, 43 deletions(-)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 89e97d5761d89..39e3c1f8a48e1 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -174,8 +174,6 @@ u64 tdh_phymem_page_reclaim(struct page *page, u64 *tdx_pt, u64 *tdx_owner, u64
> u64 tdh_mem_track(struct tdx_td *tdr);
> u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, enum pg_level level, u64 *ext_err1, u64 *ext_err2);
> u64 tdh_phymem_cache_wb(bool resume);
> -u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td);
> -u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn);
> #else
> static inline void tdx_init(void) { }
> static inline u32 tdx_get_nr_guest_keyids(void) { return 0; }
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index b272c20586a74..51327b5d5784a 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
> @@ -550,7 +550,6 @@ void tdx_mmu_release_hkid(struct kvm *kvm)
> static void tdx_reclaim_td_control_pages(struct kvm *kvm)
> {
> struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
> - u64 err;
> int i;
>
> /*
> @@ -578,15 +577,6 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
> if (__tdx_reclaim_page(kvm_tdx->td.tdr_page))
> return;
>
> - /*
> - * Use a SEAMCALL to ask the TDX module to flush the cache based on the
> - * KeyID. TDX module may access TDR while operating on TD (Especially
> - * when it is reclaiming TDCS).
> - */
> - err = tdh_phymem_page_wbinvd_tdr(&kvm_tdx->td);
> - if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
> - return;
> -
> tdx_quirk_reset_paddr(page_to_phys(kvm_tdx->td.tdr_page), PAGE_SIZE);
>
> __free_page(kvm_tdx->td.tdr_page);
> @@ -1814,10 +1804,6 @@ static int tdx_sept_remove_leaf_spte(struct kvm *kvm, gfn_t gfn,
> if (TDX_BUG_ON_2(err, TDH_MEM_PAGE_REMOVE, entry, level_state, kvm))
> return -EIO;
>
> - err = tdh_phymem_page_wbinvd_hkid((u16)kvm_tdx->hkid, pfn);
> - if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
> - return -EIO;
> -
> tdx_quirk_reset_paddr(PFN_PHYS(pfn), PAGE_SIZE);
> return 0;
> }
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 8f9d3588f200f..21f7fce38aa46 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1956,32 +1956,6 @@ u64 tdh_phymem_cache_wb(bool resume)
> }
> EXPORT_SYMBOL_FOR_KVM(tdh_phymem_cache_wb);
>
> -static inline u64 mk_keyed_paddr(u16 hkid, kvm_pfn_t pfn)
> -{
> - /* KeyID bits are just above the physical address bits. */
> - return PFN_PHYS(pfn) | ((u64)hkid << boot_cpu_data.x86_phys_bits);
> -}
> -
> -u64 tdh_phymem_page_wbinvd_tdr(struct tdx_td *td)
> -{
> - struct tdx_module_args args = {};
> -
> - args.rcx = mk_keyed_paddr(tdx_global_keyid, page_to_pfn(td->tdr_page));
> -
> - return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> -}
> -EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_tdr);
> -
> -u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, kvm_pfn_t pfn)
> -{
> - struct tdx_module_args args = {};
> -
> - args.rcx = mk_keyed_paddr(hkid, pfn);
> -
> - return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> -}
> -EXPORT_SYMBOL_FOR_KVM(tdh_phymem_page_wbinvd_hkid);
> -
> void tdx_sys_disable(void)
> {
> struct tdx_module_args args = {};
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index bdfd0e1e337ac..3289be59d14fc 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -43,7 +43,6 @@
> #define TDH_SYS_TDMR_INIT 36
> #define TDH_MEM_TRACK 38
> #define TDH_PHYMEM_CACHE_WB 40
> -#define TDH_PHYMEM_PAGE_WBINVD 41
> #define TDH_VP_WR 43
> #define TDH_SYS_CONFIG 45
> #define TDH_SYS_SHUTDOWN 52
> --
> 2.55.0
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
2026-09-23 2:18 ` Xiaoyao Li
2026-09-23 5:41 ` Yan Zhao
@ 2026-09-23 7:09 ` Xu Yilun
2 siblings, 0 replies; 13+ messages in thread
From: Xu Yilun @ 2026-09-23 7:09 UTC (permalink / raw)
To: Rick Edgecombe
Cc: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
On Tue, Sep 22, 2026 at 01:52:15PM -0700, Rick Edgecombe wrote:
> In the early days of TDX it was expected that pages being reclaimed from
> the TDX module would need any cacheline from their private KeyID alias
> flushed. The TDX docs describe doing this in a couple cases, for example
> reclaiming the TDR memory:
> Flush MODIFIED cache lines: this is required to avoid corruption due to
> cache line aliasing. Note that all cache lines for all other TD pages
> must have been flushed before the TDR page was reclaimed.
>
> For a similar reasons as why the CLFLUSH_BEFORE_ALLOC features0 bit was
> never actually set in any TDX module, this flush of the private KeyID
> cachelines turned out to not be needed by the final TDX solution. However,
> the TDX docs were never updated to reflect that.
>
> Since private KeyID virtual mappings are reserved for TDX use, the TDX
> module has to do the clflush itself.
This paragraph is a little confusing to me. Here reads like the TDX
module does the clflush internally and the host doesn't have to opt-in.
> The TDH.PHYMEM.PAGE.WBINVD SEAMCALL
> leaf was created for this purpose.
But the SEAMCALL leaf shows host should opt-in.
Ah, you want to say host can't directly do clflush because of some
reason, so we need a SEAMCALL, is it?
[...]
> @@ -578,15 +577,6 @@ static void tdx_reclaim_td_control_pages(struct kvm *kvm)
> if (__tdx_reclaim_page(kvm_tdx->td.tdr_page))
> return;
>
> - /*
> - * Use a SEAMCALL to ask the TDX module to flush the cache based on the
> - * KeyID. TDX module may access TDR while operating on TD (Especially
> - * when it is reclaiming TDCS).
> - */
> - err = tdh_phymem_page_wbinvd_tdr(&kvm_tdx->td);
> - if (TDX_BUG_ON(err, TDH_PHYMEM_PAGE_WBINVD, kvm))
> - return;
> -
> tdx_quirk_reset_paddr(page_to_phys(kvm_tdx->td.tdr_page), PAGE_SIZE);
>
> __free_page(kvm_tdx->td.tdr_page);
After removing the wbinvd, we don't need open code to reclaim tdr
anymore, is it? tdx_reclaim_control_page(kvm_tdx->td.tdr_page)?
This may not be relevant to the topic, not sure if we are good to clean
up it in this series.
Another thing. There are also 2 wbinvd_on_all_cpus() on PAMT
adding/reclaiming. Remove them?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-23 5:25 ` Yan Zhao
@ 2026-09-23 7:30 ` Binbin Wu
0 siblings, 0 replies; 13+ messages in thread
From: Binbin Wu @ 2026-09-23 7:30 UTC (permalink / raw)
To: Yan Zhao, Rick Edgecombe
Cc: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
On 9/23/2026 1:25 PM, Yan Zhao wrote:
> On Tue, Sep 22, 2026 at 01:52:14PM -0700, Rick Edgecombe wrote:
>> The TDX module exposes a bit (CLFLUSH_BEFORE_ALLOC) to say whether it
>> requires some memory to be flushed from the cache before being handed
> Drop "some"?
>
>> to the TDX module.
>>
>> During the initial TDX upstreaming, it was decided it would be simplest
>> to just always flush in these cases, rather than checking for the bit and
> "in these cases" is ambiguous.
I assume "some memory" and "these cases" refer to memory pages allocated for
TD private memory or TD control structures. I agree with Yan that it would be
better to just state that explicitly.
>
>> having conditional logic. So currently the kernel flushes in several
>> places when handing memory to the TDX module.
>>
>> In practice this bit has never been set by any TDX module, so flushing
>> is unneeded. Going forward the TDX docs will be updated to clarify that
> "Going forward, ...".
>
>> CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
>>
>> So make code even simpler by just deleting all the CLFLUSH_BEFORE_ALLOC
>> handling.
>>
>> AI was used to review the change.
>>
>> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
2026-09-23 5:25 ` Yan Zhao
@ 2026-09-23 7:47 ` Tony Lindgren
2026-09-23 15:50 ` Edgecombe, Rick P
2026-09-23 11:58 ` Nikolay Borisov
2 siblings, 1 reply; 13+ messages in thread
From: Tony Lindgren @ 2026-09-23 7:47 UTC (permalink / raw)
To: Rick Edgecombe
Cc: pbonzini, dave.hansen, seanjc, x86, kas, adrian.hunter,
yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao, linux-kernel, kvm
On Tue, Sep 22, 2026 at 01:52:14PM -0700, Rick Edgecombe wrote:
> In practice this bit has never been set by any TDX module, so flushing
> is unneeded. Going forward the TDX docs will be updated to clarify that
> CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
Should there be a separate patch first before these two patches to bail
out if the TDX module TDX_FEATURES0 CLFLUSH_BEFORE_ALLOC bit is ever set?
Then dropping the related code would be safe to do without having to
ponder if it's safe to do.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
2026-09-23 5:25 ` Yan Zhao
2026-09-23 7:47 ` Tony Lindgren
@ 2026-09-23 11:58 ` Nikolay Borisov
2026-09-23 16:26 ` Edgecombe, Rick P
2 siblings, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2026-09-23 11:58 UTC (permalink / raw)
To: Rick Edgecombe, pbonzini, dave.hansen, seanjc, x86, kas,
adrian.hunter, yan.y.zhao, binbin.wu, xiaoyao.li, chao.gao,
linux-kernel, kvm
On 22.09.26 г. 23:52 ч., Rick Edgecombe wrote:
> The TDX module exposes a bit (CLFLUSH_BEFORE_ALLOC) to say whether it
> requires some memory to be flushed from the cache before being handed
> to the TDX module.
>
> During the initial TDX upstreaming, it was decided it would be simplest
> to just always flush in these cases, rather than checking for the bit and
> having conditional logic. So currently the kernel flushes in several
> places when handing memory to the TDX module.
>
> In practice this bit has never been set by any TDX module, so flushing
> is unneeded. Going forward the TDX docs will be updated to clarify that
> CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
>
> So make code even simpler by just deleting all the CLFLUSH_BEFORE_ALLOC
> handling.
>
> AI was used to review the change.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
The code change itself is trivial, however nothing in the code being
removed is tied to CLFLUSH_BEFORE_ALLOC directly. And from the changelog
it's not entirely clear which those operations are (as per other
reviewers feedback). Looking at the TDX module spec that flag is
mentioned in relation to the following 3 operations:
TDH.MEM.SEPT.ADD
TDH.MEM.PAGE.ADD
TDH.MEM.PAGE.AUG
Whereas the patch also removes flushing from MNG_CREATE/VP_ADDCX/MNG_ADDCX
So concrete references in the changelog would be much appreciated i.e
whether flushing for those 3 additional functions was because people
were overzealous, or they simply have been omitted from the public
documentation.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-23 7:47 ` Tony Lindgren
@ 2026-09-23 15:50 ` Edgecombe, Rick P
0 siblings, 0 replies; 13+ messages in thread
From: Edgecombe, Rick P @ 2026-09-23 15:50 UTC (permalink / raw)
To: tony.lindgren
Cc: Gao, Chao, Hansen, Dave, seanjc, x86, kas, Wu, Binbin, Li,
Xiaoyao, linux-kernel, Zhao, Yan Y, Hunter, Adrian, pbonzini,
kvm
On Wed, 2026-09-23 at 10:47 +0300, Tony Lindgren wrote:
> On Tue, Sep 22, 2026 at 01:52:14PM -0700, Rick Edgecombe wrote:
> > In practice this bit has never been set by any TDX module, so flushing
> > is unneeded. Going forward the TDX docs will be updated to clarify that
> > CLFLUSH_BEFORE_ALLOC will never be set in any future TDX module either.
>
> Should there be a separate patch first before these two patches to bail
> out if the TDX module TDX_FEATURES0 CLFLUSH_BEFORE_ALLOC bit is ever set?
>
> Then dropping the related code would be safe to do without having to
> ponder if it's safe to do.
Eh, I don't think we need it. If docs are updated to say TDX will never set it,
then we would really be sanity checking the TDX module specs. Doesn't seem worth
the code.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*()
2026-09-23 11:58 ` Nikolay Borisov
@ 2026-09-23 16:26 ` Edgecombe, Rick P
0 siblings, 0 replies; 13+ messages in thread
From: Edgecombe, Rick P @ 2026-09-23 16:26 UTC (permalink / raw)
To: Gao, Chao, Hansen, Dave, seanjc, x86, kas, Wu, Binbin, Li,
Xiaoyao, linux-kernel, Hunter, Adrian, Zhao, Yan Y, pbonzini,
kvm, nik.borisov
On Wed, 2026-09-23 at 14:58 +0300, Nikolay Borisov wrote:
> The code change itself is trivial, however nothing in the code being
> removed is tied to CLFLUSH_BEFORE_ALLOC directly. And from the changelog
> it's not entirely clear which those operations are (as per other
> reviewers feedback). Looking at the TDX module spec that flag is
> mentioned in relation to the following 3 operations:
>
> TDH.MEM.SEPT.ADD
> TDH.MEM.PAGE.ADD
> TDH.MEM.PAGE.AUG
>
> Whereas the patch also removes flushing from MNG_CREATE/VP_ADDCX/MNG_ADDCX
The docs are a bit too ambiguous for my preference, but in the "ABI spec" it has
this blurb:
When allocating a memory page to be used as TD private memory or TD control
structure page, the host VMM is required to ensure that none of the cache
lines associated with the page is in a MODIFIED state.
So those others should be covered by "TD control structures". It would have been
nice if these bits were included in the base spec flows too. But I think it's a
symptom of that this arch was never really needed.
Similarly, for the system scoped memory that can have reclaim, I would have
expected that the HW limitation reasoning would be the same as the TD scoped
memory. But the docs are even less clear on that point. So the docs were never
super clear on how this should work in general.
Instead we have the original intention of the Linux code, and some recent
promises from TDX arch folks:
- CLFLUSH_BEFORE_ALLOC will not be set in the future
- VMM is not required to call TDH.PHYMEM.PAGE.WBIND on page reclaim on any
platforms supporting TDX.
Does it seem sufficient?
>
>
> So concrete references in the changelog would be much appreciated i.e
> whether flushing for those 3 additional functions was because people
> were overzealous, or they simply have been omitted from the public
> documentation.
I'll try to add something to the effect of the above, if it explains it enough.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()
2026-09-23 2:18 ` Xiaoyao Li
@ 2026-09-23 16:31 ` Edgecombe, Rick P
0 siblings, 0 replies; 13+ messages in thread
From: Edgecombe, Rick P @ 2026-09-23 16:31 UTC (permalink / raw)
To: Gao, Chao, Hansen, Dave, seanjc, x86, kas, Wu, Binbin, Li,
Xiaoyao, linux-kernel, Hunter, Adrian, Zhao, Yan Y, pbonzini,
kvm
On Wed, 2026-09-23 at 10:18 +0800, Xiaoyao Li wrote:
> What about TDH.PHYMEM.CACHE.WB? Can it be dropped as well?
Apparently not. I'm not sure we can't push harder on it to drop that that too,
but it had a slightly different reason for existing apparently. There is a
different flag for it "SKIP_PHYMEM_CACHE_WB" which we don't check.
Since it isn't needed for huge page code reduction, I think it's not a
priority.
Do you think it is worth mentioning here? Since it has a separate enumeration, I
thought it was separate enough to keep the log simpler. Keeping it down to what
is need to understand and not more.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-09-23 16:31 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 20:52 [PATCH 0/2] Drop unneeded cache flushing Rick Edgecombe
2026-09-22 20:52 ` [PATCH 1/2] x86/virt/tdx: Drop tdx_clflush_*() Rick Edgecombe
2026-09-23 5:25 ` Yan Zhao
2026-09-23 7:30 ` Binbin Wu
2026-09-23 7:47 ` Tony Lindgren
2026-09-23 15:50 ` Edgecombe, Rick P
2026-09-23 11:58 ` Nikolay Borisov
2026-09-23 16:26 ` Edgecombe, Rick P
2026-09-22 20:52 ` [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*() Rick Edgecombe
2026-09-23 2:18 ` Xiaoyao Li
2026-09-23 16:31 ` Edgecombe, Rick P
2026-09-23 5:41 ` Yan Zhao
2026-09-23 7:09 ` Xu Yilun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®