xen/x86/alternatives: Do not use sync_core() to serialize I$
authorBorislav Petkov <bp@suse.de>
Sat, 3 Dec 2016 15:02:58 +0000 (16:02 +0100)
committerAndrew Cooper <andrew.cooper3@citrix.com>
Wed, 31 May 2017 16:54:19 +0000 (17:54 +0100)
We use sync_core() in the alternatives code to stop speculative
execution of prefetched instructions because we are potentially changing
them and don't want to execute stale bytes.

What it does on most machines is call CPUID which is a serializing
instruction. And that's expensive.

However, the instruction cache is serialized when we're on the local CPU
and are changing the data through the same virtual address. So then, we
don't need the serializing CPUID but a simple control flow change. Last
being accomplished with a CALL/RET which the noinline causes.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Andy Lutomirski <luto@kernel.org>
[Linux commit 34bfab0eaf0fb5c6fb14c6b4013b06cdc7984466]

Ported to Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
xen/arch/x86/alternative.c
xen/arch/x86/livepatch.c

index 6eaa10f858e57d95accfb59e342639086bbd861c..65062c2a094f6639962e423aff9e7ff7d415a470 100644 (file)
@@ -128,13 +128,14 @@ void init_or_livepatch add_nops(void *insns, unsigned int len)
  *
  * You should run this with interrupts disabled or on code that is not
  * executing.
+ *
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
  */
-static void *init_or_livepatch text_poke(void *addr, const void *opcode, size_t len)
+static void *init_or_livepatch noinline
+text_poke(void *addr, const void *opcode, size_t len)
 {
-    memcpy(addr, opcode, len);
-    sync_core();
-
-    return addr;
+    return memcpy(addr, opcode, len);
 }
 
 /*
index 9663ef6fef6b860f6882c998e52c4d9d4a252e95..dd50dd1004efed7043ba01fd837eebb0943dd4a0 100644 (file)
@@ -46,7 +46,11 @@ int arch_livepatch_verify_func(const struct livepatch_func *func)
     return 0;
 }
 
-void arch_livepatch_apply(struct livepatch_func *func)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_apply(struct livepatch_func *func)
 {
     uint8_t *old_ptr;
     uint8_t insn[sizeof(func->opaque)];
@@ -75,15 +79,21 @@ void arch_livepatch_apply(struct livepatch_func *func)
     memcpy(old_ptr, insn, len);
 }
 
-void arch_livepatch_revert(const struct livepatch_func *func)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_revert(const struct livepatch_func *func)
 {
     memcpy(func->old_addr, func->opaque, livepatch_insn_len(func));
 }
 
-/* Serialise the CPU pipeline. */
-void arch_livepatch_post_action(void)
+/*
+ * "noinline" to cause control flow change and thus invalidate I$ and
+ * cause refetch after modification.
+ */
+void noinline arch_livepatch_post_action(void)
 {
-    cpuid_eax(0);
 }
 
 static nmi_callback_t *saved_nmi_callback;