linux-kernel - Re: [PATCH 3/3] x86/mmio: Unify VERW mitigation for guests

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: <aQOswAMVciBXu1ud@google.com>
Date: Thu, 30 Oct 2025 11:21:52 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>
Cc: Brendan Jackman <jackmanb@...gle.com>, Thomas Gleixner <tglx@...utronix.de>, 
	Borislav Petkov <bp@...en8.de>, Peter Zijlstra <peterz@...radead.org>, 
	Josh Poimboeuf <jpoimboe@...nel.org>, Ingo Molnar <mingo@...hat.com>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org, 
	"H. Peter Anvin" <hpa@...or.com>, Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org, 
	kvm@...r.kernel.org, Tao Zhang <tao1.zhang@...el.com>, 
	Jim Mattson <jmattson@...gle.com>
Subject: Re: [PATCH 3/3] x86/mmio: Unify VERW mitigation for guests

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(-)