Messages in this thread | | | Subject | Re: [RFC][PATCH 4/5] objtool: Enable compilation of objtool for all architectures | From | Julien Thierry <> | Date | Wed, 13 May 2020 17:55:31 +0100 |
| |
On 5/13/20 4:59 PM, Matt Helsley wrote: > 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? >
I do prefer that to the introduction of an arch/missing.
Still, I'm not sure I see much benefit in splitting those small implementations in separate files, but it's not a problem either. This seems more a matter of taste rather than one approach working better than the other. So it's more up to what the maintainer prefer! :)
>>> 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. >
Great, thanks!
-- Julien Thierry
| |