| Message ID | 20240408052358.5030-1-nicholas@linux.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 676b2f99b0f6cd11193eeae13c976565c3fc7545 |
| Headers | show |
| Series | [v2] Add static_key_feature_checks_initialized flag | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
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
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
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)
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(-)