Re: [PATCH v3 5/7] alpha/perf: implement ProfileMe sampling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] alpha/perf: perf_get_regs_user() checks PMPC instead of interrupted PC
--

commit b49acea08699ec26b74c2c7c29ccc098435ea611
Author: Matt Turner <mattst88@gmail.com>

alpha/perf: implement ProfileMe sampling

This commit implements ProfileMe sampling mode on Alpha EV67 and later
processors, selectable via the profileme config attribute or precise_ip. It
attributes samples to the instruction address latched in PMPC, adjusts for
I_STAT instruction overcount, and works around PALcode counter overflow
restrictions when stopping or rearming counters.

> diff --git a/arch/alpha/kernel/perf_event.c b/arch/alpha/kernel/perf_event.c
> index ecec78c355cd3..f55ddab7478e0 100644
> --- a/arch/alpha/kernel/perf_event.c
> +++ b/arch/alpha/kernel/perf_event.c
[ ... ]
> @@ -835,6 +939,131 @@ void perf_event_print_debug(void)
[ ... ]
> +static void alpha_perf_event_profileme(struct perf_event *event,
> +				       struct hw_perf_event *hwc,
> +				       struct pt_regs *regs)
> +{
[ ... ]
> +	pm_regs = regs;
> +	pc = pmpc & EV67_PMPC_PC_MASK;
> +	if (pmpc & EV67_PMPC_PAL) {
> +		pc = regs->pc;
> +	} else if (pc < TASK_SIZE && !user_mode(regs)) {
> +		if (is_user_task(current)) {
> +			pc_only = true;
> +			pm_regs = this_cpu_ptr(&alpha_profileme_regs);
> +			memset(pm_regs, 0, sizeof(*pm_regs));
> +			/* PS bit 3 is the mode bit that user_mode() tests. */
> +			pm_regs->ps = 8;
> +		} else {
> +			pc = regs->pc;
> +		}
> +	} else if (pc >= TASK_SIZE && user_mode(regs)) {
> +		pm_regs = this_cpu_ptr(&alpha_profileme_regs);
> +		memset(pm_regs, 0, sizeof(*pm_regs));
> +	}
> +
> +	perf_sample_data_init(&data, 0, hwc->last_period);
> +	if (pc_only)
> +		alpha_profileme_drop_regs(event, &data, PERF_SAMPLE_REGS_USER |
> +							PERF_SAMPLE_REGS_INTR);
> +	/*
> +	 * perf_get_regs_user() has the same check, but it will find the PMPC
> +	 * in regs->pc rather than where the interrupt was taken.
> +	 */
> +	else if (!user_mode(regs) && in_entry_save_code(regs->pc))
> +		alpha_profileme_drop_regs(event, &data, PERF_SAMPLE_REGS_USER);
> +
> +	if (!alpha_perf_event_set_period(event, hwc, 0))
> +		return;
> +
> +	saved_pc = pm_regs->pc;
> +	saved_ps = pm_regs->ps;
> +	pm_regs->pc = pc;
> +	/* PS bit 3 is the mode bit that user_mode() tests. */
> +	if (pc >= TASK_SIZE)
> +		pm_regs->ps &= ~8UL;
> +
> +	perf_event_overflow(event, &data, pm_regs);

[Severity: Medium]
When a ProfileMe sample has a kernel PMPC inside the entry save range
(in_entry_save_code(pc) is true), but the performance counter interrupt itself
is taken after leaving that range—either later in kernel mode after SAVE_ALL
has populated task_pt_regs(current), or after returning to user mode where
user_mode(regs) is true—alpha_perf_event_profileme() does not call
alpha_profileme_drop_regs() because in_entry_save_code(regs->pc) is false.

It then sets pm_regs->pc = pc and clears the user mode bit in pm_regs->ps (or
uses the zeroed per-CPU alpha_profileme_regs when user_mode(regs) is true)
before calling perf_event_overflow().

During sample preparation, perf_event_overflow() -> __perf_event_overflow() ->
perf_prepare_sample() -> perf_sample_regs_user() sees !user_mode(pm_regs) and
calls perf_get_regs_user():

arch/alpha/kernel/perf_regs.c:perf_get_regs_user() {
	if (in_entry_save_code(regs->pc)) {
		regs_user->regs = NULL;
		regs_user->abi = PERF_SAMPLE_REGS_ABI_NONE;
		return;
	}

	regs_user->regs = task_pt_regs(current);
	regs_user->abi = perf_reg_abi(current);
}

Because perf_get_regs_user() checks in_entry_save_code(pm_regs->pc), which is
now the PMPC rather than the interrupted PC regs->pc, won't it falsely discard
valid PERF_SAMPLE_REGS_USER and PERF_SAMPLE_STACK_USER data whenever the
profiled instruction is in __entry_save_start..__entry_save_end even though
the interrupt was delivered after the user frame was completely saved (or
after returning to user mode)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101-alpha-profileme-v3-0-92d511b5a044@gmail.com?part=5