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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <42009813-2a68-4147-b863-5a3f5ef7b85d@redhat.com>
Date: Wed, 20 Aug 2025 17:39:21 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: "Huang, Kai" <kai.huang@...el.com>, "seanjc@...gle.com"
 <seanjc@...gle.com>
Cc: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "ashish.kalra@....com" <ashish.kalra@....com>,
 "Hansen, Dave" <dave.hansen@...el.com>,
 "thomas.lendacky@....com" <thomas.lendacky@....com>,
 "kas@...nel.org" <kas@...nel.org>,
 "Chatre, Reinette" <reinette.chatre@...el.com>,
 "dwmw@...zon.co.uk" <dwmw@...zon.co.uk>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "mingo@...hat.com" <mingo@...hat.com>,
 "Yamahata, Isaku" <isaku.yamahata@...el.com>,
 "nik.borisov@...e.com" <nik.borisov@...e.com>,
 "tglx@...utronix.de" <tglx@...utronix.de>, "hpa@...or.com" <hpa@...or.com>,
 "peterz@...radead.org" <peterz@...radead.org>,
 "sagis@...gle.com" <sagis@...gle.com>, "Chen, Farrah"
 <farrah.chen@...el.com>, "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
 "bp@...en8.de" <bp@...en8.de>,
 "binbin.wu@...ux.intel.com" <binbin.wu@...ux.intel.com>,
 "Gao, Chao" <chao.gao@...el.com>, "Williams, Dan J"
 <dan.j.williams@...el.com>, "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v6 7/7] KVM: TDX: Explicitly do WBINVD when no more TDX
 SEAMCALLs

On 8/19/25 23:53, Huang, Kai wrote:
> 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.

I agree about not checking everywhere.  But I think this is a good
compromise too if v6 is not acceptable as is (and as far as I am
concerned, it would be):

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,8 @@ 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)
+#ifdef CONFIG_KEXEC_CORE
+void tdx_cpu_flush_cache_for_kexec(void)
  {
  	lockdep_assert_preemption_disabled();
  
@@ -1881,4 +1881,5 @@ 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);
+#endif


It solves pretty much all the objections that Sean had, in one fell
swoop.  The name clearly references kexec, it's stubbed out if not
in use, and it's not anymore unnecessarily under CONFIG_INTEL_TDX_HOST.

Paolo