| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f533d3a4-183e-4b3d-9b3a-95defb1876e0@zytor.com>
Date: Wed, 17 Sep 2025 10:30:31 -0700
From: Xin Li <xin@...or.com>
To: Sean Christopherson <seanjc@...gle.com>,
Arjan van de Ven <arjan@...ux.intel.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
linux-pm@...r.kernel.org, pbonzini@...hat.com, tglx@...utronix.de,
mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
x86@...nel.org, hpa@...or.com, rafael@...nel.org, pavel@...nel.org,
brgerst@...il.com, david.kaplan@....com, peterz@...radead.org,
andrew.cooper3@...rix.com, kprateek.nayak@....com, chao.gao@...el.com,
rick.p.edgecombe@...el.com, dan.j.williams@...el.com,
"adrian.hunter@...el.com" <adrian.hunter@...el.com>
Subject: Re: [RFC PATCH v1 0/5] x86/boot, KVM: Move VMXON/VMXOFF handling from
KVM to CPU lifecycle
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;
fault:
Powered by blists - more mailing lists