Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Feb 2022 12:05:06 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Josh Poimboeuf <jpoimboe@redhat.com>
Cc:     x86@kernel.org, joao@overdrivepizza.com, hjl.tools@gmail.com,
        andrew.cooper3@citrix.com, linux-kernel@vger.kernel.org,
        ndesaulniers@google.com, keescook@chromium.org,
        samitolvanen@google.com, mark.rutland@arm.com,
        alyssa.milburn@intel.com
Subject: Re: [PATCH 21/29] objtool: Rename --duplicate to --lto
Message-ID: <YhysYkcfwLr68Job@hirez.programming.kicks-ass.net>
References: <20220218164902.008644515@infradead.org>
 <20220218171409.814392411@infradead.org>
 <20220226194209.bvv3t65hhtnwltmk@treble>
 <20220226214802.4chmsrtstlerefmu@treble>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220226214802.4chmsrtstlerefmu@treble>
Precedence: bulk

On Sat, Feb 26, 2022 at 01:48:02PM -0800, Josh Poimboeuf wrote:
> On Sat, Feb 26, 2022 at 11:42:13AM -0800, Josh Poimboeuf wrote:
> > > +	OPT_BOOLEAN(0, "lto", &lto, "whole-archive like runs"),
> > 
> > "--lto" is a confusing name, since this "feature" isn't specific to LTO.
> > 
> > Also, it gives no indication of what it actually does.
> >
> > What it does is, run objtool on vmlinux or module just like it's a
> > normal object, and *don't* do noinstr validation.  Right?

How about --whole-archive, much like the linker then?

The distinction is that we run objtool *only* on vmlinux and modules and
not also on the individual .o files.

There's 3 models:

 A) every translation unit
    (module parts get --module)

 B) every translation unit + shallow vmlinux
    (module parts get --module, vmlinux.o gets --vmlinux)

 C) vmlinux + modules
    (modules get --module, vmlinux.o gets --vmlinux
    --duplicate/lto/whole-archive, pick your poison).


objtool started out with (A); then for noinstr validation I added a
shallow vmlinux pass that *only* checks .noinstr.text and .entry.text
for escapes (B). This is to not unduly add time to the slowest (single
threaded) part of the kernel build, linking vmlinux.

Then CLANG_LTO added (C), due to LTO there simply isn't asm to poke at
until the whole-archive thing. But this means that the vmlinux run needs
to do all validation, not only the shallow noinstr validation.
--duplicate was added there, a bad name because it really doesn't do
duplicate work, it's the first and only objtool run (it's only duplicate
if you also run on each TU, but we don't do that).

Now with these patches I need whole-archive objtool passes and instead
of making a 4th mode, or extend (B), I chose to just bite the bullet and
go full LTO style (C).

Now, I figured it would be good to have a flag to indicate we're running
LTO style and --duplicate is more or less that, except for the terrible
name.

> > It's weird for the noinstr-only-mode to be the default.
> > 
> > BTW "--duplicate" had similar problems...
> > 
> > So how about:
> > 
> > - Default to normal mode on vmlinux/module, i.e. validate and/or
> >   generate ORC like any other object.  This default is more logically
> >   consistent and makes sense for the future once we get around to
> >   parallelizing objtool.
> > 
> > - Have "--noinstr", which does noinstr validation, in addition to all
> >   the other objtool validation/generation.  So it's additive, like any
> >   other cmdline option.  (Maybe this option isn't necessarily needed for
> >   now.)
> 
> It just dawned on me that "--noinstr" already exists.  But I'm
> scratching my head trying to figure out the difference between
> "--noinstr" and omitting "--lto".

If you run: "objtool check --vmlinux --noinstr vmlinux.o", it'll only do
the shallow .noinstr.text/.entry.text checks. If OTOH you do: "objtool
check --vmlinux --noinstr --lto vmlinux.o" it'll do everything
(including noinstr).

Similarlt, "--module --lto" will come to mean whole module (which is
currently not distinguishable from a regular module part run).

(barring the possible 's/--lto/--whole-archive/' rename proposed up top)