Re: [PATCH 4/6] perf/x86/intel/pt: Clear TraceEn in hardware in cpu_emergency_stop_pt()

[Adrian Hunter <adrian.hunter@intel.com>]

On 02/11/2026 12:07, Peter Zijlstra wrote:
> On Mon, Nov 02, 2026 at 09:40:37AM +0200, Adrian Hunter wrote:
>> cpu_emergency_stop_pt() stops tracing through pt_event_stop(), which
>> assumes the software state accurately reflects whether tracing is
>> enabled.  That assumption can fail when a crash NMI interrupts
>> pt_config_start().
>>
>> pt_config_start() sets TraceEn in IA32_RTIT_CTL before updating
>> event->hw.aux_config.  If a crash NMI arrives in that window,
>> pt_config_stop() sees TraceEn clear in aux_config and skips the MSR
>> write needed to disable tracing.  Tracing remains active in hardware.
>>
>> Alternatively, an earlier NMI in the same window can stop the event
>> without restarting it, for example a group throttle calling
>> pt_event_stop(event, 0), or a PT PMI for which perf_aux_output_begin()
>> fails.  pt_config_stop() again skips the write, so the event is stopped
>> only in software: TraceEn remains set in IA32_RTIT_CTL, and
>> pt->handle.event becomes NULL.  When pt_config_start() continues, it
>> sets TraceEn in aux_config, so tracing continues until the event is
>> next stopped.  If a crash occurs before then, cpu_emergency_stop_pt()
>> sees no event and does nothing.
>>
>> In both cases tracing can remain active into the kdump kernel.  Moreover,
>> pt_event_stop() may access PT MSRs while tracing is still enabled and
>> can trigger a #GP by writing IA32_RTIT_STATUS with TraceEn set.
>>
>> Fix this by making cpu_emergency_stop_pt() disable tracing directly from
>> the hardware state.  Set pt_shutdown and invoke pt_shutdown_cpu(), which
>> reads IA32_RTIT_CTL and clears TraceEn whenever it is set, independent
>> of perf's software state.  The shutdown flag prevents any path from
>> re-enabling tracing afterwards, including intel_pt_handle_vmx(0) during
>> the emergency VMXOFF sequence on other CPUs.
>>
>> Continue to call pt_event_stop() afterwards so that AUX state, including
>> the buffer head, is updated for vmcore collection.  Its PT MSR accesses
>> now occur with tracing disabled.
>>
>> Do this only if the driver initialized successfully.  In particular, if
>> pt_init() finds tracing already enabled at boot, it leaves Intel PT
>> alone, and the emergency path must do the same.
>>
>> The race is reachable only through restart paths that call
>> pt_config_start() with TraceEn clear in the software copy.  These were
>> introduced by the commits with the Fixes tags below.  The restart path
>> in the PT PMI handler runs in NMI context, so only a machine check can
>> interrupt it.
>>
>> Fixes: 8e105a1fc2a0 ("perf/x86/intel/pt: Factor out pt_config_start()")
>> Fixes: 25e8920b301c ("perf/x86/intel/pt: Add sampling support")
>> Fixes: 08c7454ceb94 ("perf/x86/intel/pt: Add support for pause / resume")
>> Fixes: 2e17bf3a469a ("perf/x86/intel/pt: Fix stop/start with no update")
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  arch/x86/events/intel/pt.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
>> index 7453e9880b25..8d71c00f9ff6 100644
>> --- a/arch/x86/events/intel/pt.c
>> +++ b/arch/x86/events/intel/pt.c
>> @@ -38,6 +38,8 @@ static struct pt_pmu pt_pmu;
>>  /* Set at shutdown, after which TraceEn must not be set */
>>  static bool pt_shutdown;
>>  
>> +static bool pt_initialized __ro_after_init;
>> +
>>  /*
>>   * Capabilities of Intel PT hardware, such as number of address bits or
>>   * supported output schemes, are cached and exported to userspace as "caps"
>> @@ -1900,6 +1902,12 @@ void cpu_emergency_stop_pt(void)
>>  {
>>  	struct pt *pt = this_cpu_ptr(&pt_ctx);
>>  
>> +	if (!pt_initialized)
>> +		return;
>> +
>> +	WRITE_ONCE(pt_shutdown, true);
>> +	pt_shutdown_cpu(NULL);
>> +
>>  	if (pt->handle.event)
>>  		pt_event_stop(pt->handle.event, PERF_EF_UPDATE);
> 
> So in general pmu::stop() is reachable from PTI/NMI context, through
> things like throttling. So arguably it not functioning correctly when
> interleaving with pmu::start() is a problem.
> 

Yes, and it looks like there is a separate fix for the 1st
scenario described in the commit message: write aux_config both
before and after the WRMSR.  I will add that fix next time around,
and re-evaluate how much this patch is still needed.