Re: [PATCH v2 3/3] Kconfig: add ARCH_WANTS_NO_INSTR+CC_HAS_NO_PROFILE_FN_ATTR, depend on for GCOV and PGO

[Nathan Chancellor <nathan@kernel.org>]

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.
>