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
