Re: [PATCH v2 5/8] modpost: Integrate klp-convert

[Joao Moreira]

On 4/4/19 8:31 AM, Miroslav Benes wrote:
> On Fri, 22 Mar 2019, Joe Lawrence wrote:
>
> > On Fri, Mar 01, 2019 at 11:13:10AM -0300, Joao Moreira wrote:
> > > From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > >
> > > Create cmd_klp_convert and hook it into scripts/Makefile.modpost.
> > > cmd_klp_convert invokes klp-convert with the right arguments for the
> > > conversion of unresolved symbols inside a livepatch.
> > >
> > > [khlebnikov:
> > > * save cmd_ld_ko_o into .module.cmd, if_changed_rule doesn't do that
> > > * fix bashisms for debian where /bin/sh is a symlink to /bin/dash
> > > * rename rule_link_module to rule_ld_ko_o, otherwise arg-check inside
> > >    if_changed_rule compares cmd_link_module and cmd_ld_ko_o
> > > * check modinfo -F livepatch only if CONFIG_LIVEPATCH is true
> > > ]
> > >
> > > [mbenes:
> > > * remove modinfo call. LIVEPATCH_ in Makefiled
> > > ]
> > >
> > > [jmoreira:
> > > * split up: move the .livepatch file-based scheme for identifying
> > > livepatches to a previous patch, as it was required for correctly
> > > building Symbols.list there.
> > > ]
> > >
> > > Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
> > > Signed-off-by: Miroslav Benes <mbenes@xxxxxxx>
> > > Signed-off-by: Joao Moreira <jmoreira@xxxxxxx>
> > > ---
> > >   scripts/Kbuild.include   |  4 +++-
> > >   scripts/Makefile.modpost | 16 +++++++++++++++-
> > >   scripts/mod/modpost.c    |  6 +++++-
> > >   scripts/mod/modpost.h    |  1 +
> > >   4 files changed, 24 insertions(+), 3 deletions(-)
> > >
> > >
> > > [ ... snip ... ]
> > >
> > > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> > > index 7d4af0d0accb..da779a185218 100644
> > > --- a/scripts/Makefile.modpost
> > > +++ b/scripts/Makefile.modpost
> > > @@ -125,8 +125,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
> > >                    -o $@ $(filter-out FORCE,$^) ;                         \
> > >   	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> > >   
> > > +SLIST = $(objtree)/Symbols.list
> > > +KLP_CONVERT = scripts/livepatch/klp-convert
> > > +quiet_cmd_klp_convert = KLP $@
> > > +      cmd_klp_convert = mv $@ $(@:.ko=.klp.o);				\
> > > +			$(KLP_CONVERT) $(SLIST) $(@:.ko=.klp.o) $@
> > > +
> > > +define rule_ld_ko_o
> > > +	$(call cmd,ld_ko_o) $(cmd_ld_ko_o) ;				\
> >                             ^
> > Should there be a ';' semicolon here (and maybe a line-break) between
> > $(call cmd,ld_ko_o) and $(cmd_ld_ko_o)?
>
> Probably yes.
>
> Originally there was
> 	$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ;
>
> but "echo-" disappeared somewhere. Joao?

Hi,

I just verified my sources and the echo-cmd got lost in between the v2 
rebases. I believe it must have happened while I was manually fixing a 
conflict, or something. Sorry.

Here is a piece of the original snippet, as it was supposed to be:

define rule_ld_ko_o 

       $(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ; 
\
       $(call save-cmd,ld_ko_o) ; 
\
       $(if $(CONFIG_LIVEPATCH),                                       \

Joe, do you mind fixing it for v3?

Thanks for spotting it.

> So maybe even
> 	$(call cmd,ld_ko_o)
> could be enough, because cmd contains echo-cmd, but I'm not good at
> reading scripts/Kbuild.include.
>   
> > I didn't see this in my x86_64 VM, but on a ppc64le box, I kept getting
> > really strange build errors that I traced to this line.  Without a
> > semicolon, the build was trying to run a make command with a linker
> > command smashed onto the end of it:
> >
> >    make -f ./arch/powerpc/Makefile.postlink crypto/xts.ko ld -r  -EL -m elf64lppc -T ./scripts/module-common.lds -T ./arch/powerpc/kernel/module.lds --save-restore-funcs  --build-id  -o crypto/xts.ko crypto/xts.o crypto/xts.mod.o
>
> Miroslav