Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220309190917.w3tq72alughslanq@ast-mbp.dhcp.thefacebook.com>
 <YinGZObp37b27LjK@hirez.programming.kicks-ass.net> <YioBZmicMj7aAlLf@hirez.programming.kicks-ass.net>
 <YionV0+v/cUBiOh0@hirez.programming.kicks-ass.net> <YisnG9lW6kp8lBp3@hirez.programming.kicks-ass.net>
 <CAADnVQJfffD9tH_cWThktCCwXeoRV1XLZq69rKK5vKy_y6BN8A@mail.gmail.com>
 <20220312154407.GF28057@worktop.programming.kicks-ass.net>
 <CAADnVQL7xrafAviUJg47LfvFSJpgZLwyP18Bm3S_KQwRyOpheQ@mail.gmail.com>
 <20220313085214.GK28057@worktop.programming.kicks-ass.net>
 <Yi9YOdn5Nbq9BBwd@hirez.programming.kicks-ass.net> <20220315081522.GA8939@worktop.programming.kicks-ass.net>
In-Reply-To: <20220315081522.GA8939@worktop.programming.kicks-ass.net>
From:   Masahiro Yamada <masahiroy@kernel.org>
Date:   Wed, 16 Mar 2022 01:28:08 +0900
Message-ID: <CAK7LNAReAKXT97NHEnC-1UXozdcPdYNHR55knNRDatJr_GqrrA@mail.gmail.com>
Subject: Re: [PATCH v4 00/45] x86: Kernel IBT
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Kumar Kartikeya Dwivedi <memxor@gmail.com>,
        X86 ML <x86@kernel.org>, joao@overdrivepizza.com,
        "H . J . Lu" <hjl.tools@gmail.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Kees Cook <keescook@chromium.org>,
        Sami Tolvanen <samitolvanen@google.com>,
        Mark Rutland <mark.rutland@arm.com>, alyssa.milburn@intel.com,
        Miroslav Benes <mbenes@suse.cz>,
        Steven Rostedt <rostedt@goodmis.org>,
        Masami Hiramatsu <mhiramat@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Mar 15, 2022 at 5:15 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Mar 14, 2022 at 03:59:05PM +0100, Peter Zijlstra wrote:
> > On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > > During the build with gcc 8.5 I see:
> > > >
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .ibt_endbr_seal, skipping
> > > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > > .orc_unwind section, skipping
> > > >   LD [M]  crypto/async_tx/async_xor.ko
> > > >   LD [M]  crypto/authenc.ko
> > > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > > make[3]: *** Waiting for unfinished jobs....
> > > >
> > > > but make clean cures it.
> > > > I suspect it's some missing makefile dependency.
> > >
> > > Yes, I recently ran into it; I've been trying to kick Makefile into
> > > submission but have not had success yet. Will try again on Monday.
> > >
> > > Problem appears to be that it will re-link .ko even though .o hasn't
> > > changed, resulting in duplicate objtool runs. I've been trying to have
> > > makefile generate .o.objtool empty file to serve as dependency marker to
> > > avoid doing second objtool run, but like said, no luck yet.
> >
> > Masahiro-san, I'm trying the below, but afaict it's not working because
> > the rule for the .o file itself:
> >
> Ha, sleep, it is marvelous!
>
> The below appears to be working as desired.
>
> ---
> Index: linux-2.6/scripts/Makefile.build
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.build
> +++ linux-2.6/scripts/Makefile.build
> @@ -86,12 +86,18 @@ ifdef need-builtin
>  targets-for-builtin += $(obj)/built-in.a
>  endif
>
> -targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +targets-for-modules :=


Why do you need to change this line?


>
>  ifdef CONFIG_LTO_CLANG
>  targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
>  endif
>
> +ifdef CONFIG_X86_KERNEL_IBT
> +targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
> +endif
> +
> +targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
> +
>  ifdef need-modorder
>  targets-for-modules += $(obj)/modules.order
>  endif
> @@ -276,6 +282,19 @@ cmd_mod = { \
>  $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
>         $(call if_changed,mod)
>
> +#
> +# Since objtool will re-write the file it will change the timestamps, therefore
> +# it is critical that the %.objtool file gets a timestamp *after* objtool runs.


Thanks for explaining how stupidly this works.
NACK.


I guess re-using the current clang-lto rule is much cleaner.
(but please rename  %.lto.o  to  %.prelink.o)


Roughly like this:


if CONFIG_LTO_CLANG || CONFIG_X86_KERNEL_IBT

$(obj)/%.prelink:  $(obj)/%.o FORCE
        [  $(LD) if CONFIG_LTO_CLANG ]   +  $(cmd_objtool)

endif


> +#
> +# Additionally, care must be had with ordering this rule against the other rules
> +# that take %.o as a dependency.
> +#
> +cmd_objtool_mod =                                                      \
> +       true $(cmd_objtool) ; touch $@
> +
> +$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
> +       $(call if_changed,objtool_mod)
> +
>  quiet_cmd_cc_lst_c = MKLST   $@
>        cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
>                      $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
> Index: linux-2.6/scripts/Makefile.lib
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.lib
> +++ linux-2.6/scripts/Makefile.lib
> @@ -552,9 +552,8 @@ objtool_args =                                                              \
>         $(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)             \
>         $(if $(CONFIG_SLS), --sls)
>
> -cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
> -cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
> -cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
> +cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
> +cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
>
>  endif # CONFIG_STACK_VALIDATION
>
> @@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
>
>  # instead run objtool on the module as a whole, right before
>  # the final link pass with the linker script.
> -%.ko: objtool-enabled = y
> -%.ko: part-of-module := y
> +$(obj)/%.objtool: objtool-enabled = y
> +$(obj)/%.objtool: part-of-module := y
>
>  else
>
> Index: linux-2.6/scripts/Makefile.modfinal
> ===================================================================
> --- linux-2.6.orig/scripts/Makefile.modfinal
> +++ linux-2.6/scripts/Makefile.modfinal
> @@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
>
>  quiet_cmd_ld_ko_o = LD [M]  $@
>        cmd_ld_ko_o +=                                                   \
> -       $(cmd_objtool_mod)                                              \
>         $(LD) -r $(KBUILD_LDFLAGS)                                      \
>                 $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)              \
>                 -T scripts/module.lds -o $@ $(filter %.o, $^);          \


-- 
Best Regards
Masahiro Yamada