Received-SPF: pass (google.com: best guess record for 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 03/36] objtool: Enable compilation of objtool for all
 architectures
To:     Matt Helsley <mhelsley@vmware.com>, linux-kernel@vger.kernel.org
Cc:     Josh Poimboeuf <jpoimboe@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Miroslav Benes <mbenes@suse.cz>
References: <cover.1586468801.git.mhelsley@vmware.com>
 <a96d42818c7e8f2a8bd2e151b8c220193f4ae986.1586468801.git.mhelsley@vmware.com>
From:   Julien Thierry <jthierry@redhat.com>
Message-ID: <e8a52162-dd38-6092-7217-cc5c088abadc@redhat.com>
Date:   Tue, 14 Apr 2020 08:39:23 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.4.1
MIME-Version: 1.0
In-Reply-To: <a96d42818c7e8f2a8bd2e151b8c220193f4ae986.1586468801.git.mhelsley@vmware.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Matt,

On 4/10/20 8: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
> 
> This uses the weak, no-op definitions of the "check" and "orc"
> commands for the newly-supported architecture. Presently these
> exit with 127 to indicate that the subcommands are missing.
> Subsequent patches can then be made to determine if the weak
> definitions are used and explicitly report a missing command,
> and even to list which subcommands are supported (perhaps if
> no subcommand is specified it can list the supported subcommands).
> 
> 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.
> 
> This commit allows us to demonstrate the pattern of adding
> architecture support and isolates moving the various files from
> adding support for more objtool subcommands.
> 
> Signed-off-by: Matt Helsley <mhelsley@vmware.com>
> ---
>   tools/objtool/Build                     |  4 ---
>   tools/objtool/arch/missing/Build        |  3 ++
>   tools/objtool/arch/missing/check.c      | 14 +++++++++
>   tools/objtool/arch/missing/orc_dump.c   | 11 +++++++
>   tools/objtool/arch/missing/orc_gen.c    | 16 ++++++++++
>   tools/objtool/arch/x86/Build            |  4 +++
>   tools/objtool/{ => arch/x86}/arch.h     | 42 ++++++++++++++++++++++++-
>   tools/objtool/{ => arch/x86}/cfi.h      |  0
>   tools/objtool/{ => arch/x86}/check.c    | 11 ++++---
>   tools/objtool/arch/x86/decode.c         |  2 +-
>   tools/objtool/{ => arch/x86}/orc_dump.c |  5 +--
>   tools/objtool/{ => arch/x86}/orc_gen.c  |  9 ++++--
>   tools/objtool/{ => arch/x86}/special.c  |  4 +--
>   tools/objtool/{ => arch/x86}/special.h  |  2 +-
>   tools/objtool/builtin-orc.c             |  2 +-
>   tools/objtool/check.h                   | 37 ----------------------
>   tools/objtool/objtool.h                 |  2 +-
>   tools/objtool/orc.h                     |  2 --
>   18 files changed, 110 insertions(+), 60 deletions(-)
>   create mode 100644 tools/objtool/arch/missing/Build
>   create mode 100644 tools/objtool/arch/missing/check.c
>   create mode 100644 tools/objtool/arch/missing/orc_dump.c
>   create mode 100644 tools/objtool/arch/missing/orc_gen.c
>   rename tools/objtool/{ => arch/x86}/arch.h (59%)
>   rename tools/objtool/{ => arch/x86}/cfi.h (100%)
>   rename tools/objtool/{ => arch/x86}/check.c (99%)
>   rename tools/objtool/{ => arch/x86}/orc_dump.c (98%)
>   rename tools/objtool/{ => arch/x86}/orc_gen.c (96%)
>   rename tools/objtool/{ => arch/x86}/special.c (98%)
>   rename tools/objtool/{ => arch/x86}/special.h (95%)
> 

My concern with this it that most of the structures and code in arch.h 
and check.c can/should be reused across architectures. So, when 
providing support for a new architecutre, the first thing that will be 
needed is to move those back under tools/objtool whithout disturbing the 
arches that don't yet provide support for "check" subcommand.

So, if it is decided that recordmcount should be an objtool subcommand, 
the code itself should probably stay under tools/objtool and then have 
different compilation configurations for objtool depending on the 
architecture (e.g. HAVE_OBJTOOL_CHECK, HAVE_OBJTOOL_ORC) or something of 
the sort.

Cheers,

-- 
Julien Thierry