linux-kernel - Re: [PATCH v3 6/6] KVM: TDX: Explicitly do WBINVD upon reboot notifier

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: <aGTl09wV1Kt6b0Hz@intel.com>
Date: Wed, 2 Jul 2025 15:54:59 +0800
From: Chao Gao <chao.gao@...el.com>
To: Kai Huang <kai.huang@...el.com>
CC: <dave.hansen@...el.com>, <bp@...en8.de>, <tglx@...utronix.de>,
	<peterz@...radead.org>, <mingo@...hat.com>, <hpa@...or.com>,
	<thomas.lendacky@....com>, <x86@...nel.org>,
	<kirill.shutemov@...ux.intel.com>, <rick.p.edgecombe@...el.com>,
	<linux-kernel@...r.kernel.org>, <pbonzini@...hat.com>, <seanjc@...gle.com>,
	<kvm@...r.kernel.org>, <reinette.chatre@...el.com>,
	<isaku.yamahata@...el.com>, <dan.j.williams@...el.com>,
	<ashish.kalra@....com>, <nik.borisov@...e.com>, <sagis@...gle.com>, "Farrah
 Chen" <farrah.chen@...el.com>
Subject: Re: [PATCH v3 6/6] KVM: TDX: Explicitly do WBINVD upon reboot
 notifier

>--- a/arch/x86/virt/vmx/tdx/tdx.c
>+++ b/arch/x86/virt/vmx/tdx/tdx.c
>@@ -1870,3 +1870,12 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page)
> 	return seamcall(TDH_PHYMEM_PAGE_WBINVD, &args);
> }
> EXPORT_SYMBOL_GPL(tdh_phymem_page_wbinvd_hkid);
>+
>+void tdx_cpu_flush_cache(void)
>+{
>+	lockdep_assert_preemption_disabled();
>+
>+	wbinvd();

Shouldn't you check the per-CPU variable first? so that WBINVD can be
skipped if there is nothing incoherent.

And reboot notifier looks the wrong place for WBINVD. Because SEAMCALLs
(see below) called after the reboot notifier will set the per-CPU variable
again. So in some cases, this patch will result in an *extra* WBINVD
instead of moving WBINVD to an earlier stage.

kernel_kexec()
  ->kernel_restart_prepare()
    ->blocking_notifier_call_chain() // reboot notifier
  ->syscore_shutdown()
    -> ...
      ->tdx_disable_virtualization_cpu()
        ->tdx_flush_vp()

>+	this_cpu_write(cache_state_incoherent, false);
>+}
>+EXPORT_SYMBOL_GPL(tdx_cpu_flush_cache);

I wonder why we don't simply perform WBINVD in
vt_disable_virtualization_cpu() after VMXOFF, i.e.,

diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index d1e02e567b57..1ad3c28b8eff 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -19,6 +19,8 @@ static void vt_disable_virtualization_cpu(void)
	if (enable_tdx)
		tdx_disable_virtualization_cpu();
	vmx_disable_virtualization_cpu();
+	/* Explain why WBINVD is needed */
+	if (enable_tdx)
+		wbinvd();
 }
 
 static __init int vt_hardware_setup(void)

It can solve the cache line aliasing problem and is much simpler than
patches 1-2 and 6.