[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190812070236.GA8062@leoy-ThinkPad-X240s>
Date: Mon, 12 Aug 2019 15:02:36 +0800
From: Leo Yan <leo.yan@...aro.org>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
David Miller <davem@...emloft.net>,
Milian Wolff <milian.wolff@...b.com>,
Donald Yandt <donald.yandt@...il.com>,
Davidlohr Bueso <dave@...olabs.net>,
Wei Li <liwei391@...wei.com>, Mark Drayton <mbd@...com>,
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@...il.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
bpf@...r.kernel.org, clang-built-linux@...glegroups.com,
Mathieu Poirier <mathieu.poirier@...aro.org>
Subject: Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel
start address
On Mon, Aug 12, 2019 at 09:37:33AM +0300, Adrian Hunter wrote:
> On 10/08/19 10:21 AM, Leo Yan wrote:
> > machine__get_kernel_start() gives out the kernel start address; some
> > architectures need to tweak the start address so that can reflect the
> > kernel start address correctly. This is not only for x86_64 arch, but
> > it is also required by other architectures, e.g. arm/arm64 needs to
> > tweak the kernel start address so can include the kernel memory regions
> > which are used before the '_stext' symbol.
> >
> > This patch refactors machine__get_kernel_start() by adding a weak
> > arch__fix_kernel_text_start(), any architecture can implement it to
> > tweak its specific start address; this also allows the arch specific
> > code to be placed into 'arch' folder.
> >
> > Signed-off-by: Leo Yan <leo.yan@...aro.org>
> > ---
> > tools/perf/arch/x86/util/machine.c | 10 ++++++++++
> > tools/perf/util/machine.c | 13 +++++++------
> > tools/perf/util/machine.h | 2 ++
> > 3 files changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/tools/perf/arch/x86/util/machine.c b/tools/perf/arch/x86/util/machine.c
> > index 1e9ec783b9a1..9f012131534a 100644
> > --- a/tools/perf/arch/x86/util/machine.c
> > +++ b/tools/perf/arch/x86/util/machine.c
> > @@ -101,4 +101,14 @@ int machine__create_extra_kernel_maps(struct machine *machine,
> > return ret;
> > }
> >
> > +void arch__fix_kernel_text_start(u64 *start)
> > +{
> > + /*
> > + * On x86_64, PTI entry trampolines are less than the
> > + * start of kernel text, but still above 2^63. So leave
> > + * kernel_start = 1ULL << 63 for x86_64.
> > + */
> > + *start = 1ULL << 63;
> > +}
>
> That is needed for reporting x86 data on any arch i.e. it is not specific to
> the compile-time architecture, it is specific to the perf.data file
> architecture, which is what machine__is() compares. So, this looks wrong.
Thanks for reviewing, Adrian.
If so, I think we should extend the function machine__get_kernel_start()
as below; for building successfully, will always define the macro
ARM_PRE_START_SIZE in Makefile.config.
@Arnaldo, @Adrian, Please let me know if this works for you?
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index f6ee7fbad3e4..30a0ff627263 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2687,13 +2687,26 @@ int machine__get_kernel_start(struct machine *machine)
machine->kernel_start = 1ULL << 63;
if (map) {
err = map__load(map);
+ if (err)
+ return err;
+
/*
* On x86_64, PTI entry trampolines are less than the
* start of kernel text, but still above 2^63. So leave
* kernel_start = 1ULL << 63 for x86_64.
*/
- if (!err && !machine__is(machine, "x86_64"))
+ if (!machine__is(machine, "x86_64"))
machine->kernel_start = map->start;
+
+ /*
+ * On arm/arm64, some memory regions are prior to '_stext'
+ * symbol; to reflect the complete kernel address space,
+ * compensate these pre-defined regions for kernel start
+ * address.
+ */
+ if (machine__is(machine, "arm64") ||
+ machine__is(machine, "arm"))
+ machine->kernel_start -= ARM_PRE_START_SIZE;
}
return err;
}
Thanks,
Leo Yan
> > +
> > #endif
> > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> > index f6ee7fbad3e4..603518835692 100644
> > --- a/tools/perf/util/machine.c
> > +++ b/tools/perf/util/machine.c
> > @@ -2671,6 +2671,10 @@ int machine__nr_cpus_avail(struct machine *machine)
> > return machine ? perf_env__nr_cpus_avail(machine->env) : 0;
> > }
> >
> > +void __weak arch__fix_kernel_text_start(u64 *start __maybe_unused)
> > +{
> > +}
> > +
> > int machine__get_kernel_start(struct machine *machine)
> > {
> > struct map *map = machine__kernel_map(machine);
> > @@ -2687,14 +2691,11 @@ int machine__get_kernel_start(struct machine *machine)
> > machine->kernel_start = 1ULL << 63;
> > if (map) {
> > err = map__load(map);
> > - /*
> > - * On x86_64, PTI entry trampolines are less than the
> > - * start of kernel text, but still above 2^63. So leave
> > - * kernel_start = 1ULL << 63 for x86_64.
> > - */
> > - if (!err && !machine__is(machine, "x86_64"))
> > + if (!err)
> > machine->kernel_start = map->start;
> > }
> > +
> > + arch__fix_kernel_text_start(&machine->kernel_start);
> > return err;
> > }
> >
> > diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
> > index ef803f08ae12..9cb459f4bfbc 100644
> > --- a/tools/perf/util/machine.h
> > +++ b/tools/perf/util/machine.h
> > @@ -278,6 +278,8 @@ void machine__get_kallsyms_filename(struct machine *machine, char *buf,
> > int machine__create_extra_kernel_maps(struct machine *machine,
> > struct dso *kernel);
> >
> > +void arch__fix_kernel_text_start(u64 *start);
> > +
> > /* Kernel-space maps for symbols that are outside the main kernel map and module maps */
> > struct extra_kernel_map {
> > u64 start;
> >
>
Powered by blists - more mailing lists