Re: [RFC PATCH v1 0/5] x86/boot, KVM: Move VMXON/VMXOFF handling from KVM to CPU lifecycle

Xin Li
2025-09-17T17:31:18Z

On 9/16/2025 10:54 AM, Sean Christopherson wrote:
> On Thu, Sep 11, 2025, Arjan van de Ven wrote:
>> Hi,
>>> I also want to keep the code as a module, both to avoid doing VMXON unconditionally,
>>
>> can you expand on what the problem is with having VMXON unconditionally enabled?
>
> Unlike say EFER.SVME, VMXON fundamentally changes CPU behavior. E.g. blocks INIT,
> activates VMCS caches (which aren't cleared by VMXOFF on pre-SPR CPUs, and AFAIK
> Intel hasn't even publicly committed to that behavior for SPR+), restricts allowed
> CR0 and CR4 values, raises questions about ucode patch updates, triggers unique
> flows in SMI/RSM, prevents Intel PT from tracing on certain CPUs, and probably a
> few other things I'm forgetting.

Regarding Intel PT, if VMXON/VMXOFF are moved to CPU startup/shutdown, as
Intel PT is initialized during arch_initcall() stage, entering and leaving
VMX operation no longer happen while Intel PT is _active_, thus
intel_pt_handle_vmx() no longer needs to "handles" VMX state transitions.

Thus, the function's purpose is simplified to signaling Intel pt not to
write to IA32_RTIT_CTL during VMX operation if the processor supports Intel
PT but disallows its use in VMX operation, indicated by IA32_VMX_MISC[14]
being cleared. Otherwise, it does nothing and leaves pt_ctx.vmx_on as 0.

If the following patch is correct, it's more of a simplification then :)

diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index e8cf29d2b10c..8325a824700a 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -225,17 +225,6 @@ static int __init pt_pmu_hw_init(void)
break;
}

- if (boot_cpu_has(X86_FEATURE_VMX)) {
- /*
- * Intel SDM, 36.5 "Tracing post-VMXON" says that
- * "IA32_VMX_MISC[bit 14]" being 1 means PT can trace
- * post-VMXON.
- */
- rdmsrq(MSR_IA32_VMX_MISC, reg);
- if (reg & BIT(14))
- pt_pmu.vmx = true;
- }
-
for (i = 0; i < PT_CPUID_LEAVES; i++) {
cpuid_count(20, i,
&pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM],
@@ -1556,41 +1545,39 @@ void intel_pt_interrupt(void)
}
}

-void intel_pt_handle_vmx(int on)
+/*
+ * VMXON is done in the CPU startup phase, thus pt is initialized later.
+ *
+ * Signal pt to not write IA32_RTIT_CTL while in VMX operation if the
+ * processor supports Intel PT but does not allow it to be used in VMX
+ * operation, i.e. IA32_VMX_MISC[bit 14] is cleared.
+ *
+ * Note: If IA32_VMX_MISC[bit 14] is set, vmx_on in pt_ctx remains 0.
+ */
+void intel_pt_set_vmx(int on)
{
struct pt *pt = this_cpu_ptr(&pt_ctx);
- struct perf_event *event;
- unsigned long flags;
+ int cpu = raw_smp_processor_id();
+
+ if (!cpu && cpu_feature_enabled(X86_FEATURE_VMX)) {
+ u64 misc;
+
+ /*
+ * Intel SDM, 36.5 "Tracing post-VMXON" says that
+ * "IA32_VMX_MISC[bit 14]" being 1 means PT can trace
+ * post-VMXON.
+ */
+ rdmsrq(MSR_IA32_VMX_MISC, misc);
+ if (misc & BIT(14))
+ pt_pmu.vmx = true;
+ }

/* PT plays nice with VMX, do nothing */
if (pt_pmu.vmx)
return;

- /*
- * VMXON will clear RTIT_CTL.TraceEn; we need to make
- * sure to not try to set it while VMX is on. Disable
- * interrupts to avoid racing with pmu callbacks;
- * concurrent PMI should be handled fine.
- */
- local_irq_save(flags);
WRITE_ONCE(pt->vmx_on, on);
-
- /*
- * If an AUX transaction is in progress, it will contain
- * gap(s), so flag it PARTIAL to inform the user.
- */
- event = pt->handle.event;
- if (event)
- perf_aux_output_flag(&pt->handle,
- PERF_AUX_FLAG_PARTIAL);
-
- /* Turn PTs back on */
- if (!on && event)
- wrmsrq(MSR_IA32_RTIT_CTL, event->hw.aux_config);
-
- local_irq_restore(flags);
}
-EXPORT_SYMBOL_GPL(intel_pt_handle_vmx);

/*
* PMU callbacks
diff --git a/arch/x86/include/asm/perf_event.h
b/arch/x86/include/asm/perf_event.h
index 70d1d94aca7e..9140796e6268 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -659,12 +659,9 @@ static inline void x86_perf_get_lbr(struct x86_pmu_lbr
*lbr)
#endif

#ifdef CONFIG_CPU_SUP_INTEL
- extern void intel_pt_handle_vmx(int on);
+extern void intel_pt_set_vmx(int on);
#else
-static inline void intel_pt_handle_vmx(int on)
-{
-
-}
+static inline void intel_pt_set_vmx(int on) { }
#endif

#if defined(CONFIG_PERF_EVENTS) && defined(CONFIG_CPU_SUP_AMD)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 03b28fa2e91e..9dad23c86152 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2009,7 +2009,7 @@ void cpu_enable_virtualization(void)
rdmsrq(MSR_IA32_VMX_BASIC, basic_msr);
this_cpu_ptr(&vmxon_vmcs)->hdr.revision_id =
vmx_basic_vmcs_revision_id(basic_msr);

- intel_pt_handle_vmx(1);
+ intel_pt_set_vmx(1);

cr4_set_bits(X86_CR4_VMXE);

@@ -2023,7 +2023,7 @@ void cpu_enable_virtualization(void)
fault:
pr_err("VMXON faulted on CPU%d\n", cpu);
cr4_clear_bits(X86_CR4_VMXE);
- intel_pt_handle_vmx(0);
+ intel_pt_set_vmx(0);
}

/*
@@ -2055,7 +2055,7 @@ void cpu_disable_virtualization(void)

exit:
cr4_clear_bits(X86_CR4_VMXE);
- intel_pt_handle_vmx(0);
+ intel_pt_set_vmx(0);
return;