Re: [PATCH 2/2] x86/virt/tdx: Drop tdh_phymem_page_wbinvd_*()

[Yan Zhao <yan.y.zhao@intel.com>]

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
>