lkml.org 
[lkml]   [2020]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/3] objtool: Enable compilation of objtool for all architectures
On Wed, May 20, 2020 at 09:31:46AM +0100, Julien Thierry wrote:
>
>
> On 5/19/20 9:55 PM, Matt Helsley wrote:
> > objtool currently only compiles for x86 architectures. This is
> > fine as it presently does not support tooling for other
> > architectures. However, we would like to be able to convert other
> > kernel tools to run as objtool sub commands because they too
> > process ELF object files. This will allow us to convert tools
> > such as recordmcount to use objtool's ELF code.
> >
> > Since much of recordmcount's ELF code is copy-paste code to/from
> > a variety of other kernel tools (look at modpost for example) this
> > means that if we can convert recordmcount we can convert more.
> >
> > We define "missing" weak definitions for subcommand entry functions
> > and other weak definitions for shared functions critical to
> > building existing subcommands. These return 127 when the command is
> > missing which signify tools that do not exist on all architectures.
> > In this case the "check" and "orc" tools do not exist on all
> > architectures so we only add them for x86. Future changes adding
> > support for "check", to arm64 for example, can then modify the
> > SUBCMD_CHECK variable when building for arm64.
> >
> > objtool is not currently wired in to KConfig to be built for other
> > architectures because it's not needed for those architectures and
> > there are no commands it supports other than those for x86. As more
> > command support is enabled on various architectures the necessary
> > KConfig changes can be made (e.g. adding "STACK_VALIDATION") to
> > trigger building objtool.
> >
> > Signed-off-by: Matt Helsley <mhelsley@vmware.com>
> > Cc: Julien Thierry <jthierry@redhat.com>
> > ---
> > tools/objtool/Build | 13 +++++++++----
> > tools/objtool/Makefile | 11 ++++++++++-
> > tools/objtool/arch.h | 4 +++-
> > tools/objtool/builtin-check.c | 2 +-
> > tools/objtool/builtin-orc.c | 3 +--
> > tools/objtool/check.c | 4 ++--
> > tools/objtool/check.h | 4 ----
> > tools/objtool/objtool.h | 14 ++++++++++++++
> > tools/objtool/orc.h | 18 ------------------
> > tools/objtool/orc_dump.c | 3 ++-
> > tools/objtool/orc_gen.c | 1 -
> > tools/objtool/weak.c | 35 +++++++++++++++++++++++++++++++++++
> > 12 files changed, 77 insertions(+), 35 deletions(-)
> > delete mode 100644 tools/objtool/orc.h
> > create mode 100644 tools/objtool/weak.c
> >
> > diff --git a/tools/objtool/Build b/tools/objtool/Build
> > index 66f44f5cd2a6..b7222d5cc7bc 100644
> > --- a/tools/objtool/Build
> > +++ b/tools/objtool/Build
> > @@ -1,11 +1,16 @@
> > objtool-y += arch/$(SRCARCH)/
> > +
> > +objtool-y += weak.o
> > +
> > +objtool-$(SUBCMD_CHECK) += check.o
> > +objtool-$(SUBCMD_CHECK) += special.o
> > +objtool-$(SUBCMD_ORC) += check.o
> > +objtool-$(SUBCMD_ORC) += orc_gen.o
> > +objtool-$(SUBCMD_ORC) += orc_dump.o
> > +
> > objtool-y += builtin-check.o
> > objtool-y += builtin-orc.o
> > -objtool-y += check.o
> > -objtool-y += orc_gen.o
> > -objtool-y += orc_dump.o
> > objtool-y += elf.o
> > -objtool-y += special.o
> > objtool-y += objtool.o
> > objtool-y += libstring.o
> > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile
> > index 6b91388aecbb..12686e2f1a56 100644
> > --- a/tools/objtool/Makefile
> > +++ b/tools/objtool/Makefile
> > @@ -46,7 +46,16 @@ elfshdr := $(shell echo '$(pound)include <libelf.h>' | $(CC) $(CFLAGS) -x c -E -
> > CFLAGS += $(if $(elfshdr),,-DLIBELF_USE_DEPRECATED)
> > AWK = awk
> > -export srctree OUTPUT CFLAGS SRCARCH AWK
> > +
> > +SUBCMD_CHECK := n
> > +SUBCMD_ORC := n
> > +
> > +ifeq ($(SRCARCH),x86)
> > + SUBCMD_CHECK := y
> > + SUBCMD_ORC := y
> > +endif
> > +
> > +export srctree OUTPUT CFLAGS SRCARCH AWK SUBCMD_CHECK SUBCMD_ORC
>
> Nit: I was thinking, since the list of SUBCMD_* is only going to grow
> maybe it would be nicer to have a single export line for the SUBCMD_*
> variables and leave the export line of [srctree..AWK] untouched.

That's a really good idea actually. Glad to see Josh included it in the
changed patch.

>
> Just a suggestion, and only in case you respin this taking into account
> Josh's comment.
>
> Otherwise things look good to me.

Thanks for all the reviews and good ideas!

Cheers,
-Matt Helsley

\
 
 \ /
  Last update: 2020-05-20 18:41    [W:0.222 / U:0.372 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site