* [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
@ 2023-11-20 10:55 ` Valentin Schneider
2023-11-20 21:38 ` kernel test robot
2023-11-20 22:47 ` kernel test robot
2023-11-20 10:55 ` [PATCH 2/5] context_tracking: Make context_tracking_key __ro_after_init Valentin Schneider
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Valentin Schneider @ 2023-11-20 10:55 UTC (permalink / raw)
To: linux-kernel, kvm, linux-arch, x86
Cc: Peter Zijlstra, Thomas Gleixner, Borislav Petkov, Josh Poimboeuf,
Pawan Gupta, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
From: Peter Zijlstra <peterz@infradead.org>
When a static_key is marked ro_after_init, its state will never change
(after init), therefore jump_label_update() will never need to iterate
the entries, and thus module load won't actually need to track this --
avoiding the static_key::next write.
Therefore, mark these keys such that jump_label_add_module() might
recognise them and avoid the modification.
Use the special state: 'static_key_linked(key) && !static_key_mod(key)'
to denote such keys.
Link: http://lore.kernel.org/r/20230705204142.GB2813335@hirez.programming.kicks-ass.net
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
include/asm-generic/sections.h | 5 ++++
include/linux/jump_label.h | 1 +
init/main.c | 1 +
kernel/jump_label.c | 49 ++++++++++++++++++++++++++++++++++
4 files changed, 56 insertions(+)
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index db13bb620f527..c768de6f19a9a 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -180,6 +180,11 @@ static inline bool is_kernel_rodata(unsigned long addr)
addr < (unsigned long)__end_rodata;
}
+static inline bool is_kernel_ro_after_init(unsigned long addr)
+{
+ return addr >= (unsigned long)__start_ro_after_init &&
+ addr < (unsigned long)__end_ro_after_init;
+}
/**
* is_kernel_inittext - checks if the pointer address is located in the
* .init.text section
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index f0a949b7c9733..88ef9e776af8d 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -216,6 +216,7 @@ extern struct jump_entry __start___jump_table[];
extern struct jump_entry __stop___jump_table[];
extern void jump_label_init(void);
+extern void jump_label_ro(void);
extern void jump_label_lock(void);
extern void jump_label_unlock(void);
extern void arch_jump_label_transform(struct jump_entry *entry,
diff --git a/init/main.c b/init/main.c
index e24b0780fdff7..5f51d8b910dc1 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1407,6 +1407,7 @@ static void mark_readonly(void)
* insecure pages which are W+X.
*/
rcu_barrier();
+ jump_label_ro();
mark_rodata_ro();
rodata_test();
} else
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index d9c822bbffb8d..661ef74dee9b7 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -530,6 +530,45 @@ void __init jump_label_init(void)
cpus_read_unlock();
}
+static inline bool static_key_sealed(struct static_key *key)
+{
+ return (key->type & JUMP_TYPE_LINKED) && !(key->type & ~JUMP_TYPE_MASK);
+}
+
+static inline void static_key_seal(struct static_key *key)
+{
+ unsigned long type = key->type & JUMP_TYPE_TRUE;
+ key->type = JUMP_TYPE_LINKED | type;
+}
+
+void jump_label_ro(void)
+{
+ struct jump_entry *iter_start = __start___jump_table;
+ struct jump_entry *iter_stop = __stop___jump_table;
+ struct jump_entry *iter;
+
+ if (WARN_ON_ONCE(!static_key_initialized))
+ return;
+
+ cpus_read_lock();
+ jump_label_lock();
+
+ for (iter = iter_start; iter < iter_stop; iter++) {
+ struct static_key *iterk = jump_entry_key(iter);
+
+ if (!is_kernel_ro_after_init((unsigned long)iterk))
+ continue;
+
+ if (static_key_sealed(iterk))
+ continue;
+
+ static_key_seal(iterk);
+ }
+
+ jump_label_unlock();
+ cpus_read_unlock();
+}
+
#ifdef CONFIG_MODULES
enum jump_label_type jump_label_init_type(struct jump_entry *entry)
@@ -650,6 +689,15 @@ static int jump_label_add_module(struct module *mod)
static_key_set_entries(key, iter);
continue;
}
+
+ /*
+ * If the key was sealed at init, then there's no need to keep a
+ * a reference to its module entries - just patch them now and
+ * be done with it.
+ */
+ if (static_key_sealed(key))
+ goto do_poke;
+
jlm = kzalloc(sizeof(struct static_key_mod), GFP_KERNEL);
if (!jlm)
return -ENOMEM;
@@ -675,6 +723,7 @@ static int jump_label_add_module(struct module *mod)
static_key_set_linked(key);
/* Only update if we've changed from our initial state */
+do_poke:
if (jump_label_type(iter) != jump_label_init_type(iter))
__jump_label_update(key, iter, iter_stop, true);
}
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
2023-11-20 10:55 ` [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys Valentin Schneider
@ 2023-11-20 21:38 ` kernel test robot
2023-11-20 22:47 ` kernel test robot
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-11-20 21:38 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kvm, linux-arch, x86
Cc: oe-kbuild-all, Peter Zijlstra, Thomas Gleixner, Borislav Petkov,
Josh Poimboeuf, Pawan Gupta, Ingo Molnar, Dave Hansen,
H. Peter Anvin, Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov,
Arnd Bergmann, Jason Baron, Steven Rostedt, Ard Biesheuvel,
Frederic Weisbecker, Paul E. McKenney, Feng Tang, Andrew Morton,
Linux Memory Management List, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
Hi Valentin,
kernel test robot noticed the following build errors:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on linus/master v6.7-rc2 next-20231120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Valentin-Schneider/jump_label-module-Don-t-alloc-static_key_mod-for-__ro_after_init-keys/20231120-190044
base: tip/x86/core
patch link: https://lore.kernel.org/r/20231120105528.760306-2-vschneid%40redhat.com
patch subject: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
config: s390-allnoconfig (https://download.01.org/0day-ci/archive/20231121/202311210541.RAdnu4yL-lkp@intel.com/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231121/202311210541.RAdnu4yL-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311210541.RAdnu4yL-lkp@intel.com/
All errors (new ones prefixed by >>):
init/main.c: In function 'mark_readonly':
>> init/main.c:1406:17: error: implicit declaration of function 'jump_label_ro'; did you mean 'jump_label_lock'? [-Werror=implicit-function-declaration]
1406 | jump_label_ro();
| ^~~~~~~~~~~~~
| jump_label_lock
cc1: some warnings being treated as errors
vim +1406 init/main.c
1394
1395 #ifdef CONFIG_STRICT_KERNEL_RWX
1396 static void mark_readonly(void)
1397 {
1398 if (rodata_enabled) {
1399 /*
1400 * load_module() results in W+X mappings, which are cleaned
1401 * up with call_rcu(). Let's make sure that queued work is
1402 * flushed so that we don't hit false positives looking for
1403 * insecure pages which are W+X.
1404 */
1405 rcu_barrier();
> 1406 jump_label_ro();
1407 mark_rodata_ro();
1408 rodata_test();
1409 } else
1410 pr_info("Kernel memory protection disabled.\n");
1411 }
1412 #elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)
1413 static inline void mark_readonly(void)
1414 {
1415 pr_warn("Kernel memory protection not selected by kernel config.\n");
1416 }
1417 #else
1418 static inline void mark_readonly(void)
1419 {
1420 pr_warn("This architecture does not have kernel memory protection.\n");
1421 }
1422 #endif
1423
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
2023-11-20 10:55 ` [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys Valentin Schneider
2023-11-20 21:38 ` kernel test robot
@ 2023-11-20 22:47 ` kernel test robot
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-11-20 22:47 UTC (permalink / raw)
To: Valentin Schneider, linux-kernel, kvm, linux-arch, x86
Cc: llvm, oe-kbuild-all, Peter Zijlstra, Thomas Gleixner,
Borislav Petkov, Josh Poimboeuf, Pawan Gupta, Ingo Molnar,
Dave Hansen, H. Peter Anvin, Paolo Bonzini, Wanpeng Li,
Vitaly Kuznetsov, Arnd Bergmann, Jason Baron, Steven Rostedt,
Ard Biesheuvel, Frederic Weisbecker, Paul E. McKenney, Feng Tang,
Andrew Morton, Linux Memory Management List, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
Hi Valentin,
kernel test robot noticed the following build errors:
[auto build test ERROR on tip/x86/core]
[also build test ERROR on linus/master v6.7-rc2 next-20231120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Valentin-Schneider/jump_label-module-Don-t-alloc-static_key_mod-for-__ro_after_init-keys/20231120-190044
base: tip/x86/core
patch link: https://lore.kernel.org/r/20231120105528.760306-2-vschneid%40redhat.com
patch subject: [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys
config: i386-allnoconfig (https://download.01.org/0day-ci/archive/20231121/202311210601.9dbatTYU-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231121/202311210601.9dbatTYU-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311210601.9dbatTYU-lkp@intel.com/
All errors (new ones prefixed by >>):
>> init/main.c:1406:3: error: call to undeclared function 'jump_label_ro'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
jump_label_ro();
^
init/main.c:1406:3: note: did you mean 'jump_label_lock'?
include/linux/jump_label.h:315:20: note: 'jump_label_lock' declared here
static inline void jump_label_lock(void) {}
^
1 error generated.
vim +/jump_label_ro +1406 init/main.c
1394
1395 #ifdef CONFIG_STRICT_KERNEL_RWX
1396 static void mark_readonly(void)
1397 {
1398 if (rodata_enabled) {
1399 /*
1400 * load_module() results in W+X mappings, which are cleaned
1401 * up with call_rcu(). Let's make sure that queued work is
1402 * flushed so that we don't hit false positives looking for
1403 * insecure pages which are W+X.
1404 */
1405 rcu_barrier();
> 1406 jump_label_ro();
1407 mark_rodata_ro();
1408 rodata_test();
1409 } else
1410 pr_info("Kernel memory protection disabled.\n");
1411 }
1412 #elif defined(CONFIG_ARCH_HAS_STRICT_KERNEL_RWX)
1413 static inline void mark_readonly(void)
1414 {
1415 pr_warn("Kernel memory protection not selected by kernel config.\n");
1416 }
1417 #else
1418 static inline void mark_readonly(void)
1419 {
1420 pr_warn("This architecture does not have kernel memory protection.\n");
1421 }
1422 #endif
1423
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/5] context_tracking: Make context_tracking_key __ro_after_init
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
2023-11-20 10:55 ` [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys Valentin Schneider
@ 2023-11-20 10:55 ` Valentin Schneider
2023-11-20 10:55 ` [PATCH 3/5] x86/kvm: Make kvm_async_pf_enabled __ro_after_init Valentin Schneider
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Valentin Schneider @ 2023-11-20 10:55 UTC (permalink / raw)
To: linux-kernel, kvm, linux-arch, x86
Cc: Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Josh Poimboeuf,
Pawan Gupta, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
context_tracking_key is only ever enabled in __init ct_cpu_tracker_user(),
so mark it as __ro_after_init.
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
kernel/context_tracking.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c
index 6ef0b35fc28c5..cc4f3a57f848c 100644
--- a/kernel/context_tracking.c
+++ b/kernel/context_tracking.c
@@ -432,7 +432,7 @@ static __always_inline void ct_kernel_enter(bool user, int offset) { }
#define CREATE_TRACE_POINTS
#include <trace/events/context_tracking.h>
-DEFINE_STATIC_KEY_FALSE(context_tracking_key);
+DEFINE_STATIC_KEY_FALSE_RO(context_tracking_key);
EXPORT_SYMBOL_GPL(context_tracking_key);
static noinstr bool context_tracking_recursion_enter(void)
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/5] x86/kvm: Make kvm_async_pf_enabled __ro_after_init
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
2023-11-20 10:55 ` [PATCH 1/5] jump_label,module: Don't alloc static_key_mod for __ro_after_init keys Valentin Schneider
2023-11-20 10:55 ` [PATCH 2/5] context_tracking: Make context_tracking_key __ro_after_init Valentin Schneider
@ 2023-11-20 10:55 ` Valentin Schneider
2023-11-27 22:20 ` Sean Christopherson
2023-11-20 10:55 ` [PATCH 4/5] x86/speculation: Make mds_user_clear __ro_after_init Valentin Schneider
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Valentin Schneider @ 2023-11-20 10:55 UTC (permalink / raw)
To: linux-kernel, kvm, linux-arch, x86
Cc: Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Josh Poimboeuf,
Pawan Gupta, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
kvm_async_pf_enabled is only ever enabled in __init kvm_guest_init(), so
mark it as __ro_after_init.
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
arch/x86/kernel/kvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 0ddb3bd0f1aac..146e16f420edf 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -44,7 +44,7 @@
#include <asm/svm.h>
#include <asm/e820/api.h>
-DEFINE_STATIC_KEY_FALSE(kvm_async_pf_enabled);
+DEFINE_STATIC_KEY_FALSE_RO(kvm_async_pf_enabled);
static int kvmapf = 1;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/5] x86/kvm: Make kvm_async_pf_enabled __ro_after_init
2023-11-20 10:55 ` [PATCH 3/5] x86/kvm: Make kvm_async_pf_enabled __ro_after_init Valentin Schneider
@ 2023-11-27 22:20 ` Sean Christopherson
0 siblings, 0 replies; 14+ messages in thread
From: Sean Christopherson @ 2023-11-27 22:20 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Pawan Gupta,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
On Mon, Nov 20, 2023, Valentin Schneider wrote:
> kvm_async_pf_enabled is only ever enabled in __init kvm_guest_init(), so
> mark it as __ro_after_init.
>
> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
> ---
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/5] x86/speculation: Make mds_user_clear __ro_after_init
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
` (2 preceding siblings ...)
2023-11-20 10:55 ` [PATCH 3/5] x86/kvm: Make kvm_async_pf_enabled __ro_after_init Valentin Schneider
@ 2023-11-20 10:55 ` Valentin Schneider
2023-11-20 10:55 ` [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init Valentin Schneider
2023-12-02 16:36 ` [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Josh Poimboeuf
5 siblings, 0 replies; 14+ messages in thread
From: Valentin Schneider @ 2023-11-20 10:55 UTC (permalink / raw)
To: linux-kernel, kvm, linux-arch, x86
Cc: Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Josh Poimboeuf,
Pawan Gupta, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
mds_user_clear is only ever enabled in:
o __init mds_select_mitigation()
o __init taa_select_mitigation()
o __init mmio_select_mitigation()
mark it as __ro_after_init.
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
arch/x86/kernel/cpu/bugs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index bb0ab8466b919..bab36096015d8 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -112,7 +112,7 @@ DEFINE_STATIC_KEY_FALSE(switch_mm_cond_ibpb);
DEFINE_STATIC_KEY_FALSE(switch_mm_always_ibpb);
/* Control MDS CPU buffer clear before returning to user space */
-DEFINE_STATIC_KEY_FALSE(mds_user_clear);
+DEFINE_STATIC_KEY_FALSE_RO(mds_user_clear);
EXPORT_SYMBOL_GPL(mds_user_clear);
/* Control MDS CPU buffer clear before idling (halt, mwait) */
DEFINE_STATIC_KEY_FALSE(mds_idle_clear);
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
` (3 preceding siblings ...)
2023-11-20 10:55 ` [PATCH 4/5] x86/speculation: Make mds_user_clear __ro_after_init Valentin Schneider
@ 2023-11-20 10:55 ` Valentin Schneider
2023-11-20 12:05 ` Peter Zijlstra
2023-12-02 16:36 ` [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Josh Poimboeuf
5 siblings, 1 reply; 14+ messages in thread
From: Valentin Schneider @ 2023-11-20 10:55 UTC (permalink / raw)
To: linux-kernel, kvm, linux-arch, x86
Cc: Thomas Gleixner, Borislav Petkov, Peter Zijlstra, Josh Poimboeuf,
Pawan Gupta, Ingo Molnar, Dave Hansen, H. Peter Anvin,
Paolo Bonzini, Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann,
Jason Baron, Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
__use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
it as __ro_after_init.
Signed-off-by: Valentin Schneider <vschneid@redhat.com>
---
arch/x86/kernel/tsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 15f97c0abc9d0..f19b42ea40573 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
static int __read_mostly tsc_unstable;
static unsigned int __initdata tsc_early_khz;
-static DEFINE_STATIC_KEY_FALSE(__use_tsc);
+static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
int tsc_clocksource_reliable;
--
2.41.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init
2023-11-20 10:55 ` [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init Valentin Schneider
@ 2023-11-20 12:05 ` Peter Zijlstra
2023-12-04 16:51 ` Valentin Schneider
0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2023-11-20 12:05 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Josh Poimboeuf, Pawan Gupta, Ingo Molnar,
Dave Hansen, H. Peter Anvin, Paolo Bonzini, Wanpeng Li,
Vitaly Kuznetsov, Arnd Bergmann, Jason Baron, Steven Rostedt,
Ard Biesheuvel, Frederic Weisbecker, Paul E. McKenney, Feng Tang,
Andrew Morton, Mike Rapoport (IBM), Vlastimil Babka,
David Hildenbrand, ndesaulniers@google.com, Michael Kelley,
Masami Hiramatsu (Google)
On Mon, Nov 20, 2023 at 11:55:28AM +0100, Valentin Schneider wrote:
> __use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
> it as __ro_after_init.
>
> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
> ---
> arch/x86/kernel/tsc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 15f97c0abc9d0..f19b42ea40573 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
> static int __read_mostly tsc_unstable;
> static unsigned int __initdata tsc_early_khz;
>
> -static DEFINE_STATIC_KEY_FALSE(__use_tsc);
> +static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
So sure, we can absolutely do that, but do we want to take this one
further perhaps? "notsc" on x86_64 makes no sense what so ever. Lets
drag things into this millennium.
---
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 15f97c0abc9d..4cfcf5946162 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -44,7 +44,9 @@ EXPORT_SYMBOL(tsc_khz);
static int __read_mostly tsc_unstable;
static unsigned int __initdata tsc_early_khz;
+#ifndef CONFIG_X86_64
static DEFINE_STATIC_KEY_FALSE(__use_tsc);
+#endif
int tsc_clocksource_reliable;
@@ -230,24 +232,26 @@ static void __init cyc2ns_init_secondary_cpus(void)
*/
noinstr u64 native_sched_clock(void)
{
- if (static_branch_likely(&__use_tsc)) {
- u64 tsc_now = rdtsc();
+#ifndef CONFIG_X86_64
+ if (!static_branch_unlikely(&__use_tsc)) {
+ /*
+ * Fall back to jiffies if there's no TSC available:
+ * ( But note that we still use it if the TSC is marked
+ * unstable. We do this because unlike Time Of Day,
+ * the scheduler clock tolerates small errors and it's
+ * very important for it to be as fast as the platform
+ * can achieve it. )
+ */
- /* return the value in ns */
- return __cycles_2_ns(tsc_now);
+ /* No locking but a rare wrong value is not a big deal: */
+ return (jiffies_64 - INITIAL_JIFFIES) * (1000000000 / HZ);
}
+#endif
- /*
- * Fall back to jiffies if there's no TSC available:
- * ( But note that we still use it if the TSC is marked
- * unstable. We do this because unlike Time Of Day,
- * the scheduler clock tolerates small errors and it's
- * very important for it to be as fast as the platform
- * can achieve it. )
- */
+ u64 tsc_now = rdtsc();
- /* No locking but a rare wrong value is not a big deal: */
- return (jiffies_64 - INITIAL_JIFFIES) * (1000000000 / HZ);
+ /* return the value in ns */
+ return __cycles_2_ns(tsc_now);
}
/*
@@ -291,7 +295,8 @@ int check_tsc_unstable(void)
}
EXPORT_SYMBOL_GPL(check_tsc_unstable);
-#ifdef CONFIG_X86_TSC
+#ifndef CONFIG_X86_64
+#if defined(CONFIG_X86_TSC
int __init notsc_setup(char *str)
{
mark_tsc_unstable("boot parameter notsc");
@@ -310,6 +315,7 @@ int __init notsc_setup(char *str)
#endif
__setup("notsc", notsc_setup);
+#endif
static int no_sched_irq_time;
static int no_tsc_watchdog;
@@ -1556,7 +1562,9 @@ static void __init tsc_enable_sched_clock(void)
/* Sanitize TSC ADJUST before cyc2ns gets initialized */
tsc_store_and_check_tsc_adjust(true);
cyc2ns_init_boot_cpu();
+#ifndef CONFIG_X86_64
static_branch_enable(&__use_tsc);
+#endif
}
void __init tsc_early_init(void)
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init
2023-11-20 12:05 ` Peter Zijlstra
@ 2023-12-04 16:51 ` Valentin Schneider
2023-12-04 18:20 ` Peter Zijlstra
0 siblings, 1 reply; 14+ messages in thread
From: Valentin Schneider @ 2023-12-04 16:51 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Josh Poimboeuf, Pawan Gupta, Ingo Molnar,
Dave Hansen, H. Peter Anvin, Paolo Bonzini, Wanpeng Li,
Vitaly Kuznetsov, Arnd Bergmann, Jason Baron, Steven Rostedt,
Ard Biesheuvel, Frederic Weisbecker, Paul E. McKenney, Feng Tang,
Andrew Morton, Mike Rapoport (IBM), Vlastimil Babka,
David Hildenbrand, ndesaulniers@google.com, Michael Kelley,
Masami Hiramatsu (Google)
On 20/11/23 13:05, Peter Zijlstra wrote:
> On Mon, Nov 20, 2023 at 11:55:28AM +0100, Valentin Schneider wrote:
>> __use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
>> it as __ro_after_init.
>>
>> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>> ---
>> arch/x86/kernel/tsc.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> index 15f97c0abc9d0..f19b42ea40573 100644
>> --- a/arch/x86/kernel/tsc.c
>> +++ b/arch/x86/kernel/tsc.c
>> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
>> static int __read_mostly tsc_unstable;
>> static unsigned int __initdata tsc_early_khz;
>>
>> -static DEFINE_STATIC_KEY_FALSE(__use_tsc);
>> +static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
>
> So sure, we can absolutely do that, but do we want to take this one
> further perhaps? "notsc" on x86_64 makes no sense what so ever. Lets
> drag things into this millennium.
>
Just to make sure I follow: currently, for the static key to be enabled, we
(mostly) need:
o X86_FEATURE_TSC is in CPUID
o determine_cpu_tsc_frequencies()->pit_hpet_ptimer_calibrate_cpu() passes
IIUC all X86_64 systems have a TSC, so the CPUID feature should be a given.
AFAICT pit_hpt_ptimer_calibrate_cpu() relies on having either HPET or the
ACPI PM timer, the latter should be widely available, though X86_PM_TIMER
can be disabled via EXPERT - is that a fringe case we don't care about, or
did I miss something? I don't really know this stuff, and I'm trying to
write a changelog...
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init
2023-12-04 16:51 ` Valentin Schneider
@ 2023-12-04 18:20 ` Peter Zijlstra
2024-01-02 15:09 ` Valentin Schneider
0 siblings, 1 reply; 14+ messages in thread
From: Peter Zijlstra @ 2023-12-04 18:20 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Josh Poimboeuf, Pawan Gupta, Ingo Molnar,
Dave Hansen, H. Peter Anvin, Paolo Bonzini, Wanpeng Li,
Vitaly Kuznetsov, Arnd Bergmann, Jason Baron, Steven Rostedt,
Ard Biesheuvel, Frederic Weisbecker, Paul E. McKenney, Feng Tang,
Andrew Morton, Mike Rapoport (IBM), Vlastimil Babka,
David Hildenbrand, ndesaulniers@google.com, Michael Kelley,
Masami Hiramatsu (Google)
On Mon, Dec 04, 2023 at 05:51:49PM +0100, Valentin Schneider wrote:
> On 20/11/23 13:05, Peter Zijlstra wrote:
> > On Mon, Nov 20, 2023 at 11:55:28AM +0100, Valentin Schneider wrote:
> >> __use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
> >> it as __ro_after_init.
> >>
> >> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
> >> ---
> >> arch/x86/kernel/tsc.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> >> index 15f97c0abc9d0..f19b42ea40573 100644
> >> --- a/arch/x86/kernel/tsc.c
> >> +++ b/arch/x86/kernel/tsc.c
> >> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
> >> static int __read_mostly tsc_unstable;
> >> static unsigned int __initdata tsc_early_khz;
> >>
> >> -static DEFINE_STATIC_KEY_FALSE(__use_tsc);
> >> +static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
> >
> > So sure, we can absolutely do that, but do we want to take this one
> > further perhaps? "notsc" on x86_64 makes no sense what so ever. Lets
> > drag things into this millennium.
> >
>
> Just to make sure I follow: currently, for the static key to be enabled, we
> (mostly) need:
> o X86_FEATURE_TSC is in CPUID
> o determine_cpu_tsc_frequencies()->pit_hpet_ptimer_calibrate_cpu() passes
>
> IIUC all X86_64 systems have a TSC, so the CPUID feature should be a given.
>
> AFAICT pit_hpt_ptimer_calibrate_cpu() relies on having either HPET or the
> ACPI PM timer, the latter should be widely available, though X86_PM_TIMER
> can be disabled via EXPERT - is that a fringe case we don't care about, or
> did I miss something? I don't really know this stuff, and I'm trying to
> write a changelog...
Ah, I was mostly just going by the fact that all of x86_64 have TSC and
disabling it makes no sense.
TSC calibration is always 'fun', but I don't know of a system where its
failure causes us to not use TSC, Thomas?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init
2023-12-04 18:20 ` Peter Zijlstra
@ 2024-01-02 15:09 ` Valentin Schneider
0 siblings, 0 replies; 14+ messages in thread
From: Valentin Schneider @ 2024-01-02 15:09 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Josh Poimboeuf, Pawan Gupta, Ingo Molnar,
Dave Hansen, H. Peter Anvin, Paolo Bonzini, Wanpeng Li,
Vitaly Kuznetsov, Arnd Bergmann, Jason Baron, Steven Rostedt,
Ard Biesheuvel, Frederic Weisbecker, Paul E. McKenney, Feng Tang,
Andrew Morton, Mike Rapoport (IBM), Vlastimil Babka,
David Hildenbrand, ndesaulniers@google.com, Michael Kelley,
Masami Hiramatsu (Google)
On 04/12/23 19:20, Peter Zijlstra wrote:
> On Mon, Dec 04, 2023 at 05:51:49PM +0100, Valentin Schneider wrote:
>> On 20/11/23 13:05, Peter Zijlstra wrote:
>> > On Mon, Nov 20, 2023 at 11:55:28AM +0100, Valentin Schneider wrote:
>> >> __use_tsc is only ever enabled in __init tsc_enable_sched_clock(), so mark
>> >> it as __ro_after_init.
>> >>
>> >> Signed-off-by: Valentin Schneider <vschneid@redhat.com>
>> >> ---
>> >> arch/x86/kernel/tsc.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> >> index 15f97c0abc9d0..f19b42ea40573 100644
>> >> --- a/arch/x86/kernel/tsc.c
>> >> +++ b/arch/x86/kernel/tsc.c
>> >> @@ -44,7 +44,7 @@ EXPORT_SYMBOL(tsc_khz);
>> >> static int __read_mostly tsc_unstable;
>> >> static unsigned int __initdata tsc_early_khz;
>> >>
>> >> -static DEFINE_STATIC_KEY_FALSE(__use_tsc);
>> >> +static DEFINE_STATIC_KEY_FALSE_RO(__use_tsc);
>> >
>> > So sure, we can absolutely do that, but do we want to take this one
>> > further perhaps? "notsc" on x86_64 makes no sense what so ever. Lets
>> > drag things into this millennium.
>> >
>>
>> Just to make sure I follow: currently, for the static key to be enabled, we
>> (mostly) need:
>> o X86_FEATURE_TSC is in CPUID
>> o determine_cpu_tsc_frequencies()->pit_hpet_ptimer_calibrate_cpu() passes
>>
>> IIUC all X86_64 systems have a TSC, so the CPUID feature should be a given.
>>
>> AFAICT pit_hpt_ptimer_calibrate_cpu() relies on having either HPET or the
>> ACPI PM timer, the latter should be widely available, though X86_PM_TIMER
>> can be disabled via EXPERT - is that a fringe case we don't care about, or
>> did I miss something? I don't really know this stuff, and I'm trying to
>> write a changelog...
>
> Ah, I was mostly just going by the fact that all of x86_64 have TSC and
> disabling it makes no sense.
>
> TSC calibration is always 'fun', but I don't know of a system where its
> failure causes us to not use TSC, Thomas?
Having another look at this, it looks like the actual requirements for the
TSC being used are either of:
o CPUID accepting 0x16 as eax input (cf. cpu_khz_from_cpuid())
o MSR_FSB_FREQ being available (cf. cpu_khz_from_msr())
o pit_hpet_ptimer_calibrate_cpu() doesn't mess up
I couldn't find any guarantees for x86_64 on having the processor frequency
information CPUID leaf, nor for the FSB_FREQ MSR (both tsc_msr_cpu_ids and
the SDM seem to point at only a handful of models).
Also for x86_64 there is this "apicpmtimer" cmdline arg which currently
disables the TSC. The commit that introduced it [1] suggests there are x86_64
systems out there with calibration issues, so now I'm not sure whether we
can kill the static key for x86_64 :(
[1]: 0c3749c41f5e ("[PATCH] x86_64: Calibrate APIC timer using PM timer")
followed by: 7fd67843b96f ("[PATCH] x86_64: Disable tsc when apicpmtimer is active")
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys
2023-11-20 10:55 [PATCH 0/5] jump_label: Fix __ro_after_init keys for modules & annotate some keys Valentin Schneider
` (4 preceding siblings ...)
2023-11-20 10:55 ` [PATCH 5/5] x86/tsc: Make __use_tsc __ro_after_init Valentin Schneider
@ 2023-12-02 16:36 ` Josh Poimboeuf
5 siblings, 0 replies; 14+ messages in thread
From: Josh Poimboeuf @ 2023-12-02 16:36 UTC (permalink / raw)
To: Valentin Schneider
Cc: linux-kernel, kvm, linux-arch, x86, Thomas Gleixner,
Borislav Petkov, Peter Zijlstra, Josh Poimboeuf, Pawan Gupta,
Ingo Molnar, Dave Hansen, H. Peter Anvin, Paolo Bonzini,
Wanpeng Li, Vitaly Kuznetsov, Arnd Bergmann, Jason Baron,
Steven Rostedt, Ard Biesheuvel, Frederic Weisbecker,
Paul E. McKenney, Feng Tang, Andrew Morton, Mike Rapoport (IBM),
Vlastimil Babka, David Hildenbrand, ndesaulniers@google.com,
Michael Kelley, Masami Hiramatsu (Google)
On Mon, Nov 20, 2023 at 11:55:23AM +0100, Valentin Schneider wrote:
> Hi folks,
>
> After chatting about deferring IPIs [1] at LPC I had another look at my patches
> and realized a handful of them could already be sent as-is.
>
> This series contains the __ro_after_init static_key bits, which fixes
> __ro_after_init keys used in modules (courtesy of PeterZ) and flags more keys as
> __ro_after_init.
>
> [1]: https://lore.kernel.org/lkml/20230720163056.2564824-1-vschneid@redhat.com/
Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
--
Josh
^ permalink raw reply [flat|nested] 14+ messages in thread