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

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

[Catalin Marinas]

This series is aimed to optimise 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 may not provide a clear 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).

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

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

[PATCH 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: Jason Baron <jbaron@akamai.com>
Cc: Jonathan Corbet <corbet@lwn.net>
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
 Documentation/static-keys.txt |  8 ++++++++
 include/linux/jump_label.h    | 12 ++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index 477927becacb..fd490f8b1cfa 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,12 @@ 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 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/cpuinfo.c         |  6 ++++++
 3 files changed, 20 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/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index ed1b84fe6925..6a141e399daf 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -377,6 +377,12 @@ void cpuinfo_store_cpu(void)
 void __init cpuinfo_store_boot_cpu(void)
 {
 	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
+
+	/*
+	 * Initialise the static keys early as they may be enabled by
+	 * check_local_cpu_errata() -> update_cpu_capabilities().
+	 */
+	jump_label_init();
 	__cpuinfo_store_cpu(info);
 
 	boot_cpu_data = *info;

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

[Suzuki K Poulose]

On 25/08/16 18:26, 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().
>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Suzuki K. Poulose <Suzuki.Poulose@arm.com>
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> ---


>  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/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> index ed1b84fe6925..6a141e399daf 100644
> --- a/arch/arm64/kernel/cpuinfo.c
> +++ b/arch/arm64/kernel/cpuinfo.c
> @@ -377,6 +377,12 @@ void cpuinfo_store_cpu(void)
>  void __init cpuinfo_store_boot_cpu(void)
>  {
>  	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
> +
> +	/*
> +	 * Initialise the static keys early as they may be enabled by
> +	 * check_local_cpu_errata() -> update_cpu_capabilities().
> +	 */
> +	jump_label_init();
>  	__cpuinfo_store_cpu(info);

Catalin,

Just a heads up. I have a patch [1] which moves the "check_local_cpu_errata()"
around to smp_prepare_boot_cpu(). This patch should still work fine with that
case. Only that may be we could move the jump_lable_init() to smp_prepare_boot_cpu(),
before we call "update_cpu_errata_work_arounds()" for Boot CPU.

Either way, this will be useful for some of the other feature checks.

Thanks
Suzuki

[1] https://lkml.kernel.org/r/1471525832-21209-4-git-send-email-suzuki.poulose@arm.com

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

[Catalin Marinas]

On Thu, Aug 25, 2016 at 06:26:31PM +0100, Catalin Marinas wrote:
> diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
> index 477927becacb..fd490f8b1cfa 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,12 @@ 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);

Nitpick to myself: for consistency, we need an empty line between "or:"
and "DEFINE_STATIC_KEY_ARRAY_FALSE" (I fixed it locally).

-- 
Catalin

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

[Catalin Marinas]

On Fri, Aug 26, 2016 at 10:22:13AM +0100, Suzuki K. Poulose wrote:
> On 25/08/16 18:26, Catalin Marinas wrote:
> > 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/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> >index ed1b84fe6925..6a141e399daf 100644
> >--- a/arch/arm64/kernel/cpuinfo.c
> >+++ b/arch/arm64/kernel/cpuinfo.c
> >@@ -377,6 +377,12 @@ void cpuinfo_store_cpu(void)
> > void __init cpuinfo_store_boot_cpu(void)
> > {
> > 	struct cpuinfo_arm64 *info = &per_cpu(cpu_data, 0);
> >+
> >+	/*
> >+	 * Initialise the static keys early as they may be enabled by
> >+	 * check_local_cpu_errata() -> update_cpu_capabilities().
> >+	 */
> >+	jump_label_init();
> > 	__cpuinfo_store_cpu(info);
> 
> Just a heads up. I have a patch [1] which moves the "check_local_cpu_errata()"
> around to smp_prepare_boot_cpu(). This patch should still work fine with that
> case. Only that may be we could move the jump_lable_init() to smp_prepare_boot_cpu(),
> before we call "update_cpu_errata_work_arounds()" for Boot CPU.

IIUC, we wouldn't call update_cpu_errata_work_arounds() until the CPU
feature infrastructure is initialised via cpuinfo_store_boot_cpu(). So
I don't think moving the jump_label_init() call above is necessary.

> [1] https://lkml.kernel.org/r/1471525832-21209-4-git-send-email-suzuki.poulose@arm.com

-- 
Catalin

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

[Suzuki K Poulose]

On 02/09/16 16:52, Catalin Marinas wrote:
> On Fri, Aug 26, 2016 at 10:22:13AM +0100, Suzuki K. Poulose wrote:
>> On 25/08/16 18:26, Catalin Marinas wrote:

>>
>> Just a heads up. I have a patch [1] which moves the "check_local_cpu_errata()"
>> around to smp_prepare_boot_cpu(). This patch should still work fine with that
>> case. Only that may be we could move the jump_lable_init() to smp_prepare_boot_cpu(),
>> before we call "update_cpu_errata_work_arounds()" for Boot CPU.
>
> IIUC, we wouldn't call update_cpu_errata_work_arounds() until the CPU
> feature infrastructure is initialised via cpuinfo_store_boot_cpu(). So
> I don't think moving the jump_label_init() call above is necessary.

Right, as I said, your patch should work fine even with that change. Its just that,
jump_label_init() (a generic kernel setup) can be called from a better visible
place (smp_prepare_boot_cpu()) than from a less interesting place with the patch
below.

Cheers
Suzuki


>
>> [1] https://lkml.kernel.org/r/1471525832-21209-4-git-send-email-suzuki.poulose@arm.com
>