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

[Pawan Gupta]

On Thu, Oct 30, 2025 at 11:21:52AM -0700, Sean Christopherson wrote:
> 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

Agree.

> > 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.

I don't see a problem with that.

> After half an hour of debugging godawful assembler errors because I used stringify()
> instead of __stringify(),

Not surprised at all :-)

> 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;			\

Curious what this is doing, I will wait for your patches.

> 				  .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

Ok, sounds good to me.