x86: stop machine text poke should issue sync core

Intel Archiecture Software Developer's Manual section 7.1.3 specifies that a core serializing instruction such as "cpuid" should be executed on _each_ core before the new instruction is made visible.

Failure to do so can lead to unspecified behavior (Intel XMC erratas include General Protection Fault in the list), so we should avoid this at all cost.

This problem can affect modified code executed by interrupt handlers after interrupt are re-enabled at the end of stop_machine, because no core serializing instruction is executed between the code modification and the moment interrupts are reenabled.

Because stop_machine_text_poke performs the text modification from the first CPU decrementing stop_machine_first, modified code executed in thread context is also affected by this problem. To explain why, we have to split the CPUs in two
categories: the CPU that initiates the text modification (calls text_poke_smp) and all the others. The scheduler, executed on all other CPUs after stop_machine, issues an "iret" core serializing instruction, and therefore handles core serialization for all these CPUs. However, the text modification initiator can continue its execution on the same thread and access the modified text without any scheduler call. Given that the CPU that initiates the code modification is not guaranteed to be the one actually performing the code modification, it falls into the XMC errata.

Signed-off-by Mathieu Desnoyers <[email protected]>

CC Peter Zijlstra <[email protected]>

CC Arjan van de Ven <[email protected]>

CC "H. Peter Anvin" <[email protected]>

CC Thomas Gleixner <[email protected]>

CC Steven Rostedt <[email protected]>

CC Ingo Molnar <[email protected]>

CC Andrew Morton <[email protected]>

CC Andi Kleen <[email protected]>

CC Frederic Weisbecker <[email protected]>

CC Masami Hiramatsu <[email protected]>

---
 arch/x86/kernel/alternative.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-2.6-lttng/arch/x86/kernel/alternative.c
===================================================================

arch/x86/kernel/alternative.c +6 -0

@@ -612,6 +612,12 @@ static int __kprobes stop_machine_text_p
 
 	flush_icache_range((unsigned long)tpp->addr,
 			   (unsigned long)tpp->addr + tpp->len);
+	/*
+	 * Intel Archiecture Software Developer's Manual section 7.1.3 specifies
+	 * that a core serializing instruction such as "cpuid" should be
+	 * executed on _each_ core before the new instruction is made visible.
+	 */
+	sync_core();
 	return 0;
 }
 

Index: linux-2.6-lttng/arch/x86/kernel/alternative.c
===================================================================

arch/x86/kernel/alternative.c +0 -0

@@ -612,6 +612,12 @@ static int __kprobes stop_machine_text_p
 
 	flush_icache_range((unsigned long)tpp->addr,

Quoted 7 lines · Mathieu Desnoyers

Index: linux-2.6-lttng/arch/x86/kernel/alternative.c
===================================================================

arch/x86/kernel/alternative.c +0 -0

@@ -612,6 +612,12 @@ static int __kprobes stop_machine_text_p
 

arch/x86/kernel/alternative.c static int __kprobes stop_machine_text_p · line 623

Quoted 11 lines

arch/x86/kernel/alternative.c +6 -0

 	flush_icache_range((unsigned long)tpp->addr,
 			   (unsigned long)tpp->addr + tpp->len);
+	/*
+	 * Intel Archiecture Software Developer's Manual section 7.1.3 specifies
+	 * that a core serializing instruction such as "cpuid" should be
+	 * executed on _each_ core before the new instruction is made visible.
+	 */
+	sync_core();
 	return 0;
 }
 

Isn't this executed from an IPI handler, which will return with IRET (a serializing instruction) anyway?

Quoted 8 lines · H. Peter Anvin

Quoted 7 lines · Mathieu Desnoyers

Index: linux-2.6-lttng/arch/x86/kernel/alternative.c
===================================================================

arch/x86/kernel/alternative.c +0 -0

@@ -612,6 +612,12 @@ static int __kprobes stop_machine_text_p
 

Quoted 14 lines

arch/x86/kernel/alternative.c static int __kprobes stop_machine_text_p · line 623

Quoted 11 lines

arch/x86/kernel/alternative.c +6 -0

 	flush_icache_range((unsigned long)tpp->addr,
 			   (unsigned long)tpp->addr + tpp->len);
+	/*
+	 * Intel Archiecture Software Developer's Manual section 7.1.3 specifies
+	 * that a core serializing instruction such as "cpuid" should be
+	 * executed on _each_ core before the new instruction is made visible.
+	 */
+	sync_core();
 	return 0;
 }
 

Isn't this executed from an IPI handler, which will return with IRET (a serializing instruction) anyway?

No, now stop_machine uses per-cpu workqueue, so that handler will be executed from worker threads. There is no iret anymore.

BTW, Mathieu, since the latest kernel has batch-text_poke_smp, this patch needs to be updated.

Thank you,

Signature · 4 lines

Masami HIRAMATSU
2nd Dept. Linux Technology Center
Hitachi, Ltd., Systems Development Laboratory
E-mail: [email protected]