Re: [PATCH v6 7/7] KVM: TDX: Explicitly do WBINVD when no more TDX SEAMCALLs

[Paolo Bonzini <pbonzini@redhat.com>]

On 8/19/25 23:53, Huang, Kai wrote:
> On Tue, 2025-08-19 at 12:31 +0200, Paolo Bonzini wrote:
>> 2) ... but anyway, KVM is the wrong place to do the test.  If anything,
>> since we need a v7 to change the unnecessary stub, you could move that
>> stub under #ifndef CONFIG_KEXEC_CORE and rename the function to
>> tdx_cpu_flush_cache_for_kexec().
> 
> Agreed on renaming to tdx_cpu_flush_cache_for_kexec().
> 
> But with the "for_kexec()" part in the function name, it already implies
> it is related to kexec, and I kinda think there's no need to test
> IS_ENABLED(CONFIG_KEXEC_CORE) anymore.
> 
> One of the main purpose of this series is to unblock TDX_HOST and KEXEC in
> the Kconfig, since otherwise I've been told distros will simply choose to
> disable TDX_HOST in the Kconfig.  So in reality, I suppose they will be on
> together probably in like 95% cases, if not 100%.
> 
> If we want to test CONFIG_KEXEC_CORE in tdx_cpu_flush_cache_for_kexec(),
> then it would be a little bit weird that why we don't test it in other
> places, e.g., when setting up the boolean.  Testing it in all places would
> make the code unnecessarily long and harder to read.

No I don't mean testing it there, but just making
tdx_cpu_flush_cache_for_kexec() a stub when CONFIG_KEXEC_CORE is
undefined:

diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
index e9a213582f03..913199b1954b 100644
--- a/arch/x86/include/asm/tdx.h
+++ b/arch/x86/include/asm/tdx.h
@@ -217,7 +217,6 @@ u64 tdh_mem_page_remove(struct tdx_td *td, u64 gpa, u64 level, u64 *ext_err1, u6
  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, struct page *page);
-void tdx_cpu_flush_cache(void);
  #else
  static inline void tdx_init(void) { }
  static inline int tdx_cpu_enable(void) { return -ENODEV; }
@@ -225,8 +224,13 @@ static inline int tdx_enable(void)  { return -ENODEV; }
  static inline u32 tdx_get_nr_guest_keyids(void) { return 0; }
  static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; }
  static inline const struct tdx_sys_info *tdx_get_sysinfo(void) { return NULL; }
-static inline void tdx_cpu_flush_cache(void) { }
  #endif	/* CONFIG_INTEL_TDX_HOST */
  
+#ifdef CONFIG_KEXEC_CORE
+void tdx_cpu_flush_cache_for_kexec(void);
+#else
+static inline void tdx_cpu_flush_cache_for_kexec(void) { }
+#endif
+
  #endif /* !__ASSEMBLER__ */
  #endif /* _ASM_X86_TDX_H */
diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index 93477233baae..376d49ef4472 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -453,7 +453,7 @@ void tdx_disable_virtualization_cpu(void)
  	 * remote CPUs to stop them.  Doing WBINVD in stop_this_cpu()
  	 * could potentially increase the possibility of the "race".
  	 */
-	tdx_cpu_flush_cache();
+	tdx_cpu_flush_cache_for_kexec();
  }
  
  #define TDX_SEAMCALL_RETRIES 10000
diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
index c26e2e07ff6b..cd2a36dbbfc5 100644
--- a/arch/x86/virt/vmx/tdx/tdx.c
+++ b/arch/x86/virt/vmx/tdx/tdx.c
@@ -1871,7 +1871,7 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
  }
  EXPORT_SYMBOL_GPL(tdh_phymem_page_wbinvd_hkid);
  
-void tdx_cpu_flush_cache(void)
+void tdx_cpu_flush_cache_for_kexec(void)
  {
  	lockdep_assert_preemption_disabled();
  
@@ -1881,4 +1881,4 @@ void tdx_cpu_flush_cache(void)
  	wbinvd();
  	this_cpu_write(cache_state_incoherent, false);
  }
-EXPORT_SYMBOL_GPL(tdx_cpu_flush_cache);
+EXPORT_SYMBOL_GPL(tdx_cpu_flush_cache_for_kexec);


Personally, I'm totally okay with v6.  But the above change seems
to me like the best way to obey Sean's objection, better than
adding the test in KVM.

Paolo