Re: [Patch v2 2/4] perf/x86/intel: Disable PMI for self-reloaded ACR events

[Mi, Dapeng]

On 4/22/2026 6:37 AM, Andi Kleen wrote:
> On Mon, Apr 20, 2026 at 10:45:26AM +0800, Dapeng Mi wrote:
>> @@ -3306,6 +3306,15 @@ static void intel_pmu_enable_event(struct perf_event *event)
>>  		intel_set_masks(event, idx);
>>  		static_call_cond(intel_pmu_enable_acr_event)(event);
>>  		static_call_cond(intel_pmu_enable_event_ext)(event);
>> +		/*
>> +		 * For self-reloaded ACR event, don't enable PMI since
>> +		 * HW won't set overflow bit in GLOBAL_STATUS. Otherwise,
>> +		 * the PMI would be recognized as a suspicious NMI.
>> +		 */
>> +		if (is_acr_self_reload_event(event))
>> +			hwc->config &= ~ARCH_PERFMON_EVENTSEL_INT;
>> +		else if (!event->attr.precise_ip)
>> +			hwc->config |= ARCH_PERFMON_EVENTSEL_INT;
> It seems weird to either clear or set the bit. You don't know the previous
> state of the bit here? I would assume it starts with zero?

It's hard and unsafe to trace the previous state. Generally speaking, the
PMI bit would always be set by default at the initialization, then it would
be cleared later if it's a PEBS or ACR self-reloaded event. 


>
>> +static inline bool is_acr_self_reload_event(struct perf_event *event)
>> +{
>> +	struct hw_perf_event *hwc = &event->hw;
>> +
>> +	if (hwc->idx < 0)
>> +		return false;
>> +
>> +	return test_bit(hwc->idx, (unsigned long *)&hwc->config1);
> Are you sure this doesn't conflict with some other non ACR usage of config1?

Yes, currently hw.config1 is only used to store ACR  event indices.

Thanks.


>
>
> -Andi