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

["Huang, Kai" <kai.huang@intel.com>]

On Wed, 2025-07-02 at 15:54 +0800, Chao Gao wrote:
> 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.

After more thinking, I think the percpu boolean isn't conflicting with
what you suggested above.  As Dave mentioned here[*], it can help catch
wbinvd() at kexec-ing time if something screwed up at earlier time:

  ...
  hopefully at point after you're sure no more TDCALLs are being made. If
  you screw it up, no biggie: the kexec-time one will make up for it,
  exposing TDX systems to the kexec timing bugs. But if the on_each_cpu()
  thing works in the common case, you get no additional bug exposure.

Btw, the reason that I wanted to do wbinvd() in rebooting notifier is it
suits a good place to reset TDX private memory managed by KVM on "partial
write machine check" erratum platforms (which isn't included in this patch
though).  We need to flush cache before resetting TDX private memory.

So while doing wbinvd() after vmx_disable_virtualization_cpu() sounds
promising for cache flush, it is not ideal for handling erratum.  Using
rebooting notifier makes such code self-contained in tdx.c in KVM.

[*]:
https://lore.kernel.org/lkml/31e17bc8-2e9e-4e93-a912-3d54826e59d0@intel.com/