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: <78253405-bff8-476c-a505-3737a499151b@redhat.com>
Date: Wed, 20 Aug 2025 11:51:28 +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:
> 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