[PATCH v2 0/2] arm64: Use static keys for CPU features

[PATCH v2 0/2] arm64: Use static keys for CPU features

[Catalin Marinas]

This series is aimed at optimising the arm64 cpus_have_cap()
functionality (checking for the presence of certain CPU
capabilities/features) to avoid a bitmap test and use a jump label
instead, patched at boot time.

While this series does not provide a significant performance improvement
with the current kernel, it will be more beneficial with new features
like TTBR0 PAN are which are used on hot paths (get_user/put_user,
thread switching).

Jon, if you are happy with this patch (especially the documentation
update), are ok for it to go into mainline via the arm64 tree?

While there doesn't seem to be a specific maintainer for jump_label.*,
cc'ing Peter and Jason for comments/acks/naks.

Thanks.

Changes since v1:
- Moved the jump_label_init() call to smp_prepare_boot_cpu() following
  Suzuki's suggestion
- Fixed missing empty line in Documentation/static-keys.txt

Catalin Marinas (2):
  jump_labels: Allow array initialisers
  arm64: Use static keys for CPU features

 Documentation/static-keys.txt       |  9 +++++++++
 arch/arm64/include/asm/cpufeature.h | 14 +++++++++++---
 arch/arm64/kernel/cpufeature.c      |  3 +++
 arch/arm64/kernel/smp.c             |  5 +++++
 include/linux/jump_label.h          | 12 ++++++++++++
 5 files changed, 40 insertions(+), 3 deletions(-)

[PATCH v2 1/2] jump_labels: Allow array initialisers

[Catalin Marinas]

The static key API is currently designed around single variable
definitions. There are cases where an array of static keys is desirable,
so extend the API to allow this rather than using the internal static
key implementation directly.

Suggested-by: Dave P Martin <Dave.Martin@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jason Baron <jbaron@akamai.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/static-keys.txt |  9 +++++++++
 include/linux/jump_label.h    | 12 ++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index 477927becacb..ea8d7b4e53f0 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -15,6 +15,8 @@ The updated API replacements are:
 
 DEFINE_STATIC_KEY_TRUE(key);
 DEFINE_STATIC_KEY_FALSE(key);
+DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
+DEFINE_STATIC_KEY_ARRAY_FALSE(keys, count);
 static_branch_likely()
 static_branch_unlikely()
 
@@ -140,6 +142,13 @@ static_branch_inc(), will change the branch back to true. Likewise, if the
 key is initialized false, a 'static_branch_inc()', will change the branch to
 true. And then a 'static_branch_dec()', will again make the branch false.
 
+Where an array of keys is required, it can be defined as:
+
+	DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
+
+or:
+
+	DEFINE_STATIC_KEY_ARRAY_FALSE(keys, count);
 
 4) Architecture level code patching interface, 'jump labels'
 
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 661af564fae8..a534c7f15a61 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -21,6 +21,8 @@
  *
  * DEFINE_STATIC_KEY_TRUE(key);
  * DEFINE_STATIC_KEY_FALSE(key);
+ * DEFINE_STATIC_KEY_ARRAY_TRUE(keys, count);
+ * DEFINE_STATIC_KEY_ARRAY_FALSE(keys, count);
  * static_branch_likely()
  * static_branch_unlikely()
  *
@@ -270,6 +272,16 @@ struct static_key_false {
 #define DEFINE_STATIC_KEY_FALSE(name)	\
 	struct static_key_false name = STATIC_KEY_FALSE_INIT
 
+#define DEFINE_STATIC_KEY_ARRAY_TRUE(name, count)		\
+	struct static_key_true name[count] = {			\
+		[0 ... (count) - 1] = STATIC_KEY_TRUE_INIT,	\
+	}
+
+#define DEFINE_STATIC_KEY_ARRAY_FALSE(name, count)		\
+	struct static_key_false name[count] = {			\
+		[0 ... (count) - 1] = STATIC_KEY_FALSE_INIT,	\
+	}
+
 extern bool ____wrong_branch_error(void);
 
 #define static_key_enabled(x)							\

[PATCH v2 2/2] arm64: Use static keys for CPU features

[Catalin Marinas]

This patch adds static keys transparently for all the cpu_hwcaps
features by implementing an array of default-false static keys and
enabling them when detected. The cpus_have_cap() check uses the static
keys if the feature being checked is a constant, otherwise the compiler
generates the bitmap test.

Because of the early call to static_branch_enable() via
check_local_cpu_errata() -> update_cpu_capabilities(), the jump labels
are initialised in cpuinfo_store_boot_cpu().

Cc: Will Deacon <will.deacon@arm.com>
Cc: Suzuki K. Poulose <Suzuki.Poulose@arm.com>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 14 +++++++++++---
 arch/arm64/kernel/cpufeature.c      |  3 +++
 arch/arm64/kernel/smp.c             |  5 +++++
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 7099f26e3702..c9dfb1e4c435 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -9,6 +9,8 @@
 #ifndef __ASM_CPUFEATURE_H
 #define __ASM_CPUFEATURE_H
 
+#include <linux/jump_label.h>
+
 #include <asm/hwcap.h>
 #include <asm/sysreg.h>
 
@@ -109,6 +111,7 @@ struct arm64_cpu_capabilities {
 };
 
 extern DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
+extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
 
 bool this_cpu_has_cap(unsigned int cap);
 
@@ -121,16 +124,21 @@ static inline bool cpus_have_cap(unsigned int num)
 {
 	if (num >= ARM64_NCAPS)
 		return false;
-	return test_bit(num, cpu_hwcaps);
+	if (__builtin_constant_p(num))
+		return static_branch_unlikely(&cpu_hwcap_keys[num]);
+	else
+		return test_bit(num, cpu_hwcaps);
 }
 
 static inline void cpus_set_cap(unsigned int num)
 {
-	if (num >= ARM64_NCAPS)
+	if (num >= ARM64_NCAPS) {
 		pr_warn("Attempt to set an illegal CPU capability (%d >= %d)\n",
 			num, ARM64_NCAPS);
-	else
+	} else {
 		__set_bit(num, cpu_hwcaps);
+		static_branch_enable(&cpu_hwcap_keys[num]);
+	}
 }
 
 static inline int __attribute_const__
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 62272eac1352..919b2d0d68ae 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -46,6 +46,9 @@ unsigned int compat_elf_hwcap2 __read_mostly;
 
 DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
 
+DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS);
+EXPORT_SYMBOL(cpu_hwcap_keys);
+
 #define __ARM64_FTR_BITS(SIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
 	{						\
 		.sign = SIGNED,				\
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index d93d43352504..c3c08368a685 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -437,6 +437,11 @@ void __init smp_cpus_done(unsigned int max_cpus)
 void __init smp_prepare_boot_cpu(void)
 {
 	set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
+	/*
+	 * Initialise the static keys early as they may be enabled by the
+	 * cpufeature code.
+	 */
+	jump_label_init();
 	cpuinfo_store_boot_cpu();
 	save_boot_cpu_run_el();
 }

Re: [PATCH v2 1/2] jump_labels: Allow array initialisers

[Will Deacon]

On Mon, Sep 05, 2016 at 06:25:47PM +0100, Catalin Marinas wrote:
> The static key API is currently designed around single variable
> definitions. There are cases where an array of static keys is desirable,
> so extend the API to allow this rather than using the internal static
> key implementation directly.
> 
> Suggested-by: Dave P Martin <Dave.Martin@arm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jason Baron <jbaron@akamai.com>
> Cc: Jonathan Corbet <corbet@lwn.net>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  Documentation/static-keys.txt |  9 +++++++++
>  include/linux/jump_label.h    | 12 ++++++++++++
>  2 files changed, 21 insertions(+)

This looks pretty straightforward to me, and I'd like to take it via
the arm64 tree given that your subsequent patch depends on it.

Peter -- are you ok with this?

Cheers,

Will

Re: [PATCH v2 1/2] jump_labels: Allow array initialisers

[Peter Zijlstra]

On Tue, Sep 06, 2016 at 07:11:46PM +0100, Will Deacon wrote:
> On Mon, Sep 05, 2016 at 06:25:47PM +0100, Catalin Marinas wrote:
> > The static key API is currently designed around single variable
> > definitions. There are cases where an array of static keys is desirable,
> > so extend the API to allow this rather than using the internal static
> > key implementation directly.
> > 
> > Suggested-by: Dave P Martin <Dave.Martin@arm.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Jason Baron <jbaron@akamai.com>
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  Documentation/static-keys.txt |  9 +++++++++
> >  include/linux/jump_label.h    | 12 ++++++++++++
> >  2 files changed, 21 insertions(+)
> 
> This looks pretty straightforward to me, and I'd like to take it via
> the arm64 tree given that your subsequent patch depends on it.
> 
> Peter -- are you ok with this?

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Re: [PATCH v2 2/2] arm64: Use static keys for CPU features

[Jason Baron]

On 09/05/2016 01:25 PM, Catalin Marinas wrote:
> This patch adds static keys transparently for all the cpu_hwcaps
> features by implementing an array of default-false static keys and
> enabling them when detected. The cpus_have_cap() check uses the static
> keys if the feature being checked is a constant, otherwise the compiler
> generates the bitmap test.
>
> Because of the early call to static_branch_enable() via
> check_local_cpu_errata() -> update_cpu_capabilities(), the jump labels
> are initialised in cpuinfo_store_boot_cpu().

Was there a reason the jump_label_init() couldn't be moved
earlier in the common code?

Thanks,

-Jason

>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K. Poulose <Suzuki.Poulose@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---
>   arch/arm64/include/asm/cpufeature.h | 14 +++++++++++---
>   arch/arm64/kernel/cpufeature.c      |  3 +++
>   arch/arm64/kernel/smp.c             |  5 +++++
>   3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 7099f26e3702..c9dfb1e4c435 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -9,6 +9,8 @@
>   #ifndef __ASM_CPUFEATURE_H
>   #define __ASM_CPUFEATURE_H
>   
> +#include <linux/jump_label.h>
> +
>   #include <asm/hwcap.h>
>   #include <asm/sysreg.h>
>   
> @@ -109,6 +111,7 @@ struct arm64_cpu_capabilities {
>   };
>   
>   extern DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
> +extern struct static_key_false cpu_hwcap_keys[ARM64_NCAPS];
>   
>   bool this_cpu_has_cap(unsigned int cap);
>   
> @@ -121,16 +124,21 @@ static inline bool cpus_have_cap(unsigned int num)
>   {
>   	if (num >= ARM64_NCAPS)
>   		return false;
> -	return test_bit(num, cpu_hwcaps);
> +	if (__builtin_constant_p(num))
> +		return static_branch_unlikely(&cpu_hwcap_keys[num]);
> +	else
> +		return test_bit(num, cpu_hwcaps);
>   }
>   
>   static inline void cpus_set_cap(unsigned int num)
>   {
> -	if (num >= ARM64_NCAPS)
> +	if (num >= ARM64_NCAPS) {
>   		pr_warn("Attempt to set an illegal CPU capability (%d >= %d)\n",
>   			num, ARM64_NCAPS);
> -	else
> +	} else {
>   		__set_bit(num, cpu_hwcaps);
> +		static_branch_enable(&cpu_hwcap_keys[num]);
> +	}
>   }
>   
>   static inline int __attribute_const__
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 62272eac1352..919b2d0d68ae 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -46,6 +46,9 @@ unsigned int compat_elf_hwcap2 __read_mostly;
>   
>   DECLARE_BITMAP(cpu_hwcaps, ARM64_NCAPS);
>   
> +DEFINE_STATIC_KEY_ARRAY_FALSE(cpu_hwcap_keys, ARM64_NCAPS);
> +EXPORT_SYMBOL(cpu_hwcap_keys);
> +
>   #define __ARM64_FTR_BITS(SIGNED, STRICT, TYPE, SHIFT, WIDTH, SAFE_VAL) \
>   	{						\
>   		.sign = SIGNED,				\
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index d93d43352504..c3c08368a685 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -437,6 +437,11 @@ void __init smp_cpus_done(unsigned int max_cpus)
>   void __init smp_prepare_boot_cpu(void)
>   {
>   	set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
> +	/*
> +	 * Initialise the static keys early as they may be enabled by the
> +	 * cpufeature code.
> +	 */
> +	jump_label_init();
>   	cpuinfo_store_boot_cpu();
>   	save_boot_cpu_run_el();
>   }

Re: [PATCH v2 2/2] arm64: Use static keys for CPU features

[Catalin Marinas]

On Wed, Sep 07, 2016 at 12:59:52PM -0400, Jason Baron wrote:
> On 09/05/2016 01:25 PM, Catalin Marinas wrote:
> > This patch adds static keys transparently for all the cpu_hwcaps
> > features by implementing an array of default-false static keys and
> > enabling them when detected. The cpus_have_cap() check uses the static
> > keys if the feature being checked is a constant, otherwise the compiler
> > generates the bitmap test.
> > 
> > Because of the early call to static_branch_enable() via
> > check_local_cpu_errata() -> update_cpu_capabilities(), the jump labels
> > are initialised in cpuinfo_store_boot_cpu().
> 
> Was there a reason the jump_label_init() couldn't be moved
> earlier in the common code?

No particular reason, only that I wasn't sure what the arch requirements
to be able to initialise the jump labels early are (for example,
jump_label_init() calls arch_jump_label_transform_static(); there don't
seem to be any issues at a first look but I don't have the hardware to
test and confirm). Therefore I followed the powerpc idea of calling
jump_label_init() directly earlier.

We also don't know how early it needs to be to benefit other
architectures (powerpc seems to call it on a very early path via
early_setup()).

-- 
Catalin