Re: [PATCH v2 13/16] perf parse-events: Improvements to modifier parsing

Ian Rogers
2024-04-24T15:19:19Z

On Fri, Apr 19, 2024 at 6:20 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2024-04-19 2:22 a.m., Ian Rogers wrote:
> >>> + /* Simple modifiers copied to the evsel. */
> >>> + if (mod.precise) {
> >>> + u8 precise = evsel->core.attr.precise_ip + mod.precise;
> >>> + /*
> >>> + * precise ip:
> >>> + *
> >>> + * 0 - SAMPLE_IP can have arbitrary skid
> >>> + * 1 - SAMPLE_IP must have constant skid
> >>> + * 2 - SAMPLE_IP requested to have 0 skid
> >>> + * 3 - SAMPLE_IP must have 0 skid
> >>> + *
> >>> + * See also PERF_RECORD_MISC_EXACT_IP
> >>> + */
> >>> + if (precise > 3) {
> >> The pmu_max_precise() should return the max precise the current kernel
> >> supports. It checks the /sys/devices/cpu/caps/max_precise.
> >>
> >> I think we should use that value rather than hard code it to 3.
> > I'll add an extra patch to do that. I'm a bit concerned it may break
> > event parsing on platforms not supporting max_precise of 3.
>
> The kernel already rejects the precise_ip > max_precise (using the same
> x86_pmu_max_precise()). It should be fine to apply the same logic in the
> tool.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/core.c#n566
>
> Will the extra patch be sent separately?

Let's do it separately. I'm concerned about the behavior on AMD (and
possibly similar architectures) where certain events support precision
like cycles, as they detour to the IBS PMU, but not all events support
it. The max_precise should reflect that AMD's Zen core PMU does
support precision as a consequence of detouring to IBS, but maybe
things in sysfs aren't set up correctly.

Thanks,
Ian

> Thanks,
> Kan