| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200228160126.GI36089@lakrids.cambridge.arm.com>
Date: Fri, 28 Feb 2020 16:01:26 +0000
From: Mark Rutland <mark.rutland@....com>
To: James Clark <james.clark@....com>
Cc: Adrian Hunter <adrian.hunter@...el.com>, jolsa@...hat.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
nd@....com, Tan Xiaojun <tanxiaojun@...wei.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Al Grant <al.grant@....com>,
Namhyung Kim <namhyung@...nel.org>,
Will Deacon <will@...nel.org>
Subject: Re: [PATCH v4 4/4] perf tools: Support "branch-misses:pp" on arm64
Hi James,
On Mon, Feb 24, 2020 at 05:08:26PM +0000, James Clark wrote:
> On 2/17/20 11:42 AM, Adrian Hunter wrote:
> > On 11/02/20 4:04 pm, James Clark wrote:
> >> From: Tan Xiaojun <tanxiaojun@...wei.com>
> >>
> >> At the suggestion of James Clark, use spe to support the precise
> >> ip of some events. Currently its support event is:
> >> branch-misses.
> >>
> >> Example usage:
> >>
> >> $ ./perf record -e branch-misses:pp dd if=/dev/zero of=/dev/null count=10000
> >> (:p/pp/ppp is same for this case.)
> >>
> >> $ ./perf report --stdio
> >> ("--stdio is not necessary")
> >>
> >> --------------------------------------------------------------------
> >> ...
> >> # Samples: 14 of event 'branch-misses:pp'
> >> # Event count (approx.): 14
> >> #
> >> # Children Self Command Shared Object Symbol
> >> # ........ ........ ....... ................. ..........................
> >> #
> >> 14.29% 14.29% dd [kernel.kallsyms] [k] __arch_copy_from_user
> >> 14.29% 14.29% dd libc-2.28.so [.] _dl_addr
> >> 7.14% 7.14% dd [kernel.kallsyms] [k] __free_pages
> >> 7.14% 7.14% dd [kernel.kallsyms] [k] __pi_memcpy
> >> 7.14% 7.14% dd [kernel.kallsyms] [k] pagecache_get_page
> >> 7.14% 7.14% dd [kernel.kallsyms] [k] unmap_single_vma
> >> 7.14% 7.14% dd dd [.] 0x00000000000025ec
> >> 7.14% 7.14% dd ld-2.28.so [.] _dl_lookup_symbol_x
> >> 7.14% 7.14% dd ld-2.28.so [.] check_match
> >> 7.14% 7.14% dd libc-2.28.so [.] __mpn_rshift
> >> 7.14% 7.14% dd libc-2.28.so [.] _nl_intern_locale_data
> >> 7.14% 7.14% dd libc-2.28.so [.] read_alias_file
> >> ...
> >> --------------------------------------------------------------------
> >>
> >> Signed-off-by: Tan Xiaojun <tanxiaojun@...wei.com>
> >> Suggested-by: James Clark <James.Clark@....com>
> >> Tested-by: Qi Liu <liuqi115@...ilicon.com>
> >> Signed-off-by: James Clark <james.clark@....com>
> >> Cc: Will Deacon <will@...nel.org>
> >> Cc: Mark Rutland <mark.rutland@....com>
> >> Cc: Peter Zijlstra <peterz@...radead.org>
> >> Cc: Ingo Molnar <mingo@...hat.com>
> >> Cc: Arnaldo Carvalho de Melo <acme@...nel.org>
> >> Cc: Alexander Shishkin <alexander.shishkin@...ux.intel.com>
> >> Cc: Jiri Olsa <jolsa@...hat.com>
> >> Cc: Tan Xiaojun <tanxiaojun@...wei.com>
> >> Cc: Al Grant <al.grant@....com>
> >> Cc: Namhyung Kim <namhyung@...nel.org>
> >> ---
> >> tools/perf/arch/arm/util/auxtrace.c | 38 +++++++++++++++++++++++++++++
> >> tools/perf/builtin-record.c | 5 ++++
> >> tools/perf/util/arm-spe.c | 9 +++++++
> >> tools/perf/util/arm-spe.h | 3 +++
> >> tools/perf/util/auxtrace.h | 6 +++++
> >> 5 files changed, 61 insertions(+)
> >>
> >> diff --git a/tools/perf/arch/arm/util/auxtrace.c b/tools/perf/arch/arm/util/auxtrace.c
> >> index 0a6e75b8777a..18f0ea7556e7 100644
> >> --- a/tools/perf/arch/arm/util/auxtrace.c
> >> +++ b/tools/perf/arch/arm/util/auxtrace.c
> >> @@ -10,11 +10,25 @@
> >>
> >> #include "../../util/auxtrace.h"
> >> #include "../../util/debug.h"
> >> +#include "../../util/env.h"
> >> #include "../../util/evlist.h"
> >> #include "../../util/pmu.h"
> >> #include "cs-etm.h"
> >> #include "arm-spe.h"
> >>
> >> +#define SPE_ATTR_TS_ENABLE BIT(0)
> >> +#define SPE_ATTR_PA_ENABLE BIT(1)
> >> +#define SPE_ATTR_PCT_ENABLE BIT(2)
> >> +#define SPE_ATTR_JITTER BIT(16)
> >> +#define SPE_ATTR_BRANCH_FILTER BIT(32)
> >> +#define SPE_ATTR_LOAD_FILTER BIT(33)
> >> +#define SPE_ATTR_STORE_FILTER BIT(34)
> >> +
> >> +#define SPE_ATTR_EV_RETIRED BIT(1)
> >> +#define SPE_ATTR_EV_CACHE BIT(3)
> >> +#define SPE_ATTR_EV_TLB BIT(5)
> >> +#define SPE_ATTR_EV_BRANCH BIT(7)
> >> +
> >> static struct perf_pmu **find_all_arm_spe_pmus(int *nr_spes, int *err)
> >> {
> >> struct perf_pmu **arm_spe_pmus = NULL;
> >> @@ -108,3 +122,27 @@ struct auxtrace_record
> >> *err = 0;
> >> return NULL;
> >> }
> >> +
> >> +void auxtrace__preprocess_evlist(struct evlist *evlist)
> >> +{
> >> + struct evsel *evsel;
> >> + struct perf_pmu *pmu;
> >> +
> >> + evlist__for_each_entry(evlist, evsel) {
> >> + /* Currently only supports precise_ip for branch-misses on arm64 */
> >> + if (!strcmp(perf_env__arch(evlist->env), "arm64")
> >
> > Isn't config ambiguous unless you also check type i.e.
> >
> > && evsel->core.attr.type == PERF_TYPE_HARDWARE
> >
>
> Yes you're right I will add this.
>
> >> + && evsel->core.attr.config == PERF_COUNT_HW_BRANCH_MISSES
> >> + && evsel->core.attr.precise_ip)
> >> + {
> >> + pmu = perf_pmu__find("arm_spe_0");
> >> + if (pmu) {
> >
> > Changing the event seems a bit weird.
> >
>
> This is because there is no support in the kernel for the precise_ip attribute on Arm.
> SPE can give you precise ip data for the same event, but changing the event is required.
I don't think this is the right level to override the event.
It's true that contemporary CPU PMUs can't generate synchronous events,
and hence the kernel doesn't have a precise IP, but that's not
necessarily going to be the case in future. I'd rather we didn't
silently override the event requested by the user, as I think that is
going to cause more problems for us in future.
When the SPE buffer overflows, events will be silently dropped, which I
don't believe is in keeping with the usual semantics of precise events.
Additionally, hard-coding the "arm_spe_0" name means that this will not
work reliably on big.LITTLE systems.
Instead, can we have the user explicitly request to use the SPE PMU in
this way? If the perf tool could be smart with the "_%d" suffix, and
collate PMUs differing only by that, the user would only need to do
something like:
perf record -e arm_spe/branch-misses/pp
... which doesn't seem to onerous.
Is something like that possible?
Thanks,
Mark.
Powered by blists - more mailing lists