Messages in this thread | | | Date | Wed, 13 May 2020 08:59:48 -0700 | From | Matt Helsley <> | Subject | Re: [RFC][PATCH 4/5] objtool: Enable compilation of objtool for all architectures |
| |
On Tue, May 12, 2020 at 06:04:56PM +0100, Julien Thierry wrote: > Hi Matt, > > On 5/11/20 6:35 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 a "missing" architecture which contains weak definitions > > for tools that do not exist on all architectures. In this case the > > "check" and "orc" tools do not exist on all architectures. > > > > To test building for other architectures ($arch below): > > > > cd tools/objtool/arch > > ln -s missing $arch > > make O=build-$arch ARCH=$arch tools/objtool > > > > Since the stuff under arch/missing is only weak symbols to make up for > missing subcmd implementations, can we put everything in a file > subcmd_defaults.c (name up for debate!) that would be always be compiled an > linked. And some SUBCMD_XXX is set to "y", the corresponding object file > gets compiled and overrides the weak symbols from subcmd_defaults.c .
Hmm, I like keeping them separated along similar lines to the other code because it makes it easier to see the intended correspondence and likely will keep the files more readable / smaller. I could just move them out of arch/missing and into missing_check.c and so forth.
What do you think of that?
> > diff --git a/tools/objtool/Build b/tools/objtool/Build > > index 66f44f5cd2a6..fb6e6faf6f10 100644 > > --- a/tools/objtool/Build > > +++ b/tools/objtool/Build > > @@ -1,11 +1,12 @@ > > objtool-y += arch/$(SRCARCH)/ > > + > > +objtool-$(SUBCMD_CHECK) += 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 > > I'm not convinced by the moving of special under arch/x86 and I didn't > understand it at first. > > I guess you did it because it is only used by the check subcmd, which is > currently only implemented by x86. Is that the reason?
Yeah, that was the original reasoning and this is an artifact of the previous patch set.
> I feel that the proper way to do it would be to leave special.c/h where they > are and have "objtool-$(SUBCMD_CHECK) += special.o". Unless there was some > other motivation for it.
This makes sense. I'll incorporate that in the next posting.
> > objtool-y += objtool.o > > objtool-y += libstring.o > > diff --git a/tools/objtool/Makefile b/tools/objtool/Makefile > > index f591c4d1b6fe..8aac9e133188 100644 > > --- a/tools/objtool/Makefile > > +++ b/tools/objtool/Makefile > > @@ -45,7 +45,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 > > + > > +ifeq ($(SRCARCH),x86) > > + SUBCMD_CHECK := y > > + SUBCMD_ORC := y > > +else > > + SUBCMD_CHECK := n > > + SUBCMD_ORC := n > > +endif > > + > > Nit: Can we default all SUBCMD_* variables to 'n' outside a branch and then > individual arches override the relevant variables to 'y' in their "ifeq > ($SRCARCH, <arch>)" branch?
Definitely -- I'll set them all to n above then we can just have one ifeq block per arch.
Thanks for the Review!
Cheers, -Matt Helsley
| |