Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [RFC][PATCH 4/5] objtool: Enable compilation of objtool for all
 architectures
To:     Matt Helsley <mhelsley@vmware.com>, linux-kernel@vger.kernel.org,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Miroslav Benes <mbenes@suse.cz>,
        Steven Rostedt <rostedt@goodmis.org>
References: <cover.1588888003.git.mhelsley@vmware.com>
 <9f709ea2ae66cc03b3ff3329baa8f670ccd0e368.1588888003.git.mhelsley@vmware.com>
 <f9eedb02-54fe-fb96-fbcc-5f40f41e571a@redhat.com>
 <20200513155948.GI9040@rlwimi.vmware.com>
From:   Julien Thierry <jthierry@redhat.com>
Message-ID: <e369f0a9-30fc-5754-8cff-bf238ab0b716@redhat.com>
Date:   Wed, 13 May 2020 17:55:31 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <20200513155948.GI9040@rlwimi.vmware.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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