[PATCH v2] Add static_key_feature_checks_initialized flag

[PATCH v2] Add static_key_feature_checks_initialized flag

[Nicholas Miehlbradt]

JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_intialized to determine
whether {cpu,mmu}_has_feature() is used before static keys were
initialized. However, {cpu,mmu}_has_feature() should not be used before
setup_feature_keys() is called but static_key_initialized is set well
before this by the call to jump_label_init() in early_init_devtree().
This creates a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG will not
detect misuse and report errors. Add a flag specifically to indicate
when {cpu,mmu}_has_feature() is safe to use.

Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
---
v2: Reword commit message

v1: https://lore.kernel.org/linuxppc-dev/20240327045911.64543-1-nicholas@linux.ibm.com/
---
 arch/powerpc/include/asm/cpu_has_feature.h | 2 +-
 arch/powerpc/include/asm/feature-fixups.h  | 2 ++
 arch/powerpc/include/asm/mmu.h             | 2 +-
 arch/powerpc/lib/feature-fixups.c          | 8 ++++++++
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/cpu_has_feature.h b/arch/powerpc/include/asm/cpu_has_feature.h
index 727d4b321937..0efabccd820c 100644
--- a/arch/powerpc/include/asm/cpu_has_feature.h
+++ b/arch/powerpc/include/asm/cpu_has_feature.h
@@ -29,7 +29,7 @@ static __always_inline bool cpu_has_feature(unsigned long feature)
 #endif
 
 #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
-	if (!static_key_initialized) {
+	if (!static_key_feature_checks_initialized) {
 		printk("Warning! cpu_has_feature() used prior to jump label init!\n");
 		dump_stack();
 		return early_cpu_has_feature(feature);
diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index 77824bd289a3..17d168dd8b49 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -291,6 +291,8 @@ extern long __start___rfi_flush_fixup, __stop___rfi_flush_fixup;
 extern long __start___barrier_nospec_fixup, __stop___barrier_nospec_fixup;
 extern long __start__btb_flush_fixup, __stop__btb_flush_fixup;
 
+extern bool static_key_feature_checks_initialized;
+
 void apply_feature_fixups(void);
 void update_mmu_feature_fixups(unsigned long mask);
 void setup_feature_keys(void);
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 3b72c7ed24cf..24f830cf9bb4 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -251,7 +251,7 @@ static __always_inline bool mmu_has_feature(unsigned long feature)
 #endif
 
 #ifdef CONFIG_JUMP_LABEL_FEATURE_CHECK_DEBUG
-	if (!static_key_initialized) {
+	if (!static_key_feature_checks_initialized) {
 		printk("Warning! mmu_has_feature() used prior to jump label init!\n");
 		dump_stack();
 		return early_mmu_has_feature(feature);
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 4f82581ca203..b7201ba50b2e 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -25,6 +25,13 @@
 #include <asm/firmware.h>
 #include <asm/inst.h>
 
+/*
+ * Used to generate warnings if mmu or cpu feature check functions that
+ * use static keys before they are initialized.
+ */
+bool static_key_feature_checks_initialized __read_mostly;
+EXPORT_SYMBOL_GPL(static_key_feature_checks_initialized);
+
 struct fixup_entry {
 	unsigned long	mask;
 	unsigned long	value;
@@ -679,6 +686,7 @@ void __init setup_feature_keys(void)
 	jump_label_init();
 	cpu_feature_keys_init();
 	mmu_feature_keys_init();
+	static_key_feature_checks_initialized = true;
 }
 
 static int __init check_features(void)
-- 
2.40.1

Re: [PATCH v2] Add static_key_feature_checks_initialized flag

[Michael Ellerman]

Nicholas Miehlbradt <nicholas@linux.ibm.com> writes:
> JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_intialized to determine
> whether {cpu,mmu}_has_feature() is used before static keys were
> initialized. However, {cpu,mmu}_has_feature() should not be used before
> setup_feature_keys() is called but static_key_initialized is set well
> before this by the call to jump_label_init() in early_init_devtree().
> This creates a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG will not
> detect misuse and report errors. Add a flag specifically to indicate
> when {cpu,mmu}_has_feature() is safe to use.

I'm not sure it warrants a Fixes: tag, but it was commit
ca829e05d3d4 ("powerpc/64: Init jump labels before parse_early_param()")
which added the earlier call to jump_label_init() and created the window
where the debug checks don't fire.

cheers

Re: [PATCH v2] Add static_key_feature_checks_initialized flag

[Michael Ellerman]

On Mon, 08 Apr 2024 05:23:58 +0000, Nicholas Miehlbradt wrote:
> JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_intialized to determine
> whether {cpu,mmu}_has_feature() is used before static keys were
> initialized. However, {cpu,mmu}_has_feature() should not be used before
> setup_feature_keys() is called but static_key_initialized is set well
> before this by the call to jump_label_init() in early_init_devtree().
> This creates a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG will not
> detect misuse and report errors. Add a flag specifically to indicate
> when {cpu,mmu}_has_feature() is safe to use.
> 
> [...]

Applied to powerpc/next.

[1/1] Add static_key_feature_checks_initialized flag
      https://git.kernel.org/powerpc/c/676b2f99b0f6cd11193eeae13c976565c3fc7545

cheers