Re: [RFC PATCH 06/18] KVM: VMX: Wire up Intel MBEC enable/disable logic

[Chao Gao <chao.gao@intel.com>]

On Thu, Mar 13, 2025 at 01:36:45PM -0700, Jon Kohler wrote:
>Add logic to enable / disable Intel Mode Based Execution Control (MBEC)
>based on specific conditions.
>
>MBEC depends on:
>- User space exposing secondary execution control bit 22

The code below doesn't check this.

>- Extended Page Tables (EPT)
>- The KVM module parameter `enable_pt_guest_exec_control`
>
>If any of these conditions are not met, MBEC will be disabled
>accordingly.
>
>Store runtime enablement within `kvm_vcpu_arch.pt_guest_exec_control`.
>
>Signed-off-by: Jon Kohler <jon@nutanix.com>
>
>---
> arch/x86/kvm/vmx/vmx.c | 11 +++++++++++
> arch/x86/kvm/vmx/vmx.h |  7 +++++++
> 2 files changed, 18 insertions(+)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 7a98f03ef146..116910159a3f 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -2694,6 +2694,7 @@ static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
> 			return -EIO;
> 
> 		vmx_cap->ept = 0;
>+		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_MODE_BASED_EPT_EXEC;
> 		_cpu_based_2nd_exec_control &= ~SECONDARY_EXEC_EPT_VIOLATION_VE;
> 	}
> 	if (!(_cpu_based_2nd_exec_control & SECONDARY_EXEC_ENABLE_VPID) &&
>@@ -4641,11 +4642,15 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
> 		exec_control &= ~SECONDARY_EXEC_ENABLE_VPID;
> 	if (!enable_ept) {
> 		exec_control &= ~SECONDARY_EXEC_ENABLE_EPT;
>+		exec_control &= ~SECONDARY_EXEC_MODE_BASED_EPT_EXEC;
> 		exec_control &= ~SECONDARY_EXEC_EPT_VIOLATION_VE;
> 		enable_unrestricted_guest = 0;
> 	}
> 	if (!enable_unrestricted_guest)
> 		exec_control &= ~SECONDARY_EXEC_UNRESTRICTED_GUEST;
>+	if (!enable_pt_guest_exec_control)
>+		exec_control &= ~SECONDARY_EXEC_MODE_BASED_EPT_EXEC;
>+
> 	if (kvm_pause_in_guest(vmx->vcpu.kvm))
> 		exec_control &= ~SECONDARY_EXEC_PAUSE_LOOP_EXITING;
> 	if (!kvm_vcpu_apicv_active(vcpu))
>@@ -4770,6 +4775,9 @@ static void init_vmcs(struct vcpu_vmx *vmx)
> 		if (vmx->ve_info)
> 			vmcs_write64(VE_INFORMATION_ADDRESS,
> 				     __pa(vmx->ve_info));
>+
>+		vmx->vcpu.arch.pt_guest_exec_control =
>+			enable_pt_guest_exec_control && vmx_has_mbec(vmx);

Is it possible for vmx->vcpu.arch.pt_guest_exec_control and
enable_pt_guest_exec_control to differ?

To me, the answer is no. So, why not use enable_pt_guest_exec_control
directly?

> 	}
> 
> 	if (cpu_has_tertiary_exec_ctrls())
>@@ -8472,6 +8480,9 @@ __init int vmx_hardware_setup(void)
> 	if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
> 		enable_unrestricted_guest = 0;
> 
>+	if (!cpu_has_vmx_mbec() || !enable_ept)
>+		enable_pt_guest_exec_control = false;
>+
> 	if (!cpu_has_vmx_flexpriority())
> 		flexpriority_enabled = 0;
> 
>diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
>index d1e537bf50ea..9f4ae3139a90 100644
>--- a/arch/x86/kvm/vmx/vmx.h
>+++ b/arch/x86/kvm/vmx/vmx.h
>@@ -580,6 +580,7 @@ static inline u8 vmx_get_rvi(void)
> 	 SECONDARY_EXEC_ENABLE_VMFUNC |					\
> 	 SECONDARY_EXEC_BUS_LOCK_DETECTION |				\
> 	 SECONDARY_EXEC_NOTIFY_VM_EXITING |				\
>+	 SECONDARY_EXEC_MODE_BASED_EPT_EXEC |				\
> 	 SECONDARY_EXEC_ENCLS_EXITING |					\
> 	 SECONDARY_EXEC_EPT_VIOLATION_VE)
> 
>@@ -721,6 +722,12 @@ static inline bool vmx_has_waitpkg(struct vcpu_vmx *vmx)
> 		SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
> }
> 
>+static inline bool vmx_has_mbec(struct vcpu_vmx *vmx)
>+{
>+	return secondary_exec_controls_get(vmx) &
>+		SECONDARY_EXEC_MODE_BASED_EPT_EXEC;
>+}
>+
> static inline bool vmx_need_pf_intercept(struct kvm_vcpu *vcpu)
> {
> 	if (!enable_ept)
>-- 
>2.43.0
>