Re: [PATCH v2 7/8] arm64: alternatives: add alternative_has_feature_*()

Re: [PATCH v2 7/8] arm64: alternatives: add alternative_has_feature_*()

[Nathan Chancellor]

Hi Mark,

On Mon, Sep 12, 2022 at 05:22:09PM +0100, Mark Rutland wrote:
> Currrently we use a mixture of alternative sequences and static branches
> to handle features detected at boot time. For ease of maintenance we
> generally prefer to use static branches in C code, but this has a few
> downsides:
> 
> * Each static branch has metadata in the __jump_table section, which is
>   not discarded after features are finalized. This wastes some space,
>   and slows down the patching of other static branches.
> 
> * The static branches are patched at a different point in time from the
>   alternatives, so changes are not atomic. This leaves a transient
>   period where there could be a mismatch between the behaviour of
>   alternatives and static branches, which could be problematic for some
>   features (e.g. pseudo-NMI).
> 
> * More (instrumentable) kernel code is executed to patch each static
>   branch, which can be risky when patching certain features (e.g.
>   irqflags management for pseudo-NMI).
> 
> * When CONFIG_JUMP_LABEL=n, static branches are turned into a load of a
>   flag and a conditional branch. This means it isn't safe to use such
>   static branches in an alternative address space (e.g. the NVHE/PKVM
>   hyp code), where the generated address isn't safe to acccess.
> 
> To deal with these issues, this patch introduces new
> alternative_has_feature_*() helpers, which work like static branches but
> are patched using alternatives. This ensures the patching is performed
> at the same time as other alternative patching, allows the metadata to
> be freed after patching, and is safe for use in alternative address
> spaces.
> 
> Note that all supported toolchains have asm goto support, and since
> commit:
> 
>   a0a12c3ed057af57 ("asm goto: eradicate CC_HAS_ASM_GOTO)"
> 
> ... the CC_HAS_ASM_GOTO Kconfig symbol has been removed, so no feature
> check is necessary, and we can always make use of asm goto.
> 
> Additionally, note that:
> 
> * This has no impact on cpus_have_cap(), which is a dynamic check.
> 
> * This has no functional impact on cpus_have_const_cap(). The branches
>   are patched slightly later than before this patch, but these branches
>   are not reachable until caps have been finalised.
> 
> * It is now invalid to use cpus_have_final_cap() in the window between
>   feature detection and patching. All existing uses are only expected
>   after patching anyway, so this should not be a problem.
> 
> * The LSE atomics will now be enabled during alternatives patching
>   rather than immediately before. As the LL/SC an LSE atomics are
>   functionally equivalent this should not be problematic.
> 
> When building defconfig with GCC 12.1.0, the resulting Image is 64KiB
> smaller:
> 
> | % ls -al Image-*
> | -rw-r--r-- 1 mark mark 37108224 Aug 23 09:56 Image-after
> | -rw-r--r-- 1 mark mark 37173760 Aug 23 09:54 Image-before
> 
> According to bloat-o-meter.pl:
> 
> | add/remove: 44/34 grow/shrink: 602/1294 up/down: 39692/-61108 (-21416)
> | Function                                     old     new   delta
> | [...]
> | Total: Before=16618336, After=16596920, chg -0.13%
> | add/remove: 0/2 grow/shrink: 0/0 up/down: 0/-1296 (-1296)
> | Data                                         old     new   delta
> | arm64_const_caps_ready                        16       -     -16
> | cpu_hwcap_keys                              1280       -   -1280
> | Total: Before=8987120, After=8985824, chg -0.01%
> | add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> | RO Data                                      old     new   delta
> | Total: Before=18408, After=18408, chg +0.00%
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/alternative-macros.h | 41 +++++++++++++++++++++
>  arch/arm64/include/asm/cpufeature.h         |  7 ++--
>  arch/arm64/include/asm/lse.h                |  5 +--
>  arch/arm64/kernel/cpufeature.c              | 25 -------------
>  arch/arm64/kernel/image-vars.h              |  4 --
>  5 files changed, 46 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> index 189c31be163ce..eaba9ec127897 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -213,4 +213,45 @@ alternative_endif
>  #define ALTERNATIVE(oldinstr, newinstr, ...)   \
>  	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
>  
> +#ifndef __ASSEMBLY__
> +
> +#include <linux/bug.h>
> +#include <linux/types.h>
> +
> +static __always_inline bool
> +alternative_has_feature_likely(unsigned long feature)
> +{
> +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +
> +	asm_volatile_goto(
> +	ALTERNATIVE("b	%l[l_no]", "nop", %[feature])
> +	:
> +	: [feature] "i" (feature)
> +	:
> +	: l_no);
> +
> +	return true;
> +l_no:
> +	return false;
> +}
> +
> +static __always_inline bool
> +alternative_has_feature_unlikely(unsigned long feature)
> +{
> +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +
> +	asm_volatile_goto(
> +	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> +	:
> +	: [feature] "i" (feature)
> +	:
> +	: l_yes);
> +
> +	return false;
> +l_yes:
> +	return true;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +
>  #endif /* __ASM_ALTERNATIVE_MACROS_H */

The addition of BUILD_BUG_ON() in these functions ends up blowing up
when building with clang + CONFIG_LTO:

  In file included from kernel/bounds.c:10:
  In file included from ./include/linux/page-flags.h:10:
  In file included from ./include/linux/bug.h:5:
  In file included from ./arch/arm64/include/asm/bug.h:26:
  In file included from ./include/asm-generic/bug.h:5:
  In file included from ./include/linux/compiler.h:248:
  In file included from ./arch/arm64/include/asm/rwonce.h:11:
  ./arch/arm64/include/asm/alternative-macros.h:224:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
          BUILD_BUG_ON(feature >= ARM64_NCAPS);
          ^
  ./arch/arm64/include/asm/alternative-macros.h:241:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
          BUILD_BUG_ON(feature >= ARM64_NCAPS);
          ^
  2 errors generated.

As you can see from the include chain, alternative-macros.h can end up
being included from bug.h through compiler.h and rwonce.h, which will be
before the definition of BUILD_BUG_ON() in build_bug.h, so it does not
get expanded by the preprocessor. I could not come up with a clean
solution other than moving these functions into their own header so that
they do not appear in the alternative-macros.h include path (see below).
Would that be acceptable (the file name is a placeholder, I could not
come up with anything better) or do you have any other suggestions?
Another solution that might work is open coding these BUILD_BUG_ON()
calls with compiletime_assert() directly, as that should be available at
this point.

Cheers,
Nathan

diff --git a/arch/arm64/include/asm/alternative-likely.h b/arch/arm64/include/asm/alternative-likely.h
new file mode 100644
index 000000000000..f4e63d65e9eb
--- /dev/null
+++ b/arch/arm64/include/asm/alternative-likely.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_ALTERNATIVE_LIKELY_H
+#define __ASM_ALTERNATIVE_LIKELY_H
+
+#ifndef __ASSEMBLY__
+
+#include <asm/alternative-macros.h>
+#include <linux/bug.h>
+#include <linux/types.h>
+
+static __always_inline bool
+alternative_has_feature_likely(unsigned long feature)
+{
+	BUILD_BUG_ON(feature >= ARM64_NCAPS);
+
+	asm_volatile_goto(
+	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
+	:
+	: [feature] "i" (feature)
+	:
+	: l_no);
+
+	return true;
+l_no:
+	return false;
+}
+
+static __always_inline bool
+alternative_has_feature_unlikely(unsigned long feature)
+{
+	BUILD_BUG_ON(feature >= ARM64_NCAPS);
+
+	asm_volatile_goto(
+	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
+	:
+	: [feature] "i" (feature)
+	:
+	: l_yes);
+
+	return false;
+l_yes:
+	return true;
+}
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __ASM_ALTERNATIVE_LIKELY_H */
diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
index 4a2a98d6d222..189c31be163c 100644
--- a/arch/arm64/include/asm/alternative-macros.h
+++ b/arch/arm64/include/asm/alternative-macros.h
@@ -213,45 +213,4 @@ alternative_endif
 #define ALTERNATIVE(oldinstr, newinstr, ...)   \
 	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
 
-#ifndef __ASSEMBLY__
-
-#include <linux/bug.h>
-#include <linux/types.h>
-
-static __always_inline bool
-alternative_has_feature_likely(unsigned long feature)
-{
-	BUILD_BUG_ON(feature >= ARM64_NCAPS);
-
-	asm_volatile_goto(
-	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
-	:
-	: [feature] "i" (feature)
-	:
-	: l_no);
-
-	return true;
-l_no:
-	return false;
-}
-
-static __always_inline bool
-alternative_has_feature_unlikely(unsigned long feature)
-{
-	BUILD_BUG_ON(feature >= ARM64_NCAPS);
-
-	asm_volatile_goto(
-	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
-	:
-	: [feature] "i" (feature)
-	:
-	: l_yes);
-
-	return false;
-l_yes:
-	return true;
-}
-
-#endif /* __ASSEMBLY__ */
-
 #endif /* __ASM_ALTERNATIVE_MACROS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ba8a47433b5a..147d326c4439 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -6,7 +6,7 @@
 #ifndef __ASM_CPUFEATURE_H
 #define __ASM_CPUFEATURE_H
 
-#include <asm/alternative-macros.h>
+#include <asm/alternative-likely.h>
 #include <asm/cpucaps.h>
 #include <asm/cputype.h>
 #include <asm/hwcap.h>
diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
index c503db8e73b0..b28b5cb73387 100644
--- a/arch/arm64/include/asm/lse.h
+++ b/arch/arm64/include/asm/lse.h
@@ -13,7 +13,7 @@
 #include <linux/jump_label.h>
 #include <linux/stringify.h>
 #include <asm/alternative.h>
-#include <asm/alternative-macros.h>
+#include <asm/alternative-likely.h>
 #include <asm/atomic_lse.h>
 #include <asm/cpucaps.h>
 

Re: [PATCH v2 7/8] arm64: alternatives: add alternative_has_feature_*()

[Mark Rutland]

On Mon, Sep 19, 2022 at 10:01:02AM -0700, Nathan Chancellor wrote:
> Hi Mark,

Hi Nathan,

> > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> > index 189c31be163ce..eaba9ec127897 100644
> > --- a/arch/arm64/include/asm/alternative-macros.h
> > +++ b/arch/arm64/include/asm/alternative-macros.h
> > @@ -213,4 +213,45 @@ alternative_endif
> >  #define ALTERNATIVE(oldinstr, newinstr, ...)   \
> >  	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> >  
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <linux/bug.h>
> > +#include <linux/types.h>
> > +
> > +static __always_inline bool
> > +alternative_has_feature_likely(unsigned long feature)
> > +{
> > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > +
> > +	asm_volatile_goto(
> > +	ALTERNATIVE("b	%l[l_no]", "nop", %[feature])
> > +	:
> > +	: [feature] "i" (feature)
> > +	:
> > +	: l_no);
> > +
> > +	return true;
> > +l_no:
> > +	return false;
> > +}
> > +
> > +static __always_inline bool
> > +alternative_has_feature_unlikely(unsigned long feature)
> > +{
> > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > +
> > +	asm_volatile_goto(
> > +	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> > +	:
> > +	: [feature] "i" (feature)
> > +	:
> > +	: l_yes);
> > +
> > +	return false;
> > +l_yes:
> > +	return true;
> > +}
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> >  #endif /* __ASM_ALTERNATIVE_MACROS_H */
> 
> The addition of BUILD_BUG_ON() in these functions ends up blowing up
> when building with clang + CONFIG_LTO:
> 
>   In file included from kernel/bounds.c:10:
>   In file included from ./include/linux/page-flags.h:10:
>   In file included from ./include/linux/bug.h:5:
>   In file included from ./arch/arm64/include/asm/bug.h:26:
>   In file included from ./include/asm-generic/bug.h:5:
>   In file included from ./include/linux/compiler.h:248:
>   In file included from ./arch/arm64/include/asm/rwonce.h:11:
>   ./arch/arm64/include/asm/alternative-macros.h:224:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>           BUILD_BUG_ON(feature >= ARM64_NCAPS);
>           ^
>   ./arch/arm64/include/asm/alternative-macros.h:241:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>           BUILD_BUG_ON(feature >= ARM64_NCAPS);
>           ^
>   2 errors generated.
> 
> As you can see from the include chain, alternative-macros.h can end up
> being included from bug.h through compiler.h and rwonce.h, which will be
> before the definition of BUILD_BUG_ON() in build_bug.h, so it does not
> get expanded by the preprocessor.

Sorry for this!

> I could not come up with a clean solution other than moving these functions
> into their own header so that they do not appear in the alternative-macros.h
> include path (see below).  Would that be acceptable (the file name is a
> placeholder, I could not come up with anything better) or do you have any
> other suggestions?

I think that'd be ok; I'd suggest `alternative_branch.h` if we do that...

> Another solution that might work is open coding these BUILD_BUG_ON()
> calls with compiletime_assert() directly, as that should be available at
> this point.

... but this sounds slightly nicer to me. We already use
compiletime_assert_atomic_type() elsewhere, so we're already relying on
compiletime_assert(), and the diff looks simple enough:

---->8----
diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
index 4a2a98d6d2227..966767debaa3a 100644
--- a/arch/arm64/include/asm/alternative-macros.h
+++ b/arch/arm64/include/asm/alternative-macros.h
@@ -215,13 +215,13 @@ alternative_endif
 
 #ifndef __ASSEMBLY__
 
-#include <linux/bug.h>
 #include <linux/types.h>
 
 static __always_inline bool
 alternative_has_feature_likely(unsigned long feature)
 {
-       BUILD_BUG_ON(feature >= ARM64_NCAPS);
+       compiletime_assert(feature < ARM64_NCAPS,
+                          "feature must be < ARM64_NCAPS");
 
        asm_volatile_goto(
        ALTERNATIVE_CB("b       %l[l_no]", %[feature], alt_cb_patch_nops)
@@ -238,7 +238,8 @@ alternative_has_feature_likely(unsigned long feature)
 static __always_inline bool
 alternative_has_feature_unlikely(unsigned long feature)
 {
-       BUILD_BUG_ON(feature >= ARM64_NCAPS);
+       compiletime_assert(feature < ARM64_NCAPS,
+                          "feature must be < ARM64_NCAPS");
 
        asm_volatile_goto(
        ALTERNATIVE("nop", "b   %l[l_yes]", %[feature])
---->8----

... and seems to work in a local build test (with LLVM 14.0.0 and LTO_THIN).

If that sounds ok to you I can spin that as a patch shortly.

Thanks,
Mark.

> 
> Cheers,
> Nathan
> 
> diff --git a/arch/arm64/include/asm/alternative-likely.h b/arch/arm64/include/asm/alternative-likely.h
> new file mode 100644
> index 000000000000..f4e63d65e9eb
> --- /dev/null
> +++ b/arch/arm64/include/asm/alternative-likely.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_ALTERNATIVE_LIKELY_H
> +#define __ASM_ALTERNATIVE_LIKELY_H
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <asm/alternative-macros.h>
> +#include <linux/bug.h>
> +#include <linux/types.h>
> +
> +static __always_inline bool
> +alternative_has_feature_likely(unsigned long feature)
> +{
> +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +
> +	asm_volatile_goto(
> +	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
> +	:
> +	: [feature] "i" (feature)
> +	:
> +	: l_no);
> +
> +	return true;
> +l_no:
> +	return false;
> +}
> +
> +static __always_inline bool
> +alternative_has_feature_unlikely(unsigned long feature)
> +{
> +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +
> +	asm_volatile_goto(
> +	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> +	:
> +	: [feature] "i" (feature)
> +	:
> +	: l_yes);
> +
> +	return false;
> +l_yes:
> +	return true;
> +}
> +
> +#endif /* __ASSEMBLY__ */
> +
> +#endif /* __ASM_ALTERNATIVE_LIKELY_H */
> diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> index 4a2a98d6d222..189c31be163c 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -213,45 +213,4 @@ alternative_endif
>  #define ALTERNATIVE(oldinstr, newinstr, ...)   \
>  	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
>  
> -#ifndef __ASSEMBLY__
> -
> -#include <linux/bug.h>
> -#include <linux/types.h>
> -
> -static __always_inline bool
> -alternative_has_feature_likely(unsigned long feature)
> -{
> -	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> -
> -	asm_volatile_goto(
> -	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
> -	:
> -	: [feature] "i" (feature)
> -	:
> -	: l_no);
> -
> -	return true;
> -l_no:
> -	return false;
> -}
> -
> -static __always_inline bool
> -alternative_has_feature_unlikely(unsigned long feature)
> -{
> -	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> -
> -	asm_volatile_goto(
> -	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> -	:
> -	: [feature] "i" (feature)
> -	:
> -	: l_yes);
> -
> -	return false;
> -l_yes:
> -	return true;
> -}
> -
> -#endif /* __ASSEMBLY__ */
> -
>  #endif /* __ASM_ALTERNATIVE_MACROS_H */
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ba8a47433b5a..147d326c4439 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -6,7 +6,7 @@
>  #ifndef __ASM_CPUFEATURE_H
>  #define __ASM_CPUFEATURE_H
>  
> -#include <asm/alternative-macros.h>
> +#include <asm/alternative-likely.h>
>  #include <asm/cpucaps.h>
>  #include <asm/cputype.h>
>  #include <asm/hwcap.h>
> diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
> index c503db8e73b0..b28b5cb73387 100644
> --- a/arch/arm64/include/asm/lse.h
> +++ b/arch/arm64/include/asm/lse.h
> @@ -13,7 +13,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/stringify.h>
>  #include <asm/alternative.h>
> -#include <asm/alternative-macros.h>
> +#include <asm/alternative-likely.h>
>  #include <asm/atomic_lse.h>
>  #include <asm/cpucaps.h>
>  

Re: [PATCH v2 7/8] arm64: alternatives: add alternative_has_feature_*()

[Nathan Chancellor]

On Tue, Sep 20, 2022 at 01:09:27PM +0100, Mark Rutland wrote:
> On Mon, Sep 19, 2022 at 10:01:02AM -0700, Nathan Chancellor wrote:
> > Hi Mark,
> 
> Hi Nathan,
> 
> > > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> > > index 189c31be163ce..eaba9ec127897 100644
> > > --- a/arch/arm64/include/asm/alternative-macros.h
> > > +++ b/arch/arm64/include/asm/alternative-macros.h
> > > @@ -213,4 +213,45 @@ alternative_endif
> > >  #define ALTERNATIVE(oldinstr, newinstr, ...)   \
> > >  	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> > >  
> > > +#ifndef __ASSEMBLY__
> > > +
> > > +#include <linux/bug.h>
> > > +#include <linux/types.h>
> > > +
> > > +static __always_inline bool
> > > +alternative_has_feature_likely(unsigned long feature)
> > > +{
> > > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > > +
> > > +	asm_volatile_goto(
> > > +	ALTERNATIVE("b	%l[l_no]", "nop", %[feature])
> > > +	:
> > > +	: [feature] "i" (feature)
> > > +	:
> > > +	: l_no);
> > > +
> > > +	return true;
> > > +l_no:
> > > +	return false;
> > > +}
> > > +
> > > +static __always_inline bool
> > > +alternative_has_feature_unlikely(unsigned long feature)
> > > +{
> > > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > > +
> > > +	asm_volatile_goto(
> > > +	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> > > +	:
> > > +	: [feature] "i" (feature)
> > > +	:
> > > +	: l_yes);
> > > +
> > > +	return false;
> > > +l_yes:
> > > +	return true;
> > > +}
> > > +
> > > +#endif /* __ASSEMBLY__ */
> > > +
> > >  #endif /* __ASM_ALTERNATIVE_MACROS_H */
> > 
> > The addition of BUILD_BUG_ON() in these functions ends up blowing up
> > when building with clang + CONFIG_LTO:
> > 
> >   In file included from kernel/bounds.c:10:
> >   In file included from ./include/linux/page-flags.h:10:
> >   In file included from ./include/linux/bug.h:5:
> >   In file included from ./arch/arm64/include/asm/bug.h:26:
> >   In file included from ./include/asm-generic/bug.h:5:
> >   In file included from ./include/linux/compiler.h:248:
> >   In file included from ./arch/arm64/include/asm/rwonce.h:11:
> >   ./arch/arm64/include/asm/alternative-macros.h:224:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> >           BUILD_BUG_ON(feature >= ARM64_NCAPS);
> >           ^
> >   ./arch/arm64/include/asm/alternative-macros.h:241:2: error: call to undeclared function 'BUILD_BUG_ON'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> >           BUILD_BUG_ON(feature >= ARM64_NCAPS);
> >           ^
> >   2 errors generated.
> > 
> > As you can see from the include chain, alternative-macros.h can end up
> > being included from bug.h through compiler.h and rwonce.h, which will be
> > before the definition of BUILD_BUG_ON() in build_bug.h, so it does not
> > get expanded by the preprocessor.
> 
> Sorry for this!

Totally okay, these include chains have bit us before :)

> > I could not come up with a clean solution other than moving these functions
> > into their own header so that they do not appear in the alternative-macros.h
> > include path (see below).  Would that be acceptable (the file name is a
> > placeholder, I could not come up with anything better) or do you have any
> > other suggestions?
> 
> I think that'd be ok; I'd suggest `alternative_branch.h` if we do that...
> 
> > Another solution that might work is open coding these BUILD_BUG_ON()
> > calls with compiletime_assert() directly, as that should be available at
> > this point.
> 
> ... but this sounds slightly nicer to me. We already use
> compiletime_assert_atomic_type() elsewhere, so we're already relying on
> compiletime_assert(), and the diff looks simple enough:

This diff looks a lot better to me too. I build tested it against
defconfig and allmodconfig, with and without CONFIG_LTO_CLANG_THIN and I
saw no additional failures. If you send it along formally, please feel
free to add:

Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---->8----
> diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> index 4a2a98d6d2227..966767debaa3a 100644
> --- a/arch/arm64/include/asm/alternative-macros.h
> +++ b/arch/arm64/include/asm/alternative-macros.h
> @@ -215,13 +215,13 @@ alternative_endif
>  
>  #ifndef __ASSEMBLY__
>  
> -#include <linux/bug.h>
>  #include <linux/types.h>
>  
>  static __always_inline bool
>  alternative_has_feature_likely(unsigned long feature)
>  {
> -       BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +       compiletime_assert(feature < ARM64_NCAPS,
> +                          "feature must be < ARM64_NCAPS");
>  
>         asm_volatile_goto(
>         ALTERNATIVE_CB("b       %l[l_no]", %[feature], alt_cb_patch_nops)
> @@ -238,7 +238,8 @@ alternative_has_feature_likely(unsigned long feature)
>  static __always_inline bool
>  alternative_has_feature_unlikely(unsigned long feature)
>  {
> -       BUILD_BUG_ON(feature >= ARM64_NCAPS);
> +       compiletime_assert(feature < ARM64_NCAPS,
> +                          "feature must be < ARM64_NCAPS");
>  
>         asm_volatile_goto(
>         ALTERNATIVE("nop", "b   %l[l_yes]", %[feature])
> ---->8----
> 
> ... and seems to work in a local build test (with LLVM 14.0.0 and LTO_THIN).
> 
> If that sounds ok to you I can spin that as a patch shortly.
> 
> Thanks,
> Mark.
> 
> > 
> > Cheers,
> > Nathan
> > 
> > diff --git a/arch/arm64/include/asm/alternative-likely.h b/arch/arm64/include/asm/alternative-likely.h
> > new file mode 100644
> > index 000000000000..f4e63d65e9eb
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/alternative-likely.h
> > @@ -0,0 +1,47 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __ASM_ALTERNATIVE_LIKELY_H
> > +#define __ASM_ALTERNATIVE_LIKELY_H
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <asm/alternative-macros.h>
> > +#include <linux/bug.h>
> > +#include <linux/types.h>
> > +
> > +static __always_inline bool
> > +alternative_has_feature_likely(unsigned long feature)
> > +{
> > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > +
> > +	asm_volatile_goto(
> > +	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
> > +	:
> > +	: [feature] "i" (feature)
> > +	:
> > +	: l_no);
> > +
> > +	return true;
> > +l_no:
> > +	return false;
> > +}
> > +
> > +static __always_inline bool
> > +alternative_has_feature_unlikely(unsigned long feature)
> > +{
> > +	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > +
> > +	asm_volatile_goto(
> > +	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> > +	:
> > +	: [feature] "i" (feature)
> > +	:
> > +	: l_yes);
> > +
> > +	return false;
> > +l_yes:
> > +	return true;
> > +}
> > +
> > +#endif /* __ASSEMBLY__ */
> > +
> > +#endif /* __ASM_ALTERNATIVE_LIKELY_H */
> > diff --git a/arch/arm64/include/asm/alternative-macros.h b/arch/arm64/include/asm/alternative-macros.h
> > index 4a2a98d6d222..189c31be163c 100644
> > --- a/arch/arm64/include/asm/alternative-macros.h
> > +++ b/arch/arm64/include/asm/alternative-macros.h
> > @@ -213,45 +213,4 @@ alternative_endif
> >  #define ALTERNATIVE(oldinstr, newinstr, ...)   \
> >  	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> >  
> > -#ifndef __ASSEMBLY__
> > -
> > -#include <linux/bug.h>
> > -#include <linux/types.h>
> > -
> > -static __always_inline bool
> > -alternative_has_feature_likely(unsigned long feature)
> > -{
> > -	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > -
> > -	asm_volatile_goto(
> > -	ALTERNATIVE_CB("b	%l[l_no]", %[feature], alt_cb_patch_nops)
> > -	:
> > -	: [feature] "i" (feature)
> > -	:
> > -	: l_no);
> > -
> > -	return true;
> > -l_no:
> > -	return false;
> > -}
> > -
> > -static __always_inline bool
> > -alternative_has_feature_unlikely(unsigned long feature)
> > -{
> > -	BUILD_BUG_ON(feature >= ARM64_NCAPS);
> > -
> > -	asm_volatile_goto(
> > -	ALTERNATIVE("nop", "b	%l[l_yes]", %[feature])
> > -	:
> > -	: [feature] "i" (feature)
> > -	:
> > -	: l_yes);
> > -
> > -	return false;
> > -l_yes:
> > -	return true;
> > -}
> > -
> > -#endif /* __ASSEMBLY__ */
> > -
> >  #endif /* __ASM_ALTERNATIVE_MACROS_H */
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index ba8a47433b5a..147d326c4439 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -6,7 +6,7 @@
> >  #ifndef __ASM_CPUFEATURE_H
> >  #define __ASM_CPUFEATURE_H
> >  
> > -#include <asm/alternative-macros.h>
> > +#include <asm/alternative-likely.h>
> >  #include <asm/cpucaps.h>
> >  #include <asm/cputype.h>
> >  #include <asm/hwcap.h>
> > diff --git a/arch/arm64/include/asm/lse.h b/arch/arm64/include/asm/lse.h
> > index c503db8e73b0..b28b5cb73387 100644
> > --- a/arch/arm64/include/asm/lse.h
> > +++ b/arch/arm64/include/asm/lse.h
> > @@ -13,7 +13,7 @@
> >  #include <linux/jump_label.h>
> >  #include <linux/stringify.h>
> >  #include <asm/alternative.h>
> > -#include <asm/alternative-macros.h>
> > +#include <asm/alternative-likely.h>
> >  #include <asm/atomic_lse.h>
> >  #include <asm/cpucaps.h>
> >