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(-)
next prev parent 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