On Wed, May 28, 2025, Pawan Gupta wrote: > On Thu, May 22, 2025 at 06:17:54PM -0700, Sean Christopherson wrote: > > @@ -7282,7 +7288,7 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > > if (static_branch_unlikely(&vmx_l1d_should_flush)) > > vmx_l1d_flush(vcpu); > > else if (static_branch_unlikely(&mmio_stale_data_clear) && > > - kvm_arch_has_assigned_device(vcpu->kvm)) > > + (flags & VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO)) > > mds_clear_cpu_buffers(); > > I think this also paves way for buffer clear for MDS and MMIO to be done at > a single place. Please let me know if below is feasible: It's definitely feasible (this thought crossed my mind as well), but because CLEAR_CPU_BUFFERS emits VERW iff X86_FEATURE_CLEAR_CPU_BUF is enabled, the below would do nothing for the MMIO case (either that, or I'm missing something). We could obviously rework CLEAR_CPU_BUFFERS, I'm just not sure that's worth the effort at this point. I'm definitely not opposed to it though. > diff --git a/arch/x86/kvm/vmx/run_flags.h b/arch/x86/kvm/vmx/run_flags.h > index 2f20fb170def..004fe1ca89f0 100644 > --- a/arch/x86/kvm/vmx/run_flags.h > +++ b/arch/x86/kvm/vmx/run_flags.h > @@ -2,12 +2,12 @@ > #ifndef __KVM_X86_VMX_RUN_FLAGS_H > #define __KVM_X86_VMX_RUN_FLAGS_H > > -#define VMX_RUN_VMRESUME_SHIFT 0 > -#define VMX_RUN_SAVE_SPEC_CTRL_SHIFT 1 > -#define VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO_SHIFT 2 > +#define VMX_RUN_VMRESUME_SHIFT 0 > +#define VMX_RUN_SAVE_SPEC_CTRL_SHIFT 1 > +#define VMX_RUN_CLEAR_CPU_BUFFERS_SHIFT 2 > > -#define VMX_RUN_VMRESUME BIT(VMX_RUN_VMRESUME_SHIFT) > -#define VMX_RUN_SAVE_SPEC_CTRL BIT(VMX_RUN_SAVE_SPEC_CTRL_SHIFT) > -#define VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO BIT(VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO_SHIFT) > +#define VMX_RUN_VMRESUME BIT(VMX_RUN_VMRESUME_SHIFT) > +#define VMX_RUN_SAVE_SPEC_CTRL BIT(VMX_RUN_SAVE_SPEC_CTRL_SHIFT) > +#define VMX_RUN_CLEAR_CPU_BUFFERS BIT(VMX_RUN_CLEAR_CPU_BUFFERS_SHIFT) > > #endif /* __KVM_X86_VMX_RUN_FLAGS_H */ > diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S > index f6986dee6f8c..ab602ce4967e 100644 > --- a/arch/x86/kvm/vmx/vmenter.S > +++ b/arch/x86/kvm/vmx/vmenter.S > @@ -141,6 +141,8 @@ SYM_FUNC_START(__vmx_vcpu_run) > /* Check if vmlaunch or vmresume is needed */ > bt $VMX_RUN_VMRESUME_SHIFT, %ebx > > + test $VMX_RUN_CLEAR_CPU_BUFFERS, %ebx > + > /* Load guest registers. Don't clobber flags. */ > mov VCPU_RCX(%_ASM_AX), %_ASM_CX > mov VCPU_RDX(%_ASM_AX), %_ASM_DX > @@ -161,8 +163,11 @@ SYM_FUNC_START(__vmx_vcpu_run) > /* Load guest RAX. This kills the @regs pointer! */ > mov VCPU_RAX(%_ASM_AX), %_ASM_AX > > + /* Check EFLAGS.ZF from the VMX_RUN_CLEAR_CPU_BUFFERS bit test above */ > + jz .Lskip_clear_cpu_buffers > /* Clobbers EFLAGS.ZF */ > CLEAR_CPU_BUFFERS > +.Lskip_clear_cpu_buffers: > > /* Check EFLAGS.CF from the VMX_RUN_VMRESUME bit test above. */ > jnc .Lvmlaunch > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 1e4790c8993a..1415aeea35f7 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -958,9 +958,10 @@ 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(&mmio_stale_data_clear) && > - kvm_vcpu_can_access_host_mmio(&vmx->vcpu)) > - flags |= VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO; > + if (cpu_feature_enabled(X86_FEATURE_CLEAR_CPU_BUF) || > + (static_branch_unlikely(&mmio_stale_data_clear) && > + kvm_vcpu_can_access_host_mmio(&vmx->vcpu))) > + flags |= VMX_RUN_CLEAR_CPU_BUFFERS; > > return flags; > } > @@ -7296,9 +7297,6 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu, > */ > if (static_branch_unlikely(&vmx_l1d_should_flush)) > vmx_l1d_flush(vcpu); > - else if (static_branch_unlikely(&mmio_stale_data_clear) && > - (flags & VMX_RUN_CLEAR_CPU_BUFFERS_FOR_MMIO)) > - mds_clear_cpu_buffers(); > > vmx_disable_fb_clear(vmx); >
