Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] perf: arm_spe: Fix consistency of PMSCR register bit CX | From | German Gomez <> | Date | Thu, 10 Feb 2022 17:23:50 +0000 |
| |
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
| |