Re: [RESEND Patch v10 11/23] perf/x86: Enable XMM register sampling for REGS_USER case

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 8/24/2026 6:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] NMI re-entrancy corrupts per-CPU `x86_user_regs` during PEBS drains
> --
>
> --- Patch [11]: [RESEND Patch v10 11/23] perf/x86: Enable XMM register sampling for REGS_USER case ---
> commit 1dabe6f859e16c09faeea0ab1b864b14dc315cd4
> Author: Dapeng Mi <dapeng1.mi@linux.intel.com>
>
> perf/x86: Enable XMM register sampling for REGS_USER case
>
> This commit adds support for XMM register sampling in the REGS_USER case.
> It introduces a per-CPU x86_user_regs variable to store REGS_USER-specific
> XMM registers to prevent overwriting by REGS_INTR-specific data. A helper
> function x86_pmu_update_user_xregs() is introduced to read these registers
> from either cached task FPU states or hardware registers.
>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 1c3e2bfa2673a..20140643e21c9 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
> [ ... ]
>> @@ -1832,14 +1826,129 @@ static void x86_pmu_update_regs_intr(struct perf_event *event,
>>  	data->sample_flags |= PERF_SAMPLE_REGS_INTR;
>>  }
>>  
>> +static DEFINE_PER_CPU(struct x86_perf_regs, x86_user_regs);
>> +
>> +static void x86_pmu_get_regs_user(struct perf_sample_data *data,
>> +				  struct pt_regs *regs)
>> +{
>> +	struct x86_perf_regs *x86_regs_user = this_cpu_ptr(&x86_user_regs);
>> +	struct perf_regs regs_user;
>> +
>> +	x86_pmu_clear_perf_regs(&x86_regs_user->regs);
>> +
>> +	perf_get_regs_user(&regs_user, regs);
>> +	data->regs_user.abi = regs_user.abi;
>> +	if (regs_user.regs) {
>> +		x86_regs_user->regs = *regs_user.regs;
>> +		data->regs_user.regs = &x86_regs_user->regs;
>> +	} else {
>> +		data->regs_user.regs = NULL;
>> +	}
>> +}
>> +
>> +/*
>> + * The x86 specific variant of perf_sample_regs_user().
>> + * Update data->regs_user fields for extended registers (e.g., SIMD).
>> + */
>> +static void x86_pmu_update_regs_user(struct perf_event *event,
>> +				     struct perf_sample_data *data,
>> +				     struct pt_regs *regs)
>> +{
>> +	struct x86_perf_regs *x86_regs_user = this_cpu_ptr(&x86_user_regs);
>> +	struct perf_event_attr *attr = &event->attr;
>> +	struct x86_perf_regs *perf_regs;
>> +
>> +	/*
>> +	 * PERF_SAMPLE_REGS_INTR and PERF_SAMPLE_REGS_USER can both be
>> +	 * requested for one event. Keep user regs in a separate x86_perf_regs
>> +	 * instance, so intr-reg collection does not overwrite user-reg data.
>> +	 */
>> +	if (user_mode(regs)) {
>> +		x86_pmu_clear_perf_regs(&x86_regs_user->regs);
>> +		perf_regs = container_of(regs, struct x86_perf_regs, regs);
>> +		/* Copy all sampled regs data to x86_regs_user. */
>> +		*x86_regs_user = *perf_regs;
>> +		data->regs_user.regs = &x86_regs_user->regs;
> [Severity: High]
> Does this code corrupt the un-nested per-CPU x86_user_regs during NMIs?
>
> If a context-switch PEBS drain runs with local IRQs disabled but NMIs enabled,
> a PMI (NMI) could interrupt the drain. If the PMI handler processes a perf event
> requesting REGS_USER, it will call x86_pmu_update_regs_user(), which
> unconditionally overwrites the same per-CPU x86_user_regs variable here without
> in_nmi() protection.
>
> Upon returning from the NMI, the interrupted PEBS drain would resume copying
> the now-corrupted x86_user_regs into the ring buffer, silently corrupting the
> recorded user registers.

The x86_pmu_update_regs_user() is called in two places, one is in the NMI
handler (handle_pmi_common()), which naturally prevents the NMI reentry,
the other place is in the drain_pebs() helper. The patch
https://lore.kernel.org/all/20260813064346.335458-1-dapeng1.mi@linux.intel.com/
would block the drain_pebs() helper reentry.

Thanks.


>