public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	 Peter Zijlstra <peterz@infradead.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	 Ingo Molnar <mingo@redhat.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org,  "H. Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	linux-kernel@vger.kernel.org,  kvm@vger.kernel.org,
	Tao Zhang <tao1.zhang@intel.com>,
	 Jim Mattson <jmattson@google.com>,
	Brendan Jackman <jackmanb@google.com>
Subject: Re: [PATCH 3/3] x86/mmio: Unify VERW mitigation for guests
Date: Wed, 29 Oct 2025 17:27:37 -0700	[thread overview]
Message-ID: <aQKw-a73mo1nLiJw@google.com> (raw)
In-Reply-To: <20251029-verw-vm-v1-3-babf9b961519@linux.intel.com>

On Wed, Oct 29, 2025, Pawan Gupta wrote:
> When a system is only affected by MMIO Stale Data, VERW mitigation is
> currently handled differently than other data sampling attacks like
> MDS/TAA/RFDS, that do the VERW in asm. This is because for MMIO Stale Data,
> VERW is needed only when the guest can access host MMIO, this was tricky to
> check in asm.
> 
> Refactoring done by:
> 
>   83ebe7157483 ("KVM: VMX: Apply MMIO Stale Data mitigation if KVM maps
>   MMIO into the guest")
> 
> now makes it easier to execute VERW conditionally in asm based on
> VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO.
> 
> Unify MMIO Stale Data mitigation with other VERW-based mitigations and only
> have single VERW callsite in __vmx_vcpu_run(). Remove the now unnecessary
> call to x86_clear_cpu_buffer() in vmx_vcpu_enter_exit().
> 
> This also untangles L1D Flush and MMIO Stale Data mitigation. Earlier, an
> L1D Flush would skip the VERW for MMIO Stale Data. Now, both the
> mitigations are independent of each other. Although, this has little
> practical implication since there are no CPUs that are affected by L1TF and
> are *only* affected by MMIO Stale Data (i.e. not affected by MDS/TAA/RFDS).
> But, this makes the code cleaner and easier to maintain.

Heh, and largely makes our discussion on the L1TF cleanup moot :-)

> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
> ---

...

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 451be757b3d1b2fec6b2b79157f26dd43bc368b8..303935882a9f8d1d8f81a499cdce1fdc8dad62f0 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -903,9 +903,16 @@ unsigned int __vmx_vcpu_run_flags(struct vcpu_vmx *vmx)
>  	if (!msr_write_intercepted(vmx, MSR_IA32_SPEC_CTRL))
>  		flags |= VMX_RUN_SAVE_SPEC_CTRL;
>  
> -	if (static_branch_unlikely(&cpu_buf_vm_clear_mmio_only) &&
> -	    kvm_vcpu_can_access_host_mmio(&vmx->vcpu))
> -		flags |= VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO;
> +	/*
> +	 * When affected by MMIO Stale Data only (and not other data sampling
> +	 * attacks) only clear for MMIO-capable guests.
> +	 */
> +	if (static_branch_unlikely(&cpu_buf_vm_clear_mmio_only)) {
> +		if (kvm_vcpu_can_access_host_mmio(&vmx->vcpu))
> +			flags |= VMX_RUN_CLEAR_CPU_BUFFERS;
> +	} else {
> +		flags |= VMX_RUN_CLEAR_CPU_BUFFERS;
> +	}

This is quire confusing and subtle.  E.g. it requires the reader to know that
cpu_buf_vm_clear_mmio_only is mutually exlusive with X86_FEATURE_CLEAR_CPU_BUF,
and that VMX_RUN_CLEAR_CPU_BUFFERS is ignored if X86_FEATURE_CLEAR_CPU_BUF=n.

At least, I think that's how it works :-)

Isn't the above equivalent to this when all is said and done?

	if (cpu_feature_enabled(X86_FEATURE_CLEAR_CPU_BUF) ||
	    (static_branch_unlikely(&cpu_buf_vm_clear_mmio_only) &&
	     kvm_vcpu_can_access_host_mmio(&vmx->vcpu)))
		flags |= VMX_RUN_CLEAR_CPU_BUFFERS;

>  
>  	return flags;
>  }
> @@ -7320,21 +7327,8 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
>  
>  	guest_state_enter_irqoff();
>  
> -	/*
> -	 * L1D Flush includes CPU buffer clear to mitigate MDS, but VERW
> -	 * mitigation for MDS is done late in VMentry and is still
> -	 * executed in spite of L1D Flush. This is because an extra VERW
> -	 * should not matter much after the big hammer L1D Flush.
> -	 *
> -	 * cpu_buf_vm_clear is used when system is not vulnerable to MDS/TAA,
> -	 * and is affected by MMIO Stale Data. In such cases mitigation in only
> -	 * needed against an MMIO capable guest.
> -	 */
>  	if (static_branch_unlikely(&vmx_l1d_should_flush))
>  		vmx_l1d_flush(vcpu);
> -	else if (static_branch_unlikely(&cpu_buf_vm_clear) &&
> -		 (flags & VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO))
> -		x86_clear_cpu_buffers();
>  
>  	vmx_disable_fb_clear(vmx);

  reply	other threads:[~2025-10-30  0:27 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-29 21:26 [PATCH 0/3] Unify VERW mitigation for guests Pawan Gupta
2025-10-29 21:26 ` [PATCH 1/3] x86/bugs: Use VM_CLEAR_CPU_BUFFERS in VMX as well Pawan Gupta
2025-10-29 22:13   ` Pawan Gupta
2025-10-30 12:28   ` Brendan Jackman
2025-10-30 18:43     ` Pawan Gupta
2025-10-31 11:25       ` Brendan Jackman
2025-10-29 21:26 ` [PATCH 2/3] x86/mmio: Rename cpu_buf_vm_clear to cpu_buf_vm_clear_mmio_only Pawan Gupta
2025-10-30  0:18   ` Sean Christopherson
2025-10-30  5:40     ` Pawan Gupta
2025-10-30 12:29   ` Brendan Jackman
2025-10-30 16:56     ` Pawan Gupta
2025-10-29 21:26 ` [PATCH 3/3] x86/mmio: Unify VERW mitigation for guests Pawan Gupta
2025-10-30  0:27   ` Sean Christopherson [this message]
2025-10-30  6:11     ` Pawan Gupta
2025-10-30  0:33   ` Pawan Gupta
2025-10-30  5:52     ` Yao Yuan
2025-10-30  6:17       ` Pawan Gupta
2025-10-30 12:52   ` Brendan Jackman
2025-10-30 16:06     ` Sean Christopherson
2025-10-30 16:26       ` Brendan Jackman
2025-10-30 18:06         ` Pawan Gupta
2025-10-30 17:54       ` Pawan Gupta
2025-10-30 17:28     ` Pawan Gupta
2025-10-30 18:21       ` Sean Christopherson
2025-10-30 19:11         ` Pawan Gupta
2025-10-30  0:29 ` [PATCH 0/3] " Sean Christopherson
2025-10-30 10:28   ` Borislav Petkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aQKw-a73mo1nLiJw@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jackmanb@google.com \
    --cc=jmattson@google.com \
    --cc=jpoimboe@kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pawan.kumar.gupta@linux.intel.com \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tao1.zhang@intel.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox