linux-kernel - Re: [PATCH v4 1/2] perf machine: Support arch's specific kernel start address

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20190812073931.GB8062@leoy-ThinkPad-X240s>
Date:   Mon, 12 Aug 2019 15:39:31 +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 10:23:21AM +0300, Adrian Hunter wrote:
> On 12/08/19 10:02 AM, Leo Yan wrote:
> > 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?
> 
> I don't know how you intend to calculate ARM_PRE_START_SIZE, but below is OK
> for x86.
> 
> > 
> > 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__is() does not normalize the architecture, so you may want to use
> perf_env__arch() instead.

You are right, thanks for suggestion.  Will use perf_env__arch() in
next spin.

Thanks,
Leo Yan

> 
> > +                       machine->kernel_start -= ARM_PRE_START_SIZE;
> >         }
> >         return err;
> >  }

[...]