[PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools

[PATCH 0/3] kbuild: add scripts/Makefile.toolcheck to check necessary host tools

[Masahiro Yamada]

I wrote this patch set to cater to some reports:

 - Bernd Edlinger reports false positive check of libelf
   once CONFIG_UNWINDER_ORC is enabled.
   https://lore.kernel.org/patchwork/patch/960411/

 - Borislav Petkov reports the build fails very late
   if CONFIG_KERNEL_LZ4 is enabled but lz4 is missing.
   https://patchwork.kernel.org/patch/10635381/

From the discussion
https://patchwork.kernel.org/patch/10516049/
people like to see a build error when some host tools
are missing.

So, we need to check host tools in makefiles.
(Or, do not check at all. The build will fail anyway,
but the error message might not be clear enough in some cases.)

A problem is people check host tools
in random places in the top Makefile.
This does not work because the top Makefile is parsed twice
when the include/config/auto.config is updated.
Once with a stale auto.config, and once again with a
new auto.conf.
This will cause false positive build error.

One solution is to provide a systematic way to
do host-tool checks.

I added small macros in scripts/Makefile.toolcheck
to describe tool checks in a Kbuild-ish taste.


Masahiro Yamada (3):
  kbuild: provide a new place to check necessary host tools
  objtool: move libelf check out of top Makefile
  kbuild: check the presence of lzo and lz4 tools when necessary

 Makefile                   | 22 ++++-----------------
 scripts/Makefile.build     |  2 --
 scripts/Makefile.toolcheck | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 20 deletions(-)
 create mode 100644 scripts/Makefile.toolcheck

-- 
2.7.4

[PATCH 1/3] kbuild: provide a new place to check necessary host tools

[Masahiro Yamada]

Enabling CONFIG options often requires additional features, like
target compiler capabilities, host tools, etc.

As for the target compiler capabilities, Kconfig is able to add
proper dependencies. For example,

  config STACKPROTECTOR
          ...
          depends on $(cc-option,-fstack-protector)

The CONFIG option is simply disabled if unsupported by the compiler.
This seems sensible since otherwise, there is no way to avoid the
build error.

In the discussion [1], doing similar for host tools was unacceptable.
Build errors caused by missing host tools are easy to fix; just
install them to your build machine. So, let the build fail instead
of disabling CONFIG options silently.

We already have such checks in random places in makefiles. What I
do not like is that people tend to do that in the top Makefile.

The top Makefile is too cluttered, and what is worse, $(error ...)
functions in the top Makefile could be false positive. (Please note
the top Makefile includes the stale include/config/auto.conf before
syncconfig updates it.)

This adds a new file scripts/Makefile.toolcheck to check additional
tools depending on the kernel configuration. This check is run
immediately after syncconfig, i.e., when a user attempts to build
something with a new set of CONFIG options.

[1] https://patchwork.kernel.org/patch/10516049/

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile                   |  1 +
 scripts/Makefile.toolcheck | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 scripts/Makefile.toolcheck

diff --git a/Makefile b/Makefile
index bf3786e..23a204a 100644
--- a/Makefile
+++ b/Makefile
@@ -633,6 +633,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
 # include/config/auto.conf (which mirrors .config).
 include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
 	$(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
+	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.toolcheck
 else
 # External modules and some install targets need include/generated/autoconf.h
 # and include/config/auto.conf but do not care if they are up-to-date.
diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
new file mode 100644
index 0000000..f3c165d
--- /dev/null
+++ b/scripts/Makefile.toolcheck
@@ -0,0 +1,35 @@
+# SPDX-License-Identifier: GPL-2.0
+# ===========================================================================
+# Host tools check
+# ===========================================================================
+#
+# Some additional tools might be required depending on the kernel configuration.
+# Check if they are installed on the host machine, and if missing, error out
+# with a user-friendly message.
+
+include include/config/auto.conf
+
+__toolcheck:
+	@:
+
+PHONY += $(toolcheck-y)
+__toolcheck: $(toolcheck-y)
+
+define populate_toolcheck
+__toolcheck: check_$(1)
+PHONY += check_$(1)
+check_$(1):
+	$(Q){ $(chk_$(1)); } >/dev/null 2>&1 || {	\
+		echo "*" >&2;				\
+		for msg in $(msg_$(1));			\
+		do					\
+			echo "* $$$${msg}" >&2;		\
+		done;					\
+		echo "*" >&2;				\
+		/bin/false;				\
+	}
+endef
+
+$(foreach c, $(toolcheck-y), $(eval $(call populate_toolcheck,$(c))))
+
+.PHONY: $(PHONY)
-- 
2.7.4

[PATCH 2/3] objtool: move libelf check out of top Makefile

[Masahiro Yamada]

Bernd Edlinger reports:

  The next make after an oldconfig reads in the outdated
  include/config/auto.conf which can kill the make before
  it is able to call the syncconfig target.

  $ make defconfig
  *** Default configuration is based on 'x86_64_defconfig'
  $ make
  scripts/kconfig/conf  --syncconfig Kconfig
  Makefile:936: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y,
  please install libelf-dev, libelf-devel or elfutils-libelf-devel".  Stop.

  $ sed -i s/CONFIG_UNWINDER_ORC=y// .configs
  $ make oldconfig
  Choose kernel unwinder
  > 1. ORC unwinder (UNWINDER_ORC) (NEW)
    2. Frame pointer unwinder (UNWINDER_FRAME_POINTER)
  choice[1-2?]: 2
  $ make
  Makefile:936: *** "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y,
  please install libelf-dev, libelf-devel or elfutils-libelf-devel".  Stop.

I tried to fix it by moving the libelf evaluation to the Kconfig
stage [1], but it was rejected.

Kconfig is now able to run shell commands to evaluate any prerequisites
needed to enable CONFIG options.

For example, as shown in commit 2a61f4747eea ("stack-protector: test
compiler capability in Kconfig and drop AUTO mode"), the new Kconfig
syntax is useful to evaluate target compiler capabilities.

However, disabling the CONFIG option silently just because of missing
libelf would not be a preferred behavior; in this case, installing an
appropriate package will solve the problem. Hence, this check will be
kept in a makefile, but somewhere else than the top Makefile.

Move the check to scripts/Makefile.toolcheck so that it is run after
syncconfig.

Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
was previously a warning, but now a error.

[1] https://patchwork.kernel.org/patch/10516049/

Reported-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Makefile                   | 21 +++------------------
 scripts/Makefile.build     |  2 --
 scripts/Makefile.toolcheck |  5 +++++
 3 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/Makefile b/Makefile
index 23a204a..71940b7 100644
--- a/Makefile
+++ b/Makefile
@@ -949,23 +949,6 @@ mod_sign_cmd = true
 endif
 export mod_sign_cmd
 
-ifdef CONFIG_STACK_VALIDATION
-  has_libelf := $(call try-run,\
-		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -,1,0)
-  ifeq ($(has_libelf),1)
-    objtool_target := tools/objtool FORCE
-  else
-    ifdef CONFIG_UNWINDER_ORC
-      $(error "Cannot generate ORC metadata for CONFIG_UNWINDER_ORC=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
-    else
-      $(warning "Cannot use CONFIG_STACK_VALIDATION=y, please install libelf-dev, libelf-devel or elfutils-libelf-devel")
-    endif
-    SKIP_STACK_VALIDATION := 1
-    export SKIP_STACK_VALIDATION
-  endif
-endif
-
-
 ifeq ($(KBUILD_EXTMOD),)
 core-y		+= kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
 
@@ -1115,7 +1098,9 @@ uapi-asm-generic:
 	            src=uapi/asm obj=arch/$(SRCARCH)/include/generated/uapi/asm
 
 PHONY += prepare-objtool
-prepare-objtool: $(objtool_target)
+ifdef CONFIG_STACK_VALIDATION
+prepare-objtool: tools/objtool
+endif
 
 # Generate some files
 # ---------------------------------------------------------------------------
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 54da4b0..e9dabe4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -233,7 +233,6 @@ endif # CC_USING_RECORD_MCOUNT
 endif # CONFIG_FTRACE_MCOUNT_RECORD
 
 ifdef CONFIG_STACK_VALIDATION
-ifneq ($(SKIP_STACK_VALIDATION),1)
 
 __objtool_obj := $(objtree)/tools/objtool/objtool
 
@@ -270,7 +269,6 @@ objtool_obj = $(if $(patsubst y%,, \
 	$(OBJECT_FILES_NON_STANDARD_$(basetarget).o)$(OBJECT_FILES_NON_STANDARD)n), \
 	$(__objtool_obj))
 
-endif # SKIP_STACK_VALIDATION
 endif # CONFIG_STACK_VALIDATION
 
 # Rebuild all objects when objtool changes, or is enabled/disabled.
diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
index f3c165d..bc26fc0 100644
--- a/scripts/Makefile.toolcheck
+++ b/scripts/Makefile.toolcheck
@@ -12,6 +12,11 @@ include include/config/auto.conf
 __toolcheck:
 	@:
 
+chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
+msg_stack_validation = "libelf is necessary for building the objtool." \
+		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
+toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
+
 PHONY += $(toolcheck-y)
 __toolcheck: $(toolcheck-y)
 
-- 
2.7.4

[PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary

[Masahiro Yamada]

If CONFIG_KERNEL_LZ4 is enabled without lz4 tool installed on the
system, the build fails at the very last stage (reported by
Borislav Petkov [1]).

  LZO     arch/x86/boot/compressed/vmlinux.bin.lzo
/bin/sh: 1: lzop: not found
arch/x86/boot/compressed/Makefile:141: recipe for target 'arch/x86/boot/compressed/vmlinux.bin.lzo' failed
make[2]: *** [arch/x86/boot/compressed/vmlinux.bin.lzo] Error 1
arch/x86/boot/Makefile:112: recipe for target 'arch/x86/boot/compressed/vmlinux' failed
make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2
arch/x86/Makefile:284: recipe for target 'bzImage' failed
make: *** [bzImage] Error 2

Check the tools in scripts/Makefile.toolcheck to fail the build
earlier with a more readable message.

[1] https://patchwork.kernel.org/patch/10635381/

Suggested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 scripts/Makefile.toolcheck | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
index bc26fc0..5e336e4 100644
--- a/scripts/Makefile.toolcheck
+++ b/scripts/Makefile.toolcheck
@@ -17,6 +17,14 @@ msg_stack_validation = "libelf is necessary for building the objtool." \
 		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
 toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
 
+chk_lzo = command -v lzop
+msg_lzo = "lzo tool not found. Please install it."
+toolcheck-$(CONFIG_KERNEL_LZO) += lzo
+
+chk_lz4 = command -v lz4c
+msg_lz4 = "lz4 tool not found. Please install it."
+toolcheck-$(CONFIG_KERNEL_LZ4) += lz4
+
 PHONY += $(toolcheck-y)
 __toolcheck: $(toolcheck-y)
 
-- 
2.7.4

Re: [PATCH 2/3] objtool: move libelf check out of top Makefile

[Josh Poimboeuf]

On Tue, Oct 16, 2018 at 06:10:52PM +0900, Masahiro Yamada wrote:
> Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
> was previously a warning, but now a error.

This behavioral change should be fine.  It was already an error for
UNWINDER_ORC, so this would only upgrade a warning to an error for
people using STACK_VALIDATION without ORC, which should be a small
number of people by this point.

> diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> index f3c165d..bc26fc0 100644
> --- a/scripts/Makefile.toolcheck
> +++ b/scripts/Makefile.toolcheck
> @@ -12,6 +12,11 @@ include include/config/auto.conf
>  __toolcheck:
>  	@:
>  
> +chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> +msg_stack_validation = "libelf is necessary for building the objtool." \
> +		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> +toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> +
>  PHONY += $(toolcheck-y)
>  __toolcheck: $(toolcheck-y)

This is a nice improvement.

It would probably be a good idea to clarify to the user which config
option(s) are the cause for the error, by putting
"CONFIG_STACK_VALIDATION" in the error string, for example.

Though, for this particular case, it would be clearer to have a
different error, based on which option is enabled, like we had before.

Like:


ifdef CONFIG_UNWINDER_ORC

chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc

else

chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation

endif


What do you think?

-- 
Josh

Re: [PATCH 1/3] kbuild: provide a new place to check necessary host tools

[Sam Ravnborg]

Hi Masahiro.

Looks good, a few nits below.

	Sam

> The top Makefile is too cluttered
So true...

> This adds a new file scripts/Makefile.toolcheck to check additional
> tools depending on the kernel configuration. This check is run
> immediately after syncconfig, i.e., when a user attempts to build
> something with a new set of CONFIG options.
> 
> [1] https://patchwork.kernel.org/patch/10516049/
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
The use of toolcheck-y should be documented in Documentation/kbuild/...


> 
>  Makefile                   |  1 +
>  scripts/Makefile.toolcheck | 35 +++++++++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
>  create mode 100644 scripts/Makefile.toolcheck
> 
> diff --git a/Makefile b/Makefile
> index bf3786e..23a204a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -633,6 +633,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ;
>  # include/config/auto.conf (which mirrors .config).
>  include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd
>  	$(Q)$(MAKE) -f $(srctree)/Makefile syncconfig
> +	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.toolcheck
>  else
>  # External modules and some install targets need include/generated/autoconf.h
>  # and include/config/auto.conf but do not care if they are up-to-date.
> diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> new file mode 100644
> index 0000000..f3c165d
> --- /dev/null
> +++ b/scripts/Makefile.toolcheck
> @@ -0,0 +1,35 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ===========================================================================
> +# Host tools check
> +# ===========================================================================
> +#
> +# Some additional tools might be required depending on the kernel configuration.
> +# Check if they are installed on the host machine, and if missing, error out
> +# with a user-friendly message.
> +
> +include include/config/auto.conf
> +
> +__toolcheck:
> +	@:
> +
> +PHONY += $(toolcheck-y)
> +__toolcheck: $(toolcheck-y)
> +
> +define populate_toolcheck
> +__toolcheck: check_$(1)
> +PHONY += check_$(1)
> +check_$(1):
> +	$(Q){ $(chk_$(1)); } >/dev/null 2>&1 || {	\
> +		echo "*" >&2;				\
> +		for msg in $(msg_$(1));			\
> +		do					\
> +			echo "* $$$${msg}" >&2;		\
> +		done;					\
> +		echo "*" >&2;				\
> +		/bin/false;				\
> +	}
Very dense, but usign if [ ... ] may result is somethign that
is a little more readable.

And maybe a litle hint on the usage...

> +$(foreach c, $(toolcheck-y), $(eval $(call populate_toolcheck,$(c))))
Good use of $(eval ...)

Re: [PATCH 2/3] objtool: move libelf check out of top Makefile

[Masahiro Yamada]

On Tue, Oct 16, 2018 at 11:25 PM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Tue, Oct 16, 2018 at 06:10:52PM +0900, Masahiro Yamada wrote:
> > Another behavioral change is, missing libelf for CONFIG_STACK_VALIDATION
> > was previously a warning, but now a error.
>
> This behavioral change should be fine.  It was already an error for
> UNWINDER_ORC, so this would only upgrade a warning to an error for
> people using STACK_VALIDATION without ORC, which should be a small
> number of people by this point.
>
> > diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> > index f3c165d..bc26fc0 100644
> > --- a/scripts/Makefile.toolcheck
> > +++ b/scripts/Makefile.toolcheck
> > @@ -12,6 +12,11 @@ include include/config/auto.conf
> >  __toolcheck:
> >       @:
> >
> > +chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > +msg_stack_validation = "libelf is necessary for building the objtool." \
> > +                    "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > +toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > +
> >  PHONY += $(toolcheck-y)
> >  __toolcheck: $(toolcheck-y)
>
> This is a nice improvement.
>
> It would probably be a good idea to clarify to the user which config
> option(s) are the cause for the error, by putting
> "CONFIG_STACK_VALIDATION" in the error string, for example.
>
> Though, for this particular case, it would be clearer to have a
> different error, based on which option is enabled, like we had before.
>
> Like:
>
>
> ifdef CONFIG_UNWINDER_ORC
>
> chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
>                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
>
> else
>
> chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
>                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
>
> endif
>
>
> What do you think?


It is ugly.

Do you need such detailed information like ORC metadata stuff here?

This Makefile aims to error out, showing why the build failed.
That's it.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] objtool: move libelf check out of top Makefile

[Josh Poimboeuf]

On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > ifdef CONFIG_UNWINDER_ORC
> >
> > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
> >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> >
> > else
> >
> > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
> >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> >
> > endif
> >
> >
> > What do you think?
> 
> 
> It is ugly.
> 
> Do you need such detailed information like ORC metadata stuff here?
> 
> This Makefile aims to error out, showing why the build failed.
> That's it.

Yeah, it is kind of ugly.  But the "showing why the build failed" part
is important.  I was trying to give the user a clear error message,
similar to what we have today.

Without context, the user won't know what objtool is, or why it needs to
be built.

If we have just a single error message for all cases, it should at least
mention the config option.  Like

   "Cannot build objtool for CONFIG_STACK_VALIDATION."

But then, most users will only have that enabled because of ORC.  So an
ORC-specific message would be more appropriate in most cases.

So maybe it can just be something more vague:

msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION.  " \
		       "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."

That would probably be good enough.  Then we could drop the ugly ifdef.

-- 
Josh

Re: [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary

[Masahiro Yamada]

On Wed, Oct 17, 2018 at 4:56 AM Borislav Petkov <bp@suse.de> wrote:
>
> On Tue, Oct 16, 2018 at 06:10:53PM +0900, Masahiro Yamada wrote:
> > If CONFIG_KERNEL_LZ4 is enabled without lz4 tool installed on the
> > system, the build fails at the very last stage (reported by
> > Borislav Petkov [1]).
> >
> >   LZO     arch/x86/boot/compressed/vmlinux.bin.lzo
> > /bin/sh: 1: lzop: not found
> > arch/x86/boot/compressed/Makefile:141: recipe for target 'arch/x86/boot/compressed/vmlinux.bin.lzo' failed
> > make[2]: *** [arch/x86/boot/compressed/vmlinux.bin.lzo] Error 1
> > arch/x86/boot/Makefile:112: recipe for target 'arch/x86/boot/compressed/vmlinux' failed
> > make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2
> > arch/x86/Makefile:284: recipe for target 'bzImage' failed
> > make: *** [bzImage] Error 2
> >
> > Check the tools in scripts/Makefile.toolcheck to fail the build
> > earlier with a more readable message.
> >
> > [1] https://patchwork.kernel.org/patch/10635381/
> >
> > Suggested-by: Borislav Petkov <bp@suse.de>
> > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > ---
> >
> >  scripts/Makefile.toolcheck | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/scripts/Makefile.toolcheck b/scripts/Makefile.toolcheck
> > index bc26fc0..5e336e4 100644
> > --- a/scripts/Makefile.toolcheck
> > +++ b/scripts/Makefile.toolcheck
> > @@ -17,6 +17,14 @@ msg_stack_validation = "libelf is necessary for building the objtool." \
> >                      "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> >  toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> >
> > +chk_lzo = command -v lzop
> > +msg_lzo = "lzo tool not found. Please install it."
> > +toolcheck-$(CONFIG_KERNEL_LZO) += lzo
> > +
> > +chk_lz4 = command -v lz4c
> > +msg_lz4 = "lz4 tool not found. Please install it."
> > +toolcheck-$(CONFIG_KERNEL_LZ4) += lz4
> > +
> >  PHONY += $(toolcheck-y)
> >  __toolcheck: $(toolcheck-y)
> >
> > --
>
> Cool, thanks for doing that!
>
> However, I applied all three, cleaned up the tree properly:
>
> $ git clean -dqfx
>
> and then used my .config (attached) which selects CONFIG_KERNEL_LZ4.
>
> And I built it twice just to make sure I'm seeing correctly but the
> error still happens at the end:
>
>   DESCEND  objtool
>   CALL    scripts/checksyscalls.sh
>   CHK     include/generated/compile.h
>   Building modules, stage 2.
>   CC      arch/x86/boot/compressed/misc.o
>   LZ4     arch/x86/boot/compressed/vmlinux.bin.lz4
> /bin/sh: 1: lz4c: not found
> make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
> make: *** [arch/x86/Makefile:290: bzImage] Error 2
> make: *** Waiting for unfinished jobs....
>   MODPOST 956 modules
>
> What am I missing?


I have no idea.

With liblz4-tool package installed on my machine,
I can build x86 kernel with CONFIG_KERNEL_LZ4.




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary

[Borislav Petkov]

On Wed, Oct 17, 2018 at 05:59:02PM +0900, Masahiro Yamada wrote:
> >   DESCEND  objtool
> >   CALL    scripts/checksyscalls.sh
> >   CHK     include/generated/compile.h
> >   Building modules, stage 2.
> >   CC      arch/x86/boot/compressed/misc.o
> >   LZ4     arch/x86/boot/compressed/vmlinux.bin.lz4
> > /bin/sh: 1: lz4c: not found
> > make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
> > make[2]: *** Waiting for unfinished jobs....
> > make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
> > make: *** [arch/x86/Makefile:290: bzImage] Error 2
> > make: *** Waiting for unfinished jobs....
> >   MODPOST 956 modules
> >
> > What am I missing?
> 
> 
> I have no idea.
> 
> With liblz4-tool package installed on my machine,
> I can build x86 kernel with CONFIG_KERNEL_LZ4.

What happens if you remove the lz4c executable? Do you see the ugly
error above or the "lz4 tool not found. Please install it." message with
my config?

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary

[Masahiro Yamada]

On Wed, Oct 17, 2018 at 6:12 PM Borislav Petkov <bp@suse.de> wrote:
>
> On Wed, Oct 17, 2018 at 05:59:02PM +0900, Masahiro Yamada wrote:
> > >   DESCEND  objtool
> > >   CALL    scripts/checksyscalls.sh
> > >   CHK     include/generated/compile.h
> > >   Building modules, stage 2.
> > >   CC      arch/x86/boot/compressed/misc.o
> > >   LZ4     arch/x86/boot/compressed/vmlinux.bin.lz4
> > > /bin/sh: 1: lz4c: not found
> > > make[2]: *** [arch/x86/boot/compressed/Makefile:143: arch/x86/boot/compressed/vmlinux.bin.lz4] Error 1
> > > make[2]: *** Waiting for unfinished jobs....
> > > make[1]: *** [arch/x86/boot/Makefile:112: arch/x86/boot/compressed/vmlinux] Error 2
> > > make: *** [arch/x86/Makefile:290: bzImage] Error 2
> > > make: *** Waiting for unfinished jobs....
> > >   MODPOST 956 modules
> > >
> > > What am I missing?
> >
> >
> > I have no idea.
> >
> > With liblz4-tool package installed on my machine,
> > I can build x86 kernel with CONFIG_KERNEL_LZ4.
>
> What happens if you remove the lz4c executable? Do you see the ugly
> error above or the "lz4 tool not found. Please install it." message with
> my config?
>

I tried that.
I can see the expected error message.



masahiro@pug:~/ref/linux$ git log  --oneline  | head -n 4
6cd6271 kbuild: check the presence of lzo and lz4 tools when necessary
b72cfb7 objtool: move libelf check out of top Makefile
e3061d8 kbuild: provide a new place to check necessary host tools
b955a91 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc
masahiro@pug:~/ref/linux$ git clean -dqfx
masahiro@pug:~/ref/linux$ cp ~/Downloads/config.lz4  .config
masahiro@pug:~/ref/linux$ sudo mv /usr/bin/lz4c /usr/bin/lz4c.backup
[sudo] password for masahiro:
masahiro@pug:~/ref/linux$ make
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  YACC    scripts/kconfig/zconf.tab.c
  LEX     scripts/kconfig/zconf.lex.c
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --syncconfig Kconfig
*
* Restart config...
*
*
* GCC plugins
*
GCC plugins (GCC_PLUGINS) [N/y/?] (NEW)
*
* lz4 tool not found. Please install it.
*
make[1]: *** [scripts/Makefile.toolcheck;46: check_lz4] Error 1
Makefile:595: include/config/auto.conf: No such file or directory
make: *** [Makefile;636: include/config/auto.conf] Error 2
make: *** Deleting file 'include/config/auto.conf'





-- 
Best Regards
Masahiro Yamada

Re: [PATCH 3/3] kbuild: check the presence of lzo and lz4 tools when necessary

[Borislav Petkov]

On Wed, Oct 17, 2018 at 06:25:23PM +0900, Masahiro Yamada wrote:
> masahiro@pug:~/ref/linux$ git log  --oneline  | head -n 4
> 6cd6271 kbuild: check the presence of lzo and lz4 tools when necessary
> b72cfb7 objtool: move libelf check out of top Makefile
> e3061d8 kbuild: provide a new place to check necessary host tools
> b955a91 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc
> masahiro@pug:~/ref/linux$ git clean -dqfx
> masahiro@pug:~/ref/linux$ cp ~/Downloads/config.lz4  .config
> masahiro@pug:~/ref/linux$ sudo mv /usr/bin/lz4c /usr/bin/lz4c.backup
> [sudo] password for masahiro:
> masahiro@pug:~/ref/linux$ make
>   HOSTCC  scripts/basic/fixdep
>   HOSTCC  scripts/kconfig/conf.o
>   YACC    scripts/kconfig/zconf.tab.c
>   LEX     scripts/kconfig/zconf.lex.c
>   HOSTCC  scripts/kconfig/zconf.tab.o
>   HOSTLD  scripts/kconfig/conf
> scripts/kconfig/conf  --syncconfig Kconfig
> *
> * Restart config...
> *
> *
> * GCC plugins
> *
> GCC plugins (GCC_PLUGINS) [N/y/?] (NEW)
> *
> * lz4 tool not found. Please install it.
> *
> make[1]: *** [scripts/Makefile.toolcheck;46: check_lz4] Error 1
> Makefile:595: include/config/auto.conf: No such file or directory
> make: *** [Makefile;636: include/config/auto.conf] Error 2
> make: *** Deleting file 'include/config/auto.conf'

Ok, gotcha. The difference is the "make oldconfig" I do.

[boris@zn: ~/kernel/linux> git log  --oneline  | head -n 3
57ac85467970 kbuild: check the presence of lzo and lz4 tools when necessary
92a3b44ab988 objtool: move libelf check out of top Makefile
23b142c064a7 kbuild: provide a new place to check necessary host tools
[boris@zn: ~/kernel/linux> git clean -dqfx
[boris@zn: ~/kernel/linux> cp ~/tmp/config.lz4 .config
[boris@zn: ~/kernel/linux> make oldconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  YACC    scripts/kconfig/zconf.tab.c
  LEX     scripts/kconfig/zconf.lex.c
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
scripts/kconfig/conf  --oldconfig Kconfig
#
# configuration written to .config
#
[boris@zn: ~/kernel/linux> make
  SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_32_ia32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_64_x32.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_64.h
  HYPERCALLS arch/x86/include/generated/asm/xen-hypercalls.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
  HOSTCC  arch/x86/tools/relocs_32.o
  HOSTCC  arch/x86/tools/relocs_64.o
  HOSTCC  arch/x86/tools/relocs_common.o
  HOSTLD  arch/x86/tools/relocs
...

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

Re: [PATCH 2/3] objtool: move libelf check out of top Makefile

[Masahiro Yamada]

Hi Josh,


On Wed, Oct 17, 2018 at 1:16 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
>
> On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > > ifdef CONFIG_UNWINDER_ORC
> > >
> > > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
> > >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> > >
> > > else
> > >
> > > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
> > >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > >
> > > endif
> > >
> > >
> > > What do you think?
> >
> >
> > It is ugly.
> >
> > Do you need such detailed information like ORC metadata stuff here?
> >
> > This Makefile aims to error out, showing why the build failed.
> > That's it.
>
> Yeah, it is kind of ugly.  But the "showing why the build failed" part
> is important.  I was trying to give the user a clear error message,
> similar to what we have today.
>
> Without context, the user won't know what objtool is, or why it needs to
> be built.
>
> If we have just a single error message for all cases, it should at least
> mention the config option.  Like
>
>    "Cannot build objtool for CONFIG_STACK_VALIDATION."
>
> But then, most users will only have that enabled because of ORC.  So an
> ORC-specific message would be more appropriate in most cases.
>
> So maybe it can just be something more vague:
>
> msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION.  " \
>                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
>
> That would probably be good enough.  Then we could drop the ugly ifdef.


Fair point, but I am confused by the current
STACK_VALIDATION / UNWINDER_ORC logic.

In my understanding, objtool is
an all-in-one object check/manipulation tool.

STACK_VALIDATION and UNWINDER_ORC
is a selection of a sub-command, 'check' or 'orc generate'.

(Correct me if am wrong.)


However, STACK_VALIDATION is still used to
decide whether or not to compile the objtool.


Adding a new symbol OBJTOOL would clarify the logic.



config OBJTOOL
        bool

config STACK_VALIDATION
        bool "Compile-time stack metadata validation"
        depends on HAVE_STACK_VALIDATION
        select OBJTOOL
        ...


config UNWINDER_ORC
        bool "ORC unwinder"
        depends on X86_64
        select OBJTOOL
        ...




-- 
Best Regards
Masahiro Yamada

Re: [PATCH 2/3] objtool: move libelf check out of top Makefile

[Josh Poimboeuf]

On Fri, Oct 19, 2018 at 03:04:22PM +0900, Masahiro Yamada wrote:
> Hi Josh,
> 
> 
> On Wed, Oct 17, 2018 at 1:16 AM Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> >
> > On Wed, Oct 17, 2018 at 12:51:40AM +0900, Masahiro Yamada wrote:
> > > > ifdef CONFIG_UNWINDER_ORC
> > > >
> > > > chk_unwinder_orc = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > > msg_unwinder_orc = "Cannot build objtool to generate ORC metadata for CONFIG_UNWINDER_ORC=y.  " \
> > > >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > > toolcheck-$(CONFIG_UNWINDER_ORC) += unwinder_orc
> > > >
> > > > else
> > > >
> > > > chk_stack_validation = echo "int main() {}" | $(HOSTCC) -xc -o /dev/null -lelf -
> > > > msg_stack_validation = "Cannot build objtool for CONFIG_STACK_VALIDATION=y.  " \
> > > >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> > > > toolcheck-$(CONFIG_STACK_VALIDATION) += stack_validation
> > > >
> > > > endif
> > > >
> > > >
> > > > What do you think?
> > >
> > >
> > > It is ugly.
> > >
> > > Do you need such detailed information like ORC metadata stuff here?
> > >
> > > This Makefile aims to error out, showing why the build failed.
> > > That's it.
> >
> > Yeah, it is kind of ugly.  But the "showing why the build failed" part
> > is important.  I was trying to give the user a clear error message,
> > similar to what we have today.
> >
> > Without context, the user won't know what objtool is, or why it needs to
> > be built.
> >
> > If we have just a single error message for all cases, it should at least
> > mention the config option.  Like
> >
> >    "Cannot build objtool for CONFIG_STACK_VALIDATION."
> >
> > But then, most users will only have that enabled because of ORC.  So an
> > ORC-specific message would be more appropriate in most cases.
> >
> > So maybe it can just be something more vague:
> >
> > msg_stack_validation = "Cannot build objtool for CONFIG_UNWINDER_ORC and/or CONFIG_STACK_VALIDATION.  " \
> >                        "Please install libelf-dev, libelf-devel or elfutils-libelf-devel."
> >
> > That would probably be good enough.  Then we could drop the ugly ifdef.
> 
> 
> Fair point, but I am confused by the current
> STACK_VALIDATION / UNWINDER_ORC logic.
> 
> In my understanding, objtool is
> an all-in-one object check/manipulation tool.
> 
> STACK_VALIDATION and UNWINDER_ORC
> is a selection of a sub-command, 'check' or 'orc generate'.
> 
> (Correct me if am wrong.)
> 
> 
> However, STACK_VALIDATION is still used to
> decide whether or not to compile the objtool.
> 
> 
> Adding a new symbol OBJTOOL would clarify the logic.
> 
> 
> 
> config OBJTOOL
>         bool
> 
> config STACK_VALIDATION
>         bool "Compile-time stack metadata validation"
>         depends on HAVE_STACK_VALIDATION
>         select OBJTOOL
>         ...
> 
> 
> config UNWINDER_ORC
>         bool "ORC unwinder"
>         depends on X86_64
>         select OBJTOOL
>         ...

While 'orc generate' and 'check' are indeed separate subcommands of
objtool, the functionality of 'orc generate' is actually a superset of
the functionality of 'check'.  In other words, ORC generation relies on
the stack validation feature, which is consistent with the current
config logic.

-- 
Josh