* [PATCH 01/49] x86/alternatives: Rename 'struct bp_patching_desc' to 'struct int3_patching_desc'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 02/49] x86/alternatives: Rename 'bp_refs' to 'int3_refs' Ingo Molnar
` (49 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5f448142aa99..4e932e95c744 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2471,17 +2471,17 @@ struct text_poke_loc {
u8 old;
};
-struct bp_patching_desc {
+struct int3_patching_desc {
struct text_poke_loc *vec;
int nr_entries;
};
static DEFINE_PER_CPU(atomic_t, bp_refs);
-static struct bp_patching_desc bp_desc;
+static struct int3_patching_desc bp_desc;
static __always_inline
-struct bp_patching_desc *try_get_desc(void)
+struct int3_patching_desc *try_get_desc(void)
{
atomic_t *refs = this_cpu_ptr(&bp_refs);
@@ -2517,7 +2517,7 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
noinstr int poke_int3_handler(struct pt_regs *regs)
{
- struct bp_patching_desc *desc;
+ struct int3_patching_desc *desc;
struct text_poke_loc *tp;
int ret = 0;
void *ip;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 02/49] x86/alternatives: Rename 'bp_refs' to 'int3_refs'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
2025-03-28 13:26 ` [PATCH 01/49] x86/alternatives: Rename 'struct bp_patching_desc' to 'struct int3_patching_desc' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 03/49] x86/alternatives: Rename 'text_poke_bp_batch()' to 'smp_text_poke_batch_process()' Ingo Molnar
` (48 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 4e932e95c744..cb9ac69694fb 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2476,14 +2476,14 @@ struct int3_patching_desc {
int nr_entries;
};
-static DEFINE_PER_CPU(atomic_t, bp_refs);
+static DEFINE_PER_CPU(atomic_t, int3_refs);
static struct int3_patching_desc bp_desc;
static __always_inline
struct int3_patching_desc *try_get_desc(void)
{
- atomic_t *refs = this_cpu_ptr(&bp_refs);
+ atomic_t *refs = this_cpu_ptr(&int3_refs);
if (!raw_atomic_inc_not_zero(refs))
return NULL;
@@ -2493,7 +2493,7 @@ struct int3_patching_desc *try_get_desc(void)
static __always_inline void put_desc(void)
{
- atomic_t *refs = this_cpu_ptr(&bp_refs);
+ atomic_t *refs = this_cpu_ptr(&int3_refs);
smp_mb__before_atomic();
raw_atomic_dec(refs);
@@ -2529,9 +2529,9 @@ noinstr int poke_int3_handler(struct pt_regs *regs)
* Having observed our INT3 instruction, we now must observe
* bp_desc with non-zero refcount:
*
- * bp_refs = 1 INT3
+ * int3_refs = 1 INT3
* WMB RMB
- * write INT3 if (bp_refs != 0)
+ * write INT3 if (int3_refs != 0)
*/
smp_rmb();
@@ -2638,7 +2638,7 @@ static void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_entries
* ensure reading a non-zero refcount provides up to date bp_desc data.
*/
for_each_possible_cpu(i)
- atomic_set_release(per_cpu_ptr(&bp_refs, i), 1);
+ atomic_set_release(per_cpu_ptr(&int3_refs, i), 1);
/*
* Function tracing can enable thousands of places that need to be
@@ -2760,7 +2760,7 @@ static void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_entries
* unused.
*/
for_each_possible_cpu(i) {
- atomic_t *refs = per_cpu_ptr(&bp_refs, i);
+ atomic_t *refs = per_cpu_ptr(&int3_refs, i);
if (unlikely(!atomic_dec_and_test(refs)))
atomic_cond_read_acquire(refs, !VAL);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 03/49] x86/alternatives: Rename 'text_poke_bp_batch()' to 'smp_text_poke_batch_process()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
2025-03-28 13:26 ` [PATCH 01/49] x86/alternatives: Rename 'struct bp_patching_desc' to 'struct int3_patching_desc' Ingo Molnar
2025-03-28 13:26 ` [PATCH 02/49] x86/alternatives: Rename 'bp_refs' to 'int3_refs' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 04/49] x86/alternatives: Rename 'text_poke_bp()' to 'smp_text_poke_single()' Ingo Molnar
` (47 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index cb9ac69694fb..7305c29bc8d1 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2467,7 +2467,7 @@ struct text_poke_loc {
u8 len;
u8 opcode;
const u8 text[POKE_MAX_OPCODE_SIZE];
- /* see text_poke_bp_batch() */
+ /* see smp_text_poke_batch_process() */
u8 old;
};
@@ -2540,7 +2540,7 @@ noinstr int poke_int3_handler(struct pt_regs *regs)
return 0;
/*
- * Discount the INT3. See text_poke_bp_batch().
+ * Discount the INT3. See smp_text_poke_batch_process().
*/
ip = (void *) regs->ip - INT3_INSN_SIZE;
@@ -2602,7 +2602,7 @@ static struct text_poke_loc tp_vec[TP_VEC_MAX];
static int tp_vec_nr;
/**
- * text_poke_bp_batch() -- update instructions on live kernel on SMP
+ * smp_text_poke_batch_process() -- update instructions on live kernel on SMP
* @tp: vector of instructions to patch
* @nr_entries: number of entries in the vector
*
@@ -2622,7 +2622,7 @@ static int tp_vec_nr;
* replacing opcode
* - sync cores
*/
-static void text_poke_bp_batch(struct text_poke_loc *tp, unsigned int nr_entries)
+static void smp_text_poke_batch_process(struct text_poke_loc *tp, unsigned int nr_entries)
{
unsigned char int3 = INT3_INSN_OPCODE;
unsigned int i;
@@ -2866,7 +2866,7 @@ static bool tp_order_fail(void *addr)
static void text_poke_flush(void *addr)
{
if (tp_vec_nr == TP_VEC_MAX || tp_order_fail(addr)) {
- text_poke_bp_batch(tp_vec, tp_vec_nr);
+ smp_text_poke_batch_process(tp_vec, tp_vec_nr);
tp_vec_nr = 0;
}
}
@@ -2902,5 +2902,5 @@ void __ref text_poke_bp(void *addr, const void *opcode, size_t len, const void *
struct text_poke_loc tp;
text_poke_loc_init(&tp, addr, opcode, len, emulate);
- text_poke_bp_batch(&tp, 1);
+ smp_text_poke_batch_process(&tp, 1);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 04/49] x86/alternatives: Rename 'text_poke_bp()' to 'smp_text_poke_single()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (2 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 03/49] x86/alternatives: Rename 'text_poke_bp_batch()' to 'smp_text_poke_batch_process()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 05/49] x86/alternatives: Rename 'poke_int3_handler()' to 'smp_text_poke_int3_trap_handler()' Ingo Molnar
` (46 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 4 ++--
arch/x86/kernel/ftrace.c | 8 ++++----
arch/x86/kernel/jump_label.c | 2 +-
arch/x86/kernel/kprobes/opt.c | 2 +-
arch/x86/kernel/static_call.c | 2 +-
arch/x86/net/bpf_jit_comp.c | 2 +-
7 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index ab9e143ec9fe..5189188b5e49 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -39,7 +39,7 @@ extern void *text_poke_copy(void *addr, const void *opcode, size_t len);
extern void *text_poke_copy_locked(void *addr, const void *opcode, size_t len, bool core_ok);
extern void *text_poke_set(void *addr, int c, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
-extern void text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate);
+extern void smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate);
extern void text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate);
extern void text_poke_finish(void);
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7305c29bc8d1..826c389b22eb 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2887,7 +2887,7 @@ void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const voi
}
/**
- * text_poke_bp() -- update instructions on live kernel on SMP
+ * smp_text_poke_single() -- update instructions on live kernel on SMP
* @addr: address to patch
* @opcode: opcode of new instruction
* @len: length to copy
@@ -2897,7 +2897,7 @@ void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const voi
* dynamically allocated memory. This function should be used when it is
* not possible to allocate memory.
*/
-void __ref text_poke_bp(void *addr, const void *opcode, size_t len, const void *emulate)
+void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
struct text_poke_loc tp;
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index cace6e8d7cc7..7175a0404def 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -186,11 +186,11 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ip = (unsigned long)(&ftrace_call);
new = ftrace_call_replace(ip, (unsigned long)func);
- text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
ip = (unsigned long)(&ftrace_regs_call);
new = ftrace_call_replace(ip, (unsigned long)func);
- text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
return 0;
}
@@ -492,7 +492,7 @@ void arch_ftrace_update_trampoline(struct ftrace_ops *ops)
mutex_lock(&text_mutex);
/* Do a safe modify in case the trampoline is executing */
new = ftrace_call_replace(ip, (unsigned long)func);
- text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
mutex_unlock(&text_mutex);
}
@@ -586,7 +586,7 @@ static int ftrace_mod_jmp(unsigned long ip, void *func)
const char *new;
new = ftrace_jmp_replace(ip, (unsigned long)func);
- text_poke_bp((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
return 0;
}
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index f5b8ef02d172..166e12037199 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -102,7 +102,7 @@ __jump_label_transform(struct jump_entry *entry,
return;
}
- text_poke_bp((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
+ smp_text_poke_single((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
}
static void __ref jump_label_transform(struct jump_entry *entry,
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 36d6809c6c9e..9307a40f4983 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -488,7 +488,7 @@ void arch_optimize_kprobes(struct list_head *oplist)
insn_buff[0] = JMP32_INSN_OPCODE;
*(s32 *)(&insn_buff[1]) = rel;
- text_poke_bp(op->kp.addr, insn_buff, JMP32_INSN_SIZE, NULL);
+ smp_text_poke_single(op->kp.addr, insn_buff, JMP32_INSN_SIZE, NULL);
list_del_init(&op->list);
}
diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
index 9e51242ed125..ca6c90d89407 100644
--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -108,7 +108,7 @@ static void __ref __static_call_transform(void *insn, enum insn_type type,
if (system_state == SYSTEM_BOOTING || modinit)
return text_poke_early(insn, code, size);
- text_poke_bp(insn, code, size, emulate);
+ smp_text_poke_single(insn, code, size, emulate);
}
static void __static_call_validate(u8 *insn, bool tail, bool tramp)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 72776dcb75aa..a6b650909c08 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -629,7 +629,7 @@ static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
goto out;
ret = 1;
if (memcmp(ip, new_insn, X86_PATCH_SIZE)) {
- text_poke_bp(ip, new_insn, X86_PATCH_SIZE, NULL);
+ smp_text_poke_single(ip, new_insn, X86_PATCH_SIZE, NULL);
ret = 0;
}
out:
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 05/49] x86/alternatives: Rename 'poke_int3_handler()' to 'smp_text_poke_int3_trap_handler()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (3 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 04/49] x86/alternatives: Rename 'text_poke_bp()' to 'smp_text_poke_single()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 06/49] x86/alternatives: Rename 'poking_mm' to 'text_poke_mm' Ingo Molnar
` (45 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
All related functions in this subsystem already have a
text_poke_int3_ prefix - add it to the trap handler
as well.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 2 +-
arch/x86/kernel/traps.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 5189188b5e49..42eb607c8629 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -38,7 +38,7 @@ extern void *text_poke_copy(void *addr, const void *opcode, size_t len);
#define text_poke_copy text_poke_copy
extern void *text_poke_copy_locked(void *addr, const void *opcode, size_t len, bool core_ok);
extern void *text_poke_set(void *addr, int c, size_t len);
-extern int poke_int3_handler(struct pt_regs *regs);
+extern int smp_text_poke_int3_trap_handler(struct pt_regs *regs);
extern void smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate);
extern void text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate);
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 826c389b22eb..e4c97c39eda1 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2515,7 +2515,7 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
return 0;
}
-noinstr int poke_int3_handler(struct pt_regs *regs)
+noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
struct int3_patching_desc *desc;
struct text_poke_loc *tp;
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 9f88b8a78e50..e84fdc39de6a 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -882,16 +882,16 @@ static void do_int3_user(struct pt_regs *regs)
DEFINE_IDTENTRY_RAW(exc_int3)
{
/*
- * poke_int3_handler() is completely self contained code; it does (and
+ * smp_text_poke_int3_trap_handler() is completely self contained code; it does (and
* must) *NOT* call out to anything, lest it hits upon yet another
* INT3.
*/
- if (poke_int3_handler(regs))
+ if (smp_text_poke_int3_trap_handler(regs))
return;
/*
* irqentry_enter_from_user_mode() uses static_branch_{,un}likely()
- * and therefore can trigger INT3, hence poke_int3_handler() must
+ * and therefore can trigger INT3, hence smp_text_poke_int3_trap_handler() must
* be done before. If the entry came from kernel mode, then use
* nmi_enter() because the INT3 could have been hit in any context
* including NMI.
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 06/49] x86/alternatives: Rename 'poking_mm' to 'text_poke_mm'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (4 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 05/49] x86/alternatives: Rename 'poke_int3_handler()' to 'smp_text_poke_int3_trap_handler()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 07/49] x86/alternatives: Rename 'poking_addr' to 'text_poke_mm_addr' Ingo Molnar
` (44 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Put it into the text_poke_* namespace of <asm/text-patching.h>.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 18 +++++++++---------
arch/x86/mm/init.c | 8 ++++----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 42eb607c8629..afff3d886e30 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -128,7 +128,7 @@ void *text_gen_insn(u8 opcode, const void *addr, const void *dest)
}
extern int after_bootmem;
-extern __ro_after_init struct mm_struct *poking_mm;
+extern __ro_after_init struct mm_struct *text_poke_mm;
extern __ro_after_init unsigned long poking_addr;
#ifndef CONFIG_UML_X86
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e4c97c39eda1..01e2c42e45a2 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2191,7 +2191,7 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
return temp_state;
}
-__ro_after_init struct mm_struct *poking_mm;
+__ro_after_init struct mm_struct *text_poke_mm;
__ro_after_init unsigned long poking_addr;
static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
@@ -2201,7 +2201,7 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
switch_mm_irqs_off(NULL, prev_state.mm, current);
/* Clear the cpumask, to indicate no TLB flushing is needed anywhere */
- cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(poking_mm));
+ cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(text_poke_mm));
/*
* Restore the breakpoints if they were disabled before the temporary mm
@@ -2266,7 +2266,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
/*
* The lock is not really needed, but this allows to avoid open-coding.
*/
- ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+ ptep = get_locked_pte(text_poke_mm, poking_addr, &ptl);
/*
* This must not fail; preallocated in poking_init().
@@ -2276,18 +2276,18 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
local_irq_save(flags);
pte = mk_pte(pages[0], pgprot);
- set_pte_at(poking_mm, poking_addr, ptep, pte);
+ set_pte_at(text_poke_mm, poking_addr, ptep, pte);
if (cross_page_boundary) {
pte = mk_pte(pages[1], pgprot);
- set_pte_at(poking_mm, poking_addr + PAGE_SIZE, ptep + 1, pte);
+ set_pte_at(text_poke_mm, poking_addr + PAGE_SIZE, ptep + 1, pte);
}
/*
* Loading the temporary mm behaves as a compiler barrier, which
* guarantees that the PTE will be set at the time memcpy() is done.
*/
- prev = use_temporary_mm(poking_mm);
+ prev = use_temporary_mm(text_poke_mm);
kasan_disable_current();
func((u8 *)poking_addr + offset_in_page(addr), src, len);
@@ -2299,9 +2299,9 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
*/
barrier();
- pte_clear(poking_mm, poking_addr, ptep);
+ pte_clear(text_poke_mm, poking_addr, ptep);
if (cross_page_boundary)
- pte_clear(poking_mm, poking_addr + PAGE_SIZE, ptep + 1);
+ pte_clear(text_poke_mm, poking_addr + PAGE_SIZE, ptep + 1);
/*
* Loading the previous page-table hierarchy requires a serializing
@@ -2314,7 +2314,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
* Flushing the TLB might involve IPIs, which would require enabled
* IRQs, but not if the mm is not used, as it is in this point.
*/
- flush_tlb_mm_range(poking_mm, poking_addr, poking_addr +
+ flush_tlb_mm_range(text_poke_mm, poking_addr, poking_addr +
(cross_page_boundary ? 2 : 1) * PAGE_SIZE,
PAGE_SHIFT, false);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index bfa444a7dbb0..84b52a1ebd48 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -824,11 +824,11 @@ void __init poking_init(void)
spinlock_t *ptl;
pte_t *ptep;
- poking_mm = mm_alloc();
- BUG_ON(!poking_mm);
+ text_poke_mm = mm_alloc();
+ BUG_ON(!text_poke_mm);
/* Xen PV guests need the PGD to be pinned. */
- paravirt_enter_mmap(poking_mm);
+ paravirt_enter_mmap(text_poke_mm);
/*
* Randomize the poking address, but make sure that the following page
@@ -848,7 +848,7 @@ void __init poking_init(void)
* needed for poking now. Later, poking may be performed in an atomic
* section, which might cause allocation to fail.
*/
- ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+ ptep = get_locked_pte(text_poke_mm, poking_addr, &ptl);
BUG_ON(!ptep);
pte_unmap_unlock(ptep, ptl);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 07/49] x86/alternatives: Rename 'poking_addr' to 'text_poke_mm_addr'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (5 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 06/49] x86/alternatives: Rename 'poking_mm' to 'text_poke_mm' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 08/49] x86/alternatives: Rename 'bp_desc' to 'int3_desc' Ingo Molnar
` (43 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Put it into the text_poke_* namespace of <asm/text-patching.h>.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 16 ++++++++--------
arch/x86/mm/init.c | 10 +++++-----
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index afff3d886e30..7f296a05c5fe 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -129,7 +129,7 @@ void *text_gen_insn(u8 opcode, const void *addr, const void *dest)
extern int after_bootmem;
extern __ro_after_init struct mm_struct *text_poke_mm;
-extern __ro_after_init unsigned long poking_addr;
+extern __ro_after_init unsigned long text_poke_mm_addr;
#ifndef CONFIG_UML_X86
static __always_inline
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 01e2c42e45a2..e26fa6d9f133 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2192,7 +2192,7 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
}
__ro_after_init struct mm_struct *text_poke_mm;
-__ro_after_init unsigned long poking_addr;
+__ro_after_init unsigned long text_poke_mm_addr;
static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
{
@@ -2266,7 +2266,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
/*
* The lock is not really needed, but this allows to avoid open-coding.
*/
- ptep = get_locked_pte(text_poke_mm, poking_addr, &ptl);
+ ptep = get_locked_pte(text_poke_mm, text_poke_mm_addr, &ptl);
/*
* This must not fail; preallocated in poking_init().
@@ -2276,11 +2276,11 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
local_irq_save(flags);
pte = mk_pte(pages[0], pgprot);
- set_pte_at(text_poke_mm, poking_addr, ptep, pte);
+ set_pte_at(text_poke_mm, text_poke_mm_addr, ptep, pte);
if (cross_page_boundary) {
pte = mk_pte(pages[1], pgprot);
- set_pte_at(text_poke_mm, poking_addr + PAGE_SIZE, ptep + 1, pte);
+ set_pte_at(text_poke_mm, text_poke_mm_addr + PAGE_SIZE, ptep + 1, pte);
}
/*
@@ -2290,7 +2290,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
prev = use_temporary_mm(text_poke_mm);
kasan_disable_current();
- func((u8 *)poking_addr + offset_in_page(addr), src, len);
+ func((u8 *)text_poke_mm_addr + offset_in_page(addr), src, len);
kasan_enable_current();
/*
@@ -2299,9 +2299,9 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
*/
barrier();
- pte_clear(text_poke_mm, poking_addr, ptep);
+ pte_clear(text_poke_mm, text_poke_mm_addr, ptep);
if (cross_page_boundary)
- pte_clear(text_poke_mm, poking_addr + PAGE_SIZE, ptep + 1);
+ pte_clear(text_poke_mm, text_poke_mm_addr + PAGE_SIZE, ptep + 1);
/*
* Loading the previous page-table hierarchy requires a serializing
@@ -2314,7 +2314,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
* Flushing the TLB might involve IPIs, which would require enabled
* IRQs, but not if the mm is not used, as it is in this point.
*/
- flush_tlb_mm_range(text_poke_mm, poking_addr, poking_addr +
+ flush_tlb_mm_range(text_poke_mm, text_poke_mm_addr, text_poke_mm_addr +
(cross_page_boundary ? 2 : 1) * PAGE_SIZE,
PAGE_SHIFT, false);
diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c
index 84b52a1ebd48..f8c74d19bebb 100644
--- a/arch/x86/mm/init.c
+++ b/arch/x86/mm/init.c
@@ -835,20 +835,20 @@ void __init poking_init(void)
* will be mapped at the same PMD. We need 2 pages, so find space for 3,
* and adjust the address if the PMD ends after the first one.
*/
- poking_addr = TASK_UNMAPPED_BASE;
+ text_poke_mm_addr = TASK_UNMAPPED_BASE;
if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
- poking_addr += (kaslr_get_random_long("Poking") & PAGE_MASK) %
+ text_poke_mm_addr += (kaslr_get_random_long("Poking") & PAGE_MASK) %
(TASK_SIZE - TASK_UNMAPPED_BASE - 3 * PAGE_SIZE);
- if (((poking_addr + PAGE_SIZE) & ~PMD_MASK) == 0)
- poking_addr += PAGE_SIZE;
+ if (((text_poke_mm_addr + PAGE_SIZE) & ~PMD_MASK) == 0)
+ text_poke_mm_addr += PAGE_SIZE;
/*
* We need to trigger the allocation of the page-tables that will be
* needed for poking now. Later, poking may be performed in an atomic
* section, which might cause allocation to fail.
*/
- ptep = get_locked_pte(text_poke_mm, poking_addr, &ptl);
+ ptep = get_locked_pte(text_poke_mm, text_poke_mm_addr, &ptl);
BUG_ON(!ptep);
pte_unmap_unlock(ptep, ptl);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 08/49] x86/alternatives: Rename 'bp_desc' to 'int3_desc'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (6 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 07/49] x86/alternatives: Rename 'poking_addr' to 'text_poke_mm_addr' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 09/49] x86/alternatives: Remove duplicate 'text_poke_early()' prototype Ingo Molnar
` (42 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e26fa6d9f133..4aa407af8648 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2478,7 +2478,7 @@ struct int3_patching_desc {
static DEFINE_PER_CPU(atomic_t, int3_refs);
-static struct int3_patching_desc bp_desc;
+static struct int3_patching_desc int3_desc;
static __always_inline
struct int3_patching_desc *try_get_desc(void)
@@ -2488,7 +2488,7 @@ struct int3_patching_desc *try_get_desc(void)
if (!raw_atomic_inc_not_zero(refs))
return NULL;
- return &bp_desc;
+ return &int3_desc;
}
static __always_inline void put_desc(void)
@@ -2527,7 +2527,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/*
* Having observed our INT3 instruction, we now must observe
- * bp_desc with non-zero refcount:
+ * int3_desc with non-zero refcount:
*
* int3_refs = 1 INT3
* WMB RMB
@@ -2630,12 +2630,12 @@ static void smp_text_poke_batch_process(struct text_poke_loc *tp, unsigned int n
lockdep_assert_held(&text_mutex);
- bp_desc.vec = tp;
- bp_desc.nr_entries = nr_entries;
+ int3_desc.vec = tp;
+ int3_desc.nr_entries = nr_entries;
/*
* Corresponds to the implicit memory barrier in try_get_desc() to
- * ensure reading a non-zero refcount provides up to date bp_desc data.
+ * ensure reading a non-zero refcount provides up to date int3_desc data.
*/
for_each_possible_cpu(i)
atomic_set_release(per_cpu_ptr(&int3_refs, i), 1);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 09/49] x86/alternatives: Remove duplicate 'text_poke_early()' prototype
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (7 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 08/49] x86/alternatives: Rename 'bp_desc' to 'int3_desc' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 10/49] x86/alternatives: Update comments in int3_emulate_push() Ingo Molnar
` (41 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
It's declared in <asm/text-patching.h> already.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 4aa407af8648..056872aebe6e 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -176,7 +176,6 @@ extern s32 __return_sites[], __return_sites_end[];
extern s32 __cfi_sites[], __cfi_sites_end[];
extern s32 __ibt_endbr_seal[], __ibt_endbr_seal_end[];
extern s32 __smp_locks[], __smp_locks_end[];
-void text_poke_early(void *addr, const void *opcode, size_t len);
/*
* Matches NOP and NOPL, not any of the other possible NOPs.
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 10/49] x86/alternatives: Update comments in int3_emulate_push()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (8 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 09/49] x86/alternatives: Remove duplicate 'text_poke_early()' prototype Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction Ingo Molnar
` (40 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
The idtentry macro in entry_64.S hasn't had a create_gap
option for 5 years - update the comment.
(Also clean up the entire comment block while at it.)
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 7f296a05c5fe..b7e8fb8f3648 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -142,13 +142,14 @@ static __always_inline
void int3_emulate_push(struct pt_regs *regs, unsigned long val)
{
/*
- * The int3 handler in entry_64.S adds a gap between the
+ * The INT3 handler in entry_64.S adds a gap between the
* stack where the break point happened, and the saving of
* pt_regs. We can extend the original stack because of
- * this gap. See the idtentry macro's create_gap option.
+ * this gap. See the idtentry macro's X86_TRAP_BP logic.
*
- * Similarly entry_32.S will have a gap on the stack for (any) hardware
- * exception and pt_regs; see FIXUP_FRAME.
+ * Similarly, entry_32.S will have a gap on the stack for
+ * (any) hardware exception and pt_regs; see the
+ * FIXUP_FRAME macro.
*/
regs->sp -= sizeof(unsigned long);
*(unsigned long *)regs->sp = val;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (9 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 10/49] x86/alternatives: Update comments in int3_emulate_push() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-04-01 14:36 ` Peter Zijlstra
2025-03-28 13:26 ` [PATCH 12/49] x86/alternatives: Rename 'text_poke_flush()' to 'smp_text_poke_batch_flush()' Ingo Molnar
` (39 subsequent siblings)
50 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
So the temp_mm_state_t abstraction used by use_temporary_mm() and
unuse_temporary_mm() is super confusing:
- The whole machinery is about temporarily switching to the
text_poke_mm utility MM that got allocated during bootup
for text-patching purposes alone:
temp_mm_state_t prev;
/*
* Loading the temporary mm behaves as a compiler barrier, which
* guarantees that the PTE will be set at the time memcpy() is done.
*/
prev = use_temporary_mm(text_poke_mm);
- Yet the value that gets saved in the temp_mm_state_t variable
is not the temporary MM ... but the previous MM...
- Ie. we temporarily put the non-temporary MM into a variable
that has the temp_mm_state_t type. This makes no sense whatsoever.
- The confusion continues in unuse_temporary_mm():
static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
Here we unuse an MM that is ... not the temporary MM, but the
previous MM. :-/
Fix up all this confusion by removing the unnecessary layer of
abstraction and using a bog-standard 'struct mm_struct *prev_mm'
variable to save the MM to.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 056872aebe6e..edff7714556e 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2139,10 +2139,6 @@ void __init_or_module text_poke_early(void *addr, const void *opcode,
}
}
-typedef struct {
- struct mm_struct *mm;
-} temp_mm_state_t;
-
/*
* Using a temporary mm allows to set temporary mappings that are not accessible
* by other CPUs. Such mappings are needed to perform sensitive memory writes
@@ -2156,9 +2152,9 @@ typedef struct {
* loaded, thereby preventing interrupt handler bugs from overriding
* the kernel memory protection.
*/
-static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
+static inline struct mm_struct *use_temporary_mm(struct mm_struct *temp_mm)
{
- temp_mm_state_t temp_state;
+ struct mm_struct *prev_mm;
lockdep_assert_irqs_disabled();
@@ -2170,8 +2166,8 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
if (this_cpu_read(cpu_tlbstate_shared.is_lazy))
leave_mm();
- temp_state.mm = this_cpu_read(cpu_tlbstate.loaded_mm);
- switch_mm_irqs_off(NULL, mm, current);
+ prev_mm = this_cpu_read(cpu_tlbstate.loaded_mm);
+ switch_mm_irqs_off(NULL, temp_mm, current);
/*
* If breakpoints are enabled, disable them while the temporary mm is
@@ -2187,17 +2183,17 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
if (hw_breakpoint_active())
hw_breakpoint_disable();
- return temp_state;
+ return prev_mm;
}
__ro_after_init struct mm_struct *text_poke_mm;
__ro_after_init unsigned long text_poke_mm_addr;
-static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
+static inline void unuse_temporary_mm(struct mm_struct *prev_mm)
{
lockdep_assert_irqs_disabled();
- switch_mm_irqs_off(NULL, prev_state.mm, current);
+ switch_mm_irqs_off(NULL, prev_mm, current);
/* Clear the cpumask, to indicate no TLB flushing is needed anywhere */
cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(text_poke_mm));
@@ -2228,7 +2224,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
{
bool cross_page_boundary = offset_in_page(addr) + len > PAGE_SIZE;
struct page *pages[2] = {NULL};
- temp_mm_state_t prev;
+ struct mm_struct *prev_mm;
unsigned long flags;
pte_t pte, *ptep;
spinlock_t *ptl;
@@ -2286,7 +2282,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
* Loading the temporary mm behaves as a compiler barrier, which
* guarantees that the PTE will be set at the time memcpy() is done.
*/
- prev = use_temporary_mm(text_poke_mm);
+ prev_mm = use_temporary_mm(text_poke_mm);
kasan_disable_current();
func((u8 *)text_poke_mm_addr + offset_in_page(addr), src, len);
@@ -2307,7 +2303,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
* instruction that already allows the core to see the updated version.
* Xen-PV is assumed to serialize execution in a similar manner.
*/
- unuse_temporary_mm(prev);
+ unuse_temporary_mm(prev_mm);
/*
* Flushing the TLB might involve IPIs, which would require enabled
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction
2025-03-28 13:26 ` [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction Ingo Molnar
@ 2025-04-01 14:36 ` Peter Zijlstra
2025-04-01 19:06 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-01 14:36 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Fri, Mar 28, 2025 at 02:26:26PM +0100, Ingo Molnar wrote:
> So the temp_mm_state_t abstraction used by use_temporary_mm() and
> unuse_temporary_mm() is super confusing:
I thing I see what you're saying, but we also still have these patches
pending:
https://lkml.kernel.org/r/20241119162527.952745944@infradead.org
:-(
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction
2025-04-01 14:36 ` Peter Zijlstra
@ 2025-04-01 19:06 ` Ingo Molnar
2025-04-02 4:07 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-01 19:06 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 28, 2025 at 02:26:26PM +0100, Ingo Molnar wrote:
> > So the temp_mm_state_t abstraction used by use_temporary_mm() and
> > unuse_temporary_mm() is super confusing:
>
> I thing I see what you're saying, but we also still have these patches
> pending:
>
> https://lkml.kernel.org/r/20241119162527.952745944@infradead.org
>
> :-(
Yeah, so I think we should do your queue on top of mine, the
whole temp_mm_state_t abstraction was rather nonsensical,
and we shouldn't be iterating confusing code...
I've ported patches #1 and #3 from your queue on top, see
attached below - these should be the two that represent 99%
of the conflicts between these two efforts AFAICS.
Does that work for you?
Thanks,
Ingo
================>
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 19 Nov 2024 17:25:28 +0100
Subject: [PATCH] x86/mm: Add mm argument to unuse_temporary_mm()
In commit 209954cbc7d0 ("x86/mm/tlb: Update mm_cpumask lazily")
unuse_temporary_mm() grew the assumption that it gets used on
poking_nn exclusively. While this is currently true, lets not hard
code this assumption.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20241119163035.322525475@infradead.org
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5b1a6252a4b9..cfffcb80f564 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2161,14 +2161,14 @@ static inline struct mm_struct *use_temporary_mm(struct mm_struct *temp_mm)
__ro_after_init struct mm_struct *text_poke_mm;
__ro_after_init unsigned long text_poke_mm_addr;
-static inline void unuse_temporary_mm(struct mm_struct *prev_mm)
+static inline void unuse_temporary_mm(struct mm_struct *mm, struct mm_struct *prev_mm)
{
lockdep_assert_irqs_disabled();
switch_mm_irqs_off(NULL, prev_mm, current);
/* Clear the cpumask, to indicate no TLB flushing is needed anywhere */
- cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(text_poke_mm));
+ cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(mm));
/*
* Restore the breakpoints if they were disabled before the temporary mm
@@ -2275,7 +2275,7 @@ static void *__text_poke(text_poke_f func, void *addr, const void *src, size_t l
* instruction that already allows the core to see the updated version.
* Xen-PV is assumed to serialize execution in a similar manner.
*/
- unuse_temporary_mm(prev_mm);
+ unuse_temporary_mm(text_poke_mm, prev_mm);
/*
* Flushing the TLB might involve IPIs, which would require enabled
======================================>
From: Andy Lutomirski <luto@kernel.org>
Date: Tue, 19 Nov 2024 17:25:30 +0100
Subject: [PATCH] x86/mm: Make use/unuse_temporary_mm() non-static
This prepares them for use outside of the alternative machinery.
The code is unchanged.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lkml.kernel.org/r/d1205bc7e165e249c52b7fe8cb1254f06e8a0e2a.1641659630.git.luto@kernel.org
Link: https://lore.kernel.org/r/20241119163035.533822339@infradead.org
---
arch/x86/include/asm/mmu_context.h | 3 ++
arch/x86/kernel/alternative.c | 64 --------------------------------------
arch/x86/mm/tlb.c | 64 ++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 64 deletions(-)
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 2398058b6e83..b103e1709a67 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -272,4 +272,7 @@ unsigned long __get_current_cr3_fast(void);
#include <asm-generic/mmu_context.h>
+extern struct mm_struct *use_temporary_mm(struct mm_struct *temp_mm);
+extern void unuse_temporary_mm(struct mm_struct *mm, struct mm_struct *prev_mm);
+
#endif /* _ASM_X86_MMU_CONTEXT_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index cfffcb80f564..25abadaf8751 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2111,73 +2111,9 @@ void __init_or_module text_poke_early(void *addr, const void *opcode,
}
}
-/*
- * Using a temporary mm allows to set temporary mappings that are not accessible
- * by other CPUs. Such mappings are needed to perform sensitive memory writes
- * that override the kernel memory protections (e.g., W^X), without exposing the
- * temporary page-table mappings that are required for these write operations to
- * other CPUs. Using a temporary mm also allows to avoid TLB shootdowns when the
- * mapping is torn down.
- *
- * Context: The temporary mm needs to be used exclusively by a single core. To
- * harden security IRQs must be disabled while the temporary mm is
- * loaded, thereby preventing interrupt handler bugs from overriding
- * the kernel memory protection.
- */
-static inline struct mm_struct *use_temporary_mm(struct mm_struct *temp_mm)
-{
- struct mm_struct *prev_mm;
-
- lockdep_assert_irqs_disabled();
-
- /*
- * Make sure not to be in TLB lazy mode, as otherwise we'll end up
- * with a stale address space WITHOUT being in lazy mode after
- * restoring the previous mm.
- */
- if (this_cpu_read(cpu_tlbstate_shared.is_lazy))
- leave_mm();
-
- prev_mm = this_cpu_read(cpu_tlbstate.loaded_mm);
- switch_mm_irqs_off(NULL, temp_mm, current);
-
- /*
- * If breakpoints are enabled, disable them while the temporary mm is
- * used. Userspace might set up watchpoints on addresses that are used
- * in the temporary mm, which would lead to wrong signals being sent or
- * crashes.
- *
- * Note that breakpoints are not disabled selectively, which also causes
- * kernel breakpoints (e.g., perf's) to be disabled. This might be
- * undesirable, but still seems reasonable as the code that runs in the
- * temporary mm should be short.
- */
- if (hw_breakpoint_active())
- hw_breakpoint_disable();
-
- return prev_mm;
-}
-
__ro_after_init struct mm_struct *text_poke_mm;
__ro_after_init unsigned long text_poke_mm_addr;
-static inline void unuse_temporary_mm(struct mm_struct *mm, struct mm_struct *prev_mm)
-{
- lockdep_assert_irqs_disabled();
-
- switch_mm_irqs_off(NULL, prev_mm, current);
-
- /* Clear the cpumask, to indicate no TLB flushing is needed anywhere */
- cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(mm));
-
- /*
- * Restore the breakpoints if they were disabled before the temporary mm
- * was loaded.
- */
- if (hw_breakpoint_active())
- hw_breakpoint_restore();
-}
-
static void text_poke_memcpy(void *dst, const void *src, size_t len)
{
memcpy(dst, src, len);
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 0925768d00cb..06a1ad39be74 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -972,6 +972,70 @@ void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
this_cpu_write(cpu_tlbstate_shared.is_lazy, true);
}
+/*
+ * Using a temporary mm allows to set temporary mappings that are not accessible
+ * by other CPUs. Such mappings are needed to perform sensitive memory writes
+ * that override the kernel memory protections (e.g., W^X), without exposing the
+ * temporary page-table mappings that are required for these write operations to
+ * other CPUs. Using a temporary mm also allows to avoid TLB shootdowns when the
+ * mapping is torn down.
+ *
+ * Context: The temporary mm needs to be used exclusively by a single core. To
+ * harden security IRQs must be disabled while the temporary mm is
+ * loaded, thereby preventing interrupt handler bugs from overriding
+ * the kernel memory protection.
+ */
+struct mm_struct *use_temporary_mm(struct mm_struct *temp_mm)
+{
+ struct mm_struct *prev_mm;
+
+ lockdep_assert_irqs_disabled();
+
+ /*
+ * Make sure not to be in TLB lazy mode, as otherwise we'll end up
+ * with a stale address space WITHOUT being in lazy mode after
+ * restoring the previous mm.
+ */
+ if (this_cpu_read(cpu_tlbstate_shared.is_lazy))
+ leave_mm();
+
+ prev_mm = this_cpu_read(cpu_tlbstate.loaded_mm);
+ switch_mm_irqs_off(NULL, temp_mm, current);
+
+ /*
+ * If breakpoints are enabled, disable them while the temporary mm is
+ * used. Userspace might set up watchpoints on addresses that are used
+ * in the temporary mm, which would lead to wrong signals being sent or
+ * crashes.
+ *
+ * Note that breakpoints are not disabled selectively, which also causes
+ * kernel breakpoints (e.g., perf's) to be disabled. This might be
+ * undesirable, but still seems reasonable as the code that runs in the
+ * temporary mm should be short.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_disable();
+
+ return prev_mm;
+}
+
+void unuse_temporary_mm(struct mm_struct *mm, struct mm_struct *prev_mm)
+{
+ lockdep_assert_irqs_disabled();
+
+ switch_mm_irqs_off(NULL, prev_mm, current);
+
+ /* Clear the cpumask, to indicate no TLB flushing is needed anywhere */
+ cpumask_clear_cpu(raw_smp_processor_id(), mm_cpumask(mm));
+
+ /*
+ * Restore the breakpoints if they were disabled before the temporary mm
+ * was loaded.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_restore();
+}
+
/*
* Call this when reinitializing a CPU. It fixes the following potential
* problems:
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction
2025-04-01 19:06 ` Ingo Molnar
@ 2025-04-02 4:07 ` Ingo Molnar
2025-04-02 7:54 ` Peter Zijlstra
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-02 4:07 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
* Ingo Molnar <mingo@kernel.org> wrote:
>
> * Peter Zijlstra <peterz@infradead.org> wrote:
>
> > On Fri, Mar 28, 2025 at 02:26:26PM +0100, Ingo Molnar wrote:
> > > So the temp_mm_state_t abstraction used by use_temporary_mm() and
> > > unuse_temporary_mm() is super confusing:
> >
> > I thing I see what you're saying, but we also still have these patches
> > pending:
> >
> > https://lkml.kernel.org/r/20241119162527.952745944@infradead.org
> >
> > :-(
>
> Yeah, so I think we should do your queue on top of mine, the
> whole temp_mm_state_t abstraction was rather nonsensical,
> and we shouldn't be iterating confusing code...
>
> I've ported patches #1 and #3 from your queue on top, see
> attached below - these should be the two that represent 99%
> of the conflicts between these two efforts AFAICS.
>
> Does that work for you?
To make this an easier decision, I've ported Andy's and your patches on
top of the x86/alternatives series, into WIP.x86/mm, resolving the
conflicts, with a few touchups:
git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/mm
Seems to work fine, after some light testing.
I'll send it out for another round of review if that's fine to you.
Thanks,
Ingo
==========>
git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/mm
Andy Lutomirski (5):
x86/events, x86/insn-eval: Remove incorrect current->active_mm references
x86/mm: Make use/unuse_temporary_mm() non-static
x86/mm: Allow temporary mms when IRQs are on
x86/efi: Make efi_enter/leave_mm use the *_temporary_mm() machinery
x86/mm: Opt in to IRQs-off activate_mm()
Peter Zijlstra (2):
x86/mm: Add mm argument to unuse_temporary_mm()
x86/mm: Remove 'mm' argument from unuse_temporary_mm() again
arch/x86/Kconfig | 1 +
arch/x86/events/core.c | 9 ++++-
arch/x86/include/asm/mmu_context.h | 5 ++-
arch/x86/kernel/alternative.c | 64 -----------------------------------
arch/x86/lib/insn-eval.c | 13 +++++--
arch/x86/mm/tlb.c | 69 ++++++++++++++++++++++++++++++++++++++
arch/x86/platform/efi/efi_64.c | 7 ++--
7 files changed, 94 insertions(+), 74 deletions(-)
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction
2025-04-02 4:07 ` Ingo Molnar
@ 2025-04-02 7:54 ` Peter Zijlstra
0 siblings, 0 replies; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-02 7:54 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Wed, Apr 02, 2025 at 06:07:19AM +0200, Ingo Molnar wrote:
>
> * Ingo Molnar <mingo@kernel.org> wrote:
>
> >
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > > On Fri, Mar 28, 2025 at 02:26:26PM +0100, Ingo Molnar wrote:
> > > > So the temp_mm_state_t abstraction used by use_temporary_mm() and
> > > > unuse_temporary_mm() is super confusing:
> > >
> > > I thing I see what you're saying, but we also still have these patches
> > > pending:
> > >
> > > https://lkml.kernel.org/r/20241119162527.952745944@infradead.org
> > >
> > > :-(
> >
> > Yeah, so I think we should do your queue on top of mine, the
> > whole temp_mm_state_t abstraction was rather nonsensical,
> > and we shouldn't be iterating confusing code...
> >
> > I've ported patches #1 and #3 from your queue on top, see
> > attached below - these should be the two that represent 99%
> > of the conflicts between these two efforts AFAICS.
> >
> > Does that work for you?
>
> To make this an easier decision, I've ported Andy's and your patches on
> top of the x86/alternatives series, into WIP.x86/mm, resolving the
> conflicts, with a few touchups:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/mm
>
> Seems to work fine, after some light testing.
>
> I'll send it out for another round of review if that's fine to you.
Yep, that looks right, Thanks!
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 12/49] x86/alternatives: Rename 'text_poke_flush()' to 'smp_text_poke_batch_flush()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (10 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 11/49] x86/alternatives: Remove the confusing, inaccurate & unnecessary 'temp_mm_state_t' abstraction Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 13/49] x86/alternatives: Rename 'text_poke_finish()' to 'smp_text_poke_batch_finish()' Ingo Molnar
` (38 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This name is actually actively confusing, because the simple text_poke*()
APIs use MM-switching based code patching, while text_poke_flush()
is part of the INT3 based text_poke_int3_*() machinery that is an
additional layer of functionality on top of regular text_poke*() functionality.
Rename it to smp_text_poke_batch_flush() to make it clear which layer
it belongs to.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index edff7714556e..e49c67c3942f 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2858,7 +2858,7 @@ static bool tp_order_fail(void *addr)
return false;
}
-static void text_poke_flush(void *addr)
+static void smp_text_poke_batch_flush(void *addr)
{
if (tp_vec_nr == TP_VEC_MAX || tp_order_fail(addr)) {
smp_text_poke_batch_process(tp_vec, tp_vec_nr);
@@ -2868,14 +2868,14 @@ static void text_poke_flush(void *addr)
void text_poke_finish(void)
{
- text_poke_flush(NULL);
+ smp_text_poke_batch_flush(NULL);
}
void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate)
{
struct text_poke_loc *tp;
- text_poke_flush(addr);
+ smp_text_poke_batch_flush(addr);
tp = &tp_vec[tp_vec_nr++];
text_poke_loc_init(tp, addr, opcode, len, emulate);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 13/49] x86/alternatives: Rename 'text_poke_finish()' to 'smp_text_poke_batch_finish()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (11 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 12/49] x86/alternatives: Rename 'text_poke_flush()' to 'smp_text_poke_batch_flush()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 14/49] x86/alternatives: Rename 'text_poke_queue()' to 'smp_text_poke_batch_add()' Ingo Molnar
` (37 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This name is actively confusing as well, because the simple text_poke*()
APIs use MM-switching based code patching, while text_poke_finish()
is part of the INT3 based text_poke_int3_*() machinery that is an
additional layer of functionality on top of regular text_poke*() functionality.
Rename it to smp_text_poke_batch_finish() to make it clear which layer
it belongs to.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 2 +-
arch/x86/kernel/ftrace.c | 4 ++--
arch/x86/kernel/jump_label.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index b7e8fb8f3648..6534c40b7427 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -42,7 +42,7 @@ extern int smp_text_poke_int3_trap_handler(struct pt_regs *regs);
extern void smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate);
extern void text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate);
-extern void text_poke_finish(void);
+extern void smp_text_poke_batch_finish(void);
#define INT3_INSN_SIZE 1
#define INT3_INSN_OPCODE 0xCC
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index e49c67c3942f..abe6a9fa9be7 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2866,7 +2866,7 @@ static void smp_text_poke_batch_flush(void *addr)
}
}
-void text_poke_finish(void)
+void smp_text_poke_batch_finish(void)
{
smp_text_poke_batch_flush(NULL);
}
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 7175a0404def..c35a928364b9 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -58,7 +58,7 @@ void ftrace_arch_code_modify_post_process(void)
* module load, and we need to finish the text_poke_queue()
* that they do, here.
*/
- text_poke_finish();
+ smp_text_poke_batch_finish();
ftrace_poke_late = 0;
mutex_unlock(&text_mutex);
}
@@ -250,7 +250,7 @@ void ftrace_replace_code(int enable)
text_poke_queue((void *)rec->ip, new, MCOUNT_INSN_SIZE, NULL);
ftrace_update_record(rec, enable);
}
- text_poke_finish();
+ smp_text_poke_batch_finish();
}
void arch_ftrace_update_code(int command)
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 166e12037199..28be6eb6cb3d 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -143,6 +143,6 @@ bool arch_jump_label_transform_queue(struct jump_entry *entry,
void arch_jump_label_transform_apply(void)
{
mutex_lock(&text_mutex);
- text_poke_finish();
+ smp_text_poke_batch_finish();
mutex_unlock(&text_mutex);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 14/49] x86/alternatives: Rename 'text_poke_queue()' to 'smp_text_poke_batch_add()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (12 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 13/49] x86/alternatives: Rename 'text_poke_finish()' to 'smp_text_poke_batch_finish()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 15/49] x86/alternatives: Rename 'text_poke_loc_init()' to 'text_poke_int3_loc_init()' Ingo Molnar
` (36 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This name is actively confusing as well, because the simple text_poke*()
APIs use MM-switching based code patching, while text_poke_queue()
is part of the INT3 based text_poke_int3_*() machinery that is an
additional layer of functionality on top of regular text_poke*() functionality.
Rename it to smp_text_poke_batch_add() to make it clear which layer
it belongs to.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 2 +-
arch/x86/kernel/ftrace.c | 6 +++---
arch/x86/kernel/jump_label.c | 2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 6534c40b7427..20ce4eda6d47 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -41,7 +41,7 @@ extern void *text_poke_set(void *addr, int c, size_t len);
extern int smp_text_poke_int3_trap_handler(struct pt_regs *regs);
extern void smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate);
-extern void text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate);
+extern void smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate);
extern void smp_text_poke_batch_finish(void);
#define INT3_INSN_SIZE 1
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index abe6a9fa9be7..3d395128f913 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2871,7 +2871,7 @@ void smp_text_poke_batch_finish(void)
smp_text_poke_batch_flush(NULL);
}
-void __ref text_poke_queue(void *addr, const void *opcode, size_t len, const void *emulate)
+void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
struct text_poke_loc *tp;
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index c35a928364b9..0853ba3fd04a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -55,7 +55,7 @@ void ftrace_arch_code_modify_post_process(void)
{
/*
* ftrace_make_{call,nop}() may be called during
- * module load, and we need to finish the text_poke_queue()
+ * module load, and we need to finish the smp_text_poke_batch_add()
* that they do, here.
*/
smp_text_poke_batch_finish();
@@ -119,7 +119,7 @@ ftrace_modify_code_direct(unsigned long ip, const char *old_code,
/* replace the text with the new text */
if (ftrace_poke_late)
- text_poke_queue((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_batch_add((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
else
text_poke_early((void *)ip, new_code, MCOUNT_INSN_SIZE);
return 0;
@@ -247,7 +247,7 @@ void ftrace_replace_code(int enable)
break;
}
- text_poke_queue((void *)rec->ip, new, MCOUNT_INSN_SIZE, NULL);
+ smp_text_poke_batch_add((void *)rec->ip, new, MCOUNT_INSN_SIZE, NULL);
ftrace_update_record(rec, enable);
}
smp_text_poke_batch_finish();
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index 28be6eb6cb3d..a7949a54a0ff 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -135,7 +135,7 @@ bool arch_jump_label_transform_queue(struct jump_entry *entry,
mutex_lock(&text_mutex);
jlp = __jump_label_patch(entry, type);
- text_poke_queue((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
+ smp_text_poke_batch_add((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
mutex_unlock(&text_mutex);
return true;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 15/49] x86/alternatives: Rename 'text_poke_loc_init()' to 'text_poke_int3_loc_init()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (13 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 14/49] x86/alternatives: Rename 'text_poke_queue()' to 'smp_text_poke_batch_add()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 16/49] x86/alternatives: Rename 'struct text_poke_loc' to 'struct smp_text_poke_loc' Ingo Molnar
` (35 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This name is actively confusing as well, because the simple text_poke*()
APIs use MM-switching based code patching, while text_poke_loc_init()
is part of the INT3 based text_poke_int3_*() machinery that is an
additional layer of functionality on top of regular text_poke*() functionality.
Rename it to text_poke_int3_loc_init() to make it clear which layer
it belongs to.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 3d395128f913..36c7708f40e7 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2762,7 +2762,7 @@ static void smp_text_poke_batch_process(struct text_poke_loc *tp, unsigned int n
}
}
-static void text_poke_loc_init(struct text_poke_loc *tp, void *addr,
+static void text_poke_int3_loc_init(struct text_poke_loc *tp, void *addr,
const void *opcode, size_t len, const void *emulate)
{
struct insn insn;
@@ -2878,7 +2878,7 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
smp_text_poke_batch_flush(addr);
tp = &tp_vec[tp_vec_nr++];
- text_poke_loc_init(tp, addr, opcode, len, emulate);
+ text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
}
/**
@@ -2896,6 +2896,6 @@ void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, cons
{
struct text_poke_loc tp;
- text_poke_loc_init(&tp, addr, opcode, len, emulate);
+ text_poke_int3_loc_init(&tp, addr, opcode, len, emulate);
smp_text_poke_batch_process(&tp, 1);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 16/49] x86/alternatives: Rename 'struct text_poke_loc' to 'struct smp_text_poke_loc'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (14 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 15/49] x86/alternatives: Rename 'text_poke_loc_init()' to 'text_poke_int3_loc_init()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 17/49] x86/alternatives: Rename 'struct int3_patching_desc' to 'struct text_poke_int3_vec' Ingo Molnar
` (34 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Make it clear that this structure is part of the INT3 based
SMP patching facility, not the regular text_poke*() MM-switch
based facility.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 36c7708f40e7..c42bad65bb03 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2455,7 +2455,7 @@ void text_poke_sync(void)
* this thing. When len == 6 everything is prefixed with 0x0f and we map
* opcode to Jcc.d8, using len to distinguish.
*/
-struct text_poke_loc {
+struct smp_text_poke_loc {
/* addr := _stext + rel_addr */
s32 rel_addr;
s32 disp;
@@ -2467,7 +2467,7 @@ struct text_poke_loc {
};
struct int3_patching_desc {
- struct text_poke_loc *vec;
+ struct smp_text_poke_loc *vec;
int nr_entries;
};
@@ -2494,14 +2494,14 @@ static __always_inline void put_desc(void)
raw_atomic_dec(refs);
}
-static __always_inline void *text_poke_addr(struct text_poke_loc *tp)
+static __always_inline void *text_poke_addr(struct smp_text_poke_loc *tp)
{
return _stext + tp->rel_addr;
}
static __always_inline int patch_cmp(const void *key, const void *elt)
{
- struct text_poke_loc *tp = (struct text_poke_loc *) elt;
+ struct smp_text_poke_loc *tp = (struct smp_text_poke_loc *) elt;
if (key < text_poke_addr(tp))
return -1;
@@ -2513,7 +2513,7 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
struct int3_patching_desc *desc;
- struct text_poke_loc *tp;
+ struct smp_text_poke_loc *tp;
int ret = 0;
void *ip;
@@ -2544,7 +2544,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
*/
if (unlikely(desc->nr_entries > 1)) {
tp = __inline_bsearch(ip, desc->vec, desc->nr_entries,
- sizeof(struct text_poke_loc),
+ sizeof(struct smp_text_poke_loc),
patch_cmp);
if (!tp)
goto out_put;
@@ -2592,8 +2592,8 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
return ret;
}
-#define TP_VEC_MAX (PAGE_SIZE / sizeof(struct text_poke_loc))
-static struct text_poke_loc tp_vec[TP_VEC_MAX];
+#define TP_VEC_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
+static struct smp_text_poke_loc tp_vec[TP_VEC_MAX];
static int tp_vec_nr;
/**
@@ -2617,7 +2617,7 @@ static int tp_vec_nr;
* replacing opcode
* - sync cores
*/
-static void smp_text_poke_batch_process(struct text_poke_loc *tp, unsigned int nr_entries)
+static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned int nr_entries)
{
unsigned char int3 = INT3_INSN_OPCODE;
unsigned int i;
@@ -2762,7 +2762,7 @@ static void smp_text_poke_batch_process(struct text_poke_loc *tp, unsigned int n
}
}
-static void text_poke_int3_loc_init(struct text_poke_loc *tp, void *addr,
+static void text_poke_int3_loc_init(struct smp_text_poke_loc *tp, void *addr,
const void *opcode, size_t len, const void *emulate)
{
struct insn insn;
@@ -2843,7 +2843,7 @@ static void text_poke_int3_loc_init(struct text_poke_loc *tp, void *addr,
*/
static bool tp_order_fail(void *addr)
{
- struct text_poke_loc *tp;
+ struct smp_text_poke_loc *tp;
if (!tp_vec_nr)
return false;
@@ -2873,7 +2873,7 @@ void smp_text_poke_batch_finish(void)
void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct text_poke_loc *tp;
+ struct smp_text_poke_loc *tp;
smp_text_poke_batch_flush(addr);
@@ -2894,7 +2894,7 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
*/
void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct text_poke_loc tp;
+ struct smp_text_poke_loc tp;
text_poke_int3_loc_init(&tp, addr, opcode, len, emulate);
smp_text_poke_batch_process(&tp, 1);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 17/49] x86/alternatives: Rename 'struct int3_patching_desc' to 'struct text_poke_int3_vec'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (15 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 16/49] x86/alternatives: Rename 'struct text_poke_loc' to 'struct smp_text_poke_loc' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 18/49] x86/alternatives: Rename 'int3_desc' to 'int3_vec' Ingo Molnar
` (33 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Follow the INT3 text-poking nomenclature, and also adopt the
'vector' name for the entire object, instead of the rather
opaque 'descriptor' naming.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index c42bad65bb03..db5b06ea08d0 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2466,17 +2466,17 @@ struct smp_text_poke_loc {
u8 old;
};
-struct int3_patching_desc {
+struct text_poke_int3_vec {
struct smp_text_poke_loc *vec;
int nr_entries;
};
static DEFINE_PER_CPU(atomic_t, int3_refs);
-static struct int3_patching_desc int3_desc;
+static struct text_poke_int3_vec int3_desc;
static __always_inline
-struct int3_patching_desc *try_get_desc(void)
+struct text_poke_int3_vec *try_get_desc(void)
{
atomic_t *refs = this_cpu_ptr(&int3_refs);
@@ -2512,7 +2512,7 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
- struct int3_patching_desc *desc;
+ struct text_poke_int3_vec *desc;
struct smp_text_poke_loc *tp;
int ret = 0;
void *ip;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 18/49] x86/alternatives: Rename 'int3_desc' to 'int3_vec'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (16 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 17/49] x86/alternatives: Rename 'struct int3_patching_desc' to 'struct text_poke_int3_vec' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 19/49] x86/alternatives: Add text_mutex) assert to smp_text_poke_batch_flush() Ingo Molnar
` (32 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index db5b06ea08d0..be836d1f6d99 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2473,7 +2473,7 @@ struct text_poke_int3_vec {
static DEFINE_PER_CPU(atomic_t, int3_refs);
-static struct text_poke_int3_vec int3_desc;
+static struct text_poke_int3_vec int3_vec;
static __always_inline
struct text_poke_int3_vec *try_get_desc(void)
@@ -2483,7 +2483,7 @@ struct text_poke_int3_vec *try_get_desc(void)
if (!raw_atomic_inc_not_zero(refs))
return NULL;
- return &int3_desc;
+ return &int3_vec;
}
static __always_inline void put_desc(void)
@@ -2522,7 +2522,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/*
* Having observed our INT3 instruction, we now must observe
- * int3_desc with non-zero refcount:
+ * int3_vec with non-zero refcount:
*
* int3_refs = 1 INT3
* WMB RMB
@@ -2625,12 +2625,12 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
lockdep_assert_held(&text_mutex);
- int3_desc.vec = tp;
- int3_desc.nr_entries = nr_entries;
+ int3_vec.vec = tp;
+ int3_vec.nr_entries = nr_entries;
/*
* Corresponds to the implicit memory barrier in try_get_desc() to
- * ensure reading a non-zero refcount provides up to date int3_desc data.
+ * ensure reading a non-zero refcount provides up to date int3_vec data.
*/
for_each_possible_cpu(i)
atomic_set_release(per_cpu_ptr(&int3_refs, i), 1);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 19/49] x86/alternatives: Add text_mutex) assert to smp_text_poke_batch_flush()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (17 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 18/49] x86/alternatives: Rename 'int3_desc' to 'int3_vec' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 20/49] x86/alternatives: Assert that smp_text_poke_int3_trap_handler() can only ever handle 'tp_vec[]' based requests Ingo Molnar
` (31 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
It's possible to escape the text_mutex-held assert in
smp_text_poke_batch_process() if the caller uses a properly
batched and sorted series of patch requests, so add
an explicit lockdep_assert_held() to make sure it's
held by all callers.
All text_poke_int3_*() APIs will call either smp_text_poke_batch_process()
or smp_text_poke_batch_flush() internally.
The text_mutex must be held, because tp_vec and tp_vec_nr et al
are all globals, and the INT3 patching machinery itself relies on
external serialization.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index be836d1f6d99..378db5f0b59b 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2860,6 +2860,8 @@ static bool tp_order_fail(void *addr)
static void smp_text_poke_batch_flush(void *addr)
{
+ lockdep_assert_held(&text_mutex);
+
if (tp_vec_nr == TP_VEC_MAX || tp_order_fail(addr)) {
smp_text_poke_batch_process(tp_vec, tp_vec_nr);
tp_vec_nr = 0;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 20/49] x86/alternatives: Assert that smp_text_poke_int3_trap_handler() can only ever handle 'tp_vec[]' based requests
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (18 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 19/49] x86/alternatives: Add text_mutex) assert to smp_text_poke_batch_flush() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 21/49] x86/alternatives: Use non-inverted logic instead of 'tp_order_fail()' Ingo Molnar
` (30 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 378db5f0b59b..5fe54f3c6211 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2510,6 +2510,10 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
return 0;
}
+#define TP_VEC_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
+static struct smp_text_poke_loc tp_vec[TP_VEC_MAX];
+static int tp_vec_nr;
+
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
struct text_poke_int3_vec *desc;
@@ -2534,6 +2538,8 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
if (!desc)
return 0;
+ WARN_ON_ONCE(desc->vec != tp_vec);
+
/*
* Discount the INT3. See smp_text_poke_batch_process().
*/
@@ -2592,10 +2598,6 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
return ret;
}
-#define TP_VEC_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
-static struct smp_text_poke_loc tp_vec[TP_VEC_MAX];
-static int tp_vec_nr;
-
/**
* smp_text_poke_batch_process() -- update instructions on live kernel on SMP
* @tp: vector of instructions to patch
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 21/49] x86/alternatives: Use non-inverted logic instead of 'tp_order_fail()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (19 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 20/49] x86/alternatives: Assert that smp_text_poke_int3_trap_handler() can only ever handle 'tp_vec[]' based requests Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 22/49] x86/alternatives: Remove the 'addr == NULL means forced-flush' hack from smp_text_poke_batch_finish()/smp_text_poke_batch_flush()/text_poke_addr_ordered() Ingo Molnar
` (29 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
tp_order_fail() uses inverted logic: it returns true in case something
is false, which is only a plus at the IOCCC.
Instead rename it to regular parity as 'text_poke_addr_ordered()',
and adjust the code accordingly.
Also add a comment explaining how the address ordering should be
understood.
No change in functionality intended.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5fe54f3c6211..66778dac257f 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2843,28 +2843,34 @@ static void text_poke_int3_loc_init(struct smp_text_poke_loc *tp, void *addr,
* We hard rely on the tp_vec being ordered; ensure this is so by flushing
* early if needed.
*/
-static bool tp_order_fail(void *addr)
+static bool text_poke_addr_ordered(void *addr)
{
struct smp_text_poke_loc *tp;
if (!tp_vec_nr)
- return false;
+ return true;
if (!addr) /* force */
- return true;
+ return false;
- tp = &tp_vec[tp_vec_nr - 1];
+ /*
+ * If the last current entry's address is higher than the
+ * new entry's address we'd like to add, then ordering
+ * is violated and we must first flush all pending patching
+ * requests:
+ */
+ tp = &tp_vec[tp_vec_nr-1];
if ((unsigned long)text_poke_addr(tp) > (unsigned long)addr)
- return true;
+ return false;
- return false;
+ return true;
}
static void smp_text_poke_batch_flush(void *addr)
{
lockdep_assert_held(&text_mutex);
- if (tp_vec_nr == TP_VEC_MAX || tp_order_fail(addr)) {
+ if (tp_vec_nr == TP_VEC_MAX || !text_poke_addr_ordered(addr)) {
smp_text_poke_batch_process(tp_vec, tp_vec_nr);
tp_vec_nr = 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 22/49] x86/alternatives: Remove the 'addr == NULL means forced-flush' hack from smp_text_poke_batch_finish()/smp_text_poke_batch_flush()/text_poke_addr_ordered()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (20 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 21/49] x86/alternatives: Use non-inverted logic instead of 'tp_order_fail()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs Ingo Molnar
` (28 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
There's this weird hack used by smp_text_poke_batch_finish() to indicate
a 'forced flush':
smp_text_poke_batch_flush(NULL);
Just open-code the vector-flush in a straightforward fashion:
smp_text_poke_batch_process(tp_vec, tp_vec_nr);
tp_vec_nr = 0;
And get rid of !addr hack from text_poke_addr_ordered().
Leave a WARN_ON_ONCE(), just in case some external code learned
to rely on this behavior.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 66778dac257f..ffffec4597b7 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2847,12 +2847,11 @@ static bool text_poke_addr_ordered(void *addr)
{
struct smp_text_poke_loc *tp;
+ WARN_ON_ONCE(!addr);
+
if (!tp_vec_nr)
return true;
- if (!addr) /* force */
- return false;
-
/*
* If the last current entry's address is higher than the
* new entry's address we'd like to add, then ordering
@@ -2866,6 +2865,14 @@ static bool text_poke_addr_ordered(void *addr)
return true;
}
+void smp_text_poke_batch_finish(void)
+{
+ if (tp_vec_nr) {
+ smp_text_poke_batch_process(tp_vec, tp_vec_nr);
+ tp_vec_nr = 0;
+ }
+}
+
static void smp_text_poke_batch_flush(void *addr)
{
lockdep_assert_held(&text_mutex);
@@ -2876,11 +2883,6 @@ static void smp_text_poke_batch_flush(void *addr)
}
}
-void smp_text_poke_batch_finish(void)
-{
- smp_text_poke_batch_flush(NULL);
-}
-
void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
struct smp_text_poke_loc *tp;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (21 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 22/49] x86/alternatives: Remove the 'addr == NULL means forced-flush' hack from smp_text_poke_batch_finish()/smp_text_poke_batch_flush()/text_poke_addr_ordered() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-04-01 14:40 ` Peter Zijlstra
2025-03-28 13:26 ` [PATCH 24/49] x86/alternatives: Assert input parameters in smp_text_poke_batch_process() Ingo Molnar
` (27 subsequent siblings)
50 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Instead of constructing a vector on-stack, just use the already
available batch-patching vector - which should always be empty
at this point.
This will allow subsequent simplifications.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index ffffec4597b7..70abc636b87c 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2906,8 +2906,13 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
*/
void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct smp_text_poke_loc tp;
+ struct smp_text_poke_loc *tp;
+
+ /* Batch-patching should not be mixed with single-patching: */
+ WARN_ON_ONCE(tp_vec_nr != 0);
+
+ tp = &tp_vec[tp_vec_nr++];
+ text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
- text_poke_int3_loc_init(&tp, addr, opcode, len, emulate);
- smp_text_poke_batch_process(&tp, 1);
+ smp_text_poke_batch_finish();
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs
2025-03-28 13:26 ` [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs Ingo Molnar
@ 2025-04-01 14:40 ` Peter Zijlstra
2025-04-01 18:49 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-01 14:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Fri, Mar 28, 2025 at 02:26:38PM +0100, Ingo Molnar wrote:
> Instead of constructing a vector on-stack, just use the already
> available batch-patching vector - which should always be empty
> at this point.
>
> This will allow subsequent simplifications.
This should go before patch 20, otherwise you'll have this intermediate
state where we trip the WARN, no?
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> arch/x86/kernel/alternative.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index ffffec4597b7..70abc636b87c 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -2906,8 +2906,13 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
> */
> void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
> {
> - struct smp_text_poke_loc tp;
> + struct smp_text_poke_loc *tp;
> +
> + /* Batch-patching should not be mixed with single-patching: */
> + WARN_ON_ONCE(tp_vec_nr != 0);
> +
> + tp = &tp_vec[tp_vec_nr++];
> + text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
>
> - text_poke_int3_loc_init(&tp, addr, opcode, len, emulate);
> - smp_text_poke_batch_process(&tp, 1);
> + smp_text_poke_batch_finish();
> }
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs
2025-04-01 14:40 ` Peter Zijlstra
@ 2025-04-01 18:49 ` Ingo Molnar
0 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-04-01 18:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 28, 2025 at 02:26:38PM +0100, Ingo Molnar wrote:
> > Instead of constructing a vector on-stack, just use the already
> > available batch-patching vector - which should always be empty
> > at this point.
> >
> > This will allow subsequent simplifications.
>
> This should go before patch 20, otherwise you'll have this intermediate
> state where we trip the WARN, no?
Yeah, indeed - I've shuffled them around in the latest
WIP.x86/alternatives.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 24/49] x86/alternatives: Assert input parameters in smp_text_poke_batch_process()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (22 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 23/49] x86/alternatives: Simplify smp_text_poke_single() by using tp_vec and existing APIs Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 25/49] x86/alternatives: Introduce 'struct smp_text_poke_array' and move tp_vec and tp_vec_nr to it Ingo Molnar
` (26 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
At this point the 'tp' input parameter must always be the
global 'tp_vec' array, and 'nr_entries' must always be equal
to 'tp_vec_nr'.
Assert these conditions - which will allow the removal of
a layer of indirection between these values.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 70abc636b87c..6652863539e8 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2627,6 +2627,9 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
lockdep_assert_held(&text_mutex);
+ WARN_ON_ONCE(tp != tp_vec);
+ WARN_ON_ONCE(nr_entries != tp_vec_nr);
+
int3_vec.vec = tp;
int3_vec.nr_entries = nr_entries;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 25/49] x86/alternatives: Introduce 'struct smp_text_poke_array' and move tp_vec and tp_vec_nr to it
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (23 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 24/49] x86/alternatives: Assert input parameters in smp_text_poke_batch_process() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 26/49] x86/alternatives: Remove the tp_vec indirection Ingo Molnar
` (25 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
struct text_poke_array is an equivalent structure to these global variables:
static struct smp_text_poke_loc tp_vec[TP_VEC_MAX];
static int tp_vec_nr;
Note that we intentionally mirror much of the naming of
'struct text_poke_int3_vec', which will further highlight
the unecessary layering going on in this code, and will
ease its removal.
No change in functionality.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 43 +++++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 6652863539e8..b4b5e4db8cf9 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2467,14 +2467,21 @@ struct smp_text_poke_loc {
};
struct text_poke_int3_vec {
- struct smp_text_poke_loc *vec;
int nr_entries;
+ struct smp_text_poke_loc *vec;
};
static DEFINE_PER_CPU(atomic_t, int3_refs);
static struct text_poke_int3_vec int3_vec;
+#define TP_ARRAY_NR_ENTRIES_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
+
+static struct smp_text_poke_array {
+ int nr_entries;
+ struct smp_text_poke_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
+} text_poke_array;
+
static __always_inline
struct text_poke_int3_vec *try_get_desc(void)
{
@@ -2510,10 +2517,6 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
return 0;
}
-#define TP_VEC_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
-static struct smp_text_poke_loc tp_vec[TP_VEC_MAX];
-static int tp_vec_nr;
-
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
struct text_poke_int3_vec *desc;
@@ -2538,7 +2541,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
if (!desc)
return 0;
- WARN_ON_ONCE(desc->vec != tp_vec);
+ WARN_ON_ONCE(desc->vec != text_poke_array.vec);
/*
* Discount the INT3. See smp_text_poke_batch_process().
@@ -2627,8 +2630,8 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
lockdep_assert_held(&text_mutex);
- WARN_ON_ONCE(tp != tp_vec);
- WARN_ON_ONCE(nr_entries != tp_vec_nr);
+ WARN_ON_ONCE(tp != text_poke_array.vec);
+ WARN_ON_ONCE(nr_entries != text_poke_array.nr_entries);
int3_vec.vec = tp;
int3_vec.nr_entries = nr_entries;
@@ -2843,7 +2846,7 @@ static void text_poke_int3_loc_init(struct smp_text_poke_loc *tp, void *addr,
}
/*
- * We hard rely on the tp_vec being ordered; ensure this is so by flushing
+ * We hard rely on the text_poke_array.vec being ordered; ensure this is so by flushing
* early if needed.
*/
static bool text_poke_addr_ordered(void *addr)
@@ -2852,7 +2855,7 @@ static bool text_poke_addr_ordered(void *addr)
WARN_ON_ONCE(!addr);
- if (!tp_vec_nr)
+ if (!text_poke_array.nr_entries)
return true;
/*
@@ -2861,7 +2864,7 @@ static bool text_poke_addr_ordered(void *addr)
* is violated and we must first flush all pending patching
* requests:
*/
- tp = &tp_vec[tp_vec_nr-1];
+ tp = &text_poke_array.vec[text_poke_array.nr_entries-1];
if ((unsigned long)text_poke_addr(tp) > (unsigned long)addr)
return false;
@@ -2870,9 +2873,9 @@ static bool text_poke_addr_ordered(void *addr)
void smp_text_poke_batch_finish(void)
{
- if (tp_vec_nr) {
- smp_text_poke_batch_process(tp_vec, tp_vec_nr);
- tp_vec_nr = 0;
+ if (text_poke_array.nr_entries) {
+ smp_text_poke_batch_process(text_poke_array.vec, text_poke_array.nr_entries);
+ text_poke_array.nr_entries = 0;
}
}
@@ -2880,9 +2883,9 @@ static void smp_text_poke_batch_flush(void *addr)
{
lockdep_assert_held(&text_mutex);
- if (tp_vec_nr == TP_VEC_MAX || !text_poke_addr_ordered(addr)) {
- smp_text_poke_batch_process(tp_vec, tp_vec_nr);
- tp_vec_nr = 0;
+ if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr)) {
+ smp_text_poke_batch_process(text_poke_array.vec, text_poke_array.nr_entries);
+ text_poke_array.nr_entries = 0;
}
}
@@ -2892,7 +2895,7 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
smp_text_poke_batch_flush(addr);
- tp = &tp_vec[tp_vec_nr++];
+ tp = &text_poke_array.vec[text_poke_array.nr_entries++];
text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
}
@@ -2912,9 +2915,9 @@ void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, cons
struct smp_text_poke_loc *tp;
/* Batch-patching should not be mixed with single-patching: */
- WARN_ON_ONCE(tp_vec_nr != 0);
+ WARN_ON_ONCE(text_poke_array.nr_entries != 0);
- tp = &tp_vec[tp_vec_nr++];
+ tp = &text_poke_array.vec[text_poke_array.nr_entries++];
text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
smp_text_poke_batch_finish();
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 26/49] x86/alternatives: Remove the tp_vec indirection
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (24 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 25/49] x86/alternatives: Introduce 'struct smp_text_poke_array' and move tp_vec and tp_vec_nr to it Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 27/49] x86/alternatives: Rename 'try_get_desc()' to 'try_get_text_poke_array()' Ingo Molnar
` (24 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
At this point we are always working out of an uptodate
text_poke_array, there's no need for smp_text_poke_int3_trap_handler()
to read via the int3_vec indirection - remove it.
This simplifies the code:
1 file changed, 5 insertions(+), 15 deletions(-)
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index b4b5e4db8cf9..82808deb1501 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2466,15 +2466,6 @@ struct smp_text_poke_loc {
u8 old;
};
-struct text_poke_int3_vec {
- int nr_entries;
- struct smp_text_poke_loc *vec;
-};
-
-static DEFINE_PER_CPU(atomic_t, int3_refs);
-
-static struct text_poke_int3_vec int3_vec;
-
#define TP_ARRAY_NR_ENTRIES_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
static struct smp_text_poke_array {
@@ -2482,15 +2473,17 @@ static struct smp_text_poke_array {
struct smp_text_poke_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
} text_poke_array;
+static DEFINE_PER_CPU(atomic_t, int3_refs);
+
static __always_inline
-struct text_poke_int3_vec *try_get_desc(void)
+struct smp_text_poke_array *try_get_desc(void)
{
atomic_t *refs = this_cpu_ptr(&int3_refs);
if (!raw_atomic_inc_not_zero(refs))
return NULL;
- return &int3_vec;
+ return &text_poke_array;
}
static __always_inline void put_desc(void)
@@ -2519,7 +2512,7 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
- struct text_poke_int3_vec *desc;
+ struct smp_text_poke_array *desc;
struct smp_text_poke_loc *tp;
int ret = 0;
void *ip;
@@ -2529,7 +2522,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/*
* Having observed our INT3 instruction, we now must observe
- * int3_vec with non-zero refcount:
+ * text_poke_array with non-zero refcount:
*
* int3_refs = 1 INT3
* WMB RMB
@@ -2633,12 +2626,9 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
WARN_ON_ONCE(tp != text_poke_array.vec);
WARN_ON_ONCE(nr_entries != text_poke_array.nr_entries);
- int3_vec.vec = tp;
- int3_vec.nr_entries = nr_entries;
-
/*
* Corresponds to the implicit memory barrier in try_get_desc() to
- * ensure reading a non-zero refcount provides up to date int3_vec data.
+ * ensure reading a non-zero refcount provides up to date text_poke_array data.
*/
for_each_possible_cpu(i)
atomic_set_release(per_cpu_ptr(&int3_refs, i), 1);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 27/49] x86/alternatives: Rename 'try_get_desc()' to 'try_get_text_poke_array()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (25 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 26/49] x86/alternatives: Remove the tp_vec indirection Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 28/49] x86/alternatives: Rename 'put_desc()' to 'put_text_poke_array()' Ingo Molnar
` (23 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This better reflects what the underlying code is doing,
there's no 'descriptor' indirection anymore.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 82808deb1501..26bb2a731208 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2476,7 +2476,7 @@ static struct smp_text_poke_array {
static DEFINE_PER_CPU(atomic_t, int3_refs);
static __always_inline
-struct smp_text_poke_array *try_get_desc(void)
+struct smp_text_poke_array *try_get_text_poke_array(void)
{
atomic_t *refs = this_cpu_ptr(&int3_refs);
@@ -2530,7 +2530,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
*/
smp_rmb();
- desc = try_get_desc();
+ desc = try_get_text_poke_array();
if (!desc)
return 0;
@@ -2627,7 +2627,7 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
WARN_ON_ONCE(nr_entries != text_poke_array.nr_entries);
/*
- * Corresponds to the implicit memory barrier in try_get_desc() to
+ * Corresponds to the implicit memory barrier in try_get_text_poke_array() to
* ensure reading a non-zero refcount provides up to date text_poke_array data.
*/
for_each_possible_cpu(i)
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 28/49] x86/alternatives: Rename 'put_desc()' to 'put_text_poke_array()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (26 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 27/49] x86/alternatives: Rename 'try_get_desc()' to 'try_get_text_poke_array()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 29/49] x86/alternatives: Simplify try_get_text_poke_array() Ingo Molnar
` (22 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Just like with try_get_text_poke_array(), this name better reflects
what the underlying code is doing, there's no 'descriptor'
indirection anymore.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 26bb2a731208..202720228c98 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2486,7 +2486,7 @@ struct smp_text_poke_array *try_get_text_poke_array(void)
return &text_poke_array;
}
-static __always_inline void put_desc(void)
+static __always_inline void put_text_poke_array(void)
{
atomic_t *refs = this_cpu_ptr(&int3_refs);
@@ -2590,7 +2590,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
ret = 1;
out_put:
- put_desc();
+ put_text_poke_array();
return ret;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 29/49] x86/alternatives: Simplify try_get_text_poke_array()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (27 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 28/49] x86/alternatives: Rename 'put_desc()' to 'put_text_poke_array()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 30/49] x86/alternatives: Simplify smp_text_poke_int3_trap_handler() Ingo Molnar
` (21 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
There's no need to return a pointer on success - it's always
the same pointer.
Return a bool instead.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 202720228c98..12f0fd35773d 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2475,15 +2475,14 @@ static struct smp_text_poke_array {
static DEFINE_PER_CPU(atomic_t, int3_refs);
-static __always_inline
-struct smp_text_poke_array *try_get_text_poke_array(void)
+static bool try_get_text_poke_array(void)
{
atomic_t *refs = this_cpu_ptr(&int3_refs);
if (!raw_atomic_inc_not_zero(refs))
- return NULL;
+ return false;
- return &text_poke_array;
+ return true;
}
static __always_inline void put_text_poke_array(void)
@@ -2530,9 +2529,9 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
*/
smp_rmb();
- desc = try_get_text_poke_array();
- if (!desc)
+ if (!try_get_text_poke_array())
return 0;
+ desc = &text_poke_array;
WARN_ON_ONCE(desc->vec != text_poke_array.vec);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 30/49] x86/alternatives: Simplify smp_text_poke_int3_trap_handler()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (28 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 29/49] x86/alternatives: Simplify try_get_text_poke_array() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 31/49] x86/alternatives: Simplify smp_text_poke_batch_process() Ingo Molnar
` (20 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Remove the 'desc' local variable indirection and use
text_poke_array directly.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 12f0fd35773d..57d7032eca41 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2511,7 +2511,6 @@ static __always_inline int patch_cmp(const void *key, const void *elt)
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
- struct smp_text_poke_array *desc;
struct smp_text_poke_loc *tp;
int ret = 0;
void *ip;
@@ -2531,9 +2530,6 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
if (!try_get_text_poke_array())
return 0;
- desc = &text_poke_array;
-
- WARN_ON_ONCE(desc->vec != text_poke_array.vec);
/*
* Discount the INT3. See smp_text_poke_batch_process().
@@ -2543,14 +2539,14 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/*
* Skip the binary search if there is a single member in the vector.
*/
- if (unlikely(desc->nr_entries > 1)) {
- tp = __inline_bsearch(ip, desc->vec, desc->nr_entries,
+ if (unlikely(text_poke_array.nr_entries > 1)) {
+ tp = __inline_bsearch(ip, text_poke_array.vec, text_poke_array.nr_entries,
sizeof(struct smp_text_poke_loc),
patch_cmp);
if (!tp)
goto out_put;
} else {
- tp = desc->vec;
+ tp = text_poke_array.vec;
if (text_poke_addr(tp) != ip)
goto out_put;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 31/49] x86/alternatives: Simplify smp_text_poke_batch_process()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (29 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 30/49] x86/alternatives: Simplify smp_text_poke_int3_trap_handler() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 32/49] x86/alternatives: Rename 'int3_refs' to 'text_poke_array_refs' Ingo Molnar
` (19 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This function is now using the text_poke_array state exclusively,
make that explicit by removing the redundant input parameters.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 43 ++++++++++++++++++++-----------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 57d7032eca41..34d3c69595a0 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2591,8 +2591,8 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/**
* smp_text_poke_batch_process() -- update instructions on live kernel on SMP
- * @tp: vector of instructions to patch
- * @nr_entries: number of entries in the vector
+ * @text_poke_array.vec: vector of instructions to patch
+ * @text_poke_array.nr_entries: number of entries in the vector
*
* Modify multi-byte instruction by using int3 breakpoint on SMP.
* We completely avoid stop_machine() here, and achieve the
@@ -2610,7 +2610,7 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
* replacing opcode
* - sync cores
*/
-static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned int nr_entries)
+static void smp_text_poke_batch_process(void)
{
unsigned char int3 = INT3_INSN_OPCODE;
unsigned int i;
@@ -2618,9 +2618,6 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
lockdep_assert_held(&text_mutex);
- WARN_ON_ONCE(tp != text_poke_array.vec);
- WARN_ON_ONCE(nr_entries != text_poke_array.nr_entries);
-
/*
* Corresponds to the implicit memory barrier in try_get_text_poke_array() to
* ensure reading a non-zero refcount provides up to date text_poke_array data.
@@ -2640,16 +2637,16 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
/*
* Corresponding read barrier in int3 notifier for making sure the
- * nr_entries and handler are correctly ordered wrt. patching.
+ * text_poke_array.nr_entries and handler are correctly ordered wrt. patching.
*/
smp_wmb();
/*
* First step: add a int3 trap to the address that will be patched.
*/
- for (i = 0; i < nr_entries; i++) {
- tp[i].old = *(u8 *)text_poke_addr(&tp[i]);
- text_poke(text_poke_addr(&tp[i]), &int3, INT3_INSN_SIZE);
+ for (i = 0; i < text_poke_array.nr_entries; i++) {
+ text_poke_array.vec[i].old = *(u8 *)text_poke_addr(&text_poke_array.vec[i]);
+ text_poke(text_poke_addr(&text_poke_array.vec[i]), &int3, INT3_INSN_SIZE);
}
text_poke_sync();
@@ -2657,15 +2654,15 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
/*
* Second step: update all but the first byte of the patched range.
*/
- for (do_sync = 0, i = 0; i < nr_entries; i++) {
- u8 old[POKE_MAX_OPCODE_SIZE+1] = { tp[i].old, };
+ for (do_sync = 0, i = 0; i < text_poke_array.nr_entries; i++) {
+ u8 old[POKE_MAX_OPCODE_SIZE+1] = { text_poke_array.vec[i].old, };
u8 _new[POKE_MAX_OPCODE_SIZE+1];
- const u8 *new = tp[i].text;
- int len = tp[i].len;
+ const u8 *new = text_poke_array.vec[i].text;
+ int len = text_poke_array.vec[i].len;
if (len - INT3_INSN_SIZE > 0) {
memcpy(old + INT3_INSN_SIZE,
- text_poke_addr(&tp[i]) + INT3_INSN_SIZE,
+ text_poke_addr(&text_poke_array.vec[i]) + INT3_INSN_SIZE,
len - INT3_INSN_SIZE);
if (len == 6) {
@@ -2674,7 +2671,7 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
new = _new;
}
- text_poke(text_poke_addr(&tp[i]) + INT3_INSN_SIZE,
+ text_poke(text_poke_addr(&text_poke_array.vec[i]) + INT3_INSN_SIZE,
new + INT3_INSN_SIZE,
len - INT3_INSN_SIZE);
@@ -2705,7 +2702,7 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
* The old instruction is recorded so that the event can be
* processed forwards or backwards.
*/
- perf_event_text_poke(text_poke_addr(&tp[i]), old, len, new, len);
+ perf_event_text_poke(text_poke_addr(&text_poke_array.vec[i]), old, len, new, len);
}
if (do_sync) {
@@ -2721,16 +2718,16 @@ static void smp_text_poke_batch_process(struct smp_text_poke_loc *tp, unsigned i
* Third step: replace the first byte (int3) by the first byte of
* replacing opcode.
*/
- for (do_sync = 0, i = 0; i < nr_entries; i++) {
- u8 byte = tp[i].text[0];
+ for (do_sync = 0, i = 0; i < text_poke_array.nr_entries; i++) {
+ u8 byte = text_poke_array.vec[i].text[0];
- if (tp[i].len == 6)
+ if (text_poke_array.vec[i].len == 6)
byte = 0x0f;
if (byte == INT3_INSN_OPCODE)
continue;
- text_poke(text_poke_addr(&tp[i]), &byte, INT3_INSN_SIZE);
+ text_poke(text_poke_addr(&text_poke_array.vec[i]), &byte, INT3_INSN_SIZE);
do_sync++;
}
@@ -2859,7 +2856,7 @@ static bool text_poke_addr_ordered(void *addr)
void smp_text_poke_batch_finish(void)
{
if (text_poke_array.nr_entries) {
- smp_text_poke_batch_process(text_poke_array.vec, text_poke_array.nr_entries);
+ smp_text_poke_batch_process();
text_poke_array.nr_entries = 0;
}
}
@@ -2869,7 +2866,7 @@ static void smp_text_poke_batch_flush(void *addr)
lockdep_assert_held(&text_mutex);
if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr)) {
- smp_text_poke_batch_process(text_poke_array.vec, text_poke_array.nr_entries);
+ smp_text_poke_batch_process();
text_poke_array.nr_entries = 0;
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 32/49] x86/alternatives: Rename 'int3_refs' to 'text_poke_array_refs'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (30 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 31/49] x86/alternatives: Simplify smp_text_poke_batch_process() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 33/49] x86/alternatives: Move the text_poke_array manipulation into text_poke_int3_loc_init() and rename it to __smp_text_poke_batch_add() Ingo Molnar
` (18 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Make it clear that these reference counts lock access
to text_poke_array.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 34d3c69595a0..566b857d210d 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2473,11 +2473,11 @@ static struct smp_text_poke_array {
struct smp_text_poke_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
} text_poke_array;
-static DEFINE_PER_CPU(atomic_t, int3_refs);
+static DEFINE_PER_CPU(atomic_t, text_poke_array_refs);
static bool try_get_text_poke_array(void)
{
- atomic_t *refs = this_cpu_ptr(&int3_refs);
+ atomic_t *refs = this_cpu_ptr(&text_poke_array_refs);
if (!raw_atomic_inc_not_zero(refs))
return false;
@@ -2487,7 +2487,7 @@ static bool try_get_text_poke_array(void)
static __always_inline void put_text_poke_array(void)
{
- atomic_t *refs = this_cpu_ptr(&int3_refs);
+ atomic_t *refs = this_cpu_ptr(&text_poke_array_refs);
smp_mb__before_atomic();
raw_atomic_dec(refs);
@@ -2522,9 +2522,9 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
* Having observed our INT3 instruction, we now must observe
* text_poke_array with non-zero refcount:
*
- * int3_refs = 1 INT3
+ * text_poke_array_refs = 1 INT3
* WMB RMB
- * write INT3 if (int3_refs != 0)
+ * write INT3 if (text_poke_array_refs != 0)
*/
smp_rmb();
@@ -2623,7 +2623,7 @@ static void smp_text_poke_batch_process(void)
* ensure reading a non-zero refcount provides up to date text_poke_array data.
*/
for_each_possible_cpu(i)
- atomic_set_release(per_cpu_ptr(&int3_refs, i), 1);
+ atomic_set_release(per_cpu_ptr(&text_poke_array_refs, i), 1);
/*
* Function tracing can enable thousands of places that need to be
@@ -2745,7 +2745,7 @@ static void smp_text_poke_batch_process(void)
* unused.
*/
for_each_possible_cpu(i) {
- atomic_t *refs = per_cpu_ptr(&int3_refs, i);
+ atomic_t *refs = per_cpu_ptr(&text_poke_array_refs, i);
if (unlikely(!atomic_dec_and_test(refs)))
atomic_cond_read_acquire(refs, !VAL);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 33/49] x86/alternatives: Move the text_poke_array manipulation into text_poke_int3_loc_init() and rename it to __smp_text_poke_batch_add()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (31 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 32/49] x86/alternatives: Rename 'int3_refs' to 'text_poke_array_refs' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 34/49] x86/alternatives: Remove the mixed-patching restriction on smp_text_poke_single() Ingo Molnar
` (17 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This simplifies the code and code generation a bit:
text data bss dec hex filename
14802 1029 4112 19943 4de7 alternative.o.before
14784 1029 4112 19925 4dd5 alternative.o.after
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 566b857d210d..f0351abcbfaa 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2752,12 +2752,14 @@ static void smp_text_poke_batch_process(void)
}
}
-static void text_poke_int3_loc_init(struct smp_text_poke_loc *tp, void *addr,
- const void *opcode, size_t len, const void *emulate)
+static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
+ struct smp_text_poke_loc *tp;
struct insn insn;
int ret, i = 0;
+ tp = &text_poke_array.vec[text_poke_array.nr_entries++];
+
if (len == 6)
i = 1;
memcpy((void *)tp->text, opcode+i, len-i);
@@ -2873,12 +2875,8 @@ static void smp_text_poke_batch_flush(void *addr)
void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct smp_text_poke_loc *tp;
-
smp_text_poke_batch_flush(addr);
-
- tp = &text_poke_array.vec[text_poke_array.nr_entries++];
- text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
+ __smp_text_poke_batch_add(addr, opcode, len, emulate);
}
/**
@@ -2894,13 +2892,9 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
*/
void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct smp_text_poke_loc *tp;
-
/* Batch-patching should not be mixed with single-patching: */
WARN_ON_ONCE(text_poke_array.nr_entries != 0);
- tp = &text_poke_array.vec[text_poke_array.nr_entries++];
- text_poke_int3_loc_init(tp, addr, opcode, len, emulate);
-
+ __smp_text_poke_batch_add(addr, opcode, len, emulate);
smp_text_poke_batch_finish();
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 34/49] x86/alternatives: Remove the mixed-patching restriction on smp_text_poke_single()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (32 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 33/49] x86/alternatives: Move the text_poke_array manipulation into text_poke_int3_loc_init() and rename it to __smp_text_poke_batch_add() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 35/49] x86/alternatives: Document 'smp_text_poke_single()' Ingo Molnar
` (16 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
At this point smp_text_poke_single(addr, opcode, len, emulate) is equivalent to:
smp_text_poke_batch_add(addr, opcode, len, emulate);
smp_text_poke_batch_finish();
So remove the restriction on mixing single-instruction patching
with multi-instruction patching.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index f0351abcbfaa..4f880a435e98 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2892,9 +2892,6 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
*/
void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
- /* Batch-patching should not be mixed with single-patching: */
- WARN_ON_ONCE(text_poke_array.nr_entries != 0);
-
__smp_text_poke_batch_add(addr, opcode, len, emulate);
smp_text_poke_batch_finish();
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 35/49] x86/alternatives: Document 'smp_text_poke_single()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (33 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 34/49] x86/alternatives: Remove the mixed-patching restriction on smp_text_poke_single() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 36/49] x86/alternatives: Add documentation for smp_text_poke_batch_add() Ingo Molnar
` (15 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Extend the documentation to better describe its purpose.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 4f880a435e98..a32959019589 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2880,7 +2880,7 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
}
/**
- * smp_text_poke_single() -- update instructions on live kernel on SMP
+ * smp_text_poke_single() -- update instruction on live kernel on SMP immediately
* @addr: address to patch
* @opcode: opcode of new instruction
* @len: length to copy
@@ -2888,7 +2888,8 @@ void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, c
*
* Update a single instruction with the vector in the stack, avoiding
* dynamically allocated memory. This function should be used when it is
- * not possible to allocate memory.
+ * not possible to allocate memory for a vector. The single instruction
+ * is patched in immediately.
*/
void __ref smp_text_poke_single(void *addr, const void *opcode, size_t len, const void *emulate)
{
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 36/49] x86/alternatives: Add documentation for smp_text_poke_batch_add()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (34 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 35/49] x86/alternatives: Document 'smp_text_poke_single()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process() Ingo Molnar
` (14 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index a32959019589..1df8fac6740d 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2873,6 +2873,19 @@ static void smp_text_poke_batch_flush(void *addr)
}
}
+/**
+ * smp_text_poke_batch_add() -- update instruction on live kernel on SMP, batched
+ * @addr: address to patch
+ * @opcode: opcode of new instruction
+ * @len: length to copy
+ * @emulate: instruction to be emulated
+ *
+ * Add a new instruction to the current queue of to-be-patched instructions
+ * the kernel maintains. The patching request will not be executed immediately,
+ * but becomes part of an array of patching requests, optimized for batched
+ * execution. All pending patching requests will be executed on the next
+ * smp_text_poke_batch_finish() call.
+ */
void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
smp_text_poke_batch_flush(addr);
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (35 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 36/49] x86/alternatives: Add documentation for smp_text_poke_batch_add() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-04-03 13:08 ` Nikolay Borisov
2025-03-28 13:26 ` [PATCH 38/49] x86/alternatives: Rename 'text_poke_sync()' to 'smp_text_poke_sync_each_cpu()' Ingo Molnar
` (13 subsequent siblings)
50 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Simplifies the code and improves code generation a bit:
text data bss dec hex filename
14769 1017 4112 19898 4dba alternative.o.before
14742 1017 4112 19871 4d9f alternative.o.after
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 1df8fac6740d..5293929488f0 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
if (unlikely(!atomic_dec_and_test(refs)))
atomic_cond_read_acquire(refs, !VAL);
}
+
+ /* They are all completed: */
+ text_poke_array.nr_entries = 0;
}
static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
@@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
void smp_text_poke_batch_finish(void)
{
- if (text_poke_array.nr_entries) {
+ if (text_poke_array.nr_entries)
smp_text_poke_batch_process();
- text_poke_array.nr_entries = 0;
- }
}
static void smp_text_poke_batch_flush(void *addr)
{
lockdep_assert_held(&text_mutex);
- if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr)) {
+ if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr))
smp_text_poke_batch_process();
- text_poke_array.nr_entries = 0;
- }
}
/**
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-03-28 13:26 ` [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process() Ingo Molnar
@ 2025-04-03 13:08 ` Nikolay Borisov
2025-04-03 13:38 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Nikolay Borisov @ 2025-04-03 13:08 UTC (permalink / raw)
To: Ingo Molnar, linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner
On 28.03.25 г. 15:26 ч., Ingo Molnar wrote:
> Simplifies the code and improves code generation a bit:
>
> text data bss dec hex filename
> 14769 1017 4112 19898 4dba alternative.o.before
> 14742 1017 4112 19871 4d9f alternative.o.after
>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> ---
> arch/x86/kernel/alternative.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 1df8fac6740d..5293929488f0 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
> if (unlikely(!atomic_dec_and_test(refs)))
> atomic_cond_read_acquire(refs, !VAL);
> }
> +
> + /* They are all completed: */
> + text_poke_array.nr_entries = 0;
> }
>
> static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
> @@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
>
> void smp_text_poke_batch_finish(void)
> {
> - if (text_poke_array.nr_entries) {
> + if (text_poke_array.nr_entries)
> smp_text_poke_batch_process();
> - text_poke_array.nr_entries = 0;
> - }
> }
This function becomes trivial, why not simply move the check into
smp_text_poke_batch_process and rename it to smp_text_poke_batch_finish ?
>
> static void smp_text_poke_batch_flush(void *addr)
> {
> lockdep_assert_held(&text_mutex);
>
> - if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr)) {
> + if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr))
> smp_text_poke_batch_process();
> - text_poke_array.nr_entries = 0;
> - }
> }
>
> /**
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 13:08 ` Nikolay Borisov
@ 2025-04-03 13:38 ` Ingo Molnar
2025-04-03 13:41 ` Nikolay Borisov
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-03 13:38 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
* Nikolay Borisov <nik.borisov@suse.com> wrote:
>
>
> On 28.03.25 г. 15:26 ч., Ingo Molnar wrote:
> > Simplifies the code and improves code generation a bit:
> >
> > text data bss dec hex filename
> > 14769 1017 4112 19898 4dba alternative.o.before
> > 14742 1017 4112 19871 4d9f alternative.o.after
> >
> > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > ---
> > arch/x86/kernel/alternative.c | 11 +++++------
> > 1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > index 1df8fac6740d..5293929488f0 100644
> > --- a/arch/x86/kernel/alternative.c
> > +++ b/arch/x86/kernel/alternative.c
> > @@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
> > if (unlikely(!atomic_dec_and_test(refs)))
> > atomic_cond_read_acquire(refs, !VAL);
> > }
> > +
> > + /* They are all completed: */
> > + text_poke_array.nr_entries = 0;
> > }
> > static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
> > @@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
> > void smp_text_poke_batch_finish(void)
> > {
> > - if (text_poke_array.nr_entries) {
> > + if (text_poke_array.nr_entries)
> > smp_text_poke_batch_process();
> > - text_poke_array.nr_entries = 0;
> > - }
> > }
>
> This function becomes trivial, why not simply move the check into
> smp_text_poke_batch_process and rename it to smp_text_poke_batch_finish ?
Yeah, that's pretty much what happens in patch #47:
x86/alternatives: Remove 'smp_text_poke_batch_flush()'
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 13:38 ` Ingo Molnar
@ 2025-04-03 13:41 ` Nikolay Borisov
2025-04-03 14:13 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Nikolay Borisov @ 2025-04-03 13:41 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
On 3.04.25 г. 16:38 ч., Ingo Molnar wrote:
>
> * Nikolay Borisov <nik.borisov@suse.com> wrote:
>
>>
>>
>> On 28.03.25 г. 15:26 ч., Ingo Molnar wrote:
>>> Simplifies the code and improves code generation a bit:
>>>
>>> text data bss dec hex filename
>>> 14769 1017 4112 19898 4dba alternative.o.before
>>> 14742 1017 4112 19871 4d9f alternative.o.after
>>>
>>> Signed-off-by: Ingo Molnar <mingo@kernel.org>
>>> ---
>>> arch/x86/kernel/alternative.c | 11 +++++------
>>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>>> index 1df8fac6740d..5293929488f0 100644
>>> --- a/arch/x86/kernel/alternative.c
>>> +++ b/arch/x86/kernel/alternative.c
>>> @@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
>>> if (unlikely(!atomic_dec_and_test(refs)))
>>> atomic_cond_read_acquire(refs, !VAL);
>>> }
>>> +
>>> + /* They are all completed: */
>>> + text_poke_array.nr_entries = 0;
>>> }
>>> static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
>>> @@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
>>> void smp_text_poke_batch_finish(void)
>>> {
>>> - if (text_poke_array.nr_entries) {
>>> + if (text_poke_array.nr_entries)
>>> smp_text_poke_batch_process();
>>> - text_poke_array.nr_entries = 0;
>>> - }
>>> }
>>
>> This function becomes trivial, why not simply move the check into
>> smp_text_poke_batch_process and rename it to smp_text_poke_batch_finish ?
>
> Yeah, that's pretty much what happens in patch #47:
>
> x86/alternatives: Remove 'smp_text_poke_batch_flush()'
Well, that patch removes poke_batch_flush but still retains
poke_batch_finish + poke_batch_process. My suggestion is to eliminate
poke_batch_process name and turn it into poke_batch_finish by moving the
check from poke_batch_finish into poke_batch_process.
>
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 13:41 ` Nikolay Borisov
@ 2025-04-03 14:13 ` Ingo Molnar
2025-04-03 14:39 ` Nikolay Borisov
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-03 14:13 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
* Nikolay Borisov <nik.borisov@suse.com> wrote:
>
>
> On 3.04.25 г. 16:38 ч., Ingo Molnar wrote:
> >
> > * Nikolay Borisov <nik.borisov@suse.com> wrote:
> >
> > >
> > >
> > > On 28.03.25 г. 15:26 ч., Ingo Molnar wrote:
> > > > Simplifies the code and improves code generation a bit:
> > > >
> > > > text data bss dec hex filename
> > > > 14769 1017 4112 19898 4dba alternative.o.before
> > > > 14742 1017 4112 19871 4d9f alternative.o.after
> > > >
> > > > Signed-off-by: Ingo Molnar <mingo@kernel.org>
> > > > ---
> > > > arch/x86/kernel/alternative.c | 11 +++++------
> > > > 1 file changed, 5 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> > > > index 1df8fac6740d..5293929488f0 100644
> > > > --- a/arch/x86/kernel/alternative.c
> > > > +++ b/arch/x86/kernel/alternative.c
> > > > @@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
> > > > if (unlikely(!atomic_dec_and_test(refs)))
> > > > atomic_cond_read_acquire(refs, !VAL);
> > > > }
> > > > +
> > > > + /* They are all completed: */
> > > > + text_poke_array.nr_entries = 0;
> > > > }
> > > > static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
> > > > @@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
> > > > void smp_text_poke_batch_finish(void)
> > > > {
> > > > - if (text_poke_array.nr_entries) {
> > > > + if (text_poke_array.nr_entries)
> > > > smp_text_poke_batch_process();
> > > > - text_poke_array.nr_entries = 0;
> > > > - }
> > > > }
> > >
> > > This function becomes trivial, why not simply move the check into
> > > smp_text_poke_batch_process and rename it to smp_text_poke_batch_finish ?
> >
> > Yeah, that's pretty much what happens in patch #47:
> >
> > x86/alternatives: Remove 'smp_text_poke_batch_flush()'
>
> Well, that patch removes poke_batch_flush but still retains
> poke_batch_finish + poke_batch_process. My suggestion is to eliminate
> poke_batch_process name and turn it into poke_batch_finish by moving the
> check from poke_batch_finish into poke_batch_process.
So, in the context of the full tree at:
git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/alternatives
Standalone smp_text_poke_batch_process() is still needed, because
smp_text_poke_batch_add() uses it to drive the whole 'batching'
machinery.
If smp_text_poke_batch_process() finishes for each call, if I
understand your suggestion correctly, that reduces the amount of
batching done, which is a disadvantage.
Note how right now it's possible to do something like this:
smp_text_poke_batch_add(1);
smp_text_poke_batch_add(1);
smp_text_poke_batch_add(1);
smp_text_poke_batch_add(1);
smp_text_poke_batch_finish();
This results in a single call to smp_text_poke_batch_process(), with 4
entries, a single 4-entry flush in essence.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 14:13 ` Ingo Molnar
@ 2025-04-03 14:39 ` Nikolay Borisov
2025-04-03 15:29 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Nikolay Borisov @ 2025-04-03 14:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
On 3.04.25 г. 17:13 ч., Ingo Molnar wrote:
>
> * Nikolay Borisov <nik.borisov@suse.com> wrote:
>
>>
>>
>> On 3.04.25 г. 16:38 ч., Ingo Molnar wrote:
>>>
>>> * Nikolay Borisov <nik.borisov@suse.com> wrote:
>>>
>>>>
>>>>
>>>> On 28.03.25 г. 15:26 ч., Ingo Molnar wrote:
>>>>> Simplifies the code and improves code generation a bit:
>>>>>
>>>>> text data bss dec hex filename
>>>>> 14769 1017 4112 19898 4dba alternative.o.before
>>>>> 14742 1017 4112 19871 4d9f alternative.o.after
>>>>>
>>>>> Signed-off-by: Ingo Molnar <mingo@kernel.org>
>>>>> ---
>>>>> arch/x86/kernel/alternative.c | 11 +++++------
>>>>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>>>>> index 1df8fac6740d..5293929488f0 100644
>>>>> --- a/arch/x86/kernel/alternative.c
>>>>> +++ b/arch/x86/kernel/alternative.c
>>>>> @@ -2750,6 +2750,9 @@ static void smp_text_poke_batch_process(void)
>>>>> if (unlikely(!atomic_dec_and_test(refs)))
>>>>> atomic_cond_read_acquire(refs, !VAL);
>>>>> }
>>>>> +
>>>>> + /* They are all completed: */
>>>>> + text_poke_array.nr_entries = 0;
>>>>> }
>>>>> static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
>>>>> @@ -2857,20 +2860,16 @@ static bool text_poke_addr_ordered(void *addr)
>>>>> void smp_text_poke_batch_finish(void)
>>>>> {
>>>>> - if (text_poke_array.nr_entries) {
>>>>> + if (text_poke_array.nr_entries)
>>>>> smp_text_poke_batch_process();
>>>>> - text_poke_array.nr_entries = 0;
>>>>> - }
>>>>> }
>>>>
>>>> This function becomes trivial, why not simply move the check into
>>>> smp_text_poke_batch_process and rename it to smp_text_poke_batch_finish ?
>>>
>>> Yeah, that's pretty much what happens in patch #47:
>>>
>>> x86/alternatives: Remove 'smp_text_poke_batch_flush()'
>>
>> Well, that patch removes poke_batch_flush but still retains
>> poke_batch_finish + poke_batch_process. My suggestion is to eliminate
>> poke_batch_process name and turn it into poke_batch_finish by moving the
>> check from poke_batch_finish into poke_batch_process.
>
> So, in the context of the full tree at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git WIP.x86/alternatives
>
> Standalone smp_text_poke_batch_process() is still needed, because
> smp_text_poke_batch_add() uses it to drive the whole 'batching'
> machinery.
>
> If smp_text_poke_batch_process() finishes for each call, if I
> understand your suggestion correctly, that reduces the amount of
> batching done, which is a disadvantage.
>
> Note how right now it's possible to do something like this:
>
> smp_text_poke_batch_add(1);
> smp_text_poke_batch_add(1);
> smp_text_poke_batch_add(1);
> smp_text_poke_batch_add(1);
> smp_text_poke_batch_finish();
>
> This results in a single call to smp_text_poke_batch_process(), with 4
> entries, a single 4-entry flush in essence.
I meant doing this:
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5b1a6252a4b9..b6a781b9de26 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2587,12 +2587,16 @@ noinstr int
smp_text_poke_int3_trap_handler(struct pt_regs *regs)
* replacing opcode
* - SMP sync all CPUs
*/
-static void smp_text_poke_batch_process(void)
+void smp_text_poke_batch_finish(void)
{
unsigned char int3 = INT3_INSN_OPCODE;
unsigned int i;
int do_sync;
+
+ if (!text_poke_array.nr_entries)
+ return;
+
lockdep_assert_held(&text_mutex);
/*
@@ -2832,12 +2836,6 @@ static bool text_poke_addr_ordered(void *addr)
return true;
}
-void smp_text_poke_batch_finish(void)
-{
- if (text_poke_array.nr_entries)
- smp_text_poke_batch_process();
-}
-
/**
* smp_text_poke_batch_add() -- update instruction on live kernel on
SMP, batched
* @addr: address to patch
@@ -2854,7 +2852,7 @@ void smp_text_poke_batch_finish(void)
void __ref smp_text_poke_batch_add(void *addr, const void *opcode,
size_t len, const void *emulate)
{
if (text_poke_array.nr_entries == TEXT_POKE_ARRAY_MAX ||
!text_poke_addr_ordered(addr))
- smp_text_poke_batch_process();
+ smp_text_poke_batch_finish();
__smp_text_poke_batch_add(addr, opcode, len, emulate);
}
AFAICS this doesn't change the semantics. I.e smp_text_poke_batch_add
will call poke_batch_finish iff the address to be added violates the
sorted order of text_poke_array. The net effect is we have 1 less
function name to care about.
>
> Thanks,
>
> Ingo
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 14:39 ` Nikolay Borisov
@ 2025-04-03 15:29 ` Ingo Molnar
2025-04-03 15:34 ` Nikolay Borisov
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-03 15:29 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
* Nikolay Borisov <nik.borisov@suse.com> wrote:
> I meant doing this:
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 5b1a6252a4b9..b6a781b9de26 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -2587,12 +2587,16 @@ noinstr int smp_text_poke_int3_trap_handler(struct
> pt_regs *regs)
> * replacing opcode
> * - SMP sync all CPUs
> */
> -static void smp_text_poke_batch_process(void)
> +void smp_text_poke_batch_finish(void)
> {
> unsigned char int3 = INT3_INSN_OPCODE;
> unsigned int i;
> int do_sync;
>
> +
> + if (!text_poke_array.nr_entries)
> + return;
> - smp_text_poke_batch_process();
> + smp_text_poke_batch_finish();
I suppose we could do this - it adds one more check to
smp_text_poke_batch_add() though.
> AFAICS this doesn't change the semantics. I.e smp_text_poke_batch_add
> will call poke_batch_finish iff the address to be added violates the
> sorted order of text_poke_array. The net effect is we have 1 less
> function name to care about.
Yeah, it doesn't change semantics, but it's a very small
deoptimization.
Mind sending a patch? It does simplify the facility some more and that
single branch will wash away against costs like the CR3 flushes done
...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 15:29 ` Ingo Molnar
@ 2025-04-03 15:34 ` Nikolay Borisov
2025-04-04 7:48 ` Ingo Molnar
0 siblings, 1 reply; 69+ messages in thread
From: Nikolay Borisov @ 2025-04-03 15:34 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
On 3.04.25 г. 18:29 ч., Ingo Molnar wrote:
>
> * Nikolay Borisov <nik.borisov@suse.com> wrote:
>
>> I meant doing this:
>>
>> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
>> index 5b1a6252a4b9..b6a781b9de26 100644
>> --- a/arch/x86/kernel/alternative.c
>> +++ b/arch/x86/kernel/alternative.c
>> @@ -2587,12 +2587,16 @@ noinstr int smp_text_poke_int3_trap_handler(struct
>> pt_regs *regs)
>> * replacing opcode
>> * - SMP sync all CPUs
>> */
>> -static void smp_text_poke_batch_process(void)
>> +void smp_text_poke_batch_finish(void)
>> {
>> unsigned char int3 = INT3_INSN_OPCODE;
>> unsigned int i;
>> int do_sync;
>>
>> +
>> + if (!text_poke_array.nr_entries)
>> + return;
>
>> - smp_text_poke_batch_process();
>> + smp_text_poke_batch_finish();
>
> I suppose we could do this - it adds one more check to
> smp_text_poke_batch_add() though.
poke_batch_finish you meant?
>
>> AFAICS this doesn't change the semantics. I.e smp_text_poke_batch_add
>> will call poke_batch_finish iff the address to be added violates the
>> sorted order of text_poke_array. The net effect is we have 1 less
>> function name to care about.
>
> Yeah, it doesn't change semantics, but it's a very small
> deoptimization.
> > Mind sending a patch? It does simplify the facility some more and that
> single branch will wash away against costs like the CR3 flushes done
Given that poke_batch_finish does a cond_resched and sync_each_cpu which
is an IPI can it even be considered a performance critical path ?
> ...
>
> Thanks,
>
> Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process()
2025-04-03 15:34 ` Nikolay Borisov
@ 2025-04-04 7:48 ` Ingo Molnar
0 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-04-04 7:48 UTC (permalink / raw)
To: Nikolay Borisov
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Peter Zijlstra, Borislav Petkov, Thomas Gleixner
* Nikolay Borisov <nik.borisov@suse.com> wrote:
> > Yeah, it doesn't change semantics, but it's a very small
> > deoptimization.
> > > Mind sending a patch? It does simplify the facility some more and that
> > single branch will wash away against costs like the CR3 flushes done
>
> Given that poke_batch_finish does a cond_resched and sync_each_cpu
> which is an IPI can it even be considered a performance critical path
> ?
Probably not, but even if it was, I think your change would still be an
overall win, so please send a changelogged patch against
WIP.x86/alternatives.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread
* [PATCH 38/49] x86/alternatives: Rename 'text_poke_sync()' to 'smp_text_poke_sync_each_cpu()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (36 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 37/49] x86/alternatives: Move text_poke_array completion from smp_text_poke_batch_finish() and smp_text_poke_batch_flush() to smp_text_poke_batch_process() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 39/49] x86/alternatives: Simplify text_poke_addr_ordered() Ingo Molnar
` (12 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Unlike sync_core(), text_poke_sync() is a very heavy operation, as
it sends an IPI to every online CPU in the system and waits for
completion.
Reflect this in the name.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 12 ++++++------
arch/x86/kernel/kprobes/core.c | 4 ++--
arch/x86/kernel/kprobes/opt.c | 4 ++--
arch/x86/kernel/module.c | 2 +-
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 20ce4eda6d47..6834e652d26c 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -32,7 +32,7 @@ extern void apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u
* an inconsistent instruction while you patch.
*/
extern void *text_poke(void *addr, const void *opcode, size_t len);
-extern void text_poke_sync(void);
+extern void smp_text_poke_sync_each_cpu(void);
extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern void *text_poke_copy(void *addr, const void *opcode, size_t len);
#define text_poke_copy text_poke_copy
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5293929488f0..b0bd8cd524c5 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2445,7 +2445,7 @@ static void do_sync_core(void *info)
sync_core();
}
-void text_poke_sync(void)
+void smp_text_poke_sync_each_cpu(void)
{
on_each_cpu(do_sync_core, NULL, 1);
}
@@ -2469,8 +2469,8 @@ struct smp_text_poke_loc {
#define TP_ARRAY_NR_ENTRIES_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
static struct smp_text_poke_array {
- int nr_entries;
struct smp_text_poke_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
+ int nr_entries;
} text_poke_array;
static DEFINE_PER_CPU(atomic_t, text_poke_array_refs);
@@ -2649,7 +2649,7 @@ static void smp_text_poke_batch_process(void)
text_poke(text_poke_addr(&text_poke_array.vec[i]), &int3, INT3_INSN_SIZE);
}
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
/*
* Second step: update all but the first byte of the patched range.
@@ -2711,7 +2711,7 @@ static void smp_text_poke_batch_process(void)
* not necessary and we'd be safe even without it. But
* better safe than sorry (plus there's not only Intel).
*/
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
}
/*
@@ -2732,13 +2732,13 @@ static void smp_text_poke_batch_process(void)
}
if (do_sync)
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
/*
* Remove and wait for refs to be zero.
*
* Notably, if after step-3 above the INT3 got removed, then the
- * text_poke_sync() will have serialized against any running INT3
+ * smp_text_poke_sync_each_cpu() will have serialized against any running INT3
* handlers and the below spin-wait will not happen.
*
* IOW. unless the replacement instruction is INT3, this case goes
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 09608fd93687..47cb8eb138ba 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -808,7 +808,7 @@ void arch_arm_kprobe(struct kprobe *p)
u8 int3 = INT3_INSN_OPCODE;
text_poke(p->addr, &int3, 1);
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
perf_event_text_poke(p->addr, &p->opcode, 1, &int3, 1);
}
@@ -818,7 +818,7 @@ void arch_disarm_kprobe(struct kprobe *p)
perf_event_text_poke(p->addr, &int3, 1, &p->opcode, 1);
text_poke(p->addr, &p->opcode, 1);
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
}
void arch_remove_kprobe(struct kprobe *p)
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 9307a40f4983..0aabd4c4e2c4 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -513,11 +513,11 @@ void arch_unoptimize_kprobe(struct optimized_kprobe *op)
JMP32_INSN_SIZE - INT3_INSN_SIZE);
text_poke(addr, new, INT3_INSN_SIZE);
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
text_poke(addr + INT3_INSN_SIZE,
new + INT3_INSN_SIZE,
JMP32_INSN_SIZE - INT3_INSN_SIZE);
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
perf_event_text_poke(op->kp.addr, old, JMP32_INSN_SIZE, new, JMP32_INSN_SIZE);
}
diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index a7998f351701..231d6326d1fd 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -206,7 +206,7 @@ static int write_relocate_add(Elf64_Shdr *sechdrs,
write, apply);
if (!early) {
- text_poke_sync();
+ smp_text_poke_sync_each_cpu();
mutex_unlock(&text_mutex);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 39/49] x86/alternatives: Simplify text_poke_addr_ordered()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (37 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 38/49] x86/alternatives: Rename 'text_poke_sync()' to 'smp_text_poke_sync_each_cpu()' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 40/49] x86/alternatives: Constify text_poke_addr() Ingo Molnar
` (11 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
- Use direct 'void *' pointer comparison, there's no
need to force the type to 'unsigned long'.
- Remove the 'tp' local variable indirection
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index b0bd8cd524c5..27f1c7bdd384 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2838,8 +2838,6 @@ static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len
*/
static bool text_poke_addr_ordered(void *addr)
{
- struct smp_text_poke_loc *tp;
-
WARN_ON_ONCE(!addr);
if (!text_poke_array.nr_entries)
@@ -2851,8 +2849,7 @@ static bool text_poke_addr_ordered(void *addr)
* is violated and we must first flush all pending patching
* requests:
*/
- tp = &text_poke_array.vec[text_poke_array.nr_entries-1];
- if ((unsigned long)text_poke_addr(tp) > (unsigned long)addr)
+ if (text_poke_addr(text_poke_array.vec + text_poke_array.nr_entries-1) > addr)
return false;
return true;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 40/49] x86/alternatives: Constify text_poke_addr()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (38 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 39/49] x86/alternatives: Simplify text_poke_addr_ordered() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 41/49] x86/alternatives: Simplify and clean up patch_cmp() Ingo Molnar
` (10 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
This will also allow the simplification of patch_cmp().
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 27f1c7bdd384..5823d4764d92 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2493,7 +2493,7 @@ static __always_inline void put_text_poke_array(void)
raw_atomic_dec(refs);
}
-static __always_inline void *text_poke_addr(struct smp_text_poke_loc *tp)
+static __always_inline void *text_poke_addr(const struct smp_text_poke_loc *tp)
{
return _stext + tp->rel_addr;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 41/49] x86/alternatives: Simplify and clean up patch_cmp()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (39 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 40/49] x86/alternatives: Constify text_poke_addr() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 42/49] x86/alternatives: Standardize on 'tpl' local variable names for 'struct smp_text_poke_loc *' Ingo Molnar
` (9 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
- No need to cast over to 'struct smp_text_poke_loc *', void * is just fine
for a binary search,
- Use the canonical (a, b) input parameter nomenclature of cmp_func_t
functions and rename the input parameters from (tp, elt) to
(tpl_a, tpl_b).
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 5823d4764d92..d784738369d5 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2498,13 +2498,11 @@ static __always_inline void *text_poke_addr(const struct smp_text_poke_loc *tp)
return _stext + tp->rel_addr;
}
-static __always_inline int patch_cmp(const void *key, const void *elt)
+static __always_inline int patch_cmp(const void *tpl_a, const void *tpl_b)
{
- struct smp_text_poke_loc *tp = (struct smp_text_poke_loc *) elt;
-
- if (key < text_poke_addr(tp))
+ if (tpl_a < text_poke_addr(tpl_b))
return -1;
- if (key > text_poke_addr(tp))
+ if (tpl_a > text_poke_addr(tpl_b))
return 1;
return 0;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 42/49] x86/alternatives: Standardize on 'tpl' local variable names for 'struct smp_text_poke_loc *'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (40 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 41/49] x86/alternatives: Simplify and clean up patch_cmp() Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 43/49] x86/alternatives: Rename 'TP_ARRAY_NR_ENTRIES_MAX' to 'TEXT_POKE_ARRAY_MAX' Ingo Molnar
` (8 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
There's no toilet paper in this code.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 54 +++++++++++++++++++++++++++---------------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d784738369d5..62a6c69184c9 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2493,9 +2493,9 @@ static __always_inline void put_text_poke_array(void)
raw_atomic_dec(refs);
}
-static __always_inline void *text_poke_addr(const struct smp_text_poke_loc *tp)
+static __always_inline void *text_poke_addr(const struct smp_text_poke_loc *tpl)
{
- return _stext + tp->rel_addr;
+ return _stext + tpl->rel_addr;
}
static __always_inline int patch_cmp(const void *tpl_a, const void *tpl_b)
@@ -2509,7 +2509,7 @@ static __always_inline int patch_cmp(const void *tpl_a, const void *tpl_b)
noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
{
- struct smp_text_poke_loc *tp;
+ struct smp_text_poke_loc *tpl;
int ret = 0;
void *ip;
@@ -2538,20 +2538,20 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
* Skip the binary search if there is a single member in the vector.
*/
if (unlikely(text_poke_array.nr_entries > 1)) {
- tp = __inline_bsearch(ip, text_poke_array.vec, text_poke_array.nr_entries,
+ tpl = __inline_bsearch(ip, text_poke_array.vec, text_poke_array.nr_entries,
sizeof(struct smp_text_poke_loc),
patch_cmp);
- if (!tp)
+ if (!tpl)
goto out_put;
} else {
- tp = text_poke_array.vec;
- if (text_poke_addr(tp) != ip)
+ tpl = text_poke_array.vec;
+ if (text_poke_addr(tpl) != ip)
goto out_put;
}
- ip += tp->len;
+ ip += tpl->len;
- switch (tp->opcode) {
+ switch (tpl->opcode) {
case INT3_INSN_OPCODE:
/*
* Someone poked an explicit INT3, they'll want to handle it,
@@ -2564,16 +2564,16 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
break;
case CALL_INSN_OPCODE:
- int3_emulate_call(regs, (long)ip + tp->disp);
+ int3_emulate_call(regs, (long)ip + tpl->disp);
break;
case JMP32_INSN_OPCODE:
case JMP8_INSN_OPCODE:
- int3_emulate_jmp(regs, (long)ip + tp->disp);
+ int3_emulate_jmp(regs, (long)ip + tpl->disp);
break;
case 0x70 ... 0x7f: /* Jcc */
- int3_emulate_jcc(regs, tp->opcode & 0xf, (long)ip, tp->disp);
+ int3_emulate_jcc(regs, tpl->opcode & 0xf, (long)ip, tpl->disp);
break;
default:
@@ -2755,33 +2755,33 @@ static void smp_text_poke_batch_process(void)
static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
- struct smp_text_poke_loc *tp;
+ struct smp_text_poke_loc *tpl;
struct insn insn;
int ret, i = 0;
- tp = &text_poke_array.vec[text_poke_array.nr_entries++];
+ tpl = &text_poke_array.vec[text_poke_array.nr_entries++];
if (len == 6)
i = 1;
- memcpy((void *)tp->text, opcode+i, len-i);
+ memcpy((void *)tpl->text, opcode+i, len-i);
if (!emulate)
emulate = opcode;
ret = insn_decode_kernel(&insn, emulate);
BUG_ON(ret < 0);
- tp->rel_addr = addr - (void *)_stext;
- tp->len = len;
- tp->opcode = insn.opcode.bytes[0];
+ tpl->rel_addr = addr - (void *)_stext;
+ tpl->len = len;
+ tpl->opcode = insn.opcode.bytes[0];
if (is_jcc32(&insn)) {
/*
* Map Jcc.d32 onto Jcc.d8 and use len to distinguish.
*/
- tp->opcode = insn.opcode.bytes[1] - 0x10;
+ tpl->opcode = insn.opcode.bytes[1] - 0x10;
}
- switch (tp->opcode) {
+ switch (tpl->opcode) {
case RET_INSN_OPCODE:
case JMP32_INSN_OPCODE:
case JMP8_INSN_OPCODE:
@@ -2790,14 +2790,14 @@ static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len
* next instruction can be padded with INT3.
*/
for (i = insn.length; i < len; i++)
- BUG_ON(tp->text[i] != INT3_INSN_OPCODE);
+ BUG_ON(tpl->text[i] != INT3_INSN_OPCODE);
break;
default:
BUG_ON(len != insn.length);
}
- switch (tp->opcode) {
+ switch (tpl->opcode) {
case INT3_INSN_OPCODE:
case RET_INSN_OPCODE:
break;
@@ -2806,21 +2806,21 @@ static void __smp_text_poke_batch_add(void *addr, const void *opcode, size_t len
case JMP32_INSN_OPCODE:
case JMP8_INSN_OPCODE:
case 0x70 ... 0x7f: /* Jcc */
- tp->disp = insn.immediate.value;
+ tpl->disp = insn.immediate.value;
break;
default: /* assume NOP */
switch (len) {
case 2: /* NOP2 -- emulate as JMP8+0 */
BUG_ON(memcmp(emulate, x86_nops[len], len));
- tp->opcode = JMP8_INSN_OPCODE;
- tp->disp = 0;
+ tpl->opcode = JMP8_INSN_OPCODE;
+ tpl->disp = 0;
break;
case 5: /* NOP5 -- emulate as JMP32+0 */
BUG_ON(memcmp(emulate, x86_nops[len], len));
- tp->opcode = JMP32_INSN_OPCODE;
- tp->disp = 0;
+ tpl->opcode = JMP32_INSN_OPCODE;
+ tpl->disp = 0;
break;
default: /* unknown instruction */
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 43/49] x86/alternatives: Rename 'TP_ARRAY_NR_ENTRIES_MAX' to 'TEXT_POKE_ARRAY_MAX'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (41 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 42/49] x86/alternatives: Standardize on 'tpl' local variable names for 'struct smp_text_poke_loc *' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:26 ` [PATCH 44/49] x86/alternatives: Rename 'POKE_MAX_OPCODE_SIZE' to 'TEXT_POKE_MAX_OPCODE_SIZE' Ingo Molnar
` (7 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Standardize on TEXT_POKE_ namespace for CPP constants too.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 62a6c69184c9..04c536d36765 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2466,10 +2466,10 @@ struct smp_text_poke_loc {
u8 old;
};
-#define TP_ARRAY_NR_ENTRIES_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
+#define TEXT_POKE_ARRAY_MAX (PAGE_SIZE / sizeof(struct smp_text_poke_loc))
static struct smp_text_poke_array {
- struct smp_text_poke_loc vec[TP_ARRAY_NR_ENTRIES_MAX];
+ struct smp_text_poke_loc vec[TEXT_POKE_ARRAY_MAX];
int nr_entries;
} text_poke_array;
@@ -2863,7 +2863,7 @@ static void smp_text_poke_batch_flush(void *addr)
{
lockdep_assert_held(&text_mutex);
- if (text_poke_array.nr_entries == TP_ARRAY_NR_ENTRIES_MAX || !text_poke_addr_ordered(addr))
+ if (text_poke_array.nr_entries == TEXT_POKE_ARRAY_MAX || !text_poke_addr_ordered(addr))
smp_text_poke_batch_process();
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 44/49] x86/alternatives: Rename 'POKE_MAX_OPCODE_SIZE' to 'TEXT_POKE_MAX_OPCODE_SIZE'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (42 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 43/49] x86/alternatives: Rename 'TP_ARRAY_NR_ENTRIES_MAX' to 'TEXT_POKE_ARRAY_MAX' Ingo Molnar
@ 2025-03-28 13:26 ` Ingo Molnar
2025-03-28 13:27 ` [PATCH 45/49] x86/alternatives: Simplify the #include section Ingo Molnar
` (6 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:26 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Join the TEXT_POKE_ namespace.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 4 ++--
arch/x86/kernel/alternative.c | 6 +++---
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 6834e652d26c..53ac94743e5d 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -11,7 +11,7 @@
* JUMP_LABEL_NOP_SIZE/RELATIVEJUMP_SIZE, which are 5.
* Raise it if needed.
*/
-#define POKE_MAX_OPCODE_SIZE 5
+#define TEXT_POKE_MAX_OPCODE_SIZE 5
extern void text_poke_early(void *addr, const void *opcode, size_t len);
@@ -82,7 +82,7 @@ static __always_inline int text_opcode_size(u8 opcode)
}
union text_poke_insn {
- u8 text[POKE_MAX_OPCODE_SIZE];
+ u8 text[TEXT_POKE_MAX_OPCODE_SIZE];
struct {
u8 opcode;
s32 disp;
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 04c536d36765..d724ae8913da 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2461,7 +2461,7 @@ struct smp_text_poke_loc {
s32 disp;
u8 len;
u8 opcode;
- const u8 text[POKE_MAX_OPCODE_SIZE];
+ const u8 text[TEXT_POKE_MAX_OPCODE_SIZE];
/* see smp_text_poke_batch_process() */
u8 old;
};
@@ -2653,8 +2653,8 @@ static void smp_text_poke_batch_process(void)
* Second step: update all but the first byte of the patched range.
*/
for (do_sync = 0, i = 0; i < text_poke_array.nr_entries; i++) {
- u8 old[POKE_MAX_OPCODE_SIZE+1] = { text_poke_array.vec[i].old, };
- u8 _new[POKE_MAX_OPCODE_SIZE+1];
+ u8 old[TEXT_POKE_MAX_OPCODE_SIZE+1] = { text_poke_array.vec[i].old, };
+ u8 _new[TEXT_POKE_MAX_OPCODE_SIZE+1];
const u8 *new = text_poke_array.vec[i].text;
int len = text_poke_array.vec[i].len;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 45/49] x86/alternatives: Simplify the #include section
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (43 preceding siblings ...)
2025-03-28 13:26 ` [PATCH 44/49] x86/alternatives: Rename 'POKE_MAX_OPCODE_SIZE' to 'TEXT_POKE_MAX_OPCODE_SIZE' Ingo Molnar
@ 2025-03-28 13:27 ` Ingo Molnar
2025-03-28 13:27 ` [PATCH 46/49] x86/alternatives: Move declarations of vmlinux.lds.S defined section symbols to <asm/alternative.h> Ingo Molnar
` (5 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
We accumulated lots of unnecessary header inclusions over the years,
trim them.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index d724ae8913da..242c0b1f7e40 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1,36 +1,14 @@
// SPDX-License-Identifier: GPL-2.0-only
#define pr_fmt(fmt) "SMP alternatives: " fmt
-#include <linux/module.h>
-#include <linux/sched.h>
+#include <linux/mmu_context.h>
#include <linux/perf_event.h>
-#include <linux/mutex.h>
-#include <linux/list.h>
-#include <linux/stringify.h>
-#include <linux/highmem.h>
-#include <linux/mm.h>
#include <linux/vmalloc.h>
#include <linux/memory.h>
-#include <linux/stop_machine.h>
-#include <linux/slab.h>
-#include <linux/kdebug.h>
-#include <linux/kprobes.h>
-#include <linux/mmu_context.h>
-#include <linux/bsearch.h>
-#include <linux/sync_core.h>
+
#include <asm/text-patching.h>
-#include <asm/alternative.h>
-#include <asm/sections.h>
-#include <asm/mce.h>
-#include <asm/nmi.h>
-#include <asm/cacheflush.h>
-#include <asm/tlbflush.h>
#include <asm/insn.h>
-#include <asm/io.h>
-#include <asm/fixmap.h>
-#include <asm/paravirt.h>
-#include <asm/asm-prototypes.h>
-#include <asm/cfi.h>
+#include <asm/nmi.h>
int __read_mostly alternatives_patched;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 46/49] x86/alternatives: Move declarations of vmlinux.lds.S defined section symbols to <asm/alternative.h>
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (44 preceding siblings ...)
2025-03-28 13:27 ` [PATCH 45/49] x86/alternatives: Simplify the #include section Ingo Molnar
@ 2025-03-28 13:27 ` Ingo Molnar
2025-03-28 13:27 ` [PATCH 47/49] x86/alternatives: Remove 'smp_text_poke_batch_flush()' Ingo Molnar
` (4 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Move it from the middle of a .c file next to the similar declarations
of __alt_instructions[] et al.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/alternative.h | 6 ++++++
arch/x86/kernel/alternative.c | 6 ------
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 4a37a8bd87fd..ef84739a77f5 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -82,6 +82,12 @@ struct alt_instr {
extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
+extern s32 __retpoline_sites[], __retpoline_sites_end[];
+extern s32 __return_sites[], __return_sites_end[];
+extern s32 __cfi_sites[], __cfi_sites_end[];
+extern s32 __ibt_endbr_seal[], __ibt_endbr_seal_end[];
+extern s32 __smp_locks[], __smp_locks_end[];
+
/*
* Debug flag that can be tested to see whether alternative
* instructions were patched in already:
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 242c0b1f7e40..df718a4e88c2 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -149,12 +149,6 @@ static void add_nop(u8 *buf, unsigned int len)
*buf = INT3_INSN_OPCODE;
}
-extern s32 __retpoline_sites[], __retpoline_sites_end[];
-extern s32 __return_sites[], __return_sites_end[];
-extern s32 __cfi_sites[], __cfi_sites_end[];
-extern s32 __ibt_endbr_seal[], __ibt_endbr_seal_end[];
-extern s32 __smp_locks[], __smp_locks_end[];
-
/*
* Matches NOP and NOPL, not any of the other possible NOPs.
*/
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 47/49] x86/alternatives: Remove 'smp_text_poke_batch_flush()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (45 preceding siblings ...)
2025-03-28 13:27 ` [PATCH 46/49] x86/alternatives: Move declarations of vmlinux.lds.S defined section symbols to <asm/alternative.h> Ingo Molnar
@ 2025-03-28 13:27 ` Ingo Molnar
2025-03-28 13:27 ` [PATCH 48/49] x86/alternatives: Update the comments in smp_text_poke_batch_process() Ingo Molnar
` (3 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
It only has a single user left, merge it into smp_text_poke_batch_add()
and remove the helper function.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index df718a4e88c2..6de97646b4d6 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2831,14 +2831,6 @@ void smp_text_poke_batch_finish(void)
smp_text_poke_batch_process();
}
-static void smp_text_poke_batch_flush(void *addr)
-{
- lockdep_assert_held(&text_mutex);
-
- if (text_poke_array.nr_entries == TEXT_POKE_ARRAY_MAX || !text_poke_addr_ordered(addr))
- smp_text_poke_batch_process();
-}
-
/**
* smp_text_poke_batch_add() -- update instruction on live kernel on SMP, batched
* @addr: address to patch
@@ -2854,7 +2846,8 @@ static void smp_text_poke_batch_flush(void *addr)
*/
void __ref smp_text_poke_batch_add(void *addr, const void *opcode, size_t len, const void *emulate)
{
- smp_text_poke_batch_flush(addr);
+ if (text_poke_array.nr_entries == TEXT_POKE_ARRAY_MAX || !text_poke_addr_ordered(addr))
+ smp_text_poke_batch_process();
__smp_text_poke_batch_add(addr, opcode, len, emulate);
}
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 48/49] x86/alternatives: Update the comments in smp_text_poke_batch_process()
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (46 preceding siblings ...)
2025-03-28 13:27 ` [PATCH 47/49] x86/alternatives: Remove 'smp_text_poke_batch_flush()' Ingo Molnar
@ 2025-03-28 13:27 ` Ingo Molnar
2025-03-28 13:27 ` [PATCH 49/49] x86/alternatives: Rename 'apply_relocation()' to 'text_poke_apply_relocation()' Ingo Molnar
` (2 subsequent siblings)
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
- Capitalize 'INT3' consistently,
- make it clear that 'sync cores' means an SMP sync to all CPUs,
- fix typos and spelling.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/kernel/alternative.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 6de97646b4d6..a855db342c6a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -2561,24 +2561,24 @@ noinstr int smp_text_poke_int3_trap_handler(struct pt_regs *regs)
/**
* smp_text_poke_batch_process() -- update instructions on live kernel on SMP
- * @text_poke_array.vec: vector of instructions to patch
+ * @text_poke_array.vec: vector of instructions to patch
* @text_poke_array.nr_entries: number of entries in the vector
*
- * Modify multi-byte instruction by using int3 breakpoint on SMP.
- * We completely avoid stop_machine() here, and achieve the
- * synchronization using int3 breakpoint.
+ * Modify multi-byte instructions by using INT3 breakpoints on SMP.
+ * We completely avoid using stop_machine() here, and achieve the
+ * synchronization using INT3 breakpoints and SMP cross-calls.
*
* The way it is done:
* - For each entry in the vector:
- * - add a int3 trap to the address that will be patched
- * - sync cores
+ * - add an INT3 trap to the address that will be patched
+ * - SMP sync all CPUs
* - For each entry in the vector:
* - update all but the first byte of the patched range
- * - sync cores
+ * - SMP sync all CPUs
* - For each entry in the vector:
- * - replace the first byte (int3) by the first byte of
+ * - replace the first byte (INT3) by the first byte of the
* replacing opcode
- * - sync cores
+ * - SMP sync all CPUs
*/
static void smp_text_poke_batch_process(void)
{
@@ -2606,13 +2606,13 @@ static void smp_text_poke_batch_process(void)
cond_resched();
/*
- * Corresponding read barrier in int3 notifier for making sure the
+ * Corresponding read barrier in INT3 notifier for making sure the
* text_poke_array.nr_entries and handler are correctly ordered wrt. patching.
*/
smp_wmb();
/*
- * First step: add a int3 trap to the address that will be patched.
+ * First step: add a INT3 trap to the address that will be patched.
*/
for (i = 0; i < text_poke_array.nr_entries; i++) {
text_poke_array.vec[i].old = *(u8 *)text_poke_addr(&text_poke_array.vec[i]);
@@ -2685,7 +2685,7 @@ static void smp_text_poke_batch_process(void)
}
/*
- * Third step: replace the first byte (int3) by the first byte of
+ * Third step: replace the first byte (INT3) by the first byte of the
* replacing opcode.
*/
for (do_sync = 0, i = 0; i < text_poke_array.nr_entries; i++) {
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* [PATCH 49/49] x86/alternatives: Rename 'apply_relocation()' to 'text_poke_apply_relocation()'
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (47 preceding siblings ...)
2025-03-28 13:27 ` [PATCH 48/49] x86/alternatives: Update the comments in smp_text_poke_batch_process() Ingo Molnar
@ 2025-03-28 13:27 ` Ingo Molnar
2025-04-01 14:31 ` [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Peter Zijlstra
2025-04-01 14:31 ` Peter Zijlstra
50 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-03-28 13:27 UTC (permalink / raw)
To: linux-kernel
Cc: Juergen Gross, H . Peter Anvin, Linus Torvalds, Peter Zijlstra,
Borislav Petkov, Thomas Gleixner, Ingo Molnar
Join the text_poke_*() API namespace.
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/x86/include/asm/text-patching.h | 2 +-
arch/x86/kernel/alternative.c | 6 +++---
arch/x86/kernel/callthunks.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index 53ac94743e5d..335f88b54ed0 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -15,7 +15,7 @@
extern void text_poke_early(void *addr, const void *opcode, size_t len);
-extern void apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u8 *repl, size_t repl_len);
+extern void text_poke_apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u8 *repl, size_t repl_len);
/*
* Clear and restore the kernel write-protection flag on the local CPU.
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index a855db342c6a..cd5b69b6eff8 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -340,7 +340,7 @@ static void __apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen,
}
}
-void apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u8 *repl, size_t repl_len)
+void text_poke_apply_relocation(u8 *buf, const u8 * const instr, size_t instrlen, u8 *repl, size_t repl_len)
{
__apply_relocation(buf, instr, instrlen, repl, repl_len);
optimize_nops(instr, buf, instrlen);
@@ -496,7 +496,7 @@ void __init_or_module noinline apply_alternatives(struct alt_instr *start,
for (; insn_buff_sz < a->instrlen; insn_buff_sz++)
insn_buff[insn_buff_sz] = 0x90;
- apply_relocation(insn_buff, instr, a->instrlen, replacement, a->replacementlen);
+ text_poke_apply_relocation(insn_buff, instr, a->instrlen, replacement, a->replacementlen);
DUMP_BYTES(ALT, instr, a->instrlen, "%px: old_insn: ", instr);
DUMP_BYTES(ALT, replacement, a->replacementlen, "%px: rpl_insn: ", replacement);
@@ -1981,7 +1981,7 @@ __visible noinline void __init __alt_reloc_selftest(void *arg)
static noinline void __init alt_reloc_selftest(void)
{
/*
- * Tests apply_relocation().
+ * Tests text_poke_apply_relocation().
*
* This has a relative immediate (CALL) in a place other than the first
* instruction and additionally on x86_64 we get a RIP-relative LEA:
diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index 25ae54250112..e016c5780d42 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -186,7 +186,7 @@ static void *patch_dest(void *dest, bool direct)
u8 *pad = dest - tsize;
memcpy(insn_buff, skl_call_thunk_template, tsize);
- apply_relocation(insn_buff, pad, tsize, skl_call_thunk_template, tsize);
+ text_poke_apply_relocation(insn_buff, pad, tsize, skl_call_thunk_template, tsize);
/* Already patched? */
if (!bcmp(pad, insn_buff, tsize))
@@ -295,7 +295,7 @@ static bool is_callthunk(void *addr)
pad = (void *)(dest - tmpl_size);
memcpy(insn_buff, skl_call_thunk_template, tmpl_size);
- apply_relocation(insn_buff, pad, tmpl_size, skl_call_thunk_template, tmpl_size);
+ text_poke_apply_relocation(insn_buff, pad, tmpl_size, skl_call_thunk_template, tmpl_size);
return !bcmp(pad, insn_buff, tmpl_size);
}
@@ -313,7 +313,7 @@ int x86_call_depth_emit_accounting(u8 **pprog, void *func, void *ip)
return 0;
memcpy(insn_buff, skl_call_thunk_template, tmpl_size);
- apply_relocation(insn_buff, ip, tmpl_size, skl_call_thunk_template, tmpl_size);
+ text_poke_apply_relocation(insn_buff, ip, tmpl_size, skl_call_thunk_template, tmpl_size);
memcpy(*pprog, insn_buff, tmpl_size);
*pprog += tmpl_size;
--
2.45.2
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c)
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (48 preceding siblings ...)
2025-03-28 13:27 ` [PATCH 49/49] x86/alternatives: Rename 'apply_relocation()' to 'text_poke_apply_relocation()' Ingo Molnar
@ 2025-04-01 14:31 ` Peter Zijlstra
2025-04-09 20:51 ` Ingo Molnar
2025-04-01 14:31 ` Peter Zijlstra
50 siblings, 1 reply; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-01 14:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Fri, Mar 28, 2025 at 02:26:15PM +0100, Ingo Molnar wrote:
> This series has 3 main parts:
>
> (1)
>
> The first part of this series performs a thorough text-patching API namespace
> cleanup discussed with Linus for the -v1 series:
>
> # boot/UP APIs & single-thread helpers:
>
> text_poke()
> text_poke_kgdb()
> [ unchanged APIs: ] text_poke_copy()
> text_poke_copy_locked()
> text_poke_set()
>
> text_poke_addr()
>
> # SMP API & helpers namespace:
>
> text_poke_bp() => smp_text_poke_single()
> text_poke_loc_init() => __smp_text_poke_batch_add()
> text_poke_queue() => smp_text_poke_batch_add()
> text_poke_finish() => smp_text_poke_batch_finish()
>
> text_poke_flush() => [removed]
>
> text_poke_bp_batch() => smp_text_poke_batch_process()
> poke_int3_handler() => smp_text_poke_int3_trap_handler()
> text_poke_sync() => smp_text_poke_sync_each_cpu()
>
Not sure I like that; smp_text_poke_ is a bit of a mouth full, esp. if
you're then adding even more text.
Do we really need function names this long?
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c)
2025-04-01 14:31 ` [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Peter Zijlstra
@ 2025-04-09 20:51 ` Ingo Molnar
2025-04-10 12:57 ` Peter Zijlstra
0 siblings, 1 reply; 69+ messages in thread
From: Ingo Molnar @ 2025-04-09 20:51 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 28, 2025 at 02:26:15PM +0100, Ingo Molnar wrote:
> > This series has 3 main parts:
> >
> > (1)
> >
> > The first part of this series performs a thorough text-patching API namespace
> > cleanup discussed with Linus for the -v1 series:
> >
> > # boot/UP APIs & single-thread helpers:
> >
> > text_poke()
> > text_poke_kgdb()
> > [ unchanged APIs: ] text_poke_copy()
> > text_poke_copy_locked()
> > text_poke_set()
> >
> > text_poke_addr()
> >
> > # SMP API & helpers namespace:
> >
> > text_poke_bp() => smp_text_poke_single()
> > text_poke_loc_init() => __smp_text_poke_batch_add()
> > text_poke_queue() => smp_text_poke_batch_add()
> > text_poke_finish() => smp_text_poke_batch_finish()
> >
> > text_poke_flush() => [removed]
> >
> > text_poke_bp_batch() => smp_text_poke_batch_process()
> > poke_int3_handler() => smp_text_poke_int3_trap_handler()
> > text_poke_sync() => smp_text_poke_sync_each_cpu()
> >
>
> Not sure I like that; smp_text_poke_ is a bit of a mouth full, esp. if
> you're then adding even more text.
>
> Do we really need function names this long?
So they are still shorter than:
perf_scope_cpu_topology_cpumask()
perf_swevent_put_recursion_context()
perf_event_max_sample_rate_handler()
perf_unregister_guest_info_callbacks()
;-)
I think we could trim the longest one via:
s/smp_text_poke_int3_trap_handler
/smp_text_poke_int3_handler
Because 'INT3 handler' is more than specific enough?
But in general, function name length is less critical for 'complex',
non-library APIs that are called in a pretty flat fashion, especially
if they have no error returns.
Here's how they are used today, after the rename:
smp_text_poke_sync_each_cpu();
smp_text_poke_sync_each_cpu();
smp_text_poke_sync_each_cpu();
smp_text_poke_batch_finish();
__smp_text_poke_batch_add(addr, opcode, len, emulate);
__smp_text_poke_batch_add(addr, opcode, len, emulate);
smp_text_poke_batch_finish();
smp_text_poke_batch_finish();
smp_text_poke_batch_add((void *)ip, new_code, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_batch_add((void *)rec->ip, new, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_batch_finish();
smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_single((void *)ip, new, MCOUNT_INSN_SIZE, NULL);
smp_text_poke_single((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
smp_text_poke_batch_add((void *)jump_entry_code(entry), jlp.code, jlp.size, NULL);
smp_text_poke_batch_finish();
smp_text_poke_sync_each_cpu();
smp_text_poke_sync_each_cpu();
smp_text_poke_single(op->kp.addr, insn_buff, JMP32_INSN_SIZE, NULL);
smp_text_poke_sync_each_cpu();
smp_text_poke_sync_each_cpu();
smp_text_poke_sync_each_cpu();
smp_text_poke_single(insn, code, size, emulate);
smp_text_poke_single(ip, new_insn, X86_PATCH_SIZE, NULL);
Note how there's no error return, no conditionals, just flat calls.
And note how easy it was to do a 'git grep smp_text_poke_' to get such
an overview. ;-)
Anyway, any other suggestions for shorter names, or can I proceed with
these plus the above shortening of the trap handler name?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c)
2025-04-09 20:51 ` Ingo Molnar
@ 2025-04-10 12:57 ` Peter Zijlstra
0 siblings, 0 replies; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-10 12:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Wed, Apr 09, 2025 at 10:51:18PM +0200, Ingo Molnar wrote:
> Anyway, any other suggestions for shorter names, or can I proceed with
> these plus the above shortening of the trap handler name?
Sure; I suppose I can always rename some later if I get really annoyed
:-)
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c)
2025-03-28 13:26 [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Ingo Molnar
` (49 preceding siblings ...)
2025-04-01 14:31 ` [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c) Peter Zijlstra
@ 2025-04-01 14:31 ` Peter Zijlstra
2025-04-02 3:47 ` Ingo Molnar
50 siblings, 1 reply; 69+ messages in thread
From: Peter Zijlstra @ 2025-04-01 14:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
On Fri, Mar 28, 2025 at 02:26:15PM +0100, Ingo Molnar wrote:
> This tree can also be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip WIP.x86/alternatives
Can you please use your own tree for WIP stuff?
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: [PATCH -v2 00/49] Simplify, reorganize and clean up the x86 text-patching code (alternative.c)
2025-04-01 14:31 ` Peter Zijlstra
@ 2025-04-02 3:47 ` Ingo Molnar
0 siblings, 0 replies; 69+ messages in thread
From: Ingo Molnar @ 2025-04-02 3:47 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, Juergen Gross, H . Peter Anvin, Linus Torvalds,
Borislav Petkov, Thomas Gleixner
* Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Mar 28, 2025 at 02:26:15PM +0100, Ingo Molnar wrote:
> > This tree can also be found at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip WIP.x86/alternatives
>
> Can you please use your own tree for WIP stuff?
Certainly! Wanted to do that for some time anyway, to reduce mixups
with -tip, but procrastinated it :-)
I have just moved all of my Work-In-Progress branches to:
git://git.kernel.org/pub/scm/linux/kernel/git/mingo/tip.git
WIP.x86/alternatives
WIP.x86/msr
WIP.x86/core
WIP.x86/cpu
WIP.x86/fpu
WIP.core/bugs
...
Thanks,
Ingo
^ permalink raw reply [flat|nested] 69+ messages in thread