Re: [PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
Hi Masahiro,
On Tue, Aug 21, 2018 at 11:49:48AM +0900, Masahiro Yamada wrote:
> The code diff looks good to me.
>
> Reviewed-by: Masahiro Yamada
Thanks :)
> > A straightforward approach to the per-arch header is to make use of
> > asm-generic to provide a default empty header & adjust architectures
> > which don't need anything specific to make use of that by adding the
> > header to generic-y. Unfortunately this doesn't work so well due to
> > commit a95b37e20db9 ("kbuild: get out of
> > ") which moved the inclusion of linux/compiler.h to
> > cflags using the -include compiler flag.
>
> I doubt this statement.
>
> Commit a95b37e20db9 is not the cause of the problem.
>
>%
>
> The change happened in commit 28128c61e08e.
You're correct - I'll fix that up.
> One more thing, you are not touching any makefile in this version.
>
> Maybe, you can prefix the subject with "compiler.h:" or something
> instead of "kbuild:".
Will do.
Thanks,
Paul
Re: [PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
Hi Masahiro,
On Tue, Aug 21, 2018 at 11:49:48AM +0900, Masahiro Yamada wrote:
> The code diff looks good to me.
>
> Reviewed-by: Masahiro Yamada
Thanks :)
> > A straightforward approach to the per-arch header is to make use of
> > asm-generic to provide a default empty header & adjust architectures
> > which don't need anything specific to make use of that by adding the
> > header to generic-y. Unfortunately this doesn't work so well due to
> > commit a95b37e20db9 ("kbuild: get out of
> > ") which moved the inclusion of linux/compiler.h to
> > cflags using the -include compiler flag.
>
> I doubt this statement.
>
> Commit a95b37e20db9 is not the cause of the problem.
>
>%
>
> The change happened in commit 28128c61e08e.
You're correct - I'll fix that up.
> One more thing, you are not touching any makefile in this version.
>
> Maybe, you can prefix the subject with "compiler.h:" or something
> instead of "kbuild:".
Will do.
Thanks,
Paul
Re: [PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
Hi Paul,
The code diff looks good to me.
Reviewed-by: Masahiro Yamada
Just comments in the commit description. See below.
2018-08-21 7:36 GMT+09:00 Paul Burton :
> We have a need to override the definition of
> barrier_before_unreachable() for MIPS, which means we either need to add
> architecture-specific code into linux/compiler-gcc.h or we need to allow
> the architecture to provide a header that can define the macro before
> the generic definition. The latter seems like the better approach.
>
> A straightforward approach to the per-arch header is to make use of
> asm-generic to provide a default empty header & adjust architectures
> which don't need anything specific to make use of that by adding the
> header to generic-y. Unfortunately this doesn't work so well due to
> commit a95b37e20db9 ("kbuild: get out of
> ") which moved the inclusion of linux/compiler.h to
> cflags using the -include compiler flag.
I doubt this statement.
Commit a95b37e20db9 is not the cause of the problem.
include/linux/kconfig.h is also included
by using the -include compiler flag.
See the top-level Makefile.
USERINCLUDE:= \
-I$(srctree)/arch/$(SRCARCH)/include/uapi \
-I$(objtree)/arch/$(SRCARCH)/include/generated/uapi \
-I$(srctree)/include/uapi \
-I$(objtree)/include/generated/uapi \
-include $(srctree)/include/linux/kconfig.h
So, (then, )
would be also required for all C files
in the archprepare stage regardless of commit a95b37e20db9.
The change happened in commit 28128c61e08e.
One more thing, you are not touching any makefile in this version.
Maybe, you can prefix the subject with "compiler.h:" or something
instead of "kbuild:".
> Because the -include flag is present for all C files we compile, we need
> the architecture-provided header to be present before any C files are
> compiled. If any C files can be compiled prior to the asm-generic header
> wrappers being generated then we hit a build failure due to missing
> header. Such cases do exist - one pointed out by the kbuild test robot
> is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part
> of the archprepare target [1].
>
> This leaves us with a few options:
>
> 1) Use generic-y & fix any build failures we find by enforcing
> ordering such that the asm-generic target occurs before any C
> compilation, such that linux/compiler_types.h can always include
> the generated asm-generic wrapper which in turn includes the empty
> asm-generic header. This would rely on us finding all the
> problematic cases - I don't know for sure that the ia64 issue is
> the only one.
>
> 2) Add an actual empty header to each architecture, so that we don't
> need the generated asm-generic wrapper. This seems messy.
>
> 3) Give up & add #ifdef CONFIG_MIPS or similar to
> linux/compiler_types.h. This seems messy too.
>
> 4) Include the arch header only when it's actually needed, removing
> the need for the asm-generic wrapper for all other architectures.
>
> This patch allows us to use approach 4, by including an asm/compiler.h
> header from linux/compiler_types.h after the inclusion of the
> compiler-specific linux/compiler-*.h header(s). We do this
> conditionally, only when CONFIG_HAVE_ARCH_COMPILER_H is selected, in
> order to avoid the need for asm-generic wrappers & the associated build
> ordering issue described above. The asm/compiler.h header is included
> after the generic linux/compiler-*.h header(s) for consistency with the
> way linux/compiler-intel.h & linux/compiler-clang.h are included after
> the linux/compiler-gcc.h header that they override.
>
> [1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html
>
> Signed-off-by: Paul Burton
> Cc: Arnd Bergmann
> Cc: James Hogan
> Cc: Masahiro Yamada
> Cc: Ralf Baechle
> Cc: linux-a...@vger.kernel.org
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-m...@linux-mips.org
Re: [PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
Hi Paul,
The code diff looks good to me.
Reviewed-by: Masahiro Yamada
Just comments in the commit description. See below.
2018-08-21 7:36 GMT+09:00 Paul Burton :
> We have a need to override the definition of
> barrier_before_unreachable() for MIPS, which means we either need to add
> architecture-specific code into linux/compiler-gcc.h or we need to allow
> the architecture to provide a header that can define the macro before
> the generic definition. The latter seems like the better approach.
>
> A straightforward approach to the per-arch header is to make use of
> asm-generic to provide a default empty header & adjust architectures
> which don't need anything specific to make use of that by adding the
> header to generic-y. Unfortunately this doesn't work so well due to
> commit a95b37e20db9 ("kbuild: get out of
> ") which moved the inclusion of linux/compiler.h to
> cflags using the -include compiler flag.
I doubt this statement.
Commit a95b37e20db9 is not the cause of the problem.
include/linux/kconfig.h is also included
by using the -include compiler flag.
See the top-level Makefile.
USERINCLUDE:= \
-I$(srctree)/arch/$(SRCARCH)/include/uapi \
-I$(objtree)/arch/$(SRCARCH)/include/generated/uapi \
-I$(srctree)/include/uapi \
-I$(objtree)/include/generated/uapi \
-include $(srctree)/include/linux/kconfig.h
So, (then, )
would be also required for all C files
in the archprepare stage regardless of commit a95b37e20db9.
The change happened in commit 28128c61e08e.
One more thing, you are not touching any makefile in this version.
Maybe, you can prefix the subject with "compiler.h:" or something
instead of "kbuild:".
> Because the -include flag is present for all C files we compile, we need
> the architecture-provided header to be present before any C files are
> compiled. If any C files can be compiled prior to the asm-generic header
> wrappers being generated then we hit a build failure due to missing
> header. Such cases do exist - one pointed out by the kbuild test robot
> is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part
> of the archprepare target [1].
>
> This leaves us with a few options:
>
> 1) Use generic-y & fix any build failures we find by enforcing
> ordering such that the asm-generic target occurs before any C
> compilation, such that linux/compiler_types.h can always include
> the generated asm-generic wrapper which in turn includes the empty
> asm-generic header. This would rely on us finding all the
> problematic cases - I don't know for sure that the ia64 issue is
> the only one.
>
> 2) Add an actual empty header to each architecture, so that we don't
> need the generated asm-generic wrapper. This seems messy.
>
> 3) Give up & add #ifdef CONFIG_MIPS or similar to
> linux/compiler_types.h. This seems messy too.
>
> 4) Include the arch header only when it's actually needed, removing
> the need for the asm-generic wrapper for all other architectures.
>
> This patch allows us to use approach 4, by including an asm/compiler.h
> header from linux/compiler_types.h after the inclusion of the
> compiler-specific linux/compiler-*.h header(s). We do this
> conditionally, only when CONFIG_HAVE_ARCH_COMPILER_H is selected, in
> order to avoid the need for asm-generic wrappers & the associated build
> ordering issue described above. The asm/compiler.h header is included
> after the generic linux/compiler-*.h header(s) for consistency with the
> way linux/compiler-intel.h & linux/compiler-clang.h are included after
> the linux/compiler-gcc.h header that they override.
>
> [1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html
>
> Signed-off-by: Paul Burton
> Cc: Arnd Bergmann
> Cc: James Hogan
> Cc: Masahiro Yamada
> Cc: Ralf Baechle
> Cc: linux-a...@vger.kernel.org
> Cc: linux-kbu...@vger.kernel.org
> Cc: linux-m...@linux-mips.org
[PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
We have a need to override the definition of
barrier_before_unreachable() for MIPS, which means we either need to add
architecture-specific code into linux/compiler-gcc.h or we need to allow
the architecture to provide a header that can define the macro before
the generic definition. The latter seems like the better approach.
A straightforward approach to the per-arch header is to make use of
asm-generic to provide a default empty header & adjust architectures
which don't need anything specific to make use of that by adding the
header to generic-y. Unfortunately this doesn't work so well due to
commit a95b37e20db9 ("kbuild: get out of
") which moved the inclusion of linux/compiler.h to
cflags using the -include compiler flag.
Because the -include flag is present for all C files we compile, we need
the architecture-provided header to be present before any C files are
compiled. If any C files can be compiled prior to the asm-generic header
wrappers being generated then we hit a build failure due to missing
header. Such cases do exist - one pointed out by the kbuild test robot
is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part
of the archprepare target [1].
This leaves us with a few options:
1) Use generic-y & fix any build failures we find by enforcing
ordering such that the asm-generic target occurs before any C
compilation, such that linux/compiler_types.h can always include
the generated asm-generic wrapper which in turn includes the empty
asm-generic header. This would rely on us finding all the
problematic cases - I don't know for sure that the ia64 issue is
the only one.
2) Add an actual empty header to each architecture, so that we don't
need the generated asm-generic wrapper. This seems messy.
3) Give up & add #ifdef CONFIG_MIPS or similar to
linux/compiler_types.h. This seems messy too.
4) Include the arch header only when it's actually needed, removing
the need for the asm-generic wrapper for all other architectures.
This patch allows us to use approach 4, by including an asm/compiler.h
header from linux/compiler_types.h after the inclusion of the
compiler-specific linux/compiler-*.h header(s). We do this
conditionally, only when CONFIG_HAVE_ARCH_COMPILER_H is selected, in
order to avoid the need for asm-generic wrappers & the associated build
ordering issue described above. The asm/compiler.h header is included
after the generic linux/compiler-*.h header(s) for consistency with the
way linux/compiler-intel.h & linux/compiler-clang.h are included after
the linux/compiler-gcc.h header that they override.
[1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html
Signed-off-by: Paul Burton
Cc: Arnd Bergmann
Cc: James Hogan
Cc: Masahiro Yamada
Cc: Ralf Baechle
Cc: linux-a...@vger.kernel.org
Cc: linux-kbu...@vger.kernel.org
Cc: linux-m...@linux-mips.org
---
Changes in v9:
- Use Kconfig & a #include directive as Masahiro suggested.
- Go with asm/compiler.h rather than asm/compiler_types.h as it's really
definitions from linux/compiler-*.h that we want to override & the
conditional include means we don't need to worry about existing
asm/compiler.h headers.
- Tweak subject & commit message to reflect the changes above.
Changes in v8:
- New patch.
arch/Kconfig | 8
include/linux/compiler_types.h | 12
2 files changed, 20 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index c6148166a7b4..c0b56b0d86b0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -841,6 +841,14 @@ config REFCOUNT_FULL
against various use-after-free conditions that can be used in
security flaw exploits.
+config HAVE_ARCH_COMPILER_H
+ bool
+ help
+ An architecture can select this if it provides an
+ asm/compiler.h header that should be included after
+ linux/compiler-*.h in order to override macro definitions that those
+ headers generally provide.
+
source "kernel/gcov/Kconfig"
source "scripts/gcc-plugins/Kconfig"
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index fbf337933fd8..66239549d240 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -78,6 +78,18 @@ extern void __chk_io_ptr(const volatile void __iomem *);
#include
#endif
+/*
+ * Some architectures need to provide custom definitions of macros provided
+ * by linux/compiler-*.h, and can do so using asm/compiler.h. We include that
+ * conditionally rather than using an asm-generic wrapper in order to avoid
+ * build failures if any C compilation, which will include this file via an
+ * -include argument in c_flags, occurs prior to the asm-generic wrappers being
+ * generated.
+ */
+#ifdef CONFIG_HAVE_ARCH_COMPILER_H
+#include
+#endif
+
/*
* Generic compiler-dependent macros required for kernel
* build go below this comment. Actual compiler/compiler version
--
2.18.0
[PATCH v9 1/2] kbuild: Allow arch-specific asm/compiler.h
We have a need to override the definition of
barrier_before_unreachable() for MIPS, which means we either need to add
architecture-specific code into linux/compiler-gcc.h or we need to allow
the architecture to provide a header that can define the macro before
the generic definition. The latter seems like the better approach.
A straightforward approach to the per-arch header is to make use of
asm-generic to provide a default empty header & adjust architectures
which don't need anything specific to make use of that by adding the
header to generic-y. Unfortunately this doesn't work so well due to
commit a95b37e20db9 ("kbuild: get out of
") which moved the inclusion of linux/compiler.h to
cflags using the -include compiler flag.
Because the -include flag is present for all C files we compile, we need
the architecture-provided header to be present before any C files are
compiled. If any C files can be compiled prior to the asm-generic header
wrappers being generated then we hit a build failure due to missing
header. Such cases do exist - one pointed out by the kbuild test robot
is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part
of the archprepare target [1].
This leaves us with a few options:
1) Use generic-y & fix any build failures we find by enforcing
ordering such that the asm-generic target occurs before any C
compilation, such that linux/compiler_types.h can always include
the generated asm-generic wrapper which in turn includes the empty
asm-generic header. This would rely on us finding all the
problematic cases - I don't know for sure that the ia64 issue is
the only one.
2) Add an actual empty header to each architecture, so that we don't
need the generated asm-generic wrapper. This seems messy.
3) Give up & add #ifdef CONFIG_MIPS or similar to
linux/compiler_types.h. This seems messy too.
4) Include the arch header only when it's actually needed, removing
the need for the asm-generic wrapper for all other architectures.
This patch allows us to use approach 4, by including an asm/compiler.h
header from linux/compiler_types.h after the inclusion of the
compiler-specific linux/compiler-*.h header(s). We do this
conditionally, only when CONFIG_HAVE_ARCH_COMPILER_H is selected, in
order to avoid the need for asm-generic wrappers & the associated build
ordering issue described above. The asm/compiler.h header is included
after the generic linux/compiler-*.h header(s) for consistency with the
way linux/compiler-intel.h & linux/compiler-clang.h are included after
the linux/compiler-gcc.h header that they override.
[1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html
Signed-off-by: Paul Burton
Cc: Arnd Bergmann
Cc: James Hogan
Cc: Masahiro Yamada
Cc: Ralf Baechle
Cc: linux-a...@vger.kernel.org
Cc: linux-kbu...@vger.kernel.org
Cc: linux-m...@linux-mips.org
---
Changes in v9:
- Use Kconfig & a #include directive as Masahiro suggested.
- Go with asm/compiler.h rather than asm/compiler_types.h as it's really
definitions from linux/compiler-*.h that we want to override & the
conditional include means we don't need to worry about existing
asm/compiler.h headers.
- Tweak subject & commit message to reflect the changes above.
Changes in v8:
- New patch.
arch/Kconfig | 8
include/linux/compiler_types.h | 12
2 files changed, 20 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig
index c6148166a7b4..c0b56b0d86b0 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -841,6 +841,14 @@ config REFCOUNT_FULL
against various use-after-free conditions that can be used in
security flaw exploits.
+config HAVE_ARCH_COMPILER_H
+ bool
+ help
+ An architecture can select this if it provides an
+ asm/compiler.h header that should be included after
+ linux/compiler-*.h in order to override macro definitions that those
+ headers generally provide.
+
source "kernel/gcov/Kconfig"
source "scripts/gcc-plugins/Kconfig"
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index fbf337933fd8..66239549d240 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -78,6 +78,18 @@ extern void __chk_io_ptr(const volatile void __iomem *);
#include
#endif
+/*
+ * Some architectures need to provide custom definitions of macros provided
+ * by linux/compiler-*.h, and can do so using asm/compiler.h. We include that
+ * conditionally rather than using an asm-generic wrapper in order to avoid
+ * build failures if any C compilation, which will include this file via an
+ * -include argument in c_flags, occurs prior to the asm-generic wrappers being
+ * generated.
+ */
+#ifdef CONFIG_HAVE_ARCH_COMPILER_H
+#include
+#endif
+
/*
* Generic compiler-dependent macros required for kernel
* build go below this comment. Actual compiler/compiler version
--
2.18.0
