Re: [RFC PATCH 1/2] perf: arm_spe: Fix consistency of PMSCR register bit CX

[German Gomez]

Hi Leo,

On 08/02/2022 13:00, Leo Yan wrote:
> Hi German,
>
> On Mon, Feb 07, 2022 at 12:06:14PM +0000, German Gomez wrote:
>
> [...]
> Indeed!  I can reproduce the issue now.  And I can capture backtrace
> for arm_spe_pmu_start() with below commands:
>
> # cd /home/leoy/linux/tools/perf
> # ./perf probe --add "arm_spe_pmu_start" -s /home/leoy/linux/ -k /home/leoy/linux/vmlinux
> # echo 1 > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_start/enable
> # echo stacktrace > /sys/kernel/debug/tracing/events/probe/arm_spe_pmu_start/trigger
>
> ... run your commands with non-root user ...
>
> # cat /sys/kernel/debug/tracing/trace
>
>              dd-7697    [000] d.h2.   506.068700: arm_spe_pmu_start: (arm_spe_pmu_start+0x8/0xe0)
>              dd-7697    [000] d.h3.   506.068701: <stack trace>
> => kprobe_dispatcher
> => kprobe_breakpoint_handler
> => call_break_hook
> [...]
> => do_el0_svc
> => el0_svc
> => el0t_64_sync_handler
> => el0t_64_sync
>
> The backtrace clearly shows the function arm_spe_pmu_start() is
> invoked in the 'dd' process (dd-7697); the flow is:
> - perf program sends IPI to CPU0;
> - 'dd' process is running on CPU0 and it's interrupted to handle IPI;
> - 'dd' process has root capabilities, so it will enable context
>   tracing for non-root perf session.

Thanks for testing, and sharing the commands in your replies!

>
>>>> One way to fix this is by caching the value of the CX bit during the
>>>> initialization of the PMU event, so that it remains consistent for the
>>>> duration of the session.
>>>>
>>>> [...]
> So the patch makes sense to me.  Just a minor comment:
>
> Here we can define a u64 for recording pmscr value rather than a
> bool value.
>
> struct arm_spe_pmu {
>     ...
>     u64 pmscr;
> };

I agree with the comment from Will that it makes more sense to store the
value of the register in the perf_event somehow (due to misunderstanding
from my side, I thought arm_spe_pmu struct was local to the session).

What about perf_event's void *pmu_private?

Thanks,
German