Re: [PATCH 00/20] perf vendors events arm64: Multiple Arm CPUs

From: Ian Rogers <irogers@google.com>
To: John Garry <john.garry@huawei.com>
Cc: Nick Forrington <nick.forrington@arm.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Will Deacon <will@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Leo Yan <leo.yan@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Andi Kleen <ak@linux.intel.com>, Kajol Jain <kjain@linux.ibm.com>,
	James Clark <james.clark@arm.com>,
	Andrew Kilroy <andrew.kilroy@arm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Qi Liu <liuqi115@huawei.com>
Subject: Re: [PATCH 00/20] perf vendors events arm64: Multiple Arm CPUs
Date: Mon, 16 May 2022 13:29:04 -0700	[thread overview]
Message-ID: <CAP-5=fUUBezhJG3kXM04axK5jhsZpA_U6dXNEHYd07q-DOLBJQ@mail.gmail.com> (raw)
In-Reply-To: <f1f932f1-1b0b-120d-f082-f9df32b7b9c4@huawei.com>

On Mon, May 16, 2022 at 4:10 AM John Garry <john.garry@huawei.com> wrote:
>
> On 15/05/2022 23:03, Ian Rogers wrote:
> > I'll raise John's "ok" and say this looks great!:-D  Some thoughts:
> >
> > The mapfile.csv cpuid values don't directly align with:
> > https://github.com/ARM-software/data/blob/master/cpus.json
> > but this definitely looks deliberate.
> >
>
> Hi Ian,
>
> > The new events lack the PMU "Unit" value.
>
> For arm support we work on the basis that no "Unit" means CPU PMU. I
> assume the same for other archs, but maybe this hybrid PMU support
> changes that.
>
> > The current perf json is
> > pretty free form and leads to problems if two PMUs are present.
>
> Can you clarify - for my benefit - exactly what you mean by "two PMUs
> are present"?

On Alderlake there is a cpu_core and cpu_atom. The event codes, etc.
vary between them - there is no notion of architecture standard
events.

> > Context is here:
> > https://lore.kernel.org/lkml/CAP-5=fWRRZsyJZ-gky-FOFz79zW_3r78d_0APpj5sf66HqTpLw@mail.gmail.com/
> >
>
> We have another problem but I am not sure if exactly the same.
>
> The issue is that if we have an event alias "cycles" for an uncore PMU,
> then if we use "stat" command then perf tool matches "cycles" to CPU
> cycles and not the uncore PMU, which we would not want.
>
> We have ways to work around it, though.

Ack.

> > My idea to rationalize this is to mirror what is already done in
> > sysfs, that is the event data is specific to a PMU. As a lot of "Unit"
> > values are missing from events on x86 a reasonable guess if the "Unit"
> > is missing is to use "cpu".
>
> This sounds like what I mentioned in the reply to 1/20:
>
> "I had a patch series which makes perf read the armv8 pmu
> sysfs event file to learn all the events which the core supports and
> create the aliases from that. So, in this, we don't require the JSONs to
> list these events explicitly. "
>
> Is this like what Andi was talking about in terms of runtime loading?

I think Andi is talking about loading the json style events at
runtime. The existing jevents.c code could be linked into the perf
tool whereas the jevents.py rewrite would be harder.

Thanks,
Ian

> > Poking a Google Pixel 4a, I see that all
> > PMU data is in "armv8_pmuv3". So for ARM I could guess this is always
> > the case, ie all events should belong to armv8_pmuv3. This may not be
> > right and could lead to confusion like an event BR_COND_MIS_PRED
> > having an alias of "armv8_pmuv3/BR_COND_MIS_PRED/" but it really
> > should have some other PMU name in there. I just raise this in case
> > there is a fix for this we could incorporate into this patch series,
> > maybe "armv8_pmuv3" is always the PMU and my life is easy.
>
> Thanks,
> John