kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 2/2] tools: do not include scripts/Kbuild.include
       [not found] <20210415072700.147125-1-masahiroy@kernel.org>
@ 2021-04-15  7:27 ` Masahiro Yamada
  2021-04-15  7:40   ` Paolo Bonzini
  2021-04-15  8:06   ` Christian Borntraeger
  0 siblings, 2 replies; 7+ messages in thread
From: Masahiro Yamada @ 2021-04-15  7:27 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Janosch Frank, Christian Borntraeger, Masahiro Yamada,
	Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt,
	Daniel Borkmann, Harish, John Fastabend, KP Singh,
	Martin KaFai Lau, Michael Ellerman, Nathan Chancellor,
	Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan,
	Song Liu, Yonghong Song, bpf, clang-built-linux, kvm,
	linux-kernel, linux-kselftest, linuxppc-dev, netdev

Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
scripts/Makefile.compiler"), some kselftests fail to build.

The tools/ directory opted out Kbuild, and went in a different
direction. They copy any kind of files to the tools/ directory
in order to do whatever they want to do in their world.

tools/build/Build.include mimics scripts/Kbuild.include, but some
tool Makefiles included the Kbuild one to import a feature that is
missing in tools/build/Build.include:

 - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
   only if supported") included scripts/Kbuild.include from
   tools/thermal/tmon/Makefile to import the cc-option macro.

 - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
   not support -no-pie") included scripts/Kbuild.include from
   tools/testing/selftests/kvm/Makefile to import the try-run macro.

 - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
   failures") included scripts/Kbuild.include from
   tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
   target.

 - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
   unrecognized option") included scripts/Kbuild.include from
   tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
   try-run macro.

Copy what they want there, and stop including scripts/Kbuild.include
from the tool Makefiles.

Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
Reported-by: Janosch Frank <frankja@linux.ibm.com>
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 tools/testing/selftests/bpf/Makefile          |  3 ++-
 tools/testing/selftests/kvm/Makefile          | 12 +++++++++++-
 .../selftests/powerpc/pmu/ebb/Makefile        | 11 ++++++++++-
 tools/thermal/tmon/Makefile                   | 19 +++++++++++++++++--
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 044bfdcf5b74..d872b9f41543 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -1,5 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-include ../../../../scripts/Kbuild.include
 include ../../../scripts/Makefile.arch
 include ../../../scripts/Makefile.include
 
@@ -476,3 +475,5 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
 	feature								\
 	$(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc bpf_testmod.ko)
+
+.DELETE_ON_ERROR:
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..8b45bc417d83 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,15 @@
 # SPDX-License-Identifier: GPL-2.0-only
-include ../../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_$$$$
+
+try-run = $(shell set -e;		\
+	TMP=$(TMPOUT)/tmp;		\
+	mkdir -p $(TMPOUT);		\
+	trap "rm -rf $(TMPOUT)" EXIT;	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi)
 
 all:
 
diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
index af3df79d8163..d5d3e869df93 100644
--- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
+++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
@@ -1,5 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
-include ../../../../../../scripts/Kbuild.include
 
 noarg:
 	$(MAKE) -C ../../
@@ -8,6 +7,16 @@ noarg:
 CFLAGS += -m64
 
 TMPOUT = $(OUTPUT)/TMPDIR/
+
+try-run = $(shell set -e;		\
+	TMP=$(TMPOUT)/tmp;		\
+	mkdir -p $(TMPOUT);		\
+	trap "rm -rf $(TMPOUT)" EXIT;	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi)
+
 # Toolchains may build PIE by default which breaks the assembly
 no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
         $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
index 59e417ec3e13..92a683e4866c 100644
--- a/tools/thermal/tmon/Makefile
+++ b/tools/thermal/tmon/Makefile
@@ -1,6 +1,21 @@
 # SPDX-License-Identifier: GPL-2.0
-# We need this for the "cc-option" macro.
-include ../../../scripts/Kbuild.include
+
+TMPOUT = .tmp_$$$$
+
+try-run = $(shell set -e;		\
+	TMP=$(TMPOUT)/tmp;		\
+	mkdir -p $(TMPOUT);		\
+	trap "rm -rf $(TMPOUT)" EXIT;	\
+	if ($(1)) >/dev/null 2>&1;	\
+	then echo "$(2)";		\
+	else echo "$(3)";		\
+	fi)
+
+__cc-option = $(call try-run,\
+	$(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
+
+cc-option = $(call __cc-option, $(CC),\
+	$(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
 
 VERSION = 1.0
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-15  7:27 ` [PATCH 2/2] tools: do not include scripts/Kbuild.include Masahiro Yamada
@ 2021-04-15  7:40   ` Paolo Bonzini
  2021-04-15  8:04     ` Masahiro Yamada
  2021-04-15  8:06   ` Christian Borntraeger
  1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2021-04-15  7:40 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: Janosch Frank, Christian Borntraeger, Alexei Starovoitov,
	Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish,
	John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman,
	Nathan Chancellor, Nick Desaulniers, Paul Mackerras, Shuah Khan,
	Song Liu, Yonghong Song, bpf, clang-built-linux, kvm,
	linux-kernel, linux-kselftest, linuxppc-dev, netdev

On 15/04/21 09:27, Masahiro Yamada wrote:
> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> scripts/Makefile.compiler"), some kselftests fail to build.
> 
> The tools/ directory opted out Kbuild, and went in a different
> direction. They copy any kind of files to the tools/ directory
> in order to do whatever they want to do in their world.
> 
> tools/build/Build.include mimics scripts/Kbuild.include, but some
> tool Makefiles included the Kbuild one to import a feature that is
> missing in tools/build/Build.include:
> 
>   - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
>     only if supported") included scripts/Kbuild.include from
>     tools/thermal/tmon/Makefile to import the cc-option macro.
> 
>   - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
>     not support -no-pie") included scripts/Kbuild.include from
>     tools/testing/selftests/kvm/Makefile to import the try-run macro.
> 
>   - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
>     failures") included scripts/Kbuild.include from
>     tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
>     target.
> 
>   - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
>     unrecognized option") included scripts/Kbuild.include from
>     tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
>     try-run macro.
> 
> Copy what they want there, and stop including scripts/Kbuild.include
> from the tool Makefiles.

I think it would make sense to add try-run, cc-option and 
.DELETE_ON_ERROR to tools/build/Build.include?

Paolo

> Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> Reported-by: Janosch Frank <frankja@linux.ibm.com>
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>   tools/testing/selftests/bpf/Makefile          |  3 ++-
>   tools/testing/selftests/kvm/Makefile          | 12 +++++++++++-
>   .../selftests/powerpc/pmu/ebb/Makefile        | 11 ++++++++++-
>   tools/thermal/tmon/Makefile                   | 19 +++++++++++++++++--
>   4 files changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 044bfdcf5b74..d872b9f41543 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -1,5 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0
> -include ../../../../scripts/Kbuild.include
>   include ../../../scripts/Makefile.arch
>   include ../../../scripts/Makefile.include
>   
> @@ -476,3 +475,5 @@ EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
>   	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
>   	feature								\
>   	$(addprefix $(OUTPUT)/,*.o *.skel.h no_alu32 bpf_gcc bpf_testmod.ko)
> +
> +.DELETE_ON_ERROR:
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..8b45bc417d83 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -1,5 +1,15 @@
>   # SPDX-License-Identifier: GPL-2.0-only
> -include ../../../../scripts/Kbuild.include
> +
> +TMPOUT = .tmp_$$$$
> +
> +try-run = $(shell set -e;		\
> +	TMP=$(TMPOUT)/tmp;		\
> +	mkdir -p $(TMPOUT);		\
> +	trap "rm -rf $(TMPOUT)" EXIT;	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi)
>   
>   all:
>   
> diff --git a/tools/testing/selftests/powerpc/pmu/ebb/Makefile b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
> index af3df79d8163..d5d3e869df93 100644
> --- a/tools/testing/selftests/powerpc/pmu/ebb/Makefile
> +++ b/tools/testing/selftests/powerpc/pmu/ebb/Makefile
> @@ -1,5 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0
> -include ../../../../../../scripts/Kbuild.include
>   
>   noarg:
>   	$(MAKE) -C ../../
> @@ -8,6 +7,16 @@ noarg:
>   CFLAGS += -m64
>   
>   TMPOUT = $(OUTPUT)/TMPDIR/
> +
> +try-run = $(shell set -e;		\
> +	TMP=$(TMPOUT)/tmp;		\
> +	mkdir -p $(TMPOUT);		\
> +	trap "rm -rf $(TMPOUT)" EXIT;	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi)
> +
>   # Toolchains may build PIE by default which breaks the assembly
>   no-pie-option := $(call try-run, echo 'int main() { return 0; }' | \
>           $(CC) -Werror $(KBUILD_CPPFLAGS) $(CC_OPTION_CFLAGS) -no-pie -x c - -o "$$TMP", -no-pie)
> diff --git a/tools/thermal/tmon/Makefile b/tools/thermal/tmon/Makefile
> index 59e417ec3e13..92a683e4866c 100644
> --- a/tools/thermal/tmon/Makefile
> +++ b/tools/thermal/tmon/Makefile
> @@ -1,6 +1,21 @@
>   # SPDX-License-Identifier: GPL-2.0
> -# We need this for the "cc-option" macro.
> -include ../../../scripts/Kbuild.include
> +
> +TMPOUT = .tmp_$$$$
> +
> +try-run = $(shell set -e;		\
> +	TMP=$(TMPOUT)/tmp;		\
> +	mkdir -p $(TMPOUT);		\
> +	trap "rm -rf $(TMPOUT)" EXIT;	\
> +	if ($(1)) >/dev/null 2>&1;	\
> +	then echo "$(2)";		\
> +	else echo "$(3)";		\
> +	fi)
> +
> +__cc-option = $(call try-run,\
> +	$(1) -Werror $(2) $(3) -c -x c /dev/null -o "$$TMP",$(3),$(4))
> +
> +cc-option = $(call __cc-option, $(CC),\
> +	$(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS),$(1),$(2))
>   
>   VERSION = 1.0
>   
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-15  7:40   ` Paolo Bonzini
@ 2021-04-15  8:04     ` Masahiro Yamada
  2021-04-15  8:25       ` Paolo Bonzini
  0 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2021-04-15  8:04 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Linux Kbuild mailing list, Janosch Frank, Christian Borntraeger,
	Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt,
	Daniel Borkmann, Harish, John Fastabend, KP Singh,
	Martin KaFai Lau, Michael Ellerman, Nathan Chancellor,
	Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu,
	Yonghong Song, bpf, clang-built-linux, kvm,
	Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
	linuxppc-dev, Networking

On Thu, Apr 15, 2021 at 4:40 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 15/04/21 09:27, Masahiro Yamada wrote:
> > Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> > scripts/Makefile.compiler"), some kselftests fail to build.
> >
> > The tools/ directory opted out Kbuild, and went in a different
> > direction. They copy any kind of files to the tools/ directory
> > in order to do whatever they want to do in their world.
> >
> > tools/build/Build.include mimics scripts/Kbuild.include, but some
> > tool Makefiles included the Kbuild one to import a feature that is
> > missing in tools/build/Build.include:
> >
> >   - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
> >     only if supported") included scripts/Kbuild.include from
> >     tools/thermal/tmon/Makefile to import the cc-option macro.
> >
> >   - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
> >     not support -no-pie") included scripts/Kbuild.include from
> >     tools/testing/selftests/kvm/Makefile to import the try-run macro.
> >
> >   - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
> >     failures") included scripts/Kbuild.include from
> >     tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
> >     target.
> >
> >   - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
> >     unrecognized option") included scripts/Kbuild.include from
> >     tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
> >     try-run macro.
> >
> > Copy what they want there, and stop including scripts/Kbuild.include
> > from the tool Makefiles.
>
> I think it would make sense to add try-run, cc-option and
> .DELETE_ON_ERROR to tools/build/Build.include?


To be safe, I just copy-pasted what the makefiles need.
If someone wants to refactor the tool build system, that is fine,
but, to me, I do not see consistent rules or policy under tools/.

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-15  7:27 ` [PATCH 2/2] tools: do not include scripts/Kbuild.include Masahiro Yamada
  2021-04-15  7:40   ` Paolo Bonzini
@ 2021-04-15  8:06   ` Christian Borntraeger
  2021-04-16  5:56     ` Christian Borntraeger
  1 sibling, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2021-04-15  8:06 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: Janosch Frank, Alexei Starovoitov, Andrii Nakryiko,
	Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend,
	KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor,
	Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan,
	Song Liu, Yonghong Song, bpf, clang-built-linux, kvm,
	linux-kernel, linux-kselftest, linuxppc-dev, netdev


On 15.04.21 09:27, Masahiro Yamada wrote:
> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> scripts/Makefile.compiler"), some kselftests fail to build.
> 
> The tools/ directory opted out Kbuild, and went in a different
> direction. They copy any kind of files to the tools/ directory
> in order to do whatever they want to do in their world.
> 
> tools/build/Build.include mimics scripts/Kbuild.include, but some
> tool Makefiles included the Kbuild one to import a feature that is
> missing in tools/build/Build.include:
> 
>   - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
>     only if supported") included scripts/Kbuild.include from
>     tools/thermal/tmon/Makefile to import the cc-option macro.
> 
>   - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
>     not support -no-pie") included scripts/Kbuild.include from
>     tools/testing/selftests/kvm/Makefile to import the try-run macro.
> 
>   - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
>     failures") included scripts/Kbuild.include from
>     tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
>     target.
> 
>   - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
>     unrecognized option") included scripts/Kbuild.include from
>     tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
>     try-run macro.
> 
> Copy what they want there, and stop including scripts/Kbuild.include
> from the tool Makefiles.
> 
> Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> Reported-by: Janosch Frank <frankja@linux.ibm.com>
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

When applying this on top of d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")

I still do get

# ==== Test Assertion Failure ====
#   lib/kvm_util.c:142: vm->fd >= 0
#   pid=315635 tid=315635 - Invalid argument
#      1	0x0000000001002f4b: vm_open at kvm_util.c:142
#      2	 (inlined by) vm_create at kvm_util.c:258
#      3	0x00000000010015ef: test_add_max_memory_regions at set_memory_region_test.c:351
#      4	 (inlined by) main at set_memory_region_test.c:397
#      5	0x000003ff971abb89: ?? ??:0
#      6	0x00000000010017ad: .annobin_abi_note.c.hot at crt1.o:?
#   KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
not ok 7 selftests: kvm: set_memory_region_test # exit=254

and the testcase compilation does not pickup the pgste option.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-15  8:04     ` Masahiro Yamada
@ 2021-04-15  8:25       ` Paolo Bonzini
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Bonzini @ 2021-04-15  8:25 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Linux Kbuild mailing list, Janosch Frank, Christian Borntraeger,
	Alexei Starovoitov, Andrii Nakryiko, Benjamin Herrenschmidt,
	Daniel Borkmann, Harish, John Fastabend, KP Singh,
	Martin KaFai Lau, Michael Ellerman, Nathan Chancellor,
	Nick Desaulniers, Paul Mackerras, Shuah Khan, Song Liu,
	Yonghong Song, bpf, clang-built-linux, kvm,
	Linux Kernel Mailing List, open list:KERNEL SELFTEST FRAMEWORK,
	linuxppc-dev, Networking

On 15/04/21 10:04, Masahiro Yamada wrote:
> On Thu, Apr 15, 2021 at 4:40 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>> I think it would make sense to add try-run, cc-option and
>> .DELETE_ON_ERROR to tools/build/Build.include?
> 
> To be safe, I just copy-pasted what the makefiles need.
> If someone wants to refactor the tool build system, that is fine,
> but, to me, I do not see consistent rules or policy under tools/.

"Please put this in a common file instead of introducing duplication" is 
not asking for wholesale refactoring.

Paolo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-15  8:06   ` Christian Borntraeger
@ 2021-04-16  5:56     ` Christian Borntraeger
  2021-04-16 12:53       ` Masahiro Yamada
  0 siblings, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2021-04-16  5:56 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: Janosch Frank, Alexei Starovoitov, Andrii Nakryiko,
	Benjamin Herrenschmidt, Daniel Borkmann, Harish, John Fastabend,
	KP Singh, Martin KaFai Lau, Michael Ellerman, Nathan Chancellor,
	Nick Desaulniers, Paolo Bonzini, Paul Mackerras, Shuah Khan,
	Song Liu, Yonghong Song, bpf, clang-built-linux, kvm,
	linux-kernel, linux-kselftest, linuxppc-dev, netdev


On 15.04.21 10:06, Christian Borntraeger wrote:
> 
> On 15.04.21 09:27, Masahiro Yamada wrote:
>> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
>> scripts/Makefile.compiler"), some kselftests fail to build.
>>
>> The tools/ directory opted out Kbuild, and went in a different
>> direction. They copy any kind of files to the tools/ directory
>> in order to do whatever they want to do in their world.
>>
>> tools/build/Build.include mimics scripts/Kbuild.include, but some
>> tool Makefiles included the Kbuild one to import a feature that is
>> missing in tools/build/Build.include:
>>
>>   - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
>>     only if supported") included scripts/Kbuild.include from
>>     tools/thermal/tmon/Makefile to import the cc-option macro.
>>
>>   - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
>>     not support -no-pie") included scripts/Kbuild.include from
>>     tools/testing/selftests/kvm/Makefile to import the try-run macro.
>>
>>   - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
>>     failures") included scripts/Kbuild.include from
>>     tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
>>     target.
>>
>>   - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
>>     unrecognized option") included scripts/Kbuild.include from
>>     tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
>>     try-run macro.
>>
>> Copy what they want there, and stop including scripts/Kbuild.include
>> from the tool Makefiles.
>>
>> Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
>> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
>> Reported-by: Janosch Frank <frankja@linux.ibm.com>
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> 
> When applying this on top of d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> 
> I still do get
> 
> # ==== Test Assertion Failure ====
> #   lib/kvm_util.c:142: vm->fd >= 0
> #   pid=315635 tid=315635 - Invalid argument
> #      1    0x0000000001002f4b: vm_open at kvm_util.c:142
> #      2     (inlined by) vm_create at kvm_util.c:258
> #      3    0x00000000010015ef: test_add_max_memory_regions at set_memory_region_test.c:351
> #      4     (inlined by) main at set_memory_region_test.c:397
> #      5    0x000003ff971abb89: ?? ??:0
> #      6    0x00000000010017ad: .annobin_abi_note.c.hot at crt1.o:?
> #   KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
> not ok 7 selftests: kvm: set_memory_region_test # exit=254
> 
> and the testcase compilation does not pickup the pgste option.


What does work is the following:
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index a6d61f451f88..d9c6d9c2069e 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -1,5 +1,6 @@
  # SPDX-License-Identifier: GPL-2.0-only
  include ../../../../scripts/Kbuild.include
+include ../../../../scripts/Makefile.compiler
  
  all:
  

as it does pickup the linker option handling.



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] tools: do not include scripts/Kbuild.include
  2021-04-16  5:56     ` Christian Borntraeger
@ 2021-04-16 12:53       ` Masahiro Yamada
  0 siblings, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2021-04-16 12:53 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Linux Kbuild mailing list, Janosch Frank, Alexei Starovoitov,
	Andrii Nakryiko, Benjamin Herrenschmidt, Daniel Borkmann, Harish,
	John Fastabend, KP Singh, Martin KaFai Lau, Michael Ellerman,
	Nathan Chancellor, Nick Desaulniers, Paolo Bonzini,
	Paul Mackerras, Shuah Khan, Song Liu, Yonghong Song, bpf,
	clang-built-linux, kvm, Linux Kernel Mailing List,
	open list:KERNEL SELFTEST FRAMEWORK, linuxppc-dev, Networking

On Fri, Apr 16, 2021 at 2:56 PM Christian Borntraeger
<borntraeger@de.ibm.com> wrote:
>
>
> On 15.04.21 10:06, Christian Borntraeger wrote:
> >
> > On 15.04.21 09:27, Masahiro Yamada wrote:
> >> Since commit d9f4ff50d2aa ("kbuild: spilt cc-option and friends to
> >> scripts/Makefile.compiler"), some kselftests fail to build.
> >>
> >> The tools/ directory opted out Kbuild, and went in a different
> >> direction. They copy any kind of files to the tools/ directory
> >> in order to do whatever they want to do in their world.
> >>
> >> tools/build/Build.include mimics scripts/Kbuild.include, but some
> >> tool Makefiles included the Kbuild one to import a feature that is
> >> missing in tools/build/Build.include:
> >>
> >>   - Commit ec04aa3ae87b ("tools/thermal: tmon: use "-fstack-protector"
> >>     only if supported") included scripts/Kbuild.include from
> >>     tools/thermal/tmon/Makefile to import the cc-option macro.
> >>
> >>   - Commit c2390f16fc5b ("selftests: kvm: fix for compilers that do
> >>     not support -no-pie") included scripts/Kbuild.include from
> >>     tools/testing/selftests/kvm/Makefile to import the try-run macro.
> >>
> >>   - Commit 9cae4ace80ef ("selftests/bpf: do not ignore clang
> >>     failures") included scripts/Kbuild.include from
> >>     tools/testing/selftests/bpf/Makefile to import the .DELETE_ON_ERROR
> >>     target.
> >>
> >>   - Commit 0695f8bca93e ("selftests/powerpc: Handle Makefile for
> >>     unrecognized option") included scripts/Kbuild.include from
> >>     tools/testing/selftests/powerpc/pmu/ebb/Makefile to import the
> >>     try-run macro.
> >>
> >> Copy what they want there, and stop including scripts/Kbuild.include
> >> from the tool Makefiles.
> >>
> >> Link: https://lore.kernel.org/lkml/86dadf33-70f7-a5ac-cb8c-64966d2f45a1@linux.ibm.com/
> >> Fixes: d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> >> Reported-by: Janosch Frank <frankja@linux.ibm.com>
> >> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> >
> > When applying this on top of d9f4ff50d2aa ("kbuild: spilt cc-option and friends to scripts/Makefile.compiler")
> >
> > I still do get
> >
> > # ==== Test Assertion Failure ====
> > #   lib/kvm_util.c:142: vm->fd >= 0
> > #   pid=315635 tid=315635 - Invalid argument
> > #      1    0x0000000001002f4b: vm_open at kvm_util.c:142
> > #      2     (inlined by) vm_create at kvm_util.c:258
> > #      3    0x00000000010015ef: test_add_max_memory_regions at set_memory_region_test.c:351
> > #      4     (inlined by) main at set_memory_region_test.c:397
> > #      5    0x000003ff971abb89: ?? ??:0
> > #      6    0x00000000010017ad: .annobin_abi_note.c.hot at crt1.o:?
> > #   KVM_CREATE_VM ioctl failed, rc: -1 errno: 22
> > not ok 7 selftests: kvm: set_memory_region_test # exit=254
> >
> > and the testcase compilation does not pickup the pgste option.
>
> What does work is the following:
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index a6d61f451f88..d9c6d9c2069e 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -1,5 +1,6 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   include ../../../../scripts/Kbuild.include
> +include ../../../../scripts/Makefile.compiler
>
>   all:
>
>
> as it does pickup the linker option handling.


Kbuild and the tools are divorced.

They cannot be married unless the tools/
build system is largely refactored.
That will be a tons of works (and
I am not sure if it is welcome).

The Kbuild refactoring should not be bothered by
the tools.
For now, I want them separated from each other.



--
Best Regards

Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-16 12:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210415072700.147125-1-masahiroy@kernel.org>
2021-04-15  7:27 ` [PATCH 2/2] tools: do not include scripts/Kbuild.include Masahiro Yamada
2021-04-15  7:40   ` Paolo Bonzini
2021-04-15  8:04     ` Masahiro Yamada
2021-04-15  8:25       ` Paolo Bonzini
2021-04-15  8:06   ` Christian Borntraeger
2021-04-16  5:56     ` Christian Borntraeger
2021-04-16 12:53       ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).