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: Brendan Jackman <jackmanb@google.com>,
	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>
Subject: Re: [PATCH 3/3] x86/mmio: Unify VERW mitigation for guests
Date: Thu, 30 Oct 2025 11:21:52 -0700	[thread overview]
Message-ID: <aQOswAMVciBXu1ud@google.com> (raw)
In-Reply-To: <20251030172836.5ys2wag3dax5fmwk@desk>

On Thu, Oct 30, 2025, Pawan Gupta wrote:
> On Thu, Oct 30, 2025 at 12:52:12PM +0000, Brendan Jackman wrote:
> > On Wed Oct 29, 2025 at 9:26 PM UTC, Pawan Gupta wrote:
> > > +	/* Check EFLAGS.ZF from the VMX_RUN_CLEAR_CPU_BUFFERS bit test above */
> > > +	jz .Lskip_clear_cpu_buffers
> > 
> > Hm, it's a bit weird that we have the "alternative" inside
> > VM_CLEAR_CPU_BUFFERS, but then we still keep the test+jz
> > unconditionally. 
> 
> Exactly, but it is tricky to handle the below 2 cases in asm:
> 
> 1. MDS -> Always do VM_CLEAR_CPU_BUFFERS
> 
> 2. MMIO -> Do VM_CLEAR_CPU_BUFFERS only if guest can access host MMIO

Overloading VM_CLEAR_CPU_BUFFERS for MMIO is all kinds of confusing, e.g. my
pseudo-patch messed things.  It's impossible to understand

> In th MMIO case, one guest may have access to host MMIO while another may
> not. Alternatives alone can't handle this case as they patch code at boot
> which is then set in stone. One way is to move the conditional inside
> VM_CLEAR_CPU_BUFFERS that gets a flag as an arguement.
> 
> > If we really want to super-optimise the no-mitigations-needed case,
> > shouldn't we want to avoid the conditional in the asm if it never
> > actually leads to a flush?
> 
> Ya, so effectively, have VM_CLEAR_CPU_BUFFERS alternative spit out
> conditional VERW when affected by MMIO_only, otherwise an unconditional
> VERW.
> 
> > On the other hand, if we don't mind a couple of extra instructions,
> > shouldn't we be fine with just having the whole asm code based solely
> > on VMX_RUN_CLEAR_CPU_BUFFERS and leaving the
> > X86_FEATURE_CLEAR_CPU_BUF_VM to the C code?
> 
> That's also an option.
> 
> > I guess the issue is that in the latter case we'd be back to having
> > unnecessary inconsistency with AMD code while in the former case... well
> > that would just be really annoying asm code - am I on the right
> > wavelength there? So I'm not necessarily asking for changes here, just
> > probing in case it prompts any interesting insights on your side.
> > 
> > (Also, maybe this test+jz has a similar cost to the nops that the
> > "alternative" would inject anyway...?)
> 
> Likely yes. test+jz is a necessary evil that is needed for MMIO Stale Data
> for different per-guest handling.

I don't like any of those options :-)

I again vote to add X86_FEATURE_CLEAR_CPU_BUF_MMIO, and then have it be mutually
exlusive with X86_FEATURE_CLEAR_CPU_BUF_VM, i.e. be an alterantive, not a subset.
Because as proposed, the MMIO case _isn't_ a strict subset, it's a subset iff
the MMIO mitigation is enabled, otherwise it's something else entirely.

After half an hour of debugging godawful assembler errors because I used stringify()
instead of __stringify(), I was able to get to this, which IMO is readable and
intuitive.

	/* Clobbers EFLAGS.ZF */
	ALTERNATIVE_2 "",							\
		      __CLEAR_CPU_BUFFERS, X86_FEATURE_CLEAR_CPU_BUF_VM,	\
		      __stringify(jz .Lskip_clear_cpu_buffers;			\
				  CLEAR_CPU_BUFFERS_SEQ;			\
				  .Lskip_clear_cpu_buffers:),			\
		      X86_FEATURE_CLEAR_CPU_BUF_MMIO

Without overloading X86_FEATURE_CLEAR_CPU_BUF_VM, e.g. the conversion from a
static branch to X86_FEATURE_CLEAR_CPU_BUF_MMIO is a pure conversion and yields:

	if (verw_clear_cpu_buf_mitigation_selected) {
		setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF);
		setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF_VM);
	} else {
		setup_force_cpu_cap(X86_FEATURE_CLEAR_CPU_BUF_MMIO);
	}

Give me a few hours to test, and I'll post a v2.  The patches are:

Pawan Gupta (1):
  x86/bugs: Use VM_CLEAR_CPU_BUFFERS in VMX as well

Sean Christopherson (4):
  x86/bugs: Decouple ALTERNATIVE usage from VERW macro definition
  x86/bugs: Use an X86_FEATURE_xxx flag for the MMIO Stale Data mitigation
  KVM: VMX: Handle MMIO Stale Data in VM-Enter assembly via ALTERNATIVES_2
  x86/bugs: KVM: Move VM_CLEAR_CPU_BUFFERS into SVM as SVM_CLEAR_CPU_BUFFERS

 arch/x86/include/asm/cpufeatures.h   |  1 +
 arch/x86/include/asm/nospec-branch.h | 24 +++++++++---------------
 arch/x86/kernel/cpu/bugs.c           | 18 +++++++-----------
 arch/x86/kvm/mmu/spte.c              |  2 +-
 arch/x86/kvm/svm/vmenter.S           |  6 ++++--
 arch/x86/kvm/vmx/vmenter.S           | 13 ++++++++++++-
 arch/x86/kvm/vmx/vmx.c               | 15 +--------------
 7 files changed, 35 insertions(+), 44 deletions(-)

  reply	other threads:[~2025-10-30 18:21 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
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 [this message]
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=aQOswAMVciBXu1ud@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