* [PATCH] Add static_key_feature_checks_initialized flag
@ 2024-03-27 4:59 Nicholas Miehlbradt
2024-03-27 15:20 ` Christophe Leroy
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Miehlbradt @ 2024-03-27 4:59 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, Nicholas Miehlbradt
JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_initialized to determine
whether {cpu,mmu}_has_feature() was used before static keys were
initialized. However, {cpu,mmu}_has_feature() should not be used before
setup_feature_keys() is called. As static_key_initalized is set much
earlier during boot there is a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG
will not 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>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Add static_key_feature_checks_initialized flag
2024-03-27 4:59 [PATCH] Add static_key_feature_checks_initialized flag Nicholas Miehlbradt
@ 2024-03-27 15:20 ` Christophe Leroy
2024-04-02 1:53 ` Nicholas Miehlbradt
2024-04-02 1:57 ` Nicholas Miehlbradt
0 siblings, 2 replies; 4+ messages in thread
From: Christophe Leroy @ 2024-03-27 15:20 UTC (permalink / raw)
To: Nicholas Miehlbradt, mpe; +Cc: linuxppc-dev
Le 27/03/2024 à 05:59, Nicholas Miehlbradt a écrit :
> JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_initialized to determine
> whether {cpu,mmu}_has_feature() was used before static keys were
> initialized. However, {cpu,mmu}_has_feature() should not be used before
> setup_feature_keys() is called. As static_key_initalized is set much
> earlier during boot there is a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG
> will not report errors. Add a flag specifically to indicate when
> {cpu,mmu}_has_feature() is safe to use.
What do you mean by "much earlier" ?
As far as I can see, static_key_initialized is set by jump_label_init()
as cpu_feature_keys_init() and mmu_feature_keys_init() are call
immediately after. I don't think it is possible to do anything inbetween.
Or maybe you mean the problem is the call to jump_label_init() in
early_init_devtree() ? You should make it explicit in the message, and
see if it wouldn't be better to call cpu_feature_keys_init() and
mmu_feature_keys_init() as well in early_init_devtree() in that case ?
Christophe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Add static_key_feature_checks_initialized flag
2024-03-27 15:20 ` Christophe Leroy
@ 2024-04-02 1:53 ` Nicholas Miehlbradt
2024-04-02 1:57 ` Nicholas Miehlbradt
1 sibling, 0 replies; 4+ messages in thread
From: Nicholas Miehlbradt @ 2024-04-02 1:53 UTC (permalink / raw)
To: Christophe Leroy, mpe; +Cc: linuxppc-dev
On 28/3/2024 2:20 am, Christophe Leroy wrote:
>
>
> Le 27/03/2024 à 05:59, Nicholas Miehlbradt a écrit :
>> JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_initialized to determine
>> whether {cpu,mmu}_has_feature() was used before static keys were
>> initialized. However, {cpu,mmu}_has_feature() should not be used before
>> setup_feature_keys() is called. As static_key_initalized is set much
>> earlier during boot there is a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG
>> will not report errors. Add a flag specifically to indicate when
>> {cpu,mmu}_has_feature() is safe to use.
>
> What do you mean by "much earlier" ?
>
> As far as I can see, static_key_initialized is set by jump_label_init()
> as cpu_feature_keys_init() and mmu_feature_keys_init() are call
> immediately after. I don't think it is possible to do anything inbetween.
>
> Or maybe you mean the problem is the call to jump_label_init() in
> early_init_devtree() ? You should make it explicit in the message, and
> see if it wouldn't be better to call cpu_feature_keys_init() and
> mmu_feature_keys_init() as well in early_init_devtree() in that case ?
>
The jump_label_init() call in early_init_devtree() is exactly the issue.
I don't think it's possible to move the call to mmu_feature_keys_init()
earlier without significant refactoring since mmu features are being set
as late as setup_kup().
I'll still sent a v2 with a better worded commit message.
Nicholas
> Christophe
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Add static_key_feature_checks_initialized flag
2024-03-27 15:20 ` Christophe Leroy
2024-04-02 1:53 ` Nicholas Miehlbradt
@ 2024-04-02 1:57 ` Nicholas Miehlbradt
1 sibling, 0 replies; 4+ messages in thread
From: Nicholas Miehlbradt @ 2024-04-02 1:57 UTC (permalink / raw)
To: Christophe Leroy, mpe; +Cc: linuxppc-dev
On 28/3/2024 2:20 am, Christophe Leroy wrote:
>
>
> Le 27/03/2024 à 05:59, Nicholas Miehlbradt a écrit :
>> JUMP_LABEL_FEATURE_CHECK_DEBUG used static_key_initialized to determine
>> whether {cpu,mmu}_has_feature() was used before static keys were
>> initialized. However, {cpu,mmu}_has_feature() should not be used before
>> setup_feature_keys() is called. As static_key_initalized is set much
>> earlier during boot there is a window in which JUMP_LABEL_FEATURE_CHECK_DEBUG
>> will not report errors. Add a flag specifically to indicate when
>> {cpu,mmu}_has_feature() is safe to use.
>
> What do you mean by "much earlier" ?
>
> As far as I can see, static_key_initialized is set by jump_label_init()
> as cpu_feature_keys_init() and mmu_feature_keys_init() are call
> immediately after. I don't think it is possible to do anything inbetween.
>
> Or maybe you mean the problem is the call to jump_label_init() in
> early_init_devtree() ? You should make it explicit in the message, and
> see if it wouldn't be better to call cpu_feature_keys_init() and
> mmu_feature_keys_init() as well in early_init_devtree() in that case ?
>
The jump_label_init() call in early_init_devtree() is exactly the issue.
I don't think it's possible to move the call to mmu_feature_keys_init()
earlier without significant refactoring since mmu features are being set
as late as setup_kup().
I'll still sent a v2 with a better worded commit message.
Nicholas
> Christophe
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-02 1:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 4:59 [PATCH] Add static_key_feature_checks_initialized flag Nicholas Miehlbradt
2024-03-27 15:20 ` Christophe Leroy
2024-04-02 1:53 ` Nicholas Miehlbradt
2024-04-02 1:57 ` Nicholas Miehlbradt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).