* [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-21 23:18 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers
The kernel has been using noinstr for correctness to politely request
that the compiler avoid adding various forms of instrumentation to
certain functions.
GCOV and PGO can both instrument functions, yet the function attribute
to disable such instrumentation (no_profile_instrument_function) was not
being used to suppress such implementation. Also, clang only just
recently gained support for no_profile_instrument_function. GCC has
supported that since 7.1+.
Add a new function annotation __no_profile that expands to
__attribute__((__no_profile_instrument_function__)) and Kconfig values
CC_HAS_NO_PROFILE_FN_ATTR and ARCH_WANTS_NO_INSTR. Make GCOV and PGO
depend on either !ARCH_WANTS_NO_INSTR or CC_HAS_NO_PROFILE_FN_ATTR.
Changes V1 -> V2:
* s/no_profile/no_profile_instrument_function/
* fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
* Pick up Fangrui + Miguel's reviewed-by tag.
* Add link to GCC's doc.
* Fix clang's doc format; will appear once clang-13 is released.
* New cleanup patch 2/3. Orthogonal to the series, but while I'm here...
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (3):
compiler_attributes.h: define __no_profile, add to noinstr
compiler_attributes.h: cleanups for GCC 4.9+
Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on
for GCOV and PGO
arch/Kconfig | 7 +++++++
arch/arm64/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
include/linux/compiler_attributes.h | 19 ++++++++++++++++---
include/linux/compiler_types.h | 2 +-
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
9 files changed, 33 insertions(+), 5 deletions(-)
base-commit: 4356bc4c0425c81e204f561acf4dd0095544a6cb
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-21 23:18 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers
The kernel has been using noinstr for correctness to politely request
that the compiler avoid adding various forms of instrumentation to
certain functions.
GCOV and PGO can both instrument functions, yet the function attribute
to disable such instrumentation (no_profile_instrument_function) was not
being used to suppress such implementation. Also, clang only just
recently gained support for no_profile_instrument_function. GCC has
supported that since 7.1+.
Add a new function annotation __no_profile that expands to
__attribute__((__no_profile_instrument_function__)) and Kconfig values
CC_HAS_NO_PROFILE_FN_ATTR and ARCH_WANTS_NO_INSTR. Make GCOV and PGO
depend on either !ARCH_WANTS_NO_INSTR or CC_HAS_NO_PROFILE_FN_ATTR.
Changes V1 -> V2:
* s/no_profile/no_profile_instrument_function/
* fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
* Pick up Fangrui + Miguel's reviewed-by tag.
* Add link to GCC's doc.
* Fix clang's doc format; will appear once clang-13 is released.
* New cleanup patch 2/3. Orthogonal to the series, but while I'm here...
Base is
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=for-next/clang/pgo.
Nick Desaulniers (3):
compiler_attributes.h: define __no_profile, add to noinstr
compiler_attributes.h: cleanups for GCC 4.9+
Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on
for GCOV and PGO
arch/Kconfig | 7 +++++++
arch/arm64/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
include/linux/compiler_attributes.h | 19 ++++++++++++++++---
include/linux/compiler_types.h | 2 +-
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
9 files changed, 33 insertions(+), 5 deletions(-)
base-commit: 4356bc4c0425c81e204f561acf4dd0095544a6cb
--
2.32.0.288.g62a8d224e6-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 1/3] compiler_attributes.h: define __no_profile, add to noinstr
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:18 ` Nick Desaulniers
-1 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers, Miguel Ojeda
noinstr implies that we would like the compiler to avoid instrumenting a
function. Add support for the compiler attribute
no_profile_instrument_function to compiler_attributes.h, then add
__no_profile to the definition of noinstr.
Link: https://lore.kernel.org/lkml/20210614162018.GD68749@worktop.programming.kicks-ass.net/
Link: https://reviews.llvm.org/D104257
Link: https://reviews.llvm.org/D104475
Link: https://reviews.llvm.org/D104658
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Reviewed-by: Fangrui Song <maskray@google.com>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* s/no_profile/no_profile_instrument_function/
* fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
* Pick up Fangrui + Miguel's reviewed-by tag.
* Add link to GCC's doc.
* Fix clang's doc format; will appear once clang-13 is released.
include/linux/compiler_attributes.h | 13 +++++++++++++
include/linux/compiler_types.h | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index c043b8d2b17b..225511b17223 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -33,6 +33,7 @@
# define __GCC4_has_attribute___externally_visible__ 1
# define __GCC4_has_attribute___no_caller_saved_registers__ 0
# define __GCC4_has_attribute___noclone__ 1
+# define __GCC4_has_attribute___no_profile_instrument_function__ 0
# define __GCC4_has_attribute___nonstring__ 0
# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
@@ -237,6 +238,18 @@
# define __nonstring
#endif
+/*
+ * Optional: only supported since GCC >= 7.1, clang >= 13.0.
+ *
+ * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fprofile_005finstrument_005ffunction-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#no-profile-instrument-function
+ */
+#if __has_attribute(__no_profile_instrument_function__)
+# define __no_profile __attribute__((__no_profile_instrument_function__))
+#else
+# define __no_profile
+#endif
+
/*
* gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
* clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index d29bda7f6ebd..d509169860f1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -210,7 +210,7 @@ struct ftrace_likely_data {
/* Section for code which can't be instrumented at all */
#define noinstr \
noinline notrace __attribute((__section__(".noinstr.text"))) \
- __no_kcsan __no_sanitize_address
+ __no_kcsan __no_sanitize_address __no_profile
#endif /* __KERNEL__ */
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 1/3] compiler_attributes.h: define __no_profile, add to noinstr
@ 2021-06-21 23:18 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers, Miguel Ojeda
noinstr implies that we would like the compiler to avoid instrumenting a
function. Add support for the compiler attribute
no_profile_instrument_function to compiler_attributes.h, then add
__no_profile to the definition of noinstr.
Link: https://lore.kernel.org/lkml/20210614162018.GD68749@worktop.programming.kicks-ass.net/
Link: https://reviews.llvm.org/D104257
Link: https://reviews.llvm.org/D104475
Link: https://reviews.llvm.org/D104658
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
Reviewed-by: Fangrui Song <maskray@google.com>
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* s/no_profile/no_profile_instrument_function/
* fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
* Pick up Fangrui + Miguel's reviewed-by tag.
* Add link to GCC's doc.
* Fix clang's doc format; will appear once clang-13 is released.
include/linux/compiler_attributes.h | 13 +++++++++++++
include/linux/compiler_types.h | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index c043b8d2b17b..225511b17223 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -33,6 +33,7 @@
# define __GCC4_has_attribute___externally_visible__ 1
# define __GCC4_has_attribute___no_caller_saved_registers__ 0
# define __GCC4_has_attribute___noclone__ 1
+# define __GCC4_has_attribute___no_profile_instrument_function__ 0
# define __GCC4_has_attribute___nonstring__ 0
# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
@@ -237,6 +238,18 @@
# define __nonstring
#endif
+/*
+ * Optional: only supported since GCC >= 7.1, clang >= 13.0.
+ *
+ * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fprofile_005finstrument_005ffunction-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#no-profile-instrument-function
+ */
+#if __has_attribute(__no_profile_instrument_function__)
+# define __no_profile __attribute__((__no_profile_instrument_function__))
+#else
+# define __no_profile
+#endif
+
/*
* gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
* clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index d29bda7f6ebd..d509169860f1 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -210,7 +210,7 @@ struct ftrace_likely_data {
/* Section for code which can't be instrumented at all */
#define noinstr \
noinline notrace __attribute((__section__(".noinstr.text"))) \
- __no_kcsan __no_sanitize_address
+ __no_kcsan __no_sanitize_address __no_profile
#endif /* __KERNEL__ */
--
2.32.0.288.g62a8d224e6-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:18 ` Nick Desaulniers
-1 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers, Miguel Ojeda
Since
commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
we no longer support building the kernel with GCC 4.8; drop the
preprocess checks for __GNUC_MINOR__ version. It's implied that if
__GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
left is 9.
Cc: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
include/linux/compiler_attributes.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 225511b17223..84b1c970acb3 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -27,7 +27,7 @@
*/
#ifndef __has_attribute
# define __has_attribute(x) __GCC4_has_attribute_##x
-# define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9)
+# define __GCC4_has_attribute___assume_aligned__ 1
# define __GCC4_has_attribute___copy__ 0
# define __GCC4_has_attribute___designated_init__ 0
# define __GCC4_has_attribute___externally_visible__ 1
@@ -35,8 +35,8 @@
# define __GCC4_has_attribute___noclone__ 1
# define __GCC4_has_attribute___no_profile_instrument_function__ 0
# define __GCC4_has_attribute___nonstring__ 0
-# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
-# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
+# define __GCC4_has_attribute___no_sanitize_address__ 1
+# define __GCC4_has_attribute___no_sanitize_undefined__ 1
# define __GCC4_has_attribute___fallthrough__ 0
#endif
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
@ 2021-06-21 23:18 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers, Miguel Ojeda
Since
commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
we no longer support building the kernel with GCC 4.8; drop the
preprocess checks for __GNUC_MINOR__ version. It's implied that if
__GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
left is 9.
Cc: Miguel Ojeda <ojeda@kernel.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
include/linux/compiler_attributes.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
index 225511b17223..84b1c970acb3 100644
--- a/include/linux/compiler_attributes.h
+++ b/include/linux/compiler_attributes.h
@@ -27,7 +27,7 @@
*/
#ifndef __has_attribute
# define __has_attribute(x) __GCC4_has_attribute_##x
-# define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9)
+# define __GCC4_has_attribute___assume_aligned__ 1
# define __GCC4_has_attribute___copy__ 0
# define __GCC4_has_attribute___designated_init__ 0
# define __GCC4_has_attribute___externally_visible__ 1
@@ -35,8 +35,8 @@
# define __GCC4_has_attribute___noclone__ 1
# define __GCC4_has_attribute___no_profile_instrument_function__ 0
# define __GCC4_has_attribute___nonstring__ 0
-# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
-# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
+# define __GCC4_has_attribute___no_sanitize_address__ 1
+# define __GCC4_has_attribute___no_sanitize_undefined__ 1
# define __GCC4_has_attribute___fallthrough__ 0
#endif
--
2.32.0.288.g62a8d224e6-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:18 ` Nick Desaulniers
-1 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile_instrument_function function
attribute. Add a Kconfig test for this and make PGO and GCOV depend on
it.
If an architecture is using noinstr, it should denote that via this
Kconfig value. That makes Kconfigs that depend on noinstr able to
express dependencies in an architecturally agnostic way.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* Add ARCH_WANTS_NO_INSTR
* Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
rather than list architectures explicitly, as per Nathan.
* s/no_profile/no_profile_instrument_function/
arch/Kconfig | 7 +++++++
arch/arm64/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
7 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 2b4109b0edee..2113c6b3b801 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
config ARCH_WANTS_DYNAMIC_TASK_STRUCT
bool
+config ARCH_WANTS_NO_INSTR
+ bool
+ help
+ An architecure should select this if the noinstr macro is being used on
+ functions to denote that the toolchain should avoid instrumenting such
+ functions and is required for correctness.
+
config ARCH_32BIT_OFF_T
bool
depends on !64BIT
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9f1d8566bbf9..39bf982b06f8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -93,6 +93,7 @@ config ARM64
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
select ARCH_WANT_LD_ORPHAN_WARN
+ select ARCH_WANTS_NO_INSTR
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARM_AMBA
select ARM_ARCH_TIMER
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b4c7c34069f8..bd60310f33b9 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -117,6 +117,7 @@ config S390
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_DEFAULT_BPF_JIT
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_TABLE_SORT
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index da43fd046149..7d6a44bb9b0e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -114,6 +114,7 @@ config X86
select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_HUGE_PMD_SHARE
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WANTS_THP_SWAP if X86_64
diff --git a/init/Kconfig b/init/Kconfig
index 1ea12c64e4c9..31397a7a45fb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
config CC_HAS_ASM_INLINE
def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
+config CC_HAS_NO_PROFILE_FN_ATTR
+ def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
+
config CONSTRUCTORS
bool
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 58f87a3092f3..053447183ac5 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -5,6 +5,7 @@ config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
select CONSTRUCTORS
default n
help
diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
index d2053df1111c..ce7fe04f303d 100644
--- a/kernel/pgo/Kconfig
+++ b/kernel/pgo/Kconfig
@@ -8,7 +8,8 @@ config PGO_CLANG
bool "Enable clang's PGO-based kernel profiling"
depends on DEBUG_FS
depends on ARCH_SUPPORTS_PGO_CLANG
- depends on CC_IS_CLANG && CLANG_VERSION >= 120000
+ depends on CC_IS_CLANG
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
help
This option enables clang's PGO (Profile Guided Optimization) based
code profiling to better optimize the kernel.
--
2.32.0.288.g62a8d224e6-goog
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-21 23:18 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-21 23:18 UTC (permalink / raw)
To: Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Nick Desaulniers
We don't want compiler instrumentation to touch noinstr functions, which
are annotated with the no_profile_instrument_function function
attribute. Add a Kconfig test for this and make PGO and GCOV depend on
it.
If an architecture is using noinstr, it should denote that via this
Kconfig value. That makes Kconfigs that depend on noinstr able to
express dependencies in an architecturally agnostic way.
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1 -> V2:
* Add ARCH_WANTS_NO_INSTR
* Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
rather than list architectures explicitly, as per Nathan.
* s/no_profile/no_profile_instrument_function/
arch/Kconfig | 7 +++++++
arch/arm64/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 3 +++
kernel/gcov/Kconfig | 1 +
kernel/pgo/Kconfig | 3 ++-
7 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 2b4109b0edee..2113c6b3b801 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
config ARCH_WANTS_DYNAMIC_TASK_STRUCT
bool
+config ARCH_WANTS_NO_INSTR
+ bool
+ help
+ An architecure should select this if the noinstr macro is being used on
+ functions to denote that the toolchain should avoid instrumenting such
+ functions and is required for correctness.
+
config ARCH_32BIT_OFF_T
bool
depends on !64BIT
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 9f1d8566bbf9..39bf982b06f8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -93,6 +93,7 @@ config ARM64
select ARCH_WANT_FRAME_POINTERS
select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
select ARCH_WANT_LD_ORPHAN_WARN
+ select ARCH_WANTS_NO_INSTR
select ARCH_HAS_UBSAN_SANITIZE_ALL
select ARM_AMBA
select ARM_ARCH_TIMER
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index b4c7c34069f8..bd60310f33b9 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -117,6 +117,7 @@ config S390
select ARCH_USE_BUILTIN_BSWAP
select ARCH_USE_CMPXCHG_LOCKREF
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_DEFAULT_BPF_JIT
select ARCH_WANT_IPC_PARSE_VERSION
select BUILDTIME_TABLE_SORT
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index da43fd046149..7d6a44bb9b0e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -114,6 +114,7 @@ config X86
select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
select ARCH_WANTS_DYNAMIC_TASK_STRUCT
+ select ARCH_WANTS_NO_INSTR
select ARCH_WANT_HUGE_PMD_SHARE
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WANTS_THP_SWAP if X86_64
diff --git a/init/Kconfig b/init/Kconfig
index 1ea12c64e4c9..31397a7a45fb 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
config CC_HAS_ASM_INLINE
def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
+config CC_HAS_NO_PROFILE_FN_ATTR
+ def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
+
config CONSTRUCTORS
bool
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index 58f87a3092f3..053447183ac5 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -5,6 +5,7 @@ config GCOV_KERNEL
bool "Enable gcov-based kernel profiling"
depends on DEBUG_FS
depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
select CONSTRUCTORS
default n
help
diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
index d2053df1111c..ce7fe04f303d 100644
--- a/kernel/pgo/Kconfig
+++ b/kernel/pgo/Kconfig
@@ -8,7 +8,8 @@ config PGO_CLANG
bool "Enable clang's PGO-based kernel profiling"
depends on DEBUG_FS
depends on ARCH_SUPPORTS_PGO_CLANG
- depends on CC_IS_CLANG && CLANG_VERSION >= 120000
+ depends on CC_IS_CLANG
+ depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
help
This option enables clang's PGO (Profile Guided Optimization) based
code profiling to better optimize the kernel.
--
2.32.0.288.g62a8d224e6-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:31 ` Miguel Ojeda
-1 siblings, 0 replies; 36+ messages in thread
From: Miguel Ojeda @ 2021-06-21 23:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, Linux Doc Mailing List, Linux Kbuild mailing list,
Dmitry Vyukov, Johannes Berg, linux-toolchains, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, linux-s390, Linux ARM,
Catalin Marinas, Miguel Ojeda
On Tue, Jun 22, 2021 at 1:18 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Since
> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
> we no longer support building the kernel with GCC 4.8; drop the
> preprocess checks for __GNUC_MINOR__ version. It's implied that if
> __GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
> left is 9.
Yeah, I was waiting for the raise to 5.x to remove the entire block,
but this is of course good since we did not get that yet :-)
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
@ 2021-06-21 23:31 ` Miguel Ojeda
0 siblings, 0 replies; 36+ messages in thread
From: Miguel Ojeda @ 2021-06-21 23:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, Linux Doc Mailing List, Linux Kbuild mailing list,
Dmitry Vyukov, Johannes Berg, linux-toolchains, Heiko Carstens,
Vasily Gorbik, Christian Borntraeger, linux-s390, Linux ARM,
Catalin Marinas, Miguel Ojeda
On Tue, Jun 22, 2021 at 1:18 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Since
> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
> we no longer support building the kernel with GCC 4.8; drop the
> preprocess checks for __GNUC_MINOR__ version. It's implied that if
> __GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
> left is 9.
Yeah, I was waiting for the raise to 5.x to remove the entire block,
but this is of course good since we did not get that yet :-)
Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] compiler_attributes.h: define __no_profile, add to noinstr
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:41 ` Nathan Chancellor
-1 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:41 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Miguel Ojeda
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> noinstr implies that we would like the compiler to avoid instrumenting a
> function. Add support for the compiler attribute
> no_profile_instrument_function to compiler_attributes.h, then add
> __no_profile to the definition of noinstr.
>
> Link: https://lore.kernel.org/lkml/20210614162018.GD68749@worktop.programming.kicks-ass.net/
> Link: https://reviews.llvm.org/D104257
> Link: https://reviews.llvm.org/D104475
> Link: https://reviews.llvm.org/D104658
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
> Reviewed-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes V1 -> V2:
> * s/no_profile/no_profile_instrument_function/
> * fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
> * Pick up Fangrui + Miguel's reviewed-by tag.
> * Add link to GCC's doc.
> * Fix clang's doc format; will appear once clang-13 is released.
>
> include/linux/compiler_attributes.h | 13 +++++++++++++
> include/linux/compiler_types.h | 2 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index c043b8d2b17b..225511b17223 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -33,6 +33,7 @@
> # define __GCC4_has_attribute___externally_visible__ 1
> # define __GCC4_has_attribute___no_caller_saved_registers__ 0
> # define __GCC4_has_attribute___noclone__ 1
> +# define __GCC4_has_attribute___no_profile_instrument_function__ 0
> # define __GCC4_has_attribute___nonstring__ 0
> # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> # define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
> @@ -237,6 +238,18 @@
> # define __nonstring
> #endif
>
> +/*
> + * Optional: only supported since GCC >= 7.1, clang >= 13.0.
> + *
> + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fprofile_005finstrument_005ffunction-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#no-profile-instrument-function
> + */
> +#if __has_attribute(__no_profile_instrument_function__)
> +# define __no_profile __attribute__((__no_profile_instrument_function__))
> +#else
> +# define __no_profile
> +#endif
> +
> /*
> * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> * clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index d29bda7f6ebd..d509169860f1 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -210,7 +210,7 @@ struct ftrace_likely_data {
> /* Section for code which can't be instrumented at all */
> #define noinstr \
> noinline notrace __attribute((__section__(".noinstr.text"))) \
> - __no_kcsan __no_sanitize_address
> + __no_kcsan __no_sanitize_address __no_profile
>
> #endif /* __KERNEL__ */
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/3] compiler_attributes.h: define __no_profile, add to noinstr
@ 2021-06-21 23:41 ` Nathan Chancellor
0 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:41 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Miguel Ojeda
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> noinstr implies that we would like the compiler to avoid instrumenting a
> function. Add support for the compiler attribute
> no_profile_instrument_function to compiler_attributes.h, then add
> __no_profile to the definition of noinstr.
>
> Link: https://lore.kernel.org/lkml/20210614162018.GD68749@worktop.programming.kicks-ass.net/
> Link: https://reviews.llvm.org/D104257
> Link: https://reviews.llvm.org/D104475
> Link: https://reviews.llvm.org/D104658
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223
> Reviewed-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Changes V1 -> V2:
> * s/no_profile/no_profile_instrument_function/
> * fix trailing double underscore on GCC 4 define, as per Fangrui+Miguel.
> * Pick up Fangrui + Miguel's reviewed-by tag.
> * Add link to GCC's doc.
> * Fix clang's doc format; will appear once clang-13 is released.
>
> include/linux/compiler_attributes.h | 13 +++++++++++++
> include/linux/compiler_types.h | 2 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index c043b8d2b17b..225511b17223 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -33,6 +33,7 @@
> # define __GCC4_has_attribute___externally_visible__ 1
> # define __GCC4_has_attribute___no_caller_saved_registers__ 0
> # define __GCC4_has_attribute___noclone__ 1
> +# define __GCC4_has_attribute___no_profile_instrument_function__ 0
> # define __GCC4_has_attribute___nonstring__ 0
> # define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> # define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
> @@ -237,6 +238,18 @@
> # define __nonstring
> #endif
>
> +/*
> + * Optional: only supported since GCC >= 7.1, clang >= 13.0.
> + *
> + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fprofile_005finstrument_005ffunction-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#no-profile-instrument-function
> + */
> +#if __has_attribute(__no_profile_instrument_function__)
> +# define __no_profile __attribute__((__no_profile_instrument_function__))
> +#else
> +# define __no_profile
> +#endif
> +
> /*
> * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> * clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index d29bda7f6ebd..d509169860f1 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -210,7 +210,7 @@ struct ftrace_likely_data {
> /* Section for code which can't be instrumented at all */
> #define noinstr \
> noinline notrace __attribute((__section__(".noinstr.text"))) \
> - __no_kcsan __no_sanitize_address
> + __no_kcsan __no_sanitize_address __no_profile
>
> #endif /* __KERNEL__ */
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:42 ` Nathan Chancellor
-1 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:42 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Miguel Ojeda
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> Since
> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
> we no longer support building the kernel with GCC 4.8; drop the
> preprocess checks for __GNUC_MINOR__ version. It's implied that if
> __GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
> left is 9.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> include/linux/compiler_attributes.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 225511b17223..84b1c970acb3 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -27,7 +27,7 @@
> */
> #ifndef __has_attribute
> # define __has_attribute(x) __GCC4_has_attribute_##x
> -# define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9)
> +# define __GCC4_has_attribute___assume_aligned__ 1
> # define __GCC4_has_attribute___copy__ 0
> # define __GCC4_has_attribute___designated_init__ 0
> # define __GCC4_has_attribute___externally_visible__ 1
> @@ -35,8 +35,8 @@
> # define __GCC4_has_attribute___noclone__ 1
> # define __GCC4_has_attribute___no_profile_instrument_function__ 0
> # define __GCC4_has_attribute___nonstring__ 0
> -# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> -# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
> +# define __GCC4_has_attribute___no_sanitize_address__ 1
> +# define __GCC4_has_attribute___no_sanitize_undefined__ 1
> # define __GCC4_has_attribute___fallthrough__ 0
> #endif
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+
@ 2021-06-21 23:42 ` Nathan Chancellor
0 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:42 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas, Miguel Ojeda
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> Since
> commit 6ec4476ac825 ("Raise gcc version requirement to 4.9")
> we no longer support building the kernel with GCC 4.8; drop the
> preprocess checks for __GNUC_MINOR__ version. It's implied that if
> __GNUC_MAJOR__ is 4, then the only supported version of __GNUC_MINOR__
> left is 9.
>
> Cc: Miguel Ojeda <ojeda@kernel.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> include/linux/compiler_attributes.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> index 225511b17223..84b1c970acb3 100644
> --- a/include/linux/compiler_attributes.h
> +++ b/include/linux/compiler_attributes.h
> @@ -27,7 +27,7 @@
> */
> #ifndef __has_attribute
> # define __has_attribute(x) __GCC4_has_attribute_##x
> -# define __GCC4_has_attribute___assume_aligned__ (__GNUC_MINOR__ >= 9)
> +# define __GCC4_has_attribute___assume_aligned__ 1
> # define __GCC4_has_attribute___copy__ 0
> # define __GCC4_has_attribute___designated_init__ 0
> # define __GCC4_has_attribute___externally_visible__ 1
> @@ -35,8 +35,8 @@
> # define __GCC4_has_attribute___noclone__ 1
> # define __GCC4_has_attribute___no_profile_instrument_function__ 0
> # define __GCC4_has_attribute___nonstring__ 0
> -# define __GCC4_has_attribute___no_sanitize_address__ (__GNUC_MINOR__ >= 8)
> -# define __GCC4_has_attribute___no_sanitize_undefined__ (__GNUC_MINOR__ >= 9)
> +# define __GCC4_has_attribute___no_sanitize_address__ 1
> +# define __GCC4_has_attribute___no_sanitize_undefined__ 1
> # define __GCC4_has_attribute___fallthrough__ 0
> #endif
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-21 23:45 ` Nathan Chancellor
-1 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:45 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Small nit below.
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
Architecture
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-21 23:45 ` Nathan Chancellor
0 siblings, 0 replies; 36+ messages in thread
From: Nathan Chancellor @ 2021-06-21 23:45 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On 6/21/2021 4:18 PM, 'Nick Desaulniers' via Clang Built Linux wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Small nit below.
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
Architecture
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 7:25 ` Peter Zijlstra
-1 siblings, 0 replies; 36+ messages in thread
From: Peter Zijlstra @ 2021-06-22 7:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:19PM -0700, Nick Desaulniers wrote:
> Nick Desaulniers (3):
> compiler_attributes.h: define __no_profile, add to noinstr
> compiler_attributes.h: cleanups for GCC 4.9+
> Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on
> for GCOV and PGO
Thanks for sorting this Nick!
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-22 7:25 ` Peter Zijlstra
0 siblings, 0 replies; 36+ messages in thread
From: Peter Zijlstra @ 2021-06-22 7:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:19PM -0700, Nick Desaulniers wrote:
> Nick Desaulniers (3):
> compiler_attributes.h: define __no_profile, add to noinstr
> compiler_attributes.h: cleanups for GCC 4.9+
> Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on
> for GCOV and PGO
Thanks for sorting this Nick!
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 7:25 ` Peter Zijlstra
-1 siblings, 0 replies; 36+ messages in thread
From: Peter Zijlstra @ 2021-06-22 7:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
There's also CC_HAS_WORKING_NOSANITIZE_ADDRESS in lib/Kconfig.kasan that
might want to be hooked into this, but that can be done separately I
suppose.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 7:25 ` Peter Zijlstra
0 siblings, 0 replies; 36+ messages in thread
From: Peter Zijlstra @ 2021-06-22 7:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Will Deacon, Arnd Bergmann,
Andrew Morton, Rasmus Villemoes, linux-kernel, clang-built-linux,
x86, Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
There's also CC_HAS_WORKING_NOSANITIZE_ADDRESS in lib/Kconfig.kasan that
might want to be hooked into this, but that can be done separately I
suppose.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-22 7:25 ` Peter Zijlstra
@ 2021-06-22 8:09 ` Marco Elver
-1 siblings, 0 replies; 36+ messages in thread
From: Marco Elver @ 2021-06-22 8:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Nick Desaulniers, Kees Cook, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Jonathan Corbet, Fangrui Song, linux-doc,
linux-kbuild, Dmitry Vyukov, johannes.berg, linux-toolchains,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390,
linux-arm-kernel, Catalin Marinas
On Tue, 22 Jun 2021 at 09:26, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> > Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> > Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > Changes V1 -> V2:
> > * Add ARCH_WANTS_NO_INSTR
> > * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> > rather than list architectures explicitly, as per Nathan.
> > * s/no_profile/no_profile_instrument_function/
> >
> > arch/Kconfig | 7 +++++++
> > arch/arm64/Kconfig | 1 +
> > arch/s390/Kconfig | 1 +
> > arch/x86/Kconfig | 1 +
> > init/Kconfig | 3 +++
> > kernel/gcov/Kconfig | 1 +
> > kernel/pgo/Kconfig | 3 ++-
> > 7 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 2b4109b0edee..2113c6b3b801 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> > config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> > bool
> >
> > +config ARCH_WANTS_NO_INSTR
> > + bool
> > + help
> > + An architecure should select this if the noinstr macro is being used on
> > + functions to denote that the toolchain should avoid instrumenting such
> > + functions and is required for correctness.
> > +
> > config ARCH_32BIT_OFF_T
> > bool
> > depends on !64BIT
>
> There's also CC_HAS_WORKING_NOSANITIZE_ADDRESS in lib/Kconfig.kasan that
> might want to be hooked into this, but that can be done separately I
> suppose.
KASAN already depends on this for all compiler instrumentation modes,
not just for 'noinstr' but also to avoid false positives. So it's not
just for noinstr's benefit, and we should not weaken the requirement
there.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 8:09 ` Marco Elver
0 siblings, 0 replies; 36+ messages in thread
From: Marco Elver @ 2021-06-22 8:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Nick Desaulniers, Kees Cook, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Jonathan Corbet, Fangrui Song, linux-doc,
linux-kbuild, Dmitry Vyukov, johannes.berg, linux-toolchains,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390,
linux-arm-kernel, Catalin Marinas
On Tue, 22 Jun 2021 at 09:26, Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> > Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> > Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> > Changes V1 -> V2:
> > * Add ARCH_WANTS_NO_INSTR
> > * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> > rather than list architectures explicitly, as per Nathan.
> > * s/no_profile/no_profile_instrument_function/
> >
> > arch/Kconfig | 7 +++++++
> > arch/arm64/Kconfig | 1 +
> > arch/s390/Kconfig | 1 +
> > arch/x86/Kconfig | 1 +
> > init/Kconfig | 3 +++
> > kernel/gcov/Kconfig | 1 +
> > kernel/pgo/Kconfig | 3 ++-
> > 7 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 2b4109b0edee..2113c6b3b801 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> > config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> > bool
> >
> > +config ARCH_WANTS_NO_INSTR
> > + bool
> > + help
> > + An architecure should select this if the noinstr macro is being used on
> > + functions to denote that the toolchain should avoid instrumenting such
> > + functions and is required for correctness.
> > +
> > config ARCH_32BIT_OFF_T
> > bool
> > depends on !64BIT
>
> There's also CC_HAS_WORKING_NOSANITIZE_ADDRESS in lib/Kconfig.kasan that
> might want to be hooked into this, but that can be done separately I
> suppose.
KASAN already depends on this for all compiler instrumentation modes,
not just for 'noinstr' but also to avoid false positives. So it's not
just for noinstr's benefit, and we should not weaken the requirement
there.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 9:05 ` Mark Rutland
-1 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-06-22 9:05 UTC (permalink / raw)
To: Nick Desaulniers, Catalin Marinas, Will Deacon
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Arnd Bergmann, Andrew Morton, Rasmus Villemoes, linux-kernel,
clang-built-linux, x86, Borislav Petkov, Martin Liska,
Marco Elver, Jonathan Corbet, Fangrui Song, linux-doc,
linux-kbuild, Dmitry Vyukov, johannes.berg, linux-toolchains,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390,
linux-arm-kernel
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
FWIW, this looks good to me:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Catalin, Will, are you happy iwth the arm64 bit?
Thanks,
Makr.
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
> --
> 2.32.0.288.g62a8d224e6-goog
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 9:05 ` Mark Rutland
0 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-06-22 9:05 UTC (permalink / raw)
To: Nick Desaulniers, Catalin Marinas, Will Deacon
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Arnd Bergmann, Andrew Morton, Rasmus Villemoes, linux-kernel,
clang-built-linux, x86, Borislav Petkov, Martin Liska,
Marco Elver, Jonathan Corbet, Fangrui Song, linux-doc,
linux-kbuild, Dmitry Vyukov, johannes.berg, linux-toolchains,
Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390,
linux-arm-kernel
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
FWIW, this looks good to me:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Catalin, Will, are you happy iwth the arm64 bit?
Thanks,
Makr.
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 2b4109b0edee..2113c6b3b801 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -285,6 +285,13 @@ config ARCH_THREAD_STACK_ALLOCATOR
> config ARCH_WANTS_DYNAMIC_TASK_STRUCT
> bool
>
> +config ARCH_WANTS_NO_INSTR
> + bool
> + help
> + An architecure should select this if the noinstr macro is being used on
> + functions to denote that the toolchain should avoid instrumenting such
> + functions and is required for correctness.
> +
> config ARCH_32BIT_OFF_T
> bool
> depends on !64BIT
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9f1d8566bbf9..39bf982b06f8 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -93,6 +93,7 @@ config ARM64
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_WANT_HUGE_PMD_SHARE if ARM64_4K_PAGES || (ARM64_16K_PAGES && !ARM64_VA_BITS_36)
> select ARCH_WANT_LD_ORPHAN_WARN
> + select ARCH_WANTS_NO_INSTR
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> select ARM_AMBA
> select ARM_ARCH_TIMER
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index b4c7c34069f8..bd60310f33b9 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -117,6 +117,7 @@ config S390
> select ARCH_USE_BUILTIN_BSWAP
> select ARCH_USE_CMPXCHG_LOCKREF
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_DEFAULT_BPF_JIT
> select ARCH_WANT_IPC_PARSE_VERSION
> select BUILDTIME_TABLE_SORT
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index da43fd046149..7d6a44bb9b0e 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -114,6 +114,7 @@ config X86
> select ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH
> select ARCH_WANT_DEFAULT_BPF_JIT if X86_64
> select ARCH_WANTS_DYNAMIC_TASK_STRUCT
> + select ARCH_WANTS_NO_INSTR
> select ARCH_WANT_HUGE_PMD_SHARE
> select ARCH_WANT_LD_ORPHAN_WARN
> select ARCH_WANTS_THP_SWAP if X86_64
> diff --git a/init/Kconfig b/init/Kconfig
> index 1ea12c64e4c9..31397a7a45fb 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -83,6 +83,9 @@ config TOOLS_SUPPORT_RELR
> config CC_HAS_ASM_INLINE
> def_bool $(success,echo 'void foo(void) { asm inline (""); }' | $(CC) -x c - -c -o /dev/null)
>
> +config CC_HAS_NO_PROFILE_FN_ATTR
> + def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
> +
> config CONSTRUCTORS
> bool
>
> diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
> index 58f87a3092f3..053447183ac5 100644
> --- a/kernel/gcov/Kconfig
> +++ b/kernel/gcov/Kconfig
> @@ -5,6 +5,7 @@ config GCOV_KERNEL
> bool "Enable gcov-based kernel profiling"
> depends on DEBUG_FS
> depends on !CC_IS_CLANG || CLANG_VERSION >= 110000
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> select CONSTRUCTORS
> default n
> help
> diff --git a/kernel/pgo/Kconfig b/kernel/pgo/Kconfig
> index d2053df1111c..ce7fe04f303d 100644
> --- a/kernel/pgo/Kconfig
> +++ b/kernel/pgo/Kconfig
> @@ -8,7 +8,8 @@ config PGO_CLANG
> bool "Enable clang's PGO-based kernel profiling"
> depends on DEBUG_FS
> depends on ARCH_SUPPORTS_PGO_CLANG
> - depends on CC_IS_CLANG && CLANG_VERSION >= 120000
> + depends on CC_IS_CLANG
> + depends on !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> help
> This option enables clang's PGO (Profile Guided Optimization) based
> code profiling to better optimize the kernel.
> --
> 2.32.0.288.g62a8d224e6-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 9:33 ` Heiko Carstens
-1 siblings, 0 replies; 36+ messages in thread
From: Heiko Carstens @ 2021-06-22 9:33 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Vasily Gorbik, Christian Borntraeger,
linux-s390, linux-arm-kernel, Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
For s390:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 9:33 ` Heiko Carstens
0 siblings, 0 replies; 36+ messages in thread
From: Heiko Carstens @ 2021-06-22 9:33 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Vasily Gorbik, Christian Borntraeger,
linux-s390, linux-arm-kernel, Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1 -> V2:
> * Add ARCH_WANTS_NO_INSTR
> * Change depdendencies to be !ARCH_WANTS_NO_INSTR || CC_HAS_NO_PROFILE_FN_ATTR
> rather than list architectures explicitly, as per Nathan.
> * s/no_profile/no_profile_instrument_function/
>
> arch/Kconfig | 7 +++++++
> arch/arm64/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 3 +++
> kernel/gcov/Kconfig | 1 +
> kernel/pgo/Kconfig | 3 ++-
> 7 files changed, 16 insertions(+), 1 deletion(-)
For s390:
Acked-by: Heiko Carstens <hca@linux.ibm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 9:35 ` Peter Oberparleiter
-1 siblings, 0 replies; 36+ messages in thread
From: Peter Oberparleiter @ 2021-06-22 9:35 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen, Masahiro Yamada,
Miguel Ojeda, Nathan Chancellor, Luc Van Oostenryck,
Ard Biesheuvel, Will Deacon, Arnd Bergmann, Andrew Morton,
Rasmus Villemoes, linux-kernel, clang-built-linux, x86,
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On 22.06.2021 01:18, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Looks good to me - thanks for resolving this problem!
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
Regards,
Peter
--
Peter Oberparleiter
Linux on Z Development - IBM Germany
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 9:35 ` Peter Oberparleiter
0 siblings, 0 replies; 36+ messages in thread
From: Peter Oberparleiter @ 2021-06-22 9:35 UTC (permalink / raw)
To: Nick Desaulniers, Kees Cook
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen, Masahiro Yamada,
Miguel Ojeda, Nathan Chancellor, Luc Van Oostenryck,
Ard Biesheuvel, Will Deacon, Arnd Bergmann, Andrew Morton,
Rasmus Villemoes, linux-kernel, clang-built-linux, x86,
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On 22.06.2021 01:18, Nick Desaulniers wrote:
> We don't want compiler instrumentation to touch noinstr functions, which
> are annotated with the no_profile_instrument_function function
> attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> it.
>
> If an architecture is using noinstr, it should denote that via this
> Kconfig value. That makes Kconfigs that depend on noinstr able to
> express dependencies in an architecturally agnostic way.
>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Looks good to me - thanks for resolving this problem!
Reviewed-by: Peter Oberparleiter <oberpar@linux.ibm.com>
Regards,
Peter
--
Peter Oberparleiter
Linux on Z Development - IBM Germany
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
2021-06-22 9:05 ` Mark Rutland
@ 2021-06-22 11:10 ` Will Deacon
-1 siblings, 0 replies; 36+ messages in thread
From: Will Deacon @ 2021-06-22 11:10 UTC (permalink / raw)
To: Mark Rutland
Cc: Nick Desaulniers, Catalin Marinas, Kees Cook, Peter Zijlstra,
Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Arnd Bergmann, Andrew Morton,
Rasmus Villemoes, linux-kernel, clang-built-linux, x86,
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel
On Tue, Jun 22, 2021 at 10:05:40AM +0100, Mark Rutland wrote:
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> > Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> > Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> FWIW, this looks good to me:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> Catalin, Will, are you happy iwth the arm64 bit?
Looks fine to me.
Will
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
@ 2021-06-22 11:10 ` Will Deacon
0 siblings, 0 replies; 36+ messages in thread
From: Will Deacon @ 2021-06-22 11:10 UTC (permalink / raw)
To: Mark Rutland
Cc: Nick Desaulniers, Catalin Marinas, Kees Cook, Peter Zijlstra,
Bill Wendling, Sami Tolvanen, Peter Oberparleiter,
Masahiro Yamada, Miguel Ojeda, Nathan Chancellor,
Luc Van Oostenryck, Ard Biesheuvel, Arnd Bergmann, Andrew Morton,
Rasmus Villemoes, linux-kernel, clang-built-linux, x86,
Borislav Petkov, Martin Liska, Marco Elver, Jonathan Corbet,
Fangrui Song, linux-doc, linux-kbuild, Dmitry Vyukov,
johannes.berg, linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel
On Tue, Jun 22, 2021 at 10:05:40AM +0100, Mark Rutland wrote:
> On Mon, Jun 21, 2021 at 04:18:22PM -0700, Nick Desaulniers wrote:
> > We don't want compiler instrumentation to touch noinstr functions, which
> > are annotated with the no_profile_instrument_function function
> > attribute. Add a Kconfig test for this and make PGO and GCOV depend on
> > it.
> >
> > If an architecture is using noinstr, it should denote that via this
> > Kconfig value. That makes Kconfigs that depend on noinstr able to
> > express dependencies in an architecturally agnostic way.
> >
> > Cc: Masahiro Yamada <masahiroy@kernel.org>
> > Cc: Peter Oberparleiter <oberpar@linux.ibm.com>
> > Link: https://lore.kernel.org/lkml/YMTn9yjuemKFLbws@hirez.programming.kicks-ass.net/
> > Link: https://lore.kernel.org/lkml/YMcssV%2Fn5IBGv4f0@hirez.programming.kicks-ass.net/
> > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
>
> FWIW, this looks good to me:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
> Catalin, Will, are you happy iwth the arm64 bit?
Looks fine to me.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-22 18:19 ` Kees Cook
-1 siblings, 0 replies; 36+ messages in thread
From: Kees Cook @ 2021-06-22 18:19 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:19PM -0700, Nick Desaulniers wrote:
> Add a new function annotation __no_profile that expands to
> __attribute__((__no_profile_instrument_function__)) and Kconfig values
> CC_HAS_NO_PROFILE_FN_ATTR and ARCH_WANTS_NO_INSTR. Make GCOV and PGO
> depend on either !ARCH_WANTS_NO_INSTR or CC_HAS_NO_PROFILE_FN_ATTR.
Awesome; thanks everyone! I'm doing a Clang rebuild now, and will do
kernel testing and push this to my for-next/clang/features tree shortly.
--
Kees Cook
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-22 18:19 ` Kees Cook
0 siblings, 0 replies; 36+ messages in thread
From: Kees Cook @ 2021-06-22 18:19 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Peter Zijlstra, Bill Wendling, Sami Tolvanen,
Peter Oberparleiter, Masahiro Yamada, Miguel Ojeda,
Nathan Chancellor, Luc Van Oostenryck, Ard Biesheuvel,
Will Deacon, Arnd Bergmann, Andrew Morton, Rasmus Villemoes,
linux-kernel, clang-built-linux, x86, Borislav Petkov,
Martin Liska, Marco Elver, Jonathan Corbet, Fangrui Song,
linux-doc, linux-kbuild, Dmitry Vyukov, johannes.berg,
linux-toolchains, Heiko Carstens, Vasily Gorbik,
Christian Borntraeger, linux-s390, linux-arm-kernel,
Catalin Marinas
On Mon, Jun 21, 2021 at 04:18:19PM -0700, Nick Desaulniers wrote:
> Add a new function annotation __no_profile that expands to
> __attribute__((__no_profile_instrument_function__)) and Kconfig values
> CC_HAS_NO_PROFILE_FN_ATTR and ARCH_WANTS_NO_INSTR. Make GCOV and PGO
> depend on either !ARCH_WANTS_NO_INSTR or CC_HAS_NO_PROFILE_FN_ATTR.
Awesome; thanks everyone! I'm doing a Clang rebuild now, and will do
kernel testing and push this to my for-next/clang/features tree shortly.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
2021-06-21 23:18 ` Nick Desaulniers
@ 2021-06-23 6:15 ` Kees Cook
-1 siblings, 0 replies; 36+ messages in thread
From: Kees Cook @ 2021-06-23 6:15 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Catalin Marinas, Vasily Gorbik, Ard Biesheuvel,
Rasmus Villemoes, linux-arm-kernel, Nathan Chancellor,
Fangrui Song, linux-kbuild, linux-doc, linux-kernel, linux-s390,
Peter Oberparleiter, Borislav Petkov, Luc Van Oostenryck,
Marco Elver, Christian Borntraeger, Masahiro Yamada,
Peter Zijlstra, Heiko Carstens, Andrew Morton, Miguel Ojeda,
Dmitry Vyukov, Bill Wendling, Arnd Bergmann, johannes.berg,
clang-built-linux, Jonathan Corbet, Martin Liska,
linux-toolchains, x86, Sami Tolvanen, Will Deacon
On Mon, 21 Jun 2021 16:18:19 -0700, Nick Desaulniers wrote:
> The kernel has been using noinstr for correctness to politely request
> that the compiler avoid adding various forms of instrumentation to
> certain functions.
>
> GCOV and PGO can both instrument functions, yet the function attribute
> to disable such instrumentation (no_profile_instrument_function) was not
> being used to suppress such implementation. Also, clang only just
> recently gained support for no_profile_instrument_function. GCC has
> supported that since 7.1+.
>
> [...]
Applied to for-next/clang/features, thanks!
[1/3] compiler_attributes.h: define __no_profile, add to noinstr
https://git.kernel.org/kees/c/380d53c45ff2
[2/3] compiler_attributes.h: cleanups for GCC 4.9+
https://git.kernel.org/kees/c/ae4d682dfd33
[3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
https://git.kernel.org/kees/c/51c2ee6d121c
Note that I've tweaked the series slightly to move the PGO Kconfig change into
the PGO patch.
--
Kees Cook
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-23 6:15 ` Kees Cook
0 siblings, 0 replies; 36+ messages in thread
From: Kees Cook @ 2021-06-23 6:15 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Kees Cook, Catalin Marinas, Vasily Gorbik, Ard Biesheuvel,
Rasmus Villemoes, linux-arm-kernel, Nathan Chancellor,
Fangrui Song, linux-kbuild, linux-doc, linux-kernel, linux-s390,
Peter Oberparleiter, Borislav Petkov, Luc Van Oostenryck,
Marco Elver, Christian Borntraeger, Masahiro Yamada,
Peter Zijlstra, Heiko Carstens, Andrew Morton, Miguel Ojeda,
Dmitry Vyukov, Bill Wendling, Arnd Bergmann, johannes.berg,
clang-built-linux, Jonathan Corbet, Martin Liska,
linux-toolchains, x86, Sami Tolvanen, Will Deacon
On Mon, 21 Jun 2021 16:18:19 -0700, Nick Desaulniers wrote:
> The kernel has been using noinstr for correctness to politely request
> that the compiler avoid adding various forms of instrumentation to
> certain functions.
>
> GCOV and PGO can both instrument functions, yet the function attribute
> to disable such instrumentation (no_profile_instrument_function) was not
> being used to suppress such implementation. Also, clang only just
> recently gained support for no_profile_instrument_function. GCC has
> supported that since 7.1+.
>
> [...]
Applied to for-next/clang/features, thanks!
[1/3] compiler_attributes.h: define __no_profile, add to noinstr
https://git.kernel.org/kees/c/380d53c45ff2
[2/3] compiler_attributes.h: cleanups for GCC 4.9+
https://git.kernel.org/kees/c/ae4d682dfd33
[3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
https://git.kernel.org/kees/c/51c2ee6d121c
Note that I've tweaked the series slightly to move the PGO Kconfig change into
the PGO patch.
--
Kees Cook
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
2021-06-23 6:15 ` Kees Cook
@ 2021-06-24 19:36 ` Nick Desaulniers
-1 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-24 19:36 UTC (permalink / raw)
To: Kees Cook
Cc: Catalin Marinas, Vasily Gorbik, Ard Biesheuvel, Rasmus Villemoes,
linux-arm-kernel, Nathan Chancellor, Fangrui Song, linux-kbuild,
linux-doc, linux-kernel, linux-s390, Peter Oberparleiter,
Borislav Petkov, Luc Van Oostenryck, Marco Elver,
Christian Borntraeger, Masahiro Yamada, Peter Zijlstra,
Heiko Carstens, Andrew Morton, Miguel Ojeda, Dmitry Vyukov,
Bill Wendling, Arnd Bergmann, johannes.berg, clang-built-linux,
Jonathan Corbet, Martin Liska, linux-toolchains, x86,
Sami Tolvanen, Will Deacon
On Tue, Jun 22, 2021 at 11:17 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, 21 Jun 2021 16:18:19 -0700, Nick Desaulniers wrote:
> > The kernel has been using noinstr for correctness to politely request
> > that the compiler avoid adding various forms of instrumentation to
> > certain functions.
> >
> > GCOV and PGO can both instrument functions, yet the function attribute
> > to disable such instrumentation (no_profile_instrument_function) was not
> > being used to suppress such implementation. Also, clang only just
> > recently gained support for no_profile_instrument_function. GCC has
> > supported that since 7.1+.
> >
> > [...]
>
> Applied to for-next/clang/features, thanks!
>
> [1/3] compiler_attributes.h: define __no_profile, add to noinstr
> https://git.kernel.org/kees/c/380d53c45ff2
> [2/3] compiler_attributes.h: cleanups for GCC 4.9+
> https://git.kernel.org/kees/c/ae4d682dfd33
> [3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
> https://git.kernel.org/kees/c/51c2ee6d121c
>
> Note that I've tweaked the series slightly to move the PGO Kconfig change into
> the PGO patch.
Ok, LGTM.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO
@ 2021-06-24 19:36 ` Nick Desaulniers
0 siblings, 0 replies; 36+ messages in thread
From: Nick Desaulniers @ 2021-06-24 19:36 UTC (permalink / raw)
To: Kees Cook
Cc: Catalin Marinas, Vasily Gorbik, Ard Biesheuvel, Rasmus Villemoes,
linux-arm-kernel, Nathan Chancellor, Fangrui Song, linux-kbuild,
linux-doc, linux-kernel, linux-s390, Peter Oberparleiter,
Borislav Petkov, Luc Van Oostenryck, Marco Elver,
Christian Borntraeger, Masahiro Yamada, Peter Zijlstra,
Heiko Carstens, Andrew Morton, Miguel Ojeda, Dmitry Vyukov,
Bill Wendling, Arnd Bergmann, johannes.berg, clang-built-linux,
Jonathan Corbet, Martin Liska, linux-toolchains, x86,
Sami Tolvanen, Will Deacon
On Tue, Jun 22, 2021 at 11:17 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, 21 Jun 2021 16:18:19 -0700, Nick Desaulniers wrote:
> > The kernel has been using noinstr for correctness to politely request
> > that the compiler avoid adding various forms of instrumentation to
> > certain functions.
> >
> > GCOV and PGO can both instrument functions, yet the function attribute
> > to disable such instrumentation (no_profile_instrument_function) was not
> > being used to suppress such implementation. Also, clang only just
> > recently gained support for no_profile_instrument_function. GCC has
> > supported that since 7.1+.
> >
> > [...]
>
> Applied to for-next/clang/features, thanks!
>
> [1/3] compiler_attributes.h: define __no_profile, add to noinstr
> https://git.kernel.org/kees/c/380d53c45ff2
> [2/3] compiler_attributes.h: cleanups for GCC 4.9+
> https://git.kernel.org/kees/c/ae4d682dfd33
> [3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO
> https://git.kernel.org/kees/c/51c2ee6d121c
>
> Note that I've tweaked the series slightly to move the PGO Kconfig change into
> the PGO patch.
Ok, LGTM.
--
Thanks,
~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2021-06-24 19:38 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 23:18 [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO Nick Desaulniers
2021-06-21 23:18 ` Nick Desaulniers
2021-06-21 23:18 ` [PATCH v2 1/3] compiler_attributes.h: define __no_profile, add to noinstr Nick Desaulniers
2021-06-21 23:18 ` Nick Desaulniers
2021-06-21 23:41 ` Nathan Chancellor
2021-06-21 23:41 ` Nathan Chancellor
2021-06-21 23:18 ` [PATCH v2 2/3] compiler_attributes.h: cleanups for GCC 4.9+ Nick Desaulniers
2021-06-21 23:18 ` Nick Desaulniers
2021-06-21 23:31 ` Miguel Ojeda
2021-06-21 23:31 ` Miguel Ojeda
2021-06-21 23:42 ` Nathan Chancellor
2021-06-21 23:42 ` Nathan Chancellor
2021-06-21 23:18 ` [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO Nick Desaulniers
2021-06-21 23:18 ` Nick Desaulniers
2021-06-21 23:45 ` Nathan Chancellor
2021-06-21 23:45 ` Nathan Chancellor
2021-06-22 7:25 ` Peter Zijlstra
2021-06-22 7:25 ` Peter Zijlstra
2021-06-22 8:09 ` Marco Elver
2021-06-22 8:09 ` Marco Elver
2021-06-22 9:05 ` Mark Rutland
2021-06-22 9:05 ` Mark Rutland
2021-06-22 11:10 ` Will Deacon
2021-06-22 11:10 ` Will Deacon
2021-06-22 9:33 ` Heiko Carstens
2021-06-22 9:33 ` Heiko Carstens
2021-06-22 9:35 ` Peter Oberparleiter
2021-06-22 9:35 ` Peter Oberparleiter
2021-06-22 7:25 ` [PATCH v2 0/3] no_profile fn attr and Kconfig for GCOV+PGO Peter Zijlstra
2021-06-22 7:25 ` Peter Zijlstra
2021-06-22 18:19 ` Kees Cook
2021-06-22 18:19 ` Kees Cook
2021-06-23 6:15 ` Kees Cook
2021-06-23 6:15 ` Kees Cook
2021-06-24 19:36 ` Nick Desaulniers
2021-06-24 19:36 ` Nick Desaulniers
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.